Refactor Promotions Environment Configuration

CHANGELOG.md

@@ -110,6 +110,7 @@ return_item.process_inventory_unit!
 
 ### Core
 
+- Refactor Promotions Environment Configuration [#3239](https://github.com/solidusio/solidus/pull/3239) ([kennyadsl](https://github.com/kennyadsl))
 - Add preferred_reimbursement_type_id as permitted attributes for ReturnAuthorization [#3215](https://github.com/solidusio/solidus/pull/3215) ([ibudiallo](https://github.com/ibudiallo))
 - Add OriginalPayment reimbursement type in seeds [#3213](https://github.com/solidusio/solidus/pull/3213) ([kennyadsl](https://github.com/kennyadsl))
 - Fixes for discard 1.1.0 [#3202](https://github.com/solidusio/solidus/pull/3202) ([kennyadsl](https://github.com/kennyadsl))

core/lib/spree/app_configuration.rb

@@ -475,6 +475,27 @@ def events
 
     def environment
       @environment ||= Spree::Core::Environment.new(self).tap do |env|
+        env.calculators.promotion_actions_create_adjustments = %w[
+          Spree::Calculator::FlatPercentItemTotal
+          Spree::Calculator::FlatRate
+          Spree::Calculator::FlexiRate
+          Spree::Calculator::TieredPercent
+          Spree::Calculator::TieredFlatRate
+        ]
+
+        env.calculators.promotion_actions_create_item_adjustments = %w[
+          Spree::Calculator::DistributedAmount
+          Spree::Calculator::FlatRate
+          Spree::Calculator::FlexiRate
+          Spree::Calculator::PercentOnLineItem
+          Spree::Calculator::TieredPercent
+        ]
+
+        env.calculators.promotion_actions_create_quantity_adjustments = %w[
+          Spree::Calculator::PercentOnLineItem
+          Spree::Calculator::FlatRate
+        ]
+
         env.calculators.shipping_methods = %w[
           Spree::Calculator::Shipping::FlatPercentItemTotal
           Spree::Calculator::Shipping::FlatRate
@@ -487,65 +508,42 @@ def environment
           Spree::Calculator::DefaultTax
         ]
 
-        env.stock_splitters = %w[
-          Spree::Stock::Splitter::ShippingCategory
-          Spree::Stock::Splitter::Backordered
-        ]
-
         env.payment_methods = %w[
           Spree::PaymentMethod::BogusCreditCard
           Spree::PaymentMethod::SimpleBogusCreditCard
           Spree::PaymentMethod::StoreCredit
           Spree::PaymentMethod::Check
         ]
 
-        env.promotions = Spree::Promo::Environment.new.tap do |promos|
-          promos.rules = %w[
-            Spree::Promotion::Rules::ItemTotal
-            Spree::Promotion::Rules::Product
-            Spree::Promotion::Rules::User
-            Spree::Promotion::Rules::FirstOrder
-            Spree::Promotion::Rules::UserLoggedIn
-            Spree::Promotion::Rules::OneUsePerUser
-            Spree::Promotion::Rules::Taxon
-            Spree::Promotion::Rules::NthOrder
-            Spree::Promotion::Rules::OptionValue
-            Spree::Promotion::Rules::FirstRepeatPurchaseSince
-            Spree::Promotion::Rules::UserRole
-            Spree::Promotion::Rules::Store
-          ]
-
-          promos.actions = %w[
-            Spree::Promotion::Actions::CreateAdjustment
-            Spree::Promotion::Actions::CreateItemAdjustments
-            Spree::Promotion::Actions::CreateQuantityAdjustments
-            Spree::Promotion::Actions::FreeShipping
-          ]
-
-          promos.shipping_actions = %w[
-            Spree::Promotion::Actions::FreeShipping
-          ]
-        end
+        env.promotions.rules = %w[
+          Spree::Promotion::Rules::ItemTotal
+          Spree::Promotion::Rules::Product
+          Spree::Promotion::Rules::User
+          Spree::Promotion::Rules::FirstOrder
+          Spree::Promotion::Rules::UserLoggedIn
+          Spree::Promotion::Rules::OneUsePerUser
+          Spree::Promotion::Rules::Taxon
+          Spree::Promotion::Rules::NthOrder
+          Spree::Promotion::Rules::OptionValue
+          Spree::Promotion::Rules::FirstRepeatPurchaseSince
+          Spree::Promotion::Rules::UserRole
+          Spree::Promotion::Rules::Store
+        ]
 
-        env.calculators.promotion_actions_create_adjustments = %w[
-          Spree::Calculator::FlatPercentItemTotal
-          Spree::Calculator::FlatRate
-          Spree::Calculator::FlexiRate
-          Spree::Calculator::TieredPercent
-          Spree::Calculator::TieredFlatRate
+        env.promotions.actions = %w[
+          Spree::Promotion::Actions::CreateAdjustment
+          Spree::Promotion::Actions::CreateItemAdjustments
+          Spree::Promotion::Actions::CreateQuantityAdjustments
+          Spree::Promotion::Actions::FreeShipping
         ]
 
-        env.calculators.promotion_actions_create_item_adjustments = %w[
-          Spree::Calculator::DistributedAmount
-          Spree::Calculator::FlatRate
-          Spree::Calculator::FlexiRate
-          Spree::Calculator::PercentOnLineItem
-          Spree::Calculator::TieredPercent
+        env.promotions.shipping_actions = %w[
+          Spree::Promotion::Actions::FreeShipping
         ]
 
-        env.calculators.promotion_actions_create_quantity_adjustments = %w[
-          Spree::Calculator::PercentOnLineItem
-          Spree::Calculator::FlatRate
+        env.stock_splitters = %w[
+          Spree::Stock::Splitter::ShippingCategory
+          Spree::Stock::Splitter::Backordered
         ]
       end
     end

core/lib/spree/core.rb

@@ -59,6 +59,7 @@ class GatewayError < RuntimeError; end
 require 'spree/core/class_constantizer'
 require 'spree/core/environment_extension'
 require 'spree/core/environment/calculators'
+require 'spree/core/environment/promotions'
 require 'spree/core/environment'
 require 'spree/promo/environment'
 require 'spree/migrations'

core/lib/spree/core/class_constantizer.rb

@@ -22,6 +22,10 @@ def concat(klasses)
 
         delegate :clear, :empty?, to: :@collection
 
+        def delete(object)
+          @collection.delete(object.to_s)
+        end
+
         def each
           @collection.each do |klass|
             yield klass.constantize

core/lib/spree/core/environment.rb

@@ -15,7 +15,7 @@ class Environment
       def initialize(spree_config)
         @calculators = Calculators.new
         @preferences = spree_config
-        @promotions = Spree::Promo::Environment.new
+        @promotions = Promotions.new
       end
     end
   end

core/lib/spree/core/environment/promotions.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+module Spree
+  module Core
+    class Environment
+      class Promotions
+        include EnvironmentExtension
+
+        add_class_set :rules
+        add_class_set :actions
+        add_class_set :shipping_actions
+      end
+    end
+  end
+end

core/lib/spree/promo/environment.rb

@@ -2,12 +2,11 @@
 
 module Spree
   module Promo
-    class Environment
-      include Core::EnvironmentExtension
-
-      add_class_set :rules
-      add_class_set :actions
-      add_class_set :shipping_actions
-    end
+    Environment =
+      ActiveSupport::Deprecation::DeprecatedConstantProxy.new(
+        'Spree::Promo::Environment',
+        'Spree::Core::Environment::Promotions',
+        Spree::Deprecation
+      )
   end
 end

core/spec/lib/spree/app_configuration_spec.rb

@@ -53,6 +53,81 @@
     end
   end
 
+  describe '#environment' do
+    class DummyClass; end;
+
+    subject(:environment) { prefs.environment }
+    it { is_expected.to be_a Spree::Core::Environment }
+
+    shared_examples "working preferences set" do
+      it "allows adding new items" do
+        preferences_set << DummyClass
+        expect(preferences_set).to include DummyClass
+        preferences_set.delete DummyClass
+      end
+    end
+
+    context '.payment_methods' do
+      subject(:preferences_set) { environment.payment_methods }
+      it_should_behave_like "working preferences set"
+    end
+
+    context '.stock_splitters' do
+      subject(:preferences_set) { environment.stock_splitters }
+      it_should_behave_like "working preferences set"
+    end
+
+    context '.calculators' do
+      subject(:calculators) { environment.calculators }
+      it { is_expected.to be_a Spree::Core::Environment::Calculators }
+
+      context '.calculators.shipping_methods' do
+        subject(:preferences_set) { calculators.shipping_methods }
+        it_should_behave_like "working preferences set"
+      end
+
+      context '.calculators.tax_rates' do
+        subject(:preferences_set) { calculators.tax_rates }
+        it_should_behave_like "working preferences set"
+      end
+
+      context '.calculators.promotion_actions_create_adjustments' do
+        subject(:preferences_set) { calculators.promotion_actions_create_adjustments }
+        it_should_behave_like "working preferences set"
+      end
+
+      context '.calculators.promotion_actions_create_item_adjustments' do
+        subject(:preferences_set) { calculators.promotion_actions_create_item_adjustments }
+        it_should_behave_like "working preferences set"
+      end
+
+      context '.calculators.promotion_actions_create_quantity_adjustments' do
+        subject(:preferences_set) { calculators.promotion_actions_create_quantity_adjustments }
+        it_should_behave_like "working preferences set"
+      end
+    end
+
+    context '.promotions' do
+      subject(:promotions) { environment.promotions }
+      it { is_expected.to be_a Spree::Core::Environment::Promotions }
+
+      context '.promotions.rules' do
+        subject(:preferences_set) { promotions.rules }
+        it_should_behave_like "working preferences set"
+      end
+
+      context '.promotions.actions' do
+        subject(:preferences_set) { promotions.actions }
+        it_should_behave_like "working preferences set"
+      end
+
+      context '.promotions.shipping_actions' do
+        subject(:preferences_set) { promotions.shipping_actions }
+        it_should_behave_like "working preferences set"
+      end
+    end
+  end
+
   it 'has a default admin VAT location with nil values by default' do
     expect(prefs.admin_vat_location).to eq(Spree::Tax::TaxLocation.new)
     expect(prefs.admin_vat_location.state_id).to eq(nil)

core/spec/lib/spree/core/class_constantizer_spec.rb

@@ -68,4 +68,20 @@ def self.reload
       end
     end
   end
+
+  describe "#delete" do
+    before do
+      set << ClassConstantizerTest::ClassA
+    end
+
+    it "can delete by string" do
+      set.delete "ClassConstantizerTest::ClassA"
+      expect(set).not_to include(ClassConstantizerTest::ClassA)
+    end
+
+    it "can delete by class" do
+      set.delete ClassConstantizerTest::ClassA
+      expect(set).not_to include(ClassConstantizerTest::ClassA)
+    end
+  end
 end

core/spec/lib/spree/promo/environment_spec.rb

@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'spree/core/environment/promotions'
+require 'spree/promo/environment'
+
+RSpec.describe 'Spree::Promo::Environment' do
+  it 'is deprecated' do
+    expect(Spree::Deprecation).to receive(:warn)
+
+    Spree::Promo::Environment.new
+  end
+
+  context 'when customized' do
+    after do
+      # This is needed to cleanup classes after the following specs run.
+      # They will add two new methods but we don't want them to be defined
+      # on the described class for specs that will run later.
+      Spree::Core::Environment::Promotions.remove_method :custom_rules, :custom_rules=
+    end
+
+    context 'with class_eval' do
+      it 'raises a deprecation warning but keep changes' do
+        expect(Spree::Deprecation).to receive(:warn)
+
+        Spree::Promo::Environment.class_eval do
+          add_class_set :custom_rules
+        end
+
+        promo_environment_instance = Spree::Core::Environment::Promotions.new
+        expect(promo_environment_instance).to respond_to(:custom_rules)
+        expect(promo_environment_instance).to respond_to(:custom_rules=)
+      end
+    end
+
+    context 'with prepend' do
+      it 'raises a deprecation warning but keep changes' do
+        expect(Spree::Deprecation).to receive(:warn)
+
+        module CustomRules
+          def self.prepended(base)
+            base.add_class_set :custom_rules
+          end
+        end
+        Spree::Promo::Environment.prepend CustomRules
+
+        promo_environment_instance = Spree::Core::Environment::Promotions.new
+        expect(promo_environment_instance).to respond_to(:custom_rules)
+        expect(promo_environment_instance).to respond_to(:custom_rules=)
+      end
+    end
+  end
+end