From c45eb8a6183bd40b8657344b5d867642b836f898 Mon Sep 17 00:00:00 2001 From: Selase Krakani <849886+s3lase@users.noreply.github.com> Date: Thu, 8 Jun 2023 17:41:44 +0000 Subject: [PATCH] FIX: Create new PM notifications for `watching_first_post` groups (#21997) At the moment, PMs to groups with default notification level set to `watching_first_post` do not generate "emailable" notifications. This happens because, topic user notification level which is indirectly derived from the group's default notification level is set to `tracking` if the group's notification level happens to be `watching_first_post`. This leads to a `group_message_summary` notification being created instead of a `private_message` notification which results in no email alerts being sent when a topic is created. As this `watching_first_post` --> `tracking` switcheroo appears to be intentional instead being a bug, this change extends `PostAlerter`'s `notify_pm_users` method to create a `private_message` notification for first posts created in a `watching_first_post` group even if the topic user notification level is set to `tracking` --- app/services/post_alerter.rb | 15 ++++- spec/services/post_alerter_spec.rb | 89 ++++++++++++++++++++++++++++++ 2 files changed, 101 insertions(+), 3 deletions(-) diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index 1433369d863..4e533e09a05 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -196,7 +196,7 @@ class PostAlerter if new_record if post.topic.private_message? # private messages - notified += notify_pm_users(post, reply_to_user, quoted_users, notified) + notified += notify_pm_users(post, reply_to_user, quoted_users, notified, new_record) elsif notify_about_reply?(post) # posts notified += @@ -724,7 +724,7 @@ class PostAlerter end end - def notify_pm_users(post, reply_to_user, quoted_users, notified) + def notify_pm_users(post, reply_to_user, quoted_users, notified, new_record = false) return [] unless post.topic warn_if_not_sidekiq @@ -761,7 +761,12 @@ class PostAlerter when TopicUser.notification_levels[:watching] create_pm_notification(user, post, emails_to_skip_send) when TopicUser.notification_levels[:tracking] - if is_replying?(user, reply_to_user, quoted_users) + # TopicUser is the canonical source of topic notification levels, except for + # new topics created within a group with default notification level set to + # `watching_first_post`. TopicUser notification level is set to `tracking` + # for these. + if is_replying?(user, reply_to_user, quoted_users) || + (new_record && group_watched_first_post?(user, post)) create_pm_notification(user, post, emails_to_skip_send) else notify_group_summary(user, post.topic) @@ -1009,4 +1014,8 @@ class PostAlerter notification_level: TopicUser.notification_levels[:watching], ) end + + def group_watched_first_post?(user, post) + post.is_first_post? && group_watchers(post.topic).include?(user.id) + end end diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb index a06ef1b0e70..fe3b95bf49a 100644 --- a/spec/services/post_alerter_spec.rb +++ b/spec/services/post_alerter_spec.rb @@ -124,6 +124,15 @@ RSpec.describe PostAlerter do fab!(:group) do Fabricate(:group, users: [user2], name: "TestGroup", default_notification_level: 2) end + fab!(:watching_first_post_group) do + Fabricate( + :group, + name: "some_group", + users: [evil_trout, coding_horror], + messageable_level: Group::ALIAS_LEVELS[:everyone], + default_notification_level: NotificationLevels.all[:watching_first_post], + ) + end fab!(:pm) do Fabricate(:topic, archetype: "private_message", category_id: nil, allowed_groups: [group]) end @@ -334,6 +343,86 @@ RSpec.describe PostAlerter do ) }.not_to change(user2.notifications, :count) end + + context "with watching_first_post notification level" do + it "notifies group members of first post" do + post = + PostCreator.create!( + user, + title: "Hi there, welcome to my topic", + raw: "This is my awesome message", + archetype: Archetype.private_message, + target_group_names: watching_first_post_group.name, + ) + + PostAlerter.new.after_save_post(post, true) + + expect( + evil_trout + .notifications + .where(notification_type: Notification.types[:private_message]) + .count, + ).to eq(1) + expect( + coding_horror + .notifications + .where(notification_type: Notification.types[:private_message]) + .count, + ).to eq(1) + end + + it "doesn't notify group members of replies" do + post = + PostCreator.create!( + user, + title: "Hi there, welcome to my topic", + raw: "This is my awesome message", + archetype: Archetype.private_message, + target_group_names: watching_first_post_group.name, + ) + + expect( + evil_trout + .notifications + .where(notification_type: Notification.types[:private_message]) + .count, + ).to eq(0) + expect( + coding_horror + .notifications + .where(notification_type: Notification.types[:private_message]) + .count, + ).to eq(0) + + PostAlerter.new.after_save_post(post, true) + + expect( + evil_trout + .notifications + .where(notification_type: Notification.types[:private_message]) + .count, + ).to eq(1) + expect( + coding_horror + .notifications + .where(notification_type: Notification.types[:private_message]) + .count, + ).to eq(1) + + reply = + Fabricate( + :post, + raw: "Reply to PM", + user: user, + topic: post.topic, + reply_to_post_number: post.post_number, + ) + + expect do PostAlerter.new.after_save_post(reply, false) end.to_not change { + Notification.count + } + end + end end end