WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

#19810 closed task (blessed) (fixed)

Autocomplete for users and sites in Network admin

Reported by: markjaquith Owned by:
Milestone: 3.4 Priority: high
Severity: normal Version:
Component: Multisite Keywords: has-patch commit
Focuses: Cc:

Description (last modified by markjaquith)

This is one of the action items for the 3.4 release.

Administering Multisite installs would be a lot easier if we had autocomplete for users and sites. As it is, the UI for sites is optimized for a small number of sites, and the UI for users often requires you to know the user's exact info.

Any place where multisite is prompting you for a site or a user, or you're trying to find a specific site or user, we should have autocomplete. This includes for Super Admins when adding a user from the Site admin instead of the Network admin.

Agenda:

  • Identify the areas to hit.
  • Discuss whether regular site Administrators should get autocomplete on the global users list.
  • Determine whether existing autocomplete JS tools are satisfactory. If not, improve.
  • Divide and conquer.

Attachments (23)

19810.patch (8.5 KB) - added by Japh 2 years ago.
Modified patch from #18160 for better styling and closer appearance to Tags autocomplete UI
19810.2.patch (10.3 KB) - added by DrewAPicture 2 years ago.
Refreshed date in script loader and added user-new.dev.js
19810.3.patch (11.3 KB) - added by Japh 2 years ago.
Refreshed for [19738] and [19739] as requested
19810.4.patch (11.0 KB) - added by markjaquith 2 years ago.
Patch didn't apply cleanly. Please watch out for "no newline" stuff and keep it out of your patches.
19810.5.patch (4.9 KB) - added by markjaquith 2 years ago.
Remove multi-user-add. Fix throbber-that-never-dies on empty result set.
19810.diff (6.3 KB) - added by nacin 2 years ago.
19810.7.patch (6.9 KB) - added by DrewAPicture 2 years ago.
Adds autocomplete to site-users.php, several other small bits
19810.6.patch (6.3 KB) - added by PeteMall 2 years ago.
Patch refreshed for [19831]
19810.8.patch (6.7 KB) - added by PeteMall 2 years ago.
Added better cap checks and filter for allowing autocomplete in site admin for site admins.
19810.9.patch (7.4 KB) - added by PeteMall 2 years ago.
Added the missing js file.
19810.10.patch (8.2 KB) - added by PeteMall 2 years ago.
Fixes user exclusion in network admin.
19810.11.patch (6.4 KB) - added by PeteMall 2 years ago.
Refreshed and cleaned up.
19810.12.patch (5.5 KB) - added by koopersmith 2 years ago.
Cleans up the JS/CSS and removes unnecessary code.
19810.13.patch (3.3 KB) - added by Japh 2 years ago.
A modification to allow the autocomplete to be used for more purposes, such as search fields. This should also make it easier to add availability checking too.
19810.14.patch (5.3 KB) - added by DrewAPicture 2 years ago.
Enqueues user-search and adds input id on dashboard.php for "Right Now" user search
19810.15.patch (7.7 KB) - added by DrewAPicture 2 years ago.
Increments onto 19810.14.patch, ads 1st pass at site search autocomplete
19810.16.patch (8.2 KB) - added by Japh 2 years ago.
Adjustment to the first pass at implementing site autocomplete. It will now user the search term in the query, and inserts the domain for searching.
19810.17.patch (7.7 KB) - added by DrewAPicture 2 years ago.
Refined query in autocomplete_site to format similarly to autocomplete_user, returned several waywardly removed filters, nearly #thereforebeer
19810.18.patch (7.8 KB) - added by Japh 2 years ago.
Implemented @dd32's suggestions
19810.19.patch (9.0 KB) - added by Japh 2 years ago.
Separated autocomplete for sites and users to allow proper loading / checking
19810.site-admin-logic.diff (1.5 KB) - added by duck_ 2 years ago.
19810.20.patch (10.1 KB) - added by Japh 2 years ago.
Patch including changes from [19934], and applying them across the other new user and site autocomplete areas
19810.21.patch (9.7 KB) - added by DrewAPicture 2 years ago.
Removes all uses of autocomplete_sites_for_site_admins filter, other cap checks, basic whitespacing fixes.

Download all attachments as: .zip

Change History (83)

comment:1 markjaquith2 years ago

  • Description modified (diff)

comment:2 DrewAPicture2 years ago

  • Cc xoodrew@… added

comment:3 SergeyBiryukov2 years ago

Related: #15942, #18160 (has patches).

comment:4 japh2 years ago

  • Cc japh@… added

comment:5 markjaquith2 years ago

  • Description modified (diff)

Taking this out of the description to make it clear that contributions are welcomed from all. We'll call out responsible parties elsewhere.

comment:6 Ipstenu2 years ago

  • Cc ipstenu@… added

comment:7 japh2 years ago

I've begun identifying the areas under Network Admin that would fall under these improvements:

  • Add auto-complete to Users field on "Right Now" Dashboard widget
  • Add auto-complete to Sites fields on "Right Now" Dashboard widget
  • Add auto-complete to Sites field on Site Listing page
  • Add availability check to Site Address field on Add New Site page
  • Add auto-complete to Users field on User Listing page
  • Correct counts on User Listing page
  • Add availability check to Username field on Add New User page
  • Add auto-complete to Username search field on Edit Site -> Users tab
  • Add auto-complete to Username field under "Add Existing User" on Edit Site -> Users tab
  • Add availability check to Username field under "Add New User" on Edit Site -> Users tab
Last edited 2 years ago by japh (previous) (diff)

comment:8 follow-up: DrewAPicture2 years ago

Some more I think might be worth including:

  • Add auto-complete to Admin Email on "Add New Site" page (Per #15942, #17890 also related)
  • Add auto-complete to "Add Existing User" field on Users > Add New (#18160)

And for consistency's sake, maybe we should look at the single-site-only user-related fields as well. That might be worthy of a separate ticket/cycle though.

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

comment:9 in reply to: ↑ 8 japh2 years ago

Good one, also same for "Edit Site" page then, I guess

comment:10 DrewAPicture2 years ago

Related plugins:

Adds auto-complete to "Add Existing User" under Dashboard > Users > Add New (#18160). Interestingly, it also allows for multi-user adding -- might be possible to incorporate #18162 and #18161 with reversed logic. @boonebgorges never ceases to amaze.

Also extends WP_User_Query to search more fields, might help with #16366.

It might be worth looking at the user-search and multiple-user adding autocomplete logic in this one.

Version 6, edited 2 years ago by DrewAPicture (previous) (next) (diff)

comment:11 follow-up: DrewAPicture2 years ago

Some thoughts:

Autocomplete Libary

  • Based on most of the implementations I'm seeing, it looks like the standard jQuery Autocomplete library is pretty popular. Anybody have tested alternatives in mind? Edit: suggest.js already in use.

Scaling

  • Whatever implementation we go with, we're (obv) going to have to take scaling into account. I've got a MultiSite in the wild with a (modest) couple hundred users and the autocomplete searches are taking longer than they should. I can't even imagine what it would be like on larger-scale installs. Regardless, my first instinct is transients, we'd just have to figure out the logistics of resetting the stored query data when new users are registered/added so the query stays current. (Feel free to call me out if this is an absurd concept).

Global User List

  • Should Site Admins get access to the global user list? Good question. My first instinct is to answer 'yes', if they have X information about the user already, such as a username, but then it would make autocomplete nearly moot. If 'no', then only Super Admins would have the ability to add users, which would probably be neither efficient nor reasonable. I guess it would be pertinent to look at use cases for why Site Admins would need access the user list or not.
  • Taking it from a different tack, it might be prudent to consider an option (instead of a decision) in this case. Super Admins already have the ability to enable Plugin administration menus, why not enable/disable Site Admin access to the global user list? Then it would become a question of whether that was network-wide or per-site.

Organization

  • Just from a cursory glance around Trac, we've already pinpointed many tickets that tie into the autocomplete stuff we're tackling here. Do we bring it all under one roof, or deal with the scattered tickets and related patches separately?
Last edited 2 years ago by DrewAPicture (previous) (diff)

comment:12 in reply to: ↑ 11 ; follow-ups: japh2 years ago

Replying to DrewAPicture:

Autocomplete Libary

  • Based on most of the implementations I'm seeing, it looks like the standard jQuery Autocomplete library is pretty popular. Anybody have tested alternatives in mind?

At the moment suggest.js is being used in the admin, so unless there's a specific reason to go with Autocomplete instead then perhaps we see if that can be used? I'm working on a patch now just to see if that will transfer easily and be useable.

For an example of where this is used, check out Tags on the Edit Post page.

Scaling

  • Whatever implementation we go with, we're (obv) going to have to take scaling into account. I've got a MultiSite in the wild with a (modest) couple hundred users and the autocomplete searches are taking longer than they should. I can't even imagine what it would be like on larger-scale installs. Regardless, my first instinct is transients, we'd just have to figure out the logistics of resetting the stored query data when new users are registered/added so the query stays current. (Feel free to call me out if this is an absurd concept).

I wonder if a closer look at the way it works for Tags would help? I mean, surely some sites have a LOT of Tags, so perhaps that scales ok.

Global User List

  • Should Site Admins get access to the global user list? Good question. My first instinct is to answer 'yes', if they have X information about the user already, such as a username, but then it would make autocomplete nearly moot. If 'no', then only Super Admins would have the ability to add users, which would probably be neither efficient nor reasonable. I guess it would be pertinent to look at use cases for why Site Admins would need access the user list or not.
  • Taking it from a different tack, it might be prudent to consider an option (instead of a decision) in this case. Super Admins already have the ability to enable Plugin administration menus, why not enable/disable Site Admin access to the global user list? Then it would become a question of whether that was network-wide or per-site.

I actually think I might agree with an option rather than decision in this case too. Some sites will be used like WordPress.com where global user list access might be useful(?), and some will prefer to keep their sites completely siloed.

comment:13 in reply to: ↑ 12 DrewAPicture2 years ago

Replying to japh:

Replying to DrewAPicture:

Autocomplete Libary

  • Based on most of the implementations I'm seeing, it looks like the standard jQuery Autocomplete library is pretty popular. Anybody have tested alternatives in mind?

At the moment suggest.js is being used in the admin, so unless there's a specific reason to go with Autocomplete instead then perhaps we see if that can be used? I'm working on a patch now just to see if that will transfer easily and be useable.

For an example of where this is used, check out Tags on the Edit Post page.

I really don't know enough about either of them to say whether one is more efficient than the other.

comment:14 in reply to: ↑ 12 DrewAPicture2 years ago

Replying to japh:

I wonder if a closer look at the way it works for Tags would help? I mean, surely some sites have a LOT of Tags, so perhaps that scales ok.

My thinking was more along the lines of having to query more information with users than terms. I'll poke around a bit too and we can figure out what's up.

Japh2 years ago

Modified patch from #18160 for better styling and closer appearance to Tags autocomplete UI

comment:15 boonebgorges2 years ago

  • Cc boonebgorges@… added

I think 19810.patch is missing the js file.

DrewAPicture2 years ago

Refreshed date in script loader and added user-new.dev.js

comment:16 markjaquith2 years ago

  • Keywords needs-refresh added

Patch needs a refresh after [19738] and [19739].

comment:17 Japh2 years ago

It would be good to add wp_is_large_network() in somewhere to test before running these queries. I thought perhaps in script-loader to determine whether or not to add the javascript for it at all, but wp_is_large_network() isn't available at that point at the moment. Any suggestions?

Japh2 years ago

Refreshed for [19738] and [19739] as requested

comment:18 Japh2 years ago

  • Keywords needs-refresh removed

comment:19 Japh2 years ago

  • Keywords has-patch added

comment:20 DrewAPicture2 years ago

UI Note:

  • user-new labels/help tabs will probably need plural ambiguity since autocomplete actually allows for adding multiple users at once. This goes for anywhere we duplicate this functionality.

comment:21 follow-up: markjaquith2 years ago

Let's lose the multi-user stuff. It is out of scope, and it will involve more UI work. Instead just fill the e-mail address into the field when they select.

comment:22 in reply to: ↑ 21 jane2 years ago

Replying to markjaquith:

Let's lose the multi-user stuff. It is out of scope, and it will involve more UI work. Instead just fill the e-mail address into the field when they select.

Yes, please. That can be a future improvement if desired. Let's keep initial passes focused, small, and in scope.

comment:23 Japh2 years ago

Just to clarify, the multi-user stuff was from the patch on #18160 which was used as a starting point. The main changes that are in the patch here are CSS and refreshing for compatibility with changes that have happened since.

I will submit a new patch later today with the changes to simplify the functionality.

markjaquith2 years ago

Patch didn't apply cleanly. Please watch out for "no newline" stuff and keep it out of your patches.

markjaquith2 years ago

Remove multi-user-add. Fix throbber-that-never-dies on empty result set.

comment:24 markjaquith2 years ago

Reworked the patch. Removed the multi-user-add stuff. Changed the throbber to be CSS-based (fixes a bug where it'd get stuck on). Removed JS that wasn't needed. Please test!

comment:25 nacin2 years ago

var ainput = $('#adduser-email'); needs to be inside the document.ready.

comment:26 follow-up: nacin2 years ago

Also, wp_ajax_autocomplete_user() should now use wp_die() to send a response. [19801], #15327

comment:27 in reply to: ↑ 26 DrewAPicture2 years ago

Replying to nacin:

var ainput = $('#adduser-email'); needs to be inside the document.ready.

Also, wp_ajax_autocomplete_user() should now use wp_die() to send a response. [19801], #15327

Noted

comment:28 danielbachhuber2 years ago

  • Cc wordpress@… added

nacin2 years ago

comment:29 follow-up: nacin2 years ago

19810.diff is a refresh.

It leverages get_users() rather than WP_User_Query, which like get_posts() simply returns results (cast to an array, which makes it easy for foreach loops).

Introduces a new WP_User_Query argument, search_columns, to allow for explicit column choices (of a limited indexed set, for now), to bypass the column auto-detection code, which is nifty but does not work well for autocomplete.

comment:30 nacin2 years ago

Not considered in the refresh:

  1. That this exposes logins and emails to regular admins. "Discuss whether regular site Administrators should get autocomplete on the global users list." -> Thinking no. We should limit this to the network admin, and perhaps user-new but only for supes. (Can drop a filter on it for other networks that may want it.)
  1. There's no cap check on the ajax request.

comment:31 in reply to: ↑ 29 DrewAPicture2 years ago

19810.7.patch works off of 19810.diff

Adds a promote_users cap check, a first effort at adding user autocomplete to site-users.php and a couple other small bits. Also added an is_super_admin check to the enqueue call for user-new.php.

Having issues with getting a properly formed user array from the network admin side in ajax-actions.php which feeds into the exclude array of the 2nd get_users query.

DrewAPicture2 years ago

Adds autocomplete to site-users.php, several other small bits

PeteMall2 years ago

Patch refreshed for [19831]

PeteMall2 years ago

Added better cap checks and filter for allowing autocomplete in site admin for site admins.

PeteMall2 years ago

Added the missing js file.

comment:32 PeteMall2 years ago

Known Issues: User exclusion is broken in network admin site-users screen.

PeteMall2 years ago

Fixes user exclusion in network admin.

comment:33 nacin2 years ago

In [19882]:

Add search_columns arg to WP_User_Query to allow for explicit column choices. Without it, the columns will be detected based on the search term. see #19810.

PeteMall2 years ago

Refreshed and cleaned up.

koopersmith2 years ago

Cleans up the JS/CSS and removes unnecessary code.

comment:34 koopersmith2 years ago

Would be nice to spruce up the general autocomplete styles, but this should be a good start.

comment:35 PeteMall2 years ago

  • Keywords commit added

comment:36 markjaquith2 years ago

In [19897]:

Autocomplete for add-user screens in multisite. props boonebgorges, Japh, DrewAPicture, PeteMall, nacin, koopersmith, markjaquith. see #19810.

comment:37 koopersmith2 years ago

In [19903]:

Trailing commas make JS cry. see #19810.

comment:38 DrewAPicture2 years ago

Just for posterity, here's an update of where we're at.

This is the status of the 7 user-related fields we're working on:

Dashboard:

  • [DONE] E-mail or Username field under "Add Existing User" on Users > Add New
  • Search Users input on Users Listing screen

Network Admin:

  • Users field on "Right Now" Dashboard widget
  • Search Users input on Users Listing screen
  • Search Users input on Edit Site > Users tab
  • [DONE] Username field under "Add Existing User" on Edit Site > Users tab
  • [PETE] Admin Email on Sites > Add New Site screen

Japh brings up a valid point and it is that we're actually working with 3 separate types of searches here:

  1. Get users NOT on the current site
  2. Get users ONLY on the current site
  3. Get ALL network users

Japh2 years ago

A modification to allow the autocomplete to be used for more purposes, such as search fields. This should also make it easier to add availability checking too.

comment:39 Japh2 years ago

The above mentioned patch now services the user search fields on the Network Admin -> Users page and the Site -> Users page.

We should then be able to add this feature to any user field just by adding the ID to the appropriate selector in the user-search.js

comment:40 DrewAPicture2 years ago

19810.14.patch adds user-search-input id to the Search Users field in 'Right Now' in Network admin.

UI Note: The 'Right Now' search fields have a size of 17 but might look better with something more like 25. Thought we'd leave it up to you folks to decide.

New status of the 7 user-related fields we're working on:

Dashboard:

  • [DONE] E-mail or Username field under "Add Existing User" on Users > Add New
  • [DONE] Search Users input on Users Listing screen

Network Admin:

  • [DONE] Users field on "Right Now" Dashboard widget
  • [DONE] Search Users input on Users Listing screen
  • [DONE] Search Users input on Edit Site > Users tab
  • [DONE] Username field under "Add Existing User" on Edit Site > Users tab
  • [Waiting on PeteMall] Admin Email on Sites > Add New Site screen

DrewAPicture2 years ago

Enqueues user-search and adds input id on dashboard.php for "Right Now" user search

DrewAPicture2 years ago

Increments onto 19810.14.patch, ads 1st pass at site search autocomplete

Japh2 years ago

Adjustment to the first pass at implementing site autocomplete. It will now user the search term in the query, and inserts the domain for searching.

comment:41 Japh2 years ago

It would be good for someone to take a look at the query used for autocomplete sites. Also, in the next patch, perhaps we should break out autocomplete for sites from users.

Breaking them apart will make it easier to do the correct is_large_network() test etc.

comment:42 follow-up: DrewAPicture2 years ago

Site Autocomplete fields:

  • 'Search Sites' input in Network Admin > Sites
  • 'Search Sites' input in Network Admin > Dashboard > Right Now

comment:43 follow-up: dd322 years ago

19810.16.patch

You'll probably want to throw like_escape() around the $_REQUEST['term'], and also stripslash it.

'label' => sprintf( __( '%1$s' ), $blogname ) - Does the blogname really need to be translatable?

I'd also suggest that get_blog_option() (or one of the other option functions that accepts a $blog_id) should be used instead of the raw Query, however, I believe that has a large overhead as it uses switch_to_blog(), so that might not be feasible.

wp_ajax_autocomplete_site() - I'd suggest doing:

$sites = ...
if ( empty($sites) )
  wp_die(..)

foreach ()...

to minimise the nesting levels.

DrewAPicture2 years ago

Refined query in autocomplete_site to format similarly to autocomplete_user, returned several waywardly removed filters, nearly #thereforebeer

comment:44 DrewAPicture2 years ago

19810.17.patch Adds domain to returned site values in the form of Site name (domain) also returns some accidentally removed filters.

TODO: Figure out renaming user-search.js or moving the the site-search js out to a separate file.

Will look at @dd32's suggestions next.

Japh2 years ago

Implemented @dd32's suggestions

comment:45 Japh2 years ago

Thanks for the suggestions @dd32, I've implemented them all now in patch 18

comment:46 in reply to: ↑ 43 Japh2 years ago

Replying to dd32:

19810.16.patch

You'll probably want to throw like_escape() around the $_REQUEST['term'], and also stripslash it.

'label' => sprintf( __( '%1$s' ), $blogname ) - Does the blogname really need to be translatable?

I'd also suggest that get_blog_option() (or one of the other option functions that accepts a $blog_id) should be used instead of the raw Query, however, I believe that has a large overhead as it uses switch_to_blog(), so that might not be feasible.

wp_ajax_autocomplete_site() - I'd suggest doing:

$sites = ...
if ( empty($sites) )
  wp_die(..)

foreach ()...

to minimise the nesting levels.

get_blog_option() seems to have improved performance, and from a look over the function, doesn't appear to use switch_to_blog().

comment:47 in reply to: ↑ 42 DrewAPicture2 years ago

Status update

Site Autocomplete fields:

  • [DONE] 'Search Sites' input in Network Admin > Sites
  • [DONE] 'Search Sites' input in Network Admin > Dashboard > Right Now

Japh2 years ago

Separated autocomplete for sites and users to allow proper loading / checking

comment:48 Japh2 years ago

In patch 19 I've split the user and site scripts from each other so that is_large_network() can be used properly. This way if a network is only large on one of them, autocomplete will still work on the other.

comment:49 Japh2 years ago

Somehow it appears that the filter "autocomplete_users_for_site_admins" isn't actually defined anywhere... perhaps @PeteMall could re-add it to the latest patch?

comment:50 duck_2 years ago

Added 19810.site-admin-logic.diff to fix some brokenness with the autocomplete_users_for_site_admins filter.

In wp-admin/includes/ajax-actions.php a site admin can perform the user search even without any filters being added because we're missing "!". Also wrapped the && clause in parens for clarity.

In wp-admin/user-new.php added parens around the || clause to combat && vs. || precedence. Currently it's possible for the script to be loaded on single site if someone has added add_filter( 'autocomplete_users_for_site_admins', '__return_true' );... stupid, but it shouldn't happen.

comment:51 duck_2 years ago

In [19934]:

Correct faulty logic when dealing with autocomplete_users_for_site_admins, and break logic into multiple lines. See #19810.

Japh2 years ago

Patch including changes from [19934], and applying them across the other new user and site autocomplete areas

comment:52 Japh2 years ago

Patch 20 is an adjustment to the cap checks in the ajax actions, as well as the reverse logic version of those at each instance the script is to be enqueued.

comment:53 DrewAPicture2 years ago

Attached 19810.21.patch accomplishes several things:

  • Removes all instances of the (undefined) autocomplete_sites_for_site_admins filter as the only site-searching fields affected are in the network admin, which is only accessible by super admins
  • Removes several uses of the autocomplete_users_for_site_admins filter in network admin files as network admin pages are only accessible by super admins
  • Adds is_network_admin cap checks to the wp-admin/includes/dashboard.php enqueues, without which the scripts would load on individual sites' dashboard.php for no valid purpose
  • Whitespacing fixes in several affected code blocks
Last edited 2 years ago by DrewAPicture (previous) (diff)

DrewAPicture2 years ago

Removes all uses of autocomplete_sites_for_site_admins filter, other cap checks, basic whitespacing fixes.

comment:54 follow-up: Japh2 years ago

Waiting on commit. If this isn't ready for commit, can someone please provide feedback so we can get it ready?

comment:55 in reply to: ↑ 54 westi2 years ago

Replying to Japh:

Waiting on commit. If this isn't ready for commit, can someone please provide feedback so we can get it ready?

This looks pretty good (I've not tested the functionality just read the code)

Some feedback:

You have lots of code like this:

if ( is_multisite() 
	        && current_user_can( 'promote_users' ) 
	        && ! wp_is_large_network( 'users' ) 
	        && is_super_admin() 
	) { 

If it is in a file in wp-admin/network then I don't see why we need to check is_multisite() or is_super_admin() and removing those checks will simplify the code and then the if can be on a single line.

I think wp_ajax_autocomplete_site should probably have a nonce check so as to avoid any chance of someone DOSing a site when a logged in Super Admin visits a page elsewhere.

comment:56 DrewAPicture2 years ago

  • Keywords needs-refresh added

Been a few changes since 19810.21.patch was submitted. Need to refresh and work over westi's suggestions.

comment:57 markjaquith2 years ago

In [20279]:

Autocomplete site names in Network Admin. More user completion areas. props Japh, DrewAPicture. see #19810.

comment:58 nacin2 years ago

In [20332]:

Clean up cap checks for autocompletes for sites and users in a network. see #19810.

comment:59 SergeyBiryukov2 years ago

  • Keywords needs-refresh removed

comment:60 ryan2 years ago

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.