Mark adjustments for removal in OrderTaxation

Instead of actually destroying adjustments in the OrderTaxation class,
we can just mark them for removal. This will allow them to be removed
when the order is persisted, instead of being removed during the
recalculate process. This is an improvement in service of recalculating
order changes without writing to the database.

We also need to ensure `Order#adjustments` association has autosave
enabled so that the marked for destruction adjustments are removed when
we persist the order.

We're also improving our OrderUpdater tests to better verify our changes
maintain the existing behaviour.

Co-authored-by: Adam Mueller <[email protected]>
Co-authored-by: Alistair Norman <[email protected]>
Co-authored-by: Chris Todorov <[email protected]>
Co-authored-by: Harmony Bouvier <[email protected]>
Co-authored-by: Kendra Riga <[email protected]>
Co-authored-by: Senem Soy <[email protected]>
Co-authored-by: Sofia Besenski <[email protected]>
Co-authored-by: benjamin wil <[email protected]>

Author: 9 people

core/app/models/spree/order.rb

@@ -102,7 +102,7 @@ def states
     has_many :cartons, -> { distinct }, through: :inventory_units
 
     # Adjustments and promotions
-    has_many :adjustments, -> { order(:created_at) }, as: :adjustable, inverse_of: :adjustable, dependent: :destroy
+    has_many :adjustments, -> { order(:created_at) }, as: :adjustable, inverse_of: :adjustable, dependent: :destroy, autosave: true
     has_many :line_item_adjustments, through: :line_items, source: :adjustments
     has_many :shipment_adjustments, through: :shipments, source: :adjustments
     has_many :all_adjustments,

core/app/models/spree/order_taxation.rb

@@ -57,7 +57,7 @@ def update_adjustments(item, taxed_items)
 
       # Remove any tax adjustments tied to rates which no longer match.
       unmatched_adjustments = tax_adjustments - active_adjustments
-      item.adjustments.destroy(unmatched_adjustments)
+      unmatched_adjustments.each(&:mark_for_destruction)
     end
 
     # Update or create a new tax adjustment on an item.

core/app/models/spree/order_updater.rb

@@ -157,9 +157,12 @@ def update_adjustment_total
       recalculate_adjustments
 
       all_items = line_items + shipments
-      order_tax_adjustments = adjustments.select(&:tax?)
+      # Ignore any adjustments that have been marked for destruction in our
+      # calculations. They'll get removed when/if we persist the order.
+      valid_adjustments = adjustments.reject(&:marked_for_destruction?)
+      order_tax_adjustments = valid_adjustments.select(&:tax?)
 
-      order.adjustment_total = all_items.sum(&:adjustment_total) + adjustments.sum(&:amount)
+      order.adjustment_total = all_items.sum(&:adjustment_total) + valid_adjustments.sum(&:amount)
       order.included_tax_total = all_items.sum(&:included_tax_total) + order_tax_adjustments.select(&:included?).sum(&:amount)
       order.additional_tax_total = all_items.sum(&:additional_tax_total) + order_tax_adjustments.reject(&:included?).sum(&:amount)
 

core/spec/models/spree/order_taxation_spec.rb

@@ -132,12 +132,11 @@
         )
       end
 
-      it "removes the tax adjustment" do
-        expect {
-          taxation.apply(new_taxes)
-        }.to change {
-          line_item.adjustments.size
-        }.from(1).to(0)
+      it "marks the tax adjustment for destruction" do
+        order.save!
+        taxation.apply(new_taxes)
+
+        expect(line_item.adjustments.first).to be_marked_for_destruction
       end
     end
 

core/spec/models/spree/order_updater_spec.rb

@@ -69,28 +69,39 @@ module Spree
       end
 
       describe 'tax recalculation' do
-        let!(:ship_address) { create(:address) }
-        let!(:tax_zone) { create(:global_zone) } # will include the above address
-        let!(:tax_rate) { create(:tax_rate, zone: tax_zone, tax_categories: [tax_category]) }
+        let(:tax_category) { create(:tax_category) }
+        let(:ship_address) { create(:address, state: new_york) }
+        let(:new_york) { create(:state, state_code: "NY") }
+        let(:tax_zone) { create(:zone, states: [new_york]) }
+
+        let!(:tax_rate) do
+          create(
+            :tax_rate,
+            name: "New York Sales Tax",
+            tax_categories: [tax_category],
+            zone: tax_zone,
+            included_in_price: false,
+            amount: 0.1
+          )
+        end
 
         let(:order) do
           create(
             :order_with_line_items,
-            line_items_attributes: [{ price: 10, variant: }],
-            ship_address:,
+            line_items_attributes: [{ price: 10, variant: variant }],
+            ship_address: ship_address,
           )
         end
         let(:line_item) { order.line_items[0] }
 
         let(:variant) { create(:variant, tax_category:) }
-        let(:tax_category) { create(:tax_category) }
 
         context 'when the item quantity has changed' do
           before do
             line_item.update!(quantity: 2)
           end
 
-          it 'updates the promotion amount' do
+          it 'updates the additional_tax_total' do
             expect {
               order.recalculate
             }.to change {
@@ -99,6 +110,60 @@ module Spree
           end
         end
 
+        context 'when the address has changed to a different state' do
+          let(:new_shipping_address) { create(:address) }
+
+          before do
+            order.ship_address = new_shipping_address
+          end
+
+          it 'removes the old taxes' do
+            expect {
+              order.recalculate
+            }.to change {
+              order.all_adjustments.tax.count
+            }.from(1).to(0)
+
+            expect(order.additional_tax_total).to eq 0
+            expect(order.adjustment_total).to eq 0
+          end
+        end
+
+        context "with an order-level tax adjustment" do
+          let(:colorado) { create(:state, state_code: "CO") }
+          let(:colorado_tax_zone) { create(:zone, states: [colorado]) }
+          let(:ship_address) { create(:address, state: colorado) }
+
+          let!(:colorado_delivery_fee) do
+            create(
+              :tax_rate,
+              amount: 0.27,
+              calculator: Spree::Calculator::FlatFee.new,
+              level: "order",
+              name: "Colorado Delivery Fee",
+              tax_categories: [tax_category],
+              zone: colorado_tax_zone
+            )
+          end
+
+          before { order.recalculate }
+
+          it "updates the order-level tax adjustment" do
+            expect {
+              order.ship_address = create(:address)
+              order.recalculate
+            }.to change { order.additional_tax_total }.from(0.27).to(0).
+                and change { order.adjustment_total }.from(0.27).to(0)
+          end
+
+          it "deletes the order-level tax adjustments when it persists the order" do
+            expect {
+              order.ship_address = create(:address)
+              order.recalculate
+            }.to change { order.all_adjustments.count }.from(1).to(0)
+          end
+        end
+
         context 'with a custom tax_calculator_class' do
           let(:custom_calculator_class) { double }
           let(:custom_calculator_instance) { double }