From b9ce1ec65f28f6a050c5292717787f0f046d26b4 Mon Sep 17 00:00:00 2001
From: Martin Meyerhoff <mamhoff@gmail.com>
Date: Sun, 20 Mar 2022 19:20:18 +0100
Subject: [PATCH] Do not consider unpromotionable line items discountable

---
 core/app/models/spree/promotion.rb       |  3 ++-
 core/spec/models/spree/promotion_spec.rb | 28 ++++++++++++++++++++----
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/core/app/models/spree/promotion.rb b/core/app/models/spree/promotion.rb
index e72e7c02c9c..b4ed4b7e509 100644
--- a/core/app/models/spree/promotion.rb
+++ b/core/app/models/spree/promotion.rb
@@ -254,7 +254,8 @@ def order_discountable?(order)
     end
 
     def line_item_discountable?(line_item)
-      rules.all? { |rule| rule.line_item_discountable?(line_item) }
+      line_item.variant.product.promotionable? &&
+        rules.all? { |rule| rule.line_item_discountable?(line_item) }
     end
 
     def shipment_discountable?(shipment)
diff --git a/core/spec/models/spree/promotion_spec.rb b/core/spec/models/spree/promotion_spec.rb
index 1580681cebf..8a52b5f8ed7 100644
--- a/core/spec/models/spree/promotion_spec.rb
+++ b/core/spec/models/spree/promotion_spec.rb
@@ -934,7 +934,9 @@
   end
 
   describe "#line_item_discountable?" do
-    let(:line_item) { double Spree::LineItem }
+    let(:product) { build(:product) }
+    let(:variant) { build(:variant, product: product) }
+    let(:line_item) { build(:line_item, variant: variant) }
     let(:true_rule) { stub_model Spree::PromotionRule, line_item_discountable?: true }
     let(:false_rule) { stub_model Spree::PromotionRule, line_item_discountable?: false }
     let(:rules) { [] }
@@ -946,15 +948,33 @@
 
     subject { promotion.line_item_discountable?(line_item) }
 
-    context 'when the order is eligible for promotion' do
-      context 'when there are no rules' do
+    context 'when there are no rules' do
+      it { is_expected.to be }
+    end
+
+    context 'when there are rules' do
+      context 'when all rules allow action on the line item' do
+        let(:rules) { [true_rule] }
         it { is_expected.to be }
       end
 
+      context 'when at least one rule does not allow action on the line item' do
+        let(:rules) { [true_rule, false_rule] }
+        it { is_expected.not_to be }
+      end
+    end
+
+    context 'when the line_item is not eligible for promotion' do
+      let(:product) { build(:product, promotionable: false) }
+
+      context 'when there are no rules' do
+        it { is_expected.not_to be }
+      end
+
       context 'when there are rules' do
         context 'when all rules allow action on the line item' do
           let(:rules) { [true_rule] }
-          it { is_expected.to be }
+          it { is_expected.not_to be }
         end
 
         context 'when at least one rule does not allow action on the line item' do