process payments separately from completing the order

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 })

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
 

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)

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

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?

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

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

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

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