WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 5 months ago

#19867 new defect (bug)

wp_dropdown_users() still not scalable

Reported by: prettyboymp Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.3.1
Component: Users Keywords: has-patch needs-refresh
Focuses: ui, performance Cc:

Description

#14572 made huge improvements to the performance of wp_dropdown_users(), however, on certain sites that have an unusually large set of authors, wp_dropdown_users() still isn't usable. It either causes a memory error on the server, or potentially crashes the client browser due to content size.

Attachments (7)

user_autocomplete.diff (9.2 KB) - added by prettyboymp 3 years ago.
user_dropdown_filter.diff (962 bytes) - added by prettyboymp 3 years ago.
user_autocomplete.2.diff (11.6 KB) - added by prettyboymp 3 years ago.
wp_suggest_users.alpha.1.diff (7.4 KB) - added by mpvanwinkle77 2 years ago.
19867.diff (4.3 KB) - added by helen 14 months ago.
19867.2.diff (16.9 KB) - added by helen 14 months ago.
19867.3.diff (17.1 KB) - added by helen 14 months ago.

Download all attachments as: .zip

Change History (51)

comment:1 @prettyboymp3 years ago

attachment:ticket:19867:user_autocomplete.diff is a suggested solution using an autocomplete handling to replace the select box after a threshold of results is hit.

As an alternative, I'm working on a second patch that will just be a tweak to add a filter that will a plugin to add this functionality before the dropdown is generated, in case a built in solution isn't accepted.

comment:2 @prettyboymp3 years ago

attachment:ticket:19867:user_dropdown_filter.diff is alternative patch to at least allow a plugin to override the user dropdown before any queries are run.

comment:3 @prettyboymp3 years ago

Just realized that my first patch didn't include the supporting js. Added attachment:ticket:19867:user_autocomplete.2.diff with it included.

comment:4 @nacin3 years ago

+1 on the filter.

Additional +1 on autocomplete after a certain number of users. Since we are doing autocomplete for sites/users in the network admin for 3.4, the bits can likely be shared quite nicely.

comment:6 @DrewAPicture3 years ago

  • Cc xoodrew@… added

comment:7 @Japh3 years ago

  • Cc Japh added

comment:8 @scribu3 years ago

Closed #20769 as dup.

comment:9 @SergeyBiryukov3 years ago

#21286 closed as a duplicate.

comment:10 @scribu2 years ago

  • Milestone changed from Awaiting Review to Future Release

comment:11 @brokentone2 years ago

  • Cc kenton.jacobsen@… added

comment:12 @SergeyBiryukov2 years ago

#23129 was marked as a duplicate.

comment:13 @mpvanwinkle772 years ago

What's the status on this? Says targeted to 3.3.1 but it doesn't look like these changes ever made it through. I can help test if there's an approved patch for testing.

comment:14 follow-up: @helen2 years ago

Version is when something was reported/requested or the earliest a bug can be reproduced. Milestone is for the target, as it were.

Could probably use a patch that implements autocomplete the way it's done now for the network admin after a certain number of users. There's wp_is_large_network( 'users' ) to check for a large number of users but the default of 10k is probably still way too many for a dropdown.

comment:15 in reply to: ↑ 14 @kovshenin2 years ago

  • Cc kovshenin added
  • Keywords needs-refresh added

Replying to helen: I think wp_is_large_network is actually used to prevent autocomplete because such searches would be really slow on large databases. Here's site-users.php:

if ( ! wp_is_large_network( 'users' ) )
	wp_enqueue_script( 'user-suggest' );

We can probably just reuse the whole thing as is, with a different ajax callback.

comment:16 @mpvanwinkle772 years ago

  • Cc mpvanwinkle77 added

So here's a new approach. creates a new function wp_suggest_users ... allows for several criterion in wp_dropdown_users to initiate a bail to wp_suggest_users() ... minor updates to js and to wp-admin/includes/post.php required to make this work. There's still a lot to do to make this production ready, but wanted to get some general feed back on the approach before continuing.

comment:17 @SergeyBiryukov2 years ago

#23439 was marked as a duplicate.

comment:18 @toscho2 years ago

  • Cc info@… added

comment:19 @SergeyBiryukov23 months ago

#24557 was marked as a duplicate.

comment:20 @mpvanwinkle7721 months ago

Anyone ever get a chance to look at this? Is this the route we want to go? or is there a better strategy?

comment:21 @kevinlangleyjr20 months ago

  • Cc kevinlangleyjr added

comment:23 @timfield16 months ago

  • Cc tim@… added

comment:24 follow-up: @nacin15 months ago

  • Component changed from Performance to Users
  • Focuses performance added
  • Milestone changed from Future Release to 3.9

Could we use the same user autocomplete we used in the network admin?

comment:25 in reply to: ↑ 24 @helen15 months ago

Replying to nacin:

Could we use the same user autocomplete we used in the network admin?

I think so - two things occur to me that are probably different, though. It needs the ID as the value, as opposed to user_login in the current instances or user_email as in the patch on #25348, and the UI needs to show something more friendly than the ID and should prevent freeform entry.

Also, the patch on #25348 whitelists the field against an enumerated list (in_array) - should we keep expanding on that list, make it filterable, something else?

comment:26 @nacin15 months ago

#8648 was marked as a duplicate.

comment:27 @helen15 months ago

  • Focuses ui added
  • Keywords ui-feedback removed

comment:28 @ircbot15 months ago

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.

comment:29 @ircbot15 months ago

This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.

comment:30 @ircbot15 months ago

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.

comment:31 @ircbot14 months ago

This ticket was mentioned in IRC in #wordpress-dev by DH-Shredder. View the logs.

@helen14 months ago

@helen14 months ago

@helen14 months ago

comment:32 @helen13 months ago

  • Milestone changed from 3.9 to Future Release

I dove way down, and didn't come back up in time for 3.9. Here's where I landed (with help from gcorne). tl;dr: Autocomplete needs to enhance the right UI for the given data, whether that's a text input, a select element, or something else.

We have two instances of jQuery UI autocomplete in core right now, both in multisite. One is for adding existing users to a site (either from the network admin or the individual site admin) and the other, added this cycle, is for the new site admin email (#25348).

In exploring what it would take to allow wp_dropdown_users() (and wp_dropdown_pages() in #9864) to use this same method in order to scale, I realized that a freeform text input as the non-JS part of the UI was not actually ideal, and in fact, incorrect. If you test some of the patches I've been periodically putting onto the ticket for progress tracking, you'll see that I implemented a rough alternate UI, but too late in the cycle to do proper design and testing.

What I also realized is that the existing usages are actually two different things, and one of them uses a misleading UI. For a new site admin email, you can enter any email address and it will create a new user if that user doesn't already exist. The "autocomplete" in that case is more of a "prompt" or "suggest" - much like tags. However, when adding an existing user to a site, the UI really should only allow you to select an existing user, and not allow for arbitrary input. Essentially, this latter case should operate much as a select element would. Server-side catches it, of course, but we should have better UI.

I still want to push forward on approach and aim for early next release cycle. I'm thinking there are a few aspects:

  • What we use as the JS helper for autocomplete. jQuery UI autocomplete with a custom set of skins, something else? Either way, we should definitely use our own JS wrapper so that switching libraries or maintaining shims is a little more future-proofed (see: Masonry).
  • The UX and design of said helper.
  • Being smart about offering up some initial options to speed things up (more of a nice-to-have enhancement than a necessity), e.g. 10 users with the most posts for author selection dropdowns.

comment:33 @SergeyBiryukov12 months ago

#28078 was marked as a duplicate.

comment:34 @docjohn11 months ago

Yes, having initial options would be nice. Solution needs to work on large-author sites (we have a site with 30,000 authors, and some of the backend screens for admin are now unstable in Chrome, constantly crashing the browser). Thanks.

comment:35 @dd327 months ago

#29665 was marked as a duplicate.

comment:36 @ircbot7 months ago

This ticket was mentioned in IRC in #wordpress-dev by johnbillion. View the logs.

comment:37 @ircbot7 months ago

This ticket was mentioned in IRC in #wordpress-dev by johnbillion. View the logs.

comment:38 @chriscct76 months ago

#12014 was marked as a duplicate.

comment:39 @ircbot6 months ago

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.

comment:40 @chriscct76 months ago

Related (and could be implemented in #19867) #13269

comment:41 @ircbot6 months ago

This ticket was mentioned in IRC in #wordpress-dev by johnbillion. View the logs.

comment:42 @slackbot6 months ago

This ticket was mentioned in Slack in #core by drew. View the logs.

comment:43 @tharsheblows5 months ago

Hi - I know this is well underway but thought I'd put in a quick request just in case.

Would it be possible to hook the wp_dropdown_users select and eventually the autosuggest to an action that we could remove or add to? It would be really helpful to be able to completely remove that action and add my own - not just filter the wp_dropdown_users query, although that would be cool, too.

What I've done for users who want to be deleted is assign their content to a dummy user creatively named "_deleted account" (the underscore was to pop it to the top of the dropdown list). However, now I can't use the list (85k users) although it still loads and only breaks if I touch it.

To work around this, I've put a checkbox on the new and edit users pages that allows me to include them in a list of users to whom it's ok to reassign content. I keep this list as an array in wp_options so I don't have to hunt through either of the users tables piecing it together. I'm currently using this array to replace the options in the wp_dropdown_users select using jQuery.

Having a small curated list really works for me. Putting users on it might take a tiny bit more time but then once I do, I never have to remember again who I am using for deleted user accounts. And I don't have to worry about me or anyone else accidentally assigning content to the wrong person (disastrous! or maybe hilarious, it would depend...)

comment:44 @slackbot5 months ago

This ticket was mentioned in Slack in #core by boone. View the logs.

Note: See TracTickets for help on using tickets.