Make WordPress Core

Opened 9 years ago

Closed 8 years ago

#37528 closed enhancement (fixed)

Add network ID parameter to wp_update_network_site_counts()

Reported by: johnjamesjacoby's profile johnjamesjacoby Owned by: flixos90's profile flixos90
Milestone: 4.8 Priority: normal
Severity: normal Version: 4.6
Component: Networks and Sites Keywords: good-first-bug has-patch has-unit-tests commit
Focuses: multisite Cc:

Description

Now that wp_update_network_site_counts() uses get_sites() internally, we can now consider passing a network_id parameter through.

Currently, we assume $wpdb->siteid which I think is a fine fallback, though we could also consider using get_current_network_id() if we felt like not exposing the $wpdb global.

Attachments (3)

37528.patch (798 bytes) - added by PieWP 9 years ago.
Patch file
37528.2.patch (964 bytes) - added by johnjamesjacoby 8 years ago.
Refresh. Coding standards, and use update_network_option() so $count gets saved on the correct network
37528.diff (2.2 KB) - added by flixos90 8 years ago.

Download all attachments as: .zip

Change History (16)

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


9 years ago

@PieWP
9 years ago

Patch file

#2 follow-up: @PieWP
9 years ago

  • Keywords has-patch added; needs-patch removed

Thanks for your suggestion johnjamesjacoby, I've added a possible patch file

#3 in reply to: ↑ 2 @johnjamesjacoby
9 years ago

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

Replying to PieWP:

Thanks for your suggestion johnjamesjacoby, I've added a possible patch file

Great! Patch looks good! There are some very minor formatting issues that will get scrutinized before this is ready for core, but otherwise this matches what I was envisioning.

Thanks!

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


8 years ago

@johnjamesjacoby
8 years ago

Refresh. Coding standards, and use update_network_option() so $count gets saved on the correct network

#5 @johnjamesjacoby
8 years ago

  • Keywords commit added

#6 @johnjamesjacoby
8 years ago

@jeremyfelt this is good to go IMO

#7 @jeremyfelt
8 years ago

  • Keywords needs-unit-tests added; commit removed
  • Milestone changed from Awaiting Review to Future Release

Excellent. I'm going to push to Future Release and we can look at this for the 4.8 cycle.

In the meantime, we should see if tests make sense for this.

#8 @johnjamesjacoby
8 years ago

  • Keywords early added

I'm going to open another ticket that's related to this one, specifically about a bug. It's a more invasive fix across a few functions, but is reproducible.

Adding the "early" keyword so we come back to this in 4.8 right away.

#9 @johnjamesjacoby
8 years ago

See #38699 for relative bug.

#10 @flixos90
8 years ago

  • Keywords early removed
  • Milestone changed from Future Release to 4.8

#11 @flixos90
8 years ago

  • Keywords changed from good-first-bug, has-patch, needs-unit-tests to good-first-bug has-patch needs-unit-tests

See 40370 as well. The unit test test_get_blog_count_on_different_network() introduced there can be made a bit nicer once the new parameter on wp_update_network_site_counts() is introduced.

@flixos90
8 years ago

#12 @flixos90
8 years ago

  • Keywords has-unit-tests commit added; needs-unit-tests removed
  • Owner changed from jeremyfelt to flixos90

37528.diff adds unit tests and adjusts some documentation.

#13 @flixos90
8 years ago

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

In 40484:

Multisite: Add $network_id parameter to wp_update_network_site_counts().

Using the new parameter, it is now possible to update the site counts on a network different from the current network.

Props PieWP, johnjamesjacoby.
Fixes #37528. See #38699.

Note: See TracTickets for help on using tickets.