Skip to content

Changes how we cancel pending events. We now use clear all and also h…#296

Merged
gin0115 merged 3 commits into
trunkfrom
feature/294-prevent-iawmlf_process_local_post-flooding-ActionScheduler-queue
Feb 11, 2026
Merged

Changes how we cancel pending events. We now use clear all and also h…#296
gin0115 merged 3 commits into
trunkfrom
feature/294-prevent-iawmlf_process_local_post-flooding-ActionScheduler-queue

Conversation

@gin0115

@gin0115 gin0115 commented Feb 11, 2026

Copy link
Copy Markdown
Collaborator

…ook in on cancel to fully remove them

Changes proposed in this Pull Request

This pull request introduces significant improvements to how duplicate scheduled actions are handled for processing local posts. The main goal is to ensure that only one event per post ID exists in the action queue at any time, preventing database flooding and improving reliability. It also adds comprehensive tests to verify this behavior.

Duplicate event prevention and queue management:

  • Added a new private method ensure_single_event to Process_Local_Post_Event that unschedules all existing actions for a given post ID before scheduling a new one, ensuring only one event per post is queued at any time. This method is now called in both add_to_queue_with_delay and the event handler itself. [1] [2]
  • Implemented handle_hard_delete to forcefully delete canceled actions, and ensured it is only temporarily hooked to avoid interfering with unrelated events.

Testing and validation:

  • Updated and expanded tests in Test_Process_Local_Post_Event to:
    • Check that multiple attempts to add the same post to the queue result in only one scheduled action.
    • Simulate and resolve cases where the database contains multiple duplicate pending actions for the same post.
    • Verify that running the event removes all other pending events for that post from the queue.

These changes address issues with duplicate scheduled actions and ensure robust, predictable event processing.

Testing instructions

Mentions #294

Summary by CodeRabbit

  • Bug Fixes

    • Ensures only one scheduled event exists per post to prevent duplicate queue entries.
    • Adds handling to remove hard-deleted scheduled actions and improves cleanup during event processing.
  • Tests

    • Added end-to-end tests validating duplicate prevention and queue cleanup when multiple pending events exist.

@coderabbitai

coderabbitai Bot commented Feb 11, 2026

Copy link
Copy Markdown

Walkthrough

Adds duplicate-event prevention to the post event processing flow. In class Process_Local_Post_Event, a private static method ensure_single_event(int $post_id) unschedules existing actions for a given post and a public static method handle_hard_delete(int $action_id) conditionally deletes scheduler actions. Calls to ensure_single_event were added where events are queued. Tests were updated and new tests added to verify duplicate removal and end-to-end queue cleanup.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title check ⚠️ Warning The title is truncated and incomplete, ending with 'clear all and also h…', which prevents full understanding of the change. Complete the title to fully convey the intended change. A complete title might be: 'Changes how we cancel pending events. We now use clear all and also hook in on cancel to fully remove them' or a more concise version like 'Prevent duplicate queue entries by clearing and hooking cancellation handlers'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
tests/Event/Test_Process_Local_Post_Event.php (1)

78-95: Remove unused $event variable.

The static analysis tool correctly identified that $event is created but never used since all calls are made statically via Process_Local_Post_Event::add_to_queue().

🧹 Remove unused variable
 public function test_add_local_post_to_queue_multiple_times(): void {
 	$post_id = 999;

-	// Create the event.
-	$event = new Process_Local_Post_Event();
-
 	Process_Local_Post_Event::add_to_queue( $post_id );
 	Process_Local_Post_Event::add_to_queue( $post_id );
 	Process_Local_Post_Event::add_to_queue( $post_id );
src/Event/Process_Local_Post_Event.php (1)

160-175: Check for NullAction before calling methods on it.

fetch_action returns an ActionScheduler_NullAction instance (not null) when the action doesn't exist. While calling get_hook() on a NullAction returns an empty string and is safe in this context, verifying the action exists before operating on it would be more defensive.

🛡️ Optional defensive check
 public static function handle_hard_delete( int $action_id ): void {
 	// Bail if not set.
 	if ( ! class_exists( 'ActionScheduler' ) ) {
 		return;
 	}

 	// Get the action.
 	$store  = \ActionScheduler::store();
 	$action = $store->fetch_action( $action_id );

+	// Bail if action is a NullAction.
+	if ( $action instanceof \ActionScheduler_NullAction ) {
+		return;
+	}
+
 	// If we have the right action.
 	if ( self::HANDLE === $action->get_hook() ) {
 		// Delete the action.
 		$store->delete_action( $action_id );
 	}
 }

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In `@src/Event/Process_Local_Post_Event.php`:
- Around line 140-148: The add_action/remove_action pairing in
ensure_single_event currently risks leaving the handle_hard_delete hook
registered if as_unschedule_all_actions throws; wrap the unschedule call in a
try/finally inside ensure_single_event so that add_action(self::class,
'handle_hard_delete') is always removed in the finally block via remove_action,
referencing ensure_single_event, as_unschedule_all_actions, add_action,
remove_action and the handle_hard_delete method to guarantee cleanup even on
exception.

In `@tests/Event/Test_Process_Local_Post_Event.php`:
- Line 381: There is a duplicated assignment in the test where $actions is
assigned twice; update the statement that calls $this->wpdb->get_results (the
line using $actions = $this->wpdb->get_results(...)) to remove the extra
duplicate assignment so it reads as a single assignment to $actions and
continues to fetch pending actions via $this->wpdb->get_results.
- Around line 119-138: The wpdb->insert call in the test uses 14 columns but
only supplies 12 format specifiers, causing a mismatch; update the format array
passed to $this->wpdb->insert (the third argument) to include a format specifier
for each column in the values array so counts match (refer to the insert call
that uses Process_Local_Post_Event::HANDLE and fields like action_id, hook,
status, scheduled_date_gmt, scheduled_date_local, priority, args, schedule,
group_id, attempts, last_attempt_gmt, last_attempt_local, claim_id,
extended_args) — ensure types align (e.g. %d for integers, %s for strings/null)
and the format array length equals the values array length.
🧹 Nitpick comments (1)
tests/Event/Test_Process_Local_Post_Event.php (1)

80-81: Remove unused $event variable.

The static analysis tool flagged this: the $event variable is instantiated but never used since the test now calls Process_Local_Post_Event::add_to_queue statically.

♻️ Proposed fix
-		// Create the event.
-		$event = new Process_Local_Post_Event();
-
 		Process_Local_Post_Event::add_to_queue( $post_id );

Comment thread src/Event/Process_Local_Post_Event.php
Comment thread tests/Event/Test_Process_Local_Post_Event.php
Comment thread tests/Event/Test_Process_Local_Post_Event.php Outdated
@gin0115 gin0115 merged commit a26497f into trunk Feb 11, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant