Deprecate Promotion any Match Policy

CHANGELOG.md

@@ -43,9 +43,31 @@ of Solidus recommendations (that files are monkey patches; they don't use the
 you can place those files in `app/overrides/` and remove the `decorator`
 suffix.
 
+### Changes to the promotion system
+
+Promotions with a `match_policy` of `any` are deprecated. If you have promotions
+with such a match policy, try running the following rake task:
+
+```bash
+bin/rake solidus:split_promotions_with_any_match_policy
+```
+
+This will create separate promotions for each of the rules of your promotions with `any`
+match policy, which should have the same outcome for customers.
+
+Creating new promotions with `any` match policy is turned off by default. If you still want
+to create promotions like that (knowing they will not be supported in the future), you can
+set a temporary flag in your `config/initializers/spree.rb` file:
+
+```ruby
+# Allow creating new promotions with an `any` match policy. Unsupported in the future.
+config.allow_promotions_any_match_policy = true
+```
+
 ### Core
 
 - Add configuration option for `migration_path` [#4190](https://github.com/solidusio/solidus/pull/4190) ([SuperGoodSoft](https://github.com/supergoodsoft/))
+- Deprecate Promotion `any` Match Policy [#4304](https://github.com/solidusio/solidus/pull/4304) ([mamhoff](https://www.github.com/mamhoff))
 
 ## Solidus 3.1.5 (v3.1, 2021-12-20)
 

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

@@ -19,13 +19,15 @@
 
     <%= form_for @promotion, url: object_url, method: :put do |f| %>
       <fieldset class="no-border-top">
-        <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>
+        <% 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? %>

core/app/models/spree/promotion.rb

@@ -3,6 +3,7 @@
 module Spree
   class Promotion < Spree::Base
     MATCH_POLICIES = %w(all any)
+
     UNACTIVATABLE_ORDER_STATES = ["complete", "awaiting_return", "returned"]
 
     attr_reader :eligibility_errors
@@ -163,6 +164,12 @@ def eligible_rules(promotable, options = {})
         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

core/db/migrate/20220317165036_set_promotions_with_any_policy_to_all_if_possible.rb

@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+class SetPromotionsWithAnyPolicyToAllIfPossible < ActiveRecord::Migration[5.2]
+  def up
+    Spree::Promotion.where(match_policy: :any).includes(:promotion_rules).all.each do |promotion|
+      if promotion.promotion_rules.length <= 1
+        promotion.update!(match_policy: :all)
+      else
+        raise StandardError, <<~MSG
+          You have promotions with a match policy of any and more than one rule. Please
+          run `bundle exec rake solidus:split_promotions_with_any_match_policy`.
+        MSG
+      end
+    end
+  end
+
+  def down
+    # No-Op
+  end
+end

core/lib/spree/app_configuration.rb

@@ -60,6 +60,11 @@ 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
 
+    # @!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.
+    preference :allow_promotions_any_match_policy, :boolean, default: false
+
     # @!attribute [rw] guest_token_cookie_options
     #   @return [Hash] Add additional guest_token cookie options here (ie. domain or path)
     preference :guest_token_cookie_options, :hash, default: {}

core/lib/spree/testing_support/silence_deprecations.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+RSpec.configure do |config|
+  config.around(:each, silence_deprecations: true) do |example|
+    Spree::Deprecation.silence do
+      example.run
+    end
+  end
+end

core/lib/tasks/solidus/split_promotions_with_any_match_policy.rake

@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+namespace :solidus do
+  desc "Split Promotions with 'any' match policy"
+  task split_promotions_with_any_match_policy: :environment do
+    Spree::Promotion.where(match_policy: :any).includes(:promotion_rules).all.each do |promotion|
+      if promotion.promotion_rules.length <= 1
+        promotion.update!(match_policy: :all)
+      elsif promotion.active?
+        promotion.rules.map do |rule|
+          new_promotion = promotion.dup
+          new_promotion.promotion_rules = [rule]
+          new_promotion.match_policy = "all"
+          new_promotion.promotion_actions = promotion.actions.map do |action|
+            new_action = action.dup
+            if action.respond_to?(:calculator)
+              new_action.calculator = action.calculator.dup
+            end
+            new_action.promotion = new_promotion
+            new_action.save!
+            new_action
+          end
+          new_promotion.expires_at = promotion.expires_at
+          new_promotion.starts_at = Time.current
+          new_promotion.save!
+        end
+        promotion.update!(expires_at: Time.current)
+      end
+    end
+
+    Spree::Order.where(completed_at: nil).each { |order| Spree::PromotionHandler::Cart.new(order).activate }
+  end
+end

core/spec/lib/tasks/solidus/split_promotions_with_any_match_policy_spec.rb

@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+path = Spree::Core::Engine.root.join('lib/tasks/solidus/split_promotions_with_any_match_policy.rake')
+
+RSpec.describe 'solidus' do
+  describe 'split_promotions_with_any_match_policy' do
+    include_context(
+      'rake',
+      task_path: path,
+      task_name: 'solidus:split_promotions_with_any_match_policy'
+    )
+    let(:rule_1) { Spree::Promotion::Rules::Product.new }
+    let(:rule_2) { Spree::Promotion::Rules::Product.new }
+    let(:rule_3) { Spree::Promotion::Rules::Product.new }
+    let(:rule_4) { Spree::Promotion::Rules::Product.new }
+    let!(:promotion_with_all_match_policy) { create(:promotion, :with_action, promotion_rules: [rule_1, rule_2]) }
+    let!(:promotion_with_any_match_policy) { create(:promotion, :with_action, match_policy: "any", promotion_rules: [rule_3, rule_4]) }
+
+    subject { task.invoke }
+    it 'does not touch promotions with an all match policy' do
+      expect { task.invoke }.not_to change(promotion_with_all_match_policy, :expires_at)
+    end
+
+    it "replaces promotions with any match policy with new ones, one for each rule" do
+      expect { task.invoke }.to change { promotion_with_any_match_policy.reload.expires_at }.from(nil)
+
+      expect(Spree::Promotion.count).to eq(4)
+      expect(promotion_with_any_match_policy.expires_at).to be_present
+      expect(promotion_with_any_match_policy.rules).to be_empty
+      expect((Spree::Promotion.all - [promotion_with_all_match_policy, promotion_with_any_match_policy]).flat_map(&:rules)).to match_array([rule_3, rule_4])
+    end
+  end
+end

core/spec/models/spree/promotion_spec.rb

@@ -795,7 +795,7 @@
       end
     end
 
-    context "with 'any' match policy" do
+    context "with 'any' match policy", :silence_deprecations do
       let(:promotable) { double('Promotable') }
 
       before do
@@ -861,7 +861,7 @@
           end
         end
 
-        context 'when the match policy is any' do
+        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

core/spec/spec_helper.rb

@@ -8,6 +8,7 @@
 require 'rspec/core'
 
 require 'spree/testing_support/partial_double_verification'
+require 'spree/testing_support/silence_deprecations'
 require 'spree/testing_support/preferences'
 require 'spree/config'
 require 'with_model'

guides/source/developers/promotions/overview.html.md

@@ -49,6 +49,8 @@ Take note of the following promotion attributes:
 - `match_policy`: When set to `all`, all promotion rules must be met in order
   for the promotion to be eligible. When set to `any`, just one of the
   [promotion rules](#promotion-rules) must be met.
+  Using "any" is deprecated and will be removed in future Solidus versions.
+  You can create separate promotions for each rule you have in mind instead.
 - `path`: If the promotion is activated when the customer visits a URL, this
   value is the path for the URL.
 - `per_code_usage_limit`: Specifies how many times each code can be used before
@@ -86,8 +88,13 @@ By default, `Spree::Promotion`s have a `match_policy` value of `all`, meaning
 that all of the promotion rules on a promotion must be met before the promotion
 is eligible. However, this can be changed to `any`.
 
+Using "any" is deprecated and will be removed in future Solidus versions.
+You can create separate promotions for each rule you have in mind instead.
+
 An example of a typical promotion rule would be a minimum order total of $75
-USD or that a specific product is in the cart at checkout.
+USD or that a specific product is in the cart at checkout. In this case,
+create one promotion with the order total rule, and a separate one with the
+product rule.
 
 For a list of available rule types and more information, see the
 [Promotion rules][promotion-rules] article.

guides/source/developers/promotions/promotion-rules.html.md

@@ -54,6 +54,9 @@ Administrators can change the match policy when adding or editing a promotion.
 By default, promotions use the "Match all of these rules" setting, but they can
 be changed to use "Match any of these rules".
 
+Using "any" is deprecated and will be removed in future Solidus versions.
+You can create separate promotions for each rule you have in mind instead.
+
 ## Register a custom promotion rule
 
 You can create a custom promotion rule by creating a new class that inherits