Don't persist line item on promotion application

This is in service of the in-memory order updater.

Co-authored-by: Adam <[email protected]>
Co-authored-by: Alistair <[email protected]>
Co-authored-by: An <[email protected]>
Co-authored-by: Benjamin <[email protected]>
Co-authored-by: Chris <[email protected]>
Co-authored-by: Harmony <[email protected]>
Co-authored-by: Jared <[email protected]>
Co-authored-by: Nick <[email protected]>
Co-authored-by: Noah <[email protected]>
Co-authored-by: Senem <[email protected]>
Co-authored-by: Sofia <[email protected]>

Author: 11 people

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

@@ -17,9 +17,7 @@ def perform(order)
       end
 
       def remove_from(order)
-        line_item = find_item(order)
-        # The enemy
-        order.line_items.destroy(line_item)
+        find_item(order)&.mark_for_destruction
       end
 
       private
@@ -28,9 +26,8 @@ def find_item(order)
         order.line_items.detect { |line_item| line_item.managed_by_order_benefit == self }
       end
 
-      # The enemy - create_item
       def create_item(order)
-        order.line_items.create!(quantity: determine_item_quantity(order), variant: variant, managed_by_order_benefit: self)
+        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/discount_order.rb

@@ -36,7 +36,6 @@ def call
 
       def perform_order_benefits(lane_benefits, lane)
         lane_benefits.select { |benefit| benefit.level == :order }.each do |benefit|
-          # TODO: - the enemy
           benefit.perform(order)
         end
 
@@ -48,7 +47,6 @@ def perform_order_benefits(lane_benefits, lane)
         end
 
         ineligible_line_items.each do |line_item|
-          # TODO: - the enemy
           line_item.managed_by_order_benefit.remove_from(order)
         end
       end

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

@@ -48,7 +48,7 @@ def update_adjustments(item, item_discounts)
         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
 
@@ -71,7 +71,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/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

promotions/spec/models/solidus_promotions/order_adjuster_spec.rb

@@ -5,51 +5,49 @@
 RSpec.describe SolidusPromotions::OrderAdjuster, type: :model do
   subject(:order_adjuster) { described_class.new(order, dry_run_promotion: dry_run_promotion) }
 
+  let!(:order) { line_item.order }
   let(:dry_run_promotion) { nil }
   let(:line_item) { create(:line_item) }
-  let(:order) { line_item.order }
   let(:promotion) { create(:solidus_promotion, apply_automatically: true) }
   let(:calculator) { SolidusPromotions::Calculators::Percent.new(preferred_percent: 10) }
 
   context "adding order level adjustments" do
     let(:variant) { create(:variant, price: 100) }
-    let(:benefit) do
+    let!(:benefit) do
       SolidusPromotions::Benefits::CreateDiscountedItem.create(promotion: promotion, calculator: calculator, preferences: { variant_id: variant.id })
     end
     let(:adjustable) { order }
 
-    subject do
-      benefit
-      order_adjuster.call
-    end
+    context "when not doing a dry-run of a promotion" do
+      subject do
+        order_adjuster.call
+      end
 
-    it "creates a line item of the given variant with a discount adjustment corresponding to the calculator" do
-      expect {
-        subject
-      }.to change { order.line_items.count }.by(1)
+      it "builds a line item of the given variant with a discount adjustment corresponding to the calculator", :aggregate_failures do
+        expect {
+          subject
+        }.to change { order.line_items.length }.by(1)
 
-      expect(order.line_items.last.variant).to eq(variant)
-      expect(order.line_items.last.adjustments.promotion.first&.amount).to eq(-10)
+        expect(order.line_items.last).not_to be_persisted
+        expect(order.line_items.last.variant).to eq(variant)
+        expect(order.line_items.last.adjustments.first.amount).to eq(-10)
+        expect(order.line_items.last.adjustments.first.source).to eq(benefit)
+      end
     end
 
     context 'when on a dry run' do
-      let(:dry_run_promotion) { create(:solidus_promotion, :with_adjustable_benefit, promotion_benefit_class: SolidusPromotions::Benefits::CreateDiscountedItem) }
+      let(:dry_run_promotion) { promotion }
 
       subject do
-        benefit
         order_adjuster.call
       end
 
-      it 'builds the line item but does not save it' do
-        expect {
-          subject
-        }.to change { order.line_items.length }.by(1)
-
-        pending "currently on a dry run this just doesn't happen"
-        expect(order.line_items.last.variant).to eq(variant)
-        expect(order.line_items.last.adjustments.promotion.first&.amount).to eq(-10)
+      it "does not create any line items" do
+        expect { subject }.not_to change(Spree::LineItem, :count)
+      end
 
-        expect(order.reload.line_items.length).to eq(1)
+      it "does not create any adjustments" do
+        expect { subject }.not_to change(Spree::Adjustment, :count)
       end
     end
   end
@@ -202,7 +200,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.count }
+        end.not_to change { order.shipments.first.shipping_rates.first.discounts.length }
       end
     end
   end
@@ -217,7 +215,7 @@
       expect do
         promotion
         subject.call
-      end.to change { order.shipments.first.adjustments.count }
+      end.to change { order.shipments.first.adjustments.length }
     end
 
     context "if the promo is eligible but the calculcator returns 0" do
@@ -228,7 +226,7 @@
         expect do
           promotion
           subject.call
-        end.not_to change { order.shipments.first.adjustments.count }
+        end.not_to change { order.shipments.first.adjustments.length }
       end
     end
   end

promotions/spec/rails_helper.rb

@@ -5,6 +5,7 @@
 
 require "spec_helper"
 require "solidus_legacy_promotions"
+require 'db-query-matchers'
 
 # SOLIDUS DUMMY APP
 require "spree/testing_support/dummy_app"