[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