Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow storing static preferences using string class names #4858

Merged
merged 1 commit into from
Jan 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions core/lib/spree/core/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ class Engine < ::Rails::Engine

config.after_initialize do
Spree::Config.check_load_defaults_called('Spree::Config')
Spree::Config.static_model_preferences.validate!
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if you set the preference value after the app is initialized? Do you need to call validate! manually as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the app is initialized you could change the model configuration (e.g. adding or removing a preference) not the static preference value, but I regarded that as infrequent.

If we want to do it at every request in development we could move it to a to_prepare block, would you prefer that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, they live in memory and we can't change their value at runtime. At least, not easily and it's not an expected usage.

end

config.after_initialize do
Expand Down
33 changes: 25 additions & 8 deletions core/lib/spree/preferences/static_model_preferences.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,8 @@ class Definition
attr_reader :preferences

def initialize(klass, hash)
hash = hash.symbolize_keys
hash.keys.each do |key|
if !klass.defined_preferences.include?(key)
raise "Preference #{key.inspect} is not defined on #{klass}"
end
end
@preferences = hash
@klass = klass
@preferences = hash.symbolize_keys
end

def fetch(key, &block)
Expand All @@ -27,6 +22,8 @@ def []=(key, value)
def to_hash
@preferences.deep_dup
end

delegate :keys, to: :@preferences
end

def initialize
Expand All @@ -36,12 +33,32 @@ def initialize
end

def add(klass, name, preferences)
@store[klass.to_s][name] = Definition.new(klass, preferences)
@store[klass.to_s][name] = Definition.new(klass.to_s, preferences)
end

def for_class(klass)
@store[klass.to_s]
end

def validate!
@store.keys.map(&:constantize).each do |klass|
validate_for_class!(klass)
end
end

private

def validate_for_class!(klass)
for_class(klass).each do |name, preferences|
klass_keys = klass.defined_preferences.map(&:to_s)
extra_keys = preferences.keys.map(&:to_s) - klass_keys
next if extra_keys.empty?

raise \
"Unexpected keys found for #{klass} under #{name}: #{extra_keys.sort.join(', ')} " \
"(expected keys: #{klass_keys.sort.join(', ')})"
end
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,6 @@ module Spree
expect(definitions['my_definition'].fetch(:color)).to eq("blue")
end

it "errors assigning invalid preferences" do
expect {
subject.add(preference_class, 'my_definition', { ice_cream: 'chocolate' })
}.to raise_error(/\APreference :ice_cream is not defined/)
end

context "with stored definitions" do
before do
subject.add(preference_class, 'light', { color: 'white' })
Expand Down Expand Up @@ -83,5 +77,18 @@ module Spree
expect(subject.for_class(other_class)).to be_empty
end
end

describe '.validate!' do
it "errors assigning invalid preferences" do
stub_const("SomeClass", preference_class)
subject.add(preference_class, 'my_definition', { ice_cream: 'chocolate', spoon: true })

expect {
subject.validate!
}.to raise_error(
/\AUnexpected keys found for SomeClass under my_definition: ice_cream, spoon \(expected keys: color\)/
)
end
end
end
end