From aa8067f96bbf1e8ae2f54a82850b541d9b5bae83 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Sat, 12 Jun 2021 09:29:43 -0700 Subject: [PATCH] process payments separately from completing the order --- app/controllers/checkout_controller.rb | 8 +++-- .../spree/admin/orders_controller.rb | 6 +--- app/controllers/spree/paypal_controller.rb | 1 + app/services/place_proxy_order.rb | 1 + app/services/process_payment_intent.rb | 8 ++--- .../customer_details_controller_spec.rb | 3 +- .../payments/payments_controller_spec.rb | 4 +-- spec/models/spree/order/checkout_spec.rb | 32 ------------------- spec/models/spree/order/state_machine_spec.rb | 8 ++--- 9 files changed, 18 insertions(+), 53 deletions(-) diff --git a/app/controllers/checkout_controller.rb b/app/controllers/checkout_controller.rb index 095db28a49..13b6654782 100644 --- a/app/controllers/checkout_controller.rb +++ b/app/controllers/checkout_controller.rb @@ -140,6 +140,8 @@ class CheckoutController < ::BaseController end def handle_redirect_from_stripe + return checkout_failed unless @order.process_payments! + if OrderWorkflow.new(@order).next && order_complete? checkout_succeeded redirect_to(order_path(@order)) && return @@ -150,8 +152,10 @@ class CheckoutController < ::BaseController def checkout_workflow(shipping_method_id) while @order.state != "complete" - if @order.state == "payment" && redirect_to_payment_gateway - return + if @order.state == "payment" + return if redirect_to_payment_gateway + + return action_failed unless @order.process_payments! end next if OrderWorkflow.new(@order).next({ shipping_method_id: shipping_method_id }) diff --git a/app/controllers/spree/admin/orders_controller.rb b/app/controllers/spree/admin/orders_controller.rb index 505a182d39..f77ee4070d 100644 --- a/app/controllers/spree/admin/orders_controller.rb +++ b/app/controllers/spree/admin/orders_controller.rb @@ -28,11 +28,7 @@ module Spree def edit @order.shipments.map(&:refresh_rates) - OrderWorkflow.new(@order).complete - - # The payment step shows an error of 'No pending payments' - # Clearing the errors from the order object will stop this error - # appearing on the edit page where we don't want it to. + OrderWorkflow.new(@order).advance_to_payment @order.errors.clear end diff --git a/app/controllers/spree/paypal_controller.rb b/app/controllers/spree/paypal_controller.rb index a0478d579a..1bc70cd382 100644 --- a/app/controllers/spree/paypal_controller.rb +++ b/app/controllers/spree/paypal_controller.rb @@ -50,6 +50,7 @@ module Spree amount: @order.total, payment_method: payment_method ) + @order.process_payments! @order.next if @order.complete? flash.notice = Spree.t(:order_processed_successfully) diff --git a/app/services/place_proxy_order.rb b/app/services/place_proxy_order.rb index fa505c989b..d6a0666669 100644 --- a/app/services/place_proxy_order.rb +++ b/app/services/place_proxy_order.rb @@ -21,6 +21,7 @@ class PlaceProxyOrder load_changes return handle_empty_order if empty_order? + order.process_payments! if order.payment_required? move_to_completion send_placement_email rescue StandardError => e diff --git a/app/services/process_payment_intent.rb b/app/services/process_payment_intent.rb index 1737bb0711..1dc329d875 100644 --- a/app/services/process_payment_intent.rb +++ b/app/services/process_payment_intent.rb @@ -54,12 +54,8 @@ class ProcessPaymentIntent attr_reader :order, :payment_intent, :payment def process_payment - if order.state == "payment" - # Moves the order to completed, which calls #process_payments! - OrderWorkflow.new(order).next - else - order.process_payments! - end + OrderWorkflow.new(order).next if order.state == "payment" + order.process_payments! end def ready_for_capture? diff --git a/spec/controllers/spree/admin/orders/customer_details_controller_spec.rb b/spec/controllers/spree/admin/orders/customer_details_controller_spec.rb index 89a1466248..2247eb3517 100644 --- a/spec/controllers/spree/admin/orders/customer_details_controller_spec.rb +++ b/spec/controllers/spree/admin/orders/customer_details_controller_spec.rb @@ -16,7 +16,6 @@ describe Spree::Admin::Orders::CustomerDetailsController, type: :controller do :order_with_totals_and_distribution, state: 'cart', shipments: [shipment], - payments: [create(:payment)], distributor: distributor, user: nil, email: nil, @@ -47,7 +46,7 @@ describe Spree::Admin::Orders::CustomerDetailsController, type: :controller do spree_post :update, order: { email: user.email, bill_address_attributes: address_params, ship_address_attributes: address_params }, order_id: order.number - }.to change { order.reload.state }.from("cart").to("complete") + }.to change { order.reload.state }.from("cart").to("payment") end context "when adding details of a registered user" do diff --git a/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb b/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb index 545221f941..47d0376534 100644 --- a/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb +++ b/spec/controllers/spree/admin/orders/payments/payments_controller_spec.rb @@ -48,7 +48,7 @@ describe Spree::Admin::PaymentsController, type: :controller do let!(:payment_method) { create(:stripe_connect_payment_method, distributors: [shop]) } before do allow_any_instance_of(Spree::Payment). - to receive(:process!). + to receive(:process_offline!). and_raise(Spree::Core::GatewayError.new("Payment Gateway Error")) end @@ -110,7 +110,7 @@ describe Spree::Admin::PaymentsController, type: :controller do allow_any_instance_of(Spree::Payment).to receive(:authorize!) do |payment| payment.update state: "pending" end - allow_any_instance_of(Spree::Payment).to receive(:process!).and_return(true) + allow_any_instance_of(Spree::Payment).to receive(:process_offline!).and_return(true) end it "makes a payment with the provided card details" do diff --git a/spec/models/spree/order/checkout_spec.rb b/spec/models/spree/order/checkout_spec.rb index 19f678d359..76912a62e5 100644 --- a/spec/models/spree/order/checkout_spec.rb +++ b/spec/models/spree/order/checkout_spec.rb @@ -118,38 +118,6 @@ describe Spree::Order::Checkout do end end end - - context "from payment" do - before do - order.state = 'payment' - end - - context "when payment is required" do - before do - allow(order).to receive_messages confirmation_required?: false - allow(order).to receive_messages payment_required?: true - end - - it "transitions to complete" do - expect(order).to receive(:process_payments!).once.and_return true - order.next! - expect(order.state).to eq "complete" - end - end - - # Regression test for Spree #2028 - context "when payment is not required" do - before do - allow(order).to receive_messages payment_required?: false - end - - it "does not call process payments" do - expect(order).to_not receive(:process_payments!) - order.next! - expect(order.state).to eq "complete" - end - end - end end describe 'event :restart_checkout' do diff --git a/spec/models/spree/order/state_machine_spec.rb b/spec/models/spree/order/state_machine_spec.rb index 74c9f8cf25..8da1df767a 100644 --- a/spec/models/spree/order/state_machine_spec.rb +++ b/spec/models/spree/order/state_machine_spec.rb @@ -31,9 +31,9 @@ describe Spree::Order do context "when credit card processing fails" do before { allow(order).to receive_messages process_payments!: false } - it "should not complete the order" do + it "should still complete the order" do order.next - expect(order.state).to eq "payment" + expect(order.state).to eq "complete" end end end @@ -41,9 +41,9 @@ describe Spree::Order do context "when payment processing fails" do before { allow(order).to receive_messages process_payments!: false } - it "cannot transition to complete" do + it "can transition to complete" do order.next - expect(order.state).to eq "payment" + expect(order.state).to eq "complete" end end end