From 6e79197519823b4367280e694cb03ecdd74719ff Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Fri, 23 Aug 2013 17:35:01 -0400 Subject: [PATCH] Enum site settings can have translatable names in dropdown. Add setting for how often users get digest emails by default: default_digest_email_frequency. --- .../javascripts/admin/models/site_setting.js | 11 ++++++++-- app/models/digest_email_site_setting.rb | 22 +++++++++++++++++++ app/models/locale_site_setting.rb | 8 +++++-- app/models/s3_region_site_setting.rb | 22 ++++++++++++++++--- app/models/site_setting.rb | 1 + app/models/user.rb | 12 ++++++++++ config/locales/client.en.yml | 5 +++++ config/locales/server.en.yml | 1 + ...faults_on_email_digest_columns_of_users.rb | 12 ++++++++++ lib/enum_site_setting.rb | 5 +++++ lib/jobs/enqueue_digest_emails.rb | 2 +- lib/site_setting_extension.rb | 2 +- spec/components/js_locale_helper_spec.rb | 4 ++-- .../site_setting_extenstion_spec.rb | 1 + spec/models/digest_email_site_setting_spec.rb | 18 +++++++++++++++ spec/models/locale_site_setting_spec.rb | 2 +- spec/models/s3_region_site_setting_spec.rb | 2 +- spec/models/user_spec.rb | 21 +++++++++++++++--- 18 files changed, 135 insertions(+), 16 deletions(-) create mode 100644 app/models/digest_email_site_setting.rb create mode 100644 db/migrate/20130823201420_drop_defaults_on_email_digest_columns_of_users.rb create mode 100644 lib/enum_site_setting.rb create mode 100644 spec/models/digest_email_site_setting_spec.rb diff --git a/app/assets/javascripts/admin/models/site_setting.js b/app/assets/javascripts/admin/models/site_setting.js index 3824c295c..55f45e20e 100644 --- a/app/assets/javascripts/admin/models/site_setting.js +++ b/app/assets/javascripts/admin/models/site_setting.js @@ -81,10 +81,17 @@ Discourse.SiteSetting = Discourse.Model.extend({ }, validValues: function() { - var vals; + var vals, setting; vals = Em.A(); + setting = this; _.each(this.get('valid_values'), function(v) { - if(v.length > 0) vals.addObject({ name: v, value: v }); + if (v.name && v.name.length > 0) { + if (setting.translate_names) { + vals.addObject({name: I18n.t(v.name), value: v.value}); + } else { + vals.addObject(v); + } + } }); return vals; }.property('valid_values'), diff --git a/app/models/digest_email_site_setting.rb b/app/models/digest_email_site_setting.rb new file mode 100644 index 000000000..1713178c3 --- /dev/null +++ b/app/models/digest_email_site_setting.rb @@ -0,0 +1,22 @@ +require_dependency 'enum_site_setting' + +class DigestEmailSiteSetting < EnumSiteSetting + + def self.valid_value?(val) + val.blank? or values.any? { |v| v[:value] == val.to_s } + end + + def self.values + @values ||= [ + {name: 'never', value: '' }, + {name: 'daily', value: '1' }, + {name: 'weekly', value: '7' }, + {name: 'every_two_weeks', value: '14' } + ] + end + + def self.translate_names? + true + end + +end diff --git a/app/models/locale_site_setting.rb b/app/models/locale_site_setting.rb index 1c3ae78ba..ed3ff9c7b 100644 --- a/app/models/locale_site_setting.rb +++ b/app/models/locale_site_setting.rb @@ -1,11 +1,15 @@ -class LocaleSiteSetting +require_dependency 'enum_site_setting' + +class LocaleSiteSetting < EnumSiteSetting def self.valid_value?(val) supported_locales.include?(val) end def self.values - supported_locales + supported_locales.map do |l| + {name: l, value: l} + end end diff --git a/app/models/s3_region_site_setting.rb b/app/models/s3_region_site_setting.rb index 2b66348f5..9267b7ebc 100644 --- a/app/models/s3_region_site_setting.rb +++ b/app/models/s3_region_site_setting.rb @@ -1,9 +1,25 @@ -class S3RegionSiteSetting +require_dependency 'enum_site_setting' + +class S3RegionSiteSetting < EnumSiteSetting def self.valid_value?(val) - values.include? val + valid_values.include? val end def self.values - @values ||= ['', 'us-east-1', 'us-west-1', 'us-west-2', 'eu-west-1', 'ap-southeast-1', 'ap-southeast-2', 'ap-northeast-1', 'sa-east-1'].sort + @values ||= valid_values.sort.map {|x| {name: x, value: x} } + end + + private + + def self.valid_values + [ '', + 'us-east-1', + 'us-west-1', + 'us-west-2', + 'eu-west-1', + 'ap-southeast-1', + 'ap-southeast-2', + 'ap-northeast-1', + 'sa-east-1' ] end end diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index 062099c10..e324a6c47 100644 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -115,6 +115,7 @@ class SiteSetting < ActiveRecord::Base setting(:email_time_window_mins, 10) setting(:email_posts_context, 5) + setting(:default_digest_email_frequency, '7', enum: 'DigestEmailSiteSetting') # How many characters we can import into a onebox setting(:onebox_max_chars, 5000) diff --git a/app/models/user.rb b/app/models/user.rb index 9e9451dde..b9f0dad12 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -55,6 +55,7 @@ class User < ActiveRecord::Base before_save :update_username_lower before_save :ensure_password_is_hashed after_initialize :add_trust_level + after_initialize :set_default_email_digest after_save :update_tracked_topics @@ -598,6 +599,17 @@ class User < ActiveRecord::Base ) end + def set_default_email_digest + if has_attribute?(:email_digests) && self.email_digests.nil? + if SiteSetting.default_digest_email_frequency.blank? + self.email_digests = false + else + self.email_digests = true + self.digest_after_days ||= SiteSetting.default_digest_email_frequency.to_i if has_attribute?(:digest_after_days) + end + end + end + private def self.discourse_hub_nickname_operation diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 63076bd8a..6e1454bdd 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -100,6 +100,10 @@ en: read_more: 'read more' more: "More" less: "Less" + never: "never" + daily: "daily" + weekly: "weekly" + every_two_weeks: "every two weeks" character_count: one: "{{count}} character" other: "{{count}} characters" @@ -1342,3 +1346,4 @@ en: reset: 'reset to default' none: 'none' + diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 015edf89c..2a2d34847 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -667,6 +667,7 @@ en: allow_uploaded_avatars: "Allow users to upload their custom avatars" allow_animated_avatars: "Allow users to use animated gif for avatars" + default_digest_email_frequency: "How often users receive digest emails by default. They can change this setting in their preferences." notification_types: mentioned: "%{display_username} mentioned you in %{link}" diff --git a/db/migrate/20130823201420_drop_defaults_on_email_digest_columns_of_users.rb b/db/migrate/20130823201420_drop_defaults_on_email_digest_columns_of_users.rb new file mode 100644 index 000000000..45b3f3267 --- /dev/null +++ b/db/migrate/20130823201420_drop_defaults_on_email_digest_columns_of_users.rb @@ -0,0 +1,12 @@ +class DropDefaultsOnEmailDigestColumnsOfUsers < ActiveRecord::Migration + def up + change_column_default :users, :email_digests, nil + change_column :users, :digest_after_days, :integer, default: nil, null: true + end + + def down + change_column_default :users, :email_digests, true + change_column_default :users, :digest_after_days, 7 + change_column :users, :digest_after_days, :integer, default: 7, null: false + end +end diff --git a/lib/enum_site_setting.rb b/lib/enum_site_setting.rb new file mode 100644 index 000000000..9305a8f7d --- /dev/null +++ b/lib/enum_site_setting.rb @@ -0,0 +1,5 @@ +class EnumSiteSetting + def self.translate_names? + false + end +end diff --git a/lib/jobs/enqueue_digest_emails.rb b/lib/jobs/enqueue_digest_emails.rb index 0747819ce..a19e35119 100644 --- a/lib/jobs/enqueue_digest_emails.rb +++ b/lib/jobs/enqueue_digest_emails.rb @@ -11,7 +11,7 @@ module Jobs end def target_users - # Users who want to receive emails and haven't been emailed int he last day + # Users who want to receive emails and haven't been emailed in the last day query = User.select(:id) .where(email_digests: true, active: true) .where("COALESCE(last_emailed_at, '2010-01-01') <= CURRENT_TIMESTAMP - ('1 DAY'::INTERVAL * digest_after_days)") diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb index d63a8745a..2dc91d2d8 100644 --- a/lib/site_setting_extension.rb +++ b/lib/site_setting_extension.rb @@ -80,7 +80,7 @@ module SiteSettingExtension description: description(s), default: v, type: type.to_s, - value: value.to_s}.merge( type == :enum ? {valid_values: enum_class(s).values} : {}) + value: value.to_s}.merge( type == :enum ? {valid_values: enum_class(s).values, translate_names: enum_class(s).translate_names?} : {}) end end diff --git a/spec/components/js_locale_helper_spec.rb b/spec/components/js_locale_helper_spec.rb index c28b400d0..269ee01eb 100644 --- a/spec/components/js_locale_helper_spec.rb +++ b/spec/components/js_locale_helper_spec.rb @@ -85,8 +85,8 @@ describe JsLocaleHelper do end LocaleSiteSetting.values.each do |locale| - it "generates valid date helpers for #{locale} locale" do - js = JsLocaleHelper.output_locale(locale) + it "generates valid date helpers for #{locale[:value]} locale" do + js = JsLocaleHelper.output_locale(locale[:value]) ctx = V8::Context.new ctx.load(Rails.root + 'app/assets/javascripts/locales/i18n.js') ctx.eval(js) diff --git a/spec/components/site_setting_extenstion_spec.rb b/spec/components/site_setting_extenstion_spec.rb index d84a0b012..c57708812 100644 --- a/spec/components/site_setting_extenstion_spec.rb +++ b/spec/components/site_setting_extenstion_spec.rb @@ -134,6 +134,7 @@ describe SiteSettingExtension do describe 'enum setting' do before do @enum_class = Enum.new(:test) + @enum_class.stubs(:translate_names?).returns(false) settings.setting(:test_enum, 'en', enum: @enum_class) settings.refresh! end diff --git a/spec/models/digest_email_site_setting_spec.rb b/spec/models/digest_email_site_setting_spec.rb new file mode 100644 index 000000000..5f74cbe50 --- /dev/null +++ b/spec/models/digest_email_site_setting_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe DigestEmailSiteSetting do + describe 'valid_value?' do + it 'returns true for a valid value as an int' do + DigestEmailSiteSetting.valid_value?(1).should be_true + end + + it 'returns true for a valid value as a string' do + DigestEmailSiteSetting.valid_value?('1').should be_true + end + + it 'returns false for an invalid value' do + DigestEmailSiteSetting.valid_value?(1.5).should be_false + DigestEmailSiteSetting.valid_value?('7 dogs').should be_false + end + end +end diff --git a/spec/models/locale_site_setting_spec.rb b/spec/models/locale_site_setting_spec.rb index b623a4502..c999e336d 100644 --- a/spec/models/locale_site_setting_spec.rb +++ b/spec/models/locale_site_setting_spec.rb @@ -14,7 +14,7 @@ describe LocaleSiteSetting do describe 'values' do it 'returns all the locales that we have translations for' do - expect(LocaleSiteSetting.values.sort).to eq(Dir.glob( File.join(Rails.root, 'config', 'locales', 'client.*.yml') ).map {|x| x.split('.')[-2]}.sort) + expect(LocaleSiteSetting.values.map {|x| x[:value]}.sort).to eq(Dir.glob( File.join(Rails.root, 'config', 'locales', 'client.*.yml') ).map {|x| x.split('.')[-2]}.sort) end end diff --git a/spec/models/s3_region_site_setting_spec.rb b/spec/models/s3_region_site_setting_spec.rb index f0bb5421c..506f9d0db 100644 --- a/spec/models/s3_region_site_setting_spec.rb +++ b/spec/models/s3_region_site_setting_spec.rb @@ -14,7 +14,7 @@ describe S3RegionSiteSetting do describe 'values' do it 'returns all the S3 regions and blank' do - expect(S3RegionSiteSetting.values.sort).to eq(['', 'us-east-1', 'us-west-1', 'us-west-2', 'eu-west-1', 'ap-southeast-1', 'ap-southeast-2', 'ap-northeast-1', 'sa-east-1'].sort) + expect(S3RegionSiteSetting.values.map {|x| x[:value]}.sort).to eq(['', 'us-east-1', 'us-west-1', 'us-west-2', 'eu-west-1', 'ap-southeast-1', 'ap-southeast-2', 'ap-northeast-1', 'sa-east-1'].sort) end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 971703399..28c632bd3 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -264,13 +264,28 @@ describe User do it { should_not be_approved } its(:approved_at) { should be_blank } its(:approved_by_id) { should be_blank } - its(:email_digests) { should be_true } its(:email_private_messages) { should be_true } its(:email_direct ) { should be_true } its(:time_read) { should == 0} - # Default to digests after one week - its(:digest_after_days) { should == 7 } + context 'digest emails' do + it 'defaults to digests every week' do + subject.email_digests.should be_true + subject.digest_after_days.should == 7 + end + + it 'uses default_digest_email_frequency' do + SiteSetting.stubs(:default_digest_email_frequency).returns(1) + subject.email_digests.should be_true + subject.digest_after_days.should == 1 + end + + it 'disables digests by default if site setting says so' do + SiteSetting.stubs(:default_digest_email_frequency).returns('') + subject.email_digests.should be_false + end + end + context 'after_save' do before do