Autosave shipment adjustments to persist taxes on recalculate

A commit was made in solidusio#6315 (5979f25) in service of the in memory order updater solidusio#5872 that changed the behavior in the OrderTaxation class to set the adjustment value but not persist it. Instead, the persistence happens when the order is saved. If a shipment is changed, and an associated adjustment is changed, the shipment adjustment will be autosaved when the order is.

However, if a shipment is *not* changed, but the adjustment is, the adjustment will *not* be autosaved when the order is.

A scenario where this can happen is a recalculation that just adjusts shipment taxes on an order that would otherwise not change. Let's walk through what happens in the OrderUpdater

1. `update_totals` is called which in turn...
2. calls `update_adjustment_total`, which in turn...
3. calls `recalculate_adjustments` which in turn...
4. Calls `update_tax_adjustments`
- `update_tax_adjustments` creates and sets the values of the tax adjustments for a shipment. At this point, the shipment adjustment has been changed, but not persisted, and the shipment has *not* been changed.
5. Calls `update_item_totals` which calls...
6. `Spree::Config.item_total_class.new(item).recalculate!`
- `Spree::Config.item_total_class` sets the `included_tax_total`, `additional_tax_total` and `adjustment_total` on the shipment to their new values, calculated from the non-persisted adjustment. At this point, both the shipment *and* adjustment have been changed but not persisted.
- If you were to call `order.save!` at this point, the shipment adjustment would autosave.
7. Next, `item.update_columns` is called with the columns that the `item_total_class` updated.
- At this point, the shipment, it's changes having been persisted to the database, will no longer be marked as `changed`.
- This means that the default behavior of autosave in rails will *not* save the shipment when the order is saved, which means the associated (and changed) adjustment will not get saved either

In solidusio#6315, we ensured that these changed associations cannot get left behind by setting `autosave: true`, which forces the model to check if it has any associations that need saving instead of telling it's relation "nope I haven't changed"

We can do the same thing here for the shipment to ensure changing  adjustments will make the shipment tell it's order "I have changed", even if none of it's direct properties have changed.

You can test this change by removing the `autosave: true` option the adjustments and running 'when the address has changed to a different state' specs. They fail with the error "expected `order.shipments.first.adjustments.first.amount` to have changed from 1 to 2, but did not change", demonstrating a scenario where the shipment adjustment changes do not get persisted.

Author: Noah-Silvera

core/app/models/spree/shipment.rb

@@ -9,7 +9,7 @@ class Shipment < Spree::Base
     belongs_to :order, class_name: 'Spree::Order', touch: true, inverse_of: :shipments, optional: true
     belongs_to :stock_location, class_name: 'Spree::StockLocation', optional: true
 
-    has_many :adjustments, as: :adjustable, inverse_of: :adjustable, dependent: :delete_all
+    has_many :adjustments, as: :adjustable, inverse_of: :adjustable, dependent: :delete_all, autosave: true
     has_many :inventory_units, dependent: :destroy, inverse_of: :shipment
     has_many :shipping_rates, -> { order(:cost) }, dependent: :destroy, inverse_of: :shipment
     has_many :shipping_methods, through: :shipping_rates

core/spec/models/spree/order_updater_spec.rb

@@ -72,14 +72,14 @@ module Spree
         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(:new_york_tax_zone) { create(:zone, states: [new_york]) }
 
-        let!(:tax_rate) do
+        let!(:new_york_tax_rate) do
           create(
             :tax_rate,
             name: "New York Sales Tax",
             tax_categories: [tax_category],
-            zone: tax_zone,
+            zone: new_york_tax_zone,
             included_in_price: false,
             amount: 0.1
           )
@@ -111,21 +111,67 @@ module Spree
         end
 
         context 'when the address has changed to a different state' do
-          let(:new_shipping_address) { create(:address) }
+          let(:oregon) { create(:state, state_code: "OR") }
+          let(:oregon_tax_zone) { create(:zone, states: [oregon]) }
+          let!(:oregon_tax_rate) do
+            create(
+              :tax_rate,
+              name: "Oregon Sales Tax",
+              tax_categories: [tax_category],
+              zone: oregon_tax_zone,
+              included_in_price: false,
+              amount: 0.2
+            )
+          end
+          let(:new_address) { create(:address, state: oregon) }
+          let(:shipping_method) { create(:shipping_method, tax_category:, zones: [oregon_tax_zone, new_york_tax_zone], cost: 10) }
+          let(:shipping_rate) do
+            create(:shipping_rate, cost: 10, shipping_method: shipping_method)
+          end
+          let(:shipment) { order.shipments[0] }
+
+          subject do
+            order.ship_address = new_address
+            order.bill_address = new_address
+
+            order.recalculate
+          end
 
           before do
-            order.ship_address = new_shipping_address
+            shipment.shipping_rates = [shipping_rate]
+            shipment.selected_shipping_rate_id = shipping_rate.id
+            order.recalculate
           end
 
-          it 'removes the old taxes' do
+          it 'updates the taxes to reflect the new state' do
             expect {
-              order.recalculate
+              subject
             }.to change {
-              order.all_adjustments.tax.count
-            }.from(1).to(0)
+              order.additional_tax_total
+            }.from(2).to(4)
+          end
 
-            expect(order.additional_tax_total).to eq 0
-            expect(order.adjustment_total).to eq 0
+          it 'updates the shipment taxes to reflect the new state' do
+            $started_spec = true
+            expect {
+              subject
+            }.to change {
+              order.shipments.first.additional_tax_total
+            }.from(1).to(2)
+            .and change {
+              order.shipments.first.adjustments.first.amount
+            }.from(1).to(2)
+          end
+
+          it 'updates the line item taxes to reflect the new state' do
+            expect {
+              subject
+            }.to change {
+              order.line_items.first.additional_tax_total
+            }.from(1).to(2)
+            .and change {
+              order.line_items.first.adjustments.first.amount
+            }.from(1).to(2)
           end
         end
 
@@ -179,7 +225,7 @@ module Spree
                 order_taxes: [
                   Spree::Tax::ItemTax.new(
                     label: "Delivery Fee",
-                    tax_rate:,
+                    tax_rate: new_york_tax_rate,
                     amount: 2.60,
                     included_in_price: false
                   )
@@ -188,7 +234,7 @@ module Spree
                   Spree::Tax::ItemTax.new(
                     item_id: line_item.id,
                     label: "Item Tax",
-                    tax_rate:,
+                    tax_rate: new_york_tax_rate,
                     amount: 1.40,
                     included_in_price: false
                   )