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

WordPress Trac noreply at wordpress.org
Mon Mar 21 02:04:05 UTC 2022


#54572: Add a function for updating the existing role instead of removing, then
adding one
--------------------------------------+--------------------------
 Reporter:  maksimkuzmin              |       Owner:  audrasjb
     Type:  enhancement               |      Status:  reviewing
 Priority:  normal                    |   Milestone:  6.0
Component:  Role/Capability           |     Version:
 Severity:  normal                    |  Resolution:
 Keywords:  has-patch has-unit-tests  |     Focuses:  performance
--------------------------------------+--------------------------

Comment (by peterwilsoncc):

 Thanks for the patch, @maksimkuzmin

 Based on a quick review, I have a few suggestions for improving your
 patch:

 * `update_role()` -- function's display name parameter can default to
 `null` to match the `WP_Roles::update_role()` method
 * It would be good to split the test up a bit. The first time an assertion
 fails in a test PHP Unit stops running the test (it knows it's failed). In
 this case, if the code to update the display names breaks, it would be
 handy to know whether the capabilities update code is working too.
 * The `empty( $role )` could be replaced by something more precise (ask
 yourself what you wish to test for). At the moment it could pass with `[
 54572 ]` which no body wants. I noticed this comes from `add_role` so it's
 possible that WordPress is stuck with it.

 The patch looks fairly close, thank you, it's a little tidying around the
 edges.

 Also, feel free to use a pull request on GitHub if you will find that
 easier, you'll just need to post a link to this ticket in the description.
 Most committers use the modern tools as much as possible.

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


More information about the wp-trac mailing list