WordPress.org

Make WordPress Core

Opened 11 months ago

Closed 7 months ago

Last modified 7 months ago

#37102 closed task (blessed) (fixed)

Replace get_blog_details with get_site

Reported by: spacedmonkey Owned by: jeremyfelt
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.6
Component: Networks and Sites Keywords: has-patch
Focuses: multisite Cc:

Description (last modified by jeremyfelt)

Currently there are lots of calls to the get_blog_details function. Now that get_site is in place, we should place that with the new function. Only calls that get the site by id can be replaced.

Attachments (3)

37102.patch (6.0 KB) - added by spacedmonkey 11 months ago.
37102.1.patch (740 bytes) - added by spacedmonkey 11 months ago.
37102.diff (2.3 KB) - added by jeremyfelt 7 months ago.

Download all attachments as: .zip

Change History (41)

#1 @flixos90
11 months ago

Reminder: We can only start replacing get_blog_details() when #36935 is in place.

#2 @spacedmonkey
11 months ago

@flixos90 my first patch only replaces code that doesn't require any of the none standard wp_site properties, like blogname.

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


11 months ago

#4 @spacedmonkey
11 months ago

In WP_Importer class uses get_blog_details. 37102.1.patch replaces it with get_sites.

#5 @MikeHansenMe
11 months ago

  • Keywords has-patch added

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


11 months ago

#7 @jeremyfelt
11 months ago

  • Milestone changed from Awaiting Review to 4.6

Moving this to the 4.6 milestone to see what progress we can make over the next few days. Most likely there will be many uses that we aren't able to get replaced properly until 4.7.

Anywhere that we replace get_blog_details() with get_site() should ideally have tests that pass before and after. It will make sense to create individual tickets for many of these replacements.

This ticket was mentioned in Slack in #core by ocean90. View the logs.


11 months ago

#9 @jeremyfelt
11 months ago

  • Keywords 4.7-early added
  • Milestone changed from 4.6 to Future Release

After discussing status during beta 1 bug scrub tonight, I'm pushing this back to future release. There's a lot to do and a lot of unit tests to get right before swapping things out.

Let's pick it up in 4.7-early. I'm in favor of individual tickets for many of these. Ticket containing description of how get_blog_details() is used in the function, tests or confirmation of tests for the function, and the replacement.

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


11 months ago

#11 @jeremyfelt
9 months ago

  • Keywords 4.7-early removed
  • Milestone changed from Future Release to 4.7
  • Owner set to jeremyfelt
  • Status changed from new to accepted
  • Type changed from enhancement to task (blessed)

Let's start working on this.

Repeating:

"Anywhere that we replace get_blog_details() with get_site() should ideally have tests that pass before and after. It will make sense to create individual tickets for many of these replacements."

This ticket was mentioned in Slack in #core by desrosj. View the logs.


8 months ago

#13 @desrosj
8 months ago

Anyone able to make some progress on this? Per bugscrub chat, it will most likely span a few releases, but for the start to make 4.7, we need to get to work on it.

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


8 months ago

#15 @flixos90
8 months ago

  • Owner changed from jeremyfelt to iamfriendly
  • Status changed from accepted to assigned

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


8 months ago

#17 @iamfriendly
8 months ago

get_blog_details() occurrences in core.

Found by running grep -rnw . -e "get_blog_details("

I've added recommendations for each replacement based on what params (if any) are passed to get_blog_details()
Also placed an empty bullet point on each one for a ticket number so that we're able to track the overall progress in this ticket, but each replacement should have its own ticket (a couple can be combined).


01. ./wp-admin/includes/class-wp-importer.php:141: $blog = get_blog_details( array( 'domain' => $parsed['host'], 'path' => $parsed['path'] ) );


02. ./wp-admin/includes/ms.php:69: $blog = get_blog_details( $blog_id );


03. ./wp-admin/ms-delete-site.php:27: $blog = get_blog_details();


04. ./wp-admin/my-sites.php:26: $blog = get_blog_details( (int) $_POST['primary_blog'] );


05. ./wp-admin/network/site-info.php:44: $details = get_blog_details( $id );


06. ./wp-admin/network/site-info.php:91: $existing_details = get_blog_details( $id, false );


07. ./wp-admin/network/site-info.php:104: $new_details = get_blog_details( $id, false );


08. ./wp-admin/network/site-settings.php:41: $details = get_blog_details( $id );


09. ./wp-admin/network/site-themes.php:64: $details = get_blog_details( $id );


10. ./wp-admin/network/site-users.php:57: $details = get_blog_details( $id );


11. ./wp-admin/network/sites.php:97: $site_details = get_blog_details( $id );


12. ./wp-includes/class-wp-xmlrpc-server.php:4376: $current_blog = get_blog_details();


13. ./wp-includes/ms-blogs.php:41: $bloginfo = get_blog_details( (int) $blog_id );

  • within get_blogaddress_by_id()
  • Recommendation: get_site()
  • Multiple unit tests (i.e. test_get_blogaddress_by_id_with_valid_id() and test_get_blogaddress_by_id_with_invalid_id())
  • Ticket: 38351 https://core.trac.wordpress.org/ticket/38351

14. ./wp-includes/ms-blogs.php:267: $details = get_blog_details( $blog_id, false );

  • within refresh_blog_details()
  • Recommendation: get_site()
  • No unit tests (and neither does clean_blog_cache() also used within that function - possibly a good first bug)
  • Ticket: 38351 https://core.trac.wordpress.org/ticket/38351

15. ./wp-includes/ms-blogs.php:311: $current_details = get_blog_details($blog_id, false);


16. ./wp-includes/ms-blogs.php:1040: $details = get_blog_details( $id, false );


17. ./wp-includes/ms-deprecated.php:29: return get_blog_details( $blog );

  • within (deprecated) get_dashboard_blog()
  • Recommendation: get_site() (I _think_ the dashboard_blog site option is a site ID)
  • Has unit test test_get_dashboard_blog()
  • When updating, would we also update braces on the conditional and yoda condition (even though this is deprecated)?
  • Could be combined with 18
  • Ticket: 38354 https://core.trac.wordpress.org/ticket/38354

18. ./wp-includes/ms-deprecated.php:31: return get_blog_details( get_current_site()->blog_id );


19. ./wp-includes/ms-functions.php:56: $primary = get_blog_details( $first_blog->userblog_id );


20. ./wp-includes/ms-functions.php:58: $primary = get_blog_details( $primary_blog );


21. ./wp-includes/ms-functions.php:74: $details = get_blog_details( $blog_id );


22. ./wp-includes/ms-functions.php:164: $details = get_blog_details($blog_id);


23. ./wp-includes/ms-load.php:87: $blog = get_blog_details();


24. ./wp-includes/user.php:624: $blog = get_blog_details( 1 );


25. ./wp-includes/user.php:654: $blog = get_blog_details( $blog_id );


26. ./wp-includes/user.php:722: $blog = get_blog_details( $blog_id );

Last edited 7 months ago by iamfriendly (previous) (diff)

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


8 months ago

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


7 months ago

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


7 months ago

#21 @jeremyfelt
7 months ago

  • Description modified (diff)

We made some great progress today in this discussion on Slack and created several new tickets to encourage incremental progress.

I'd like to keep this ticket attached to the 4.7 milestone until the enhancements deadline on October 26 to continue to encourage eyes on the individual tickets. We can then bring it back to the 4.8 milestone once that is created.

#22 @jeremyfelt
7 months ago

In 38820:

Multisite: Replace get_blog_details() in WP_Importer::set_blog() with get_site().

Props spacedmonkey, iamfriendly.
See #37102.
Fixes #38345.

#23 @jeremyfelt
7 months ago

In 38821:

Multisite: Replace get_blog_details() usage in wpmu_delete_blog() with get_site().

Props spacedmonkey, iamfriendly.
See #37102.
Fixes #38346.

#24 @jeremyfelt
7 months ago

In 38822:

Multisite: Replace get_blog_details() in /wp-admin/ms-delete-site.php with get_site().

Props spacedmonkey, iamfriendly.
See #37102.
Fixes #38347.

#25 @jeremyfelt
7 months ago

In 38823:

Multisite: Replace get_blog_details() in wp-admin/my-sites.php with get_site().

Props spacedmonkey, iamfriendly.
See #37102.
Fixes #38348.

#26 @jeremyfelt
7 months ago

In 38824:

Multisite: Replace get_blog_details() with get_site() in network admin screens.

Props iamfriendly.
See #37102.
Fixes #38349.

#27 @jeremyfelt
7 months ago

The first wave is in.

Remaining: #38350, #38351, #38353, #38355, #38356, #38357, and #38358.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


7 months ago

This ticket was mentioned in Slack in #core by jeremyfelt. View the logs.


7 months ago

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


7 months ago

#31 @jeremyfelt
7 months ago

Commits I forgot to tag this ticket in: [38902], [38903], [38904], [38905], [38913], and [38914].

#32 @jeremyfelt
7 months ago

In 38915:

Multisite: Replace get_blog_details() in wp-includes/ms-blogs.php with get_site().

Props flixos90.
See #37102.
Fixes #38351.

#33 @jeremyfelt
7 months ago

Remaining items:

  • Deprecate blog_details filter. #38491.
  • Adjust any documentation that may be confusing and refers to get_blog_details(). That can happen on this ticket.

#34 @flixos90
7 months ago

  • Owner changed from iamfriendly to jeremyfelt
  • Status changed from assigned to reviewing

I opened #38497 to replace get_blog_details() with get_site() in unit tests.

Thus, left to commit are #38350, #38497 and #38491

#35 @jeremyfelt
7 months ago

In 38936:

Multisite: Deprecate the blog_details filter.

get_site() has replaced get_blog_details() throughout core and is the recommended way to retrieve a current or single site object.

The blog_details filter is applied when full details are requested from get_blog_details().

To ensure backwards compatibility in the switch to get_site(), this blog_details filter is now applied in WP_Site::get_details() and marked as deprecated with a note to rely instead on the site_details filter introduced in 4.6.

Props flixos90.
See #37102.
Fixes #38491.

@jeremyfelt
7 months ago

#36 @jeremyfelt
7 months ago

37102.diff adjusts inline documentation referring to get_blog_details().

#37 @jeremyfelt
7 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 38943:

Multisite: Replace get_blog_details() in inline documentation.

Some documentation is now out of date and some can be replaced with a mention of get_site().

Fixes #37102.

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


7 months ago

Note: See TracTickets for help on using tickets.