Extract order cleanup and total recalculation

Also removes the now-unnecessary `PersistDiscountedOrder` class.

Author: mamhoff

promotions/app/models/solidus_promotions/order_adjuster.rb

@@ -20,10 +20,9 @@ def call
 
       DiscountOrder.new(order, promotions, dry_run: dry_run).call
 
-      unless dry_run
-        order.save!
-      end
-      order
+      CleanDiscountedOrder.new(order).call
+
+      RecalculatePromoTotals.new(order).call
     end
   end
 end

promotions/app/models/solidus_promotions/order_adjuster/clean_discounted_order.rb

@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+module SolidusPromotions
+  class OrderAdjuster
+    class CleanDiscountedOrder
+      attr_reader :order
+
+      def initialize(order)
+        @order = order
+      end
+
+      def call
+        order.line_items.each do |line_item|
+          line_item.adjustments.select { _1.amount.zero? }.each(&:mark_for_destruction)
+        end
+
+        order.shipments.each do |shipment|
+          shipment.adjustments.select { _1.amount.zero? }.each(&:mark_for_destruction)
+          shipment.shipping_rates.each do |shipping_rate|
+            shipping_rate.discounts.select { _1.amount.zero? }.each(&:mark_for_destruction)
+          end
+        end
+
+        order
+      end
+    end
+  end
+end

promotions/app/models/solidus_promotions/order_adjuster/discount_order.rb

@@ -25,27 +25,6 @@ def call
           end
         end
 
-        order.line_items.each do |line_item|
-          line_item.adjustments.select { _1.amount.zero? }.each(&:mark_for_destruction)
-          line_item.promo_total = line_item.adjustments.reject(&:marked_for_destruction?).sum(&:amount)
-          line_item.adjustment_total = line_item.promo_total
-        end
-
-        order.shipments.each do |shipment|
-          shipment.adjustments.select { _1.amount.zero? }.each(&:mark_for_destruction)
-          shipment.promo_total = shipment.adjustments.reject(&:marked_for_destruction?).sum(&:amount)
-          shipment.shipping_rates.each do |shipping_rate|
-            shipping_rate.discounts.select { _1.amount.zero? }.each(&:mark_for_destruction)
-          end
-          shipment.adjustment_total = shipment.promo_total
-        end
-
-        line_items = order.line_items.reject(&:marked_for_destruction?)
-        order.item_total = line_items.sum(&:amount)
-        order.item_count = line_items.sum(&:quantity)
-        order.promo_total = (line_items + order.shipments.reject(&:marked_for_destruction?)).sum(&:promo_total)
-        order.adjustment_total = order.promo_total
-
         order
       end
 
@@ -85,7 +64,6 @@ def adjust_shipments(benefits)
       def adjust_shipping_rates(benefits)
         order.shipments.flat_map(&:shipping_rates).filter_map do |rate|
           next unless rate.cost
-
           discounts = generate_discounts(benefits, rate)
           chosen_discounts = SolidusPromotions.config.discount_chooser_class.new(discounts).call
           (rate.current_lane_discounts - chosen_discounts).each(&:mark_for_destruction)

promotions/app/models/solidus_promotions/order_adjuster/recalculate_promo_totals.rb

@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+module SolidusPromotions
+  class OrderAdjuster
+    class RecalculatePromoTotals
+      attr_reader :order
+
+      def initialize(order)
+        @order = order
+      end
+
+      def call
+        order.line_items.each do |line_item|
+          line_item.promo_total = calculate_promo_total_for_adjustable(line_item)
+        end
+
+        order.shipments.each do |shipment|
+          shipment.promo_total = calculate_promo_total_for_adjustable(shipment)
+        end
+
+        line_items = order.line_items.reject(&:marked_for_destruction?)
+        order.item_total = line_items.sum(&:amount)
+        order.item_count = line_items.sum(&:quantity)
+        order.promo_total = (line_items + order.shipments.reject(&:marked_for_destruction?)).sum(&:promo_total)
+        order.adjustment_total = order.promo_total
+      end
+
+      private
+
+      def calculate_promo_total_for_adjustable(adjustable)
+        adjustable
+          .adjustments
+          .select(&:promotion?)
+          .reject(&:marked_for_destruction?)
+          .sum(&:amount)
+      end
+    end
+  end
+end

promotions/spec/models/promotion/integration_spec.rb

@@ -42,7 +42,9 @@
         it "creates a new discounted line item" do
           expect(order.adjustments).to be_empty
           expect(order.line_items.count).to eq(3)
+          # 19.99 * 2
           expect(order.total).to eq(39.98)
+          # 19.99 * 2 + 4 * 1
           expect(order.item_total).to eq(43.98)
           expect(order.item_total_before_tax).to eq(39.98)
           expect(order.line_items.flat_map(&:adjustments).length).to eq(1)
@@ -56,6 +58,7 @@
           it "creates a new discounted line item" do
             expect(order.adjustments).to be_empty
             expect(order.line_items.count).to eq(3)
+            # 19.99 * 3
             expect(order.total).to eq(59.97)
             expect(order.item_total).to eq(67.97)
             expect(order.item_total_before_tax).to eq(59.97)

promotions/spec/models/solidus_promotions/order_adjuster_spec.rb

@@ -22,7 +22,7 @@
     end
     let(:adjustable) { order }
 
-    context "when not doing a dry-run of a promotion", :pending do
+    context "when not doing a dry-run of a promotion" do
       subject do
         order_adjuster.call
       end
@@ -117,6 +117,7 @@
         it " will not create the adjustment" do
           expect {
             subject
+            order.save!
           }.not_to change { adjustable.adjustments.length }
         end
       end
@@ -133,14 +134,15 @@
         it "doesn't create an adjustment" do
           expect {
             subject
+            order.save!
           }.to change { adjustable.adjustments.length }.by(0)
         end
 
         context "for an line item that has an adjustment from the old promotion system" do
           let(:old_promotion_benefit) { create(:promotion, :with_adjustable_action, apply_automatically: false).actions.first }
           let!(:adjustment) { create(:adjustment, source: old_promotion_benefit, adjustable: line_item) }
 
-          it "marks the old adjustment for destruction", :pending do
+          it "marks the old adjustment for destruction" do
             adjustable.reload
             expect {
               subject
@@ -199,7 +201,7 @@
     end
 
     it "creates shipping rate discounts" do
-      expect { subject }.to change { SolidusPromotions::ShippingRateDiscount.count }
+      expect { subject }.to change { order.shipments.first.shipping_rates.first.discounts.length }.by(1)
     end
 
     context "if the promo is eligible but the calculcator returns 0" do
@@ -209,7 +211,7 @@
       it "will not create an adjustment on the shipping rate" do
         expect do
           subject
-        end.not_to change { order.shipments.first.shipping_rates.first.discounts.length }
+        end.not_to change { order.shipments.first.shipping_rates.first.discounts.reject(&:marked_for_destruction?).length }
       end
     end
   end
@@ -224,7 +226,7 @@
       expect do
         promotion
         subject.call
-      end.to change { order.shipments.first.adjustments.length }
+      end.to change { order.shipments.first.adjustments.length }.from(0).to(1)
     end
 
     context "if the promo is eligible but the calculcator returns 0" do
@@ -235,7 +237,7 @@
         expect do
           promotion
           subject.call
-        end.not_to change { order.shipments.first.adjustments.length }
+        end.not_to change { order.shipments.first.adjustments.reject(&:marked_for_destruction?).length }
       end
     end
   end