Make WordPress Core

Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#15871 closed defect (bug) (fixed)

Quick edit and metabox author dropdowns show all users

Reported by: duck_'s profile duck_ Owned by: scribu's profile 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 13 years ago.
Exclude subscribers
15871.2.diff (4.6 KB) - added by scribu 13 years ago.
levels.15871.diff (7.2 KB) - added by scribu 13 years ago.
user_level != 0
caps.15871.diff (4.9 KB) - added by scribu 13 years ago.
check for 'edit_posts' cap
caps.15871.2.diff (6.1 KB) - added by scribu 13 years ago.
Introduce 'capability' arg to WP_User_Query
caps.15871.3.diff (6.0 KB) - added by scribu 13 years ago.
Just check 'edit_posts'
levels.15871.2.diff (4.6 KB) - added by scribu 13 years ago.
Direct query for user_level != 0
15871.3.diff (5.5 KB) - added by ryan 13 years ago.
15871-who.diff (344 bytes) - added by zeo 13 years ago.
'who'

Download all attachments as: .zip

Change History (44)

#1 @scribu
13 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.

#2 follow-up: @nacin
13 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.

#4 in reply to: ↑ 2 @scribu
13 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.

#5 @scribu
13 years ago

  • Severity changed from blocker to normal

#6 @nacin
13 years ago

  • Severity changed from normal to blocker

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

@scribu
13 years ago

Exclude subscribers

#7 @scribu
13 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.

@scribu
13 years ago

#8 @scribu
13 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.

#9 @nacin
13 years ago

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

#10 @scribu
13 years ago

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

#11 @scribu
13 years ago

"the same" = almost identical

#12 @nacin
13 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.

#13 @scribu
13 years ago

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

#14 @nacin
13 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.

#15 @ryan
13 years ago

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

#16 @ryan
13 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 13 years ago by ryan (previous) (diff)

#17 follow-ups: @ryan
13 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.

@scribu
13 years ago

user_level != 0

#18 in reply to: ↑ 17 @scribu
13 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.

#19 in reply to: ↑ 17 @scribu
13 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.

#20 follow-up: @ryan
13 years ago

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

#21 in reply to: ↑ 20 @ryan
13 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.

@scribu
13 years ago

check for 'edit_posts' cap

#22 @scribu
13 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

#23 @ryan
13 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?

#24 @scribu
13 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.

#25 @nacin
13 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.)

#26 @nacin
13 years ago

Scratch that last comment.

@scribu
13 years ago

Introduce 'capability' arg to WP_User_Query

#27 @scribu
13 years ago

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

@scribu
13 years ago

Just check 'edit_posts'

@scribu
13 years ago

Direct query for user_level != 0

#28 @scribu
13 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.

@ryan
13 years ago

#29 @ryan
13 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().

#30 @ryan
13 years ago

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

#31 @ryan
13 years ago

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

#32 @ryan
13 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

@zeo
13 years ago

'who'

#33 @zeo
13 years ago

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

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

#34 @ryan
13 years ago

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

#35 @scribu
13 years ago

Related: #16841

Note: See TracTickets for help on using tickets.