Skip to content

Commit

Permalink
Move line_item_comparison_hooks config to Spree::Config
Browse files Browse the repository at this point in the history
Setting line item comparison hooks in a `config.to_prepare` block will
currently autoload the Spree::Order model (and subsequently any model
referenced in that class definition), slowing down the boot process in
development mode (when we want our app/ directory to be autoloaded.

This moves setting those hooks to Spree::Config, which is already loaded
by the time initializers run. This allows us to move code out of
`config.to_prepare` hooks, and stop the order class from being
inadvertently eager loaded.
  • Loading branch information
mamhoff committed Feb 10, 2025
1 parent efe8b97 commit 81bf6f6
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 26 deletions.
28 changes: 19 additions & 9 deletions core/app/models/spree/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/order_merger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions core/lib/spree/app_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
7 changes: 1 addition & 6 deletions core/spec/models/spree/order_merger_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
50 changes: 44 additions & 6 deletions core/spec/models/spree/order_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }

Expand Down Expand Up @@ -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
Expand Down
5 changes: 1 addition & 4 deletions promotions/lib/solidus_promotions/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down

0 comments on commit 81bf6f6

Please sign in to comment.