From e7b49c42c4c9bc050ad04aa5c44e4f26447c4726 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Wed, 16 Jan 2019 16:17:04 +0800 Subject: [PATCH] FIX: Allow liked notifications consolidation to be disabled. --- app/services/post_action_notifier.rb | 41 ++++++++++++++++------------ config/locales/server.en.yml | 2 +- config/site_settings.yml | 4 +-- spec/models/post_action_spec.rb | 30 ++++++++++++++++---- 4 files changed, 52 insertions(+), 25 deletions(-) diff --git a/app/services/post_action_notifier.rb b/app/services/post_action_notifier.rb index 698af277677..c392d9ff623 100644 --- a/app/services/post_action_notifier.rb +++ b/app/services/post_action_notifier.rb @@ -76,6 +76,10 @@ class PostActionNotifier post = post_action.post return if post_action.user.blank? + if SiteSetting.likes_notification_consolidation_threshold.zero? + return create_liked_notification(alerter, post, post_action) + end + user_notifications = post.user.notifications consolidation_window = @@ -114,27 +118,30 @@ class PostActionNotifier post_action ) else - alerter.create_notification( - post.user, - Notification.types[:liked], - post, - display_username: post_action.user.username, - post_action_id: post_action.id, - user_id: post_action.user_id - ) + create_liked_notification(alerter, post, post_action) end end - def self.update_consolidated_liked_notification_count!(notification) - Notification.transaction do - data = JSON.parse(notification.data) - data["count"] += 1 + def self.create_liked_notification(alerter, post, post_action) + alerter.create_notification( + post.user, + Notification.types[:liked], + post, + display_username: post_action.user.username, + post_action_id: post_action.id, + user_id: post_action.user_id + ) + end + private_class_method :create_liked_notification - notification.update!( - data: data.to_json, - read: false - ) - end + def self.update_consolidated_liked_notification_count!(notification) + data = JSON.parse(notification.data) + data["count"] += 1 + + notification.update!( + data: data.to_json, + read: false + ) end private_class_method :update_consolidated_liked_notification_count! diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 8939c16d631..a43051d6aaa 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1789,7 +1789,7 @@ en: disable_edit_notifications: "Disables edit notifications by the system user when 'download_remote_images_to_local' is active." - likes_notification_consolidation_threshold: "Number of liked notifications received before the notifications are consolidated into a single one. The window can be configured via `SiteSetting.likes_notification_consolidation_window_mins`." + likes_notification_consolidation_threshold: "Number of liked notifications received before the notifications are consolidated into a single one. Set to 0 to disable. The window can be configured via `SiteSetting.likes_notification_consolidation_window_mins`." likes_notification_consolidation_window_mins: "Duration in minutes where liked notifications are consolidated into a single notification once the threshold has been reaced. The threshold can be configured via `SiteSetting.likes_notification_consolidation_threshold`." diff --git a/config/site_settings.yml b/config/site_settings.yml index d788ca9e8d0..2f2ce61ff35 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1702,7 +1702,7 @@ uncategorized: likes_notification_consolidation_threshold: default: 5 - min: 3 + min: 0 likes_notification_consolidation_window_mins: default: 120 @@ -1819,7 +1819,7 @@ user_preferences: default_categories_watching_first_post: type: category_list default: '' - + default_text_size: type: enum default: normal diff --git a/spec/models/post_action_spec.rb b/spec/models/post_action_spec.rb index de18c4659b6..5100f9768b3 100644 --- a/spec/models/post_action_spec.rb +++ b/spec/models/post_action_spec.rb @@ -300,15 +300,35 @@ describe PostAction do let(:liker) { Fabricate(:user) } let(:likee) { Fabricate(:user) } - before do - SiteSetting.likes_notification_consolidation_threshold = 3 + it "can be disabled" do + SiteSetting.likes_notification_consolidation_threshold = 0 + + expect do + PostAction.act( + liker, + Fabricate(:post, user: likee), + PostActionType.types[:like] + ) + end.to change { likee.reload.notifications.count }.by(1) + + SiteSetting.likes_notification_consolidation_threshold = 1 + + expect do + PostAction.act( + liker, + Fabricate(:post, user: likee), + PostActionType.types[:like] + ) + end.to_not change { likee.reload.notifications.count } end it 'should consolidate likes notification when the threshold is reached' do + SiteSetting.likes_notification_consolidation_threshold = 2 + freeze_time expect do - 4.times do + 3.times do PostAction.act( liker, Fabricate(:post, user: likee), @@ -327,7 +347,7 @@ describe PostAction do expect(data["username"]).to eq(liker.username) expect(data["display_username"]).to eq(liker.username) - expect(data["count"]).to eq(4) + expect(data["count"]).to eq(3) notification.update!(read: true) @@ -344,7 +364,7 @@ describe PostAction do data = JSON.parse(notification.reload.data) expect(notification.read).to eq(false) - expect(data["count"]).to eq(6) + expect(data["count"]).to eq(5) # Like from a different user shouldn't be consolidated expect do