Opened 2 years ago
Last modified 2 months ago
#16031 assigned enhancement
New bulk actions hook missing catcher behavior
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Future Release |
| Component: | Administration | Version: | 3.1 |
| Severity: | normal | Keywords: | needs-patch |
| Cc: | sirzooro, steph@…, WordPress@…, dontmindtk, stephenh1988, mikeschinkel@…, edward@…, 24-7@…, lopo, dcowgill@…, danielbachhuber |
Description
The new bulk actions filter allows you to modify the bulk-actions arrays, but neglects to add the ability to handle the new custom actions.
The fix is to simply add one new do_action() hook to the default case of each affected page (similar to the way 'wp_ajax_' handles custom ajax requests).
For example, the following might be added to users.php at line 285, immediately after <tt>default:</tt>
do_action( 'bulk_action-'. $wp_list_table->current_action() );
Attachments (5)
Change History (52)
The hook in this patch follows the format 'do_bulk_action-screenid-currentaction'.
Without this patch, there is still no non-hacky way to handle custom batch actions that developers may create, which defeats the purpose of the bulk_action hook.
comment:2
SergeyBiryukov — 2 years ago
There are also files currently missing a default action handler:
plugins.php upload.php network/site-themes.php network/site-users.php network/themes.php
Perhaps the hook should be added to them too?
The bulk_actions filter wasn't implemented for those pages; I only added handlers for the filters that are already officially committed.
I'm not sure those screens would really benefit from custom bulk action support, though (excepting network/site-users.php).
comment:5
SergeyBiryukov — 2 years ago
- Keywords has-patch added; bulk_actions bulk has_patch removed
- Milestone changed from Awaiting Review to 3.1
- Owner set to westi
- Status changed from new to reviewing
The hook in this patch follows the format 'do_bulk_action-screenid-currentaction'.
'tags' isn't the screen id, for example. So, it's more like do_bulk_action-{list-table-type}-currentaction'.
Still, it's a good idea.
If we do want to go with screenid, it should be reflected in the code:
do_action( 'do_bulk_action-' . $current_screen->id . '-' . $doaction );
This patch looks like a good starting place but we should try and cover all of these if we are going to do this.
Sounds like working off the screen id might be a good idea so we can have context when the tables are used in more than one place.
comment:10
scribu — 2 years ago
- Owner changed from westi to scribu
- Status changed from reviewing to accepted
comment:11
scribu — 2 years ago
Actually, using the screen id would be the only consistent way, due to this:
$this->_actions = apply_filters( 'bulk_actions-' . $screen->id, $this->_actions );
Anyway, going to clean up the patch.
comment:12
scribu — 2 years ago
16031.2.patch introduces WP_List_Table::do_bulk_action() and calls it wherever WP_List_Table::current_action() is used.
There are several inconsistencies, though:
On network/sites.php, current_action() is not used, because bulk actions are handled in network/edit.php
On edit-comments.php, the action is run for each selected comment.
On users.php and edit-tags.php, the entire screen is rendered in the default: branch.
comment:13
scribu — 2 years ago
16031.minimal.patch avoids problematic screens, which would remain to be handled in a future release.
comment:14
scribu — 2 years ago
The next step would be to pass the list of elements to be processed to the action.
comment:15
scribu — 2 years ago
As useful as this may be, I think it's too big a change for this very late state in the dev cycle.
A workaround is to use the "load-$pagenow" action instead.
comment:16
scribu — 2 years ago
- Type changed from defect (bug) to enhancement
comment:17
nacin — 2 years ago
#16166 and #16187 make this higher priority. In particular, WP_List_Table::add_query_args() would need to be used by bulk action handlers... Figuring out how to handle this and do the redirect for them would be best.
Maybe these need to be filters, rather than actions. And the filter is the sendback URL, and they can add_query_arg() before returning it.
comment:18
westi — 2 years ago
For now I think the simplest solution is to remove the filter that lets you add bulk actions and revisit the ability to add extra ones in a future version.
That is the best thing for this stage of the cycle
comment:19
nacin — 2 years ago
Indeed. Pull it.
comment:20
westi — 2 years ago
Looking for a list of plugins that have already adopted this
comment:21
Veraxus — 2 years ago
We were using the bulk_actions filter in 'Page Security by Contexture'. I don't know who else might have implemented it, though.
comment:22
scribu — 2 years ago
The 'bulk_actions_' filter can also be used to remove certain actions, for example. I don't see the point in removing it.
comment:23
westi — 2 years ago
http://wordpress.org/extend/plugins/backwpup/
and
http://wordpress.org/extend/plugins/wpematico/
Both have the list tables code copied into them for back-compat
Grunion 2
comment:24
scribu — 2 years ago
Viper007Bond pointed out that we already have an 'admin_action_*' hook:
http://core.trac.wordpress.org/browser/trunk/wp-admin/admin.php?rev=17272#L232
The problem with that, as with "load-$pagenow", is that it runs before any capability checking is done.
Still, I think it's good enough for now.
comment:25
nacin — 2 years ago
Attached patch prevents no new actions from being added via the filter.
They can still do it by extending get_bulk_actions(), but that isn't supported. Otherwise, this filter ties our hands for future attempts to dry up all of the bulk action handler code (#16166) and also AJAX bulk actions.
comment:26
nacin — 2 years ago
- Resolution set to fixed
- Status changed from accepted to closed
comment:27
scribu — 20 months ago
- Keywords needs-patch added; has-patch removed
- Milestone changed from 3.1 to Future Release
- Resolution fixed deleted
- Status changed from closed to reopened
We can't really call this 'fixed'.
comment:28
sirzooro — 20 months ago
- Cc sirzooro added
comment:29
sillybean — 20 months ago
- Cc steph@… added
+1, I need exactly this for a custom plugin I'm writing, where I want admins to be able to deactivate and activate users (see http://develop.servus.at/trac/cbadevel/changeset/955).
I hate being forced to use a core hack.
- Cc WordPress@… added
comment:32
dontmindtk — 20 months ago
- Cc dontmindtk added
comment:33
stephenh1988 — 19 months ago
- Cc stephenh1988 added
comment:34
mikeschinkel — 17 months ago
- Cc mikeschinkel@… added
comment:35
follow-up:
↓ 36
edward mindreantre — 15 months ago
- Cc edward@… added
I can only wonder why the minimal patch was pulled.
My broadcast plugin would like to be able to mass-broadcast posts but that ain't happening unless I can add my own bulk action.
And would you believe that the table list code is a barren filter-desert? Not a filter or action as far as the eye I can see so I can't even add a custom button above the table, either. *sigh*
comment:36
in reply to:
↑ 35
Veraxus — 15 months ago
You have a great point edward,
While the WP_List_Table class can be extended by developers creating NEW list tables, a few well-positioned filters and actions might be exactly the thing that's needed to make this a reality. It would certainly be a clean, professional solution. I'll look into it this week. That may need a new ticket, though, because it's such a different approach - but it could also be much more future-proof.
I'll report back once I've taken a look.
As far as I can see, adding bulk actions isn't really a problem. A quick add_bulk_actions filter in the bulk_actions() function will solve the adding of filters.
It's the handling of the $_POST that's somewhat more of a problem, seeing that handling should be done before page output.
And since none of the post, link, comment, etc pages have anything in common that will mean adding bulk _POST handling code individually into each page.
Maybe $wp_list_table->handle_bulk_actions(); above $wp_list_table->prepare_items(); ?
comment:38
F J Kaiser — 15 months ago
- Cc 24-7@… added
comment:39
lopo — 13 months ago
- Cc lopo added
+1 about that: a number of users of my plugin (Duplicate Post) ask about selecting and cloning several posts at once.
comment:40
follow-up:
↓ 41
scribu — 11 months ago
- Owner scribu deleted
- Status changed from reopened to assigned
This will be fixed eventually, probably right after the media library revamp. :P
comment:41
in reply to:
↑ 40
pampfelimetten — 5 months ago
Replying to scribu:
This will be fixed eventually, probably right after the media library revamp. :P
Is there a chance to fix this for 3.6? After all, that would be exactly right after the media library revamp ;-)
comment:42
follow-up:
↓ 43
scribu — 5 months ago
After all, that would be exactly right after the media library revamp ;-)
Hm... I guess we have to stop using "after the media library revamp" as interchangeable with "never"...
Is there a chance to fix this for 3.6?
If no one steps up with a patch, there is no chance, IMO.
comment:43
in reply to:
↑ 42
pampfelimetten — 5 months ago
Replying to scribu:
If no one steps up with a patch, there is no chance, IMO.
I'm willing to give my best if you could help me a bit by providing details about what's wrong with the 16031.minimal.patch, and in which direction it should go. The patch looks good enough for me, but the comments from nacin seem to indicate that he wants to prevent some future incompatibilities - although I'm not sure what exactly he seems to have in mind.
comment:44
scribu — 4 months ago
I'm willing to give my best if you could help me a bit by providing details about what's wrong with the 16031.minimal.patch, and in which direction it should go.
Read edward mindreantre's comment again.
comment:45
dcowgill — 4 months ago
- Cc dcowgill@… added
- Cc danielbachhuber added
Maybe we can revert r17297? I'm trying to add bulk spam to Grunion right now and just ran into this. I'm not sure what we gained by making it harder to hack stuff onto a core list table.
comment:47
mark-k — 2 months ago
Was looking for adding a bulk action and ran into this ticket, then found scribu's answer on stack exchange http://wordpress.stackexchange.com/questions/29822/custom-bulk-action, and I fail to see the point of forcing people to use JS to add a bulk action. Sure it is the easier part of the solution but better then nothing.

Fixes submission handling for pages affected by the new bulk_action filter