Create new order fails if "New" customer isn't actually new.

+      // The mail is used for both the new user's name and mail. Only the name
+      // must be unique.
+      if (user_load_by_name($values['mail'])) {
+        $form_state->setError($customer_form, 'A user with this name exists already.');
+      }

While name/mail fields are both set to mail value in the Commerce form, that is not guaranteed to be true if the User was created a different way (such as the standard User forms). I tested the patch on my site and I was able to create a duplicate user. Here is a patch that includes a check for users that have the same email address. I also updated the error text to match the message given by core when validating for unique email on User.

Adding related issue from Core regarding base field constraints.

This problem still exists today, closing https://www.drupal.org/project/commerce/issues/3166998 as a duplicate of this, but I was using the patch there in production for 2 months without any problems.

The patch in #4 here triggers some deprecation warnings in D9 and just checks the mail, I added some changes from the patch I wrote in 3166998 to fix the warnings and do a more generic validation with $user->validate(), and kept the test that should still apply and the usage of #element_validate that is better than what I did there.

If someone can review this I would much appreciate it.

Can someone please review this?

#5 works for me.

This problem seems to exist like... for ever, to the point actually that we have all gotten used to it.
I believe this is good to go and will be changing the status to rtbc.

Hello, what is needed to have this committed?

Instead of using an #element_validate on the fieldset, let's just implement the validateForm method in OrderAddForm and OrderReassignForm.

And just call:

    $this->validateCustomerForm($form, $form_state);

Also, the other methods aren't static, so let's mimic that.

Regarding the functional javascript test, let's reuse OrderAdminTest (Either by updating testCreateOrder), or just add an additional method there (e.g testCreateOrderNewCustomer()), it's probably easier/faster to introduce a new method.

Let's also skip createUserObject. After validating the user, you can put it in form state, and retrieve it from submitForm.

So:

$form_state->set('customer', $user)... 

And from submitCustomerForm do $form_state->get('customer');.

@ Rishi Kulshreshtha: Any reason to assign this to you? @marchuk.vitaliy opened a PR that has passing tests?

@jsacksick, actually, I found the status in needs work & hence thought of checking the same. I've tried to implement the suggestions provided by you above.

I didn't actually notice the MR in the first place as the ticket already contained some patches, my bad. But, after your message I reviewed the MR provided by @marchuk.vitaliy seems to tackle all your suggestions as well, but I can see there the use of waitForAjaxToFinish() which is already deprecated & I've tried to create the user using DI instead of directly using User::create

Attaching the updated patch.

Wow, \Drupal\Tests\user\Traits\UserCreationTrait::createUser's signature is slightly different than \Drupal\KernelTests\Core\Entity\EntityKernelTestBase::createUser :)

The original behaviour of the form allows the creation of a user by entering an email address only. With the version of the patch in comment 18, all validations on a newly created user are applied, potentially preventing the creation of the user because of constraints related to fields that aren't on this form.

I'm attaching a version of the patch that only applies the email address constraint, to keep the behaviour the same as it was originally.

Committed! Thanks everyone!!