From 0420be88a642dad4abd57bab8c37b36ba2e8c066 Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Tue, 21 Jan 2020 08:41:13 +1100 Subject: [PATCH] FIX: when tag or category is added notify users that topic was modified (#8750) There is a feature, that when tag or category is added to the topic, customers who are watching that category or tag are notified. The problem is that it is using default notification type "new post" It would be better to use "new post" only when there really is a new post and "edited" when categories or tags were modified. --- app/jobs/regular/notify_category_change.rb | 2 +- app/jobs/regular/notify_tag_change.rb | 2 +- app/services/post_alerter.rb | 7 +++++-- spec/jobs/notify_tag_change_spec.rb | 1 + spec/models/topic_spec.rb | 2 +- 5 files changed, 9 insertions(+), 5 deletions(-) diff --git a/app/jobs/regular/notify_category_change.rb b/app/jobs/regular/notify_category_change.rb index fbad7582e4a..d6bf0dc0c2c 100644 --- a/app/jobs/regular/notify_category_change.rb +++ b/app/jobs/regular/notify_category_change.rb @@ -7,7 +7,7 @@ module Jobs if post&.topic&.visible? post_alerter = PostAlerter.new - post_alerter.notify_post_users(post, User.where(id: args[:notified_user_ids]), include_tag_watchers: false) + post_alerter.notify_post_users(post, User.where(id: args[:notified_user_ids]), include_tag_watchers: false, new_record: false) post_alerter.notify_first_post_watchers(post, post_alerter.category_watchers(post.topic)) end end diff --git a/app/jobs/regular/notify_tag_change.rb b/app/jobs/regular/notify_tag_change.rb index dff6769de97..d88deaf965c 100644 --- a/app/jobs/regular/notify_tag_change.rb +++ b/app/jobs/regular/notify_tag_change.rb @@ -7,7 +7,7 @@ module Jobs if post&.topic&.visible? post_alerter = PostAlerter.new - post_alerter.notify_post_users(post, User.where(id: args[:notified_user_ids]), include_category_watchers: false) + post_alerter.notify_post_users(post, User.where(id: args[:notified_user_ids]), include_category_watchers: false, new_record: false) post_alerter.notify_first_post_watchers(post, post_alerter.tag_watchers(post.topic)) end end diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index 379c5b97741..6fc26df6d3a 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -550,7 +550,7 @@ class PostAlerter end end - def notify_post_users(post, notified, include_category_watchers: true, include_tag_watchers: true) + def notify_post_users(post, notified, include_category_watchers: true, include_tag_watchers: true, new_record: true) return unless post.topic warn_if_not_sidekiq @@ -607,9 +607,12 @@ class PostAlerter notify = notify.where("id NOT IN (?)", exclude_user_ids) if exclude_user_ids.present? DiscourseEvent.trigger(:before_create_notifications_for_users, notify, post) + + notification_type = new_record ? Notification.types[:posted] : Notification.types[:edited] + notify.pluck(:id).each do |user_id| user = User.find_by(id: user_id) - create_notification(user, Notification.types[:posted], post) + create_notification(user, notification_type, post) end end diff --git a/spec/jobs/notify_tag_change_spec.rb b/spec/jobs/notify_tag_change_spec.rb index 9d6e0053e3a..ee96235e2e7 100644 --- a/spec/jobs/notify_tag_change_spec.rb +++ b/spec/jobs/notify_tag_change_spec.rb @@ -24,6 +24,7 @@ describe ::Jobs::NotifyTagChange do notification = Notification.last expect(notification.user_id).to eq(user.id) expect(notification.topic_id).to eq(post.topic_id) + expect(notification.notification_type).to eq(Notification.types[:edited]) end it 'doesnt create notification for user watching category' do diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 6b1d7a070cb..8d320412817 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -1404,7 +1404,7 @@ describe Topic do user_id: user.id, topic_id: topic.id, post_number: 1, - notification_type: Notification.types[:posted] + notification_type: Notification.types[:edited] ).exists?).to eq(true) expect(Notification.where(