Merge pull request #6393 from SuperGoodSoft/dont-persist-in-legacy-promotions

[legacy_promotions] Don't persist line item actions in compute_amount

Author: mamhoff

legacy_promotions/app/models/spree/promotion/actions/create_quantity_adjustments.rb

@@ -75,14 +75,19 @@ def compute_amount(line_item)
             line_item.quantity
           ].min
 
-          persist_quantity(usable_quantity, line_item)
+          set_line_item_action_quantity(usable_quantity, line_item)
 
           amount = adjustment_amount * usable_quantity
           [line_item.amount, amount].min * -1
         end
 
         private
 
+        def create_adjustment(adjustable, order, promotion_code)
+          return unless super
+          save!
+        end
+
         def actionable_line_items(order)
           order.line_items.select do |item|
             promotion.line_item_actionable? order, item
@@ -97,17 +102,16 @@ def total_applicable_quantity(line_items)
         end
 
         def total_used_quantity(line_items)
-          line_item_actions.where(
-            line_item_id: line_items.map(&:id)
-          ).sum(:quantity)
+          line_item_actions.select { |line_item_action|
+            line_items.map(&:id).include? line_item_action.line_item_id
+          }.sum(&:quantity)
         end
 
-        def persist_quantity(quantity, line_item)
+        def set_line_item_action_quantity(quantity, line_item)
           line_item_action = line_item_actions.where(
             line_item_id: line_item.id
           ).first_or_initialize
           line_item_action.quantity = quantity
-          line_item_action.save!
         end
 
         ##

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

@@ -22,6 +22,48 @@ module Spree::Promotion::Actions
     let(:quantity) { 1 }
     let(:promotion) { FactoryBot.create :promotion }
 
+    describe "#perform" do
+      subject { action.perform({ order:, promotion: }) }
+
+      let(:calculator) { FactoryBot.create :flat_rate_calculator, preferred_amount: }
+
+      context "when calculator computes 0" do
+        let(:preferred_amount) { 0 }
+
+        it "does not create an adjustment" do
+          expect { subject }
+            .not_to change { action.adjustments.count }
+        end
+      end
+
+      context "when calculator returns a non-zero value" do
+        let(:preferred_amount) { 10 }
+        let(:line_item) { order.line_items.first }
+
+        it "creates an adjustment" do
+          expect { subject }
+            .to change { action.adjustments.count }
+            .from(0).to(1)
+            .and change { line_item.adjustments.count }
+            .from(0).to(1)
+
+          expect(line_item.adjustments).to eq(action.adjustments)
+        end
+
+        it "associates the line item with the action", :aggregate_failures do
+          expect { subject }
+            .to change { line_item.line_item_actions.count }
+            .from(0).to(1)
+
+          expect(action.line_item_actions.first).to have_attributes(
+            line_item_id: line_item.id,
+            action_id: action.id,
+            quantity: 1
+          )
+        end
+      end
+    end
+
     describe "#compute_amount" do
       subject { action.compute_amount(line_item) }
 
@@ -46,6 +88,15 @@ module Spree::Promotion::Actions
           context "and an item with a quantity of 2" do
             let(:quantity) { 2 }
             it { is_expected.to eq(-10) }
+
+            it "doesn't save anything to the database" do
+              action
+              line_item
+
+              expect {
+                subject
+              }.not_to make_database_queries(manipulative: true)
+            end
           end
 
           context "and an item with a quantity of 3" do
@@ -94,7 +145,7 @@ module Spree::Promotion::Actions
               :order_with_line_items,
               line_items_attributes: [
                 { quantity: 3 }
-            ]
+              ]
             )
           end