Add status links to network/sites.php

[johnjamesjacoby]

Although sites have a "status" property, there is no interface for identifying what sites have which statuses, or their associated counts.

Screens that do have this:

• Posts
• Pages
• Users
• Comments
• Plugins
• Themes (network or list view)

[johnjamesjacoby]

No links in sites.php

[johnjamesjacoby]

Links in users.php

[johnjamesjacoby]

Links in edit.php

[flixos90]

@johnjamesjacoby Isn't this a duplicate of your own #37684? Or is there a difference I didn't get yet?

[johnjamesjacoby]

Replying to flixos90:

@johnjamesjacoby Isn't this a duplicate of your own #37684? Or is there a difference I didn't get yet?

That ticket is for inline states, in each table row.

[johnjamesjacoby]

First pass, introduces new wp_count_sites() function to ms-functions.php (akin to wp_count_posts())

[jeremyfelt]

I like the idea of having status links for sites. I didn't spend a lot of time with wp_count_sites(), but we should probably get some tests written for it. We'll also need to make sure the count cache is cleared when sites are created, updated with another status, or deleted.

[jorbin]

If this is going to make it into 4.7, it needs the unit tests that @jeremyfelt has requested.

[rheinardkorf]

Definitely needs unit tests.

The following is not a reliable method.

protected function is_base_request() {
		$vars = $_GET;
		unset( $vars['paged'] );

		if ( empty( $vars ) ) {
			return true;
		}

		return 1 === count( $vars ) && ! empty( $vars['mode'] );
	}

Plugins may add additional functionality via URI, so simply unsetting 'paged' or checking for only 'mode' is does not give the intended result. It is better to check for the presence/absence of 'site_status'.

The rest is looking pretty good.

[PieWP]

In regards to the wp_count_sites, why use the foreach to sum up the status instead of an aggregated query? Might cause issues when someone has many subsites within their network.

"SELECT 
     SUM(public) AS `public`, 
     SUM(archived) AS `archived`, 
     SUM(mature) AS `mature`, 
     SUM(spam) AS `spam`, 
     SUM(deleted) as `deleted` 
FROM {$wpdb->blogs} WHERE site_id = %d"

[flixos90]

We're too late in the release cycle to get this into 4.7 as it needs further though on the approach. Let's continue to work on it so that we can hopefully put it into 4.8.

Regarding the details, I agree with the previous comments. I also would like to investigate ways to enhance the existing get_blog_count() function for that usage as wp_count_sites(), while it works completely different in the patch implementation, sounds like it does the same. Related: #37865 (and then we could add other parameters to it and enhance how it works)

[rheinardkorf]

Replying to flixos90:

We're too late in the release cycle to get this into 4.7 as it needs further though on the approach. Let's continue to work on it so that we can hopefully put it into 4.8.

Regarding the details, I agree with the previous comments. I also would like to investigate ways to enhance the existing get_blog_count() function for that usage as wp_count_sites(), while it works completely different in the patch implementation, sounds like it does the same. Related: #37865 (and then we could add other parameters to it and enhance how it works)

Agreed. If we're going to make better use of site statuses it would also make sense to add other enhancements. E.g. filtering the statuses so that it can be extended.

[pbiron]

Guess I should have seached Trac before spending several hours today implementing almost exactly what @johnjamesjacoby did...it was a good learning experience for me, nonetheless :-)

[pbiron]

Replying to rheinardkorf:

Definitely needs unit tests.

The following is not a reliable method.

protected function is_base_request() {
		$vars = $_GET;
		unset( $vars['paged'] );

		if ( empty( $vars ) ) {
			return true;
		}

		return 1 === count( $vars ) && ! empty( $vars['mode'] );
	}

Plugins may add additional functionality via URI, so simply unsetting 'paged' or checking for only 'mode' is does not give the intended result. It is better to check for the presence/absence of 'site_status'.

The rest is looking pretty good.

This is what I came up with for is_base_request() which I think is more reliable:

protected function is_base_request() {
	$vars = $_GET;
	unset( $vars['paged'] );

	if ( empty( $vars ) ) {
		return true;
	} elseif ( 1 === count( $vars ) && ! empty( $vars['site_status'] ) ) {
		return false;
	}

	return 1 === count( $vars ) && ! empty( $vars['mode'] );
}

[pbiron]

Replying to rheinardkorf:

Definitely needs unit tests.

How does on go about writing unit tests for proposed patches?

I'd like to see this idea go forward, so I'd be willing to take a stab at writing unit tests if someone can point me to the details.

[pbiron]

here's my patch, which is against the current core code

[pbiron]

The patch I just attached is against the current core code, which has changed a bit since @johnjamesjacoby posted his patch.

I also borrowed @PieWP's idea for the implementation of wp_count_sites() and slightly modified the implementation of WP_MS_Sites_List_Table::is_base_request() I commented on above.

Again, if someone can point me to the rules for writing unit tests for WP, I'll take a stab at that over the next few days...and maybe we can get this in 4.8.

[pbiron]

The patch I just added has minor updates to the inline docs, adds a 'site_status' hidden field to the search form (so that searches are limited to the currently selected site_status as with posts) and simplifies WP_MS_Sites_List_Table::is_base_request().

[pbiron]

I found enough info about writing unit tests for the WP test-suite to take a stab at writing tests for this ticket.

Since these are the 1st tests I've written against this test-suite I highly suggest that someone double-check them to make sure they are actually testing what I think they are testing) and the setup and teardown (to make sure they don't adversely affect other tests.

I separated the tests for wp_site_counts() and those for sites.php into 2 separate files. As noted in a comment in wpMSSitesListTable-38071.php, once this ticket is accepted, those tests should be integrated into wpMSSitesListTable.php...I figured it would be easier to verify that my tests were correct if they were isolated from the existing tests in wpMSSitesListTable.php.