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`
This commit is contained in:
parent
aa2270e4c3
commit
c45eb8a618
|
@ -196,7 +196,7 @@ class PostAlerter
|
||||||
if new_record
|
if new_record
|
||||||
if post.topic.private_message?
|
if post.topic.private_message?
|
||||||
# private messages
|
# 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)
|
elsif notify_about_reply?(post)
|
||||||
# posts
|
# posts
|
||||||
notified +=
|
notified +=
|
||||||
|
@ -724,7 +724,7 @@ class PostAlerter
|
||||||
end
|
end
|
||||||
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
|
return [] unless post.topic
|
||||||
|
|
||||||
warn_if_not_sidekiq
|
warn_if_not_sidekiq
|
||||||
|
@ -761,7 +761,12 @@ class PostAlerter
|
||||||
when TopicUser.notification_levels[:watching]
|
when TopicUser.notification_levels[:watching]
|
||||||
create_pm_notification(user, post, emails_to_skip_send)
|
create_pm_notification(user, post, emails_to_skip_send)
|
||||||
when TopicUser.notification_levels[:tracking]
|
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)
|
create_pm_notification(user, post, emails_to_skip_send)
|
||||||
else
|
else
|
||||||
notify_group_summary(user, post.topic)
|
notify_group_summary(user, post.topic)
|
||||||
|
@ -1009,4 +1014,8 @@ class PostAlerter
|
||||||
notification_level: TopicUser.notification_levels[:watching],
|
notification_level: TopicUser.notification_levels[:watching],
|
||||||
)
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def group_watched_first_post?(user, post)
|
||||||
|
post.is_first_post? && group_watchers(post.topic).include?(user.id)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -124,6 +124,15 @@ RSpec.describe PostAlerter do
|
||||||
fab!(:group) do
|
fab!(:group) do
|
||||||
Fabricate(:group, users: [user2], name: "TestGroup", default_notification_level: 2)
|
Fabricate(:group, users: [user2], name: "TestGroup", default_notification_level: 2)
|
||||||
end
|
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
|
fab!(:pm) do
|
||||||
Fabricate(:topic, archetype: "private_message", category_id: nil, allowed_groups: [group])
|
Fabricate(:topic, archetype: "private_message", category_id: nil, allowed_groups: [group])
|
||||||
end
|
end
|
||||||
|
@ -334,6 +343,86 @@ RSpec.describe PostAlerter do
|
||||||
)
|
)
|
||||||
}.not_to change(user2.notifications, :count)
|
}.not_to change(user2.notifications, :count)
|
||||||
end
|
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
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue