Skip to content

Commit

Permalink
Allow accessing preferences on models that do not have any set
Browse files Browse the repository at this point in the history
We may have models that are supposed to have preferences but are not
defining them explicitly because they are not needed.

For example, when defining a custom calculator that does not need any
preference. Core code expects that preferences still responds with a
Hash instead of nil because that's how it worked before b947015.

That commit is needed because otherwise Rails would serialize the object
differently on models that do not use preferences, because seralize is
now lazy executed. Example of the wrong serialization without it:

       expected: #<Spree::Address id: 4, firstname: "John", lastname: "Von Doe", address1: "10 Lovely Street", address...00 +0000", updated_at: "2021-03-17 08:09:00.244459000 +0000", name: "John Von Doe", preferences: {}>
            got: #<Spree::Address id: 4, firstname: "John", lastname: "Von Doe", address1: "10 Lovely Street", address...0 +0000", updated_at: "2021-03-17 08:09:00.244459000 +0000", name: "John Von Doe", preferences: nil>

This commit introduces a hack to have both things. When preferences is
empty at database level, it's safe to always return a Hash, because
that's how the data would have been deserialized anyway. This allows
us to call `preferences[:something]` on models that do not explicitly
define any preference.
  • Loading branch information
kennyadsl committed Mar 17, 2021
1 parent a9a7a95 commit 6a0e60a
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
10 changes: 9 additions & 1 deletion core/app/models/spree/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ class Spree::Base < ActiveRecord::Base
include Spree::Core::Permalinks
include Spree::RansackableAttributes

def preferences
read_attribute(:preferences) || self.class.preferences_coder_class.new
end

def initialize_preference_defaults
if has_attribute?(:preferences)
self.preferences = default_preferences.merge(preferences)
Expand All @@ -16,11 +20,15 @@ def initialize_preference_defaults
def self.preference(*args)
# after_initialize can be called multiple times with the same symbol, it
# will only be called once on initialization.
serialize :preferences, Hash
serialize :preferences, preferences_coder_class
after_initialize :initialize_preference_defaults
super
end

def self.preferences_coder_class
Hash
end

self.abstract_class = true

# Provides a scope that should be included any time products
Expand Down
18 changes: 18 additions & 0 deletions core/spec/models/spree/stock/estimator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,24 @@ module Stock
context "general shipping methods" do
before { Spree::ShippingMethod.all.each(&:destroy) }

context 'with a custom shipping calculator with no preference' do
class Spree::Calculator::Shipping::NoPreferences < Spree::ShippingCalculator
def compute_package(_package)
# no op
end
end

let!(:shipping_methods) do
[
create(:shipping_method, calculator: Spree::Calculator::Shipping::NoPreferences.new)
]
end

it 'does not raise an error' do
expect { subject.shipping_rates(package) }.not_to raise_error
end
end

context 'with two shipping methods of different cost' do
let!(:shipping_methods) do
[
Expand Down

0 comments on commit 6a0e60a

Please sign in to comment.