Merge pull request #6050 from mamhoff/configure-line-item-comparison-hooks-in-config

Move line_item_comparison_hooks config to Spree::Config

Author: tvdeyen

core/app/models/spree/order.rb

@@ -163,8 +163,24 @@ def self.find_by_param!(value)
     delegate :name, to: :bill_address, prefix: true, allow_nil: true
     alias_method :billing_name, :bill_address_name
 
-    class_attribute :line_item_comparison_hooks
-    self.line_item_comparison_hooks = Set.new
+    delegate :line_item_comparison_hooks, to: :class
+    class << self
+      def line_item_comparison_hooks=(value)
+        Spree::Config.line_item_comparison_hooks = value.to_a
+      end
+      line_item_hooks_deprecation_msg = "Use Spree::Config.line_item_comparison_hooks instead."
+      deprecate :line_item_comparison_hooks= => line_item_hooks_deprecation_msg, :deprecator => Spree.deprecator
+
+      def line_item_comparison_hooks
+        Spree::Config.line_item_comparison_hooks
+      end
+      deprecate line_item_comparison_hooks: line_item_hooks_deprecation_msg, deprecator: Spree.deprecator
+
+      def register_line_item_comparison_hook(hook)
+        Spree::Config.line_item_comparison_hooks << hook
+      end
+      deprecate register_line_item_comparison_hook: line_item_hooks_deprecation_msg, deprecator: Spree.deprecator
+    end
 
     scope :created_between, ->(start_date, end_date) { where(created_at: start_date..end_date) }
     scope :completed_between, ->(start_date, end_date) { where(completed_at: start_date..end_date) }
@@ -198,12 +214,6 @@ def self.not_canceled
       where.not(state: 'canceled')
     end
 
-    # Use this method in other gems that wish to register their own custom logic
-    # that should be called when determining if two line items are equal.
-    def self.register_line_item_comparison_hook(hook)
-      line_item_comparison_hooks.add(hook)
-    end
-
     # For compatiblity with Calculator::PriceSack
     def amount
       line_items.sum(&:amount)
@@ -356,7 +366,7 @@ def find_line_item_by_variant(variant, options = {})
     def line_item_options_match(line_item, options)
       return true unless options
 
-      line_item_comparison_hooks.all? { |hook|
+      Spree::Config.line_item_comparison_hooks.all? { |hook|
         send(hook, line_item, options)
       }
     end

core/app/models/spree/order_merger.rb

@@ -78,7 +78,7 @@ def merge!(other_order, user = nil)
     def find_matching_line_item(other_order_line_item)
       order.line_items.detect do |my_li|
         my_li.variant == other_order_line_item.variant &&
-          order.line_item_comparison_hooks.all? do |hook|
+          Spree::Config.line_item_comparison_hooks.all? do |hook|
             order.send(hook, my_li, other_order_line_item.serializable_hash)
           end
       end

core/lib/spree/app_configuration.rb

@@ -168,6 +168,13 @@ class AppConfiguration < Preferences::Configuration
     #   @return [String] template to use for layout on the frontend (default: +"spree/layouts/spree_application"+)
     preference :layout, :string, default: 'spree/layouts/spree_application'
 
+    # !@attribute [rw] line_item_comparison_hooks
+    #   @return [Array<Symbol>] An array of methods to call on {Spree::Order} to determine if a line item is equal to another
+    #   (default: +[]+)
+    #   @example
+    #   config.line_item_comparison_hooks << :my_custom_method
+    preference :line_item_comparison_hooks, :array, default: []
+
     # @!attribute [rw] logo
     #   @return [String] URL of logo used on frontend (default: +'logo/solidus.svg'+)
     preference :logo, :string, default: 'logo/solidus.svg'

core/spec/models/spree/order_merger_spec.rb

@@ -67,12 +67,7 @@ module Spree
 
     context "merging using extension-specific line_item_comparison_hooks" do
       before do
-        Spree::Order.register_line_item_comparison_hook(:foos_match)
-      end
-
-      after do
-        # reset to avoid test pollution
-        Spree::Order.line_item_comparison_hooks = Set.new
+        stub_spree_preferences(line_item_comparison_hooks: [:foos_match])
       end
 
       context "2 equal line items" do

core/spec/models/spree/order_spec.rb

@@ -13,6 +13,49 @@
     it { is_expected.to contain_exactly("user", "line_items", "shipments", "bill_address", "ship_address") }
   end
 
+  describe ".line_item_comparison_hooks=" do
+    it "allows setting the line item comparison hooks but emits a deprecation message" do
+      expect(Spree::Config).to receive(:line_item_comparison_hooks=).with([:foos_match])
+      expect(Spree.deprecator).to receive(:warn)
+        .with(
+          "line_item_comparison_hooks= is deprecated and will be removed from Solidus 5.0 (Use Spree::Config.line_item_comparison_hooks instead.)",
+          an_instance_of(Array)
+        )
+      described_class.line_item_comparison_hooks = [:foos_match]
+    end
+  end
+
+  describe ".line_item_comparison_hooks" do
+    before do |example|
+      stub_spree_preferences(line_item_comparison_hooks: [:foos_match])
+    end
+
+    it "allows getting the comparison hooks but emits a deprecation message" do
+      expect(Spree.deprecator).to receive(:warn)
+        .with(
+          "line_item_comparison_hooks is deprecated and will be removed from Solidus 5.0 (Use Spree::Config.line_item_comparison_hooks instead.)",
+          an_instance_of(Array)
+        )
+      described_class.line_item_comparison_hooks
+    end
+  end
+
+  describe ".register_line_item_comparison_hook" do
+    after do
+      expect(Spree::Config.line_item_comparison_hooks).to be_empty
+    end
+    it "allows setting the line item comparison hooks but emits a deprecation message" do
+      expect(Spree::Config.line_item_comparison_hooks).to receive(:<<).with(:foos_match)
+
+      expect(Spree.deprecator).to receive(:warn)
+        .with(
+          "register_line_item_comparison_hook is deprecated and will be removed from Solidus 5.0 (Use Spree::Config.line_item_comparison_hooks instead.)",
+          an_instance_of(Array)
+        )
+      Spree::Order.register_line_item_comparison_hook(:foos_match)
+    end
+  end
+
   context '#store' do
     it { is_expected.to respond_to(:store) }
 
@@ -741,12 +784,7 @@ def merge!(other_order, user = nil)
 
     context "match line item with options", partial_double_verification: false do
       before do
-        Spree::Order.register_line_item_comparison_hook(:foos_match)
-      end
-
-      after do
-        # reset to avoid test pollution
-        Spree::Order.line_item_comparison_hooks = Set.new
+        stub_spree_preferences(line_item_comparison_hooks: [:foos_match])
       end
 
       it "matches line item when options match" do

promotions/lib/solidus_promotions/engine.rb

@@ -40,10 +40,7 @@ class Engine < Rails::Engine
 
     initializer "solidus_promotions.spree_config", after: "spree.load_config_initializers" do
       Spree::Config.adjustment_promotion_source_types << "SolidusPromotions::Benefit"
-
-      Rails.application.config.to_prepare do
-        Spree::Order.line_item_comparison_hooks << :free_from_order_benefit?
-      end
+      Spree::Config.line_item_comparison_hooks << :free_from_order_benefit?
     end
 
     initializer "solidus_promotions.core.pub_sub", after: "spree.core.pub_sub" do |app|