From 3e02023bf8b06a766678615edd055daf92d03248 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Mon, 15 Nov 2021 15:31:12 +0000
Subject: [PATCH] Move handling of unused payments to Order::Updater

An order can be set to paid in various cases that are unrelated to the order being finalized, so this bit of logic needs to be called at the point the order actually gets paid.
---
 app/models/spree/order.rb                        |  7 -------
 .../services/order_management/order/updater.rb   |  7 +++++++
 .../order_management/order/updater_spec.rb       | 16 ++++++++++++++++
 spec/models/spree/order_spec.rb                  | 15 ---------------
 4 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb
index 332af710e8..8bb09d4e5d 100644
--- a/app/models/spree/order.rb
+++ b/app/models/spree/order.rb
@@ -344,7 +344,6 @@ module Spree
 
       # update payment and shipment(s) states, and save
       updater.update_payment_state
-      cancel_payments_requiring_auth
       shipments.each do |shipment|
         shipment.update!(self)
         shipment.finalize!
@@ -612,12 +611,6 @@ module Spree
 
     private
 
-    def cancel_payments_requiring_auth
-      return unless payment_state == "paid"
-
-      payments.to_a.select(&:requires_authorization?).each(&:void_transaction!)
-    end
-
     def fee_handler
       @fee_handler ||= OrderFeesHandler.new(self)
     end
diff --git a/engines/order_management/app/services/order_management/order/updater.rb b/engines/order_management/app/services/order_management/order/updater.rb
index e17a9e3ad6..41d0374047 100644
--- a/engines/order_management/app/services/order_management/order/updater.rb
+++ b/engines/order_management/app/services/order_management/order/updater.rb
@@ -122,6 +122,7 @@ module OrderManagement
         last_payment_state = order.payment_state
 
         order.payment_state = infer_payment_state
+        cancel_payments_requiring_auth unless last_payment_state == "paid"
         track_payment_state_change(last_payment_state)
 
         order.payment_state
@@ -162,6 +163,12 @@ module OrderManagement
 
       private
 
+      def cancel_payments_requiring_auth
+        return unless order.payment_state == "paid"
+
+        payments.to_a.select(&:requires_authorization?).each(&:void_transaction!)
+      end
+
       def round_money(value)
         (value * 100).round / 100.0
       end
diff --git a/engines/order_management/spec/services/order_management/order/updater_spec.rb b/engines/order_management/spec/services/order_management/order/updater_spec.rb
index 142c042c81..3c46b8ebc4 100644
--- a/engines/order_management/spec/services/order_management/order/updater_spec.rb
+++ b/engines/order_management/spec/services/order_management/order/updater_spec.rb
@@ -360,6 +360,22 @@ module OrderManagement
           end
         end
       end
+
+      context "when unused payments records exist which require authorization, but the order is fully paid" do
+        let!(:cash_payment) { build(:payment, state: "completed", amount: order.new_outstanding_balance) }
+        let!(:stripe_payment) { build(:payment, state: "requires_authorization") }
+        before do
+          order.payments << cash_payment
+          order.payments << stripe_payment
+        end
+
+        it "cancels unused payments requiring authorization" do
+          expect(stripe_payment).to receive(:void_transaction!)
+          expect(cash_payment).to_not receive(:void_transaction!)
+
+          order.updater.update_payment_state
+        end
+      end
     end
   end
 end
diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb
index 633639d137..ecc4d5dae8 100644
--- a/spec/models/spree/order_spec.rb
+++ b/spec/models/spree/order_spec.rb
@@ -195,21 +195,6 @@ describe Spree::Order do
       expect(order.updater).to receive(:before_save_hook)
       order.finalize!
     end
-
-    context "extra payments exist that require authorization" do
-      let!(:cash_payment) { build(:payment, state: "completed", amount: order.new_outstanding_balance) }
-      let!(:stripe_payment) { build(:payment, state: "requires_authorization") }
-      before do
-        order.payments << cash_payment
-        order.payments << stripe_payment
-      end
-
-      it "cancels payments requiring authorization" do
-        expect(stripe_payment).to receive(:void_transaction!)
-        expect(cash_payment).to_not receive(:void_transaction!)
-        order.finalize!
-      end
-    end
   end
 
   context "#process_payments!" do