10up / distributor

Share content between your websites.
https://distributorplugin.com
GNU General Public License v2.0
633 stars 155 forks source link

For non-super-admins, use get_blogs_of_user() to determine authroized sites. #1179

Open peterwilsoncc opened 9 months ago

peterwilsoncc commented 9 months ago

Description of the Change

Modifies the query used to determine a user's authorized sites to use get_blogs_of_user().

As super admins have access to all sites without being a member, the existing get_sites() query is used for super admins.

Closes #1176

How to test the Change

  1. Install Distributor on a Multisite Install in sub-directory mode
  2. Make it big export XDEBUG_MODE=off; for i in {1..1050}; do wp site create --slug=$i --title="MS $i" --url=https://ms-distributor.local; done; (change the URL if needs be)
  3. Create a non super-admin user
  4. Add the new user as an administrator on sites 2, 3 and 1050
  5. Log in/switch to the new user
  6. Visit the pull screen of either site 2 or 3.
  7. The last site should be displayed (1050)

Creating the sites may take some time so you should leave the command going and switch tasks for a while.

Changelog Entry

Fixed - Improved database query for getting a users authorized sites on very large networks.

Credits

Props @jeffpaul, @boonebgorges, @peterwilsoncc.

Checklist:

kirtangajjar commented 4 months ago

@peterwilsoncc Tested and ensured that the PR is working for me.

kirtangajjar commented 4 months ago

@peterwilsoncc It seems that the 1000 site limit problem will still be there for super admins, so why not query for number of sites like this and store it in transients instead of passing a static number:

SELECT COUNT(blog_id) FROM {$wpdb->blogs} WHERE archived = '0' AND spam = '0' AND deleted = '0'
peterwilsoncc commented 3 months ago

Thanks for looking at this @kirtangajjar.

The limit of 1000 sites was added in 3077cf9ae6647fc0a15ceda9b4f02813aed9a728. Based on the commit message it was a coding standards fix to avoid a warning for an unlimited query.

I'd be surprised if many users were hitting the 1000 site limit but do you think it's worth adding a filter to allow developers to increase the size of the dropdown?

jeffpaul commented 3 months ago

Fwiw filtering for that seems like a good handling to me.