From 7982297a3e87c35d8adc3135297dce76f2690626 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Burgos?= Date: Fri, 27 Jul 2018 10:04:30 +0200 Subject: [PATCH 1/4] Allow definition of class name at setting level Prior to this, only one class name could be used for all settings within a Rails model. --- lib/rails-settings/base.rb | 34 ++++++++++++------ lib/rails-settings/configuration.rb | 41 ++++++++++++++++------ lib/rails-settings/setting_object.rb | 6 ++-- spec/configuration_spec.rb | 52 +++++++++++++++++++--------- spec/settings_spec.rb | 8 ++--- spec/spec_helper.rb | 7 ++-- 6 files changed, 101 insertions(+), 47 deletions(-) diff --git a/lib/rails-settings/base.rb b/lib/rails-settings/base.rb index a04dc28b..3b64d867 100644 --- a/lib/rails-settings/base.rb +++ b/lib/rails-settings/base.rb @@ -6,17 +6,13 @@ def self.included(base) :as => :target, :autosave => true, :dependent => :delete_all, - :class_name => self.setting_object_class_name + :class_name => "RailsSettings::SettingObject" def settings(var) raise ArgumentError unless var.is_a?(Symbol) - raise ArgumentError.new("Unknown key: #{var}") unless self.class.default_settings[var] + raise ArgumentError.new("Unknown key: #{var}") unless self.class.setting_keys[var] - if RailsSettings.can_protect_attributes? - setting_objects.detect { |s| s.var == var.to_s } || setting_objects.build({ :var => var.to_s }, :without_protection => true) - else - setting_objects.detect { |s| s.var == var.to_s } || setting_objects.build(:var => var.to_s, :target => self) - end + fetch_settings_record(var).becomes(self.class.setting_keys[var][:class_name].constantize) end def settings=(value) @@ -36,11 +32,27 @@ def settings?(var=nil) end def to_settings_hash - settings_hash = self.class.default_settings.dup - settings_hash.each do |var, vals| - settings_hash[var] = settings_hash[var].merge(settings(var.to_sym).value) + Hash[self.class.setting_keys.map do |key, options| + [key, options[:default_value].merge(settings(key.to_sym).value)] + end] + end + + private + + def fetch_settings_record(var) + find_settings_record(var) or build_settings_record(var) + end + + def find_settings_record(var) + setting_objects.detect { |s| s.var == var.to_s } + end + + def build_settings_record(var) + if RailsSettings.can_protect_attributes? + setting_objects.build({ :var => var.to_s }, :without_protection => true) + else + setting_objects.build(:var => var.to_s, :target => self) end - settings_hash end end end diff --git a/lib/rails-settings/configuration.rb b/lib/rails-settings/configuration.rb index 7a275e92..3dec4d8b 100644 --- a/lib/rails-settings/configuration.rb +++ b/lib/rails-settings/configuration.rb @@ -1,32 +1,53 @@ module RailsSettings class Configuration def initialize(*args, &block) - options = args.extract_options! + @default_options = args.extract_options! + validate_options @default_options klass = args.shift keys = args raise ArgumentError unless klass @klass = klass - @klass.class_attribute :default_settings, :setting_object_class_name - @klass.default_settings = {} - @klass.setting_object_class_name = options[:class_name] || 'RailsSettings::SettingObject' + @klass.class_attribute :setting_keys + @klass.setting_keys = {} if block_given? yield(self) else - keys.each do |k| - key(k) - end + keys.each { |k| key(k) } end - raise ArgumentError.new('has_settings: No keys defined') if @klass.default_settings.blank? + raise ArgumentError.new('has_settings: No keys defined') if @klass.setting_keys.empty? end def key(name, options={}) + validate_name name + validate_options options + options = @default_options.merge(options) + + @klass.setting_keys[name] = { + :default_value => (options[:defaults] || {}).stringify_keys.freeze, + :class_name => (options[:class_name] || 'RailsSettings::SettingObject') + } + end + + private + + def validate_name(name) raise ArgumentError.new("has_settings: Symbol expected, but got a #{name.class}") unless name.is_a?(Symbol) - raise ArgumentError.new("has_settings: Option :defaults expected, but got #{options.keys.join(', ')}") unless options.blank? || (options.keys == [:defaults]) - @klass.default_settings[name] = (options[:defaults] || {}).stringify_keys.freeze + end + + def validate_options(options) + valid_options = [:defaults, :class_name] + options.each do |key, value| + unless valid_options.include?(key) + raise ArgumentError.new("has_settings: Invalid option #{key}") + end + end + if options[:class_name] && !options[:class_name].constantize.ancestors.include?(RailsSettings::SettingObject) + raise ArgumentError.new("has_settings: #{options[:class_name]} must be a subclass of RailsSettings::SettingObject") + end end end end diff --git a/lib/rails-settings/setting_object.rb b/lib/rails-settings/setting_object.rb index 2397ed18..4ebb0382 100644 --- a/lib/rails-settings/setting_object.rb +++ b/lib/rails-settings/setting_object.rb @@ -8,7 +8,7 @@ class SettingObject < ActiveRecord::Base validate do errors.add(:value, "Invalid setting value") unless value.is_a? Hash - unless _target_class.default_settings[var.to_sym] + unless _target_class.setting_keys[var.to_sym] errors.add(:var, "#{var} is not defined!") end end @@ -55,7 +55,7 @@ def sanitize_for_mass_assignment(attributes, role = nil) private def _get_value(name) if value[name].nil? - _target_class.default_settings[var.to_sym][name] + _target_class.setting_keys[var.to_sym][:default_value][name] else value[name] end @@ -78,7 +78,7 @@ def _target_class end def _setting?(method_name) - _target_class.default_settings[var.to_sym].keys.include?(method_name.to_s) + _target_class.setting_keys[var.to_sym][:default_value].keys.include?(method_name.to_s) end end end diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index 63089d51..eb57644d 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -8,28 +8,32 @@ class Dummy it "should define single key" do Configuration.new(Dummy, :dashboard) - expect(Dummy.default_settings).to eq({ :dashboard => {} }) - expect(Dummy.setting_object_class_name).to eq('RailsSettings::SettingObject') + expect(Dummy.setting_keys[:dashboard][:default_value]).to eq({}) + expect(Dummy.setting_keys[:dashboard][:class_name]).to eq('RailsSettings::SettingObject') end it "should define multiple keys" do Configuration.new(Dummy, :dashboard, :calendar) - expect(Dummy.default_settings).to eq({ :dashboard => {}, :calendar => {} }) - expect(Dummy.setting_object_class_name).to eq('RailsSettings::SettingObject') + expect(Dummy.setting_keys[:dashboard][:default_value]).to eq({}) + expect(Dummy.setting_keys[:calendar][:default_value]).to eq({}) + expect(Dummy.setting_keys[:dashboard][:class_name]).to eq('RailsSettings::SettingObject') + expect(Dummy.setting_keys[:calendar][:class_name]).to eq('RailsSettings::SettingObject') end it "should define single key with class_name" do - Configuration.new(Dummy, :dashboard, :class_name => 'MyClass') - expect(Dummy.default_settings).to eq({ :dashboard => {} }) - expect(Dummy.setting_object_class_name).to eq('MyClass') + Configuration.new(Dummy, :dashboard, :class_name => 'ProjectSettingObject') + expect(Dummy.setting_keys[:dashboard][:default_value]).to eq({}) + expect(Dummy.setting_keys[:dashboard][:class_name]).to eq('ProjectSettingObject') end it "should define multiple keys with class_name" do - Configuration.new(Dummy, :dashboard, :calendar, :class_name => 'MyClass') + Configuration.new(Dummy, :dashboard, :calendar, :class_name => 'ProjectSettingObject') - expect(Dummy.default_settings).to eq({ :dashboard => {}, :calendar => {} }) - expect(Dummy.setting_object_class_name).to eq('MyClass') + expect(Dummy.setting_keys[:dashboard][:default_value]).to eq({}) + expect(Dummy.setting_keys[:calendar][:default_value]).to eq({}) + expect(Dummy.setting_keys[:dashboard][:class_name]).to eq('ProjectSettingObject') + expect(Dummy.setting_keys[:calendar][:class_name]).to eq('ProjectSettingObject') end it "should define using block" do @@ -38,8 +42,10 @@ class Dummy c.key :calendar end - expect(Dummy.default_settings).to eq({ :dashboard => {}, :calendar => {} }) - expect(Dummy.setting_object_class_name).to eq('RailsSettings::SettingObject') + expect(Dummy.setting_keys[:dashboard][:default_value]).to eq({}) + expect(Dummy.setting_keys[:calendar][:default_value]).to eq({}) + expect(Dummy.setting_keys[:dashboard][:class_name]).to eq('RailsSettings::SettingObject') + expect(Dummy.setting_keys[:calendar][:class_name]).to eq('RailsSettings::SettingObject') end it "should define using block with defaults" do @@ -48,18 +54,22 @@ class Dummy c.key :calendar, :defaults => { :scope => 'all' } end - expect(Dummy.default_settings).to eq({ :dashboard => { 'theme' => 'red' }, :calendar => { 'scope' => 'all'} }) - expect(Dummy.setting_object_class_name).to eq('RailsSettings::SettingObject') + expect(Dummy.setting_keys[:dashboard][:default_value]).to eq({ 'theme' => 'red' }) + expect(Dummy.setting_keys[:calendar][:default_value]).to eq({ 'scope' => 'all'}) + expect(Dummy.setting_keys[:dashboard][:class_name]).to eq('RailsSettings::SettingObject') + expect(Dummy.setting_keys[:calendar][:class_name]).to eq('RailsSettings::SettingObject') end it "should define using block and class_name" do - Configuration.new(Dummy, :class_name => 'MyClass') do |c| + Configuration.new(Dummy, :class_name => 'ProjectSettingObject') do |c| c.key :dashboard c.key :calendar end - expect(Dummy.default_settings).to eq({ :dashboard => {}, :calendar => {} }) - expect(Dummy.setting_object_class_name).to eq('MyClass') + expect(Dummy.setting_keys[:dashboard][:default_value]).to eq({}) + expect(Dummy.setting_keys[:calendar][:default_value]).to eq({}) + expect(Dummy.setting_keys[:dashboard][:class_name]).to eq('ProjectSettingObject') + expect(Dummy.setting_keys[:calendar][:class_name]).to eq('ProjectSettingObject') end end @@ -104,5 +114,13 @@ class Dummy end }.to raise_error(ArgumentError) end + + it "should fail with an invalid settings object" do + expect { + Configuration.new(Dummy) do |c| + c.key :dashboard, :class_name => "InvalidSettingObject" + end + }.to raise_error(ArgumentError) + end end end diff --git a/spec/settings_spec.rb b/spec/settings_spec.rb index 94f32936..ccdc34dd 100644 --- a/spec/settings_spec.rb +++ b/spec/settings_spec.rb @@ -2,16 +2,16 @@ describe "Defaults" do it "should be stored for simple class" do - expect(Account.default_settings).to eq(:portal => {}) + expect(Account.setting_keys[:portal][:default_value]).to eq({}) end it "should be stored for parent class" do - expect(User.default_settings).to eq(:dashboard => { 'theme' => 'blue', 'view' => 'monthly', 'filter' => true }, - :calendar => { 'scope' => 'company'}) + expect(User.setting_keys[:dashboard][:default_value]).to eq({ 'theme' => 'blue', 'view' => 'monthly', 'filter' => true }) + expect(User.setting_keys[:calendar][:default_value]).to eq({ 'scope' => 'company'}) end it "should be stored for child class" do - expect(GuestUser.default_settings).to eq(:dashboard => { 'theme' => 'red', 'view' => 'monthly', 'filter' => true }) + expect(GuestUser.setting_keys[:dashboard][:default_value]).to eq({ 'theme' => 'red', 'view' => 'monthly', 'filter' => true }) end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2f330f62..0580876a 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -60,8 +60,7 @@ class Account < ActiveRecord::Base has_settings :portal end -class Project < ActiveRecord::Base - has_settings :info, :class_name => 'ProjectSettingObject' +class InvalidSettingObject end class ProjectSettingObject < RailsSettings::SettingObject @@ -72,6 +71,10 @@ class ProjectSettingObject < RailsSettings::SettingObject end end +class Project < ActiveRecord::Base + has_settings :info, :class_name => 'ProjectSettingObject' +end + def setup_db ActiveRecord::Base.configurations = YAML.load_file(File.dirname(__FILE__) + '/database.yml') ActiveRecord::Base.establish_connection(:sqlite) From c406ad871417968295267f6acb10169256e51ce7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Burgos?= Date: Fri, 27 Jul 2018 13:08:14 +0200 Subject: [PATCH 2/4] Updates the README page to document this update --- README.md | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 0e6a334f..6ad48216 100644 --- a/README.md +++ b/README.md @@ -52,10 +52,6 @@ end Every setting is handled by the class `RailsSettings::SettingObject`. You can use your own class, e.g. for validations: ```ruby -class Project < ActiveRecord::Base - has_settings :info, :class_name => 'ProjectSettingObject' -end - class ProjectSettingObject < RailsSettings::SettingObject validate do unless self.owner_name.present? && self.owner_name.is_a?(String) @@ -65,6 +61,25 @@ class ProjectSettingObject < RailsSettings::SettingObject end ``` +Then you can use it like this: + +```ruby +class Project < ActiveRecord::Base + has_settings :info, :class_name => 'ProjectSettingObject' +end +``` + +Or use it only on some of the settings: + +```ruby +class Project < ActiveRecord::Base + has_settings do |s| + s.key :calendar # Will use the default RailsSettings::SettingObject + s.key :info, :class_name => 'ProjectSettingObject' + end +end +``` + ### Set settings ```ruby From 808252bae434124ea1c46e8182e6901d8f8f21f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Burgos?= Date: Fri, 27 Jul 2018 15:54:45 +0200 Subject: [PATCH 3/4] Fixes some problems caught by tests --- lib/rails-settings.rb | 10 +++++++++- lib/rails-settings/base.rb | 20 ++++++++++++++------ lib/rails-settings/setting_object.rb | 16 +++++++++++++++- 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/lib/rails-settings.rb b/lib/rails-settings.rb index ab768535..00414ef5 100644 --- a/lib/rails-settings.rb +++ b/lib/rails-settings.rb @@ -5,6 +5,15 @@ module RailsSettings def self.can_protect_attributes? (ActiveRecord::VERSION::MAJOR == 3) || defined?(ProtectedAttributes) end + + def self.can_use_becomes? + ![ + [3, 0], + [3, 1], + [3, 2], + [4, 0] + ].include?([ActiveRecord::VERSION::MAJOR, ActiveRecord::VERSION::MINOR]) + end end require 'rails-settings/setting_object' @@ -20,4 +29,3 @@ def self.has_settings(*args, &block) extend RailsSettings::Scopes end end - diff --git a/lib/rails-settings/base.rb b/lib/rails-settings/base.rb index 3b64d867..c65ff99d 100644 --- a/lib/rails-settings/base.rb +++ b/lib/rails-settings/base.rb @@ -12,7 +12,7 @@ def settings(var) raise ArgumentError unless var.is_a?(Symbol) raise ArgumentError.new("Unknown key: #{var}") unless self.class.setting_keys[var] - fetch_settings_record(var).becomes(self.class.setting_keys[var][:class_name].constantize) + fetch_settings_record(var) end def settings=(value) @@ -44,14 +44,22 @@ def fetch_settings_record(var) end def find_settings_record(var) - setting_objects.detect { |s| s.var == var.to_s } + setting_objects + .select { |s| s.var == var.to_s } + .map { |s| s.becomes self.class.setting_keys[var][:class_name].constantize } + .first end def build_settings_record(var) - if RailsSettings.can_protect_attributes? - setting_objects.build({ :var => var.to_s }, :without_protection => true) - else - setting_objects.build(:var => var.to_s, :target => self) + build_args = + if RailsSettings.can_protect_attributes? + [{ :var => var.to_s }, :without_protection => true] + else + [:var => var.to_s, :target => self] + end + + setting_objects.build(*build_args) do |record| + record.becomes self.class.setting_keys[var][:class_name].constantize end end end diff --git a/lib/rails-settings/setting_object.rb b/lib/rails-settings/setting_object.rb index 4ebb0382..4bc0bf4c 100644 --- a/lib/rails-settings/setting_object.rb +++ b/lib/rails-settings/setting_object.rb @@ -28,6 +28,15 @@ def respond_to?(method_name, include_priv=false) super || method_name.to_s =~ REGEX_SETTER || _setting?(method_name) end + # Annoying hack for old Rails + unless RailsSettings.can_use_becomes? + def becomes(klass) + became = super(klass) + became.instance_variable_set("@changed_attributes", @changed_attributes) + became + end + end + def method_missing(method_name, *args, &block) if block_given? super @@ -78,7 +87,12 @@ def _target_class end def _setting?(method_name) - _target_class.setting_keys[var.to_sym][:default_value].keys.include?(method_name.to_s) + return false if target_id.nil? || target_type.nil? + + _target_class + .setting_keys[var.to_sym][:default_value] + .keys + .include?(method_name.to_s) end end end From 29c1ddc52e9a1661db22f684cd82c1182c0c3774 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Burgos?= Date: Mon, 30 Jul 2018 10:41:39 +0200 Subject: [PATCH 4/4] Ensure becomes is always used, also on new records --- lib/rails-settings/base.rb | 19 ++++++------------- spec/settings_spec.rb | 10 ++++++++++ 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/lib/rails-settings/base.rb b/lib/rails-settings/base.rb index c65ff99d..af8cbc5e 100644 --- a/lib/rails-settings/base.rb +++ b/lib/rails-settings/base.rb @@ -13,6 +13,7 @@ def settings(var) raise ArgumentError.new("Unknown key: #{var}") unless self.class.setting_keys[var] fetch_settings_record(var) + .becomes(self.class.setting_keys[var][:class_name].constantize) end def settings=(value) @@ -44,22 +45,14 @@ def fetch_settings_record(var) end def find_settings_record(var) - setting_objects - .select { |s| s.var == var.to_s } - .map { |s| s.becomes self.class.setting_keys[var][:class_name].constantize } - .first + setting_objects.detect { |s| s.var == var.to_s } end def build_settings_record(var) - build_args = - if RailsSettings.can_protect_attributes? - [{ :var => var.to_s }, :without_protection => true] - else - [:var => var.to_s, :target => self] - end - - setting_objects.build(*build_args) do |record| - record.becomes self.class.setting_keys[var][:class_name].constantize + if RailsSettings.can_protect_attributes? + setting_objects.build({ :var => var.to_s }, :without_protection => true) + else + setting_objects.build(:var => var.to_s, :target => self) end end end diff --git a/spec/settings_spec.rb b/spec/settings_spec.rb index ccdc34dd..b9662aa8 100644 --- a/spec/settings_spec.rb +++ b/spec/settings_spec.rb @@ -32,6 +32,16 @@ end describe 'Objects' do + context "settings should be an instance of :class_name" do + it "should be an instance of 'SettingObject' by default" do + expect(User.new.settings(:dashboard)).to be_a(RailsSettings::SettingObject) + end + + it "should be an instance of 'ProjectSettingObject' if defined" do + expect(Project.new.settings(:info)).to be_a(ProjectSettingObject) + end + end + context 'without defaults' do let(:account) { Account.new :subdomain => 'foo' }