From 1f45b88ff3bfd97b77f2d5a8f163d1d9a6bbca17 Mon Sep 17 00:00:00 2001
From: benjamin wil <benjamin@super.gd>
Date: Fri, 11 Oct 2024 13:28:52 -0700
Subject: [PATCH 01/16] Add `db-query-matchers` gem

https://github.com/sds/db-query-matchers

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: Jared Norman <jared@super.gd>
Co-authored-by: Nick Van Doorn <nick@super.gd>
Co-authored-by: Noah Silvera <noah@super.gd>
Co-authored-by: Senem Soy <senem@super.gd>
Co-authored-by: Sofia Besenski <sofia@super.gd>
Co-authored-by: Tom Van Manen <tom@super.gd>
---
 Gemfile                   | 2 +-
 core/spec/rails_helper.rb | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Gemfile b/Gemfile
index 7f69dd3a643..921dbf3848e 100644
--- a/Gemfile
+++ b/Gemfile
@@ -21,7 +21,7 @@ gem 'pg', '~> 1.0', require: false if dbs.match?(/all|postgres/)
 gem 'fast_sqlite', require: false if dbs.match?(/all|sqlite/)
 gem 'sqlite3', '~> 1.4', require: false if dbs.match?(/all|sqlite/)
 
-
+gem 'db-query-matchers'
 gem 'database_cleaner', '~> 2.0', require: false
 gem 'rspec-activemodel-mocks', '~> 1.1', require: false
 gem 'rspec-rails', '~> 6.0.3', require: false
diff --git a/core/spec/rails_helper.rb b/core/spec/rails_helper.rb
index f3b6e161509..c135d3893f4 100644
--- a/core/spec/rails_helper.rb
+++ b/core/spec/rails_helper.rb
@@ -13,6 +13,7 @@
 require 'rspec/rails'
 require 'rspec-activemodel-mocks'
 require 'database_cleaner'
+require 'db-query-matchers'
 
 Dir["./spec/support/**/*.rb"].sort.each { |f| require f }
 

From a3003e17e9db4e2846c15a82401423b0c84f143d Mon Sep 17 00:00:00 2001
From: benjamin wil <benjamin@super.gd>
Date: Fri, 11 Oct 2024 13:43:22 -0700
Subject: [PATCH 02/16] Copy existing `OrderUpdater` implementation

In subsequent commits we'll ensure that this can update orders in
memory, without persisting changes using manipulative DB queries.

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: Jared Norman <jared@super.gd>
Co-authored-by: Nick Van Doorn <nick@super.gd>
Co-authored-by: Noah Silvera <noah@super.gd>
Co-authored-by: Senem Soy <senem@super.gd>
Co-authored-by: Sofia Besenski <sofia@super.gd>
Co-authored-by: Tom Van Manen <tom@super.gd>
---
 .../models/spree/in_memory_order_updater.rb   | 242 +++++++++++
 .../spree/in_memory_order_updater_spec.rb     | 376 ++++++++++++++++++
 2 files changed, 618 insertions(+)
 create mode 100644 core/app/models/spree/in_memory_order_updater.rb
 create mode 100644 core/spec/models/spree/in_memory_order_updater_spec.rb

diff --git a/core/app/models/spree/in_memory_order_updater.rb b/core/app/models/spree/in_memory_order_updater.rb
new file mode 100644
index 00000000000..41ac249a3ae
--- /dev/null
+++ b/core/app/models/spree/in_memory_order_updater.rb
@@ -0,0 +1,242 @@
+# frozen_string_literal: true
+
+module Spree
+  class InMemoryOrderUpdater
+    attr_reader :order
+    delegate :payments, :line_items, :adjustments, :all_adjustments, :shipments, :quantity, to: :order
+
+    def initialize(order)
+      @order = order
+    end
+
+    # This is a multi-purpose method for processing logic related to changes in the Order.
+    # It is meant to be called from various observers so that the Order is aware of changes
+    # that affect totals and other values stored in the Order.
+    #
+    # This method should never do anything to the Order that results in a save call on the
+    # object with callbacks (otherwise you will end up in an infinite recursion as the
+    # associations try to save and then in turn try to call +update!+ again.)
+    def recalculate
+      order.transaction do
+        update_item_count
+        update_shipment_amounts
+        update_totals
+        if order.completed?
+          update_payment_state
+          update_shipments
+          update_shipment_state
+        end
+        Spree::Bus.publish :order_recalculated, order: order
+        persist_totals
+      end
+    end
+    alias_method :update, :recalculate
+    deprecate update: :recalculate, deprecator: Spree.deprecator
+
+    # Updates the +shipment_state+ attribute according to the following logic:
+    #
+    # shipped   when all Shipments are in the "shipped" state
+    # partial   when at least one Shipment has a state of "shipped" and there is another Shipment with a state other than "shipped"
+    #           or there are InventoryUnits associated with the order that have a state of "sold" but are not associated with a Shipment.
+    # ready     when all Shipments are in the "ready" state
+    # backorder when there is backordered inventory associated with an order
+    # pending   when all Shipments are in the "pending" state
+    #
+    # The +shipment_state+ value helps with reporting, etc. since it provides a quick and easy way to locate Orders needing attention.
+    def update_shipment_state
+      log_state_change('shipment') do
+        order.shipment_state = determine_shipment_state
+      end
+
+      order.shipment_state
+    end
+
+    # Updates the +payment_state+ attribute according to the following logic:
+    #
+    # paid          when +payment_total+ is equal to +total+
+    # balance_due   when +payment_total+ is less than +total+
+    # credit_owed   when +payment_total+ is greater than +total+
+    # failed        when most recent payment is in the failed state
+    # void          when the order has been canceled and the payment total is 0
+    #
+    # The +payment_state+ value helps with reporting, etc. since it provides a quick and easy way to locate Orders needing attention.
+    def update_payment_state
+      log_state_change('payment') do
+        order.payment_state = determine_payment_state
+      end
+
+      order.payment_state
+    end
+
+    private
+
+    def determine_payment_state
+      if payments.present? && payments.valid.empty? && order.outstanding_balance != 0
+        'failed'
+      elsif order.state == 'canceled' && order.payment_total.zero?
+        'void'
+      elsif order.outstanding_balance > 0
+        'balance_due'
+      elsif order.outstanding_balance < 0
+        'credit_owed'
+      else
+        # outstanding_balance == 0
+        'paid'
+      end
+    end
+
+    def determine_shipment_state
+      if order.backordered?
+        'backorder'
+      else
+        # get all the shipment states for this order
+        shipment_states = shipments.states
+        if shipment_states.size > 1
+          # multiple shiment states means it's most likely partially shipped
+          'partial'
+        else
+          # will return nil if no shipments are found
+          shipment_states.first
+        end
+      end
+    end
+
+    # This will update and select the best promotion adjustment, update tax
+    # adjustments, update cancellation adjustments, and then update the total
+    # fields (promo_total, included_tax_total, additional_tax_total, and
+    # adjustment_total) on the item.
+    # @return [void]
+    def recalculate_adjustments
+      # 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_taxes
+      update_item_totals
+    end
+
+    # Updates the following Order total values:
+    #
+    # +payment_total+      The total value of all finalized Payments (NOTE: non-finalized Payments are excluded)
+    # +item_total+         The total value of all LineItems
+    # +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
+      update_payment_total
+      update_item_total
+      update_shipment_total
+      update_adjustment_total
+    end
+
+    def update_shipment_amounts
+      shipments.each(&:update_amounts)
+    end
+
+    # give each of the shipments a chance to update themselves
+    def update_shipments
+      shipments.each(&:update_state)
+    end
+
+    def update_payment_total
+      order.payment_total = payments.completed.includes(:refunds).sum { |payment| payment.amount - payment.refunds.sum(:amount) }
+    end
+
+    def update_shipment_total
+      order.shipment_total = shipments.to_a.sum(&:cost)
+      update_order_total
+    end
+
+    def update_order_total
+      order.total = order.item_total + order.shipment_total + order.adjustment_total
+    end
+
+    def update_adjustment_total
+      recalculate_adjustments
+
+      all_items = line_items + shipments
+      order_tax_adjustments = adjustments.select(&:tax?)
+
+      order.adjustment_total = all_items.sum(&:adjustment_total) + 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)
+
+      update_order_total
+    end
+
+    def update_item_count
+      order.item_count = line_items.to_a.sum(&:quantity)
+    end
+
+    def update_item_total
+      order.item_total = line_items.to_a.sum(&:amount)
+      update_order_total
+    end
+
+    def persist_totals
+      order.save!
+    end
+
+    def log_state_change(name)
+      state = "#{name}_state"
+      old_state = order.public_send(state)
+      yield
+      new_state = order.public_send(state)
+      if old_state != new_state
+        order.state_changes.new(
+          previous_state: old_state,
+          next_state:     new_state,
+          name:           name,
+          user_id:        order.user_id
+        )
+      end
+    end
+
+    def update_promotions
+      Spree::Config.promotions.order_adjuster_class.new(order).call
+    end
+
+    def update_taxes
+      Spree::Config.tax_adjuster_class.new(order).adjust!
+
+      [*line_items, *shipments].each do |item|
+        tax_adjustments = item.adjustments.select(&:tax?)
+        # Tax adjustments come in not one but *two* exciting flavours:
+        # Included & additional
+
+        # Included tax adjustments are those which are included in the price.
+        # These ones should not affect the eventual total price.
+        #
+        # Additional tax adjustments are the opposite, affecting the final total.
+        item.included_tax_total   = tax_adjustments.select(&:included?).sum(&:amount)
+        item.additional_tax_total = tax_adjustments.reject(&:included?).sum(&:amount)
+      end
+    end
+
+    def update_cancellations
+    end
+    deprecate :update_cancellations, deprecator: Spree.deprecator
+
+    def update_item_totals
+      [*line_items, *shipments].each do |item|
+        # The cancellation_total isn't persisted anywhere but is included in
+        # the adjustment_total
+        item.adjustment_total = item.adjustments.
+          reject(&:included?).
+          sum(&:amount)
+
+        if item.changed?
+          item.update_columns(
+            promo_total:          item.promo_total,
+            included_tax_total:   item.included_tax_total,
+            additional_tax_total: item.additional_tax_total,
+            adjustment_total:     item.adjustment_total,
+            updated_at:           Time.current,
+          )
+        end
+      end
+    end
+  end
+end
diff --git a/core/spec/models/spree/in_memory_order_updater_spec.rb b/core/spec/models/spree/in_memory_order_updater_spec.rb
new file mode 100644
index 00000000000..98cfb111ee2
--- /dev/null
+++ b/core/spec/models/spree/in_memory_order_updater_spec.rb
@@ -0,0 +1,376 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+module Spree
+  RSpec.describe InMemoryOrderUpdater, type: :model do
+    let!(:store) { create :store }
+    let(:order) { Spree::Order.create }
+    let(:updater) { described_class.new(order) }
+
+    context "order totals" do
+      before do
+        2.times do
+          create(:line_item, order: order, price: 10)
+        end
+      end
+
+      context 'with refund' do
+        it "updates payment totals" do
+          create(:payment_with_refund, order: order, amount: 33.25, refund_amount: 3)
+          updater.recalculate
+          expect(order.payment_total).to eq(30.25)
+        end
+      end
+
+      it "update item total" do
+        expect {
+          updater.recalculate
+        }.to change { order.item_total }.to 20
+      end
+
+      it "update shipment total" do
+        create(:shipment, order: order, cost: 10)
+        expect {
+          updater.recalculate
+        }.to change { order.shipment_total }.to 10
+      end
+
+      context 'with a source-less line item adjustment' do
+        let(:line_item) { create(:line_item, order: order, price: 10) }
+        before do
+          create(:adjustment, source: nil, adjustable: line_item, order: order, amount: -5)
+        end
+
+        it "updates the line item total" do
+          expect { updater.recalculate }.to change { line_item.reload.adjustment_total }.from(0).to(-5)
+        end
+      end
+
+      it "update order adjustments" do
+        create(:adjustment, adjustable: order, order: order, source: nil, amount: 10)
+
+        expect {
+          updater.recalculate
+        }.to change {
+          order.adjustment_total
+        }.from(0).to(10)
+      end
+    end
+
+    describe '#recalculate_adjustments ' do
+      describe 'promotion recalculation' do
+        it "calls the Promotion Adjustments Recalculator" do
+          adjuster = double(:call)
+          expect(Spree::Config.promotions.order_adjuster_class).to receive(:new).and_return(adjuster)
+          expect(adjuster).to receive(:call)
+          order.recalculate
+        end
+      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(:order) do
+          create(
+            :order_with_line_items,
+            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: 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
+            expect {
+              order.recalculate
+            }.to change {
+              line_item.additional_tax_total
+            }.from(1).to(2)
+          end
+        end
+
+        context 'with a custom tax_calculator_class' do
+          let(:custom_calculator_class) { double }
+          let(:custom_calculator_instance) { double }
+
+          before do
+            order # generate this first so we can expect it
+            stub_spree_preferences(tax_calculator_class: custom_calculator_class)
+
+            allow(custom_calculator_class).to receive(:new).and_return(custom_calculator_instance)
+            allow(custom_calculator_instance).to receive(:calculate).and_return(
+              Spree::Tax::OrderTax.new(
+                order_id: order.id,
+                order_taxes: [
+                  Spree::Tax::ItemTax.new(
+                    label: "Delivery Fee",
+                    tax_rate: tax_rate,
+                    amount: 2.60,
+                    included_in_price: false
+                  )
+                ],
+                line_item_taxes: [
+                  Spree::Tax::ItemTax.new(
+                    item_id: line_item.id,
+                    label: "Item Tax",
+                    tax_rate: tax_rate,
+                    amount: 1.40,
+                    included_in_price: false
+                  )
+                ],
+                shipment_taxes: []
+              )
+            )
+          end
+
+          it 'uses the configured class' do
+            order.recalculate
+
+            expect(custom_calculator_class).to have_received(:new).with(order).at_least(:once)
+            expect(custom_calculator_instance).to have_received(:calculate).at_least(:once)
+          end
+
+          it 'updates the aggregate columns' do
+            expect {
+              order.recalculate
+            }.to change { order.reload.additional_tax_total }.to(4.00)
+              .and change { order.reload.adjustment_total }.to(4.00)
+          end
+        end
+      end
+    end
+
+    context "updating shipment state" do
+      before do
+        allow(order).to receive_messages backordered?: false
+      end
+
+      it "is backordered" do
+        allow(order).to receive_messages backordered?: true
+        updater.update_shipment_state
+
+        expect(order.shipment_state).to eq('backorder')
+      end
+
+      it "is nil" do
+        updater.update_shipment_state
+        expect(order.shipment_state).to be_nil
+      end
+
+      ["shipped", "ready", "pending"].each do |state|
+        it "is #{state}" do
+          create(:shipment, order: order, state: state)
+          updater.update_shipment_state
+          expect(order.shipment_state).to eq(state)
+        end
+      end
+
+      it "is partial" do
+        create(:shipment, order: order, state: 'pending')
+        create(:shipment, order: order, state: 'ready')
+        updater.update_shipment_state
+        expect(order.shipment_state).to eq('partial')
+      end
+    end
+
+    context "updating payment state" do
+      let(:order) { build(:order) }
+      let(:updater) { order.recalculator }
+      before { allow(order).to receive(:refund_total).and_return(0) }
+
+      context 'no valid payments with non-zero order total' do
+        it "is failed" do
+          create(:payment, order: order, state: 'invalid')
+          order.total = 1
+          order.payment_total = 0
+
+          updater.update_payment_state
+          expect(order.payment_state).to eq('failed')
+        end
+      end
+
+      context 'invalid payments are present but order total is zero' do
+        it 'is paid' do
+          order.payments << Spree::Payment.new(state: 'invalid')
+          order.total = 0
+          order.payment_total = 0
+
+          expect {
+            updater.update_payment_state
+          }.to change { order.payment_state }.to 'paid'
+        end
+      end
+
+      context "payment total is greater than order total" do
+        it "is credit_owed" do
+          order.payment_total = 2
+          order.total = 1
+
+          expect {
+            updater.update_payment_state
+          }.to change { order.payment_state }.to 'credit_owed'
+        end
+      end
+
+      context "order total is greater than payment total" do
+        it "is balance_due" do
+          order.payment_total = 1
+          order.total = 2
+
+          expect {
+            updater.update_payment_state
+          }.to change { order.payment_state }.to 'balance_due'
+        end
+      end
+
+      context "order total equals payment total" do
+        it "is paid" do
+          order.payment_total = 30
+          order.total = 30
+
+          expect {
+            updater.update_payment_state
+          }.to change { order.payment_state }.to 'paid'
+        end
+      end
+
+      context "order is canceled" do
+        before do
+          order.state = 'canceled'
+        end
+
+        context "and is still unpaid" do
+          it "is void" do
+            order.payment_total = 0
+            order.total = 30
+            expect {
+              updater.update_payment_state
+            }.to change { order.payment_state }.to 'void'
+          end
+        end
+
+        context "and is paid" do
+          it "is credit_owed" do
+            order.payment_total = 30
+            order.total = 30
+            create(:payment, order: order, state: 'completed', amount: 30)
+            expect {
+              updater.update_payment_state
+            }.to change { order.payment_state }.to 'credit_owed'
+          end
+        end
+
+        context "and payment is refunded" do
+          it "is void" do
+            order.payment_total = 0
+            order.total = 30
+            expect {
+              updater.update_payment_state
+            }.to change { order.payment_state }.to 'void'
+          end
+        end
+      end
+    end
+
+    context "completed order" do
+      before { allow(order).to receive_messages completed?: true }
+
+      it "updates payment state" do
+        expect(updater).to receive(:update_payment_state)
+        updater.recalculate
+      end
+
+      it "updates shipment state" do
+        expect(updater).to receive(:update_shipment_state)
+        updater.recalculate
+      end
+
+      context 'with a shipment' do
+        before { create(:shipment, order: order) }
+        let(:shipment){ order.shipments[0] }
+
+        it "updates each shipment" do
+          expect(shipment).to receive(:update_state)
+          updater.recalculate
+        end
+
+        it "updates the shipment amount" do
+          expect(shipment).to receive(:update_amounts)
+          updater.recalculate
+        end
+      end
+    end
+
+    context "incompleted order" do
+      before { allow(order).to receive_messages completed?: false }
+
+      it "doesnt update payment state" do
+        expect(updater).not_to receive(:update_payment_state)
+        updater.recalculate
+      end
+
+      it "doesnt update shipment state" do
+        expect(updater).not_to receive(:update_shipment_state)
+        updater.recalculate
+      end
+
+      it "doesnt update each shipment" do
+        shipment = stub_model(Spree::Shipment)
+        order.shipments = [shipment]
+        allow(order.shipments).to receive_messages(states: [], ready: [], pending: [], shipped: [])
+        allow(updater).to receive(:update_totals) # Otherwise this gets called and causes a scene
+        expect(updater).not_to receive(:update_shipments)
+        updater.recalculate
+      end
+    end
+
+    context "with item with no adjustment and incorrect totals" do
+      let!(:line_item) { create(:line_item, order: order, price: 10) }
+
+      it "updates the totals" do
+        line_item.update!(adjustment_total: 100)
+        expect {
+          order.recalculate
+        }.to change { line_item.reload.adjustment_total }.from(100).to(0)
+      end
+    end
+
+    context "with 'order_recalculated' event subscription" do
+      let(:item) { spy('object') }
+      let(:bus) { Spree::Bus }
+
+      let!(:subscription) do
+        bus.subscribe :order_recalculated do
+          item.do_something
+        end
+      end
+
+      after { bus.unsubscribe subscription }
+
+      it "fires the 'order_recalculated' event" do
+        order.recalculate
+
+        expect(item).to have_received(:do_something)
+      end
+    end
+
+    context "with invalid associated objects" do
+      let(:order) { Spree::Order.create(ship_address: Spree::Address.new) }
+      subject { updater.recalculate }
+
+      it "raises because of the invalid object" do
+        expect { subject }.to raise_error(ActiveRecord::RecordInvalid)
+      end
+    end
+  end
+end

From d3ebe4ee65c0cc150bff0ceff82703a1a10679b2 Mon Sep 17 00:00:00 2001
From: benjamin wil <benjamin@super.gd>
Date: Fri, 11 Oct 2024 13:56:49 -0700
Subject: [PATCH 03/16] Add `persist` flag to `#recalculate`

We want our new in-memory order updater to be able to persist or not
persist changes to the order record.

WORK IN PROGRESS

This is a first step in ensuring we don't need to write to the database
using the order updater. Clearly we have more work to do to ensure this
functions like the existing updater.

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: Jared Norman <jared@super.gd>
Co-authored-by: Nick Van Doorn <nick@super.gd>
Co-authored-by: Noah Silvera <noah@super.gd>
Co-authored-by: Senem Soy <senem@super.gd>
Co-authored-by: Sofia Besenski <sofia@super.gd>
Co-authored-by: Tom Van Manen <tom@super.gd>
---
 .../models/spree/in_memory_order_updater.rb   |  5 +-
 .../spree/in_memory_order_updater_spec.rb     | 46 +++++++++++++++++++
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/core/app/models/spree/in_memory_order_updater.rb b/core/app/models/spree/in_memory_order_updater.rb
index 41ac249a3ae..5f5255a169f 100644
--- a/core/app/models/spree/in_memory_order_updater.rb
+++ b/core/app/models/spree/in_memory_order_updater.rb
@@ -16,7 +16,7 @@ def initialize(order)
     # This method should never do anything to the Order that results in a save call on the
     # object with callbacks (otherwise you will end up in an infinite recursion as the
     # associations try to save and then in turn try to call +update!+ again.)
-    def recalculate
+    def recalculate(persist: true)
       order.transaction do
         update_item_count
         update_shipment_amounts
@@ -27,7 +27,8 @@ def recalculate
           update_shipment_state
         end
         Spree::Bus.publish :order_recalculated, order: order
-        persist_totals
+
+        persist_totals if persist
       end
     end
     alias_method :update, :recalculate
diff --git a/core/spec/models/spree/in_memory_order_updater_spec.rb b/core/spec/models/spree/in_memory_order_updater_spec.rb
index 98cfb111ee2..2e9020825dd 100644
--- a/core/spec/models/spree/in_memory_order_updater_spec.rb
+++ b/core/spec/models/spree/in_memory_order_updater_spec.rb
@@ -8,6 +8,52 @@ module Spree
     let(:order) { Spree::Order.create }
     let(:updater) { described_class.new(order) }
 
+    describe "#recalculate" do
+      subject { updater.recalculate(persist:) }
+
+      let(:new_store) { create(:store) }
+
+      context "when the persist flag is set to 'false'" do
+        let(:persist) { false }
+
+        it "does not persist changes to order" do
+          order.store = new_store
+
+          expect {
+            subject
+          }.not_to make_database_queries(manipulative: true)
+
+          expect(order.store).to eq new_store
+          expect(order.reload.store).not_to eq new_store
+        end
+
+        it "does not persist changes to the item count" do
+          order.line_items << build(:line_item)
+
+          expect {
+            subject
+          }.not_to make_database_queries(manipulative: true)
+
+          expect(order.item_count).to eq 1
+          expect(order.reload.item_count).to eq 0
+        end
+      end
+
+      context "when the persist flag is set to 'true'" do
+        let(:persist) { true }
+
+        it "persists any changes to order" do
+          order.store = new_store
+
+          expect {
+            subject
+          }.to make_database_queries(manipulative: true)
+
+          expect(order.reload.store).to eq new_store
+        end
+      end
+    end
+
     context "order totals" do
       before do
         2.times do

From eaf2c02a2f0d19436ecda11b4027588e88444c95 Mon Sep 17 00:00:00 2001
From: Noah Silvera <noah.a.silvera@gmail.com>
Date: Fri, 25 Oct 2024 16:47:25 -0400
Subject: [PATCH 04/16] Add describe block to Shipment#update_amounts test

---
 core/spec/models/spree/shipment_spec.rb | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/core/spec/models/spree/shipment_spec.rb b/core/spec/models/spree/shipment_spec.rb
index 384413166d6..6578a288967 100644
--- a/core/spec/models/spree/shipment_spec.rb
+++ b/core/spec/models/spree/shipment_spec.rb
@@ -507,14 +507,23 @@
     end
   end
 
-  context "updates cost when selected shipping rate is present" do
-    let(:shipment) { create(:shipment) }
-    before { shipment.selected_shipping_rate.update!(cost: 5) }
+  describe "#update_amounts" do
+    let(:shipment) { create(:shipment, cost: 3) }
 
-    it "updates shipment totals" do
-      expect {
-        shipment.update_amounts
-      }.to change { shipment.cost }.to(5)
+    context 'when the selected shipping rate cost is different than the current shipment cost' do
+      before { shipment.selected_shipping_rate.update!(cost: 5) }
+
+      it "updates the shipments cost" do
+        expect {
+          shipment.update_amounts
+        }.to change { shipment.reload.cost }.to(5)
+      end
+
+      it 'changes the updated_at column' do
+        expect {
+          shipment.update_amounts
+        }.to change { shipment.reload.updated_at }
+      end
     end
   end
 

From 870c37a6daab9d8061aeb259a118996541558885 Mon Sep 17 00:00:00 2001
From: Noah Silvera <noah.a.silvera@gmail.com>
Date: Fri, 25 Oct 2024 17:13:33 -0400
Subject: [PATCH 05/16] Conditionally persist Shipment#update_amounts changes

This is in service of supporting the InMemoryOrderUpdater's goal to not do database writes.
---
 core/app/models/spree/shipment.rb       |  4 +--
 core/spec/models/spree/shipment_spec.rb | 33 +++++++++++++++++++------
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/core/app/models/spree/shipment.rb b/core/app/models/spree/shipment.rb
index 6808518448d..310d74fb327 100644
--- a/core/app/models/spree/shipment.rb
+++ b/core/app/models/spree/shipment.rb
@@ -258,10 +258,10 @@ def tracking_url
       @tracking_url ||= shipping_method.build_tracking_url(tracking)
     end
 
-    def update_amounts
+    def update_amounts(persist: true)
       if selected_shipping_rate
         self.cost = selected_shipping_rate.cost
-        if changed?
+        if changed? && persist
           update_columns(
             cost:,
             updated_at: Time.current
diff --git a/core/spec/models/spree/shipment_spec.rb b/core/spec/models/spree/shipment_spec.rb
index 6578a288967..cbbefe292b8 100644
--- a/core/spec/models/spree/shipment_spec.rb
+++ b/core/spec/models/spree/shipment_spec.rb
@@ -508,22 +508,41 @@
   end
 
   describe "#update_amounts" do
-    let(:shipment) { create(:shipment, cost: 3) }
+    subject { shipment.update_amounts(persist: persist) }
+
+    let(:persist) { true }
+    let(:shipment) { create(:shipment, cost: 1) }
 
     context 'when the selected shipping rate cost is different than the current shipment cost' do
-      before { shipment.selected_shipping_rate.update!(cost: 5) }
+      before { shipment.selected_shipping_rate.update!(cost: 999) }
 
-      it "updates the shipments cost" do
+      it "changes and persists the shipments cost" do
         expect {
-          shipment.update_amounts
-        }.to change { shipment.reload.cost }.to(5)
+          subject
+        }.to change { shipment.reload.cost }.to(999)
       end
 
-      it 'changes the updated_at column' do
+      it 'changes and persists the updated_at column' do
         expect {
-          shipment.update_amounts
+          subject
         }.to change { shipment.reload.updated_at }
       end
+
+      context 'when `persist: false` is passed' do
+        let(:persist) { false }
+
+        it 'does not perform any database writes' do
+          expect {
+            subject
+          }.not_to make_database_queries(manipulative: true)
+        end
+
+        it "changes but does not persist the shipments cost" do
+          subject
+          expect(shipment.cost).to eq 999
+          expect(shipment.reload.cost).to eq 1
+        end
+      end
     end
   end
 

From 203ed706b8af3e42a8818ecdabd2b9c9ff0d8ce7 Mon Sep 17 00:00:00 2001
From: Noah Silvera <noah.a.silvera@gmail.com>
Date: Fri, 25 Oct 2024 17:18:06 -0400
Subject: [PATCH 06/16] Preventing InMemoryOrderUpdater#update_shipment_amounts
 from making database 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>
---
 .../models/spree/in_memory_order_updater.rb   | 24 +++++++++----------
 .../spree/in_memory_order_updater_spec.rb     | 24 +++++++++++++++++++
 2 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/core/app/models/spree/in_memory_order_updater.rb b/core/app/models/spree/in_memory_order_updater.rb
index 5f5255a169f..d0566b886de 100644
--- a/core/app/models/spree/in_memory_order_updater.rb
+++ b/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,7 +107,7 @@ 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
@@ -115,7 +115,7 @@ def recalculate_adjustments
       # http://www.boe.ca.gov/formspubs/pub113/
       update_promotions
       update_taxes
-      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
       order_tax_adjustments = adjustments.select(&:tax?)
@@ -220,7 +220,7 @@ 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|
         # The cancellation_total isn't persisted anywhere but is included in
         # the adjustment_total
@@ -228,7 +228,7 @@ def update_item_totals
           reject(&:included?).
           sum(&:amount)
 
-        if item.changed?
+        if persist && item.changed?
           item.update_columns(
             promo_total:          item.promo_total,
             included_tax_total:   item.included_tax_total,
diff --git a/core/spec/models/spree/in_memory_order_updater_spec.rb b/core/spec/models/spree/in_memory_order_updater_spec.rb
index 2e9020825dd..0bdb503a9bd 100644
--- a/core/spec/models/spree/in_memory_order_updater_spec.rb
+++ b/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

From 8040224ca9010ce8d1b217800d473c4b00d0970e Mon Sep 17 00:00:00 2001
From: Chris Todorov <chris@super.gd>
Date: Fri, 8 Nov 2024 13:54:54 -0800
Subject: [PATCH 07/16] Rename method that recalculates shipment state

Update implies that we are persisting the change in Rails, which this
method does not do.

Co-authored-by: Adam Mueller <adam@super.gd>
Co-authored-by: Senem Soy <senem@super.gd>
Co-authored-by: Andrew Stewart <andrew@super.gd>
Co-authored-by: Kendra Riga <kendra@super.gd>
Co-authored-by: Sofia Besenski <sofia@super.gd>
Co-authored-by: Benjamin Willems <benjamin@super.gd>
---
 core/app/models/spree/in_memory_order_updater.rb     |  8 +++++---
 .../models/spree/in_memory_order_updater_spec.rb     | 12 ++++++------
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/core/app/models/spree/in_memory_order_updater.rb b/core/app/models/spree/in_memory_order_updater.rb
index d0566b886de..00d98ef1f67 100644
--- a/core/app/models/spree/in_memory_order_updater.rb
+++ b/core/app/models/spree/in_memory_order_updater.rb
@@ -24,7 +24,7 @@ def recalculate(persist: true)
         if order.completed?
           update_payment_state
           update_shipments
-          update_shipment_state
+          recalculate_shipment_state
         end
         Spree::Bus.publish :order_recalculated, order: order
 
@@ -34,7 +34,7 @@ def recalculate(persist: true)
     alias_method :update, :recalculate
     deprecate update: :recalculate, deprecator: Spree.deprecator
 
-    # Updates the +shipment_state+ attribute according to the following logic:
+    # Recalculates the +shipment_state+ attribute according to the following logic:
     #
     # shipped   when all Shipments are in the "shipped" state
     # partial   when at least one Shipment has a state of "shipped" and there is another Shipment with a state other than "shipped"
@@ -44,13 +44,15 @@ def recalculate(persist: true)
     # pending   when all Shipments are in the "pending" state
     #
     # The +shipment_state+ value helps with reporting, etc. since it provides a quick and easy way to locate Orders needing attention.
-    def update_shipment_state
+    def recalculate_shipment_state
       log_state_change('shipment') do
         order.shipment_state = determine_shipment_state
       end
 
       order.shipment_state
     end
+    alias_method :update_shipment_state, :recalculate_shipment_state
+    deprecate update_shipment_state: :recalculate_shipment_state, deprecator: Spree.deprecator
 
     # Updates the +payment_state+ attribute according to the following logic:
     #
diff --git a/core/spec/models/spree/in_memory_order_updater_spec.rb b/core/spec/models/spree/in_memory_order_updater_spec.rb
index 0bdb503a9bd..00d2e21dfd2 100644
--- a/core/spec/models/spree/in_memory_order_updater_spec.rb
+++ b/core/spec/models/spree/in_memory_order_updater_spec.rb
@@ -227,20 +227,20 @@ module Spree
 
       it "is backordered" do
         allow(order).to receive_messages backordered?: true
-        updater.update_shipment_state
+        updater.recalculate_shipment_state
 
         expect(order.shipment_state).to eq('backorder')
       end
 
       it "is nil" do
-        updater.update_shipment_state
+        updater.recalculate_shipment_state
         expect(order.shipment_state).to be_nil
       end
 
       ["shipped", "ready", "pending"].each do |state|
         it "is #{state}" do
           create(:shipment, order: order, state: state)
-          updater.update_shipment_state
+          updater.recalculate_shipment_state
           expect(order.shipment_state).to eq(state)
         end
       end
@@ -248,7 +248,7 @@ module Spree
       it "is partial" do
         create(:shipment, order: order, state: 'pending')
         create(:shipment, order: order, state: 'ready')
-        updater.update_shipment_state
+        updater.recalculate_shipment_state
         expect(order.shipment_state).to eq('partial')
       end
     end
@@ -361,7 +361,7 @@ module Spree
       end
 
       it "updates shipment state" do
-        expect(updater).to receive(:update_shipment_state)
+        expect(updater).to receive(:recalculate_shipment_state)
         updater.recalculate
       end
 
@@ -390,7 +390,7 @@ module Spree
       end
 
       it "doesnt update shipment state" do
-        expect(updater).not_to receive(:update_shipment_state)
+        expect(updater).not_to receive(:recalculate_shipment_state)
         updater.recalculate
       end
 

From 7ce587453c74c6c0cfbe25c49609e1c96b1c690b Mon Sep 17 00:00:00 2001
From: Chris Todorov <chris@super.gd>
Date: Fri, 8 Nov 2024 14:08:41 -0800
Subject: [PATCH 08/16] Rename method that recalculates payment state

Update implies that we are persisting the change in Rails, which this
method does not do.

Co-authored-by: Adam Mueller <adam@super.gd>
Co-authored-by: Andrew Stewart <andrew@super.gd>
Co-authored-by: Benjamin Willems <benjamin@super.gd>
Co-authored-by: Senem Soy <senem@super.gd>
Co-authored-by: Sofia Besenski <sofia@super.gd>
Co-authored-by: Kendra Riga <kendra@super.gd>
---
 .../models/spree/in_memory_order_updater.rb   |  8 ++++---
 .../spree/in_memory_order_updater_spec.rb     | 21 +++++++++----------
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/core/app/models/spree/in_memory_order_updater.rb b/core/app/models/spree/in_memory_order_updater.rb
index 00d98ef1f67..e0770ba59f5 100644
--- a/core/app/models/spree/in_memory_order_updater.rb
+++ b/core/app/models/spree/in_memory_order_updater.rb
@@ -22,7 +22,7 @@ def recalculate(persist: true)
         update_shipment_amounts(persist:)
         update_totals(persist:)
         if order.completed?
-          update_payment_state
+          recalculate_payment_state
           update_shipments
           recalculate_shipment_state
         end
@@ -54,7 +54,7 @@ def recalculate_shipment_state
     alias_method :update_shipment_state, :recalculate_shipment_state
     deprecate update_shipment_state: :recalculate_shipment_state, deprecator: Spree.deprecator
 
-    # Updates the +payment_state+ attribute according to the following logic:
+    # Recalculates the +payment_state+ attribute according to the following logic:
     #
     # paid          when +payment_total+ is equal to +total+
     # balance_due   when +payment_total+ is less than +total+
@@ -63,13 +63,15 @@ def recalculate_shipment_state
     # void          when the order has been canceled and the payment total is 0
     #
     # The +payment_state+ value helps with reporting, etc. since it provides a quick and easy way to locate Orders needing attention.
-    def update_payment_state
+    def recalculate_payment_state
       log_state_change('payment') do
         order.payment_state = determine_payment_state
       end
 
       order.payment_state
     end
+    alias_method :update_payment_state, :recalculate_shipment_state
+    deprecate update_payment_state: :recalculate_shipment_state, deprecator: Spree.deprecator
 
     private
 
diff --git a/core/spec/models/spree/in_memory_order_updater_spec.rb b/core/spec/models/spree/in_memory_order_updater_spec.rb
index 00d2e21dfd2..ebfb8aef1c4 100644
--- a/core/spec/models/spree/in_memory_order_updater_spec.rb
+++ b/core/spec/models/spree/in_memory_order_updater_spec.rb
@@ -255,7 +255,6 @@ module Spree
 
     context "updating payment state" do
       let(:order) { build(:order) }
-      let(:updater) { order.recalculator }
       before { allow(order).to receive(:refund_total).and_return(0) }
 
       context 'no valid payments with non-zero order total' do
@@ -264,7 +263,7 @@ module Spree
           order.total = 1
           order.payment_total = 0
 
-          updater.update_payment_state
+          updater.recalculate_payment_state
           expect(order.payment_state).to eq('failed')
         end
       end
@@ -276,7 +275,7 @@ module Spree
           order.payment_total = 0
 
           expect {
-            updater.update_payment_state
+            updater.recalculate_payment_state
           }.to change { order.payment_state }.to 'paid'
         end
       end
@@ -287,7 +286,7 @@ module Spree
           order.total = 1
 
           expect {
-            updater.update_payment_state
+            updater.recalculate_payment_state
           }.to change { order.payment_state }.to 'credit_owed'
         end
       end
@@ -298,7 +297,7 @@ module Spree
           order.total = 2
 
           expect {
-            updater.update_payment_state
+            updater.recalculate_payment_state
           }.to change { order.payment_state }.to 'balance_due'
         end
       end
@@ -309,7 +308,7 @@ module Spree
           order.total = 30
 
           expect {
-            updater.update_payment_state
+            updater.recalculate_payment_state
           }.to change { order.payment_state }.to 'paid'
         end
       end
@@ -324,7 +323,7 @@ module Spree
             order.payment_total = 0
             order.total = 30
             expect {
-              updater.update_payment_state
+              updater.recalculate_payment_state
             }.to change { order.payment_state }.to 'void'
           end
         end
@@ -335,7 +334,7 @@ module Spree
             order.total = 30
             create(:payment, order: order, state: 'completed', amount: 30)
             expect {
-              updater.update_payment_state
+              updater.recalculate_payment_state
             }.to change { order.payment_state }.to 'credit_owed'
           end
         end
@@ -345,7 +344,7 @@ module Spree
             order.payment_total = 0
             order.total = 30
             expect {
-              updater.update_payment_state
+              updater.recalculate_payment_state
             }.to change { order.payment_state }.to 'void'
           end
         end
@@ -356,7 +355,7 @@ module Spree
       before { allow(order).to receive_messages completed?: true }
 
       it "updates payment state" do
-        expect(updater).to receive(:update_payment_state)
+        expect(updater).to receive(:recalculate_payment_state)
         updater.recalculate
       end
 
@@ -385,7 +384,7 @@ module Spree
       before { allow(order).to receive_messages completed?: false }
 
       it "doesnt update payment state" do
-        expect(updater).not_to receive(:update_payment_state)
+        expect(updater).not_to receive(:recalculate_payment_state)
         updater.recalculate
       end
 

From 0e26fc02930646d822474e79ae20f7d7fb0acb3f Mon Sep 17 00:00:00 2001
From: Chris Todorov <chris@super.gd>
Date: Fri, 8 Nov 2024 14:25:33 -0800
Subject: [PATCH 09/16] Add TODO so we know what to do

---
 TODO.md | 10 ++++++++++
 1 file changed, 10 insertions(+)
 create mode 100644 TODO.md

diff --git a/TODO.md b/TODO.md
new file mode 100644
index 00000000000..7bf3c16ae02
--- /dev/null
+++ b/TODO.md
@@ -0,0 +1,10 @@
+In-Memory Order Updater TODO
+===
+
+- [ ] Finish renaming methods that don't persist ever
+- [ ] Consider Sofia's recommendation to break this class into POROs to simplify testing
+- [ ] Address FIXME on renaming `recalculate_adjustments`?
+- [ ] Add test coverage for `update_item_total` when line item totals change
+- [ ] Test coverage to ensure state changes aren't persisted (if someone changes current implementation)
+- [ ] Handle persistence in `update_promotions`
+- [ ] Handle persistence in `update_taxes`

From 74d2d3c11e6aa9e42b88b2bb9cbd1a5e2db494da Mon Sep 17 00:00:00 2001
From: Alistair Norman <alistair@super.gd>
Date: Fri, 22 Nov 2024 13:20:11 -0800
Subject: [PATCH 10/16] Rename update_ private methods

These methods don't persist so it's more accurate to say that they
recalculate the total instead of saying that they update it.

Co-Authored-By: Adam Mueller <adam@super.gd>
Co-Authored-By: Benjamin Willems <benjamin@super.gd>
Co-Authored-By: Andrew Stewart <andrew@super.gd>
Co-Authored-By: Harmony Bouvier <harmony@super.gd>
Co-Authored-By: Jared Norman <jared@super.gd>
Co-Authored-By: Kendra Riga <kendar@super.gd>
Co-Authored-By: Sofia Besenski <sofia@super.gd>
Co-Authored-By: Chris Todorov <chris@super.gd>
Co-Authored-By: Tom Van Manen <tom@super.gd>
Co-Authored-By: Noah Silvera <noah@super.gd>
---
 .../models/spree/in_memory_order_updater.rb   | 24 +++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/core/app/models/spree/in_memory_order_updater.rb b/core/app/models/spree/in_memory_order_updater.rb
index e0770ba59f5..fd8e761f285 100644
--- a/core/app/models/spree/in_memory_order_updater.rb
+++ b/core/app/models/spree/in_memory_order_updater.rb
@@ -18,7 +18,7 @@ def initialize(order)
     # associations try to save and then in turn try to call +update!+ again.)
     def recalculate(persist: true)
       order.transaction do
-        update_item_count
+        recalculate_item_count
         update_shipment_amounts(persist:)
         update_totals(persist:)
         if order.completed?
@@ -130,9 +130,9 @@ def recalculate_adjustments(persist:)
     # +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(persist:)
-      update_payment_total
-      update_item_total
-      update_shipment_total
+      recalculate_payment_total
+      recalculate_item_total
+      recalculate_shipment_total
       update_adjustment_total(persist:)
     end
 
@@ -145,16 +145,16 @@ def update_shipments
       shipments.each(&:update_state)
     end
 
-    def update_payment_total
+    def recalculate_payment_total
       order.payment_total = payments.completed.includes(:refunds).sum { |payment| payment.amount - payment.refunds.sum(:amount) }
     end
 
-    def update_shipment_total
+    def recalculate_shipment_total
       order.shipment_total = shipments.to_a.sum(&:cost)
-      update_order_total
+      recalculate_order_total
     end
 
-    def update_order_total
+    def recalculate_order_total
       order.total = order.item_total + order.shipment_total + order.adjustment_total
     end
 
@@ -168,16 +168,16 @@ def update_adjustment_total(persist:)
       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)
 
-      update_order_total
+      recalculate_order_total
     end
 
-    def update_item_count
+    def recalculate_item_count
       order.item_count = line_items.to_a.sum(&:quantity)
     end
 
-    def update_item_total
+    def recalculate_item_total
       order.item_total = line_items.to_a.sum(&:amount)
-      update_order_total
+      recalculate_order_total
     end
 
     def persist_totals

From 54e957d413e536a11f02800310b72af6cd38b207 Mon Sep 17 00:00:00 2001
From: Alistair Norman <alistair@super.gd>
Date: Fri, 22 Nov 2024 13:33:05 -0800
Subject: [PATCH 11/16] Rename recalculate_adjustments

We want all the methods that might persist data to be called update_
instead of recalculate to be clear that they hit the database.

Co-Authored-By: Adam Mueller <adam@super.gd>
Co-Authored-By: Benjamin Willems <benjamin@super.gd>
Co-Authored-By: Andrew Stewart <andrew@super.gd>
Co-Authored-By: Harmony Bouvier <harmony@super.gd>
Co-Authored-By: Jared Norman <jared@super.gd>
Co-Authored-By: Kendra Riga <kendar@super.gd>
Co-Authored-By: Sofia Besenski <sofia@super.gd>
Co-Authored-By: Chris Todorov <chris@super.gd>
Co-Authored-By: Tom Van Manen <tom@super.gd>
Co-Authored-By: Noah Silvera <noah@super.gd>
---
 core/app/models/spree/in_memory_order_updater.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/core/app/models/spree/in_memory_order_updater.rb b/core/app/models/spree/in_memory_order_updater.rb
index fd8e761f285..cf35a2ce80d 100644
--- a/core/app/models/spree/in_memory_order_updater.rb
+++ b/core/app/models/spree/in_memory_order_updater.rb
@@ -111,7 +111,7 @@ def determine_shipment_state
     # fields (promo_total, included_tax_total, additional_tax_total, and
     # adjustment_total) on the item.
     # @return [void]
-    def recalculate_adjustments(persist:)
+    def update_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
@@ -159,7 +159,7 @@ def recalculate_order_total
     end
 
     def update_adjustment_total(persist:)
-      recalculate_adjustments(persist:)
+      update_adjustments(persist:)
 
       all_items = line_items + shipments
       order_tax_adjustments = adjustments.select(&:tax?)

From b74eaecb79d6cf6ee871be5a93b54b5ae3b71081 Mon Sep 17 00:00:00 2001
From: Alistair Norman <alistair@super.gd>
Date: Fri, 22 Nov 2024 13:35:07 -0800
Subject: [PATCH 12/16] Update TODO

---
 TODO.md | 2 --
 1 file changed, 2 deletions(-)

diff --git a/TODO.md b/TODO.md
index 7bf3c16ae02..34e9d98f558 100644
--- a/TODO.md
+++ b/TODO.md
@@ -1,9 +1,7 @@
 In-Memory Order Updater TODO
 ===
 
-- [ ] Finish renaming methods that don't persist ever
 - [ ] Consider Sofia's recommendation to break this class into POROs to simplify testing
-- [ ] Address FIXME on renaming `recalculate_adjustments`?
 - [ ] Add test coverage for `update_item_total` when line item totals change
 - [ ] Test coverage to ensure state changes aren't persisted (if someone changes current implementation)
 - [ ] Handle persistence in `update_promotions`

From 4da2a3102743053207e97ada48d8455313ded313 Mon Sep 17 00:00:00 2001
From: Alistair Norman <alistair@super.gd>
Date: Fri, 22 Nov 2024 13:40:16 -0800
Subject: [PATCH 13/16] Remove describe block for private method

This is calling the recalculate method not update_adjustments.

Co-Authored-By: Adam Mueller <adam@super.gd>
Co-Authored-By: Benjamin Willems <benjamin@super.gd>
Co-Authored-By: Andrew Stewart <andrew@super.gd>
Co-Authored-By: Harmony Bouvier <harmony@super.gd>
Co-Authored-By: Jared Norman <jared@super.gd>
Co-Authored-By: Kendra Riga <kendar@super.gd>
Co-Authored-By: Sofia Besenski <sofia@super.gd>
Co-Authored-By: Chris Todorov <chris@super.gd>
Co-Authored-By: Tom Van Manen <tom@super.gd>
Co-Authored-By: Noah Silvera <noah@super.gd>
---
 .../spree/in_memory_order_updater_spec.rb     | 150 +++++++++---------
 1 file changed, 74 insertions(+), 76 deletions(-)

diff --git a/core/spec/models/spree/in_memory_order_updater_spec.rb b/core/spec/models/spree/in_memory_order_updater_spec.rb
index ebfb8aef1c4..91ce3451bbb 100644
--- a/core/spec/models/spree/in_memory_order_updater_spec.rb
+++ b/core/spec/models/spree/in_memory_order_updater_spec.rb
@@ -128,94 +128,92 @@ module Spree
       end
     end
 
-    describe '#recalculate_adjustments ' do
-      describe 'promotion recalculation' do
-        it "calls the Promotion Adjustments Recalculator" do
-          adjuster = double(:call)
-          expect(Spree::Config.promotions.order_adjuster_class).to receive(:new).and_return(adjuster)
-          expect(adjuster).to receive(:call)
-          order.recalculate
-        end
+    describe 'promotion recalculation' do
+      it "calls the Promotion Adjustments Recalculator" do
+        adjuster = double(:call)
+        expect(Spree::Config.promotions.order_adjuster_class).to receive(:new).and_return(adjuster)
+        expect(adjuster).to receive(:call)
+        order.recalculate
       end
+    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(:order) do
-          create(
-            :order_with_line_items,
-            line_items_attributes: [{ price: 10, variant: variant }],
-            ship_address: ship_address,
-          )
-        end
-        let(:line_item) { order.line_items[0] }
+    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(:order) do
+        create(
+          :order_with_line_items,
+          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: tax_category) }
-        let(:tax_category) { create(:tax_category) }
+      let(:variant) { create(:variant, tax_category: tax_category) }
+      let(:tax_category) { create(:tax_category) }
 
-        context 'when the item quantity has changed' do
-          before do
-            line_item.update!(quantity: 2)
-          end
+      context 'when the item quantity has changed' do
+        before do
+          line_item.update!(quantity: 2)
+        end
 
-          it 'updates the promotion amount' do
-            expect {
-              order.recalculate
-            }.to change {
-              line_item.additional_tax_total
-            }.from(1).to(2)
-          end
+        it 'updates the promotion amount' do
+          expect {
+            order.recalculate
+          }.to change {
+            line_item.additional_tax_total
+          }.from(1).to(2)
         end
+      end
 
-        context 'with a custom tax_calculator_class' do
-          let(:custom_calculator_class) { double }
-          let(:custom_calculator_instance) { double }
+      context 'with a custom tax_calculator_class' do
+        let(:custom_calculator_class) { double }
+        let(:custom_calculator_instance) { double }
 
-          before do
-            order # generate this first so we can expect it
-            stub_spree_preferences(tax_calculator_class: custom_calculator_class)
-
-            allow(custom_calculator_class).to receive(:new).and_return(custom_calculator_instance)
-            allow(custom_calculator_instance).to receive(:calculate).and_return(
-              Spree::Tax::OrderTax.new(
-                order_id: order.id,
-                order_taxes: [
-                  Spree::Tax::ItemTax.new(
-                    label: "Delivery Fee",
-                    tax_rate: tax_rate,
-                    amount: 2.60,
-                    included_in_price: false
-                  )
-                ],
-                line_item_taxes: [
-                  Spree::Tax::ItemTax.new(
-                    item_id: line_item.id,
-                    label: "Item Tax",
-                    tax_rate: tax_rate,
-                    amount: 1.40,
-                    included_in_price: false
-                  )
-                ],
-                shipment_taxes: []
-              )
+        before do
+          order # generate this first so we can expect it
+          stub_spree_preferences(tax_calculator_class: custom_calculator_class)
+
+          allow(custom_calculator_class).to receive(:new).and_return(custom_calculator_instance)
+          allow(custom_calculator_instance).to receive(:calculate).and_return(
+            Spree::Tax::OrderTax.new(
+              order_id: order.id,
+              order_taxes: [
+                Spree::Tax::ItemTax.new(
+                  label: "Delivery Fee",
+                  tax_rate: tax_rate,
+                  amount: 2.60,
+                  included_in_price: false
+                )
+              ],
+              line_item_taxes: [
+                Spree::Tax::ItemTax.new(
+                  item_id: line_item.id,
+                  label: "Item Tax",
+                  tax_rate: tax_rate,
+                  amount: 1.40,
+                  included_in_price: false
+                )
+              ],
+              shipment_taxes: []
             )
-          end
+          )
+        end
 
-          it 'uses the configured class' do
-            order.recalculate
+        it 'uses the configured class' do
+          order.recalculate
 
-            expect(custom_calculator_class).to have_received(:new).with(order).at_least(:once)
-            expect(custom_calculator_instance).to have_received(:calculate).at_least(:once)
-          end
+          expect(custom_calculator_class).to have_received(:new).with(order).at_least(:once)
+          expect(custom_calculator_instance).to have_received(:calculate).at_least(:once)
+        end
 
-          it 'updates the aggregate columns' do
-            expect {
-              order.recalculate
-            }.to change { order.reload.additional_tax_total }.to(4.00)
-              .and change { order.reload.adjustment_total }.to(4.00)
-          end
+        it 'updates the aggregate columns' do
+          expect {
+            order.recalculate
+          }.to change { order.reload.additional_tax_total }.to(4.00)
+            .and change { order.reload.adjustment_total }.to(4.00)
         end
       end
     end

From 5182911293fe359f1d08393af0fcacc833529687 Mon Sep 17 00:00:00 2001
From: Alistair Norman <alistair@super.gd>
Date: Fri, 22 Nov 2024 13:51:24 -0800
Subject: [PATCH 14/16] Reorder private methods

This puts all the update and recalculate methods together.

Co-Authored-By: Adam Mueller <adam@super.gd>
Co-Authored-By: Benjamin Willems <benjamin@super.gd>
Co-Authored-By: Andrew Stewart <andrew@super.gd>
Co-Authored-By: Harmony Bouvier <harmony@super.gd>
Co-Authored-By: Jared Norman <jared@super.gd>
Co-Authored-By: Kendra Riga <kendar@super.gd>
Co-Authored-By: Sofia Besenski <sofia@super.gd>
Co-Authored-By: Chris Todorov <chris@super.gd>
Co-Authored-By: Tom Van Manen <tom@super.gd>
Co-Authored-By: Noah Silvera <noah@super.gd>
---
 .../models/spree/in_memory_order_updater.rb   | 92 +++++++++----------
 1 file changed, 46 insertions(+), 46 deletions(-)

diff --git a/core/app/models/spree/in_memory_order_updater.rb b/core/app/models/spree/in_memory_order_updater.rb
index cf35a2ce80d..ed2bf81430c 100644
--- a/core/app/models/spree/in_memory_order_updater.rb
+++ b/core/app/models/spree/in_memory_order_updater.rb
@@ -140,24 +140,6 @@ def update_shipment_amounts(persist:)
       shipments.each { _1.update_amounts(persist:) }
     end
 
-    # give each of the shipments a chance to update themselves
-    def update_shipments
-      shipments.each(&:update_state)
-    end
-
-    def recalculate_payment_total
-      order.payment_total = payments.completed.includes(:refunds).sum { |payment| payment.amount - payment.refunds.sum(:amount) }
-    end
-
-    def recalculate_shipment_total
-      order.shipment_total = shipments.to_a.sum(&:cost)
-      recalculate_order_total
-    end
-
-    def recalculate_order_total
-      order.total = order.item_total + order.shipment_total + order.adjustment_total
-    end
-
     def update_adjustment_total(persist:)
       update_adjustments(persist:)
 
@@ -171,34 +153,6 @@ def update_adjustment_total(persist:)
       recalculate_order_total
     end
 
-    def recalculate_item_count
-      order.item_count = line_items.to_a.sum(&:quantity)
-    end
-
-    def recalculate_item_total
-      order.item_total = line_items.to_a.sum(&:amount)
-      recalculate_order_total
-    end
-
-    def persist_totals
-      order.save!
-    end
-
-    def log_state_change(name)
-      state = "#{name}_state"
-      old_state = order.public_send(state)
-      yield
-      new_state = order.public_send(state)
-      if old_state != new_state
-        order.state_changes.new(
-          previous_state: old_state,
-          next_state:     new_state,
-          name:           name,
-          user_id:        order.user_id
-        )
-      end
-    end
-
     def update_promotions
       Spree::Config.promotions.order_adjuster_class.new(order).call
     end
@@ -243,5 +197,51 @@ def update_item_totals(persist:)
         end
       end
     end
+
+    # give each of the shipments a chance to update themselves
+    def update_shipments
+      shipments.each(&:update_state)
+    end
+
+    def recalculate_payment_total
+      order.payment_total = payments.completed.includes(:refunds).sum { |payment| payment.amount - payment.refunds.sum(:amount) }
+    end
+
+    def recalculate_shipment_total
+      order.shipment_total = shipments.to_a.sum(&:cost)
+      recalculate_order_total
+    end
+
+    def recalculate_order_total
+      order.total = order.item_total + order.shipment_total + order.adjustment_total
+    end
+
+    def recalculate_item_count
+      order.item_count = line_items.to_a.sum(&:quantity)
+    end
+
+    def recalculate_item_total
+      order.item_total = line_items.to_a.sum(&:amount)
+      recalculate_order_total
+    end
+
+    def persist_totals
+      order.save!
+    end
+
+    def log_state_change(name)
+      state = "#{name}_state"
+      old_state = order.public_send(state)
+      yield
+      new_state = order.public_send(state)
+      if old_state != new_state
+        order.state_changes.new(
+          previous_state: old_state,
+          next_state:     new_state,
+          name:           name,
+          user_id:        order.user_id
+        )
+      end
+    end
   end
 end

From ee80a15d2acfb0f22d8af637f132c94fdbb4f85a Mon Sep 17 00:00:00 2001
From: Alistair Norman <alistair@super.gd>
Date: Fri, 22 Nov 2024 14:22:01 -0800
Subject: [PATCH 15/16] Pull out the item total updater

We want to start breaking out some of the complex logic of the in memory
updater into smaller more focused classes.

Co-Authored-By: Adam Mueller <adam@super.gd>
Co-Authored-By: Benjamin Willems <benjamin@super.gd>
Co-Authored-By: Andrew Stewart <andrew@super.gd>
Co-Authored-By: Harmony Bouvier <harmony@super.gd>
Co-Authored-By: Jared Norman <jared@super.gd>
Co-Authored-By: Kendra Riga <kendar@super.gd>
Co-Authored-By: Sofia Besenski <sofia@super.gd>
Co-Authored-By: Chris Todorov <chris@super.gd>
Co-Authored-By: Tom Van Manen <tom@super.gd>
Co-Authored-By: Noah Silvera <noah@super.gd>
---
 .../models/spree/in_memory_order_updater.rb    |  6 +-----
 core/app/models/spree/item_total_updater.rb    |  9 +++++++++
 .../models/spree/item_total_updater_spec.rb    | 18 ++++++++++++++++++
 3 files changed, 28 insertions(+), 5 deletions(-)
 create mode 100644 core/app/models/spree/item_total_updater.rb
 create mode 100644 core/spec/models/spree/item_total_updater_spec.rb

diff --git a/core/app/models/spree/in_memory_order_updater.rb b/core/app/models/spree/in_memory_order_updater.rb
index ed2bf81430c..fc59a8a29f1 100644
--- a/core/app/models/spree/in_memory_order_updater.rb
+++ b/core/app/models/spree/in_memory_order_updater.rb
@@ -180,11 +180,7 @@ def update_cancellations
 
     def update_item_totals(persist:)
       [*line_items, *shipments].each do |item|
-        # The cancellation_total isn't persisted anywhere but is included in
-        # the adjustment_total
-        item.adjustment_total = item.adjustments.
-          reject(&:included?).
-          sum(&:amount)
+        Spree::ItemTotalUpdater.recalculate(item)
 
         if persist && item.changed?
           item.update_columns(
diff --git a/core/app/models/spree/item_total_updater.rb b/core/app/models/spree/item_total_updater.rb
new file mode 100644
index 00000000000..c062f7a1ac9
--- /dev/null
+++ b/core/app/models/spree/item_total_updater.rb
@@ -0,0 +1,9 @@
+class Spree::ItemTotalUpdater
+  class << self
+    def recalculate(item)
+      item.adjustment_total = item.adjustments
+        .reject(&:included?)
+        .sum(&:amount)
+    end
+  end
+end
diff --git a/core/spec/models/spree/item_total_updater_spec.rb b/core/spec/models/spree/item_total_updater_spec.rb
new file mode 100644
index 00000000000..180772e778c
--- /dev/null
+++ b/core/spec/models/spree/item_total_updater_spec.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Spree::ItemTotalUpdater do
+  describe ".recalculate" do
+    subject { described_class.recalculate(item) }
+
+    let(:item) { create :line_item, adjustments: [adjustment] }
+    let(:adjustment) { create :adjustment, amount: 1}
+
+    it "sets the adjustment total on the item" do
+      expect { subject }
+        .to change { item.adjustment_total }
+        .from(0).to(1)
+    end
+  end
+end

From 8e805abb97a753d196a32a6de51218ac96cc4c19 Mon Sep 17 00:00:00 2001
From: Alistair Norman <alistair@super.gd>
Date: Fri, 22 Nov 2024 14:25:22 -0800
Subject: [PATCH 16/16] Add spec creation to todo

---
 TODO.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/TODO.md b/TODO.md
index 34e9d98f558..5dcf61e09bb 100644
--- a/TODO.md
+++ b/TODO.md
@@ -1,6 +1,7 @@
 In-Memory Order Updater TODO
 ===
 
+- [ ] Add additional cases to item_total_updater_spec (doesn't currently account for included adjustments)
 - [ ] Consider Sofia's recommendation to break this class into POROs to simplify testing
 - [ ] Add test coverage for `update_item_total` when line item totals change
 - [ ] Test coverage to ensure state changes aren't persisted (if someone changes current implementation)