diff --git a/app/assets/javascripts/discourse/controllers/preferences.js.es6 b/app/assets/javascripts/discourse/controllers/preferences.js.es6 index 5231e4fa430..64ea8b79457 100644 --- a/app/assets/javascripts/discourse/controllers/preferences.js.es6 +++ b/app/assets/javascripts/discourse/controllers/preferences.js.es6 @@ -73,6 +73,10 @@ export default Ember.Controller.extend(CanCheckEmails, { { name: I18n.t('user.email_digests.weekly'), value: 7 }, { name: I18n.t('user.email_digests.every_two_weeks'), value: 14 }], + likeNotificationFrequencies: [{ name: I18n.t('user.like_notification_frequency.always'), value: 0 }, + { name: I18n.t('user.like_notification_frequency.first_time_and_daily'), value: 1 }, + { name: I18n.t('user.like_notification_frequency.first_time'), value: 2 }], + autoTrackDurations: [{ name: I18n.t('user.auto_track_options.never'), value: -1 }, { name: I18n.t('user.auto_track_options.immediately'), value: 0 }, { name: I18n.t('user.auto_track_options.after_30_seconds'), value: 30000 }, diff --git a/app/assets/javascripts/discourse/models/user.js.es6 b/app/assets/javascripts/discourse/models/user.js.es6 index 57c4893fe21..7d20d129360 100644 --- a/app/assets/javascripts/discourse/models/user.js.es6 +++ b/app/assets/javascripts/discourse/models/user.js.es6 @@ -167,7 +167,8 @@ const User = RestModel.extend({ 'automatically_unpin_topics', 'digest_after_days', 'new_topic_duration_minutes', - 'auto_track_topics_after_msecs' + 'auto_track_topics_after_msecs', + 'like_notification_frequency' ].forEach(s => { data[s] = this.get(`user_option.${s}`); }); diff --git a/app/assets/javascripts/discourse/templates/user/preferences.hbs b/app/assets/javascripts/discourse/templates/user/preferences.hbs index c0aa984089e..c48736827c4 100644 --- a/app/assets/javascripts/discourse/templates/user/preferences.hbs +++ b/app/assets/javascripts/discourse/templates/user/preferences.hbs @@ -218,6 +218,11 @@ {{combo-box valueAttribute="value" content=autoTrackDurations value=model.user_option.auto_track_topics_after_msecs}} +
+ + {{combo-box valueAttribute="value" content=likeNotificationFrequencies value=model.user_option.like_notification_frequency}} +
+ {{preference-checkbox labelKey="user.external_links_in_new_tab" checked=model.user_option.external_links_in_new_tab}} {{preference-checkbox labelKey="user.enable_quoting" checked=model.user_option.enable_quoting}} {{preference-checkbox labelKey="user.dynamic_favicon" checked=model.user_option.dynamic_favicon}} diff --git a/app/models/like_notification_frequency_site_setting.rb b/app/models/like_notification_frequency_site_setting.rb new file mode 100644 index 00000000000..7ff681d6742 --- /dev/null +++ b/app/models/like_notification_frequency_site_setting.rb @@ -0,0 +1,22 @@ +require_dependency 'enum_site_setting' + +class LikeNotificationFrequencySiteSetting < EnumSiteSetting + + def self.valid_value?(val) + val.to_i.to_s == val.to_s && + values.any? { |v| v[:value] == val.to_i } + end + + def self.values + @values ||= [ + { name: 'user.like_notification_frequency.always', value: 0 }, + { name: 'user.like_notification_frequency.first_time_and_daily', value: 1 }, + { name: 'user.like_notification_frequency.first_time', value: 2 }, + ] + end + + def self.translate_names? + true + end + +end diff --git a/app/models/user_option.rb b/app/models/user_option.rb index 0e40e9e81ef..8aaca554d2d 100644 --- a/app/models/user_option.rb +++ b/app/models/user_option.rb @@ -9,6 +9,10 @@ class UserOption < ActiveRecord::Base @previous_replies_type ||= Enum.new(always: 0, unless_emailed: 1, never: 2) end + def self.like_notification_frequency_type + @like_notification_frequency_type ||= Enum.new(always: 0, first_time_and_daily: 1, first_time: 2) + end + def set_defaults self.email_always = SiteSetting.default_email_always self.mailing_list_mode = SiteSetting.default_email_mailing_list_mode @@ -27,6 +31,8 @@ class UserOption < ActiveRecord::Base self.new_topic_duration_minutes = SiteSetting.default_other_new_topic_duration_minutes self.auto_track_topics_after_msecs = SiteSetting.default_other_auto_track_topics_after_msecs + self.like_notification_frequency = SiteSetting.default_other_like_notification_frequency + if SiteSetting.default_email_digest_frequency.to_i <= 0 self.email_digests = false diff --git a/app/serializers/user_option_serializer.rb b/app/serializers/user_option_serializer.rb index 413ee9c2edd..11edfbf2bd6 100644 --- a/app/serializers/user_option_serializer.rb +++ b/app/serializers/user_option_serializer.rb @@ -15,7 +15,8 @@ class UserOptionSerializer < ApplicationSerializer :auto_track_topics_after_msecs, :new_topic_duration_minutes, :email_previous_replies, - :email_in_reply_to + :email_in_reply_to, + :like_notification_frequency def include_edit_history_public? diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index ae507d4fe29..5f67f8f3c09 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -194,6 +194,29 @@ class PostAlerter # TODO decide if it makes sense to also publish a desktop notification end + def should_notify_edit?(notification, opts) + return existing_notification.data_hash["display_username"] != opts[:display_username] + end + + def should_notify_like?(user, notification) + + return true if user.user_option.like_notification_frequency == UserOption.like_notification_frequency_type[:always] + + return true if user.user_option.like_notification_frequency == UserOption.like_notification_frequency_type[:first_time_and_daily] && notification.created_at < 1.day.ago + + return false + end + + def should_notify_previous?(user, notification, opts) + type = notification.notification_type + if type == Notification.types[:edited] + return should_notify_edit?(notification, opts) + elsif type == Notification.types[:liked] + return should_notify_like?(user, notification) + end + return false + end + def create_notification(user, type, post, opts=nil) return if user.blank? return if user.id == Discourse::SYSTEM_USER_ID @@ -226,10 +249,7 @@ class PostAlerter post_number: post.post_number, notification_type: type) - if existing_notification - return unless existing_notification.notification_type == Notification.types[:edited] && - existing_notification.data_hash["display_username"] == opts[:display_username] - end + return if existing_notification && !should_notify_previous?(user, existing_notification, opts) collapsed = false diff --git a/app/services/user_updater.rb b/app/services/user_updater.rb index 0fa1f0b6fe9..d898640bb48 100644 --- a/app/services/user_updater.rb +++ b/app/services/user_updater.rb @@ -22,7 +22,8 @@ class UserUpdater :new_topic_duration_minutes, :auto_track_topics_after_msecs, :email_previous_replies, - :email_in_reply_to + :email_in_reply_to, + :like_notification_frequency ] def initialize(actor, user) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 1c33b99827d..47536fdab0e 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -627,6 +627,11 @@ en: website: "Web Site" email_settings: "Email" + like_notification_frequency: + title: "Notify when liked" + always: "Always" + first_time_and_daily: "First time a post is liked and daily" + first_time: "First time a post is liked" email_previous_replies: title: "Include previous replies at the bottom of emails" unless_emailed: "unless previously sent" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index e26431ed98f..dd707fe8ba2 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1258,6 +1258,8 @@ en: default_other_disable_jump_reply: "Don't jump to user's post after they reply by default." default_other_edit_history_public: "Make the post revisions public by default." + default_other_like_notification_frequency: "Notify users on likes by default" + default_topics_automatic_unpin: "Automatically unpin topics when the user reaches the bottom by default." default_categories_watching: "List of categories that are watched by default." diff --git a/config/site_settings.yml b/config/site_settings.yml index e868cb09215..14ecb6fce1e 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1084,6 +1084,9 @@ user_preferences: default_other_dynamic_favicon: false default_other_disable_jump_reply: false default_other_edit_history_public: false + default_other_like_notification_frequency: + enum: 'LikeNotificationFrequencySiteSetting' + default: 1 default_topics_automatic_unpin: default: true diff --git a/db/migrate/20160302104253_add_like_notification_frequency_to_user_options.rb b/db/migrate/20160302104253_add_like_notification_frequency_to_user_options.rb new file mode 100644 index 00000000000..d5c00d7305a --- /dev/null +++ b/db/migrate/20160302104253_add_like_notification_frequency_to_user_options.rb @@ -0,0 +1,5 @@ +class AddLikeNotificationFrequencyToUserOptions < ActiveRecord::Migration + def change + add_column :user_options, :like_notification_frequency, :integer, null: false, default: 1 + end +end diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb index f4cf7699c29..98d6e5f16ed 100644 --- a/spec/services/post_alerter_spec.rb +++ b/spec/services/post_alerter_spec.rb @@ -25,7 +25,7 @@ describe PostAlerter do end context 'likes' do - it 'does not double notify users on likes' do + it 'notifies on likes correctly' do ActiveRecord::Base.observers.enable :all post = Fabricate(:post, raw: 'I love waffles') @@ -38,6 +38,31 @@ describe PostAlerter do # one like and one edit notification expect(Notification.count(post_number: 1, topic_id: post.topic_id)).to eq(2) + + + post.user.user_option.update_columns(like_notification_frequency: + UserOption.like_notification_frequency_type[:always]) + + admin2 = Fabricate(:admin) + PostAction.act(admin2, post, PostActionType.types[:like]) + # two likes one edit + expect(Notification.count(post_number: 1, topic_id: post.topic_id)).to eq(3) + + post.user.user_option.update_columns(like_notification_frequency: + UserOption.like_notification_frequency_type[:first_time_and_daily]) + + # this gets skipped + admin3 = Fabricate(:admin) + PostAction.act(admin3, post, PostActionType.types[:like]) + + Timecop.freeze(2.days.from_now) do + admin4 = Fabricate(:admin) + PostAction.act(admin4, post, PostActionType.types[:like]) + end + + # first happend within the same day, no need to notify + expect(Notification.count(post_number: 1, topic_id: post.topic_id)).to eq(4) + end end