From d1d9f83304818b52b4bb40f3af42bcd0acb851c1 Mon Sep 17 00:00:00 2001
From: Dan Ungureanu <dan@ungureanu.me>
Date: Tue, 4 May 2021 06:03:00 +0300
Subject: [PATCH] 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
---
 app/services/post_alerter.rb       | 26 ++++++++++++++-----
 spec/services/post_alerter_spec.rb | 40 ++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 6 deletions(-)

diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb
index c062bb6b448..77ef4146025 100644
--- a/app/services/post_alerter.rb
+++ b/app/services/post_alerter.rb
@@ -178,12 +178,26 @@ class PostAlerter
                SELECT last_read_post_number FROM topic_users tu
                WHERE tu.user_id = ? AND tu.topic_id = ? ),0)',
                 user.id, topic.id)
-      .where('reply_to_user_id = ? OR exists(
-            SELECT 1 from topic_users tu
-            WHERE tu.user_id = ? AND
-              tu.topic_id = ? AND
-              notification_level = ?
-            )', user.id, user.id, topic.id, TopicUser.notification_levels[:watching])
+      .where('reply_to_user_id = :user_id
+        OR exists(SELECT 1 from topic_users tu
+                  WHERE tu.user_id = :user_id AND
+                    tu.topic_id = :topic_id AND
+                    notification_level = :topic_level)
+        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)
   end
 
diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb
index 8d492c14b73..86ca47e0187 100644
--- a/spec/services/post_alerter_spec.rb
+++ b/spec/services/post_alerter_spec.rb
@@ -1101,6 +1101,26 @@ describe PostAlerter do
         }.to add_notification(staged_member, :posted)
           .and not_add_notification(staged_non_member, :posted)
       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
 
@@ -1118,6 +1138,26 @@ describe PostAlerter do
         end
         expect(events).to include(event_name: :before_create_notifications_for_users, params: [[user], post])
       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
 
     context "on change" do