diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index 7eae3ef495..de9861fa40 100644 --- a/.rubocop_manual_todo.yml +++ b/.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 diff --git a/app/jobs/subscription_confirm_job.rb b/app/jobs/subscription_confirm_job.rb index 3612b7e0d4..748257a340 100644 --- a/app/jobs/subscription_confirm_job.rb +++ b/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) diff --git a/app/services/checkout/stripe_redirect.rb b/app/services/checkout/stripe_redirect.rb index 40feebfde8..e7c40a4b9e 100644 --- a/app/services/checkout/stripe_redirect.rb +++ b/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 diff --git a/engines/order_management/app/services/order_management/subscriptions/payment_setup.rb b/engines/order_management/app/services/order_management/subscriptions/payment_setup.rb index 86ce1f2c29..1824247e37 100644 --- a/engines/order_management/app/services/order_management/subscriptions/payment_setup.rb +++ b/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 diff --git a/engines/order_management/app/services/order_management/subscriptions/stripe_payment_setup.rb b/engines/order_management/app/services/order_management/subscriptions/stripe_payment_setup.rb new file mode 100644 index 0000000000..8bc7e4cb9e --- /dev/null +++ b/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 diff --git a/engines/order_management/app/services/order_management/subscriptions/stripe_sca_payment_authorize.rb b/engines/order_management/app/services/order_management/subscriptions/stripe_sca_payment_authorize.rb new file mode 100644 index 0000000000..da8830ce70 --- /dev/null +++ b/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 diff --git a/engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb index 442fb64cb5..32bf3874d0 100644 --- a/engines/order_management/spec/services/order_management/subscriptions/payment_setup_spec.rb +++ b/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 diff --git a/engines/order_management/spec/services/order_management/subscriptions/stripe_payment_setup_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/stripe_payment_setup_spec.rb new file mode 100644 index 0000000000..c928ddd95b --- /dev/null +++ b/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 diff --git a/engines/order_management/spec/services/order_management/subscriptions/stripe_sca_payment_authorize_spec.rb b/engines/order_management/spec/services/order_management/subscriptions/stripe_sca_payment_authorize_spec.rb new file mode 100644 index 0000000000..17ece20be4 --- /dev/null +++ b/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