WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#15871 closed defect (bug) (fixed)

Quick edit and metabox author dropdowns show all users

Reported by: duck_ Owned by: scribu
Milestone: 3.1 Priority: normal
Severity: blocker Version: 3.1
Component: General Keywords: has-patch
Focuses: Cc:

Description

The author dropdowns displayed in quick edit, bulk edit and the author metabox on the post edit screen now show all users rather than just those who are user_level != 0.

The dropdown is now also shown even when there is only one author to choose from.

Both are a regression from 3.0.3.

See, 15786

Attachments (9)

15871.diff (3.7 KB) - added by scribu 3 years ago.
Exclude subscribers
15871.2.diff (4.6 KB) - added by scribu 3 years ago.
levels.15871.diff (7.2 KB) - added by scribu 3 years ago.
user_level != 0
caps.15871.diff (4.9 KB) - added by scribu 3 years ago.
check for 'edit_posts' cap
caps.15871.2.diff (6.1 KB) - added by scribu 3 years ago.
Introduce 'capability' arg to WP_User_Query
caps.15871.3.diff (6.0 KB) - added by scribu 3 years ago.
Just check 'edit_posts'
levels.15871.2.diff (4.6 KB) - added by scribu 3 years ago.
Direct query for user_level != 0
15871.3.diff (5.5 KB) - added by ryan 3 years ago.
15871-who.diff (344 bytes) - added by zeo 3 years ago.
'who'

Download all attachments as: .zip

Change History (44)

comment:1 scribu3 years ago

  • Owner set to scribu
  • Status changed from new to accepted

The author dropdowns displayed in quick edit, bulk edit and the author metabox on the post edit screen now show all users rather than just those who are user_level != 0.

We have to get rid of user levels at some point. We could probably replace that with role != subscriber.

comment:2 follow-up: nacin3 years ago

Can't afford to, based on custom roles. It would need to be any minus roles that have nothing more than the 'read' cap.

comment:4 in reply to: ↑ 2 scribu3 years ago

Replying to nacin:

Can't afford to, based on custom roles. It would need to be any minus roles that have nothing more than the 'read' cap.

Yeah, was thinking the same. But then user_level=0 would have even less meaning.

comment:5 scribu3 years ago

  • Severity changed from blocker to normal

comment:6 nacin3 years ago

  • Severity changed from normal to blocker

Still technically a blocker. A) regression in itself, B) it can cause memory exhaustion trivially.

scribu3 years ago

Exclude subscribers

comment:7 scribu3 years ago

Went with excluding subscribers. Even though it's not ideal, it's still better than user_level=0 or leaving as is.

Still need to do the same for the metabox.

scribu3 years ago

comment:8 scribu3 years ago

  • Keywords has-patch added

Made a little helper function, to ensure the list of users stays the same between the metabox and the inline edit dropdowns.

comment:9 nacin3 years ago

I would rather go with user_level != 0 unless we can properly support custom roles. Too late for changes.

comment:10 scribu3 years ago

I really don't see advantage to that. User levels are deprecated and the patch is the same either way.

comment:11 scribu3 years ago

"the same" = almost identical

comment:12 nacin3 years ago

Not if I have a role on my site that has 30,000 users in a custom role. We use 'subscriber' as former authors, and the custom role is called 'Member.' This would regress performance when the custom role was specifically designed for performance.

Now is not the time for API changes.

comment:13 scribu3 years ago

So why would using user_level!=0 be better for that case?

comment:14 nacin3 years ago

Because the custom role has read = true and user_level = 0.

I have no problem eventually dropping the user_level check. But we need to move to caps, not role names.

comment:15 ryan3 years ago

Perhaps get_users() needs a 'who' arg. who = editable_authors.

comment:16 ryan3 years ago

We should select in the same way we did in 3.0. Unfortunately it wasn't consistent between single and multisite. I think multisite looked for capabilities in the usermeta and thus probably ended up including all users.

Last edited 3 years ago by ryan (previous) (diff)

comment:17 follow-ups: ryan3 years ago

Fetch all roles that do not have edit_posts (or the CPT equivalent)? Select users that do not have those roles? Without a cap plugin this would always be just the subscriber role.

scribu3 years ago

user_level != 0

comment:18 in reply to: ↑ 17 scribu3 years ago

Replying to ryan:

Fetch all roles that do not have edit_posts (or the CPT equivalent)? Select users that do not have those roles? Without a cap plugin this would always be just the subscriber role.

Exactly. levels.15871.diff mimics 3.0 behaviour. I had to do some shufling in get_meta_sql() to accept '0' as a meta value.

comment:19 in reply to: ↑ 17 scribu3 years ago

Replying to ryan:

Fetch all roles that do not have edit_posts (or the CPT equivalent)? Select users that do not have those roles? Without a cap plugin this would always be just the subscriber role.

Or, just select those users with roles that do have the edit_posts cap or equivalent.

comment:20 follow-up: ryan3 years ago

To select on roles that do you would have to OR a lot of LIKEs versus one NOT LIKE.

comment:21 in reply to: ↑ 20 ryan3 years ago

Replying to ryan:

To select on roles that do you would have to OR a lot of LIKEs versus one NOT LIKE.

Assuming one NOT LIKE is faster. Not necessarily the case.

scribu3 years ago

check for 'edit_posts' cap

comment:22 scribu3 years ago

We'll have to go with NOT LIKE for now, since 'meta_query' doesn't support OR, like 'tax_query' does.

See caps.15871.diff

comment:23 ryan3 years ago

What do you think about adding 'who' => 'authors' to wp_dropdown_users() and perhaps get_users() and avoid the helper function and the extra passing around of users?

comment:24 scribu3 years ago

Currently, _get_potential_authors() uses the $post_type global to find the correct cap. I don't see how 'who' => 'authors' could be post type aware.

Maybe have a lower-level 'capability' arg instead.

comment:25 nacin3 years ago

I wasn't aware of the goal to be post-type aware at all. Anything is better than what we have now. How much can wait for 3.2? (Especially if we finally pull the trigger on #10201.)

comment:26 nacin3 years ago

Scratch that last comment.

scribu3 years ago

Introduce 'capability' arg to WP_User_Query

comment:27 scribu3 years ago

caps.15871.2.diff moves the capability => roles => meta_query logic to WP_User_Query.

scribu3 years ago

Just check 'edit_posts'

scribu3 years ago

Direct query for user_level != 0

comment:28 scribu3 years ago

levels.15871.2.diff makes a direct query to get the users, to avoid the modifications necessary in get_meta_sql() which could introduce other bugs.

Also, it doesn't check for !is_multisite() anymore since it's not necessary.

ryan3 years ago

comment:29 ryan3 years ago

Adds 'who' support. Add 'show_option_gt_one' for only showing the dropdown when there is more than one user present. Added lame temp hack to get '0' meta_value queries without having to redo get_meta_sql().

comment:30 ryan3 years ago

Ignore the @todo in user.php. No longer applies.

comment:31 ryan3 years ago

Per IRC discussion, name the new option something like hide_if_only_one_author and make it a bool.

comment:32 ryan3 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

(In [17088]) Add 'who' arg to wp_dropdown_users() and get_users(). Add' hide_if_only_one_author' argument to get_users(). Query only authors (user level greater than 0) when who => author is passed. Query only authors for author meta box and quick edit dropdowns. Props scribu. fixes #15871

zeo3 years ago

'who'

comment:33 zeo3 years ago

You left this one out: 'who' => ''

Version 0, edited 3 years ago by zeo (next)

comment:34 ryan3 years ago

(In [17091]) Add default for who arg. see #15871

comment:35 scribu3 years ago

Related: #16841

Note: See TracTickets for help on using tickets.