[Extract][Has TODOs]Remove support for deprecated promo rules matchin…

…g policy

Matching "all" rules is the only supported path now. You can have the
same effect of the previoys "any" matching policy by creating multiple
promotions with the same configuration.

This change also removes the rake task we provided to update the
existing promotions. That script needs to be executed on previous
versions of Solidus only.

Ref solidusio#4304

api/lib/spree/api_configuration.rb

@@ -114,7 +114,7 @@ class ApiConfiguration < Preferences::Configuration
 
     preference :promotion_attributes, :array, default: [
       :id, :name, :description, :expires_at, :starts_at, :type, :usage_limit,
-      :match_policy, :advertise, :path
+      :advertise, :path
     ]
 
     preference :store_attributes, :array, default: [

backend/app/assets/stylesheets/spree/backend/sections/_promotions.scss

@@ -18,11 +18,6 @@
   }
 }
 
-#promotion-policy-select {
-  overflow: auto;
-  margin-bottom: 1.5rem;
-}
-
 .promotion-block {
   padding: 0 1.25rem 0.5rem;
   background-color: lighten($color-border, 5);

backend/app/views/spree/admin/promotions/_rules.html.erb

@@ -18,16 +18,6 @@
 
     <%= form_for @promotion, url: object_url, method: :put do |f| %>
       <fieldset class="no-border-top">
-        <% if Spree::Config.allow_promotions_any_match_policy %>
-          <div id="promotion-policy-select" class="align-center row">
-            <% Spree::Promotion::MATCH_POLICIES.each do |policy| %>
-              <div class="col-6">
-                <label><%= f.radio_button :match_policy, policy %> <%= t "spree.promotion_form.match_policies.#{policy}" %></label>
-              </div>
-            <% end %>
-          </div>
-        <% end %>
-
         <div id="rules" class="filter_list">
           <% if @promotion.rules.any? %>
             <div class="col-12">

core/app/models/spree/promotion.rb

@@ -2,6 +2,7 @@
 
 module Spree
   class Promotion < Spree::Base
+    # TODO: deprecate this in another PR and remove it here after merging it
     MATCH_POLICIES = %w(all any)
 
     UNACTIVATABLE_ORDER_STATES = ["complete", "awaiting_return", "returned"]
@@ -166,27 +167,13 @@ def eligible_rules(promotable, options = {})
       specific_rules = rules.select { |rule| rule.applicable?(promotable) }
       return [] if specific_rules.none?
 
-      if match_all?
-        # If there are rules for this promotion, but no rules for this
-        # particular promotable, then the promotion is ineligible by default.
-        unless specific_rules.all?(&eligible)
-          @eligibility_errors = specific_rules.map(&:eligibility_errors).detect(&:present?)
-          return nil
-        end
-        specific_rules
-      else
-        Spree::Deprecation.warn(
-        <<~WARN
-          Your promotion "#{name}" with ID #{id} has a match_policy of 'any'.
-          This is deprecated, please split the promotion into separate promotions for each rule.
-        WARN
-        )
-        unless specific_rules.any?(&eligible)
-          @eligibility_errors = specific_rules.map(&:eligibility_errors).detect(&:present?)
-          return nil
-        end
-        specific_rules.select(&eligible)
+      # If there are rules for this promotion, but no rules for this
+      # particular promotable, then the promotion is ineligible by default.
+      unless specific_rules.all?(&eligible)
+        @eligibility_errors = specific_rules.map(&:eligibility_errors).detect(&:present?)
+        return nil
       end
+      specific_rules
     end
 
     def products
@@ -223,9 +210,7 @@ def line_item_actionable?(order, line_item, promotion_code: nil)
         if rules.blank?
           true
         else
-          rules.send(match_all? ? :all? : :any?) do |rule|
-            rule.actionable? line_item
-          end
+          rules.all? { |rule| rule.actionable? line_item }
         end
       else
         false
@@ -270,10 +255,6 @@ def normalize_blank_values
       self[:path] = nil if self[:path].blank?
     end
 
-    def match_all?
-      match_policy == "all"
-    end
-
     def apply_automatically_disallowed_with_paths
       return unless apply_automatically
 

core/db/migrate/20230322085416_remove_match_policy_from_spree_promotion.rb

@@ -0,0 +1,5 @@
+class RemoveMatchPolicyFromSpreePromotion < ActiveRecord::Migration[5.2]
+  def change
+    remove_column :spree_promotions, :match_policy, :string, default: "all"
+  end
+end

core/lib/spree/app_configuration.rb

@@ -61,6 +61,7 @@ class AppConfiguration < Preferences::Configuration
     #   @return [Boolean] When false, customers must create an account to complete an order (default: +true+)
     preference :allow_guest_checkout, :boolean, default: true
 
+    # Todo: remove this preference after merging https://github.com/solidusio/solidus/pull/4991
     # @!attribute [rw] allow_promotions_any_match_policy
     #   @return [Boolean] When false, admins cannot create promotions with an "any" match policy (default: +false+)
     #                     Create individual, separate promotions for each of your rules instead.

core/spec/models/spree/promotion_spec.rb

@@ -745,6 +745,9 @@
 
   context "#eligible_rules" do
     let(:promotable) { double('Promotable') }
+    let(:rule1) { Spree::PromotionRule.create!(promotion: promotion) }
+    let(:rule2) { Spree::PromotionRule.create!(promotion: promotion) }
+
     it "true if there are no rules" do
       expect(promotion.eligible_rules(promotable)).to eq []
     end
@@ -754,74 +757,36 @@
       expect(promotion.eligible_rules(promotable)).to eq []
     end
 
-    context "with 'all' match policy" do
-      let(:rule1) { Spree::PromotionRule.create!(promotion: promotion) }
-      let(:rule2) { Spree::PromotionRule.create!(promotion: promotion) }
-
-      before { promotion.match_policy = 'all' }
-
-      context "when all rules are eligible" do
-        before do
-          allow(rule1).to receive_messages(eligible?: true, applicable?: true)
-          allow(rule2).to receive_messages(eligible?: true, applicable?: true)
+    context "when all rules are eligible" do
+      before do
+        allow(rule1).to receive_messages(eligible?: true, applicable?: true)
+        allow(rule2).to receive_messages(eligible?: true, applicable?: true)
 
-          promotion.promotion_rules = [rule1, rule2]
-        end
-        it "returns the eligible rules" do
-          expect(promotion.eligible_rules(promotable)).to eq [rule1, rule2]
-        end
-        it "does set anything to eligiblity errors" do
-          promotion.eligible_rules(promotable)
-          expect(promotion.eligibility_errors).to be_nil
-        end
+        promotion.promotion_rules = [rule1, rule2]
       end
-
-      context "when any of the rules is not eligible" do
-        let(:errors) { double ActiveModel::Errors, empty?: false }
-        before do
-          allow(rule1).to receive_messages(eligible?: true, applicable?: true, eligibility_errors: nil)
-          allow(rule2).to receive_messages(eligible?: false, applicable?: true, eligibility_errors: errors)
-
-          promotion.promotion_rules = [rule1, rule2]
-        end
-        it "returns nil" do
-          expect(promotion.eligible_rules(promotable)).to be_nil
-        end
-        it "sets eligibility errors to the first non-nil one" do
-          promotion.eligible_rules(promotable)
-          expect(promotion.eligibility_errors).to eq errors
-        end
+      it "returns the eligible rules" do
+        expect(promotion.eligible_rules(promotable)).to eq [rule1, rule2]
+      end
+      it "does set anything to eligiblity errors" do
+        promotion.eligible_rules(promotable)
+        expect(promotion.eligibility_errors).to be_nil
       end
     end
 
-    context "with 'any' match policy", :silence_deprecations do
-      let(:promotable) { double('Promotable') }
-
+    context "when any of the rules is not eligible" do
+      let(:errors) { double ActiveModel::Errors, empty?: false }
       before do
-        promotion.match_policy = 'any'
-      end
+        allow(rule1).to receive_messages(eligible?: true, applicable?: true, eligibility_errors: nil)
+        allow(rule2).to receive_messages(eligible?: false, applicable?: true, eligibility_errors: errors)
 
-      it "should have eligible rules if any of the rules are eligible" do
-        true_rule = stub_model(Spree::PromotionRule, eligible?: true, applicable?: true)
-        promotion.promotion_rules = [true_rule]
-        expect(promotion.eligible_rules(promotable)).to eq [true_rule]
+        promotion.promotion_rules = [rule1, rule2]
       end
-
-      context "when none of the rules are eligible" do
-        let(:rule) { Spree::PromotionRule.create!(promotion: promotion) }
-        let(:errors) { double ActiveModel::Errors, empty?: false }
-        before do
-          allow(rule).to receive_messages(eligible?: false, applicable?: true, eligibility_errors: errors)
-
-          promotion.promotion_rules = [rule]
-        end
-        it "returns nil" do
-          expect(promotion.eligible_rules(promotable)).to be_nil
-        end
-        it "sets eligibility errors to the first non-nil one" do
-          promotion.eligible_rules(promotable)
-          expect(promotion.eligibility_errors).to eq errors
-        end
+      it "returns nil" do
+        expect(promotion.eligible_rules(promotable)).to be_nil
+      end
+      it "sets eligibility errors to the first non-nil one" do
+        promotion.eligible_rules(promotable)
+        expect(promotion.eligibility_errors).to eq errors
       end
     end
   end
@@ -846,32 +811,14 @@
       end
 
       context 'when there are rules' do
-        context 'when the match policy is all' do
-          before { promotion.match_policy = 'all' }
-
-          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
+        context 'when all rules allow action on the line item' do
+          let(:rules) { [true_rule] }
+          it { is_expected.to be }
         end
 
-        context 'when the match policy is any', :silence_deprecations do
-          before { promotion.match_policy = 'any' }
-
-          context 'when at least one rule allows action on the line item' do
-            let(:rules) { [true_rule, false_rule] }
-            it { is_expected.to be }
-          end
-
-          context 'when no rules allow action on the line item' do
-            let(:rules) { [false_rule] }
-            it { is_expected.not_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
 
         context 'when the line item has an non-promotionable product' do