fixup! WIP: high-level integration test for legacy promotion system

Author: benjaminwil

legacy_promotions/app/patches/models/solidus_legacy_promotions/spree_in_memory_order_updater_patch.rb

@@ -0,0 +1,47 @@
+module SolidusLegacyPromotions
+  module SpreeInMemoryOrderUpdaterPatch
+    def update_adjustment_total(persist:)
+      update_adjustments(persist:)
+
+      all_items = line_items + shipments
+      valid_adjustments = adjustments.select(&:eligible?).reject(&:marked_for_destruction?)
+      order_tax_adjustments = valid_adjustments.select(&:tax?)
+
+      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)
+
+      recalculate_order_total
+    end
+
+    def update_item_totals(persist:)
+      [*line_items, *shipments].each do |item|
+        Spree::Config.item_total_class.new(item).recalculate!
+
+        # The cancellation_total isn't persisted anywhere but is included in
+        # the adjustment_total.
+        #
+        # Core doesn't have "eligible" adjustments anymore, so we need to
+        # override the adjustment_total calculation to exclude them for legacy
+        # promotions.
+        item.adjustment_total = item.adjustments.
+          select(&:eligible?).
+          reject(&:included?).
+          reject(&:marked_for_destruction?).
+          sum(&:amount)
+
+        next unless persist && 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
+
+    Spree::InMemoryOrderUpdater.prepend self
+  end
+end

legacy_promotions/app/patches/models/solidus_legacy_promotions/spree_order_updater_patch.rb

@@ -42,7 +42,6 @@ def update_item_totals
       end
     end
 
-    # FIXME: We need to verify that this is the right thing to do?
-    Spree::Config.order_recalculator_class.prepend self
+    Spree::OrderUpdater.prepend self
   end
 end

legacy_promotions/spec/models/spree/promotion_integration_spec.rb

@@ -265,38 +265,66 @@
     end
 
     context 'with the in-memory order updater' do
-      subject { order.recalculate(persist: false) }
+      subject { order.recalculate(persist:) }
 
       around do |example|
         default_order_recalculator = Spree::Config.order_recalculator_class.to_s
 
-        Spree::Config.order_recalculator_class = 'Spree::InMemoryOrderUpdater'
+        Spree::Config.order_recalculator_class = Spree::InMemoryOrderUpdater
 
         example.run
 
         Spree::Config.order_recalculator_class = default_order_recalculator
       end
 
-      it "makes the promotion ineligible" do
-        expect { subject }
-          .to change { order.adjustments.first.eligible }
-          .from(true)
-          .to(false)
+      context "when not persisting updates" do
+        let(:persist) { false }
 
-        expect(promo_adjustment.reload.eligible).to eq true
-      end
+        it "changes but does not persist the promotion as ineligible" do
+          expect { subject }
+            .to change { order.adjustments.first.eligible }
+            .from(true)
+            .to(false)
+
+          expect(promo_adjustment.reload.eligible).to eq true
+        end
 
-      it "adjusts the promo_total" do
-        expect { subject }.to change { order.promo_total }.from(-10).to(0)
+        it "changes but does not persist the promo_total" do
+          expect { subject }.to change { order.promo_total }.from(-10).to(0)
 
-        expect(order.reload.promo_total).to eq -10
+          expect(order.reload.promo_total).to eq -10
+        end
+
+        it "changes the total but does not persist the promo amount" do
+          expect { subject }.to change { order.total }.from(30).to(40)
+
+          expect(order.reload.total).to eq 30
+        end
       end
 
-      it "increases the total to remove the promo",
-        pending: "This failure points to a real issue we have with the in-memory order updater" do
-        expect { subject }.to change { order.total }.from(30).to(40)
+      context "when persisting updates" do
+        let(:persist) { true }
 
-        expect(order.reload.total).to eq 40
+        it "makes the promotion ineligible" do
+          expect { subject }
+            .to change { order.adjustments.first.eligible }
+            .from(true)
+            .to(false)
+
+          expect(promo_adjustment.reload.eligible).to eq false
+        end
+
+        it "adjusts the promo_total" do
+          expect { subject }.to change { order.promo_total }.from(-10).to(0)
+
+          expect(order.reload.promo_total).to eq 0
+        end
+
+        it "increases the total to remove the promo" do
+          expect { subject }.to change { order.total }.from(30).to(40)
+
+          expect(order.reload.total).to eq 40
+        end
       end
     end
   end