FIX: Create single notification per post and user (#18091)
A user could receive more than a notification for the same post if they watched both the categories and tags at different levels. This commit makes sure that only the watching notification is created. * Add DiscourseEvent before post notifications are created
This commit is contained in:
parent
a671fb4449
commit
0f70eae7b0
|
@ -110,6 +110,8 @@ class PostAlerter
|
|||
def after_save_post(post, new_record = false)
|
||||
notified = [post.user, post.last_editor].uniq
|
||||
|
||||
DiscourseEvent.trigger(:post_alerter_before_mentions, post, new_record, notified)
|
||||
|
||||
# mentions (users/groups)
|
||||
mentioned_groups, mentioned_users, mentioned_here = extract_mentions(post)
|
||||
|
||||
|
@ -139,6 +141,8 @@ class PostAlerter
|
|||
end
|
||||
end
|
||||
|
||||
DiscourseEvent.trigger(:post_alerter_before_replies, post, new_record, notified)
|
||||
|
||||
# replies
|
||||
reply_to_user = post.reply_notification_target
|
||||
|
||||
|
@ -146,25 +150,34 @@ class PostAlerter
|
|||
notified += notify_non_pm_users(reply_to_user, :replied, post)
|
||||
end
|
||||
|
||||
DiscourseEvent.trigger(:post_alerter_before_quotes, post, new_record, notified)
|
||||
|
||||
# quotes
|
||||
quoted_users = extract_quoted_users(post)
|
||||
notified += notify_non_pm_users(quoted_users - notified, :quoted, post)
|
||||
|
||||
DiscourseEvent.trigger(:post_alerter_before_linked, post, new_record, notified)
|
||||
|
||||
# linked
|
||||
linked_users = extract_linked_users(post)
|
||||
notified += notify_non_pm_users(linked_users - notified, :linked, post)
|
||||
|
||||
# private messages
|
||||
DiscourseEvent.trigger(:post_alerter_before_post, post, new_record, notified)
|
||||
|
||||
if new_record
|
||||
if post.topic.private_message?
|
||||
notify_pm_users(post, reply_to_user, quoted_users, notified)
|
||||
# private messages
|
||||
notified += notify_pm_users(post, reply_to_user, quoted_users, notified)
|
||||
elsif notify_about_reply?(post)
|
||||
notify_post_users(post, notified, new_record: new_record)
|
||||
# posts
|
||||
notified += notify_post_users(post, notified, new_record: new_record)
|
||||
end
|
||||
end
|
||||
|
||||
sync_group_mentions(post, mentioned_groups)
|
||||
|
||||
DiscourseEvent.trigger(:post_alerter_before_first_post, post, new_record, notified)
|
||||
|
||||
if new_record && post.post_number == 1
|
||||
topic = post.topic
|
||||
|
||||
|
@ -172,7 +185,7 @@ class PostAlerter
|
|||
watchers = category_watchers(topic) + tag_watchers(topic) + group_watchers(topic)
|
||||
# Notify only users who can see the topic
|
||||
watchers &= topic.all_allowed_users.pluck(:id) if post.topic.private_message?
|
||||
notify_first_post_watchers(post, watchers)
|
||||
notified += notify_first_post_watchers(post, watchers, notified)
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -199,8 +212,8 @@ class PostAlerter
|
|||
.pluck(:user_id)
|
||||
end
|
||||
|
||||
def notify_first_post_watchers(post, user_ids)
|
||||
return if user_ids.blank?
|
||||
def notify_first_post_watchers(post, user_ids, notified = nil)
|
||||
return [] if user_ids.blank?
|
||||
user_ids.uniq!
|
||||
|
||||
warn_if_not_sidekiq
|
||||
|
@ -208,11 +221,14 @@ class PostAlerter
|
|||
# Don't notify the OP and last editor
|
||||
user_ids -= [post.user_id, post.last_editor_id]
|
||||
users = User.where(id: user_ids).includes(:do_not_disturb_timings)
|
||||
users = users.where.not(id: notified.map(&:id)) if notified.present?
|
||||
|
||||
DiscourseEvent.trigger(:before_create_notifications_for_users, users, post)
|
||||
each_user_in_batches(users) do |user|
|
||||
create_notification(user, Notification.types[:watching_first_post], post)
|
||||
end
|
||||
|
||||
users
|
||||
end
|
||||
|
||||
def sync_group_mentions(post, mentioned_groups)
|
||||
|
@ -620,7 +636,7 @@ class PostAlerter
|
|||
end
|
||||
|
||||
def notify_pm_users(post, reply_to_user, quoted_users, notified)
|
||||
return unless post.topic
|
||||
return [] unless post.topic
|
||||
|
||||
warn_if_not_sidekiq
|
||||
|
||||
|
@ -749,7 +765,7 @@ class PostAlerter
|
|||
end
|
||||
|
||||
def notify_post_users(post, notified, group_ids: nil, include_topic_watchers: true, include_category_watchers: true, include_tag_watchers: true, new_record: false)
|
||||
return unless post.topic
|
||||
return [] unless post.topic
|
||||
|
||||
warn_if_not_sidekiq
|
||||
|
||||
|
@ -845,6 +861,8 @@ class PostAlerter
|
|||
opts[:display_username] = post.last_editor.username if notification_type == Notification.types[:edited]
|
||||
create_notification(user, notification_type, post, opts)
|
||||
end
|
||||
|
||||
notify
|
||||
end
|
||||
|
||||
def warn_if_not_sidekiq
|
||||
|
|
|
@ -2021,4 +2021,53 @@ RSpec.describe PostAlerter do
|
|||
expect(notification.topic).to eq(post.topic)
|
||||
expect(notification.post_number).to eq(1)
|
||||
end
|
||||
|
||||
it "does not create multiple notifications for same post" do
|
||||
category = Fabricate(:category)
|
||||
CategoryUser.set_notification_level_for_category(
|
||||
user,
|
||||
NotificationLevels.all[:tracking],
|
||||
category.id,
|
||||
)
|
||||
watching_first_post_tag = Fabricate(:tag)
|
||||
TagUser.change(user.id, watching_first_post_tag.id, TagUser.notification_levels[:watching_first_post])
|
||||
watching_tag = Fabricate(:tag)
|
||||
TagUser.change(user.id, watching_tag.id, TagUser.notification_levels[:watching])
|
||||
|
||||
post = create_post(category: category, tags: [watching_first_post_tag.name, watching_tag.name])
|
||||
expect { PostAlerter.new.after_save_post(post, true) }.to change { Notification.count }.by(1)
|
||||
|
||||
notification = Notification.last
|
||||
expect(notification.user).to eq(user)
|
||||
expect(notification.notification_type).to eq(Notification.types[:posted])
|
||||
expect(notification.topic).to eq(post.topic)
|
||||
expect(notification.post_number).to eq(1)
|
||||
end
|
||||
|
||||
it 'triggers all discourse events' do
|
||||
expected_events = [
|
||||
:post_alerter_before_mentions,
|
||||
:post_alerter_before_replies,
|
||||
:post_alerter_before_quotes,
|
||||
:post_alerter_before_linked,
|
||||
:post_alerter_before_post,
|
||||
:post_alerter_before_first_post,
|
||||
:post_alerter_after_save_post,
|
||||
]
|
||||
|
||||
events = DiscourseEvent.track_events do
|
||||
PostAlerter.new.after_save_post(post, true)
|
||||
end
|
||||
|
||||
# Expect all the notification events are called
|
||||
# There are some other events triggered from outside after_save_post
|
||||
expect(events.map { |e| e[:event_name] }).to include(*expected_events)
|
||||
|
||||
# Expect each notification event is called with the right parameters
|
||||
events.each do |event|
|
||||
if expected_events.include?(event[:event_name])
|
||||
expect(event[:params]).to eq([post, true, [post.user]])
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in New Issue