From fcb8145a6c1aac4002e311f956f83c985a13cb9c Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 23 Mar 2021 11:18:10 +0000 Subject: [PATCH] Bring in changes to Adjustment#update! and CalculatedAdjustments#update_adjustment --- app/models/spree/adjustment.rb | 12 +++++++----- lib/spree/core/calculated_adjustments.rb | 13 ------------- .../spree/core/calculated_adjustments_spec.rb | 9 --------- spec/models/spree/adjustment_spec.rb | 17 ++++++++--------- 4 files changed, 15 insertions(+), 36 deletions(-) diff --git a/app/models/spree/adjustment.rb b/app/models/spree/adjustment.rb index 7e6e314b4c..a0bd3e1541 100644 --- a/app/models/spree/adjustment.rb +++ b/app/models/spree/adjustment.rb @@ -100,12 +100,14 @@ module Spree # adjustment calculations would not performed on proper values def update!(calculable = nil, force: false) return if immutable? && !force + return if originator.blank? - # Fix for Spree issue #3381 - # If we attempt to call 'source' before the reload, then source is currently - # the order object. After calling a reload, the source is the Shipment. - reload - originator.update_adjustment(self, calculable || source) if originator.present? + amount = originator.compute_amount(calculable || adjustable) + + update_columns( + amount: amount, + updated_at: Time.zone.now, + ) end def currency diff --git a/lib/spree/core/calculated_adjustments.rb b/lib/spree/core/calculated_adjustments.rb index 944eff67b6..f773567647 100644 --- a/lib/spree/core/calculated_adjustments.rb +++ b/lib/spree/core/calculated_adjustments.rb @@ -54,21 +54,8 @@ module Spree end end - # Updates the amount of the adjustment using our Calculator and - # calling the +compute+ method with the +calculable+ - # referenced passed to the method. - def update_adjustment(adjustment, calculable) - # Adjustment calculations done on Spree::Shipment objects MUST - # be done on their to_package'd variants instead - # It's only the package that contains the correct information. - # See https://github.com/spree/spree_active_shipping/pull/96 et. al - calculable = calculable.to_package if calculable.is_a?(Spree::Shipment) - adjustment.update_column(:amount, compute_amount(calculable)) - end - # Calculate the amount to be used when creating an adjustment # NOTE: May be overriden by classes where this module is included into. - # Such as Spree::Promotion::Action::CreateAdjustment. def compute_amount(calculable) calculator.compute(calculable) end diff --git a/spec/lib/spree/core/calculated_adjustments_spec.rb b/spec/lib/spree/core/calculated_adjustments_spec.rb index 484654e2eb..f6870807e1 100644 --- a/spec/lib/spree/core/calculated_adjustments_spec.rb +++ b/spec/lib/spree/core/calculated_adjustments_spec.rb @@ -66,13 +66,4 @@ describe Spree::Core::CalculatedAdjustments do end end end - - context "#update_adjustment" do - it "should update the adjustment using its calculator (and the specified source)" do - adjustment = double(:adjustment).as_null_object - calculable = double :calculable - expect(adjustment).to receive(:update_column).with(:amount, 10) - tax_rate.update_adjustment(adjustment, calculable) - end - end end diff --git a/spec/models/spree/adjustment_spec.rb b/spec/models/spree/adjustment_spec.rb index c1af27f7fe..377162d135 100644 --- a/spec/models/spree/adjustment_spec.rb +++ b/spec/models/spree/adjustment_spec.rb @@ -18,39 +18,38 @@ module Spree context "#update!" do context "when originator present" do - let(:originator) { double("originator", update_adjustment: nil) } + let(:originator) { double("originator", compute_amount: 10.0) } before do - allow(originator).to receive_messages update_amount: true allow(adjustment).to receive_messages originator: originator, label: 'adjustment', amount: 0 end it "should do nothing when closed" do adjustment.close - expect(originator).not_to receive(:update_adjustment) + expect(originator).not_to receive(:compute_amount) adjustment.update! end it "should do nothing when finalized" do adjustment.finalize - expect(originator).not_to receive(:update_adjustment) + expect(originator).not_to receive(:compute_amount) adjustment.update! end - it "should ask the originator to update_adjustment" do - expect(originator).to receive(:update_adjustment) + it "should ask the originator to recalculate the amount" do + expect(originator).to receive(:compute_amount) adjustment.update! end context "using the :force argument" do it "should update adjustments without changing their state" do - expect(originator).to receive(:update_adjustment) + expect(originator).to receive(:compute_amount) adjustment.update!(force: true) expect(adjustment.state).to eq "open" end it "should update closed adjustments" do adjustment.close - expect(originator).to receive(:update_adjustment) + expect(originator).to receive(:compute_amount) adjustment.update!(force: true) end end @@ -58,7 +57,7 @@ module Spree it "should do nothing when originator is nil" do allow(adjustment).to receive_messages originator: nil - expect(adjustment).not_to receive(:amount=) + expect(adjustment).not_to receive(:update_columns) adjustment.update! end end