From e87965bda02434d2976b2308ba1b4d91f5701813 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 3 Feb 2026 11:23:38 +1100 Subject: [PATCH 1/8] Simplify storing of payment profiles StripeSCA is the only payment method storing profiles following this logic. This is the first step to remove indirection and let the payment method handle this instead of the payment decided for the payment method. --- app/models/spree/payment.rb | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index da38a29665..5df898136b 100644 --- a/app/models/spree/payment.rb +++ b/app/models/spree/payment.rb @@ -34,7 +34,7 @@ module Spree # invalidate previously entered payments after_create :invalidate_old_payments - after_save :create_payment_profile, if: :profiles_supported? + after_save :create_payment_profile # update the order totals, etc. after_save :ensure_correct_adjustment, :update_order @@ -217,18 +217,13 @@ module Spree errors.blank? end - def profiles_supported? - payment_method.respond_to?(:payment_profiles_supported?) && - payment_method.payment_profiles_supported? - end - def create_payment_profile return unless source.is_a?(CreditCard) return unless source.try(:save_requested_by_customer?) return unless source.number || source.gateway_payment_profile_id return unless source.gateway_customer_profile_id.nil? - payment_method.create_profile(self) + payment_method.try(:create_profile, self) rescue ActiveMerchant::ConnectionError => e gateway_error e end From 2749965e73ac5b2fded95dc5541394996636fef6 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 3 Feb 2026 11:28:15 +1100 Subject: [PATCH 2/8] Remove useless branch calling #void StripeSCA is the only method with a different method signature for `#void` but the additional parameter wasn't used. So this special case can just be removed. --- app/models/spree/gateway/stripe_sca.rb | 2 +- app/models/spree/payment/processing.rb | 11 +---------- spec/models/spree/gateway/stripe_sca_spec.rb | 4 ++-- spec/models/spree/payment_spec.rb | 5 ++--- 4 files changed, 6 insertions(+), 16 deletions(-) diff --git a/app/models/spree/gateway/stripe_sca.rb b/app/models/spree/gateway/stripe_sca.rb index 78e658ab9c..feea5eda54 100644 --- a/app/models/spree/gateway/stripe_sca.rb +++ b/app/models/spree/gateway/stripe_sca.rb @@ -84,7 +84,7 @@ module Spree end # NOTE: this method is required by Spree::Payment::Processing - def void(payment_intent_id, _creditcard, gateway_options) + def void(payment_intent_id, gateway_options) payment_intent_response = Stripe::PaymentIntent.retrieve( payment_intent_id, stripe_account: stripe_account_id ) diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb index 599a562097..dccc339cac 100644 --- a/app/models/spree/payment/processing.rb +++ b/app/models/spree/payment/processing.rb @@ -58,16 +58,7 @@ module Spree protect_from_connection_error do check_environment - response = if payment_method.payment_profiles_supported? - # Gateways supporting payment profiles will need access to credit - # card object because this stores the payment profile information - # so supply the authorization itself as well as the credit card, - # rather than just the authorization code - payment_method.void(response_code, source, gateway_options) - else - # Standard ActiveMerchant void usage - payment_method.void(response_code, gateway_options) - end + response = payment_method.void(response_code, gateway_options) record_response(response) diff --git a/spec/models/spree/gateway/stripe_sca_spec.rb b/spec/models/spree/gateway/stripe_sca_spec.rb index 495c33aa6b..5379434c2e 100644 --- a/spec/models/spree/gateway/stripe_sca_spec.rb +++ b/spec/models/spree/gateway/stripe_sca_spec.rb @@ -109,7 +109,7 @@ RSpec.describe Spree::Gateway::StripeSCA, :vcr, :stripe_version do end it "refunds the payment" do - response = subject.void(payment_intent.id, nil, {}) + response = subject.void(payment_intent.id, {}) expect(response.success?).to eq true end @@ -131,7 +131,7 @@ RSpec.describe Spree::Gateway::StripeSCA, :vcr, :stripe_version do end it "void the payment" do - response = subject.void(payment_intent.id, nil, {}) + response = subject.void(payment_intent.id, {}) expect(response.success?).to eq true end diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index 360ad11318..a1209ed022 100644 --- a/spec/models/spree/payment_spec.rb +++ b/spec/models/spree/payment_spec.rb @@ -347,8 +347,7 @@ RSpec.describe Spree::Payment do context "when profiles are supported" do it "should call payment_enterprise.void with the payment's response_code" do - allow(payment_method).to receive(:payment_profiles_supported) { true } - expect(payment_method).to receive(:void).with('123', card, + expect(payment_method).to receive(:void).with('123', anything).and_return(success_response) payment.void_transaction! end @@ -357,7 +356,7 @@ RSpec.describe Spree::Payment do context "when profiles are not supported" do it "should call payment_gateway.void with the payment's response_code" do allow(payment_method).to receive(:payment_profiles_supported) { false } - expect(payment_method).to receive(:void).with('123', card, + expect(payment_method).to receive(:void).with('123', anything).and_return(success_response) payment.void_transaction! end From 4650e30c09e82622e8ac045ed9d56de420f83681 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 3 Feb 2026 11:39:15 +1100 Subject: [PATCH 3/8] Remove useless branch on credit method --- app/models/spree/gateway/stripe_sca.rb | 2 +- app/models/spree/payment/processing.rb | 19 +++++-------------- spec/models/spree/gateway/stripe_sca_spec.rb | 2 +- spec/models/spree/payment_spec.rb | 6 +++--- 4 files changed, 10 insertions(+), 19 deletions(-) diff --git a/app/models/spree/gateway/stripe_sca.rb b/app/models/spree/gateway/stripe_sca.rb index feea5eda54..10253377bd 100644 --- a/app/models/spree/gateway/stripe_sca.rb +++ b/app/models/spree/gateway/stripe_sca.rb @@ -101,7 +101,7 @@ module Spree end # NOTE: this method is required by Spree::Payment::Processing - def credit(money, _creditcard, payment_intent_id, gateway_options) + def credit(money, payment_intent_id, gateway_options) gateway_options[:stripe_account] = stripe_account_id provider.refund(money, payment_intent_id, gateway_options) end diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb index dccc339cac..4a78e9f727 100644 --- a/app/models/spree/payment/processing.rb +++ b/app/models/spree/payment/processing.rb @@ -77,20 +77,11 @@ module Spree credit_amount = calculate_refund_amount(credit_amount) - response = if payment_method.payment_profiles_supported? - payment_method.credit( - (credit_amount * 100).round, - source, - response_code, - gateway_options - ) - else - payment_method.credit( - (credit_amount * 100).round, - response_code, - gateway_options - ) - end + response = payment_method.credit( + (credit_amount * 100).round, + response_code, + gateway_options + ) record_response(response) diff --git a/spec/models/spree/gateway/stripe_sca_spec.rb b/spec/models/spree/gateway/stripe_sca_spec.rb index 5379434c2e..76761b001e 100644 --- a/spec/models/spree/gateway/stripe_sca_spec.rb +++ b/spec/models/spree/gateway/stripe_sca_spec.rb @@ -162,7 +162,7 @@ RSpec.describe Spree::Gateway::StripeSCA, :vcr, :stripe_version do stripe_account: stripe_test_account ) - response = subject.credit(1000, nil, payment_intent.id, {}) + response = subject.credit(1000, payment_intent.id, {}) expect(response.success?).to eq true end diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index a1209ed022..742b9d95ee 100644 --- a/spec/models/spree/payment_spec.rb +++ b/spec/models/spree/payment_spec.rb @@ -436,7 +436,7 @@ RSpec.describe Spree::Payment do end it "should call credit on the gateway with the credit amount and response_code" do - expect(payment_method).to receive(:credit).with(1000, card, '123', + expect(payment_method).to receive(:credit).with(1000, '123', anything).and_return(success_response) payment.credit! end @@ -462,7 +462,7 @@ RSpec.describe Spree::Payment do it "should call credit on the gateway with the credit amount and response_code" do expect(payment_method).to receive(:credit).with( - amount_in_cents, card, '123', anything + amount_in_cents, '123', anything ).and_return(success_response) payment.credit! end @@ -475,7 +475,7 @@ RSpec.describe Spree::Payment do it "should call credit on the gateway with original payment amount and response_code" do expect(payment_method).to receive(:credit).with( - amount_in_cents.to_f, card, '123', anything + amount_in_cents.to_f, '123', anything ).and_return(success_response) payment.credit! end From f9617b81561ed1ca28d306d6415f24da1ff5ae87 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 3 Feb 2026 12:51:02 +1100 Subject: [PATCH 4/8] Remove broken dead code branch The StripeSCA method is forwarding all missing methods to the provider gateway. The ActiveMerchant gateway used to be decorated by our code and that's how this code may have worked one day. But we removed the decorator years ago: - 549610bc355c2ad7b757e9f1013e125e0a4a9e90 A bit later we found that refunds are broken with Stripe: - https://github.com/openfoodfoundation/openfoodnetwork/issues/12843 This commit could potentiall fix that, I guess? I haven't tested that. It's a rare use case and not the aim of this work. --- app/models/spree/gateway/stripe_sca.rb | 6 ++++++ app/models/spree/payment/processing.rb | 19 +++++-------------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/app/models/spree/gateway/stripe_sca.rb b/app/models/spree/gateway/stripe_sca.rb index 10253377bd..fc9768e670 100644 --- a/app/models/spree/gateway/stripe_sca.rb +++ b/app/models/spree/gateway/stripe_sca.rb @@ -106,6 +106,12 @@ module Spree provider.refund(money, payment_intent_id, gateway_options) end + # NOTE: this method is required by Spree::Payment::Processing + def refund(money, payment_intent_id, gateway_options) + gateway_options[:stripe_account] = stripe_account_id + provider.refund(money, payment_intent_id, gateway_options) + end + def create_profile(payment) return unless payment.source.gateway_customer_profile_id.nil? diff --git a/app/models/spree/payment/processing.rb b/app/models/spree/payment/processing.rb index 4a78e9f727..46a1e784fd 100644 --- a/app/models/spree/payment/processing.rb +++ b/app/models/spree/payment/processing.rb @@ -107,20 +107,11 @@ module Spree refund_amount = calculate_refund_amount(refund_amount) - response = if payment_method.payment_profiles_supported? - payment_method.refund( - (refund_amount * 100).round, - source, - response_code, - gateway_options - ) - else - payment_method.refund( - (refund_amount * 100).round, - response_code, - gateway_options - ) - end + response = payment_method.refund( + (refund_amount * 100).round, + response_code, + gateway_options + ) record_response(response) From e86bf441abcf61873208af5ad4f10bd8430a268a Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 3 Feb 2026 13:02:53 +1100 Subject: [PATCH 5/8] Remove last use of #payment_profiles_supported? --- app/controllers/spree/admin/payments_controller.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/controllers/spree/admin/payments_controller.rb b/app/controllers/spree/admin/payments_controller.rb index e362dcc8f6..7fbc23f352 100644 --- a/app/controllers/spree/admin/payments_controller.rb +++ b/app/controllers/spree/admin/payments_controller.rb @@ -95,8 +95,7 @@ module Spree private def load_payment_source - if @payment.payment_method.is_a?(Spree::Gateway) && - @payment.payment_method.payment_profiles_supported? && + if @payment.payment_method.is_a?(Gateway::StripeSCA) && params[:card].present? && (params[:card] != 'new') @payment.source = CreditCard.find_by(id: params[:card]) From 02d47d0a0dd2f559e92676845422a4ec0829852d Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 3 Feb 2026 13:07:21 +1100 Subject: [PATCH 6/8] Remove unused method #payment_profiles_supported? And useless specs. --- app/models/spree/gateway.rb | 4 --- app/models/spree/gateway/stripe_sca.rb | 4 --- app/models/spree/payment_method.rb | 4 --- spec/models/spree/payment_spec.rb | 39 -------------------------- 4 files changed, 51 deletions(-) diff --git a/app/models/spree/gateway.rb b/app/models/spree/gateway.rb index 3cdcbc7b05..2cbfc76a15 100644 --- a/app/models/spree/gateway.rb +++ b/app/models/spree/gateway.rb @@ -42,10 +42,6 @@ module Spree end end - def payment_profiles_supported? - false - end - def method_type 'gateway' end diff --git a/app/models/spree/gateway/stripe_sca.rb b/app/models/spree/gateway/stripe_sca.rb index fc9768e670..685aaeeb90 100644 --- a/app/models/spree/gateway/stripe_sca.rb +++ b/app/models/spree/gateway/stripe_sca.rb @@ -35,10 +35,6 @@ module Spree ActiveMerchant::Billing::StripePaymentIntentsGateway end - def payment_profiles_supported? - true - end - def stripe_account_id StripeAccount.find_by(enterprise_id: preferred_enterprise_id)&.stripe_user_id end diff --git a/app/models/spree/payment_method.rb b/app/models/spree/payment_method.rb index 77474fb277..5e31e78152 100644 --- a/app/models/spree/payment_method.rb +++ b/app/models/spree/payment_method.rb @@ -93,10 +93,6 @@ module Spree unscoped { find(*) } end - def payment_profiles_supported? - false - end - def source_required? true end diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index 742b9d95ee..0d7c4ea8b8 100644 --- a/spec/models/spree/payment_spec.rb +++ b/spec/models/spree/payment_spec.rb @@ -345,23 +345,6 @@ RSpec.describe Spree::Payment do allow(payment_method).to receive(:void).and_return(success_response) end - context "when profiles are supported" do - it "should call payment_enterprise.void with the payment's response_code" do - expect(payment_method).to receive(:void).with('123', - anything).and_return(success_response) - payment.void_transaction! - end - end - - context "when profiles are not supported" do - it "should call payment_gateway.void with the payment's response_code" do - allow(payment_method).to receive(:payment_profiles_supported) { false } - expect(payment_method).to receive(:void).with('123', - anything).and_return(success_response) - payment.void_transaction! - end - end - it "should log the response" do payment.void_transaction! expect(payment).to have_received(:record_response) @@ -657,7 +640,6 @@ RSpec.describe Spree::Payment do context "when profiles are supported" do before do - allow(payment_method).to receive(:payment_profiles_supported?) { true } allow(payment.source).to receive(:has_payment_profile?) { false } end @@ -700,10 +682,6 @@ RSpec.describe Spree::Payment do end context "when profiles are not supported" do - before do - allow(payment_method).to receive(:payment_profiles_supported?) { false } - end - it "should not create a payment profile" do payment_method.name = 'Gateway' payment_method.distributors << create(:distributor_enterprise) @@ -876,23 +854,6 @@ RSpec.describe Spree::Payment do end end - describe "performing refunds" do - before do - allow(payment).to receive(:calculate_refund_amount) { 123 } - expect(payment.payment_method).to receive(:refund).and_return(success) - end - - it "performs the refund without payment profiles" do - allow(payment.payment_method).to receive(:payment_profiles_supported?) { false } - payment.refund! - end - - it "performs the refund with payment profiles" do - allow(payment.payment_method).to receive(:payment_profiles_supported?) { true } - payment.refund! - end - end - it "records the response" do allow(payment).to receive(:calculate_refund_amount) { 123 } allow(payment.payment_method).to receive(:refund).and_return(success) From 162c58ac39e20d07c515be431a909fdcd4776822 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 3 Feb 2026 15:08:31 +1100 Subject: [PATCH 7/8] Check which methods we need to delegate This implicit delegation makes it impossible to know which code is used and which code is dead. The refund method is very rarely used though. So we'll need to wait for a while. --- app/models/spree/gateway.rb | 6 +++++- spec/models/spree/gateway_spec.rb | 6 ++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/models/spree/gateway.rb b/app/models/spree/gateway.rb index 2cbfc76a15..c480d9c251 100644 --- a/app/models/spree/gateway.rb +++ b/app/models/spree/gateway.rb @@ -5,7 +5,7 @@ module Spree acts_as_taggable include PaymentMethodDistributors - delegate :authorize, :purchase, :capture, :void, :credit, to: :provider + delegate :authorize, :purchase, :capture, :void, :credit, :refund, to: :provider validates :name, :type, presence: true @@ -35,6 +35,10 @@ module Spree end def method_missing(method, *) + message = "Deprecated delegation of Gateway##{method}" + Alert.raise(message) + raise message if Rails.env.local? + if @provider.nil? || !@provider.respond_to?(method) super else diff --git a/spec/models/spree/gateway_spec.rb b/spec/models/spree/gateway_spec.rb index 0b46238436..f6e975ba38 100644 --- a/spec/models/spree/gateway_spec.rb +++ b/spec/models/spree/gateway_spec.rb @@ -14,8 +14,14 @@ RSpec.describe Spree::Gateway do end it "passes through all arguments on a method_missing call" do + expect(Rails.env).to receive(:local?).and_return(false) gateway = test_gateway.new expect(gateway.provider).to receive(:imaginary_method).with('foo') gateway.imaginary_method('foo') end + + it "raises an error in test env" do + gateway = test_gateway.new + expect { gateway.imaginary_method('foo') }.to raise_error StandardError + end end From f872201fefdb4d56a2a14303d3cf656543c99571 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 6 Feb 2026 11:35:24 +1100 Subject: [PATCH 8/8] Delete defunct spec From a comment on Github: > In this particular case, the spec is broken, actually. I just looked into it and you are right that StripeSCA supports payment profiles but the spec finds that the profile is not stored. And that is because `source.try(:save_requested_by_customer?)` returns false. We only store the profile when the customer is storing their credit card info. --- spec/models/spree/payment_spec.rb | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/spec/models/spree/payment_spec.rb b/spec/models/spree/payment_spec.rb index 0d7c4ea8b8..83ba78d711 100644 --- a/spec/models/spree/payment_spec.rb +++ b/spec/models/spree/payment_spec.rb @@ -681,22 +681,6 @@ RSpec.describe Spree::Payment do end end - context "when profiles are not supported" do - it "should not create a payment profile" do - payment_method.name = 'Gateway' - payment_method.distributors << create(:distributor_enterprise) - payment_method.save! - - expect(payment_method).not_to receive :create_profile - payment = Spree::Payment.create( - amount: 100, - order: create(:order), - source: card, - payment_method: - ) - end - end - context 'when the payment was completed but now void' do let(:payment) { create(:payment, :completed, amount: 100, order:) }