From d3c402a595d8d57ed5c57a5b0bbb10ae652760d5 Mon Sep 17 00:00:00 2001
From: Richard Wilson <richard@freerunningtech.com>
Date: Thu, 1 Oct 2015 11:08:20 -0700
Subject: [PATCH] FIx a stale data issue when using Spree::OrderCapturing.

As described in the comment, calling `payment.capture!` triggers an
after_save on Spree::Payment. In this after_save, things can happen, one
of those things is an `order.update!`.

That means if the `order.update!` happens within a payment after save, the order we have stored in `@order` is now out of date, and has to be reloaded.

If we don't reload it, the hooks in the order updater will be called
twice. If they make decisions around fresh state changes, idempotency
could be compromised.
---
 core/app/models/spree/order_capturing.rb      |  5 +++-
 .../spec/models/spree/order_capturing_spec.rb | 29 +++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/core/app/models/spree/order_capturing.rb b/core/app/models/spree/order_capturing.rb
index 94d33ecb3c2..bd5c5222a68 100644
--- a/core/app/models/spree/order_capturing.rb
+++ b/core/app/models/spree/order_capturing.rb
@@ -33,7 +33,10 @@ def capture_payments
           end
         end
       ensure
-        @order.update!
+        # FIXME: Adding the inverse_of on the payments relation for orders -should- fix this,
+        # however it only appears to make it worse (calling with changes three times instead of once.
+        # Warrants an investigation. Reloading for now.
+        @order.reload.update!
       end
     end
   end
diff --git a/core/spec/models/spree/order_capturing_spec.rb b/core/spec/models/spree/order_capturing_spec.rb
index f1594a6c4c2..a2980cc2d2f 100644
--- a/core/spec/models/spree/order_capturing_spec.rb
+++ b/core/spec/models/spree/order_capturing_spec.rb
@@ -4,6 +4,35 @@
   describe '#capture_payments' do
     subject { Spree::OrderCapturing.new(order, payment_methods).capture_payments }
 
+    # Regression for the order.update! in the ensure block.
+    # See the comment there.
+    context "updating the order" do
+      let(:order) { create :completed_order_with_totals }
+      let(:payment_methods) { [] }
+      let!(:payment) { create(:payment, order: order, amount: order.total) }
+      let(:changes_spy) { spy('changes_spy') }
+
+      before do
+        payment.pend!
+
+        allow_any_instance_of(Spree::Order).to receive(:thingamajig) do |order|
+          changes_spy.change_callback_occured if order.changes.any?
+        end
+
+        @update_hooks = Spree::Order.update_hooks.dup
+        Spree::Order.register_update_hook :thingamajig
+      end
+
+      after do
+        Spree::Order.update_hooks = @update_hooks
+      end
+
+      it "keeps the order up to date when updating and only changes it once" do
+        subject
+        expect(changes_spy).to have_received(:change_callback_occured).once
+      end
+    end
+
     context "payment methods specified" do
       let!(:order) { create(:order, ship_address: create(:address)) }