Done @zero-24 and thank you

I have just tested this and found two issue. In case there is a validation error $data is no stdClass but just an empty array. And the second is what happen when the plugin is disabled. Currently the password field is not required. But it needs to be required than.

How to reproduce (First Issue)

• set error reporting to max
• create a user with the username test
• try to create another user with the same username
• error pops up.
• when you debug you notice that the $data array is empty.

How to reproduce (Second Issue)

• Disable the plugin
• try to create a user
• pw field is not required.

How to solve the issues

• change default in com_users XML file that the pw's are always required
• restructure the plugin check so it disable the required status if running.

Disable the plugin

@zero-24 if you disabled the plugin, others things like the login logic will break, among other things. In fact, there is a string warning about this. In case this plugin is disabled because another plugin will handle User synchronization, then we no need more the "Notification Mail to User", and we no need check for this.

try to create another user with the same username

We can add an is_object() check inside the condition for avoiding this

I do not understand why the drone fail here?

In case this plugin is disabled because another plugin will handle User synchronization, then we no need more the "Notification Mail to User", and we no need check for this.

Well my point is that the password fields are than not required in that case ;)
So my suggestion was to change the logic so the fields are required allways but not required with the setting is enabeld ;)

But I'm also fine merge this without that change and when this got accepted i do a PR that would handle the proposed solution.

@Quy can you give me a hand here? This code is marked as a conflict for the merge, was moved to the plugin for solved the issue. Or maybe I understand wrong this.

I think you have to update your branch since there were changes to administrator/components/com_users/models/user.php 4 days ago.

Thank you @Quy

Thank you @SharkyKZ

Create a duplicate user.
An error message is displayed.
Password and Confirm Password are no longer marked required.

#14851 and #20313 could be related to this issue.

Data from user state is an array https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_users/models/user.php#L172.

This seems to be the case everywhere else.

I made what @SharkyKZ suggest before:

change default in com_users XML file that the pw's are always required
restructure the plugin check so it disable the required status if running.

Please, can you test again @Quy . Also, as the password field is required by default, we need to remove this, when we edit a user.

PD: Sorry for the delay

On the backend, Password and Confirm Password are not required (right). After an error, these fields are marked required (wrong).

On the backend, Password and Confirm Password are not required (right)

So, change the pw´s fields required by default is wrong?

On the backend, leave the password blank for the system to auto generate the password. Without PR, the 2 password fields are not required. With PR, the password fields are not required initially, but after a duplicate error, they are marked required.

@Quy now I got it. Sorry for be slow, sometime my English is not so good as I want it.
So, with this PR and plugin User - Joomla, with the Notification Mail to User set on No passwords fields are required (Correct operation). But if you save a duplicate user, then passwords fields after get the error, are not required (Wrong operation, need stay as required).

Checking this, somehow $options['load_data'] come with the value false when you save a duplicate user,

Youre English fine to me. ?

Please read the following about password on the backend: #20247 (comment)

I was wrong with my report. I think your PR is fine, but I will have to test again to be sure.

Please read the following about password on the backend

Thank you

The issue is that on validation error returned data is an array. This could probably be fixed in core, but for now you can remove is_object() check and cast data as object.

Tested @ 9e333a9

PHP Notice: Undefined property: stdClass::$id in \plugins\user\joomla\joomla.php on line 58

@Quy how do you get this notice?

Check in the PHP error log file.

FormController never loads form data so whatever form data check is performed in plugin is not valid when saving.

Thank you @SharkyKZ and @Quy .

Sorry I gave you wrong information prior. Here is the right information:

On the back end, when Notification Mail to User is No, then Password and Confirm Password should be marked with * and be required.

Your PR, however, after an error, these 2 fields are no longer marked with *.

This isn't right yet. On form display, $data is always populated. It's normally an object but an array after returning from error. Data from input ($jformData) is not available on form display.

On save, $data is an empty array because FormController does not load data. So on save you need to use data from input ($jformData).

This is indeed #14851.

The same issue exists without PR. When Notification Mail to User is set to No password fields appear to be required on form display but aren't validated on save. To test this, remove required attribute and class from markup using browser's developer tools and submit the form without passwords. The user will be saved.

The real fix is try to handle this from the model or the controller. But I can bet will be B/C issue.

Your PR, however, after an error, these 2 fields are no longer marked with *

Is true, I do not see this. However, if you try to save again, you get a warning telling you, the password field is required.

@Quy I does not understand why, when we get the error the * gone. Even if you save, you got the warning saying the password is required, but no put the field as required in the HTML markup. Can you give me a hand with this? I try to find where I miss something, but no luck for now

It's because of the check in the plugin. Replace this:

if (is_array($data) && $jformData)
{
	$data = (object) $jformData;
}

With this:

if ($jformData && !$data) 
 { 
 	$data = $jformData; 
 } 
  
if (is_array($data)) 
{ 
 	$data = (object) $data; 
} 

Thank you @SharkyKZ , that was But or I need close all here and go to watch a movie, or is almost the same I had in the check before :D . Anyway, thank you for the help

Done, thank @Quy

Related issue #17700 about empty $data.

This is the correct way then to do this or need a different way?

The code works fine now.

Still thinking about @zero-24's suggestion to make password fields required by default. It would make sense not to auto-generate passwords when plugin is disabled, when mail to user option in plugin is disabled or when sending passwords is disabled in com_users.

Ok, let do the suggestion of @zero-24

It never makes sense in the admin to make creating a password required.

It never makes sense in the admin to make creating a password required.

For creating a new account, I'd say it would make sense to default it to required and conditionally unrequire it based on the other settings. Otherwise you're left in a spot where admin creates an account without setting a password and the new user's only way of getting into the account is through the "forgot my password" interface. I don't think that's necessarily the best new user experience, but we all know what opinions are like.

This is a valid argument imo #20247 (comment)

@carlitorweb are the Test Instructions up-to-date?

@fran so far, yes

"Notification Mail to User"-option is "No", both Password-Fields have a "*". I'm confused about the Test Instructions.

Also "Expected" and "Actual Result" have the same Description.

@franz-wohlkoenig Behavior before and after patch should be the same.

I have tested this item ✅ successfully on fe03395

I have tested this item ✅ successfully on b633bbf

RTC

RTC

Thanks