Preventing InMemoryOrderUpdater#update_shipment_amounts from making d…

…atabase writes

We have prevented write calls to update the cost and `updated_at` of a
shipment, as well as allowed us to conditionally persist item totals, by
passing down the `persist` argument to that method.

Co-authored-by: Adam Mueller <adam@super.gd>
Co-authored-by: Andrew Stewart <andrew@super.gd>
Co-authored-by: Harmony Evangelina <harmony@super.gd>
Co-authored-by: Kendra Riga <kendra@super.gd>
Co-authored-by: Jared Norman <jared@super.gd>
Co-authored-by: Tom Van Manen <tom@super.gd>
Co-authored-by: Chris Todorov <chris@super.gd>
Co-authored-by: Sofia Besenski <sofia@super.gd>
Co-authored-by: Senem Soy <senem@super.gd>
Co-authored-by: Benjamin Willems <benjamin@super.gd>

core/app/models/spree/in_memory_order_updater.rb

@@ -19,8 +19,8 @@ def initialize(order)
     def recalculate(persist: true)
       order.transaction do
         update_item_count
-        update_shipment_amounts
-        update_totals
+        update_shipment_amounts(persist:)
+        update_totals(persist:)
         if order.completed?
           update_payment_state
           update_shipments
@@ -107,15 +107,15 @@ def determine_shipment_state
     # fields (promo_total, included_tax_total, additional_tax_total, and
     # adjustment_total) on the item.
     # @return [void]
-    def recalculate_adjustments
+    def recalculate_adjustments(persist:)
       # Promotion adjustments must be applied first, then tax adjustments.
       # This fits the criteria for VAT tax as outlined here:
       # http://www.hmrc.gov.uk/vat/managing/charging/discounts-etc.htm#1
       # It also fits the criteria for sales tax as outlined here:
       # http://www.boe.ca.gov/formspubs/pub113/
       update_promotions
       update_tax_adjustments
-      update_item_totals
+      update_item_totals(persist:)
     end
 
     # Updates the following Order total values:
@@ -125,15 +125,15 @@ def recalculate_adjustments
     # +adjustment_total+   The total value of all adjustments (promotions, credits, etc.)
     # +promo_total+        The total value of all promotion adjustments
     # +total+              The so-called "order total."  This is equivalent to +item_total+ plus +adjustment_total+.
-    def update_totals
+    def update_totals(persist:)
       update_payment_total
       update_item_total
       update_shipment_total
-      update_adjustment_total
+      update_adjustment_total(persist:)
     end
 
-    def update_shipment_amounts
-      shipments.each(&:update_amounts)
+    def update_shipment_amounts(persist:)
+      shipments.each { _1.update_amounts(persist:) }
     end
 
     # give each of the shipments a chance to update themselves
@@ -154,8 +154,8 @@ def update_order_total
       order.total = order.item_total + order.shipment_total + order.adjustment_total
     end
 
-    def update_adjustment_total
-      recalculate_adjustments
+    def update_adjustment_total(persist:)
+      recalculate_adjustments(persist:)
 
       all_items = line_items + shipments
       # Ignore any adjustments that have been marked for destruction in our
@@ -210,11 +210,11 @@ def update_cancellations
     end
     deprecate :update_cancellations, deprecator: Spree.deprecator
 
-    def update_item_totals
+    def update_item_totals(persist:)
       [*line_items, *shipments].each do |item|
         Spree::Config.item_total_class.new(item).recalculate!
 
-        next unless item.changed?
+        next unless persist && item.changed?
 
         item.update_columns(
           promo_total:          item.promo_total,

core/spec/models/spree/in_memory_order_updater_spec.rb

@@ -37,6 +37,30 @@ module Spree
           expect(order.item_count).to eq 1
           expect(order.reload.item_count).to eq 0
         end
+
+        context 'when a shipment is attached to the order' do
+          let(:shipment) { create(:shipment) }
+
+          before do
+            order.shipments << shipment
+          end
+
+          it 'does not make manipulative database queries' do
+            expect {
+              subject
+            }.not_to make_database_queries(manipulative: true)
+          end
+
+          context 'when the shipment has a selected shipping rate' do
+            let(:shipment) { create(:shipment, shipping_rates: [build(:shipping_rate, selected: true)]) }
+
+            it 'does not make manipulative database queries' do
+              expect {
+                subject
+              }.not_to make_database_queries(manipulative: true)
+            end
+          end
+        end
       end
 
       context "when the persist flag is set to 'true'" do