Skip to content

Commit

Permalink
Require that load_defaults is called
Browse files Browse the repository at this point in the history
After all the initializers are run, we check that the application called
`#load_defaults` method for every Solidus engine and, otherwise, show a
deprecation message. This is done as a security check to ensure that
users are conscious about which Solidus version defaults are using. If
they didn't call it, it would default to the latest Solidius version,
which could break their code if they didn't update through the
`solidus:update` generator.
  • Loading branch information
waiting-for-dev committed Jun 1, 2021
1 parent 4495d56 commit 770fcb6
Show file tree
Hide file tree
Showing 8 changed files with 91 additions and 24 deletions.
2 changes: 1 addition & 1 deletion api/spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
DummyApp.setup(
gem_root: File.expand_path('..', __dir__),
lib_name: 'solidus_api'
)
) { Spree::Api::Config.load_defaults(Spree.solidus_version) }

require 'rails-controller-testing'
require 'rspec/rails'
Expand Down
5 changes: 4 additions & 1 deletion backend/spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
DummyApp.setup(
gem_root: File.expand_path('..', __dir__),
lib_name: 'solidus_backend'
)
) do
Spree::Backend::Config.load_defaults(Spree.solidus_version)
Spree::Api::Config.load_defaults(Spree.solidus_version)
end

require 'rails-controller-testing'
require 'rspec-activemodel-mocks'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
# Solidus version.
#
# It allows you to enable them one by one while you adapt your application.
# When you're done with all of them, you can safely remove this file.
# When you're done with all of them, you can safely remove this file and add
# the updated `load_defaults` calls to the top of the config blocks in your
# Solidus main initializer.
Spree.config do |config|
<%= @core_changes %>
end
Expand Down
7 changes: 7 additions & 0 deletions core/lib/spree/core/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,13 @@ class Engine < ::Rails::Engine
app.config.action_mailer.preview_path = "{#{original_preview_path},#{solidus_preview_path}}"
ActionMailer::Base.preview_path = app.config.action_mailer.preview_path
end

config.after_initialize do
Spree::Config.check_load_defaults_called('Spree::Config')
Spree::Frontend::Config.check_load_defaults_called('Spree::Frontend::Config') if defined?(Spree::Frontend::Engine)
Spree::Backend::Config.check_load_defaults_called('Spree::Backend::Config') if defined?(Spree::Backend::Engine)
Spree::Api::Config.check_load_defaults_called('Spree::Api::Config') if defined?(Spree::Api::Engine)
end
end
end
end
17 changes: 17 additions & 0 deletions core/lib/spree/preferences/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,18 +38,35 @@ class Configuration
# defaults. Set via {#load_defaults}
attr_reader :loaded_defaults

attr_reader :load_defaults_called

def initialize
@loaded_defaults = Spree.solidus_version
@load_defaults_called = false
end

# @param [String] Solidus version from which take defaults when not
# overriden.
# @see #load_defaults
def load_defaults(version)
@loaded_defaults = version
@load_defaults_called = true
reset
end

def check_load_defaults_called(instance_constant_name = nil)
return if load_defaults_called

target_name = instance_constant_name || "#{self.class.name}.new"
Spree::Deprecation.warn <<~MSG
Please, indicate wich Solidus version defaults the application must use.
The method `#{target_name}.load_defaults` must be called from the Solidus initializer. E.g.:
config.load_defaults('#{Spree.solidus_version}')
MSG
end

# @yield [config] Yields this configuration object to a block
def configure
yield(self)
Expand Down
4 changes: 3 additions & 1 deletion core/lib/spree/testing_support/dummy_app.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,10 @@ module ApplicationHelper

# @private
module DummyApp
def self.setup(gem_root:, lib_name:, auto_migrate: true)
def self.setup(gem_root:, lib_name:, auto_migrate: true, &block)
ENV["LIB_NAME"] = lib_name
DummyApp::Application.config.root = File.join(gem_root, 'spec', 'dummy')
block.call if block_given?

DummyApp::Application.initialize!

Expand Down Expand Up @@ -119,6 +120,7 @@ class Application < ::Rails::Application

Spree.user_class = 'Spree::LegacyUser'
Spree.config do |config|
config.load_defaults(Spree.solidus_version)
config.mails_from = "[email protected]"

if ENV['DISABLE_ACTIVE_STORAGE']
Expand Down
71 changes: 52 additions & 19 deletions core/spec/models/spree/preferences/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,52 +3,85 @@
require 'rails_helper'

RSpec.describe Spree::Preferences::Configuration, type: :model do
before :all do
class AppConfig < Spree::Preferences::Configuration
let(:config) do
Class.new(Spree::Preferences::Configuration) do
preference :color, :string, default: :blue
preference :foo, :boolean, default: by_version(true, "3.0" => false)
end
@config = AppConfig.new
end.new
end

it "has named methods to access preferences" do
@config.color = 'orange'
expect(@config.color).to eq 'orange'
config.color = 'orange'
expect(config.color).to eq 'orange'
end

it "uses [ ] to access preferences" do
@config[:color] = 'red'
expect(@config[:color]).to eq 'red'
config[:color] = 'red'
expect(config[:color]).to eq 'red'
end

it "uses set/get to access preferences" do
@config.set(color: 'green')
expect(@config.get(:color)).to eq 'green'
config.set(color: 'green')
expect(config.get(:color)).to eq 'green'
end

it "allows defining different defaults depending on the Solidus version" do
@config.load_defaults 2.1
config.load_defaults 2.1

expect(@config.get(:foo)).to be(true)
expect(config.get(:foo)).to be(true)

@config.load_defaults 3.1
config.load_defaults 3.1

expect(@config.get(:foo)).to be(false)
expect(config.get(:foo)).to be(false)
end

describe '#load_defaults' do
it 'changes loaded_defaults' do
@config.load_defaults '2.1'
config.load_defaults '2.1'

expect(@config.loaded_defaults).to eq('2.1')
expect(config.loaded_defaults).to eq('2.1')

@config.load_defaults '3.1'
config.load_defaults '3.1'

expect(@config.loaded_defaults).to eq('3.1')
expect(config.loaded_defaults).to eq('3.1')
end

it 'returns updated preferences' do
expect(@config.load_defaults('2.1')).to eq(foo: true, color: :blue)
expect(config.load_defaults('2.1')).to eq(foo: true, color: :blue)
end

it 'sets load_defaults_called flag to true' do
expect(config.load_defaults_called).to be(false)

config.load_defaults '3.1'

expect(config.load_defaults_called).to be(true)
end
end

describe '#check_load_defaults_called' do
context 'when load_defaults_called is true' do
it 'does not emit a warning' do
config.load_defaults '3.1'

expect(Spree::Deprecation).not_to receive(:warn)

config.check_load_defaults_called
end
end

context 'when load_defaults_called is false' do
it 'emits a warning' do
expect(Spree::Deprecation).to receive(:warn).with(/load_defaults.*must be called/)

config.check_load_defaults_called
end

it 'includes constant name in the message when given' do
expect(Spree::Deprecation).to receive(:warn).with(/Spree::Config.load_defaults/, any_args)

config.check_load_defaults_called('Spree::Config')
end
end
end
end
5 changes: 4 additions & 1 deletion frontend/spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
DummyApp.setup(
gem_root: File.expand_path('..', __dir__),
lib_name: 'solidus_frontend'
)
) do
Spree::Api::Config.load_defaults(Spree.solidus_version)
Spree::Frontend::Config.load_defaults(Spree.solidus_version)
end

require 'rails-controller-testing'
require 'rspec/rails'
Expand Down

0 comments on commit 770fcb6

Please sign in to comment.