From 1238ec16a085ba8c1e5baf892a266fa06f422f50 Mon Sep 17 00:00:00 2001 From: Georg Ledermann Date: Mon, 30 Jul 2018 17:48:26 +0200 Subject: [PATCH] Revert "Allow using different classes for each setting" --- README.md | 23 +++--------- lib/rails-settings.rb | 10 +----- lib/rails-settings/base.rb | 35 ++++++------------- lib/rails-settings/configuration.rb | 41 ++++++---------------- lib/rails-settings/setting_object.rb | 20 ++--------- spec/configuration_spec.rb | 52 +++++++++------------------- spec/settings_spec.rb | 18 +++------- spec/spec_helper.rb | 7 ++-- 8 files changed, 52 insertions(+), 154 deletions(-) diff --git a/README.md b/README.md index 6ad48216..0e6a334f 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,10 @@ 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) @@ -61,25 +65,6 @@ 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 diff --git a/lib/rails-settings.rb b/lib/rails-settings.rb index 00414ef5..ab768535 100644 --- a/lib/rails-settings.rb +++ b/lib/rails-settings.rb @@ -5,15 +5,6 @@ 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' @@ -29,3 +20,4 @@ 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 af8cbc5e..a04dc28b 100644 --- a/lib/rails-settings/base.rb +++ b/lib/rails-settings/base.rb @@ -6,14 +6,17 @@ def self.included(base) :as => :target, :autosave => true, :dependent => :delete_all, - :class_name => "RailsSettings::SettingObject" + :class_name => self.setting_object_class_name def settings(var) raise ArgumentError unless var.is_a?(Symbol) - raise ArgumentError.new("Unknown key: #{var}") unless self.class.setting_keys[var] + raise ArgumentError.new("Unknown key: #{var}") unless self.class.default_settings[var] - fetch_settings_record(var) - .becomes(self.class.setting_keys[var][:class_name].constantize) + 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 end def settings=(value) @@ -33,27 +36,11 @@ def settings?(var=nil) end def to_settings_hash - 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) + 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) end + settings_hash end end end diff --git a/lib/rails-settings/configuration.rb b/lib/rails-settings/configuration.rb index 3dec4d8b..7a275e92 100644 --- a/lib/rails-settings/configuration.rb +++ b/lib/rails-settings/configuration.rb @@ -1,53 +1,32 @@ module RailsSettings class Configuration def initialize(*args, &block) - @default_options = args.extract_options! - validate_options @default_options + options = args.extract_options! klass = args.shift keys = args raise ArgumentError unless klass @klass = klass - @klass.class_attribute :setting_keys - @klass.setting_keys = {} + @klass.class_attribute :default_settings, :setting_object_class_name + @klass.default_settings = {} + @klass.setting_object_class_name = options[:class_name] || 'RailsSettings::SettingObject' if block_given? yield(self) else - keys.each { |k| key(k) } + keys.each do |k| + key(k) + end end - raise ArgumentError.new('has_settings: No keys defined') if @klass.setting_keys.empty? + raise ArgumentError.new('has_settings: No keys defined') if @klass.default_settings.blank? 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) - 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 + 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 end end diff --git a/lib/rails-settings/setting_object.rb b/lib/rails-settings/setting_object.rb index 4bc0bf4c..2397ed18 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.setting_keys[var.to_sym] + unless _target_class.default_settings[var.to_sym] errors.add(:var, "#{var} is not defined!") end end @@ -28,15 +28,6 @@ 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 @@ -64,7 +55,7 @@ def sanitize_for_mass_assignment(attributes, role = nil) private def _get_value(name) if value[name].nil? - _target_class.setting_keys[var.to_sym][:default_value][name] + _target_class.default_settings[var.to_sym][name] else value[name] end @@ -87,12 +78,7 @@ def _target_class end def _setting?(method_name) - return false if target_id.nil? || target_type.nil? - - _target_class - .setting_keys[var.to_sym][:default_value] - .keys - .include?(method_name.to_s) + _target_class.default_settings[var.to_sym].keys.include?(method_name.to_s) end end end diff --git a/spec/configuration_spec.rb b/spec/configuration_spec.rb index eb57644d..63089d51 100644 --- a/spec/configuration_spec.rb +++ b/spec/configuration_spec.rb @@ -8,32 +8,28 @@ class Dummy it "should define single key" do Configuration.new(Dummy, :dashboard) - expect(Dummy.setting_keys[:dashboard][:default_value]).to eq({}) - expect(Dummy.setting_keys[:dashboard][:class_name]).to eq('RailsSettings::SettingObject') + expect(Dummy.default_settings).to eq({ :dashboard => {} }) + expect(Dummy.setting_object_class_name).to eq('RailsSettings::SettingObject') end it "should define multiple keys" do Configuration.new(Dummy, :dashboard, :calendar) - 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') + expect(Dummy.default_settings).to eq({ :dashboard => {}, :calendar => {} }) + expect(Dummy.setting_object_class_name).to eq('RailsSettings::SettingObject') end it "should define single key with class_name" do - 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') + Configuration.new(Dummy, :dashboard, :class_name => 'MyClass') + expect(Dummy.default_settings).to eq({ :dashboard => {} }) + expect(Dummy.setting_object_class_name).to eq('MyClass') end it "should define multiple keys with class_name" do - Configuration.new(Dummy, :dashboard, :calendar, :class_name => 'ProjectSettingObject') + Configuration.new(Dummy, :dashboard, :calendar, :class_name => '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') + expect(Dummy.default_settings).to eq({ :dashboard => {}, :calendar => {} }) + expect(Dummy.setting_object_class_name).to eq('MyClass') end it "should define using block" do @@ -42,10 +38,8 @@ class Dummy c.key :calendar end - 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') + expect(Dummy.default_settings).to eq({ :dashboard => {}, :calendar => {} }) + expect(Dummy.setting_object_class_name).to eq('RailsSettings::SettingObject') end it "should define using block with defaults" do @@ -54,22 +48,18 @@ class Dummy c.key :calendar, :defaults => { :scope => 'all' } end - 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') + expect(Dummy.default_settings).to eq({ :dashboard => { 'theme' => 'red' }, :calendar => { 'scope' => 'all'} }) + expect(Dummy.setting_object_class_name).to eq('RailsSettings::SettingObject') end it "should define using block and class_name" do - Configuration.new(Dummy, :class_name => 'ProjectSettingObject') do |c| + Configuration.new(Dummy, :class_name => 'MyClass') do |c| c.key :dashboard c.key :calendar end - 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') + expect(Dummy.default_settings).to eq({ :dashboard => {}, :calendar => {} }) + expect(Dummy.setting_object_class_name).to eq('MyClass') end end @@ -114,13 +104,5 @@ 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 b9662aa8..94f32936 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.setting_keys[:portal][:default_value]).to eq({}) + expect(Account.default_settings).to eq(:portal => {}) end it "should be stored for parent class" do - 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'}) + expect(User.default_settings).to eq(:dashboard => { 'theme' => 'blue', 'view' => 'monthly', 'filter' => true }, + :calendar => { 'scope' => 'company'}) end it "should be stored for child class" do - expect(GuestUser.setting_keys[:dashboard][:default_value]).to eq({ 'theme' => 'red', 'view' => 'monthly', 'filter' => true }) + expect(GuestUser.default_settings).to eq(:dashboard => { 'theme' => 'red', 'view' => 'monthly', 'filter' => true }) end end @@ -32,16 +32,6 @@ 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' } diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 0580876a..2f330f62 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -60,7 +60,8 @@ class Account < ActiveRecord::Base has_settings :portal end -class InvalidSettingObject +class Project < ActiveRecord::Base + has_settings :info, :class_name => 'ProjectSettingObject' end class ProjectSettingObject < RailsSettings::SettingObject @@ -71,10 +72,6 @@ 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)