Opened 21 months ago
Closed 15 months ago
#54164 closed enhancement (fixed)
Consistently fire user role hooks
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.0 | Priority: | normal |
Severity: | normal | Version: | 4.3 |
Component: | Users | Keywords: | needs-patch |
Focuses: | Cc: |
Description
There are three main user role hooks:
- add_user_role
- remove_user_role
- set_user_role
There are some differences in how these hooks are fired:
add_user_role
is always fired if$user->add_role()
is called with a non-empty role. Whether the user had the role already or not.remove_user_role
is only fired if$user->remove_role()
is called with a non-empty role AND the user had that role.set_user_role
(Which combines add & remove, but doesn't fire their hooks) is only fired if the user had different roles prior to being called. If the role matches what the user already had, it's not.
This leaves with some complicated rules if you want to watch for user role changes.
add_user_role
firing doesn't mean a user has gained a role.set_user_role
may have removed a role.set_user_role
may fire without gaining a role.
Attached is a proposed patch that:
- Doesn't fire
add_user_role
if the user already had the role. - Fires
remove_user_role
andadd_user_role
from within$user->set()
when appropriate
This standardises the actions that one needs to hook to, being either add_user_role
or remove_user_role
.
Attachments (3)
Change History (8)
@
21 months ago
consistently set $this->roles
to an array, as this is the only place that sets it to an undocumented false value.
#2
follow-up:
↓ 3
@
15 months ago
- Owner set to SergeyBiryukov
- Resolution set to fixed
- Status changed from new to closed
In 52823:
#3
in reply to:
↑ 2
@
15 months ago
Replying to SergeyBiryukov:
In 52823
Hi can we merge these two if controle https://prnt.sc/VLGhGiND77NH ?
#4
@
15 months ago
- Keywords needs-patch added; has-patch removed
- Resolution fixed deleted
- Status changed from closed to reopened
Reopening as the Multisite tests started failing following this commit.
Note: See
TracTickets for help on using
tickets.
edgecase, when the user didn't have a role
$old_roles
would be falsey rather than arrayey