From a2b3d8372edfc1a09ac7d69dd42da3933eddff6b Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Tue, 18 Sep 2018 11:18:31 +0200
Subject: [PATCH] Reduce complexity of OrderUpdater decorator

---
 app/models/spree/order_updater_decorator.rb | 75 ++++++++++++++-------
 1 file changed, 51 insertions(+), 24 deletions(-)

diff --git a/app/models/spree/order_updater_decorator.rb b/app/models/spree/order_updater_decorator.rb
index 6ef5d2033c..03aba06c9b 100644
--- a/app/models/spree/order_updater_decorator.rb
+++ b/app/models/spree/order_updater_decorator.rb
@@ -1,41 +1,60 @@
 module Spree
   OrderUpdater.class_eval do
-    # TODO: This logic adapted from Spree 2.4, remove when we get there
-    # Handles state updating in a much more logical way than < 2.4
-    # Specifically, doesn't depend on payments.last to determine payment state
-    # Also swapped: == 0 for .zero?, .size == 0 for empty? and .size > 0 for !empty?
+    # TODO: This logic adapted from Spree 2.4, remove when we get there Handles
+    # state updating in a much more logical way than < 2.4 Specifically,
+    # doesn't depend on payments.last to determine payment state Also swapped:
+    # == 0 for .zero?, .size == 0 for empty? and .size > 0 for !empty?
+    #
     # See:
     # https://github.com/spree/spree/commit/38b8456183d11fc1e00e395e7c9154c76ef65b85
     # https://github.com/spree/spree/commit/7b264acff7824f5b3dc6651c106631d8f30b147a
     def update_payment_state
       last_payment_state = order.payment_state
 
-      if payments.present? && payments.valid.empty?
-        order.payment_state = 'failed'
-      elsif order.state == 'canceled' && order.payment_total.zero?
-        order.payment_state = 'void'
-      else
-        # This part added so that we don't need to override order.outstanding_balance
-        balance = order.outstanding_balance
-        balance = -1 * order.payment_total if canceled_and_paid_for?
-        order.payment_state = 'balance_due' if balance > 0
-        order.payment_state = 'credit_owed' if balance < 0
-        order.payment_state = 'paid' if balance.zero?
-
-        # Original logic
-        # order.payment_state = 'balance_due' if order.outstanding_balance > 0
-        # order.payment_state = 'credit_owed' if order.outstanding_balance < 0
-        # order.payment_state = 'paid' if !order.outstanding_balance?
-      end
-
+      order.payment_state = infer_payment_state
       track_payment_state_change(last_payment_state)
+
       order.payment_state
     end
 
     private
 
+    def infer_payment_state
+      if failed_payments?
+        'failed'
+      elsif canceled_and_not_paid_for?
+        'void'
+      else
+        infer_payment_state_from_balance
+      end
+    end
+
+    def infer_payment_state_from_balance
+      # This part added so that we don't need to override
+      # order.outstanding_balance
+      balance = order.outstanding_balance
+      balance = -1 * order.payment_total if canceled_and_paid_for?
+
+      infer_state(balance)
+    end
+
+    def infer_state(balance)
+      if balance > 0
+        'balance_due'
+      elsif balance < 0
+        'credit_owed'
+      elsif balance.zero?
+        'paid'
+      end
+    end
+
+    # Tracks the state transition through a state_change for this order. It
+    # does so until the last state is reached. That is, when the infered next
+    # state is the same as the order has now.
+    #
+    # @param last_payment_state [String]
     def track_payment_state_change(last_payment_state)
-      return unless last_payment_state != order.payment_state
+      return if last_payment_state == order.payment_state
       order.state_changed('payment')
     end
 
@@ -45,8 +64,16 @@ module Spree
       order.canceled? && paid?
     end
 
+    def canceled_and_not_paid_for?
+      order.state == 'canceled' && order.payment_total.zero?
+    end
+
     def paid?
-      order.payments.present? && !order.payments.completed.empty?
+      payments.present? && !payments.completed.empty?
+    end
+
+    def failed_payments?
+      payments.present? && payments.valid.empty?
     end
   end
 end