Make WordPress Core

Opened 7 years ago

Last modified 7 years ago

#40511 new enhancement

get_blogs_of_user should return an array of WP_Site objects

Reported by: spacedmonkey's profile spacedmonkey Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Networks and Sites Keywords:
Focuses: multisite Cc:

Description

Currently the get_blogs_of_user uses get_sites to get list of sites. The code loops through the list and creates std objects. However, this is now needed. It is much better to return a list of WP_Sites.

Attachments (2)

40511.diff (1.5 KB) - added by spacedmonkey 7 years ago.
40511.2.diff (1.6 KB) - added by spacedmonkey 7 years ago.

Download all attachments as: .zip

Change History (7)

@spacedmonkey
7 years ago

#1 @johnjamesjacoby
7 years ago

This seems OK to me.

@spacedmonkey
7 years ago

#2 @spacedmonkey
7 years ago

40511.2.diff

Force get_details to run. Just in case anyone was using get_object_vars on the object, because the magic _get will not work on that :(

#3 @flixos90
7 years ago

This was previously considered in #37061 (particularly https://core.trac.wordpress.org/ticket/37061#comment:8). While I certainly like the idea, I think we should hold off on this change. Adding userblog_id as a magic property makes sense and addresses most backward-compatibility concerns, however two issues remain:

  • WP_Site is not available outside of multisite, and I don't like the idea of the function having different return types depending on whether it's in multisite or not.
  • Something like $blogs = get_blogs_of_user( $user_id ); $site_data = get_object_vars( $blogs[0] ); echo $site_data['userblog_id']; on a returned object works currently, but wouldn't work with a WP_Site object. It's really annoying that that weird userblog_id property exists in the first place, but now it's there. I admit this is really nitpicking / edge-case, and maybe we can ignore it, but it is something to consider.

Especially the first point makes me wary of this change. In my opinion it's worth revisiting once we possibly decide to make some site functionality available on non-multisite (see #37948 for previous thoughts on this).

#4 @spacedmonkey
7 years ago

In 40511.2.diff I forced the magic _get to run, fixing the get_object_vars issue.

I agree with the point that it is a little weird to have two different results. The reason I opened this ticket was the following reason.

In the admin bar, you have a list of your sites. To display a list of sites, switch_to_blog is used. Related to #40513 want to pass the WP_Site object where it is possible. Passing the WP_Site to switch_to_blog gives much more context to the switching. I have ticket that explains it better here.

Also, when listing sites, we really should be using WP_site objects, it was really confusing to find out it didn't.

This ticket was mentioned in Slack in #core-multisite by spacedmonkey. View the logs.


7 years ago

Note: See TracTickets for help on using tickets.