From e1393c96ca2ca7138d171253199d03eebe3a84c3 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 27 Jul 2021 23:24:00 +0100 Subject: [PATCH 1/2] Correctly void transactions when payments are cancelled due to stock issues --- app/controllers/checkout_controller.rb | 2 +- spec/controllers/checkout_controller_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/checkout_controller.rb b/app/controllers/checkout_controller.rb index 547f180a72..cffa0877b7 100644 --- a/app/controllers/checkout_controller.rb +++ b/app/controllers/checkout_controller.rb @@ -119,7 +119,7 @@ class CheckoutController < ::BaseController # The checkout could not complete due to stock running out. We void any pending (incomplete) # Stripe payments here as the order will need to be changed and resubmitted (or abandoned). @order.payments.incomplete.each do |payment| - payment.void! + payment.void_transaction! payment.adjustment&.update_columns(eligible: false, state: "finalized") end flash[:notice] = I18n.t("checkout.payment_cancelled_due_to_stock") diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb index e0e00a3ff7..10bc625675 100644 --- a/spec/controllers/checkout_controller_spec.rb +++ b/spec/controllers/checkout_controller_spec.rb @@ -126,7 +126,7 @@ describe CheckoutController, type: :controller do end it "cancels the payment and resets the order to cart" do - expect(payment).to receive(:void!).and_call_original + expect(payment).to receive(:void_transaction!).and_call_original spree_post :edit From 7e3b9be5061e6643c5a4ba76fa7eddd5189c1628 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 30 Jul 2021 10:37:52 +0100 Subject: [PATCH 2/2] Allow StripeSCA Gateway to actually void payments --- app/models/spree/gateway/stripe_sca.rb | 16 ++- .../payments_controller_refunds_spec.rb | 119 +++++++++++------- spec/features/admin/payments_stripe_spec.rb | 8 +- 3 files changed, 88 insertions(+), 55 deletions(-) diff --git a/app/models/spree/gateway/stripe_sca.rb b/app/models/spree/gateway/stripe_sca.rb index a095b37166..86b8627d5b 100644 --- a/app/models/spree/gateway/stripe_sca.rb +++ b/app/models/spree/gateway/stripe_sca.rb @@ -12,6 +12,10 @@ module Spree class StripeSCA < Gateway include FullUrlHelper + VOIDABLE_STATES = [ + "requires_payment_method", "requires_capture", "requires_confirmation", "requires_action" + ] + preference :enterprise_id, :integer validate :ensure_enterprise_selected @@ -78,7 +82,13 @@ module Spree payment_intent_response = Stripe::PaymentIntent.retrieve(payment_intent_id, stripe_account: stripe_account_id) gateway_options[:stripe_account] = stripe_account_id - provider.refund(refundable_amount(payment_intent_response), response_code, gateway_options) + + # If a payment has been confirmed it cannot be voided by Stripe, and must be refunded instead + if voidable?(payment_intent_response) + provider.void(response_code, gateway_options) + else + provider.refund(refundable_amount(payment_intent_response), response_code, gateway_options) + end end # NOTE: the name of this method is determined by Spree::Payment::Processing @@ -96,6 +106,10 @@ module Spree private + def voidable?(payment_intent_response) + VOIDABLE_STATES.include? payment_intent_response.status + end + def refundable_amount(payment_intent_response) payment_intent_response.amount_received - payment_intent_response.charges.data.map(&:amount_refunded).sum diff --git a/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb b/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb index 00c550b787..dccb39edc7 100644 --- a/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb +++ b/spec/controllers/spree/admin/orders/payments/payments_controller_refunds_spec.rb @@ -137,7 +137,7 @@ describe Spree::Admin::PaymentsController, type: :controller do end context "StripeSCA" do - context "requesting a refund on a payment" do + context "voiding a payment" do let(:params) { { id: payment.id, order_id: order.number, e: :void } } # Required for the respond override in the controller decorator to work @@ -156,14 +156,79 @@ describe Spree::Admin::PaymentsController, type: :controller do allow(StripeAccount).to receive(:find_by) { stripe_account } end - context "where the request succeeds" do + context "when the payment has been confirmed" do + context "where the request succeeds" do + before do + stub_payment_intent_get_request(response: { intent_status: "succeeded" }) + # Issues the refund + stub_request(:post, "https://api.stripe.com/v1/charges/ch_1234/refunds"). + with(basic_auth: ["sk_test_12345", ""]). + to_return(status: 200, + body: JSON.generate(id: 're_123', object: 'refund', status: 'succeeded') ) + end + + it "voids the payment" do + order.reload + expect(order.payment_total).to_not eq 0 + expect(order.outstanding_balance.to_f).to eq 0 + spree_put :fire, params + expect(payment.reload.state).to eq 'void' + order.reload + expect(order.payment_total).to eq 0 + expect(order.outstanding_balance.to_f).to_not eq 0 + end + end + + context "where the request fails" do + before do + stub_payment_intent_get_request(response: { intent_status: "succeeded" }) + stub_request(:post, "https://api.stripe.com/v1/charges/ch_1234/refunds"). + with(basic_auth: ["sk_test_12345", ""]). + to_return(status: 200, body: JSON.generate(error: { message: "Bup-bow!" }) ) + end + + it "does not void the payment" do + order.reload + expect(order.payment_total).to_not eq 0 + expect(order.outstanding_balance.to_f).to eq 0 + spree_put :fire, params + expect(payment.reload.state).to eq 'completed' + order.reload + expect(order.payment_total).to_not eq 0 + expect(order.outstanding_balance.to_f).to eq 0 + expect(flash[:error]).to eq "Bup-bow!" + end + end + + context "when a partial refund has already been issued" do + before do + stub_payment_intent_get_request(response: { intent_status: "succeeded", amount_refunded: 200 }) + stub_request(:post, "https://api.stripe.com/v1/charges/ch_1234/refunds"). + with(basic_auth: ["sk_test_12345", ""]). + to_return(status: 200, + body: JSON.generate(id: 're_123', object: 'refund', status: 'succeeded') ) + end + + it "can still void the payment" do + order.reload + expect(order.payment_total).to_not eq 0 + expect(order.outstanding_balance.to_f).to eq 0 + spree_put :fire, params + expect(payment.reload.state).to eq 'void' + order.reload + expect(order.payment_total).to eq 0 + expect(order.outstanding_balance.to_f).to_not eq 0 + end + end + end + + context "when the payment has not been confirmed yet" do before do - stub_payment_intent_get_request - # Issues the refund - stub_request(:post, "https://api.stripe.com/v1/charges/ch_1234/refunds"). + stub_payment_intent_get_request(response: { intent_status: "requires_action" }) + stub_request(:post, "https://api.stripe.com/v1/payment_intents/pi_123/cancel"). with(basic_auth: ["sk_test_12345", ""]). to_return(status: 200, - body: JSON.generate(id: 're_123', object: 'refund', status: 'succeeded') ) + body: JSON.generate(id: 'pi_123', object: 'payment_intent', status: 'canceled') ) end it "voids the payment" do @@ -177,48 +242,6 @@ describe Spree::Admin::PaymentsController, type: :controller do expect(order.outstanding_balance.to_f).to_not eq 0 end end - - context "where the request fails" do - before do - stub_payment_intent_get_request - stub_request(:post, "https://api.stripe.com/v1/charges/ch_1234/refunds"). - with(basic_auth: ["sk_test_12345", ""]). - to_return(status: 200, body: JSON.generate(error: { message: "Bup-bow!" }) ) - end - - it "does not void the payment" do - order.reload - expect(order.payment_total).to_not eq 0 - expect(order.outstanding_balance.to_f).to eq 0 - spree_put :fire, params - expect(payment.reload.state).to eq 'completed' - order.reload - expect(order.payment_total).to_not eq 0 - expect(order.outstanding_balance.to_f).to eq 0 - expect(flash[:error]).to eq "Bup-bow!" - end - end - - context "when a partial refund has already been issued" do - before do - stub_payment_intent_get_request(response: { amount_refunded: 200 }) - stub_request(:post, "https://api.stripe.com/v1/charges/ch_1234/refunds"). - with(basic_auth: ["sk_test_12345", ""]). - to_return(status: 200, - body: JSON.generate(id: 're_123', object: 'refund', status: 'succeeded') ) - end - - it "can still void the payment" do - order.reload - expect(order.payment_total).to_not eq 0 - expect(order.outstanding_balance.to_f).to eq 0 - spree_put :fire, params - expect(payment.reload.state).to eq 'void' - order.reload - expect(order.payment_total).to eq 0 - expect(order.outstanding_balance.to_f).to_not eq 0 - end - end end end diff --git a/spec/features/admin/payments_stripe_spec.rb b/spec/features/admin/payments_stripe_spec.rb index c725a91561..9e438fc048 100644 --- a/spec/features/admin/payments_stripe_spec.rb +++ b/spec/features/admin/payments_stripe_spec.rb @@ -140,12 +140,8 @@ feature ' let(:payment) { OrderPaymentFinder.new(order.reload).last_payment } before do - stub_payment_intent_get_request stripe_account_header: false - stub_successful_capture_request order: order - - payment.update response_code: "pi_123", amount: order.total - payment.purchase! - + payment.update response_code: "pi_123", amount: order.total, state: "completed" + stub_payment_intent_get_request response: { intent_status: "succeeded" }, stripe_account_header: false stub_refund_request end