#45954 closed feature request (fixed)
Add a extra_tablenav() method to WP_MS_Sites_List_Table
Reported by: | pbiron | Owned by: | desrosj |
---|---|---|---|
Milestone: | 5.3 | Priority: | normal |
Severity: | normal | Version: | 5.1 |
Component: | Networks and Sites | Keywords: | has-screenshots has-patch has-dev-note |
Focuses: | administration, multisite | Cc: |
Description
With blogmeta landing in 5.1-beta1 (#37923 and #40229) it would be really sweet if WP_MS_Sites_List_Table
had an extra_tablenav()
method which fired restrict_manage_sites
and manage_sites_extra_tablenav
actions (analogous to WP_Posts_List_Table
's restrict_manage_posts and manage_posts_extra_tablenav).
This would allow plugins to add dropdowns (or other UI components) to limit the sites shown in the list table to those that have specific values in blogmeta (just like is now possible with other WP_List_Table
sub-classes).
I'm currently developing a large multisite (2000-3000 sites). The main site will serve as a directory of places of worship (searchable on the front-end by denomination and various other characteristics; blogmeta is going to make that searching much easier than it otherwise would have been!). Each place of worship will be a site in the network.
Having WP_MS_Sites_List_Table::extra_tablenav()
would make the site admin's job a lot easier, as it would allow them to perform some bulk action on just those sites with a specific denomination, etc. I'll add a screenshot of a mockup once I finish creating this ticket.
I raised the possibility of this feature a few weeks ago in #core-multisite at a time when I didn't think blogmeta was going to land. I know that technically now that 5.1-beta1 is out there are not supposed to be enhancements or feature requests added...but I'm hoping we can bend the rules a little and get this in beta2. If that is possible, I can have a patch ready in a few days.
Attachments (5)
Change History (36)
#3
@
6 years ago
- Milestone changed from Awaiting Review to Future Release
Thank you for the suggestion, @pbiron! It's too late for 5.1, but if you want to put a patch together, we can look at it for 5.2. 🙂
#4
@
6 years ago
Yeah, I figured it was too late, but didn't hurt to ask.
I've got the basic patch ready, but since it's not needed immediately, I'll work on it a little more before posting it...including adding unit tests.
This ticket was mentioned in Slack in #core-multisite by pbiron. View the logs.
6 years ago
#6
@
6 years ago
49594.diff adds the WP_MS_Sites_List_Table::extra_table()
method and the new restrict_manage_sites
and manage_sites_extra_tablenav
filters. It also changes /wp-admin/network/sites.php
to process submissions of the "Filter" button.
I've been using (some version of) this patch since the first 5.1-beta came out on a couple of in development multisites and haven't noticed any problems.
#8
follow-up:
↓ 19
@
6 years ago
I was planning on adding some unit tests, but discovered there are no tests for the equivalent filters provided by other list tables so I didn't bother with any here.
The only tests I can find related to extra_tablenav()
are for WP_Posts_List_Table
(e.g.,
Tests_Admin_includesListTable::test_filter_button_should_not_be_shown_if_there_are_no_posts()
, etc). I could add a test similar to those that checks that the "Filter" button is present if nothing hooks into the new filters, but I don't think that is really necessary.
This ticket was mentioned in Slack in #core-multisite by pbiron. View the logs.
6 years ago
This ticket was mentioned in Slack in #core-multisite by pbiron. View the logs.
6 years ago
This ticket was mentioned in Slack in #core by pbiron. View the logs.
6 years ago
This ticket was mentioned in Slack in #core-multisite by pbiron. View the logs.
6 years ago
This ticket was mentioned in Slack in #core-multisite by pbiron. View the logs.
5 years ago
This ticket was mentioned in Slack in #core-multisite by spacedmonkey. View the logs.
5 years ago
#15
@
5 years ago
- Keywords needs-refresh added
- Milestone changed from Future Release to 5.3
@pbiron It is time for this to go into core. Can you update the since, I will see if I can get this merged.
#17
@
5 years ago
- Keywords needs-refresh removed
45954.2.diff is the same as 45954.diff but with @since
updated to 5.3.0.
This ticket was mentioned in Slack in #core by david.baumwald. View the logs.
5 years ago
#19
in reply to:
↑ 8
@
5 years ago
Replying to pbiron:
I was planning on adding some unit tests, but discovered there are no tests for the equivalent filters provided by other list tables so I didn't bother with any here.
The only tests I can find related to
extra_tablenav()
are forWP_Posts_List_Table
(e.g.,
Tests_Admin_includesListTable::test_filter_button_should_not_be_shown_if_there_are_no_posts()
, etc). I could add a test similar to those that checks that the "Filter" button is present if nothing hooks into the new filters, but I don't think that is really necessary.
With 5.3 beta 1 due in 1 week (Sep 23), can we get this committed?
Should I add 1 or more unit tests similar to the ones for WP_Posts_List_Table
?
#21
@
5 years ago
45954.diff has a few changes:
- Coding standards fixes.
- Removes
()
for the exit call (there should be a separate ticket to remove all of these since it is a language construct and does not require them)
I also looked at how the other list tables handle these requests. Between edit.php
, users.php
, and edit-comments.php
, there are some inconsistencies.
- Some use
$_GET
, some use $_REQUEST` for the conditional. - Some use
$_REQUEST
some use$_SERVER
for building the redirect URL. - Both use
wp_redirect()
instead ofwp_safe_redirect()
.
I've updated the patch to be more consistent with these, even though there are some differences.
#25
@
5 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
Upon further testing, what was committed in 46211 doesn't correctly process the query defined by WP_MS_Sites_List_Table::extra_tablenav()
.
- in
network/sites.php
, the conditional at L234 should have used the$_POST
super global instead of$_GET
. This caused that branch of code to never execute. Without thatwp_redirect()
happening, the "filter" operation a user does results in the sites being filtered correctly. However, paginating the table after a filter operation results in the infamousThe link you followed has expired.
error screen. - with that conditional corrected, using
$_SERVER['REQUEST_URI']
as the basis for constructing the URL to redirect also results in theThe link you followed has expired.
error screen when doing a "filter" operation, because when$_GET['allblogs']
is set (which it is when using$_SERVER['REQUEST_URI']
to construct the redirect URL) a nonce is checked which does not exist. The redirect needs to include all of$_POST
(minus_wp_http_referer
and_wpnonce
), as was done in the original patch.
I wrote the original patch so long ago that I don't remember why I was checking the conditional in the 1st bullet point above, but it turns out that conditional doesn't even need to be there and the redirect can simply be in an else
.
New patching coming shortly that corrects the above.
#26
@
5 years ago
45954.3.diff implements the fix described in #comment:25.
@desrosj when you get a chance, can you look at the new patch?
#27
@
5 years ago
- Keywords commit added
Thanks for testing and catching this, @pbiron. The patch looks good.
mockup of a use of the proposed 'restrict_manage_sites' action