diff --git a/app/controllers/admin/site_settings_controller.rb b/app/controllers/admin/site_settings_controller.rb index aab3b386cb9..b9b9e7273e4 100644 --- a/app/controllers/admin/site_settings_controller.rb +++ b/app/controllers/admin/site_settings_controller.rb @@ -10,8 +10,8 @@ class Admin::SiteSettingsController < Admin::AdminController params.require(:id) id = params[:id] value = params[id] - StaffActionLogger.new(current_user).log_site_setting_change(id, SiteSetting.send(id), value) if SiteSetting.respond_to?(id) - SiteSetting.send("#{id}=", value) + StaffActionLogger.new(current_user).log_site_setting_change(id, SiteSetting.send(id), value) if SiteSetting.has_setting?(id) + SiteSetting.set(id, value) render nothing: true end diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb index 3417275049e..0f55aa6e44b 100644 --- a/lib/site_setting_extension.rb +++ b/lib/site_setting_extension.rb @@ -200,6 +200,18 @@ module SiteSettingExtension @last_message_sent = MessageBus.publish('/site_settings', {process: process_id}) end + def has_setting?(name) + defaults.has_key?(name.to_sym) || defaults.has_key?("#{name}?".to_sym) + end + + def set(name, value) + if has_setting?(name) + self.send("#{name}=", value) + else + raise ArgumentError.new("No setting named #{name} exists") + end + end + protected def diff_hash(new_hash, old) diff --git a/spec/components/site_setting_extension_spec.rb b/spec/components/site_setting_extension_spec.rb index 68c260bcb3e..c3c24a6bf52 100644 --- a/spec/components/site_setting_extension_spec.rb +++ b/spec/components/site_setting_extension_spec.rb @@ -58,6 +58,11 @@ describe SiteSettingExtension do settings.refresh! settings.test_setting.should_not == 77 end + + it "can be overridden with set" do + settings.set("test_setting", 12) + settings.test_setting.should == 12 + end end end @@ -89,6 +94,11 @@ describe SiteSettingExtension do settings.test_str = 100 settings.test_str.should.eql? "100" end + + it "can be overridden with set" do + settings.set("test_str", "hi") + settings.test_str.should == "hi" + end end end @@ -128,6 +138,11 @@ describe SiteSettingExtension do settings.refresh! settings.test_hello?.should_not == false end + + it "can be overridden with set" do + settings.set("test_hello", true) + settings.test_hello?.should == true + end end end @@ -189,4 +204,14 @@ describe SiteSettingExtension do end end + describe "set for an invalid setting name" do + it "raises an error" do + settings.setting(:test_setting, 77) + settings.refresh! + expect { + settings.set("provider", "haxxed") + }.to raise_error(ArgumentError) + end + end + end diff --git a/spec/controllers/admin/site_settings_controller_spec.rb b/spec/controllers/admin/site_settings_controller_spec.rb index 534da2b1e80..4f2a9678719 100644 --- a/spec/controllers/admin/site_settings_controller_spec.rb +++ b/spec/controllers/admin/site_settings_controller_spec.rb @@ -25,6 +25,10 @@ describe Admin::SiteSettingsController do context 'update' do + before do + SiteSetting.setting(:test_setting, "default") + end + it 'sets the value when the param is present' do SiteSetting.expects(:'test_setting=').with('hello').once xhr :put, :update, id: 'test_setting', test_setting: 'hello' @@ -41,6 +45,12 @@ describe Admin::SiteSettingsController do StaffActionLogger.any_instance.expects(:log_site_setting_change).with('test_setting', 'previous', 'hello') xhr :put, :update, id: 'test_setting', test_setting: 'hello' end + + it 'fails when a setting does not exist' do + expect { + xhr :put, :update, id: 'provider', provider: 'gotcha' + }.to raise_error(ArgumentError) + end end end