From 1fdc57890135cdc3ea751e34bde564a51e6f4d02 Mon Sep 17 00:00:00 2001
From: Pau Perez <saulopefa@gmail.com>
Date: Mon, 17 Sep 2018 16:48:01 +0200
Subject: [PATCH] Increase readability (a bit) of OrderUpdater

---
 app/models/spree/order_updater_decorator.rb | 17 +++++++++--
 spec/models/spree/order_updater_spec.rb     | 31 +++++++++++++++++++++
 2 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/app/models/spree/order_updater_decorator.rb b/app/models/spree/order_updater_decorator.rb
index ab1d4eaf26..6ef5d2033c 100644
--- a/app/models/spree/order_updater_decorator.rb
+++ b/app/models/spree/order_updater_decorator.rb
@@ -8,7 +8,8 @@ module Spree
     # https://github.com/spree/spree/commit/38b8456183d11fc1e00e395e7c9154c76ef65b85
     # https://github.com/spree/spree/commit/7b264acff7824f5b3dc6651c106631d8f30b147a
     def update_payment_state
-      last_state = order.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?
@@ -26,16 +27,26 @@ module Spree
         # order.payment_state = 'credit_owed' if order.outstanding_balance < 0
         # order.payment_state = 'paid' if !order.outstanding_balance?
       end
-      order.state_changed('payment') if last_state != order.payment_state
+
+      track_payment_state_change(last_payment_state)
       order.payment_state
     end
 
     private
 
+    def track_payment_state_change(last_payment_state)
+      return unless last_payment_state != order.payment_state
+      order.state_changed('payment')
+    end
+
     # Taken from order.outstanding_balance in Spree 2.4
     # See: https://github.com/spree/spree/commit/7b264acff7824f5b3dc6651c106631d8f30b147a
     def canceled_and_paid_for?
-      order.canceled? && order.payments.present? && !order.payments.completed.empty?
+      order.canceled? && paid?
+    end
+
+    def paid?
+      order.payments.present? && !order.payments.completed.empty?
     end
   end
 end
diff --git a/spec/models/spree/order_updater_spec.rb b/spec/models/spree/order_updater_spec.rb
index 79bf384d49..957d7c596e 100644
--- a/spec/models/spree/order_updater_spec.rb
+++ b/spec/models/spree/order_updater_spec.rb
@@ -86,4 +86,35 @@ describe Spree::OrderUpdater do
       end
     end
   end
+
+  context 'when the set payment_state does not match the last payment_state' do
+    before { order.payment_state = 'previous_to_paid' }
+
+    context 'and the order is being updated' do
+      before { allow(order).to receive(:persisted?) { true } }
+
+      it 'creates a new state_change for the order' do
+        expect { updater.update_payment_state }
+          .to change { order.state_changes.size }.by(1)
+      end
+    end
+
+    context 'and the order is being created' do
+      before { allow(order).to receive(:persisted?) { false } }
+
+      it 'creates a new state_change for the order' do
+        expect { updater.update_payment_state }
+          .not_to change { order.state_changes.size }
+      end
+    end
+  end
+
+  context 'when the set payment_state matches the last payment_state' do
+    before { order.payment_state = 'paid' }
+
+    it 'does not create any state_change' do
+      expect { updater.update_payment_state }
+        .not_to change { order.state_changes.size }
+    end
+  end
 end