Merge pull request #4786 from luisramos0/stripe_sca_extra_subs_fix

Make StripeSCA (with new authorize/purchase workflow) work with Subscriptions again

.rubocop_manual_todo.yml

@@ -680,7 +680,6 @@ Metrics/ModuleLength:
   - engines/order_management/spec/services/order_management/subscriptions/estimator_spec.rb
   - engines/order_management/spec/services/order_management/subscriptions/form_spec.rb
   - engines/order_management/spec/services/order_management/subscriptions/proxy_order_syncer_spec.rb
-  - engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb
   - engines/order_management/spec/services/order_management/subscriptions/summarizer_spec.rb
   - engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb
   - engines/order_management/spec/services/order_management/subscriptions/variants_list_spec.rb

app/jobs/subscription_confirm_job.rb

@@ -57,16 +57,28 @@ class SubscriptionConfirmJob
     return true unless order.payment_required?
 
     setup_payment!(order)
-    return false if order.errors.present?
+    return false if order.errors.any?
+
+    authorize_payment!(order)
+    return false if order.errors.any?
 
     order.process_payments!
-    return false if order.errors.present?
+    return false if order.errors.any?
 
     true
   end
 
   def setup_payment!(order)
     OrderManagement::Subscriptions::PaymentSetup.new(order).call!
+    return if order.errors.any?
+
+    OrderManagement::Subscriptions::StripePaymentSetup.new(order).call!
+  end
+
+  def authorize_payment!(order)
+    return if order.subscription.payment_method.class != Spree::Gateway::StripeSCA
+
+    OrderManagement::Subscriptions::StripeScaPaymentAuthorize.new(order).call!
   end
 
   def send_confirmation_email(order)

app/services/checkout/stripe_redirect.rb

@@ -12,11 +12,8 @@ module Checkout
     def path
       return unless stripe_payment_method?
 
-      payment = @order.pending_payments.last
-      return unless payment&.checkout?
-
-      payment.authorize!
-      raise unless payment.pending?
+      payment = OrderManagement::Subscriptions::StripeScaPaymentAuthorize.new(@order).call!
+      raise if @order.errors.any?
 
       field_with_url(payment) if url?(field_with_url(payment))
     end

engines/order_management/app/services/order_management/subscriptions/payment_setup.rb

@@ -8,62 +8,23 @@ module OrderManagement
       end
 
       def call!
-        create_payment
-        ensure_payment_source
-        return if order.errors.any?
+        payment = create_payment
+        return if @order.errors.any?
 
-        payment.update_attributes(amount: order.outstanding_balance)
+        payment.update_attributes(amount: @order.outstanding_balance)
+        payment
       end
 
       private
 
-      attr_reader :order
-
-      def payment
-        @payment ||= order.pending_payments.last
-      end
-
       def create_payment
-        return if payment.present?
+        payment = @order.pending_payments.last
+        return payment if payment.present?
 
-        @payment = order.payments.create(
-          payment_method_id: order.subscription.payment_method_id,
-          amount: order.outstanding_balance
+        @order.payments.create(
+          payment_method_id: @order.subscription.payment_method_id
         )
       end
-
-      def card_required?
-        [Spree::Gateway::StripeConnect,
-         Spree::Gateway::StripeSCA].include? payment.payment_method.class
-      end
-
-      def card_set?
-        payment.source is_a? Spree::CreditCard
-      end
-
-      def ensure_payment_source
-        return unless card_required? && !card_set?
-
-        ensure_credit_card || order.errors.add(:base, :no_card)
-      end
-
-      def ensure_credit_card
-        return false if saved_credit_card.blank? || !allow_charges?
-
-        payment.update_attributes(source: saved_credit_card)
-      end
-
-      def allow_charges?
-        order.customer.allow_charges?
-      end
-
-      def saved_credit_card
-        order.user.default_card
-      end
-
-      def errors_present?
-        order.errors.any?
-      end
     end
   end
 end

engines/order_management/app/services/order_management/subscriptions/stripe_payment_setup.rb

@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+module OrderManagement
+  module Subscriptions
+    class StripePaymentSetup
+      def initialize(order)
+        @order = order
+        @payment = @order.pending_payments.last
+      end
+
+      def call!
+        return if @payment.blank?
+
+        ensure_payment_source
+        @payment
+      end
+
+      private
+
+      def ensure_payment_source
+        return unless stripe_payment_method? && !card_set?
+
+        if saved_credit_card.present? && allow_charges?
+          use_saved_credit_card
+        else
+          @order.errors.add(:base, :no_card)
+        end
+      end
+
+      def stripe_payment_method?
+        [Spree::Gateway::StripeConnect,
+         Spree::Gateway::StripeSCA].include? @payment.payment_method.class
+      end
+
+      def card_set?
+        @payment.source.is_a? Spree::CreditCard
+      end
+
+      def saved_credit_card
+        @order.user.default_card
+      end
+
+      def allow_charges?
+        @order.customer.allow_charges?
+      end
+
+      def use_saved_credit_card
+        @payment.update_attributes(source: saved_credit_card)
+      end
+    end
+  end
+end

engines/order_management/app/services/order_management/subscriptions/stripe_sca_payment_authorize.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+module OrderManagement
+  module Subscriptions
+    class StripeScaPaymentAuthorize
+      def initialize(order)
+        @order = order
+        @payment = @order.pending_payments.last
+      end
+
+      def call!
+        return unless @payment&.checkout?
+
+        @payment.authorize!
+
+        @order.errors.add(:base, I18n.t('authorization_failure')) unless @payment.pending?
+
+        @payment
+      end
+    end
+  end
+end

engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb

@@ -8,44 +8,6 @@ module OrderManagement
       let(:order) { create(:order) }
       let(:payment_setup) { OrderManagement::Subscriptions::PaymentSetup.new(order) }
 
-      describe "#payment" do
-        context "when only one payment exists on the order" do
-          let!(:payment) { create(:payment, order: order) }
-
-          context "where the payment is pending" do
-            it { expect(payment_setup.__send__(:payment)).to eq payment }
-          end
-
-          context "where the payment is failed" do
-            before { payment.update_attribute(:state, 'failed') }
-            it { expect(payment_setup.__send__(:payment)).to be nil }
-          end
-        end
-
-        context "when more that one payment exists on the order" do
-          let!(:payment1) { create(:payment, order: order) }
-          let!(:payment2) { create(:payment, order: order) }
-
-          context "where more than one payment is pending" do
-            it { expect([payment1, payment2]).to include payment_setup.__send__(:payment) }
-          end
-
-          context "where only one payment is pending" do
-            before { payment1.update_attribute(:state, 'failed') }
-            it { expect(payment_setup.__send__(:payment)).to eq payment2 }
-          end
-
-          context "where no payments are pending" do
-            before do
-              payment1.update_attribute(:state, 'failed')
-              payment2.update_attribute(:state, 'failed')
-            end
-
-            it { expect(payment_setup.__send__(:payment)).to be nil }
-          end
-        end
-      end
-
       describe "#call!" do
         let!(:payment){ create(:payment, amount: 10) }
 
@@ -68,161 +30,35 @@ module OrderManagement
         context "when a payment is present" do
           before { allow(order).to receive(:pending_payments).once { [payment] } }
 
-          context "when a credit card is not required" do
-            before do
-              allow(payment_setup).to receive(:card_required?) { false }
-              expect(payment_setup).to_not receive(:card_available?)
-              expect(payment_setup).to_not receive(:ensure_credit_card)
-            end
-
-            context "when the payment total doesn't match the outstanding balance on the order" do
-              before { allow(order).to receive(:outstanding_balance) { 5 } }
-              it "updates the payment total to reflect the outstanding balance" do
-                expect{ payment_setup.call! }.to change(payment, :amount).from(10).to(5)
-              end
-            end
-
-            context "when the payment total matches the outstanding balance on the order" do
-              before { allow(order).to receive(:outstanding_balance) { 10 } }
-
-              it "does nothing" do
-                expect{ payment_setup.call! }.to_not change(payment, :amount).from(10)
-              end
+          context "when the payment total doesn't match the outstanding balance on the order" do
+            before { allow(order).to receive(:outstanding_balance) { 5 } }
+            it "updates the payment total to reflect the outstanding balance" do
+              expect{ payment_setup.call! }.to change(payment, :amount).from(10).to(5)
             end
           end
 
-          context "when a credit card is required" do
-            before do
-              expect(payment_setup).to receive(:card_required?) { true }
-            end
+          context "when the payment total matches the outstanding balance on the order" do
+            before { allow(order).to receive(:outstanding_balance) { 10 } }
 
-            context "and the payment source is not a credit card" do
-              before { expect(payment_setup).to receive(:card_set?) { false } }
-
-              context "and no default credit card has been set by the customer" do
-                before do
-                  allow(order).to receive(:user) { instance_double(Spree::User, default_card: nil) }
-                end
-
-                it "adds an error to the order and does not update the payment" do
-                  expect(payment).to_not receive(:update_attributes)
-                  expect{ payment_setup.call! }.to change(order.errors[:base], :count).from(0).to(1)
-                end
-              end
-
-              context "and the customer has not authorised the shop to charge to credit cards" do
-                before do
-                  allow(order).to receive(:user) {
-                    instance_double(Spree::User, default_card: create(:credit_card))
-                  }
-                  allow(order).to receive(:customer) {
-                    instance_double(Customer, allow_charges?: false)
-                  }
-                end
-
-                it "adds an error to the order and does not update the payment" do
-                  expect(payment).to_not receive(:update_attributes)
-                  expect{ payment_setup.call! }.to change(order.errors[:base], :count).from(0).to(1)
-                end
-              end
-
-              context "and an authorised default credit card is available to charge" do
-                before do
-                  allow(order).to receive(:user) {
-                    instance_double(Spree::User, default_card: create(:credit_card))
-                  }
-                  allow(order).to receive(:customer) {
-                    instance_double(Customer, allow_charges?: true)
-                  }
-                end
-
-                context "when the payment total doesn't match the order's outstanding balance" do
-                  before { allow(order).to receive(:outstanding_balance) { 5 } }
-                  it "updates the payment total to reflect the outstanding balance" do
-                    expect{ payment_setup.call! }.to change(payment, :amount).from(10).to(5)
-                  end
-                end
-
-                context "when the payment total matches the outstanding balance on the order" do
-                  before { allow(order).to receive(:outstanding_balance) { 10 } }
-
-                  it "does nothing" do
-                    expect{ payment_setup.call! }.to_not change(payment, :amount).from(10)
-                  end
-                end
-              end
-            end
-
-            context "and the payment source is already a credit card" do
-              before { expect(payment_setup).to receive(:card_set?) { true } }
-
-              context "when the payment total doesn't match the outstanding balance on the order" do
-                before { allow(order).to receive(:outstanding_balance) { 5 } }
-                it "updates the payment total to reflect the outstanding balance" do
-                  expect{ payment_setup.call! }.to change(payment, :amount).from(10).to(5)
-                end
-              end
-
-              context "when the payment total matches the outstanding balance on the order" do
-                before { allow(order).to receive(:outstanding_balance) { 10 } }
-
-                it "does nothing" do
-                  expect{ payment_setup.call! }.to_not change(payment, :amount).from(10)
-                end
-              end
+            it "does nothing" do
+              expect{ payment_setup.call! }.to_not change(payment, :amount).from(10)
             end
           end
         end
-      end
 
-      describe "#ensure_credit_card" do
-        let!(:payment) { create(:payment, source: nil) }
-        before { allow(payment_setup).to receive(:payment) { payment } }
+        context "when more that one payment exists on the order" do
+          let!(:payment1) { create(:payment, order: order) }
+          let!(:payment2) { create(:payment, order: order) }
 
-        context "when no default credit card is found" do
           before do
-            allow(order).to receive(:user) { instance_double(Spree::User, default_card: nil) }
+            allow(order).to receive(:outstanding_balance) { 7 }
+            allow(order).to receive(:pending_payments).once { [payment1, payment2] }
           end
 
-          it "returns false and down not update the payment source" do
-            expect do
-              expect(payment_setup.__send__(:ensure_credit_card)).to be false
-            end.to_not change(payment, :source).from(nil)
-          end
-        end
-
-        context "when a default credit card is found" do
-          let(:credit_card) { create(:credit_card) }
-          before do
-            allow(order).to receive(:user) {
-              instance_double(Spree::User, default_card: credit_card)
-            }
-          end
-
-          context "and charge have not been authorised by the customer" do
-            before do
-              allow(order).to receive(:customer) {
-                instance_double(Customer, allow_charges?: false)
-              }
-            end
-
-            it "returns false and does not update the payment source" do
-              expect do
-                expect(payment_setup.__send__(:ensure_credit_card)).to be false
-              end.to_not change(payment, :source).from(nil)
-            end
-          end
-
-          context "and charges have been authorised by the customer" do
-            before do
-              allow(order).to receive(:customer) { instance_double(Customer, allow_charges?: true) }
-            end
-
-            it "returns true and stores the credit card as the payment source" do
-              expect do
-                expect(payment_setup.__send__(:ensure_credit_card)).to be true
-              end.to change(payment, :source_id).from(nil).to(credit_card.id)
-            end
+          it "updates the amount of the last payment to reflect the outstanding balance" do
+            payment_setup.call!
+            expect(payment1.amount).to eq 45.75
+            expect(payment2.amount).to eq 7
           end
         end
       end

engines/order_management/spec/services/order_management/subscriptions/stripe_payment_setup_spec.rb

@@ -0,0 +1,109 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+module OrderManagement
+  module Subscriptions
+    describe StripePaymentSetup do
+      let(:order) { create(:order) }
+      let(:payment_setup) { OrderManagement::Subscriptions::StripePaymentSetup.new(order) }
+
+      describe "#call!" do
+        context "when no pending payments are present" do
+          before do
+            allow(order).to receive(:pending_payments).once { [] }
+          end
+
+          it "does nothing" do
+            expect(payment_setup.call!).to eq nil
+          end
+        end
+
+        context "when a payment is present" do
+          let(:payment) { create(:payment, payment_method: payment_method, amount: 10) }
+
+          before { allow(order).to receive(:pending_payments).once { [payment] } }
+
+          context "when the payment method is not a stripe payment method" do
+            let(:payment_method) { create(:payment_method) }
+
+            it "returns the pending payment with no change" do
+              expect(payment).to_not receive(:update_attributes)
+              expect(payment_setup.call!).to eq payment
+            end
+          end
+
+          context "when the payment method is a stripe payment method" do
+            let(:payment_method) { create(:stripe_payment_method) }
+
+            context "and the card is already set (the payment source is a credit card)" do
+              it "returns the pending payment with no change" do
+                expect(payment).to_not receive(:update_attributes)
+                expect(payment_setup.call!).to eq payment
+              end
+            end
+
+            context "and the card is not set (the payment source is not a credit card)" do
+              before { payment.update_attribute :source, nil }
+
+              context "and no default credit card has been saved by the customer" do
+                before do
+                  allow(order).to receive(:user) { instance_double(Spree::User, default_card: nil) }
+                end
+
+                it "adds an error to the order and does not update the payment" do
+                  payment_setup.call!
+
+                  expect(payment).to_not receive(:update_attributes)
+                  expect(payment_setup.call!).to eq payment
+                  expect(order.errors[:base].first).to eq "There are no authorised "\
+                                                            "credit cards available to charge"
+                end
+              end
+
+              context "and a default credit card has been saved by the customer" do
+                let(:saved_credit_card) { create(:credit_card) }
+
+                before do
+                  allow(order).to receive(:user) {
+                    instance_double(Spree::User, default_card: saved_credit_card)
+                  }
+                end
+
+                context "but the customer has not authorised the shop to charge credit cards" do
+                  before do
+                    allow(order).to receive(:customer) {
+                      instance_double(Customer, allow_charges?: false)
+                    }
+                  end
+
+                  it "adds an error to the order and does not update the payment" do
+                    payment_setup.call!
+
+                    expect(payment).to_not receive(:update_attributes)
+                    expect(payment_setup.call!).to eq payment
+                    expect(order.errors[:base].first).to eq "There are no authorised "\
+                                                              "credit cards available to charge"
+                  end
+                end
+
+                context "and the customer has authorised the shop to charge credit cards" do
+                  before do
+                    allow(order).to receive(:customer) {
+                      instance_double(Customer, allow_charges?: true)
+                    }
+                  end
+
+                  it "uses the saved credit card as the source for the payment" do
+                    payment_setup.call!
+                    expect(payment.source).to eq saved_credit_card
+                  end
+                end
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end

engines/order_management/spec/services/order_management/subscriptions/stripe_sca_payment_authorize_spec.rb

@@ -0,0 +1,65 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+module OrderManagement
+  module Subscriptions
+    describe StripeScaPaymentAuthorize do
+      let(:order) { create(:order) }
+      let(:payment_authorize) {
+        OrderManagement::Subscriptions::StripeScaPaymentAuthorize.new(order)
+      }
+
+      describe "#call!" do
+        context "when no pending payments are present" do
+          before { allow(order).to receive(:pending_payments).once { [] } }
+
+          it "does nothing" do
+            expect(payment_authorize.call!).to eq nil
+          end
+        end
+
+        context "when a payment is present" do
+          let(:payment) { create(:payment, amount: 10) }
+
+          before { allow(order).to receive(:pending_payments).once { [payment] } }
+
+          context "in a state that is not checkout" do
+            before { payment.state = "processing" }
+
+            it "does nothing" do
+              payment_authorize.call!
+
+              expect(payment.state).to eq "processing"
+              expect(order.errors.size).to eq 0
+            end
+          end
+
+          context "in the checkout state" do
+            before { payment.state = "checkout" }
+
+            context "and payment authorize moves the payment state to pending" do
+              before { expect(payment).to receive(:authorize!) { payment.state = "pending" } }
+
+              it "does nothing" do
+                payment_authorize.call!
+
+                expect(order.errors.size).to eq 0
+              end
+            end
+
+            context "and payment authorize does not move the payment state to pending" do
+              before { allow(payment).to receive(:authorize!) { payment.state = "failed" } }
+
+              it "adds an error to the order indicating authorization failure" do
+                payment_authorize.call!
+
+                expect(order.errors[:base].first).to eq "Authorization Failure"
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end