Merge pull request #1356 from jhawthorn/remove_adjustment_callbacks

Remove update_adjustments AR callbacks

CHANGELOG.md

@@ -49,6 +49,12 @@
 
 *   Removed "Clear cache" button from the admin [#1275](https://github.com/solidusio/solidus/pull/1275)
 
+*   Adjustments and totals are no longer updated when saving a Shipment or LineItem.
+
+    Previously adjustments and total columns were updated after saving a Shipment or LineItem.
+    This was unnecessary since it didn't update the order totals, and running
+    order.update! would recalculate the adjustments and totals again.
+
 ## Solidus 1.3.0 (unreleased)
 
 *   Order now requires a `store_id` in validations

core/app/models/spree/line_item.rb

@@ -26,7 +26,6 @@ class LineItem < Spree::Base
     after_create :update_tax_charge
 
     after_save :update_inventory
-    after_save :update_adjustments
 
     before_destroy :update_inventory
     before_destroy :destroy_inventory_units
@@ -167,17 +166,6 @@ def destroy_inventory_units
       inventory_units.destroy_all
     end
 
-    def update_adjustments
-      if quantity_changed?
-        update_tax_charge # Called to ensure pre_tax_amount is updated.
-        recalculate_adjustments
-      end
-    end
-
-    def recalculate_adjustments
-      Spree::ItemAdjustments.new(self).update
-    end
-
     def update_tax_charge
       Spree::Tax::ItemAdjuster.new(self).adjust!
     end

core/app/models/spree/shipment.rb

@@ -10,8 +10,6 @@ class Shipment < Spree::Base
     has_many :state_changes, as: :stateful
     has_many :cartons, -> { uniq }, through: :inventory_units
 
-    after_save :update_adjustments
-
     before_validation :set_cost_zero_when_nil
 
     before_destroy :ensure_can_destroy
@@ -404,20 +402,10 @@ def manifest_unstock(item)
       stock_location.unstock item.variant, item.quantity, self
     end
 
-    def recalculate_adjustments
-      Spree::ItemAdjustments.new(self).update
-    end
-
     def set_cost_zero_when_nil
       self.cost = 0 unless cost
     end
 
-    def update_adjustments
-      if cost_changed? && state != 'shipped'
-        recalculate_adjustments
-      end
-    end
-
     def ensure_can_destroy
       unless pending?
         errors.add(:state, :cannot_destroy, state: state)

core/spec/models/spree/line_item_spec.rb

@@ -26,25 +26,6 @@
   end
 
   context "#save" do
-    context "line item changes" do
-      before do
-        line_item.quantity = line_item.quantity + 1
-      end
-
-      it "triggers adjustment total recalculation" do
-        expect(line_item).to receive(:update_tax_charge) # Regression test for https://github.com/spree/spree/issues/4671
-        expect(line_item).to receive(:recalculate_adjustments)
-        line_item.save
-      end
-    end
-
-    context "line item does not change" do
-      it "does not trigger adjustment total recalculation" do
-        expect(line_item).not_to receive(:recalculate_adjustments)
-        line_item.save
-      end
-    end
-
     context "target_shipment is provided" do
       it "verifies inventory" do
         line_item.target_shipment = Spree::Shipment.new

core/spec/models/spree/shipment_spec.rb

@@ -585,32 +585,6 @@
     end
   end
 
-  context "after_save" do
-    context "line item changes" do
-      before do
-        shipment.cost = shipment.cost + 10
-      end
-
-      it "triggers adjustment total recalculation" do
-        expect(shipment).to receive(:recalculate_adjustments)
-        shipment.save
-      end
-
-      it "does not trigger adjustment recalculation if shipment has shipped" do
-        shipment.state = 'shipped'
-        expect(shipment).not_to receive(:recalculate_adjustments)
-        shipment.save
-      end
-    end
-
-    context "line item does not change" do
-      it "does not trigger adjustment total recalculation" do
-        expect(shipment).not_to receive(:recalculate_adjustments)
-        shipment.save
-      end
-    end
-  end
-
   context "currency" do
     it "returns the order currency" do
       expect(shipment.currency).to eq(order.currency)