Treat Stripe charge_already_captured as successful capture#419
Open
55728 wants to merge 1 commit into
Open
Conversation
When a charge is captured directly from the Stripe dashboard and then captured again from the Spree admin, ActiveMerchant returns a failed response with error code 'charge_already_captured'. Spree then transitions the payment to 'failed', even though Stripe already holds the funds, leaving the order stuck. Intercept that specific error in Spree::Gateway::StripeGateway#capture and wrap it in a successful ActiveMerchant::Billing::Response that carries the original charge id as the authorization, so the Spree payment transitions to 'completed' as expected. Fixes spree/spree#11974 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
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
Fixes spree/spree#11974.
When a Stripe charge is captured directly from the Stripe dashboard (or by any other out-of-band actor) and an admin subsequently clicks Capture in Spree, ActiveMerchant's
StripeGateway#capturereturns a failedResponsewhoseparams['error']['code']ischarge_already_captured. Spree currently propagates that failure and transitions the payment tofailed, even though Stripe already holds the funds. The order is then stuck in an inconsistent state (Stripe: captured, Spree: failed) that can only be resolved manually.This PR teaches
Spree::Gateway::StripeGateway#captureto recognize that specific Stripe error code and treat it as a successful capture, wrapping the originalparamsin a newActiveMerchant::Billing::Response.new(true, …)that carries the originalresponse_codeas the authorization. Any other failure (e.g.card_declined) is passed through unchanged.Why
spree_gatewayand notspree/spree?Stripe integration for Spree lives entirely in this gem — there is no Stripe-specific code left in
spree/spree. Thecapturemethod that Spree's payment state machine ultimately calls isSpree::Gateway::StripeGateway#capturehere, so this is the correct layer to intercept the Stripe response.A more complete long-term fix would be a Stripe webhook handler that reconciles Spree's payment state with Stripe events asynchronously, but that is a larger, separate piece of work and is intentionally out of scope for this PR.
Changes
app/models/spree/gateway/stripe_gateway.rb—#capturenow inspects the provider response. If it failed witherror.code == 'charge_already_captured', it returns a successfulActiveMerchant::Billing::Responsewith the original charge id as the authorization. All other responses (success or failure) are returned unchanged. A smallis_a?(Hash)guard protects against providers that return a non-hashparams(some ActiveMerchant response shapes).spec/models/gateway/stripe_gateway_spec.rb— new top-leveldescribeblock with four regression examples:charge_already_captured→success? == trueauthorizationequals the originalresponse_codecard_declined) is passed through as a failure (regression guard)The new block is deliberately placed at the top level (not nested inside the existing
describe Spree::Gateway::StripeGateway) so it uses modernallow(...).to receive(...)syntax and is not affected by the pre-existing legacy-syntaxbeforehook. See "Out of scope" below.'capturing'examples had.and_return(double(success?: true))appended to theirshould_receivestubs. This is required because#capturenow inspectsresponse.success?; without a return value the stubbedprovider.capturereturnsniland the method raisesNoMethodErroronnil.success?.Test plan
Spree::Gateway::StripeGateway#capture when Stripe already captured the charge— 4 examples, 0 failures locally.ActiveMerchant::Billing::Responsewith thecharge_already_capturedpayload and assertssuccess?,authorization, andparamson the wrapped response.Out of scope
The pre-existing examples in
spec/models/gateway/stripe_gateway_spec.rbuse rspec-mocks 2.x syntax (subject.stub(…),should_receive) which is deprecated in rspec-mocks 3.x and currently fails without:shouldsyntax explicitly enabled. This is a pre-existing condition onmain(unrelated to this PR) and is not addressed here — modernizing the rest of the file is left for a separate PR to keep this change minimal and focused on the bug fix.🤖 Generated with Claude Code