WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#41684 closed defect (bug) (fixed)

Replace $wpdb->blogid with get_current_blog_id()

Reported by: spacedmonkey Owned by: jeremyfelt
Milestone: 4.9 Priority: normal
Severity: normal Version: 3.1
Component: Networks and Sites Keywords: good-first-bug has-patch
Focuses: multisite Cc:
PR Number:

Description

Similar to #41507 currently the class level property of blogid is reference through the code. These should be replaced with get_current_blog_id() function calls.

Attachments (9)

41684.patch (8.6 KB) - added by bnap00 2 years ago.
41684-2.patch (8.6 KB) - added by bnap00 2 years ago.
41684-3.patch (8.6 KB) - added by bnap00 2 years ago.
final patch
41684-4.patch (8.2 KB) - added by bnap00 2 years ago.
41684-5.patch (7.7 KB) - added by bnap00 2 years ago.
41684.2.patch (11.3 KB) - added by spacedmonkey 2 years ago.
41684.diff (7.9 KB) - added by jeremyfelt 2 years ago.
41684.2.diff (6.9 KB) - added by jeremyfelt 2 years ago.
41684.3.diff (7.9 KB) - added by jeremyfelt 2 years ago.

Download all attachments as: .zip

Change History (23)

@bnap00
2 years ago

@bnap00
2 years ago

@bnap00
2 years ago

final patch

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


2 years ago

#2 @spacedmonkey
2 years ago

Thanks for the patches @bnap00 . Some quick notes.

  • In the switch_to_blog / restore tests, put the $wpdb->blogid. The test is testing that value has changed. That is one example that we can't remove it, as it invalidates the test.
  • Check all functions, if that function no longer needs the global $wpdb and update the documentation. See wpmu_update_blogs_date and ms_upload_constants as example.
  • Restore wp_get_db_schema back to $wpdb->blogid. This seems to be a special case where get_current_blog_id will not work as it is loaded early.

@bnap00
2 years ago

@bnap00
2 years ago

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


2 years ago

#4 @janw.oostendorp
2 years ago

  • Keywords has-patch added; needs-patch removed

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


2 years ago

#6 @flixos90
2 years ago

  • Milestone changed from Awaiting Review to Future Release

#7 @jeremyfelt
2 years ago

  • Milestone changed from Future Release to 4.9
  • Owner set to bnap00
  • Status changed from new to assigned

Thanks for your work on this, @bnap00.

In cases where value is used more than once in a function, and where extra clarity is not needed, I think it's safe to store locally for reuse ($site_id = get_current_blog_id()) instead of calling the function multiple times. The best (maybe the only) example is probably src/wp-admin/includes/upgrade.php.

And, less important, return $blogs[get_current_blog_id()]; can be updated to return $blogs[ get_current_blog_id() ]; for coding standards.

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


2 years ago

#9 @spacedmonkey
2 years ago

In 41684.2.patch I fixed the last of the formatted issues raised by @jeremyfelt .

#10 @jeremyfelt
2 years ago

  • Keywords needs-unit-tests removed
  • Owner changed from bnap00 to jeremyfelt
  • Status changed from assigned to reviewing

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


2 years ago

@jeremyfelt
2 years ago

@jeremyfelt
2 years ago

@jeremyfelt
2 years ago

#12 @jeremyfelt
2 years ago

In 41684.3.diff:

  • Fix the order of the arguments passed to $wpdb-prepare() in the 3rd query.
  • In upgrade_280(), call refresh_blog_details() without an argument, which will refresh the current blog anyway.
  • Move $site_id = get_current_blog_id() into conditional blocks where appropriate.

#13 @jeremyfelt
2 years ago

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

In 41661:

Multisite: Use get_current_blog_id() instead of $wpdb->blogid.

get_current_blog_id() is more appropriate for determining the ID of the current site in most cases. This eliminates the need for the global $wpdb in several functions and is better than the implicit global used in admin pages.

Props bnap00, spacedmonkey.
Fixes #41684.

#14 @ocean90
2 years ago

Follow-up: #42641

Note: See TracTickets for help on using tickets.