Promotions: Deprecate Spree::PromotionRule#actionable and split rules that implement it

backend/app/views/spree/admin/promotions/rules/_line_item_option_value.html.erb

@@ -0,0 +1,12 @@
+<div class="field promo-rule-option-values">
+  <div class="param-prefix hidden" data-param-prefix="<%= param_prefix %>"></div>
+  <div class="row">
+    <div class="col-6"><%= label_tag nil, Spree::Product.model_name.human %></div>
+    <div class="col-6"><%= label_tag nil, plural_resource_name(Spree::OptionValue) %></div>
+  </div>
+
+  <%= content_tag :div, nil, class: "js-promo-rule-option-values",
+                             data: { :'original-option-values' => promotion_rule.preferred_eligible_values } %>
+
+  <button class="button js-add-promo-rule-option-value"><%= t("spree.actions.add") %></button>
+</div>

backend/app/views/spree/admin/promotions/rules/_line_item_product.html.erb

@@ -0,0 +1,18 @@
+<div class="row">
+  <div class="col-12">
+    <div class="field products_rule_products">
+      <%= label_tag "#{param_prefix}_product_ids_string", t('spree.product_rule.choose_products') %>
+      <%= hidden_field_tag "#{param_prefix}[product_ids_string]", promotion_rule.product_ids.join(","), class: "product_picker fullwidth" %>
+    </div>
+  </div>
+  <div class="col-12">
+    <div class="field">
+        <%= label_tag("#{param_prefix}_preferred_match_policy", promotion_rule.class.human_attribute_name(:preferred_match_policy)) %>
+        <%= select_tag(
+          "#{param_prefix}[preferred_match_policy]",
+          options_for_select(I18n.t("spree.promotion_rules.line_item_product.match_policies").to_a.map(&:reverse), promotion_rule.preferred_match_policy),
+          class: "custom-select fullwidth"
+        ) %>
+    </div>
+  </div>
+</div>

backend/app/views/spree/admin/promotions/rules/_line_item_taxon.html.erb

@@ -0,0 +1,14 @@
+<div class="field taxons_rule_taxons">
+  <%= label_tag "#{param_prefix}_taxon_ids_string", t("spree.taxon_rule.choose_taxons") %>
+  <%= hidden_field_tag "#{param_prefix}[taxon_ids_string]", promotion_rule.taxon_ids.join(","), class: "taxon_picker fullwidth", id: "product_taxon_ids" %>
+</div>
+<div class="field">
+   <div class="field">
+        <%= label_tag("#{param_prefix}_preferred_match_policy", promotion_rule.class.human_attribute_name(:preferred_match_policy)) %>
+        <%= select_tag(
+              "#{param_prefix}[preferred_match_policy]",
+              options_for_select(I18n.t("spree.promotion_rules.line_item_taxon.match_policies").to_a.map(&:reverse), promotion_rule.preferred_match_policy),
+              class: "custom-select fullwidth",
+            ) %>
+    </div>
+</div>

backend/spec/features/admin/promotion_adjustments_spec.rb

@@ -118,7 +118,7 @@
       click_button "Create"
       expect(page).to have_title("SAVE SAVE SAVE - Promotions")
 
-      select "Product(s)", from: "Discount Rules"
+      select "Line Item Product(s)", from: "Discount Rules"
       within("#rule_fields") { click_button "Add" }
       select2_search "RoR Mug", from: "Choose products"
       within('#rule_fields') { click_button "Update" }
@@ -136,7 +136,7 @@
       expect(promotion.codes.first).to be_nil
 
       first_rule = promotion.rules.first
-      expect(first_rule.class).to eq(Spree::Promotion::Rules::Product)
+      expect(first_rule.class).to eq(Spree::Promotion::Rules::LineItemProduct)
       expect(first_rule.products.map(&:name)).to include("RoR Mug")
 
       first_action = promotion.actions.first

backend/spec/features/admin/promotions/product_rule_spec.rb

@@ -18,7 +18,7 @@ def add_promotion_rule_of_type(type)
 
   background do
     visit spree.edit_admin_promotion_path(promotion)
-    add_promotion_rule_of_type("Product(s)")
+    add_promotion_rule_of_type("Order Product(s)")
   end
 
   it "can select by product sku" do

core/app/models/spree/calculator/distributed_amount.rb

@@ -15,18 +15,18 @@ class Calculator::DistributedAmount < Calculator
     def compute_line_item(line_item)
       return 0 unless line_item
       return 0 unless preferred_currency.casecmp(line_item.currency).zero?
-      return 0 unless calculable.promotion.line_item_actionable?(line_item.order, line_item)
+      return 0 unless calculable.promotion.line_item_eligible?(line_item)
       Spree::DistributedAmountsHandler.new(
-        actionable_line_items(line_item.order),
+        eligible_line_items(line_item.order),
         preferred_amount
       ).amount(line_item)
     end
 
     private
 
-    def actionable_line_items(order)
+    def eligible_line_items(order)
       order.line_items.select do |line_item|
-        calculable.promotion.line_item_actionable?(order, line_item)
+        calculable.promotion.line_item_eligible?(line_item)
       end
     end
   end

core/app/models/spree/promotion.rb

@@ -211,20 +211,14 @@ def usage_count(excluded_orders: [])
     end
 
     def line_item_actionable?(order, line_item, promotion_code: nil)
-      return false if blacklisted?(line_item)
+      line_item_eligible?(line_item, promotion_code: promotion_code)
+    end
+    deprecate line_item_actionable?: :line_item_eligible?, deprecator: Spree::Deprecation
 
-      if eligible?(order, promotion_code: promotion_code)
-        rules = eligible_rules(order)
-        if rules.blank?
-          true
-        else
-          rules.send(match_all? ? :all? : :any?) do |rule|
-            rule.actionable? line_item
-          end
-        end
-      else
-        false
-      end
+    def line_item_eligible?(line_item, promotion_code: nil)
+      !blacklisted?(line_item) &&
+        !!eligible_rules(line_item) &&
+        deprecated_line_item_actionable?(line_item, promotion_code: promotion_code)
     end
 
     def used_by?(user, excluded_orders = [])
@@ -252,6 +246,31 @@ def remove_from(order)
 
     private
 
+    def deprecated_line_item_actionable?(line_item, promotion_code: {})
+      if eligible?(line_item.order, promotion_code: promotion_code)
+        rules = eligible_rules(line_item.order)
+        if rules.blank?
+          true
+        else
+          rules.send(match_all? ? :all? : :any?) do |rule|
+            if rule.respond_to?(:actionable?)
+              Spree::Deprecation.warn(
+                <<~WARN
+                The API of promotion rules has changed. Rather than specifying "actionable?" on your rule, create a new rule
+                that is applicable to line items and move the logic in your `actionable?` method to that rule's `eligible?` method.
+                WARN
+              )
+              rule.actionable? line_item
+            else
+              true
+            end
+          end
+        end
+      else
+        false
+      end
+    end
+
     def blacklisted?(promotable)
       case promotable
       when Spree::LineItem

core/app/models/spree/promotion/actions/create_item_adjustments.rb

@@ -35,7 +35,7 @@ def perform(payload = {})
         # item_total and ship_total
         def compute_amount(adjustable)
           order = adjustable.is_a?(Order) ? adjustable : adjustable.order
-          return 0 unless promotion.line_item_actionable?(order, adjustable)
+          return 0 unless promotion.line_item_eligible?(adjustable)
           promotion_amount = calculator.compute(adjustable)
           promotion_amount ||= BigDecimal(0)
           promotion_amount = promotion_amount.abs
@@ -89,7 +89,7 @@ def ensure_action_has_calculator
         def line_items_to_adjust(promotion, order)
           order.line_items.select do |line_item|
             line_item.adjustments.none? { |adjustment| adjustment.source == self } &&
-              promotion.line_item_actionable?(order, line_item)
+              promotion.line_item_eligible?(line_item)
           end
         end
       end

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

@@ -61,7 +61,7 @@ def compute_amount(line_item)
           adjustment_amount = adjustment_amount.abs
 
           order = line_item.order
-          line_items = actionable_line_items(order)
+          line_items = eligible_line_items(order)
 
           actioned_line_items = order.line_item_adjustments.reload.
             select { |adjustment| adjustment.source == self && adjustment.amount < 0 }.
@@ -83,9 +83,9 @@ def compute_amount(line_item)
 
         private
 
-        def actionable_line_items(order)
+        def eligible_line_items(order)
           order.line_items.select do |item|
-            promotion.line_item_actionable? order, item
+            promotion.line_item_eligible? item
           end
         end
 

core/app/models/spree/promotion/rules/line_item_option_value.rb

@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+module Spree
+  class Promotion < Spree::Base
+    module Rules
+      class LineItemOptionValue < PromotionRule
+        preference :eligible_values, :hash
+
+        def applicable?(promotable)
+          promotable.is_a?(Spree::LineItem)
+        end
+
+        def eligible?(line_item, _options = {})
+          pid = line_item.product.id
+          ovids = line_item.variant.option_values.pluck(:id)
+
+          product_ids.include?(pid) && (value_ids(pid) & ovids).present?
+        end
+
+        def preferred_eligible_values
+          values = preferences[:eligible_values] || {}
+          Hash[values.keys.map(&:to_i).zip(
+            values.values.map do |value|
+              (value.is_a?(Array) ? value : value.split(",")).map(&:to_i)
+            end
+          )]
+        end
+
+        private
+
+        def product_ids
+          preferred_eligible_values.keys
+        end
+
+        def value_ids(product_id)
+          preferred_eligible_values[product_id]
+        end
+      end
+    end
+  end
+end

core/app/models/spree/promotion/rules/line_item_product.rb

@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+module Spree
+  class Promotion < Spree::Base
+    module Rules
+      # A rule to apply a promotion only to line items with or without a chosen product
+      class LineItemProduct < Spree::PromotionRule
+        MATCH_POLICIES = %w(include exclude)
+
+        has_many :product_promotion_rules,
+                 dependent: :destroy,
+                 foreign_key: :promotion_rule_id,
+                 class_name: "Spree::ProductPromotionRule"
+        has_many :products,
+                 class_name: "Spree::Product",
+                 through: :product_promotion_rules
+
+        preference :match_policy, :string, default: MATCH_POLICIES.first
+
+        def applicable?(promotable)
+          promotable.is_a?(Spree::LineItem)
+        end
+
+        def eligible?(line_item, _options = {})
+          if inverse?
+            !product_ids.include?(line_item.variant.product_id)
+          else
+            product_ids.include?(line_item.variant.product_id)
+          end
+        end
+
+        def product_ids_string
+          product_ids.join(",")
+        end
+
+        def product_ids_string=(product_ids)
+          self.product_ids = product_ids.to_s.split(",").map(&:strip)
+        end
+
+        private
+
+        def inverse?
+          preferred_match_policy == "exclude"
+        end
+      end
+    end
+  end
+end

core/app/models/spree/promotion/rules/line_item_taxon.rb

@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+module Spree
+  class Promotion < Spree::Base
+    module Rules
+      class LineItemTaxon < PromotionRule
+        has_many :promotion_rule_taxons, class_name: 'Spree::PromotionRuleTaxon', foreign_key: :promotion_rule_id,
+          dependent: :destroy
+        has_many :taxons, through: :promotion_rule_taxons, class_name: 'Spree::Taxon'
+
+        MATCH_POLICIES = %w(include exclude)
+
+        validates_inclusion_of :preferred_match_policy, in: MATCH_POLICIES
+
+        preference :match_policy, :string, default: MATCH_POLICIES.first
+        def applicable?(promotable)
+          promotable.is_a?(Spree::LineItem)
+        end
+
+        def eligible?(line_item, _options = {})
+          found = Spree::Classification.where(
+            product_id: line_item.variant.product_id,
+            taxon_id: rule_taxon_ids_with_children
+          ).exists?
+
+          case preferred_match_policy
+          when 'include'
+            found
+          when 'exclude'
+            !found
+          else
+            raise "unexpected match policy: #{preferred_match_policy.inspect}"
+          end
+        end
+
+        def taxon_ids_string
+          taxons.pluck(:id).join(',')
+        end
+
+        def taxon_ids_string=(taxon_ids)
+          taxon_ids = taxon_ids.to_s.split(',').map(&:strip)
+          self.taxons = Spree::Taxon.find(taxon_ids)
+        end
+
+        private
+
+        # ids of taxons rules and taxons rules children
+        def rule_taxon_ids_with_children
+          taxons.flat_map { |taxon| taxon.self_and_descendants.ids }.uniq
+        end
+      end
+    end
+  end
+end

core/app/models/spree/promotion/rules/option_value.rb

@@ -4,28 +4,18 @@ module Spree
   class Promotion < Spree::Base
     module Rules
       class OptionValue < PromotionRule
-        MATCH_POLICIES = %w(any)
-        preference :match_policy, :string, default: MATCH_POLICIES.first
         preference :eligible_values, :hash
 
         def applicable?(promotable)
           promotable.is_a?(Spree::Order)
         end
 
-        def eligible?(promotable, _options = {})
-          case preferred_match_policy
-          when 'any'
-            promotable.line_items.any? { |item| actionable?(item) }
+        def eligible?(order, _options = {})
+          order.line_items.any? do |item|
+            LineItemOptionValue.new(preferred_eligible_values: preferred_eligible_values).eligible?(item)
           end
         end
 
-        def actionable?(line_item)
-          pid = line_item.product.id
-          ovids = line_item.variant.option_values.pluck(:id)
-
-          product_ids.include?(pid) && (value_ids(pid) & ovids).present?
-        end
-
         def preferred_eligible_values
           values = preferences[:eligible_values] || {}
           Hash[values.keys.map(&:to_i).zip(
@@ -34,16 +24,6 @@ def preferred_eligible_values
             end
           )]
         end
-
-        private
-
-        def product_ids
-          preferred_eligible_values.keys
-        end
-
-        def value_ids(product_id)
-          preferred_eligible_values[product_id]
-        end
       end
     end
   end