[wp-trac] [WordPress Trac] #57967: Regression: Username check introduced in WP 6.2 should allow updates to the same user

[WordPress Trac]

#57967: Regression: Username check introduced in WP 6.2 should allow updates to the
same user
-------------------------------------------------+-------------------------
 Reporter:  polevaultweb                         |       Owner:  audrasjb
     Type:  defect (bug)                         |      Status:  accepted
 Priority:  normal                               |   Milestone:  6.2
Component:  Users                                |     Version:  trunk
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch has-unit-tests needs-      |     Focuses:
  testing changes-requested                      |
-------------------------------------------------+-------------------------
Changes (by hellofromTonya):

 * keywords:  has-patch has-unit-tests needs-testing => has-patch has-unit-
     tests needs-testing changes-requested


Comment:

 In the light of a brand new day, I also agree with reverting both [55358]
 and [55360].

 Why?

 === tl;dr
 I suspect there are more go/no-go combinations to be considered,
 especially against BC.

 === Longer reasoning

 @azaozz has [https://github.com/WordPress/wordpress-
 develop/pull/4257#discussion_r1144131510 raised valid questions]:
 >{{{
 >$existing_user_id = The user with that email address
 >$user_id = The user being updated.
 >}}}
 >
 >May be missing something but this means that if user A has a username
 that matches user B's email address, user A cannot be updated? Shouldn't
 that be the other way round: prevent an user from changing their email
 address to match another user's username?

 In observing the [https://github.com/WordPress/wordpress-develop/pull/4257
 discussion in the patch] and looking through the current testing
 scenarios, I too have more questions.

 I think this area of the code needs a full list of happy and unhappy
 testing scenarios to cover the what ifs and ''what should and shouldn't
 happen''. I suspect there are more combinations to be considered,
 especially against BC.

 Other than [53501] (which made 2 strings translatable) and [52954] (which
 relocated an existing `user_nicename` length check), the
 `wp_insert_user()` function has not been touched in years, ''suggesting''
 caution is needed when adding additional no-go checks for inserting or
 updating a user.

 Inserting new users and updating existing users are fundamental tasks. The
 risk is too high for breakages by trying to fix this regression at this
 late stage in the 6.2 release, as a fix may introduce more issues.

 ==== What are the risks of reverting?
 IMO None.

 Reverting removes the risk to the 6.2 release. Why? It removes the new
 scenario added during the release, restoring the `wp_insert_user()` back
 to pre-6.2.

 ==== Are there other changes made that could possibly be impacted by these
 reverts?
 Hmm, I don't think so. Why?
 * `wp_insert_user()`: no other changes were made in the 6.2 cycle.
 * `wp_update_user()`: [55161] introduced `switch_to_user_locale()`, but I
 don't see a correlation to go/no-go for update/insert user logic.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/57967#comment:14>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform