FIX: Link notification to first unread post (#12868)

* FIX: Link notification to first unread post

If a topic with a few posts was posted in a watched category or with a
watched tag, the created notification would always point to the last
post, instead of pointing to the first one.

The root cause is that the query that fetched the first unread post
uses 'TopicUser' records and those are not created by default for
user watching a category or tag. In this case, it should use the
'CategoryUser' or 'TagUser' records.

* DEV: Use named bind variables
This commit is contained in:
Dan Ungureanu 2021-05-04 06:03:00 +03:00 committed by GitHub
parent 5cba86f321
commit d1d9f83304
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 60 additions and 6 deletions

View File

@ -178,12 +178,26 @@ class PostAlerter
SELECT last_read_post_number FROM topic_users tu SELECT last_read_post_number FROM topic_users tu
WHERE tu.user_id = ? AND tu.topic_id = ? ),0)', WHERE tu.user_id = ? AND tu.topic_id = ? ),0)',
user.id, topic.id) user.id, topic.id)
.where('reply_to_user_id = ? OR exists( .where('reply_to_user_id = :user_id
SELECT 1 from topic_users tu OR exists(SELECT 1 from topic_users tu
WHERE tu.user_id = ? AND WHERE tu.user_id = :user_id AND
tu.topic_id = ? AND tu.topic_id = :topic_id AND
notification_level = ? notification_level = :topic_level)
)', user.id, user.id, topic.id, TopicUser.notification_levels[:watching]) OR exists(SELECT 1 from category_users cu
WHERE cu.user_id = :user_id AND
cu.category_id = :category_id AND
notification_level = :category_level)
OR exists(SELECT 1 from tag_users tu
WHERE tu.user_id = :user_id AND
tu.tag_id IN (SELECT tag_id FROM topic_tags WHERE topic_id = :topic_id) AND
notification_level = :tag_level)',
user_id: user.id,
topic_id: topic.id,
category_id: topic.category_id,
topic_level: TopicUser.notification_levels[:watching],
category_level: CategoryUser.notification_levels[:watching],
tag_level: TagUser.notification_levels[:watching]
)
.where(topic_id: topic.id) .where(topic_id: topic.id)
end end

View File

@ -1101,6 +1101,26 @@ describe PostAlerter do
}.to add_notification(staged_member, :posted) }.to add_notification(staged_member, :posted)
.and not_add_notification(staged_non_member, :posted) .and not_add_notification(staged_non_member, :posted)
end end
it "does not update existing unread notification" do
category = Fabricate(:category)
CategoryUser.set_notification_level_for_category(user, CategoryUser.notification_levels[:watching], category.id)
topic = Fabricate(:topic, category: category)
post = Fabricate(:post, topic: topic)
PostAlerter.post_created(post)
notification = Notification.last
expect(notification.topic_id).to eq(topic.id)
expect(notification.post_number).to eq(1)
post = Fabricate(:post, topic: topic)
PostAlerter.post_created(post)
notification = Notification.last
expect(notification.topic_id).to eq(topic.id)
expect(notification.post_number).to eq(1)
notification_data = JSON.parse(notification.data)
expect(notification_data["display_username"]).to eq(I18n.t("embed.replies", count: 2))
end
end end
end end
@ -1118,6 +1138,26 @@ describe PostAlerter do
end end
expect(events).to include(event_name: :before_create_notifications_for_users, params: [[user], post]) expect(events).to include(event_name: :before_create_notifications_for_users, params: [[user], post])
end end
it "does not update existing unread notification" do
tag = Fabricate(:tag)
TagUser.change(user.id, tag.id, TagUser.notification_levels[:watching])
topic = Fabricate(:topic, tags: [tag])
post = Fabricate(:post, topic: topic)
PostAlerter.post_created(post)
notification = Notification.last
expect(notification.topic_id).to eq(topic.id)
expect(notification.post_number).to eq(1)
post = Fabricate(:post, topic: topic)
PostAlerter.post_created(post)
notification = Notification.last
expect(notification.topic_id).to eq(topic.id)
expect(notification.post_number).to eq(1)
notification_data = JSON.parse(notification.data)
expect(notification_data["display_username"]).to eq(I18n.t("embed.replies", count: 2))
end
end end
context "on change" do context "on change" do