From 11de63f9d8956b3b0fad21833993b413413c22ea Mon Sep 17 00:00:00 2001 From: Jordan Brough Date: Thu, 25 Aug 2016 13:32:51 -0600 Subject: [PATCH 1/2] Add `inverse_of` to order.payments This doesn't seem to happen automatically and is more important now that we're using cached association objects in OrderUpdater (see Solidus PR 1389). Solidus 1.4 broke some of our specs without it. In a Solidus sandbox without this patch: >> o = Spree::Order.create! >> p = o.payments.create! >> puts p.order.object_id, o.object_id 70240633410460 70240634674300 With this patch: >> o = Spree::Order.create! >> p = o.payments.create! >> puts p.order.object_id, o.object_id 70145793491740 70145793491740 There is some important history around this particular `inverse_of` -- see solidusio/solidus PR 407 and spree/spree_gateway PR 132. Commit d7b8c73 in Solidus removed part of the specs around this, assuming that the `inverse_of` was automatically added, but it doesn't actually seem to be (see above). The remaining tests around the issue still pass. --- core/app/models/spree/order.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/app/models/spree/order.rb b/core/app/models/spree/order.rb index b354ede6254..91b61dfb896 100644 --- a/core/app/models/spree/order.rb +++ b/core/app/models/spree/order.rb @@ -47,7 +47,7 @@ class CannotRebuildShipments < StandardError; end belongs_to :store, class_name: 'Spree::Store' has_many :state_changes, as: :stateful has_many :line_items, -> { order(:created_at, :id) }, dependent: :destroy, inverse_of: :order - has_many :payments, dependent: :destroy + has_many :payments, dependent: :destroy, inverse_of: :order has_many :return_authorizations, dependent: :destroy, inverse_of: :order has_many :reimbursements, inverse_of: :order has_many :adjustments, -> { order(:created_at) }, as: :adjustable, inverse_of: :adjustable, dependent: :destroy From 863557822ca992b5deeda3f1b13a6b33f97540f6 Mon Sep 17 00:00:00 2001 From: Jordan Brough Date: Fri, 26 Aug 2016 11:22:55 -0600 Subject: [PATCH 2/2] Remove workaround in order_capturing See code comments in spec. --- core/app/models/spree/order_capturing.rb | 23 +++++++------------ .../spec/models/spree/order_capturing_spec.rb | 4 ++-- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/core/app/models/spree/order_capturing.rb b/core/app/models/spree/order_capturing.rb index 99184e8a313..4eb398b152e 100644 --- a/core/app/models/spree/order_capturing.rb +++ b/core/app/models/spree/order_capturing.rb @@ -21,22 +21,15 @@ def capture_payments Spree::OrderMutex.with_lock!(@order) do uncaptured_amount = @order.display_total.cents - begin - sorted_payments(@order).each do |payment| - amount = [uncaptured_amount, payment.money.cents].min - - if amount > 0 - payment.capture!(amount) - uncaptured_amount -= amount - elsif Spree::OrderCapturing.void_unused_payments - payment.void_transaction! - end + sorted_payments(@order).each do |payment| + amount = [uncaptured_amount, payment.money.cents].min + + if amount > 0 + payment.capture!(amount) + uncaptured_amount -= amount + elsif Spree::OrderCapturing.void_unused_payments + payment.void_transaction! end - ensure - # 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 c0d3f4fde31..16d31ff3e63 100644 --- a/core/spec/models/spree/order_capturing_spec.rb +++ b/core/spec/models/spree/order_capturing_spec.rb @@ -4,8 +4,8 @@ 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. + # Regression for https://github.com/solidusio/solidus/pull/407 + # See also https://github.com/solidusio/solidus/pull/1406 context "updating the order" do let(:order) { create :completed_order_with_totals } let(:payment_methods) { [] }