Use `get_sites()` in `get_blogs_of_user()`

[flixos90]

Now that we have get_sites() merged (see #35791), we should use this function in get_blogs_of_user(). By adjusting this...

• the function would only need to find the site IDs of that user and then make a query using get_sites() and the site__in argument (plus additional arguments for non-archived/non-spam/non-deleted if $all is false)
• the function would then return an array of proper WP_Site objects instead of an array of stdClass

We should probably wait for #36717 and #36935 to be completed and merged first before we start playing around with a patch here. By that...

• we'd no longer need to use get_blog_details()
• we could add userblog_id as a magic property in WP_Site that returns $blog_id as an integer

This should keep it backwards-compatible and make the function a lot less cluttered.

[flixos90]

37061.diff​ uses get_sites() in get_blogs_of_user() and gets rid of the juggling with raw objects. Since the objects use a property called $userblog_id (?), this property is now added as a magic property in WP_Site to remain compatible. get_blog_details() is no longer necessary either since these properties are lazy loaded automatically now (related: #37102).

I also checked in Core where and how this function is used, and in no case is there any weird array transformation of objects that would cause problems with lazy-loaded properties. :)

A minor issue is that on a non-multisite, the array will still contain a raw object since WP_Site is not available there, so one has to be aware of that. However, I still think we should do this as it's an improvement for Multisite setups.

[jorbin]

Replying to flixos90:

A minor issue is that on a non-multisite, the array will still contain a raw object since WP_Site is not available there, so one has to be aware of that. However, I still think we should do this as it's an improvement for Multisite setups.

I don't think having something different returned under MS from single site is a good decision. Seems like something that could cause plugins to have bugs only under one or the other. Or for code originally written for MS to not work and cause confusion when run on single site installs.

[flixos90]

Replying to jorbin:

I don't think having something different returned under MS from single site is a good decision. Seems like something that could cause plugins to have bugs only under one or the other. Or for code originally written for MS to not work and cause confusion when run on single site installs.

I think the real problem is that we are loading the WP_Site class on Multisite only. I think we should made it compatible with non-multisite installs and load it on every setup which would allow us to fix this issue consistently. After all, every setup has a site - only in a Multisite there is a WP_Network, but I think the WP_Site class can be useful on a regular setup as well. Let's think about this and maybe open a ticket.

[DrewAPicture]

Replying to flixos90:

Replying to jorbin:

I don't think having something different returned under MS from single site is a good decision. Seems like something that could cause plugins to have bugs only under one or the other. Or for code originally written for MS to not work and cause confusion when run on single site installs.

I think the real problem is that we are loading the WP_Site class on Multisite only. I think we should made it compatible with non-multisite installs and load it on every setup which would allow us to fix this issue consistently. After all, every setup has a site - only in a Multisite there is a WP_Network, but I think the WP_Site class can be useful on a regular setup as well. Let's think about this and maybe open a ticket.

+1 for exploring global use of WP_Site. Care to open a ticket?

[flixos90]

Ticket opened: #37948

[jeremyfelt]

We had a good conversation around this today during multisite office hours. Here's a quick final summary:

Let's avoid adding userblog_id as a magic (deprecated) property to WP_Site and instead stick with returning an array of standard objects populated via WP_Site objects in a similar way to they are now from the DB. This keeps our options open for the future and keeps the changeset clean for both multisite and single site.

For reference, userblog_id has existed since ​MU:63.

[flixos90]

37061.2.diff​ is an updated patch that preserves the original return types (raw objects including the property userblog_id). The function still uses get_sites() as proposed in this ticket.

While I was at it, I adjusted some docs and variable names to properly reflect our current multisite naming conventions. Doing that, I noticed that the docs of the pre_get_blogs_of_user filter state that the returned value should be an array of WP_Site objects which is incorrect, so I changed it to simply "site objects" as well.

[jeremyfelt]

@flixos90, looking good, thanks!

I would normally want to avoid the churn of blog -> site naming in a change like this, but since we're modifying such a large part of the function and introducing a new loop through $site_ids and $_sites I think we're okay. Just noting. :)

A couple of things:

• There should be an additional @since 4.7.0 doc rather than using @internal.
• Two tests are failing for me in current trunk - Tests_AdminBar::test_admin_bar_contains_correct_about_link_for_users_with_no_role_in_multisite and Tests_AdminBar::test_admin_bar_contains_correct_links_for_users_with_no_role_on_network. I have not poked around further than that to see why.

[flixos90]

Replying to jeremyfelt:

I would normally want to avoid the churn of blog -> site naming in a change like this, but since we're modifying such a large part of the function and introducing a new loop through $site_ids and $_sites I think we're okay. Just noting. :)

I was hoping you would say that for this case. :) Generally I'm aware of it.

• There should be an additional @since 4.7.0 doc rather than using @internal.
• Two tests are failing for me in current trunk - Tests_AdminBar::test_admin_bar_contains_correct_about_link_for_users_with_no_role_in_multisite and Tests_AdminBar::test_admin_bar_contains_correct_links_for_users_with_no_role_on_network. I have not poked around further than that to see why.

37061.3.diff​ has fixes for these; the latter were caused by missing a ! empty( $site_ids ) check which led to get_sites() returning all sites, which is something I still need to get used to, see #33341 :)

[jeremyfelt]

In 38682:

Multisite: Use get_sites() in get_blogs_of_user().

Previously, we looped through all of the site IDs found via user meta and ran get_blog_details() for each site. Now, we can pass all of the site IDs at once to get_sites() and receive full WP_Site objects.

To avoid possible issues with return type, sites are still processed into a standard object that also has a userblog_id property attached. Nothing is changed for non-multisite configurations.

Props flixos90.
Fixes #37061.