Support new Spree Payment Session Webhooks#60
Merged
damianlegawiec merged 2 commits intomainfrom Mar 14, 2026
Merged
Conversation
WalkthroughAdds Stripe webhook handling: signature verification, event parsing (mapping payment_intent events to actions), new webhook URL behavior with optional legacy handlers, richer payment session completion (create payment, patch billing/wallet address), tests and VCR update, and a local Claude settings file. Changes
Sequence Diagram(s)sequenceDiagram
actor Stripe as Stripe (Webhook)
participant Gateway as SpreeStripe::Gateway
participant Verify as verify_webhook_signature()
participant Parse as parse_webhook_event()
participant Session as PaymentSession
participant DB as Database
Stripe->>Gateway: POST webhook (raw_body, headers)
Gateway->>Verify: verify_webhook_signature(raw_body, headers)
Verify->>Verify: read signature header\niterate webhook_signing_secrets\nStripe::Webhook.construct_event
Verify-->>Gateway: return verified event / raise error
Gateway->>Parse: parse_webhook_event(raw_body, headers)
Parse->>DB: find payment_session by external_id
Parse-->>Gateway: {action, payment_session, metadata} or nil
Gateway->>Session: apply action (captured / failed / none)
Session->>DB: persist payment/session state
DB-->>Session: confirm
Session-->>Gateway: result
sequenceDiagram
participant Client as Client
participant Gateway as SpreeStripe::Gateway
participant StripeAPI as Stripe API
participant Session as PaymentSession
participant DB as Database
Client->>Gateway: complete_payment_session(payment_session)
Gateway->>StripeAPI: retrieve payment_intent
StripeAPI-->>Gateway: intent data
Gateway->>StripeAPI: retrieve charge
StripeAPI-->>Gateway: charge data
Gateway->>Gateway: payment_intent_successful?(intent)
Gateway->>Gateway: patch_wallet_address(order, charge)
Gateway->>DB: update order billing & user
DB-->>Gateway: confirm
Gateway->>Session: find_or_create_payment!
Session->>DB: create/update Payment
DB-->>Session: payment persisted
Gateway->>Session: complete or fail session
Session-->>Client: return completion result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
spec/services/spree_stripe/create_gateway_webhooks_spec.rb (1)
17-20: Legacy stub is too broad for this spec file.This forces legacy behavior for all examples, so service behavior for the new default webhook path isn’t exercised here. Consider limiting the stub to legacy-specific contexts and adding one default-path example.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/services/spree_stripe/create_gateway_webhooks_spec.rb` around lines 17 - 20, Current stub in create_gateway_webhooks_spec.rb globally forces legacy webhook behavior by stubbing SpreeStripe::Config[:use_legacy_webhook_handlers] to true for all examples; move the allow(...).with(:use_legacy_webhook_handlers).and_return(true) into a specific context or describe block named "when legacy webhook handlers enabled" (keeping the allow(...).and_call_original in before hooks if you need other keys untouched), and add at least one example outside that context that exercises the default/new webhook path (i.e., no stub for :use_legacy_webhook_handlers) so behavior for the default webhook URL is tested.spec/models/spree_stripe/gateway/payment_sessions_spec.rb (1)
210-218: Add explicit assertion forpayment.process!in succeeded flow.The new behavior differentiates
process!vsauthorize!; this context should assert the succeeded branch invokesprocess!to prevent regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/models/spree_stripe/gateway/payment_sessions_spec.rb` around lines 210 - 218, Update the spec for complete_payment_session to assert the succeeded branch calls process! on the payment: after arranging payment_session and stubbing/expecting payment_session.find_or_create_payment! to return a payment object, add an expectation that that payment receives :process! (and does not receive :authorize!) when gateway.complete_payment_session(payment_session: payment_session) is invoked; reference the complete_payment_session method, the payment_session fixture, find_or_create_payment!, and the payment.process! / payment.authorize! methods to locate where to add the assertions..claude/settings.local.json (1)
1-22: Prefer not committing local agent permission policy to the repo.This file is environment-local and grants broad command permissions. Consider removing it from the PR and keeping it in local-only config (e.g., gitignored) to avoid policy drift across contributors/CI tooling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/settings.local.json around lines 1 - 22, Remove the committed local agent permission file and stop tracking it: delete .claude/settings.local.json from the PR (undo the addition), add the filename to .gitignore, and instead commit a minimal template (e.g., .claude/settings.example.json) that documents safe defaults; specifically remove or restrict the broad "permissions.allow" entries listed in the file and keep any necessary per-machine overrides only in ignored local files.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/models/spree_stripe/gateway.rb`:
- Around line 32-40: The webhook handler in Spree::Stripe::Gateway looks up
Spree::PaymentSessions::Stripe by external_id only (in the
'payment_intent.succeeded' and 'payment_intent.payment_failed' branches), which
can match sessions from other gateways; narrow the lookup to the current gateway
by adding the payment_method constraint (e.g., include payment_method: self or
payment_method_id: id) to the find_by call on Spree::PaymentSessions::Stripe so
both branches use something like find_by(external_id: ..., payment_method: self)
to ensure only sessions belonging to this gateway are returned.
In `@app/models/spree_stripe/gateway/payment_sessions.rb`:
- Around line 155-161: The current wallet-patching code assigns
order.bill_address = order.ship_address without checking for a nil ship_address,
which can set bill_address to nil and make order.save! fail; update the block in
payment_sessions.rb to guard the fallback by testing order.ship_address.present?
before assigning, and if absent either duplicate the existing ship address data
into a new bill address record or skip the assignment and ensure a valid
bill_address exists (or log and return/raise a clear error) so that order.save!
is never called with a nil bill_address; reference the order.bill_address and
order.ship_address symbols and ensure you still call save! only after confirming
a non-nil, valid bill_address.
In `@spec/vcr/existing_gateway_webhooks.yml`:
- Around line 491-493: Remove the leaked signing secret value in the cassette by
replacing the "secret" field's real-looking value (e.g. "whsec_...") with a
redacted placeholder, and update the VCR recording configuration (the
VCR.configure / filter_sensitive_data setup) to always filter/redact webhook
"secret" fields during recording so future cassettes omit real secrets; target
the cassette entry containing the "secret" key and the VCR configuration
function (e.g., VCR.configure or filter_sensitive_data call) to add a rule that
matches webhook secret values and replaces them with a constant token like
"[REDACTED_WEBHOOK_SECRET]".
---
Nitpick comments:
In @.claude/settings.local.json:
- Around line 1-22: Remove the committed local agent permission file and stop
tracking it: delete .claude/settings.local.json from the PR (undo the addition),
add the filename to .gitignore, and instead commit a minimal template (e.g.,
.claude/settings.example.json) that documents safe defaults; specifically remove
or restrict the broad "permissions.allow" entries listed in the file and keep
any necessary per-machine overrides only in ignored local files.
In `@spec/models/spree_stripe/gateway/payment_sessions_spec.rb`:
- Around line 210-218: Update the spec for complete_payment_session to assert
the succeeded branch calls process! on the payment: after arranging
payment_session and stubbing/expecting payment_session.find_or_create_payment!
to return a payment object, add an expectation that that payment receives
:process! (and does not receive :authorize!) when
gateway.complete_payment_session(payment_session: payment_session) is invoked;
reference the complete_payment_session method, the payment_session fixture,
find_or_create_payment!, and the payment.process! / payment.authorize! methods
to locate where to add the assertions.
In `@spec/services/spree_stripe/create_gateway_webhooks_spec.rb`:
- Around line 17-20: Current stub in create_gateway_webhooks_spec.rb globally
forces legacy webhook behavior by stubbing
SpreeStripe::Config[:use_legacy_webhook_handlers] to true for all examples; move
the allow(...).with(:use_legacy_webhook_handlers).and_return(true) into a
specific context or describe block named "when legacy webhook handlers enabled"
(keeping the allow(...).and_call_original in before hooks if you need other keys
untouched), and add at least one example outside that context that exercises the
default/new webhook path (i.e., no stub for :use_legacy_webhook_handlers) so
behavior for the default webhook URL is tested.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bc599566-c406-4d87-bbd7-4350e46dc0d3
📒 Files selected for processing (8)
.claude/settings.local.jsonapp/models/spree_stripe/gateway.rbapp/models/spree_stripe/gateway/payment_sessions.rblib/spree_stripe/configuration.rbspec/models/spree_stripe/gateway/payment_sessions_spec.rbspec/models/spree_stripe/gateway_spec.rbspec/services/spree_stripe/create_gateway_webhooks_spec.rbspec/vcr/existing_gateway_webhooks.yml
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/models/spree_stripe/gateway/payment_sessions.rb (1)
155-162:⚠️ Potential issue | 🟡 MinorSilent failure when billing address is invalid and no shipping address exists.
When
order.bill_address.invalid?is true andorder.ship_address.blank?, the method returns early at line 156 without saving or logging anything. This silently leaves the order without a valid billing address, which could cause issues downstream.Consider logging this condition or returning a status indicator so callers can handle the edge case appropriately.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/models/spree_stripe/gateway/payment_sessions.rb` around lines 155 - 162, The current early return when order.bill_address.invalid? and order.ship_address.blank? silently fails; update the method (the block handling order.bill_address logic in payment_sessions.rb) to not silently return — instead log a descriptive error via the existing logger (or process logger) mentioning the order id and that no ship_address exists to copy, and return a failure indicator (e.g. false or a specific error symbol) so callers can handle the invalid billing-address case; ensure you still avoid saving changes when invalid and keep the subsequent save! calls unchanged for the normal path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/models/spree_stripe/gateway/payment_sessions.rb`:
- Around line 155-162: The current early return when order.bill_address.invalid?
and order.ship_address.blank? silently fails; update the method (the block
handling order.bill_address logic in payment_sessions.rb) to not silently return
— instead log a descriptive error via the existing logger (or process logger)
mentioning the order id and that no ship_address exists to copy, and return a
failure indicator (e.g. false or a specific error symbol) so callers can handle
the invalid billing-address case; ensure you still avoid saving changes when
invalid and keep the subsequent save! calls unchanged for the normal path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6a8858ce-20b0-4cff-af92-54bfa9e918fc
📒 Files selected for processing (3)
app/models/spree_stripe/gateway.rbapp/models/spree_stripe/gateway/payment_sessions.rbspec/vcr/existing_gateway_webhooks.yml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Configuration
Tests