From 9bcf0d5b38cade71e772167303e65bbdfc40d4b0 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 11 Jun 2021 15:08:11 +0100 Subject: [PATCH 1/8] Handle Stripe payments when checkout fails due to stock issues This can occur when stock is reduced after the user is redirected to Stripe and before they are redirected back. The stock is insufficient, the order is not complete, the user is bounced back to the cart, but a *pending* Stripe payment is left on the order. --- app/controllers/checkout_controller.rb | 20 +++++++++++++++++--- config/locales/en.yml | 1 + 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/app/controllers/checkout_controller.rb b/app/controllers/checkout_controller.rb index 13b6654782..37bdff3954 100644 --- a/app/controllers/checkout_controller.rb +++ b/app/controllers/checkout_controller.rb @@ -37,7 +37,7 @@ class CheckoutController < ::BaseController # This is only required because of spree_paypal_express. If we implement # a version of paypal that uses this controller, and more specifically # the #action_failed method, then we can remove this call - OrderCheckoutRestart.new(@order).call + reset_order_to_cart rescue Spree::Core::GatewayError => e rescue_from_spree_gateway_error(e) end @@ -82,7 +82,7 @@ class CheckoutController < ::BaseController @order = current_order redirect_to(main_app.shop_path) && return if redirect_to_shop? - redirect_to_cart_path && return unless valid_order_line_items? + handle_invalid_stock && return unless valid_order_line_items? before_address setup_for_current_state @@ -100,7 +100,10 @@ class CheckoutController < ::BaseController distributes_order_variants?(@order) end - def redirect_to_cart_path + def handle_invalid_stock + cancel_incomplete_payments if valid_payment_intent_provided? + reset_order_to_cart + respond_to do |format| format.html do redirect_to main_app.cart_path @@ -112,6 +115,17 @@ class CheckoutController < ::BaseController end end + def cancel_incomplete_payments + # 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.pending.each(&:void!) + flash[:notice] = I18n.t("checkout.payment_cancelled_due_to_stock") + end + + def reset_order_to_cart + OrderCheckoutRestart.new(@order).call + end + def setup_for_current_state method_name = :"before_#{@order.state}" __send__(method_name) if respond_to?(method_name, true) diff --git a/config/locales/en.yml b/config/locales/en.yml index 84140e72bc..76943a0815 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1315,6 +1315,7 @@ en: message_html: "I agree to the seller's %{terms_and_conditions_link} and the platform %{tos_link}." terms_and_conditions: "Terms and Conditions" failed: "The checkout failed. Please let us know so that we can process your order." + payment_cancelled_due_to_stock: "Payment cancelled: the checkout could not be completed due to stock issues." shops: hubs: show_closed_shops: "Show closed shops" From 2cd6b05abaddd00b9818213311aa30bf89ba6b09 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 14 Jun 2021 19:51:09 +0100 Subject: [PATCH 2/8] Use Order::Contents service in test setup --- spec/controllers/checkout_controller_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb index d1813fa16a..24d4ec6e91 100644 --- a/spec/controllers/checkout_controller_spec.rb +++ b/spec/controllers/checkout_controller_spec.rb @@ -49,8 +49,7 @@ describe CheckoutController, type: :controller do let(:shipping_method) { distributor.shipping_methods.first } before do - order.line_items << create(:line_item, - variant: order_cycle.variants_distributed_by(distributor).first) + order.contents.add(order_cycle.variants_distributed_by(distributor).first) allow(controller).to receive(:current_distributor).and_return(distributor) allow(controller).to receive(:current_order_cycle).and_return(order_cycle) From a199f80ed40aaa5948f7596f96c230d8a33fae37 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 14 Jun 2021 19:51:37 +0100 Subject: [PATCH 3/8] Add test for insufficient stock behavior after Stripe redirect --- spec/controllers/checkout_controller_spec.rb | 48 ++++++++++++++++---- 1 file changed, 38 insertions(+), 10 deletions(-) diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb index 24d4ec6e91..bde41ddce5 100644 --- a/spec/controllers/checkout_controller_spec.rb +++ b/spec/controllers/checkout_controller_spec.rb @@ -91,19 +91,47 @@ describe CheckoutController, type: :controller do allow(OrderCycleDistributedVariants).to receive(:new).and_return(order_cycle_distributed_variants) end - it "redirects when some items are out of stock" do - allow(order).to receive_message_chain(:insufficient_stock_lines, :empty?).and_return false + context "running out of stock" do + it "redirects when some items are out of stock" do + allow(order).to receive_message_chain(:insufficient_stock_lines, :empty?).and_return false - get :edit - expect(response).to redirect_to cart_path - end + get :edit + expect(response).to redirect_to cart_path + end - it "redirects when some items are not available" do - allow(order).to receive_message_chain(:insufficient_stock_lines, :empty?).and_return true - expect(order_cycle_distributed_variants).to receive(:distributes_order_variants?).with(order).and_return(false) + it "redirects when some items are not available" do + allow(order).to receive_message_chain(:insufficient_stock_lines, :empty?).and_return true + expect(order_cycle_distributed_variants).to receive(:distributes_order_variants?).with(order).and_return(false) - get :edit - expect(response).to redirect_to cart_path + get :edit + expect(response).to redirect_to cart_path + end + + context "after redirecting back from Stripe" do + let(:order) { create(:order_with_totals_and_distribution) } + let!(:payment) { create(:payment, state: "pending", amount: order.total, order: order) } + + before do + allow(order).to receive_message_chain(:insufficient_stock_lines, :empty?).and_return(false) + allow(order_cycle_distributed_variants).to receive(:distributes_order_variants?). + with(order).and_return(true) + allow(controller).to receive(:valid_payment_intent_provided?) { true } + order.save + allow(order).to receive_message_chain(:payments, :completed) { [] } + allow(order).to receive_message_chain(:payments, :pending) { [payment] } + end + + it "cancels the payment and resets the order to cart" do + expect(payment).to receive(:void!).and_call_original + + spree_post :edit + + expect(response).to redirect_to cart_path + expect(flash[:notice]).to eq I18n.t('checkout.payment_cancelled_due_to_stock') + + expect(order.state).to eq "cart" + end + end end describe "when items are available and in stock" do From b1db06a3c680ade300407cf40b405435a8245862 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 9 Jul 2021 13:24:37 +0100 Subject: [PATCH 4/8] Allow payments in requires_authorization state to be voided --- app/models/spree/payment.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index 3bb4b60858..498c07577e 100644 --- a/app/models/spree/payment.rb +++ b/app/models/spree/payment.rb @@ -72,7 +72,7 @@ module Spree transition from: [:processing, :pending, :checkout, :requires_authorization], to: :completed end event :void do - transition from: [:pending, :completed, :checkout], to: :void + transition from: [:pending, :completed, :requires_authorization, :checkout], to: :void end # when the card brand isnt supported event :invalidate do From b4a9a6fea5585021f2ba945d717230055f0e66c5 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 9 Jul 2021 13:28:19 +0100 Subject: [PATCH 5/8] Add :incomplete scope in Spree::Payment --- app/models/spree/payment.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index 498c07577e..b41e085cde 100644 --- a/app/models/spree/payment.rb +++ b/app/models/spree/payment.rb @@ -46,6 +46,7 @@ module Spree scope :from_credit_card, -> { where(source_type: 'Spree::CreditCard') } scope :with_state, ->(s) { where(state: s.to_s) } scope :completed, -> { with_state('completed') } + scope :incomplete, -> { where(state: %w(checkout pending requires_authorization)) } scope :pending, -> { with_state('pending') } scope :failed, -> { with_state('failed') } scope :valid, -> { where('state NOT IN (?)', %w(failed invalid)) } From 2b8b6908921394beeb4f24647584f7cb5bd56e07 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 9 Jul 2021 13:28:38 +0100 Subject: [PATCH 6/8] Tidy up :valid scope in Spree::Payment --- app/models/spree/payment.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index b41e085cde..f33e2eab87 100644 --- a/app/models/spree/payment.rb +++ b/app/models/spree/payment.rb @@ -49,7 +49,7 @@ module Spree scope :incomplete, -> { where(state: %w(checkout pending requires_authorization)) } scope :pending, -> { with_state('pending') } scope :failed, -> { with_state('failed') } - scope :valid, -> { where('state NOT IN (?)', %w(failed invalid)) } + scope :valid, -> { where.not(state: %w(failed invalid)) } scope :authorization_action_required, -> { where.not(cvv_response_message: nil) } scope :requires_authorization, -> { with_state("requires_authorization") } scope :with_payment_intent, ->(code) { where(response_code: code) } From e6d9545c3064f7996a26f402078ac70d0ae8c454 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 9 Jul 2021 13:29:49 +0100 Subject: [PATCH 7/8] Use :incomplete scope when voiding payments that can't be processed due to stock changes --- 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 37bdff3954..5d0487ed85 100644 --- a/app/controllers/checkout_controller.rb +++ b/app/controllers/checkout_controller.rb @@ -118,7 +118,7 @@ class CheckoutController < ::BaseController def cancel_incomplete_payments # 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.pending.each(&:void!) + @order.payments.incomplete.each(&:void!) flash[:notice] = I18n.t("checkout.payment_cancelled_due_to_stock") end diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb index bde41ddce5..8e11b83bb5 100644 --- a/spec/controllers/checkout_controller_spec.rb +++ b/spec/controllers/checkout_controller_spec.rb @@ -118,7 +118,7 @@ describe CheckoutController, type: :controller do allow(controller).to receive(:valid_payment_intent_provided?) { true } order.save allow(order).to receive_message_chain(:payments, :completed) { [] } - allow(order).to receive_message_chain(:payments, :pending) { [payment] } + allow(order).to receive_message_chain(:payments, :incomplete) { [payment] } end it "cancels the payment and resets the order to cart" do From 8e1631bfc7396a41f487ebca824600af6121155c Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 12 Jul 2021 08:14:14 +0100 Subject: [PATCH 8/8] Set adjustments associated with voided payments to ineligible. Otherwise we can end up with duplicate transaction fees for voided payments. --- app/controllers/checkout_controller.rb | 5 ++++- spec/controllers/checkout_controller_spec.rb | 7 +++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/controllers/checkout_controller.rb b/app/controllers/checkout_controller.rb index 5d0487ed85..547f180a72 100644 --- a/app/controllers/checkout_controller.rb +++ b/app/controllers/checkout_controller.rb @@ -118,7 +118,10 @@ class CheckoutController < ::BaseController def cancel_incomplete_payments # 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(&:void!) + @order.payments.incomplete.each do |payment| + payment.void! + payment.adjustment&.update_columns(eligible: false, state: "finalized") + end flash[:notice] = I18n.t("checkout.payment_cancelled_due_to_stock") end diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb index 8e11b83bb5..e0e00a3ff7 100644 --- a/spec/controllers/checkout_controller_spec.rb +++ b/spec/controllers/checkout_controller_spec.rb @@ -110,6 +110,9 @@ describe CheckoutController, type: :controller do context "after redirecting back from Stripe" do let(:order) { create(:order_with_totals_and_distribution) } let!(:payment) { create(:payment, state: "pending", amount: order.total, order: order) } + let!(:transaction_fee) { + create(:adjustment, state: "open", amount: 10, order: order, adjustable: payment) + } before do allow(order).to receive_message_chain(:insufficient_stock_lines, :empty?).and_return(false) @@ -119,6 +122,7 @@ describe CheckoutController, type: :controller do order.save allow(order).to receive_message_chain(:payments, :completed) { [] } allow(order).to receive_message_chain(:payments, :incomplete) { [payment] } + allow(payment).to receive(:adjustment) { transaction_fee } end it "cancels the payment and resets the order to cart" do @@ -130,6 +134,9 @@ describe CheckoutController, type: :controller do expect(flash[:notice]).to eq I18n.t('checkout.payment_cancelled_due_to_stock') expect(order.state).to eq "cart" + expect(payment.state).to eq "void" + expect(transaction_fee.reload.eligible).to eq false + expect(transaction_fee.state).to eq "finalized" end end end