Skip to content

Commit 4d52e02

Browse files
ekamrandkotter
ekamran
and
dkotter
authored
Fix bulk actions re-running on sort and pagination clicks (#928)
Fixed - Bulk actions no longer re-run when sorting or paginating the list after the action completes. Co-authored-by: ekamran <ekamran@git.wordpress.org> Co-authored-by: dkotter <dkotter@git.wordpress.org>
1 parent 26356e9 commit 4d52e02

9 files changed

Lines changed: 464 additions & 0 deletions

File tree

‎includes/Experiments/Alt_Text_Generation/Alt_Text_Generation.php‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,15 @@
2727
* @since 0.3.0
2828
*/
2929
class Alt_Text_Generation extends Abstract_Feature {
30+
/**
31+
* One-shot query args the bulk action redirect uses to trigger generation.
32+
*
33+
* @since x.x.x
34+
*
35+
* @var list<string>
36+
*/
37+
private const BULK_QUERY_ARGS = array( 'wpai_bulk_alt_text', 'wpai_attachment_ids' ); // phpcs:ignore SlevomatCodingStandard.Classes.DisallowMultiConstantDefinition -- This is used as an array const.
38+
3039
/**
3140
* {@inheritDoc}
3241
*/
@@ -67,6 +76,7 @@ public function register(): void {
6776
add_filter( 'attachment_fields_to_edit', array( $this, 'add_button_to_media_modal' ), 10, 2 );
6877
add_filter( 'bulk_actions-upload', array( $this, 'register_bulk_action' ) );
6978
add_filter( 'handle_bulk_actions-upload', array( $this, 'handle_bulk_action' ), 10, 3 );
79+
add_filter( 'removable_query_args', array( $this, 'register_removable_query_args' ) );
7080

7181
if ( ! defined( 'WP_CLI' ) || ! WP_CLI ) {
7282
return;
@@ -259,6 +269,26 @@ public function register_bulk_action( array $actions ): array {
259269
return $actions;
260270
}
261271

272+
/**
273+
* Registers the bulk alt text trigger params as removable query args.
274+
*
275+
* The bulk action redirect carries `wpai_bulk_alt_text` and
276+
* `wpai_attachment_ids` in the URL, and the bulk script runs whenever they
277+
* are present. Listing them here lets core clean them out of the address
278+
* bar on the first paint, via the canonical URL it prints in `admin_head`,
279+
* so reloading the results page does not re-trigger the whole generation.
280+
* The sort, pagination, and view switcher links are handled by the request
281+
* URI scrub in {@see Alt_Text_Generation::maybe_enqueue_bulk_script()}.
282+
*
283+
* @since x.x.x
284+
*
285+
* @param list<string> $args Query args removed from admin URLs.
286+
* @return list<string> Args including the bulk alt text trigger params.
287+
*/
288+
public function register_removable_query_args( array $args ): array {
289+
return array_merge( $args, self::BULK_QUERY_ARGS );
290+
}
291+
262292
/**
263293
* Handles the "Generate Alt Text" bulk action by redirecting with selected image IDs.
264294
*
@@ -308,6 +338,19 @@ private function maybe_enqueue_bulk_script(): void {
308338
return;
309339
}
310340

341+
/*
342+
* The trigger params have been read; scrub them from the request URI so
343+
* the sort header links the list table builds from it do not carry them.
344+
* Sorting links only strip `paged`, not removable query args, so this
345+
* mirrors what core does for its own one-shot params in wp-admin/upload.php.
346+
* The script receives the attachment IDs through wp_localize_script()
347+
* below and does not need them to stay in the URL. The value is only
348+
* rewritten, not output, so no sanitization applies.
349+
*/
350+
if ( isset( $_SERVER['REQUEST_URI'] ) ) {
351+
$_SERVER['REQUEST_URI'] = remove_query_arg( self::BULK_QUERY_ARGS, (string) $_SERVER['REQUEST_URI'] ); // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
352+
}
353+
311354
Asset_Loader::enqueue_script( 'alt_text_generation_bulk', 'experiments/alt-text-generation-bulk', array( 'include_core_abilities' => true ) );
312355
Asset_Loader::localize_script(
313356
'alt_text_generation_bulk',

‎includes/Experiments/Comment_Moderation/Comment_Moderation.php‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,15 @@ class Comment_Moderation extends Abstract_Feature {
136136
*/
137137
public const DEFAULT_MODERATE_GUESTS = true;
138138

139+
/**
140+
* One-shot query args the bulk action redirect uses to show its notice.
141+
*
142+
* @since x.x.x
143+
*
144+
* @var list<string>
145+
*/
146+
private const BULK_NOTICE_QUERY_ARGS = array( 'wpai_analysis_queued', 'wpai_no_provider' ); // phpcs:ignore SlevomatCodingStandard.Classes.DisallowMultiConstantDefinition -- This is used as an array const.
147+
139148
/**
140149
* Gets the configuration for sentiment levels.
141150
*
@@ -253,6 +262,8 @@ public function register(): void {
253262
add_filter( 'bulk_actions-edit-comments', array( $this, 'add_bulk_actions' ) );
254263
add_filter( 'handle_bulk_actions-edit-comments', array( $this, 'handle_bulk_action' ), 10, 3 );
255264
add_action( 'admin_notices', array( $this, 'show_bulk_action_notice' ) );
265+
add_filter( 'removable_query_args', array( $this, 'register_removable_query_args' ) );
266+
add_action( 'load-edit-comments.php', array( $this, 'remove_bulk_notice_query_args' ) );
256267

257268
// Add inline action.
258269
add_filter( 'comment_row_actions', array( $this, 'add_inline_action' ), 10, 2 );
@@ -849,6 +860,45 @@ public function add_bulk_actions( $actions ): array {
849860
return $actions;
850861
}
851862

863+
/**
864+
* Registers the bulk notice trigger params as removable query args.
865+
*
866+
* The bulk action redirect carries `wpai_analysis_queued` or
867+
* `wpai_no_provider` in the URL so the notice can be shown once. Listing
868+
* them here lets core clean them out of the address bar on the first paint,
869+
* via the canonical URL it prints in `admin_head`, so reloading the results
870+
* page does not re-show the notice. The sort and pagination links are
871+
* handled by the request URI scrub in
872+
* {@see Comment_Moderation::remove_bulk_notice_query_args()}.
873+
*
874+
* @since x.x.x
875+
*
876+
* @param list<string> $args Query args removed from admin URLs.
877+
* @return list<string> Args including the bulk notice trigger params.
878+
*/
879+
public function register_removable_query_args( array $args ): array {
880+
return array_merge( $args, self::BULK_NOTICE_QUERY_ARGS );
881+
}
882+
883+
/**
884+
* Scrubs the bulk notice trigger params from the request URI.
885+
*
886+
* The notice reads the params from `$_GET`, which this does not touch, so
887+
* it still shows once on the redirect. Removing them from the request URI
888+
* stops the sort header links the list table builds from it from carrying
889+
* them, since those links only strip `paged`, not removable args. This
890+
* mirrors what core does for its own one-shot params in wp-admin/edit-comments.php.
891+
*
892+
* @since x.x.x
893+
*/
894+
public function remove_bulk_notice_query_args(): void {
895+
if ( ! isset( $_SERVER['REQUEST_URI'] ) ) {
896+
return;
897+
}
898+
899+
$_SERVER['REQUEST_URI'] = remove_query_arg( self::BULK_NOTICE_QUERY_ARGS, (string) $_SERVER['REQUEST_URI'] ); // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
900+
}
901+
852902
/**
853903
* Handles the bulk action for AI analysis.
854904
*

‎includes/Experiments/Summarization/Summarization.php‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,15 @@
2929
*/
3030
class Summarization extends Abstract_Feature {
3131

32+
/**
33+
* One-shot query args the bulk action redirect uses to trigger generation.
34+
*
35+
* @since x.x.x
36+
*
37+
* @var list<string>
38+
*/
39+
private const BULK_QUERY_ARGS = array( 'wpai_bulk_summary', 'wpai_post_ids' ); // phpcs:ignore SlevomatCodingStandard.Classes.DisallowMultiConstantDefinition -- This is used as an array const.
40+
3241
/**
3342
* {@inheritDoc}
3443
*/
@@ -58,6 +67,27 @@ public function register(): void {
5867

5968
add_action( 'load-edit.php', array( $this, 'register_bulk_action_hooks_for_screen' ) );
6069
add_action( 'admin_enqueue_scripts', array( $this, 'maybe_enqueue_bulk_assets' ) );
70+
add_filter( 'removable_query_args', array( $this, 'register_removable_query_args' ) );
71+
}
72+
73+
/**
74+
* Registers the bulk summary trigger params as removable query args.
75+
*
76+
* The bulk action redirect carries `wpai_bulk_summary` and `wpai_post_ids`
77+
* in the URL, and the bulk script runs whenever they are present. Listing
78+
* them here lets core clean them out of the address bar on the first paint,
79+
* via the canonical URL it prints in `admin_head`, so reloading the results
80+
* page does not re-trigger the whole generation. The sort and pagination
81+
* links are handled by the request URI scrub in
82+
* {@see Summarization::maybe_enqueue_bulk_assets()}.
83+
*
84+
* @since x.x.x
85+
*
86+
* @param list<string> $args Query args removed from admin URLs.
87+
* @return list<string> Args including the bulk summary trigger params.
88+
*/
89+
public function register_removable_query_args( array $args ): array {
90+
return array_merge( $args, self::BULK_QUERY_ARGS );
6191
}
6292

6393
/**
@@ -241,6 +271,19 @@ public function maybe_enqueue_bulk_assets( string $hook_suffix ): void {
241271
return;
242272
}
243273

274+
/*
275+
* The trigger params have been read; scrub them from the request URI so
276+
* the sort header links the list table builds from it do not carry them.
277+
* Sorting links only strip `paged`, not removable query args, so this
278+
* mirrors what core does for its own one-shot params in wp-admin/edit.php.
279+
* The script receives the post IDs through wp_localize_script() below and
280+
* does not need them to stay in the URL. The value is only rewritten, not
281+
* output, so no sanitization applies.
282+
*/
283+
if ( isset( $_SERVER['REQUEST_URI'] ) ) {
284+
$_SERVER['REQUEST_URI'] = remove_query_arg( self::BULK_QUERY_ARGS, (string) $_SERVER['REQUEST_URI'] ); // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
285+
}
286+
244287
// Resolve the REST base once all posts in a list table share the same post type.
245288
$post_type = isset( $_GET['post_type'] ) ? sanitize_key( $_GET['post_type'] ) : 'post'; // phpcs:ignore WordPress.Security.NonceVerification.Recommended
246289
$post_type_obj = get_post_type_object( $post_type );

‎tests/Integration/Includes/Experiments/Alt_Text_Generation/Alt_Text_GenerationTest.php‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,58 @@ public function test_handle_bulk_action_requires_capability(): void {
202202
$this->assertSame( $redirect, $result, 'Redirect should be unchanged when user lacks upload_files capability.' );
203203
}
204204

205+
/**
206+
* Test that the bulk alt text trigger params are registered as removable query args.
207+
*
208+
* Core cleans removable args out of the address bar, so a reload of the
209+
* results page does not re-trigger the whole generation.
210+
*
211+
* @since x.x.x
212+
*/
213+
public function test_bulk_alt_text_params_are_removable_query_args(): void {
214+
$experiment = new Alt_Text_Generation();
215+
$experiment->register();
216+
217+
$removable = wp_removable_query_args();
218+
219+
$this->assertContains( 'wpai_bulk_alt_text', $removable );
220+
$this->assertContains( 'wpai_attachment_ids', $removable );
221+
}
222+
223+
/**
224+
* Test that the bulk script enqueue scrubs the trigger params from the request URI.
225+
*
226+
* Sort header links are built from the request URI and only strip `paged`,
227+
* so leaving the params in place re-triggers generation on every sort click.
228+
*
229+
* @since x.x.x
230+
*/
231+
public function test_media_library_assets_scrub_bulk_params_from_request_uri(): void {
232+
$original_request_uri = $_SERVER['REQUEST_URI'] ?? ''; // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
233+
234+
$admin_id = self::factory()->user->create( array( 'role' => 'administrator' ) );
235+
wp_set_current_user( $admin_id );
236+
237+
$_GET['wpai_bulk_alt_text'] = '1';
238+
$_GET['wpai_attachment_ids'] = '1,2';
239+
$_SERVER['REQUEST_URI'] = '/wp-admin/upload.php?mode=list&wpai_bulk_alt_text=1&wpai_attachment_ids=1,2&orderby=date'; // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
240+
241+
try {
242+
$experiment = new Alt_Text_Generation();
243+
$experiment->maybe_enqueue_media_library_assets( 'upload.php' );
244+
245+
// phpcs:disable WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- Asserting on the raw value.
246+
$this->assertStringNotContainsString( 'wpai_bulk_alt_text', $_SERVER['REQUEST_URI'] );
247+
$this->assertStringNotContainsString( 'wpai_attachment_ids', $_SERVER['REQUEST_URI'] );
248+
$this->assertStringContainsString( 'mode=list', $_SERVER['REQUEST_URI'], 'Unrelated query args must survive the scrub.' );
249+
$this->assertStringContainsString( 'orderby=date', $_SERVER['REQUEST_URI'], 'Unrelated query args must survive the scrub.' );
250+
// phpcs:enable WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
251+
} finally {
252+
unset( $_GET['wpai_bulk_alt_text'], $_GET['wpai_attachment_ids'] );
253+
$_SERVER['REQUEST_URI'] = $original_request_uri;
254+
}
255+
}
256+
205257
/**
206258
* Test that the bulk script is enqueued on upload.php when valid GET params and capability are present.
207259
*

‎tests/Integration/Includes/Experiments/Comment_Moderation/Comment_ModerationTest.php‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,8 @@ public function test_register_adds_expected_hooks() {
136136
$this->assertIsInt( has_filter( 'bulk_actions-edit-comments', array( $experiment, 'add_bulk_actions' ) ) );
137137
$this->assertIsInt( has_filter( 'handle_bulk_actions-edit-comments', array( $experiment, 'handle_bulk_action' ) ) );
138138
$this->assertIsInt( has_action( 'admin_notices', array( $experiment, 'show_bulk_action_notice' ) ) );
139+
$this->assertIsInt( has_filter( 'removable_query_args', array( $experiment, 'register_removable_query_args' ) ) );
140+
$this->assertIsInt( has_action( 'load-edit-comments.php', array( $experiment, 'remove_bulk_notice_query_args' ) ) );
139141
$this->assertIsInt( has_action( 'load-edit-comments.php', array( $experiment, 'handle_inline_action' ) ) );
140142
$this->assertIsInt( has_action( 'admin_enqueue_scripts', array( $experiment, 'enqueue_assets' ) ) );
141143
$this->assertIsInt( has_action( 'admin_head-edit-comments.php', array( $experiment, 'add_inline_styles' ) ) );
@@ -242,6 +244,53 @@ public function test_handle_bulk_action_marks_comments_pending_and_adds_query_ar
242244
);
243245
}
244246

247+
/**
248+
* Test that the bulk notice trigger params are registered as removable query args.
249+
*
250+
* Core cleans removable args out of the address bar, so a reload of the
251+
* results page does not re-show the notice.
252+
*
253+
* @since x.x.x
254+
*/
255+
public function test_bulk_notice_params_are_removable_query_args(): void {
256+
$experiment = new Comment_Moderation();
257+
$experiment->register();
258+
259+
$removable = wp_removable_query_args();
260+
261+
$this->assertContains( 'wpai_analysis_queued', $removable );
262+
$this->assertContains( 'wpai_no_provider', $removable );
263+
}
264+
265+
/**
266+
* Test that the request URI scrub removes the bulk notice params.
267+
*
268+
* Sort header links are built from the request URI and only strip `paged`,
269+
* so leaving the params in place re-shows the notice on every sort click.
270+
* The notice reads the params from `$_GET`, which the scrub leaves intact.
271+
*
272+
* @since x.x.x
273+
*/
274+
public function test_remove_bulk_notice_query_args_scrubs_request_uri(): void {
275+
$original_request_uri = $_SERVER['REQUEST_URI'] ?? ''; // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
276+
277+
try {
278+
$_SERVER['REQUEST_URI'] = '/wp-admin/edit-comments.php?paged=2&wpai_analysis_queued=3&wpai_no_provider=1&orderby=date'; // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
279+
280+
$experiment = new Comment_Moderation();
281+
$experiment->remove_bulk_notice_query_args();
282+
283+
// phpcs:disable WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- Asserting on the raw value.
284+
$this->assertStringNotContainsString( 'wpai_analysis_queued', $_SERVER['REQUEST_URI'] );
285+
$this->assertStringNotContainsString( 'wpai_no_provider', $_SERVER['REQUEST_URI'] );
286+
$this->assertStringContainsString( 'paged=2', $_SERVER['REQUEST_URI'], 'Unrelated query args must survive the scrub.' );
287+
$this->assertStringContainsString( 'orderby=date', $_SERVER['REQUEST_URI'], 'Unrelated query args must survive the scrub.' );
288+
// phpcs:enable WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
289+
} finally {
290+
$_SERVER['REQUEST_URI'] = $original_request_uri;
291+
}
292+
}
293+
245294
/**
246295
* Test moderate_comment() analyzes comments when the current user cannot moderate comments.
247296
*

‎tests/Integration/Includes/Experiments/Summarization/SummarizationTest.php‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,59 @@ public function test_maybe_enqueue_bulk_assets_enqueues_script(): void {
341341
}
342342
}
343343

344+
/**
345+
* Tests that the bulk summary trigger params are registered as removable query args.
346+
*
347+
* Core cleans removable args out of the address bar, so a reload of the
348+
* results page does not re-trigger the whole generation.
349+
*
350+
* @since x.x.x
351+
*/
352+
public function test_bulk_summary_params_are_removable_query_args(): void {
353+
$experiment = new Summarization();
354+
$experiment->register();
355+
356+
$removable = wp_removable_query_args();
357+
358+
$this->assertContains( 'wpai_bulk_summary', $removable );
359+
$this->assertContains( 'wpai_post_ids', $removable );
360+
}
361+
362+
/**
363+
* Tests that maybe_enqueue_bulk_assets() scrubs the trigger params from the request URI.
364+
*
365+
* Sort header links are built from the request URI and only strip `paged`,
366+
* so leaving the params in place re-triggers generation on every sort click.
367+
*
368+
* @since x.x.x
369+
*/
370+
public function test_maybe_enqueue_bulk_assets_scrubs_request_uri(): void {
371+
$original_get = $_GET; // phpcs:ignore WordPress.Security.NonceVerification.Recommended
372+
$original_request_uri = $_SERVER['REQUEST_URI'] ?? ''; // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
373+
374+
try {
375+
$editor_id = self::factory()->user->create( array( 'role' => 'editor' ) );
376+
wp_set_current_user( $editor_id );
377+
378+
$post_id = self::factory()->post->create();
379+
$_GET['wpai_bulk_summary'] = '1';
380+
$_GET['wpai_post_ids'] = (string) $post_id;
381+
$_SERVER['REQUEST_URI'] = '/wp-admin/edit.php?wpai_bulk_summary=1&wpai_post_ids=' . $post_id . '&orderby=date&order=desc'; // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
382+
383+
$experiment = new Summarization();
384+
$experiment->maybe_enqueue_bulk_assets( 'edit.php' );
385+
386+
// phpcs:disable WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- Asserting on the raw value.
387+
$this->assertStringNotContainsString( 'wpai_bulk_summary', $_SERVER['REQUEST_URI'] );
388+
$this->assertStringNotContainsString( 'wpai_post_ids', $_SERVER['REQUEST_URI'] );
389+
$this->assertStringContainsString( 'orderby=date', $_SERVER['REQUEST_URI'], 'Unrelated query args must survive the scrub.' );
390+
// phpcs:enable WordPress.Security.ValidatedSanitizedInput.InputNotSanitized
391+
} finally {
392+
$_GET = $original_get; // phpcs:ignore WordPress.Security.NonceVerification.Recommended
393+
$_SERVER['REQUEST_URI'] = $original_request_uri;
394+
}
395+
}
396+
344397
/**
345398
* Tests that register() wires register_bulk_action_hooks_for_screen() onto load-edit.php.
346399
*

0 commit comments

Comments
 (0)