From 9906afa1a607652dfda4459bbe8577b01e6e56c2 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Tue, 27 Apr 2021 17:59:03 -0700 Subject: [PATCH 1/4] use capture! if payment is already authorized --- app/models/spree/payment/processing.rb | 12 ++++++++++-- spec/factories/payment_factory.rb | 2 +- spec/models/spree/payment_spec.rb | 24 ++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb index a40a0798da..e9e3048023 100644 --- a/app/models/spree/payment/processing.rb +++ b/app/models/spree/payment/processing.rb @@ -6,14 +6,22 @@ module Spree def process! return unless validate! - purchase! + if response_code + capture! + else + purchase! + end end def process_offline! return unless validate! return if authorization_action_required? - charge_offline! + if response_code + capture! + else + charge_offline! + end end def authorize!(return_url = nil) diff --git a/spec/factories/payment_factory.rb b/spec/factories/payment_factory.rb index 96b71bd0e4..2919b00c7c 100644 --- a/spec/factories/payment_factory.rb +++ b/spec/factories/payment_factory.rb @@ -14,7 +14,7 @@ FactoryBot.define do association(:source, factory: :credit_card) order state { 'checkout' } - response_code { '12345' } + response_code { nil } payment_method { FactoryBot.create(:payment_method, distributors: [distributor]) } end diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index 2820316bd0..6a65c546dc 100644 --- a/spec/models/spree/payment_spec.rb +++ b/spec/models/spree/payment_spec.rb @@ -97,6 +97,30 @@ describe Spree::Payment do expect { payment.process! }.to raise_error(Spree::Core::GatewayError) expect(payment.state).to eq('invalid') end + + context "the payment is already authorized" do + before do + allow(payment).to receive(:response_code) { "pi_123" } + end + + it "should call capture instead of purchase" do + expect(payment).to receive(:capture!) + expect(payment).to_not receive(:purchase!) + payment.process! + end + end + end + + context "#process_offline when payment is already authorized" do + before do + allow(payment).to receive(:response_code) { "pi_123" } + end + + it "should call capture if the payment is already authorized" do + expect(payment).to receive(:capture!) + expect(payment).to_not receive(:purchase!) + payment.process! + end end context "#authorize" do From 473601394639b461c344962a8cbdde6c2f7d9864 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Tue, 27 Apr 2021 18:28:28 -0700 Subject: [PATCH 2/4] update spec --- spec/models/spree/gateway/stripe_sca_spec.rb | 1 + spec/models/spree/payment_spec.rb | 7 +++---- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/models/spree/gateway/stripe_sca_spec.rb b/spec/models/spree/gateway/stripe_sca_spec.rb index 43844a3763..8ddfe99373 100644 --- a/spec/models/spree/gateway/stripe_sca_spec.rb +++ b/spec/models/spree/gateway/stripe_sca_spec.rb @@ -16,6 +16,7 @@ describe Spree::Gateway::StripeSCA, type: :model do amount: order.total, payment_method: subject, source: credit_card, + response_code: "12345" ) } let(:gateway_options) { diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index 6a65c546dc..03eb892b53 100644 --- a/spec/models/spree/payment_spec.rb +++ b/spec/models/spree/payment_spec.rb @@ -103,9 +103,8 @@ describe Spree::Payment do allow(payment).to receive(:response_code) { "pi_123" } end - it "should call capture instead of purchase" do - expect(payment).to receive(:capture!) - expect(payment).to_not receive(:purchase!) + it "should call purchase" do + expect(payment).to receive(:purchase!) payment.process! end end @@ -119,7 +118,7 @@ describe Spree::Payment do it "should call capture if the payment is already authorized" do expect(payment).to receive(:capture!) expect(payment).to_not receive(:purchase!) - payment.process! + payment.process_offline! end end From 73e5fd3f5b906a64fd38bb18359dde9586f34659 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Wed, 28 Apr 2021 10:21:35 -0700 Subject: [PATCH 3/4] use method to clarify intent on payment intents --- app/models/spree/payment/processing.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb index e9e3048023..b59f3663ff 100644 --- a/app/models/spree/payment/processing.rb +++ b/app/models/spree/payment/processing.rb @@ -6,18 +6,14 @@ module Spree def process! return unless validate! - if response_code - capture! - else - purchase! - end + purchase! end def process_offline! return unless validate! return if authorization_action_required? - if response_code + if preauthorized? capture! else charge_offline! @@ -192,6 +188,10 @@ module Spree private + def preauthorized? + response_code.presence&.match("pi_") + end + def validate! return false unless payment_method&.source_required? From 4f246da79fa6233d10fd2166f961944e94bbab84 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Wed, 28 Apr 2021 11:25:43 -0700 Subject: [PATCH 4/4] add feature spec --- spec/jobs/subscription_confirm_job_spec.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/spec/jobs/subscription_confirm_job_spec.rb b/spec/jobs/subscription_confirm_job_spec.rb index 39bec020b1..1a03217ae4 100644 --- a/spec/jobs/subscription_confirm_job_spec.rb +++ b/spec/jobs/subscription_confirm_job_spec.rb @@ -158,12 +158,19 @@ describe SubscriptionConfirmJob do allow(order).to receive(:pending_payments) { [stripe_sca_payment] } allow(stripe_sca_payment_method).to receive(:provider) { provider } allow(stripe_sca_payment_method.provider).to receive(:purchase) { true } + allow(stripe_sca_payment_method.provider).to receive(:capture) { true } end it "runs the charges in offline mode" do job.send(:confirm_order!, order) expect(stripe_sca_payment_method.provider).to have_received(:purchase) end + + it "uses #capture if the payment is already authorized" do + allow(stripe_sca_payment).to receive(:preauthorized?) { true } + expect(stripe_sca_payment_method.provider).to receive(:capture) + job.send(:confirm_order!, order) + end end context "Stripe Connect" do