#22959 closed enhancement (fixed)
Show all roles in user list table
Reported by: | scribu | Owned by: | johnbillion |
---|---|---|---|
Milestone: | 4.4 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Role/Capability | Keywords: | has-patch |
Focuses: | administration | Cc: |
Description (last modified by )
Attachments (6)
Change History (42)
#10
@
11 years ago
I know this ticket is now for a future release. I'm still interested in the inclusion of filtering of the roles, pre-translation. Am I the only one, or should I submit a patch to that effect?
#11
@
11 years ago
Refreshed against trunk, added inline docs, added filter for get_role_str method. Not über-sold on the filter name, but it definitely feels like a filter worth having.
#12
@
11 years ago
- Cc xoodrew@… added
- Milestone changed from Future Release to 3.7
Replying to JustinSainton:
Refreshed against trunk, added inline docs, added filter for get_role_str method. Not über-sold on the filter name, but it definitely feels like a filter worth having.
Not really sold on the method name either. And since the filter is evaluated on the return, it should match the method name.
- Changes the function/filter name to
get_role_list()|get_role_list
- Accounts for back-compat on role-named classes on single row checkboxes
- Fleshes out the doc block.
Side note: Is it kosher to just remove the $role
param from the method in this type of context? Just curious.
#13
@
11 years ago
I had a similar curiosity there. It's not likely that a ton of people were overriding or calling this method directly. Not entirely sure what the core standards are for using _deprecated_argument() (As in, I'm not sure if that is always done, or if it's only done in specific cases.). Left it that way because @scribu did it that way in the original patch and he's wicked smart.
#14
@
11 years ago
Rather than removing the argument, let's continue to leave the role key there.
Otherwise, this looks good. Might need a slight cleanup. Can get_role_list() instead return an array of role key => role name? Then the classes don't need to be a faux-sanitized version of that, and we can do the implode on the fly.
#15
@
11 years ago
- Keywords dev-feedback added
22959.4.diff implements @nacin's suggestions in comment:14.
#16
@
11 years ago
- Keywords commit added; dev-feedback removed
Looks good. I feel it may be a bit late for this kind of an enhancement — marking for 3.7 commit but may end up later today being moved to 3.8.
#17
@
11 years ago
- Milestone changed from 3.7 to Future Release
It has occurred to me that there is a distinct possibility that a plugin that implements multiple roles might not want subsequent roles to actually be displayed. Maybe this should start with one role and be filterable to add additional roles. Or maybe roles should be marked as public or private. Or maybe who cares and 22959.4.diff is good. A role name could be omitted, I guess — add_role( 'some-slug', false ) (or ''
). Might that be enough to consider a role to be "private"? Should we only list roles that are returned by get_editable_roles()? Just spitballing here. (As in, I would advise not bothering to patch any which way until a discussion comes to a consensus.)
Let's try to resolve this in 3.8, as I know it'll benefit plugins like bbPress.
(As is customary for a roles/caps ticket, see also #10201.)
#18
@
11 years ago
Wouldn't the presence of this new filter give a plugin the opportunity to hide those private/unimportant roles at their discretion?
I think that seeing the full list by default is an important diagnostic when more complex role structures are in use.
#19
@
9 years ago
Just wanted to re-ignite this ticket, recently came across a good example of where multiple roles and the way they were displayed could cause issues. If a user has both a subscriber role and an administrator role (for example the admin role added via wp-cli) then they are showing in the admin area user list with the role subscriber. While they do show up under administrator filter again, with role subscriber.
This can easily be confusing, and also allow someone to obfuscate their real role (I'm aware if someone could add a role, they can as easily add capabilities but still)
If nothing else showing the role with the most number of capabilities or if a role has edit options capability give it precedence over others.
#23
@
9 years ago
- Focuses administration added
I'd be happy to take this over if @johnbillion wants a rescue on this one.
I think there's a couple key questions that need to be answered on this:
- How many roles are shown by default? All of them is ideal unless you get 1 user with 50 roles and everyone else has 1. I think this would be so edge case core wouldn't need to worry about it, and could be avoided with a little overflow scroll thing if it gets nuts.
- How do we let plugins limit their roles from showing on this page? I think two key ways. First a filter on what shows there, for that specific screen. But also I think instead of doing a "public" key on add_role, do "capability" so if current_user doesn't have that cap they can't see/add/edit/remove the role. Then if a plugin wants to hide a cap from everyone, they could use a capability that doesn't exist. This would let plugins build out dynamic capabilities.
- Does the UI natively support multiple roles? I think it would make plugins lives incredibly easy if either WordPress core natively supported a Select2 multiselect like UI out of the box (yet another reason it'd be cool to land #31696 in 4.3) for adding to user's roles on the edit user page, or had a filter where plugins could flip it on. The absense of this would be something every plugin who wanted to use this (from EDD to Woo to bbPress to everyone using this) have to go out and implement this, and each would have to do it individually. And it would let WordPress show Super Administrator and Administrator on the multisite user listings page, which would be cool.
- If WordPress starts supporting this, a user_has_role function could be added that aliases to user_has_cap so that down the road roles could be decoupled (in terms of not evaluating as a capability) if core wanted to.
Those are the ones I could remember at the moment.
#25
@
9 years ago
- Keywords ux-feedback added
The problem with bbPress's implementation it it scales badly when you have > 3 roles for a user. On the other hand, we can keep it simple in core and just list all roles in the existing role column and if a plugin like bbPress wants a specialized column it could continue to do so (perhaps using a filter to remove its' role from the overall Role(s) list.
#26
@
9 years ago
With beta around the corner and this ticket not having seen action in 5 weeks, this is a strong candidate to not make it into 4.3.
#27
@
9 years ago
Unless @johnbillion wants to remain owner and/or keep this on 4.3, I'd be willing to own this for 4.4
#28
@
9 years ago
- Milestone changed from 4.3 to Future Release
- Owner changed from johnbillion to chriscct7
#29
@
9 years ago
- Keywords ui-feedback added
This could possibly be a UI/UX team focus area for 4.4 if someone wants to work on that. Will propose during today's meeting for that in Slack.
It's easy to show all the roles, but if we're going to show all of them, and I think it should, then some of the controls like change role should be reworked, to maybe something like GitHub's system for choosing tags for bulk issues where you select the tab and then hit add or remove. Also the role changer on the edit user screen should be turned into something like a Select2 Multiselect field or something like that to more easily support multiple roles.
#31
@
9 years ago
- Keywords ux-feedback ui-feedback removed
- Milestone changed from Future Release to 4.4
The benefits of showing all of a users' roles in this column (clarity and security) outweigh any negatives, such as the lack of a corresponding UI for controlling multiple roles for a user (#17924). Let's get this into 4.4.
Where should filtering be added, and to what end? If we're expected to find existing elements in the array, perhaps it should be filtered pre-translation. If not, this at least gives an opportunity to add elements.
Before:
return implode( ', ', $role_str );
After:
return implode( ', ', apply_filters( 'wp_user_list_get_role_str', $role_str, $user_object ) );
If the intention was to modify the column's output after the implode(), another filter would be necessary.