Build discounted linte items in memory

core/app/models/spree/order_updater.rb

@@ -168,7 +168,7 @@ def recalculate_order_total
     def update_adjustment_total
       update_adjustments
 
-      all_items = line_items + shipments
+      all_items = (line_items + shipments).reject(&:marked_for_destruction?)
       # Ignore any adjustments that have been marked for destruction in our
       # calculations. They'll get removed when/if we persist the order.
       valid_adjustments = adjustments.reject(&:marked_for_destruction?)
@@ -232,12 +232,11 @@ def recalculate_item_totals
 
         next unless item.changed?
 
-        item.update_columns(
+        item.assign_attributes(
           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,
+          adjustment_total:     item.adjustment_total
         )
       end
     end

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

@@ -5,7 +5,7 @@ module SpreeOrderUpdaterPatch
     def update_adjustment_total
       update_adjustments
 
-      all_items = line_items + shipments
+      all_items = (line_items + shipments).reject(&:marked_for_destruction?)
       order_tax_adjustments = adjustments.select(&:eligible?).select(&:tax?)
 
       order.adjustment_total = all_items.sum(&:adjustment_total) + adjustments.select(&:eligible?).sum(&:amount)
@@ -25,20 +25,20 @@ def recalculate_item_totals
         # 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?).
-          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
+        item.adjustment_total = item.adjustments.select { |adjustment|
+          adjustment.eligible? &&
+            !adjustment.marked_for_destruction? &&
+            !adjustment.included?
+        }.sum(&:amount)
+
+        next unless item.changed?
+
+        item.assign_attributes(
+          promo_total:          item.promo_total,
+          included_tax_total:   item.included_tax_total,
+          additional_tax_total: item.additional_tax_total,
+          adjustment_total:     item.adjustment_total
+        )
       end
     end
     Spree::OrderUpdater.prepend self

legacy_promotions/spec/models/spree/promotion/actions/create_adjustment_spec.rb

@@ -126,4 +126,42 @@
       is_expected.to eq(Spree::Config.promotions.calculators[described_class.to_s])
     }
   end
+
+  describe "#compute_amount" do
+    subject { action.compute_amount(order) }
+
+    before do
+      promotion.promotion_actions = [action]
+      action.calculator = Spree::Calculator::FlatRate.new(preferred_amount:)
+    end
+
+    let(:preferred_amount) { 50 }
+
+    context "when the adjustable is actionable" do
+      it "calls compute on the calculator" do
+        expect(action.calculator).to receive(:compute).with(order).and_call_original
+        subject
+      end
+
+      it "doesn't persist anything to the database" do
+        allow(action.calculator).to receive(:compute).with(order).and_call_original
+
+        expect {
+          subject
+        }.not_to make_database_queries(manipulative: true)
+      end
+
+      context "calculator returns amount greater than order total" do
+        let(:preferred_amount) { 300 }
+
+        before do
+          allow(order).to receive_messages(item_total: 50, ship_total: 50)
+        end
+
+        it "does not exceed it" do
+          expect(subject).to eql(-100)
+        end
+      end
+    end
+  end
 end

legacy_promotions/spec/models/spree/promotion/actions/create_item_adjustments_spec.rb

@@ -92,13 +92,23 @@ module Spree
       end
     end
 
-    context "#compute_amount" do
+    describe "#compute_amount" do
+      subject { action.compute_amount(line_item) }
+
       before { promotion.promotion_actions = [action] }
 
       context "when the adjustable is actionable" do
         it "calls compute on the calculator" do
           expect(action.calculator).to receive(:compute).with(line_item).and_call_original
-          action.compute_amount(line_item)
+          subject
+        end
+
+        it "doesn't persist anything to the database" do
+          allow(action.calculator).to receive(:compute).with(line_item).and_call_original
+
+          expect {
+            subject
+          }.not_to make_database_queries(manipulative: true)
         end
 
         context "calculator returns amount greater than item total" do
@@ -108,7 +118,7 @@ module Spree
           end
 
           it "does not exceed it" do
-            expect(action.compute_amount(line_item)).to eql(-100)
+            expect(subject).to eql(-100)
           end
         end
       end

legacy_promotions/spec/rails_helper.rb

@@ -27,6 +27,7 @@
 require 'rspec/rails'
 require 'rspec-activemodel-mocks'
 require 'database_cleaner'
+require 'db-query-matchers'
 
 Dir["./spec/support/**/*.rb"].sort.each { |f| require f }
 

promotions/app/models/solidus_promotions/benefits/create_discounted_item.rb

@@ -9,14 +9,13 @@ class CreateDiscountedItem < Benefit
       preference :necessary_quantity, :integer, default: 1
 
       def perform(order)
-        line_item = find_item(order) || create_item(order)
+        line_item = find_item(order) || build_item(order)
         set_quantity(line_item, determine_item_quantity(order))
         line_item.current_discounts << discount(line_item)
       end
 
       def remove_from(order)
-        line_item = find_item(order)
-        order.line_items.destroy(line_item)
+        find_item(order)&.mark_for_destruction
       end
 
       private
@@ -25,8 +24,8 @@ def find_item(order)
         order.line_items.detect { |line_item| line_item.managed_by_order_benefit == self }
       end
 
-      def create_item(order)
-        order.line_items.create!(quantity: determine_item_quantity(order), variant: variant, managed_by_order_benefit: self)
+      def build_item(order)
+        order.line_items.build(quantity: determine_item_quantity(order), variant: variant, managed_by_order_benefit: self)
       end
 
       def determine_item_quantity(order)

promotions/app/models/solidus_promotions/order_adjuster.rb

@@ -25,10 +25,12 @@ def call
       order.reset_current_discounts
 
       unless dry_run
-        # Since automations might have added a line item, we need to recalculate item total and item count here.
-        order.item_total = order.line_items.sum(&:amount)
-        order.item_count = order.line_items.sum(&:quantity)
-        order.promo_total = (order.line_items + order.shipments).sum(&:promo_total)
+        # Since automations might have added a line item, we need to recalculate
+        # item total and item count here.
+        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).sum(&:promo_total)
       end
       order
     end

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

@@ -34,33 +34,34 @@ def call
 
       attr_reader :order
 
-      # Walk through the discounts for an item and update adjustments for it. Once
-      # all of the discounts have been added as adjustments, remove any old promotion
-      # adjustments that weren't touched.
+      # Walk through the discounts for an item and update adjustments for it.
+      # Once all of the discounts have been added as adjustments, mark any old
+      # promotion adjustments that weren't touched for destruction.
       #
       # @private
       # @param [#adjustments] item a {Spree::LineItem} or {Spree::Shipment}
-      # @param [Array<SolidusPromotions::ItemDiscount>] item_discounts a list of calculated discounts for an item
+      # @param [Array<SolidusPromotions::ItemDiscount>] item_discounts a list of
+      #   calculated discounts for an item
       # @return [void]
       def update_adjustments(item, item_discounts)
         promotion_adjustments = item.adjustments.select(&:promotion?)
 
         active_adjustments = item_discounts.map do |item_discount|
           update_adjustment(item, item_discount)
         end
-        item.update(promo_total: active_adjustments.sum(&:amount))
+        item.promo_total = active_adjustments.sum(&:amount)
         # Remove any promotion adjustments tied to promotion benefits which no longer match.
         unmatched_adjustments = promotion_adjustments - active_adjustments
 
-        item.adjustments.destroy(unmatched_adjustments)
+        unmatched_adjustments.each(&:mark_for_destruction)
       end
 
-      # Update or create a new promotion adjustment on an item.
+      # Update or build a new promotion adjustment on an item.
       #
       # @private
       # @param [#adjustments] item a {Spree::LineItem} or {Spree::Shipment}
       # @param [SolidusPromotions::ItemDiscount] discount_item calculated discounts for an item
-      # @return [Spree::Adjustment] the created or updated promotion adjustment
+      # @return [Spree::Adjustment] the new or updated promotion adjustment
       def update_adjustment(item, discount_item)
         adjustment = item.adjustments.detect do |item_adjustment|
           item_adjustment.source == discount_item.source
@@ -71,7 +72,7 @@ def update_adjustment(item, discount_item)
           order_id: item.is_a?(Spree::Order) ? item.id : item.order_id,
           label: discount_item.label
         )
-        adjustment.update!(amount: discount_item.amount)
+        adjustment.amount = discount_item.amount
         adjustment
       end
     end

promotions/spec/models/solidus_promotions/benefit_spec.rb

@@ -122,6 +122,25 @@ def compute_line_item(_line_item, _options) = 1
     end
   end
 
+  describe "#compute_amount" do
+    subject { benefit.compute_amount(discountable) }
+
+    let(:variant) { create(:variant) }
+    let(:order) { create(:order) }
+    let(:discountable) { Spree::LineItem.new(order: order, variant: variant, price: 10, quantity: 1) }
+    let(:promotion) { SolidusPromotions::Promotion.new(customer_label: "20 Perzent off") }
+    let(:calculator) { SolidusPromotions::Calculators::Percent.new(preferred_percent: 20) }
+    let(:benefit) { described_class.new(promotion: promotion, calculator: calculator) }
+
+    it "doesn't save anything to the database" do
+      discountable
+
+      expect {
+        subject
+      }.not_to make_database_queries(manipulative: true)
+    end
+  end
+
   describe ".original_promotion_action" do
     let(:spree_promotion) { create :promotion, :with_adjustable_action }
     let(:spree_promotion_action) { spree_promotion.actions.first }

promotions/spec/models/solidus_promotions/benefits/adjust_line_item_quantity_groups_spec.rb

@@ -50,6 +50,14 @@
         context "and an item with a quantity of 3" do
           let(:quantity) { 3 }
           it { is_expected.to eq(-10) }
+
+          it "doesn't save anything to the database" do
+            line_item
+
+            expect {
+              subject
+            }.not_to make_database_queries(manipulative: true)
+          end
         end
 
         context "and an item with a quantity of 4" do

promotions/spec/models/solidus_promotions/benefits/create_discounted_item_spec.rb

@@ -5,22 +5,65 @@
 RSpec.describe SolidusPromotions::Benefits::CreateDiscountedItem do
   it { is_expected.to respond_to(:preferred_variant_id) }
 
+  let!(:benefit) { SolidusPromotions::Benefits::CreateDiscountedItem.new(preferred_variant_id: goodie.id, calculator: hundred_percent, promotion: promotion) }
+  let(:hundred_percent) { SolidusPromotions::Calculators::Percent.new(preferred_percent: 100) }
+  let(:promotion) { create(:solidus_promotion) }
+  let(:goodie) { create(:variant) }
+
   describe "#perform" do
-    let(:order) { create(:order_with_line_items) }
-    let(:promotion) { create(:solidus_promotion) }
-    let(:benefit) { SolidusPromotions::Benefits::CreateDiscountedItem.new(preferred_variant_id: goodie.id, calculator: hundred_percent, promotion: promotion) }
-    let(:hundred_percent) { SolidusPromotions::Calculators::Percent.new(preferred_percent: 100) }
-    let(:goodie) { create(:variant) }
+    let!(:order) { create(:order_with_line_items) }
+
     subject { benefit.perform(order) }
 
     it "creates a line item with a hundred percent discount" do
-      expect { subject }.to change { order.line_items.count }.by(1)
+      expect { subject }.to change { order.line_items.size }.by(1)
       created_item = order.line_items.detect { |line_item| line_item.managed_by_order_benefit == benefit }
       expect(created_item.discountable_amount).to be_zero
     end
 
     it "never calls the order recalculator" do
       expect(order).not_to receive(:recalculate)
     end
+
+    it "does not persist changes to order" do
+      expect {
+        subject
+      }.not_to make_database_queries(manipulative: true)
+    end
+  end
+
+  describe "remove_from" do
+    let!(:order) { create(:order_with_line_items) }
+
+    subject { benefit.remove_from(order) }
+
+    context "with an item not on the order" do
+      it "does not modify the order" do
+        expect {
+          subject
+        }.not_to make_database_queries(manipulative: true)
+      end
+    end
+
+    context "with an item present on the order" do
+      before do
+        benefit.perform(order)
+        order.save!
+      end
+
+      it "marks the the line item for destruction" do
+        expect { subject }.to change {
+          order.line_items.select(&:marked_for_destruction?).count
+        }.by(1)
+
+        expect { order.save }.to change(order.line_items, :count).by(-1)
+      end
+
+      it "does not make manipulative database queries" do
+        expect {
+          subject
+        }.not_to make_database_queries(manipulative: true)
+      end
+    end
   end
 end