diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index 15098711e96..83ae94e17a9 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -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 diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb index 3a3bb0b888f..b1e5478bc00 100644 --- a/spec/services/post_alerter_spec.rb +++ b/spec/services/post_alerter_spec.rb @@ -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