From 43ef44127c49babeae4afb65878b93a67d3498a7 Mon Sep 17 00:00:00 2001 From: Osama Sayegh Date: Tue, 25 Oct 2022 11:53:35 +0300 Subject: [PATCH] UX: Send notification of type `replied` to topic author if they're watching the topic (#18684) Related to aeee7ed. Before the change in aeee7ed, notifications for direct replies to your posts and notifications for replies in watched topics looked the same in the notifications menu -- they both used the arrow icon. We decided in aeee7ed to distinguish them by changing "watched topics" notifications to use the bell icon because it was confusing for users who watch topics to see the same icon for direct replies and "watched topics". However, that change also means that non-power/new users who receive replies to topics _they create_ will get notifications with the bell icon because technically they're watching the topic, but the arrow icon is more appropriate for this case because we use it throughout the app to indicate "replies". This commit adds a special-case so that if a user is watching a topic AND the topic is created by them, they receive notifications with the arrow icon (type `replied`) instead of the bell icon (type `posted`) for new posts in the topic. Internal topic: t/79051. --- app/services/post_alerter.rb | 19 +++++++++++++++++-- spec/services/post_alerter_spec.rb | 21 +++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index 83ae94e17a9..edd0deb69bd 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -146,8 +146,15 @@ class PostAlerter # replies reply_to_user = post.reply_notification_target - if new_record && reply_to_user && !notified.include?(reply_to_user) && notify_about_reply?(post) - notified += notify_non_pm_users(reply_to_user, :replied, post) + if new_record + if reply_to_user && !notified.include?(reply_to_user) && notify_about_reply?(post) + notified += notify_non_pm_users(reply_to_user, :replied, post) + end + + topic_author = post.topic.user + if topic_author && !notified.include?(topic_author) && user_watching_topic?(topic_author, post.topic) + notified += notify_non_pm_users(topic_author, :replied, post) + end end DiscourseEvent.trigger(:post_alerter_before_quotes, post, new_record, notified) @@ -885,4 +892,12 @@ class PostAlerter def is_replying?(user, reply_to_user, quoted_users) reply_to_user == user || quoted_users.include?(user) end + + def user_watching_topic?(user, topic) + TopicUser.exists?( + user_id: user.id, + topic_id: topic.id, + notification_level: TopicUser.notification_levels[:watching] + ) + end end diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb index 67d1508ce22..4dac502dd8e 100644 --- a/spec/services/post_alerter_spec.rb +++ b/spec/services/post_alerter_spec.rb @@ -1322,6 +1322,27 @@ RSpec.describe PostAlerter do NotificationEmailer.expects(:process_notification).once expect { PostAlerter.post_created(reply) }.to change(user.notifications, :count).by(1) end + + it "creates a notification of type `replied` instead of `posted` for the topic author if they're watching the topic" do + Jobs.run_immediately! + + u1 = Fabricate(:admin) + u2 = Fabricate(:admin) + + topic = create_topic(user: u1) + + u1.notifications.destroy_all + + expect do + create_post(topic: topic, user: u2) + end.to change { u1.reload.notifications.count }.by(1) + expect(u1.notifications.exists?( + topic_id: topic.id, + notification_type: Notification.types[:replied], + post_number: 1, + read: false + )).to eq(true) + end end context "with category" do