diff --git a/app/assets/javascripts/admin/models/site_setting.js b/app/assets/javascripts/admin/models/site_setting.js index ae487f0b3..0b92c8bd7 100644 --- a/app/assets/javascripts/admin/models/site_setting.js +++ b/app/assets/javascripts/admin/models/site_setting.js @@ -8,6 +8,8 @@ **/ Discourse.SiteSetting = Discourse.Model.extend({ + validationMessage: null, + /** Is the boolean setting true? @@ -67,6 +69,7 @@ Discourse.SiteSetting = Discourse.Model.extend({ **/ resetValue: function() { this.set('value', this.get('originalValue')); + this.set('validationMessage', null); }, /** @@ -76,13 +79,20 @@ Discourse.SiteSetting = Discourse.Model.extend({ **/ save: function() { // Update the setting - var setting = this, data = {}; + var self = this, data = {}; data[this.get('setting')] = this.get('value'); return Discourse.ajax("/admin/site_settings/" + this.get('setting'), { data: data, type: 'PUT' }).then(function() { - setting.set('originalValue', setting.get('value')); + self.set('originalValue', self.get('value')); + self.set('validationMessage', null); + }, function(e) { + if (e.responseJSON && e.responseJSON.errors) { + self.set('validationMessage', e.responseJSON.errors[0]); + } else { + self.set('validationMessage', I18n.t('generic_error')); + } }); }, diff --git a/app/assets/javascripts/admin/templates/site_settings/setting_string.js.handlebars b/app/assets/javascripts/admin/templates/site_settings/setting_string.js.handlebars index 2274938d2..a04b2d763 100644 --- a/app/assets/javascripts/admin/templates/site_settings/setting_string.js.handlebars +++ b/app/assets/javascripts/admin/templates/site_settings/setting_string.js.handlebars @@ -3,6 +3,7 @@
{{textField value=value classNames="input-setting-string"}} +
{{validationMessage}}
{{unbound description}}
{{#if dirty}} diff --git a/app/assets/stylesheets/common/admin/admin_base.scss b/app/assets/stylesheets/common/admin/admin_base.scss index 58b989039..bb1eb1098 100644 --- a/app/assets/stylesheets/common/admin/admin_base.scss +++ b/app/assets/stylesheets/common/admin/admin_base.scss @@ -229,13 +229,20 @@ } - .desc { + .desc, .validation-error { padding-top: 3px; - color: scale-color($primary, $lightness: 50%); font-size: 80%; line-height: 1.4em; } + .validation-error { + color: $danger; + } + + .desc { + color: scale-color($primary, $lightness: 50%); + } + h3 { font-size: 13px; font-weight: normal; diff --git a/app/controllers/admin/site_settings_controller.rb b/app/controllers/admin/site_settings_controller.rb index ae6be824a..7ed22e7e2 100644 --- a/app/controllers/admin/site_settings_controller.rb +++ b/app/controllers/admin/site_settings_controller.rb @@ -11,9 +11,14 @@ class Admin::SiteSettingsController < Admin::AdminController id = params[:id] value = params[id] value.strip! if value.is_a?(String) - 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 + begin + prev_value = SiteSetting.send(id) + SiteSetting.set(id, value) + StaffActionLogger.new(current_user).log_site_setting_change(id, prev_value, value) if SiteSetting.has_setting?(id) + render nothing: true + rescue Discourse::InvalidParameters => e + render json: {errors: [e.message]}, status: 422 + end end end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 33465cbdf..e216b269e 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -909,6 +909,9 @@ en: enable_cdn_js_debugging: "Allow /logs to display proper errors by adding crossorigin permissions on all js includes" show_create_topics_notice: "If the site has fewer than 5 public topics, show a notice asking admins to create some topics." + errors: + invalid_email: "Invalid email address." + notification_types: mentioned: "%{display_username} mentioned you in %{link}" liked: "%{display_username} liked your post in %{link}" diff --git a/config/site_settings.yml b/config/site_settings.yml index 03fbb9208..ab9edb5e3 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -3,8 +3,12 @@ required: client: true default: 'Discourse' site_description: '' - contact_email: '' - notification_email: 'info@discourse.org' + contact_email: + default: '' + validator: 'EmailSettingValidator' + notification_email: + default: 'info@discourse.org' + validator: 'EmailSettingValidator' site_contact_username: '' logo_url: client: true @@ -307,7 +311,9 @@ email: email_in: default: false client: true - email_in_address: '' + email_in_address: + default: '' + validator: 'EmailSettingValidator' email_in_min_trust: default: 3 enum: 'MinTrustToCreateTopicSetting' diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb index d273b2fa2..fd619bd0a 100644 --- a/lib/site_setting_extension.rb +++ b/lib/site_setting_extension.rb @@ -54,6 +54,10 @@ module SiteSettingExtension @refresh_settings ||= [] end + def validators + @validators ||= {} + end + def setting(name_arg, default = nil, opts = {}) name = name_arg.to_sym mutex.synchronize do @@ -78,6 +82,9 @@ module SiteSettingExtension if opts[:refresh] refresh_settings << name end + if v = opts[:validator] + validators[name] = v.is_a?(String) ? v.constantize : v + end current[name] = current_value setup_methods(name, current_value) @@ -232,6 +239,10 @@ module SiteSettingExtension raise Discourse::InvalidParameters.new(:value) unless enum_class(name).valid_value?(val) end + if v = validators[name] and !v.valid_value?(val) + raise Discourse::InvalidParameters.new(v.error_message(val)) + end + provider.save(name, val, type) current[name] = convert(val, type) clear_cache! diff --git a/lib/validators/email_setting_validator.rb b/lib/validators/email_setting_validator.rb new file mode 100644 index 000000000..fae1c78a5 --- /dev/null +++ b/lib/validators/email_setting_validator.rb @@ -0,0 +1,9 @@ +class EmailSettingValidator + def self.valid_value?(val) + val == '' || EmailValidator.email_regex =~ val + end + + def self.error_message(val) + I18n.t('site_settings.errors.invalid_email') + end +end diff --git a/lib/validators/email_validator.rb b/lib/validators/email_validator.rb index 777498348..06026faa2 100644 --- a/lib/validators/email_validator.rb +++ b/lib/validators/email_validator.rb @@ -21,4 +21,8 @@ class EmailValidator < ActiveModel::EachValidator value =~ regexp end + def self.email_regex + /^[a-zA-Z0-9!#$%&'*+\/=?\^_`{|}~\-]+(?:\.[a-zA-Z0-9!#$%&'\*+\/=?\^_`{|}~\-]+)*@(?:[a-zA-Z0-9](?:[a-zA-Z0-9\-]*[a-zA-Z0-9])?\.)+[a-zA-Z0-9](?:[a-zA-Z0-9\-]*[a-zA-Z0-9])?$/ + end + end \ No newline at end of file diff --git a/spec/components/site_setting_extension_spec.rb b/spec/components/site_setting_extension_spec.rb index 32517b476..2a4517030 100644 --- a/spec/components/site_setting_extension_spec.rb +++ b/spec/components/site_setting_extension_spec.rb @@ -238,6 +238,10 @@ describe SiteSettingExtension do end context 'when overridden' do + after :each do + settings.remove_override!(:validated_setting) + end + it 'stores valid values' do test_enum_class.expects(:valid_value?).with('fr').returns(true) settings.test_enum = 'fr' @@ -278,6 +282,36 @@ describe SiteSettingExtension do end end + describe "setting with a validator" do + before do + settings.setting(:validated_setting, "info@example.com", {validator: 'EmailSettingValidator'}) + settings.refresh! + end + + after :each do + settings.remove_override!(:validated_setting) + end + + it "stores valid values" do + EmailSettingValidator.expects(:valid_value?).returns(true) + settings.validated_setting = 'success@example.com' + settings.validated_setting.should == 'success@example.com' + end + + it "rejects invalid values" do + expect { + EmailSettingValidator.expects(:valid_value?).returns(false) + settings.validated_setting = 'nope' + }.to raise_error(Discourse::InvalidParameters) + settings.validated_setting.should == "info@example.com" + end + + it "allows blank values" do + settings.validated_setting = '' + settings.validated_setting.should == '' + end + end + describe "set for an invalid setting name" do it "raises an error" do settings.setting(:test_setting, 77)