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

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
diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb
index c062bb6..77ef414 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 8d492c1..86ca47e 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

GitHub sha: d1d9f833

This commit appears in #12868 which was approved by SamSaffron. It was merged by SamSaffron.