[wp-trac] [WordPress Trac] #53199: WP_User::add_role() runs even if the added role is already set

[WordPress Trac]

#53199: WP_User::add_role() runs even if the added role is already set
--------------------------+-----------------------------
 Reporter:  bhujagendra   |      Owner:  (none)
     Type:  defect (bug)  |     Status:  new
 Priority:  normal        |  Milestone:  Awaiting Review
Component:  Users         |    Version:  trunk
 Severity:  normal        |   Keywords:
  Focuses:                |
--------------------------+-----------------------------
 When adding a role using `WP_User::add_role()` the function is run even
 though the role is already set for the given user. Hence, the also action
 `add_user_role` is fired when it should not.

 This behavior is different from `WP_User::remove_role()` that exits the
 function if the role is not currently set, and hence the action
 `remove_user_role` does not fire - as one would expect.

 An easy fix would be to add `in_array( $role, $this->roles, true )` to the
 test in [[https://core.trac.wordpress.org/browser/trunk/src/wp-includes
 /class-wp-user.php#L540|class-wp-user.php on Line 540]] resulting in the
 following:


 {{{
 if ( empty( $role ) || in_array( $role, $this->roles, true ) ) {
 }}}

 Not sure if there are any backward-compatibility issues with this
 solution, as the action would not be called anymore in situations where
 previously it was called. A solution could be to run the action
 nonetheless, which would, however, kinda obsolete the fix. Another
 solution could be to introduce a new action, e.g.
 `add_user_role_requested`:


 {{{
 if ( empty( $role ) ) {
         return;
 }

 /**
  * Fires immediately after the user has been given a new role.
  *
  * @since x.x.x
  *
  * @param int    $user_id The user ID.
  * @param string $role    The new role.
  */
 do_action( 'add_user_role_requested', $this->ID, $role );

 if ( in_array( $role, $this->roles, true ) ) {
     return;
 }

 }}}

 This filter uses the same syntax, than the existing `add_user_role`. I
 personally would prefer to pass the user object (`$this`) rather than the
 user ID. Maybe the user object could be passed as a third argument to both
 actions.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/53199>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform