Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#15871 closed defect (bug) (fixed)

Quick edit and metabox author dropdowns show all users

Reported by: duck_ Owned by: scribu
Priority: normal Milestone: 3.1
Component: General Version: 3.1
Severity: blocker Keywords: has-patch
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 2 years ago.
Exclude subscribers
15871.2.diff (4.6 KB) - added by scribu 2 years ago.
levels.15871.diff (7.2 KB) - added by scribu 2 years ago.
user_level != 0
caps.15871.diff (4.9 KB) - added by scribu 2 years ago.
check for 'edit_posts' cap
caps.15871.2.diff (6.1 KB) - added by scribu 2 years ago.
Introduce 'capability' arg to WP_User_Query
caps.15871.3.diff (6.0 KB) - added by scribu 2 years ago.
Just check 'edit_posts'
levels.15871.2.diff (4.6 KB) - added by scribu 2 years ago.
Direct query for user_level != 0
15871.3.diff (5.5 KB) - added by ryan 2 years ago.
15871-who.diff (344 bytes) - added by zeo 2 years ago.
'who'

Download all attachments as: .zip

Change History (44)

  • 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: ↓ 4   nacin2 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   scribu2 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.

  • Severity changed from blocker to normal
  • Severity changed from normal to blocker

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

scribu2 years ago

Exclude subscribers

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.

scribu2 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.

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

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

"the same" = almost identical

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.

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

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.

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

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

Version 0, edited 2 years ago by ryan (next)

comment:17 follow-ups: ↓ 18 ↓ 19   ryan2 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.

scribu2 years ago

user_level != 0

comment:18 in reply to: ↑ 17   scribu2 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   scribu2 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: ↓ 21   ryan2 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   ryan2 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.

scribu2 years ago

check for 'edit_posts' cap

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

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?

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.

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.)

Scratch that last comment.

scribu2 years ago

Introduce 'capability' arg to WP_User_Query

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

scribu2 years ago

Just check 'edit_posts'

scribu2 years ago

Direct query for user_level != 0

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.

ryan2 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().

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

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

  • 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

zeo2 years ago

'who'

comment:33   zeo2 years ago

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

See patch.

Last edited 2 years ago by zeo (previous) (diff)

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

Related: #16841

Note: See TracTickets for help on using tickets.