Skip to content

Fix issue in Stripe event handling#495

Merged
sorinmarta merged 1 commit into
masterfrom
fix-multi-site-stripe-event-handling
Mar 19, 2026
Merged

Fix issue in Stripe event handling#495
sorinmarta merged 1 commit into
masterfrom
fix-multi-site-stripe-event-handling

Conversation

@sorinmarta
Copy link
Copy Markdown
Contributor

Check if the event needs to be handled before processing the event.

@sorinmarta sorinmarta added the run analysis Runs phpcs and phpunit label Mar 17, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 17, 2026

Walkthrough

The Stripe event controller now uses boolean return values from event handlers to control processing flow. Previously void-returning handlers have been refactored to return booleans, allowing the event dispatcher to short-circuit and skip standard processing when an event is handled. All major handler methods consistently signal success or failure through explicit returns.

Changes

Cohort / File(s) Summary
Stripe Event Handler Refactoring
includes/gateways/stripe/controllers/classStrpEventsController.php
Modified handle_event() and related payment processing methods to return booleans instead of void. process_invoice_payment_failed(), process_payment_intent(), and process_payment_succeeded() now return false for validation failures and true for successful handling. Caller now checks boolean returns to determine whether to continue processing or short-circuit. Default case in switch statement returns true to treat unrecognized events as handled.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title directly relates to the main change: fixing Stripe event handling logic with a new guard clause that checks if events need handling before processing.
Description check ✅ Passed The description is relevant to the changeset, accurately describing the intent to check event eligibility before processing.
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 (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-multi-site-stripe-event-handling
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Tip

CodeRabbit can generate a title for your PR based on the changes.

Add @coderabbitai placeholder anywhere in the title of your PR and CodeRabbit will replace it with a title based on the changes in the PR. You can change the placeholder by changing the reviews.auto_title_placeholder setting.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
includes/gateways/stripe/controllers/classStrpEventsController.php (2)

197-197: Drop unused $subscription parameter from process_invoice_payment_failed().

Line 225 does not use $subscription, and this private API can be simplified to reduce noise.

Refactor diff
-				return $this->process_invoice_payment_failed( $subscription, $parent_payment );
+				return $this->process_invoice_payment_failed( $parent_payment );

-	private function process_invoice_payment_failed( $subscription, $parent_payment ) {
+	private function process_invoice_payment_failed( $parent_payment ) {

Also applies to: 225-225

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@includes/gateways/stripe/controllers/classStrpEventsController.php` at line
197, The private method process_invoice_payment_failed currently accepts an
unused $subscription parameter; remove $subscription from its signature and all
call sites (e.g., change $this->process_invoice_payment_failed( $subscription,
$parent_payment ) to $this->process_invoice_payment_failed( $parent_payment )),
update the method definition to accept only $parent_payment, and adjust the
method body and any docblock/type hints accordingly to reflect the single
parameter.

294-299: Tighten return-doc semantics for process_payment_intent().

The method returns true not only when a payment is found, but also when the intent is intentionally ignored (e.g., manual confirmation). The @return text should reflect that broader meaning.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@includes/gateways/stripe/controllers/classStrpEventsController.php` around
lines 294 - 299, Update the docblock for process_payment_intent() to reflect its
broader return semantics: it returns true both when a payment is found on this
site and when the intent is intentionally ignored (e.g., missing invoice id or
'manual' confirmation_method). Edit the `@return` description in the comment above
process_payment_intent() to state that true indicates either a found/handled
payment or an intentionally ignored intent, and false indicates the payment was
not found/handled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@includes/gateways/stripe/controllers/classStrpEventsController.php`:
- Line 197: The private method process_invoice_payment_failed currently accepts
an unused $subscription parameter; remove $subscription from its signature and
all call sites (e.g., change $this->process_invoice_payment_failed(
$subscription, $parent_payment ) to $this->process_invoice_payment_failed(
$parent_payment )), update the method definition to accept only $parent_payment,
and adjust the method body and any docblock/type hints accordingly to reflect
the single parameter.
- Around line 294-299: Update the docblock for process_payment_intent() to
reflect its broader return semantics: it returns true both when a payment is
found on this site and when the intent is intentionally ignored (e.g., missing
invoice id or 'manual' confirmation_method). Edit the `@return` description in the
comment above process_payment_intent() to state that true indicates either a
found/handled payment or an intentionally ignored intent, and false indicates
the payment was not found/handled.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 03ff2d7a-6f12-4d19-b579-a56ce7dd7dc6

📥 Commits

Reviewing files that changed from the base of the PR and between a006ffc and 019a6c2.

📒 Files selected for processing (1)
  • includes/gateways/stripe/controllers/classStrpEventsController.php

@sorinmarta sorinmarta requested a review from Crabcyborg March 17, 2026 06:53
Copy link
Copy Markdown
Contributor

@Crabcyborg Crabcyborg left a comment

Choose a reason for hiding this comment

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

Looks good to me @sorinmarta!

Thanks for working on that!

🚀

@sorinmarta sorinmarta merged commit 51a857f into master Mar 19, 2026
8 checks passed
@sorinmarta sorinmarta deleted the fix-multi-site-stripe-event-handling branch March 19, 2026 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run analysis Runs phpcs and phpunit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants