From 4c9ceb97b8af944a76ad0fa06ee80bb47e4566da Mon Sep 17 00:00:00 2001 From: Osama Sayegh Date: Mon, 8 Dec 2025 00:41:14 +0300 Subject: [PATCH] FIX: Consider all eligible notification types when notifying about a post (#36472) --- app/services/post_alerter.rb | 20 +++++++++------- spec/services/post_alerter_spec.rb | 37 ++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index 80771ffd289..6bd50c7caab 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -338,11 +338,14 @@ class PostAlerter users = users.where.not(id: notified.map(&:id)) if notified.present? DiscourseEvent.trigger(:before_create_notifications_for_users, users, post) + received_notifications = [] each_user_in_batches(users) do |user| - create_notification(user, Notification.types[:watching_first_post], post) + if create_notification(user, Notification.types[:watching_first_post], post).present? + received_notifications << user + end end - users + received_notifications end def sync_group_mentions(post, mentioned_groups) @@ -798,9 +801,7 @@ class PostAlerter warn_if_not_sidekiq DiscourseEvent.trigger(:before_create_notifications_for_users, users, post) - users.each { |u| create_notification(u, Notification.types[type], post, opts) } - - users + users.select { |u| create_notification(u, Notification.types[type], post, opts).present? } end def pm_watching_users(post) @@ -843,7 +844,7 @@ class PostAlerter # flow will not be sent via group SMTP if it is enabled. users = indirectly_targeted_users(post).reject { |u| notified.include?(u) } DiscourseEvent.trigger(:before_create_notifications_for_users, users, post) - users.each do |user| + users.select do |user| case TopicUser.get(post.topic, user)&.notification_level when TopicUser.notification_levels[:watching] create_pm_notification(user, post, emails_to_skip_send) @@ -1044,6 +1045,7 @@ class PostAlerter .pluck(:user_id), ) + received_notifications = [] each_user_in_batches(notify) do |user| calculated_type = if !new_record && already_seen_user_ids.include?(user.id) @@ -1056,10 +1058,12 @@ class PostAlerter opts = {} opts[:display_username] = post.last_editor.username if calculated_type == Notification.types[:edited] - create_notification(user, calculated_type, post, opts) + if create_notification(user, calculated_type, post, opts).present? + received_notifications << user + end end - notify + received_notifications end def warn_if_not_sidekiq diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb index ebad9932d43..ddfbf723b0f 100644 --- a/spec/services/post_alerter_spec.rb +++ b/spec/services/post_alerter_spec.rb @@ -1931,6 +1931,26 @@ RSpec.describe PostAlerter do ).and not_add_notification(staged_non_member, :watching_category_or_tag) end + it "notifies even if the user has disabled link notifications" do + linked_post = Fabricate(:post, user: user) + topic = Fabricate(:topic, category: category) + + CategoryUser.set_notification_level_for_category( + user, + CategoryUser.notification_levels[:watching], + category.id, + ) + user.user_option.update!(notify_on_linked_posts: false) + + post = create_post(topic:, raw: "Check this out: #{linked_post.full_url}") + + expect(post.topic_links.first.link_post_id).to eq(linked_post.id) + expect { PostAlerter.post_created(post) }.to add_notification( + user, + :watching_category_or_tag, + ).and not_add_notification(user, :linked) + end + it "does not update existing unread notification" do CategoryUser.set_notification_level_for_category( user, @@ -2034,6 +2054,23 @@ RSpec.describe PostAlerter do expect { PostAlerter.post_created(post) }.to change { Notification.count }.by(1) end + + it "notifies even if the user has disabled link notifications" do + linked_post = Fabricate(:post, user: user) + tag = Fabricate(:tag) + topic = Fabricate(:topic, tags: [tag]) + + TagUser.change(user.id, tag.id, TagUser.notification_levels[:watching]) + user.user_option.update!(notify_on_linked_posts: false) + + post = create_post(topic:, raw: "Check this out: #{linked_post.full_url}") + + expect(post.topic_links.first.link_post_id).to eq(linked_post.id) + expect { PostAlerter.post_created(post) }.to add_notification( + user, + :watching_category_or_tag, + ).and not_add_notification(user, :linked) + end end context "with category and tags" do