[wp-trac] [WordPress Trac] #54572: Add a function for updating the existing role instead of removing, then adding one

[WordPress Trac]

#54572: Add a function for updating the existing role instead of removing, then
adding one
-------------------------------------------------+-------------------------
 Reporter:  maksimkuzmin                         |       Owner:
                                                 |  davidbaumwald
     Type:  enhancement                          |      Status:  reopened
 Priority:  normal                               |   Milestone:  6.1
Component:  Role/Capability                      |     Version:
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch has-unit-tests needs-dev-  |     Focuses:
  note                                           |
-------------------------------------------------+-------------------------
Changes (by manfcarlo):

 * status:  closed => reopened
 * resolution:  fixed =>


Comment:

 Replying to [ticket:54572 maksimkuzmin]:
 > First, I'll have to perform double query to MySQL database, which
 affects performance and doesn't feel right to me. Second, I won't be able
 to alter just one field as easily with one function, I need to carry out
 the old variable for either display name or capabilities every time I call
 this statement.

 The new method only reads and modifies public instance variables, so you
 could include and run it in your own plugin classes without the need to
 make it available in core. You can still save the extra database query by
 running the three `unset` statements rather than calling the `remove_role`
 method. I don't see any inconvenience or difficulty with carrying through
 the old variable.

 It also has some errors with reading array keys that may not be set. Line
 220 reads `$roles[ $role ]` without checking if it's set, and line 233
 reads `$role_objects[ $role ]` without checking if it's set. It can't be
 assumed that one must be set if the other is set.

 Incidentally, line 233 won't actually run because `$capabilities` was
 already set earlier in the method, but it may end up running if other
 parts of the method are re-written later.

 Similarly, the call to the `add_role` method on line 242 appears to
 wrongly assume that if `$roles[ $role ]` was not set, that the role did
 not exist and is being newly added rather than updated.

 Line 220 is the most serious problem and will introduce PHP "undefined
 offset" errors, which I think justifies re-opening the ticket, but I would
 personally argue the method is not needed at all for the reasons given in
 my first paragraph.

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