From 6a0e60a19a533d95979b2c45055a4ec8f71ed3ae Mon Sep 17 00:00:00 2001
From: Alberto Vena <kennyadsl@gmail.com>
Date: Wed, 17 Mar 2021 11:13:33 +0100
Subject: [PATCH] Allow accessing preferences on models that do not have any
 set

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 b947015bc28c.

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.
---
 core/app/models/spree/base.rb                  | 10 +++++++++-
 core/spec/models/spree/stock/estimator_spec.rb | 18 ++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/core/app/models/spree/base.rb b/core/app/models/spree/base.rb
index 1cc27bf1b09..453abcb4625 100644
--- a/core/app/models/spree/base.rb
+++ b/core/app/models/spree/base.rb
@@ -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)
@@ -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
diff --git a/core/spec/models/spree/stock/estimator_spec.rb b/core/spec/models/spree/stock/estimator_spec.rb
index 7e8f11c4c23..aba4e7116bc 100644
--- a/core/spec/models/spree/stock/estimator_spec.rb
+++ b/core/spec/models/spree/stock/estimator_spec.rb
@@ -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
               [