WordPress.org

Make WordPress Core

Opened 6 weeks ago

Last modified 12 days ago

#40180 assigned enhancement

Introduce `get_site_by()` function for multisite

Reported by: flixos90 Owned by: flixos90
Milestone: 4.8 Priority: normal
Severity: normal Version:
Component: Networks and Sites Keywords: has-patch dev-feedback has-unit-tests
Focuses: multisite Cc:

Description

As @stephdau mentioned on #40064, the new get_site() function from 4.6 is not a direct replacement for get_blog_details() which we are planning to deprecate.

Therefore I think it makes sense to introduce a function get_site_by() that is basically a wrapper for a WP_Site_Query (with LIMIT 1), and then runs array_shift() to only return the first result. This function would then become the direct replacement that people who previously relied on get_blog_details() with something other than an ID could use.

Parameter-wise, we should probably adjust it a little bit to match things like get_term_by(), which also makes the way that function works more explicit.

Attachments (1)

40180.diff (9.8 KB) - added by flixos90 5 weeks ago.

Download all attachments as: .zip

Change History (12)

#1 @stephdau
6 weeks ago

I'm liking the get_site_by() idea, from a processing standpoint.

I'm still not super fond of the idea of deprecating get_blog_details(), primarily because of the amount of person-hours that 3rd-parties will end up spending on moving to the new function, and users upgrading plugins/etc because of it (EG: I have 559 instances of the use of get_blog_details() outside of Core in the codebase I work on). But at least having a direct replacement means devs have more options (regex replacements, etc).

Keeping the full functionality around is ultimately what matters, so it's still a more viable option than the previous plan.

Thanks, @flixos90.

Last edited 6 weeks ago by stephdau (previous) (diff)

#2 @stephdau
6 weeks ago

There is one thing we'll need to contend with, for consistency, with other get_*_by() instances: get_user_by(), get_term_by(), etc all assume that the 1st parameter is a single $field, such as login (user) or slug (term).

In the case of get_blog_details(), that same equivalent parameter can accept an array of $fields, such as array( 'domain' => 'blah.com', 'path' => '/' );

#3 @stephdau
6 weeks ago

This will also have to be considered:

foreach ( get_site( 52723464 ) as $key => $value ) { echo "$key, "; }
blog_id, domain, path, site_id, registered, last_updated, public, archived, mature, spam, deleted, lang_id, ds_blog, ds_stats, 

foreach ( get_blog_details( 52723464 ) as $key => $value ) { echo "$key, "; }
blog_id, domain, path, site_id, registered, last_updated, public, archived, mature, spam, deleted, lang_id, ds_blog, ds_stats, blogname, siteurl, post_count, home, 

#4 @flixos90
6 weeks ago

@stephdau We can likely handle the parameters in the same way as a function like add_query_arg() works: You can either pass a key and a value as two separate parameters, or an array of key => value pairs (in that case the second parameter would be ignored). This makes the function flexible enough for these use-cases.

Thanks for bringing up the object iteration as well.

#5 @flixos90
5 weeks ago

  • Owner set to flixos90
  • Status changed from new to assigned

#6 @flixos90
5 weeks ago

In 40317:

Multisite: Add further unit tests for get_blog_details().

These tests verify that the returned site object is iterable and contains the expected properties.

See #40228, #40180.

@flixos90
5 weeks ago

#7 @flixos90
5 weeks ago

  • Keywords has-patch dev-feedback has-unit-tests added; 2nd-opinion removed

40180.diff is a possible implementation for get_site_by(), including unit tests. It accepts either a $field and its $value as parameters or a single array of fields and their values.

I decided to not make the function a one-to-one replacement for get_blog_details() as we're probably not going to deprecate that function. However, get_site_by() supports everything that get_blog_details() needs and does that in a much more efficient way, so I suggest we should use the new get_site_by() in get_blog_details() - all the latter needs to do then is tweak how the parameters are passed to it and modify the return value appropriately.

#8 @flixos90
5 weeks ago

I just added a patch on #40228 to demonstrate how this function could be used to improve get_blog_details() as well. My comment https://core.trac.wordpress.org/ticket/40228#comment:8 also goes a bit more into detail why I think that modifying get_blog_details() is not sufficient here and why it's worth introducing get_site_by().

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


3 weeks ago

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


12 days ago

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


12 days ago

Note: See TracTickets for help on using tickets.