From b2a727336e9ccaa9d5217bddd3f2b0955b76dbd2 Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Thu, 4 May 2023 17:28:51 +0200
Subject: [PATCH] FIX: Thread mention read state and notification links
 (#21385)

* FIX: Link to thread for mentions inside thread

When mentioning a user in a thread, when we send the
notification and display it in the UI we want the URL
of the notification to point to the thread URL to open
the panel, rather than the main channel which is confusing.

For now, we don't have a way to highlight the linked-to message
in the thread, we can revisit this later.

* FIX: Mark mention notifications read when thread opens

Since we have no scrolling/message visibility/thread membership
for now, when a user opens the thread panel we just want to mark
all mention notifications relating to messages in the thread
for the user as read.
---
 .../chat/api/thread_reads_controller.rb       | 11 +++
 .../app/jobs/regular/chat/notify_mentioned.rb | 11 ++-
 .../chat/action/mark_mentions_read.rb         | 14 ++-
 .../chat/mark_all_user_channels_read.rb       |  2 +-
 .../services/chat/update_user_last_read.rb    |  2 +-
 .../chat/update_user_thread_last_read.rb      | 61 +++++++++++++
 .../discourse/components/chat-thread.js       | 12 ++-
 .../discourse/initializers/chat-user-menu.js  | 11 ++-
 .../discourse/models/chat-channel.js          |  4 +-
 .../discourse/services/chat-api.js            | 14 +++
 .../widgets/chat-mention-notification-item.js | 12 ++-
 plugins/chat/config/routes.rb                 |  1 +
 .../regular/chat/notify_mentioned_spec.rb     | 38 +++++++-
 .../chat/api/thread_reads_controller_spec.rb  | 72 +++++++++++++++
 .../chat/update_user_thread_last_read_spec.rb | 91 +++++++++++++++++++
 .../user_menu_notifications/sidebar_spec.rb   | 25 +++++
 16 files changed, 365 insertions(+), 16 deletions(-)
 create mode 100644 plugins/chat/app/controllers/chat/api/thread_reads_controller.rb
 create mode 100644 plugins/chat/app/services/chat/update_user_thread_last_read.rb
 create mode 100644 plugins/chat/spec/requests/chat/api/thread_reads_controller_spec.rb
 create mode 100644 plugins/chat/spec/services/chat/update_user_thread_last_read_spec.rb

diff --git a/plugins/chat/app/controllers/chat/api/thread_reads_controller.rb b/plugins/chat/app/controllers/chat/api/thread_reads_controller.rb
new file mode 100644
index 00000000000..dfb72d9b70f
--- /dev/null
+++ b/plugins/chat/app/controllers/chat/api/thread_reads_controller.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+class Chat::Api::ThreadReadsController < Chat::ApiController
+  def update
+    params.require(%i[channel_id thread_id])
+
+    with_service(Chat::UpdateUserThreadLastRead) do
+      on_model_not_found(:thread) { raise Discourse::NotFound }
+    end
+  end
+end
diff --git a/plugins/chat/app/jobs/regular/chat/notify_mentioned.rb b/plugins/chat/app/jobs/regular/chat/notify_mentioned.rb
index e88798a74c0..3388d17973a 100644
--- a/plugins/chat/app/jobs/regular/chat/notify_mentioned.rb
+++ b/plugins/chat/app/jobs/regular/chat/notify_mentioned.rb
@@ -40,6 +40,8 @@ module Jobs
           is_direct_message_channel: @chat_channel.direct_message_channel?,
         }
 
+        data[:chat_thread_id] = @chat_message.thread_id if @chat_message.in_thread?
+
         if !@is_direct_message_channel
           data[:chat_channel_title] = @chat_channel.title(membership.user)
           data[:chat_channel_slug] = @chat_channel.slug
@@ -61,12 +63,19 @@ module Jobs
       end
 
       def build_payload_for(membership, identifier_type:)
+        post_url =
+          if @chat_message.in_thread?
+            @chat_message.thread.relative_url
+          else
+            "#{@chat_channel.relative_url}/#{@chat_message.id}"
+          end
+
         payload = {
           notification_type: ::Notification.types[:chat_mention],
           username: @creator.username,
           tag: ::Chat::Notifier.push_notification_tag(:mention, @chat_channel.id),
           excerpt: @chat_message.push_notification_excerpt,
-          post_url: "#{@chat_channel.relative_url}/#{@chat_message.id}",
+          post_url: post_url,
         }
 
         translation_prefix =
diff --git a/plugins/chat/app/services/chat/action/mark_mentions_read.rb b/plugins/chat/app/services/chat/action/mark_mentions_read.rb
index 602b227caf1..8d0207d63bb 100644
--- a/plugins/chat/app/services/chat/action/mark_mentions_read.rb
+++ b/plugins/chat/app/services/chat/action/mark_mentions_read.rb
@@ -10,7 +10,9 @@ module Chat
       #   marked as read.
       # @param [Integer] message_id Optional, used to limit the max message ID to mark
       #   mentions read for in the channel.
-      def self.call(user, channel_ids:, message_id: nil)
+      # @param [Integer] thread_id Optional, if provided then all notifications related
+      #   to messages in the thread will be marked as read.
+      def self.call(user, channel_ids:, message_id: nil, thread_id: nil)
         ::Notification
           .where(notification_type: Notification.types[:chat_mention])
           .where(user: user)
@@ -19,8 +21,14 @@ module Chat
           .joins("INNER JOIN chat_messages ON chat_mentions.chat_message_id = chat_messages.id")
           .where("chat_messages.chat_channel_id IN (?)", channel_ids)
           .then do |notifications|
-            break notifications if message_id.blank?
-            notifications.where("chat_messages.id <= ?", message_id)
+            break notifications if message_id.blank? && thread_id.blank?
+            break notifications.where("chat_messages.id <= ?", message_id) if message_id.present?
+            if thread_id.present?
+              notifications.where(
+                "chat_messages.id IN (SELECT id FROM chat_messages WHERE thread_id = ?)",
+                thread_id,
+              )
+            end
           end
           .update_all(read: true)
       end
diff --git a/plugins/chat/app/services/chat/mark_all_user_channels_read.rb b/plugins/chat/app/services/chat/mark_all_user_channels_read.rb
index c3a67338d85..9c6376fbc63 100644
--- a/plugins/chat/app/services/chat/mark_all_user_channels_read.rb
+++ b/plugins/chat/app/services/chat/mark_all_user_channels_read.rb
@@ -49,7 +49,7 @@ module Chat
     def mark_associated_mentions_as_read(guardian:, updated_memberships:, **)
       return if updated_memberships.empty?
 
-      Chat::Action::MarkMentionsRead.call(
+      ::Chat::Action::MarkMentionsRead.call(
         guardian.user,
         channel_ids: updated_memberships.map(&:channel_id),
       )
diff --git a/plugins/chat/app/services/chat/update_user_last_read.rb b/plugins/chat/app/services/chat/update_user_last_read.rb
index 5d2b66e60fb..3ba18cb0d4b 100644
--- a/plugins/chat/app/services/chat/update_user_last_read.rb
+++ b/plugins/chat/app/services/chat/update_user_last_read.rb
@@ -61,7 +61,7 @@ module Chat
     end
 
     def mark_associated_mentions_as_read(active_membership:, contract:, **)
-      Chat::Action::MarkMentionsRead.call(
+      ::Chat::Action::MarkMentionsRead.call(
         active_membership.user,
         channel_ids: [active_membership.chat_channel.id],
         message_id: contract.message_id,
diff --git a/plugins/chat/app/services/chat/update_user_thread_last_read.rb b/plugins/chat/app/services/chat/update_user_thread_last_read.rb
new file mode 100644
index 00000000000..70845090a8d
--- /dev/null
+++ b/plugins/chat/app/services/chat/update_user_thread_last_read.rb
@@ -0,0 +1,61 @@
+# frozen_string_literal: true
+
+module Chat
+  # Service responsible for marking messages in a thread
+  # as read. For now this just marks any mentions in the thread
+  # as read, but as we add user tracking state to threads it
+  # will work in a similar way to Chat::UpdateUserLastRead
+  #
+  # @example
+  #  Chat::UpdateUserThreadLastRead.call(channel_id: 2, thread_id: 3, guardian: guardian)
+  #
+  class UpdateUserThreadLastRead
+    include ::Service::Base
+
+    # @!method call(channel_id:, thread_id:, guardian:)
+    #   @param [Integer] channel_id
+    #   @param [Integer] thread_id
+    #   @param [Guardian] guardian
+    #   @return [Service::Base::Context]
+
+    contract
+    model :thread
+    policy :invalid_access
+    step :mark_associated_mentions_as_read
+    step :publish_new_last_read_to_clients
+
+    # @!visibility private
+    class Contract
+      attribute :thread_id, :integer
+      attribute :channel_id, :integer
+
+      validates :thread_id, :channel_id, presence: true
+    end
+
+    private
+
+    def fetch_thread(contract:, **)
+      ::Chat::Thread.find_by(id: contract.thread_id, channel_id: contract.channel_id)
+    end
+
+    def invalid_access(guardian:, thread:, **)
+      guardian.can_join_chat_channel?(thread.channel)
+    end
+
+    def mark_associated_mentions_as_read(thread:, guardian:, **)
+      ::Chat::Action::MarkMentionsRead.call(
+        guardian.user,
+        channel_ids: [thread.channel_id],
+        thread_id: thread.id,
+      )
+    end
+
+    def publish_new_last_read_to_clients(guardian:, thread:, **)
+      ::Chat::Publisher.publish_user_tracking_state(
+        guardian.user,
+        thread.channel_id,
+        thread.replies.last.id,
+      )
+    end
+  end
+end
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-thread.js b/plugins/chat/assets/javascripts/discourse/components/chat-thread.js
index 90993d746eb..baf84060e22 100644
--- a/plugins/chat/assets/javascripts/discourse/components/chat-thread.js
+++ b/plugins/chat/assets/javascripts/discourse/components/chat-thread.js
@@ -1,4 +1,5 @@
 import Component from "@glimmer/component";
+import { Promise } from "rsvp";
 import { tracked } from "@glimmer/tracking";
 import { action } from "@ember/object";
 import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message";
@@ -93,7 +94,7 @@ export default class ChatThreadPanel extends Component {
   @debounce(100)
   fetchMessages() {
     if (this._selfDeleted) {
-      return;
+      return Promise.resolve();
     }
 
     this.loadingMorePast = true;
@@ -138,6 +139,8 @@ export default class ChatThreadPanel extends Component {
         // } else if (messages.length) {
         //   this.scrollToMessage(messages.lastObject.id);
         // }
+        //
+        this.markThreadAsRead();
       })
       .catch(this.#handleErrors)
       .finally(() => {
@@ -179,6 +182,13 @@ export default class ChatThreadPanel extends Component {
     return [messages, results.meta];
   }
 
+  // NOTE: At some point we want to do this based on visible messages
+  // and scrolling; for now it's enough to do it when the thread panel
+  // opens/messages are loaded since we have no pagination for threads.
+  markThreadAsRead() {
+    return this.chatApi.markThreadAsRead(this.channel.id, this.thread.id);
+  }
+
   @action
   onSendMessage(message) {
     if (message.editing) {
diff --git a/plugins/chat/assets/javascripts/discourse/initializers/chat-user-menu.js b/plugins/chat/assets/javascripts/discourse/initializers/chat-user-menu.js
index ca30056fcda..b2c0d4ca8c5 100644
--- a/plugins/chat/assets/javascripts/discourse/initializers/chat-user-menu.js
+++ b/plugins/chat/assets/javascripts/discourse/initializers/chat-user-menu.js
@@ -59,9 +59,16 @@ export default {
                   title: this.notification.data.chat_channel_title,
                   slug: this.notification.data.chat_channel_slug,
                 });
-                return `/chat/c/${slug || "-"}/${
+
+                let notificationRoute = `/chat/c/${slug || "-"}/${
                   this.notification.data.chat_channel_id
-                }/${this.notification.data.chat_message_id}`;
+                }`;
+                if (this.notification.data.chat_thread_id) {
+                  notificationRoute += `/t/${this.notification.data.chat_thread_id}`;
+                } else {
+                  notificationRoute += `/${this.notification.data.chat_message_id}`;
+                }
+                return notificationRoute;
               }
 
               get linkTitle() {
diff --git a/plugins/chat/assets/javascripts/discourse/models/chat-channel.js b/plugins/chat/assets/javascripts/discourse/models/chat-channel.js
index 76ba2a1209b..8bdc0d26180 100644
--- a/plugins/chat/assets/javascripts/discourse/models/chat-channel.js
+++ b/plugins/chat/assets/javascripts/discourse/models/chat-channel.js
@@ -202,8 +202,8 @@ export default class ChatChannel extends RestModel {
       return;
     }
 
-    // TODO (martin) Change this to use chatApi service once we change this
-    // class not to use RestModel
+    // TODO (martin) Change this to use chatApi service markChannelAsRead once we change this
+    // class not to use RestModel.
     return ajax(`/chat/api/channels/${this.id}/read/${messageId}`, {
       method: "PUT",
     });
diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-api.js b/plugins/chat/assets/javascripts/discourse/services/chat-api.js
index 3c29d8041f3..d28d92663fc 100644
--- a/plugins/chat/assets/javascripts/discourse/services/chat-api.js
+++ b/plugins/chat/assets/javascripts/discourse/services/chat-api.js
@@ -407,6 +407,20 @@ export default class ChatApi extends Service {
     return this.#putRequest(`/channels/${channelId}/read/${messageId}`);
   }
 
+  /**
+   * Marks all messages and mentions in a thread as read. This is quite
+   * far-reaching for now, and is not granular since there is no membership/
+   * read state per-user for threads. In future this will be expanded to
+   * also pass message ID in the same way as markChannelAsRead
+   *
+   * @param {number} channelId - The ID of the channel for the thread being marked as read.
+   * @param {number} threadId - The ID of the thread being marked as read.
+   * @returns {Promise}
+   */
+  markThreadAsRead(channelId, threadId) {
+    return this.#putRequest(`/channels/${channelId}/threads/${threadId}/read`);
+  }
+
   get #basePath() {
     return "/chat/api";
   }
diff --git a/plugins/chat/assets/javascripts/discourse/widgets/chat-mention-notification-item.js b/plugins/chat/assets/javascripts/discourse/widgets/chat-mention-notification-item.js
index beeeb4c34f4..6b64fe8ed9e 100644
--- a/plugins/chat/assets/javascripts/discourse/widgets/chat-mention-notification-item.js
+++ b/plugins/chat/assets/javascripts/discourse/widgets/chat-mention-notification-item.js
@@ -48,9 +48,15 @@ const chatNotificationItem = {
       title: data.chat_channel_title,
       slug: data.chat_channel_slug,
     });
-    return `/chat/c/${slug || "-"}/${data.chat_channel_id}/${
-      data.chat_message_id
-    }`;
+
+    let notificationRoute = `/chat/c/${slug || "-"}/${data.chat_channel_id}`;
+    if (data.chat_thread_id) {
+      notificationRoute += `/t/${data.chat_thread_id}`;
+    } else {
+      notificationRoute += `/${data.chat_message_id}`;
+    }
+
+    return notificationRoute;
   },
 };
 
diff --git a/plugins/chat/config/routes.rb b/plugins/chat/config/routes.rb
index 53d1a723fb6..594f23f2b3c 100644
--- a/plugins/chat/config/routes.rb
+++ b/plugins/chat/config/routes.rb
@@ -29,6 +29,7 @@ Chat::Engine.routes.draw do
     get "/mentions/groups" => "hints#check_group_mentions", :format => :json
 
     get "/channels/:channel_id/threads/:thread_id" => "channel_threads#show"
+    put "/channels/:channel_id/threads/:thread_id/read" => "thread_reads#update"
 
     put "/channels/:channel_id/messages/:message_id/restore" => "channel_messages#restore"
     delete "/channels/:channel_id/messages/:message_id" => "channel_messages#destroy"
diff --git a/plugins/chat/spec/jobs/regular/chat/notify_mentioned_spec.rb b/plugins/chat/spec/jobs/regular/chat/notify_mentioned_spec.rb
index a72aa02b4bd..7ac9d674613 100644
--- a/plugins/chat/spec/jobs/regular/chat/notify_mentioned_spec.rb
+++ b/plugins/chat/spec/jobs/regular/chat/notify_mentioned_spec.rb
@@ -21,9 +21,20 @@ describe Jobs::Chat::NotifyMentioned do
     end
   end
 
-  def create_chat_message(channel: public_channel, author: user_1, mentioned_user: user_2)
+  def create_chat_message(
+    channel: public_channel,
+    author: user_1,
+    mentioned_user: user_2,
+    thread: nil
+  )
     message =
-      Fabricate(:chat_message, chat_channel: channel, user: author, created_at: 10.minutes.ago)
+      Fabricate(
+        :chat_message,
+        chat_channel: channel,
+        user: author,
+        created_at: 10.minutes.ago,
+        thread: thread,
+      )
     Fabricate(:chat_mention, chat_message: message, user: mentioned_user)
     message
   end
@@ -408,6 +419,29 @@ describe Jobs::Chat::NotifyMentioned do
         expect(desktop_notification.data[:translated_title]).to eq(payload_translated_title)
       end
 
+      context "when the mention is within a thread" do
+        before do
+          SiteSetting.enable_experimental_chat_threaded_discussions = true
+          public_channel.update!(threading_enabled: true)
+        end
+
+        fab!(:thread) { Fabricate(:chat_thread, channel: public_channel) }
+
+        it "uses the thread URL for the post_url in the desktop notification" do
+          message = create_chat_message(thread: thread)
+          desktop_notification =
+            track_desktop_notification(message: message, to_notify_ids_map: to_notify_ids_map)
+          expect(desktop_notification.data[:post_url]).to eq(thread.relative_url)
+        end
+
+        it "includes the thread ID in the core notification data" do
+          message = create_chat_message(thread: thread)
+          created_notification =
+            track_core_notification(message: message, to_notify_ids_map: to_notify_ids_map)
+          expect(created_notification.data_hash[:chat_thread_id]).to eq(thread.id)
+        end
+      end
+
       context "with private channels" do
         it "users a different translated title" do
           message = create_chat_message(channel: @personal_chat_channel)
diff --git a/plugins/chat/spec/requests/chat/api/thread_reads_controller_spec.rb b/plugins/chat/spec/requests/chat/api/thread_reads_controller_spec.rb
new file mode 100644
index 00000000000..4f1c0808c33
--- /dev/null
+++ b/plugins/chat/spec/requests/chat/api/thread_reads_controller_spec.rb
@@ -0,0 +1,72 @@
+# frozen_string_literal: true
+
+RSpec.describe Chat::Api::ThreadReadsController do
+  fab!(:current_user) { Fabricate(:user) }
+
+  before do
+    SiteSetting.chat_enabled = true
+    SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone]
+    sign_in(current_user)
+  end
+
+  describe "#update" do
+    describe "marking the thread messages as read" do
+      fab!(:channel) { Fabricate(:chat_channel) }
+      fab!(:other_user) { Fabricate(:user) }
+      fab!(:thread) { Fabricate(:chat_thread, channel: channel) }
+      fab!(:message_1) do
+        Fabricate(:chat_message, chat_channel: channel, user: other_user, thread: thread)
+      end
+      fab!(:message_2) do
+        Fabricate(:chat_message, chat_channel: channel, user: other_user, thread: thread)
+      end
+
+      context "when the user cannot access the channel" do
+        fab!(:channel) { Fabricate(:private_category_channel) }
+        fab!(:thread) { Fabricate(:chat_thread, channel: channel) }
+        it "raises invalid access" do
+          put "/chat/api/channels/#{channel.id}/threads/#{thread.id}/read.json"
+          expect(response.status).to eq(403)
+        end
+      end
+
+      context "when the channel_id and thread_id params do not match" do
+        it "raises a not found" do
+          put "/chat/api/channels/#{Fabricate(:chat_channel).id}/threads/#{thread.id}/read.json"
+          expect(response.status).to eq(404)
+        end
+      end
+
+      it "marks all mention notifications as read for the thread" do
+        notification_1 = create_notification_and_mention_for(current_user, other_user, message_1)
+        notification_2 = create_notification_and_mention_for(current_user, other_user, message_2)
+
+        put "/chat/api/channels/#{channel.id}/threads/#{thread.id}/read.json"
+        expect(response.status).to eq(200)
+        expect(notification_1.reload.read).to eq(true)
+        expect(notification_2.reload.read).to eq(true)
+      end
+    end
+  end
+
+  def create_notification_and_mention_for(user, sender, msg)
+    Notification
+      .create!(
+        notification_type: Notification.types[:chat_mention],
+        user: user,
+        high_priority: true,
+        read: false,
+        data: {
+          message: "chat.mention_notification",
+          chat_message_id: msg.id,
+          chat_channel_id: msg.chat_channel_id,
+          chat_channel_title: msg.chat_channel.title(user),
+          chat_channel_slug: msg.chat_channel.slug,
+          mentioned_by_username: sender.username,
+        }.to_json,
+      )
+      .tap do |notification|
+        Chat::Mention.create!(user: user, chat_message: msg, notification: notification)
+      end
+  end
+end
diff --git a/plugins/chat/spec/services/chat/update_user_thread_last_read_spec.rb b/plugins/chat/spec/services/chat/update_user_thread_last_read_spec.rb
new file mode 100644
index 00000000000..fcf8ae1863f
--- /dev/null
+++ b/plugins/chat/spec/services/chat/update_user_thread_last_read_spec.rb
@@ -0,0 +1,91 @@
+# frozen_string_literal: true
+
+RSpec.describe Chat::UpdateUserThreadLastRead do
+  describe Chat::UpdateUserThreadLastRead::Contract, type: :model do
+    it { is_expected.to validate_presence_of :channel_id }
+    it { is_expected.to validate_presence_of :thread_id }
+  end
+
+  describe ".call" do
+    subject(:result) { described_class.call(params) }
+
+    fab!(:current_user) { Fabricate(:user) }
+    fab!(:channel) { Fabricate(:chat_channel) }
+    fab!(:thread) { Fabricate(:chat_thread, channel: channel) }
+
+    let(:guardian) { Guardian.new(current_user) }
+    let(:params) { { guardian: guardian, channel_id: channel.id, thread_id: thread.id } }
+
+    context "when params are not valid" do
+      before { params.delete(:thread_id) }
+
+      it { is_expected.to fail_a_contract }
+    end
+
+    context "when params are valid" do
+      context "when user can’t access the channel" do
+        fab!(:channel) { Fabricate(:private_category_channel) }
+        fab!(:thread) { Fabricate(:chat_thread, channel: channel) }
+
+        it { is_expected.to fail_a_policy(:invalid_access) }
+      end
+
+      context "when thread cannot be found" do
+        before { params[:channel_id] = Fabricate(:chat_channel).id }
+
+        it { is_expected.to fail_to_find_a_model(:thread) }
+      end
+
+      context "when everything is fine" do
+        fab!(:notification_1) do
+          Fabricate(
+            :notification,
+            notification_type: Notification.types[:chat_mention],
+            user: current_user,
+          )
+        end
+        fab!(:notification_2) do
+          Fabricate(
+            :notification,
+            notification_type: Notification.types[:chat_mention],
+            user: current_user,
+          )
+        end
+
+        let(:messages) { MessageBus.track_publish { result } }
+
+        before do
+          Jobs.run_immediately!
+          Chat::Mention.create!(
+            notification: notification_1,
+            user: current_user,
+            chat_message: Fabricate(:chat_message, chat_channel: channel, thread: thread),
+          )
+          Chat::Mention.create!(
+            notification: notification_2,
+            user: current_user,
+            chat_message: Fabricate(:chat_message, chat_channel: channel, thread: thread),
+          )
+        end
+
+        it "sets the service result as successful" do
+          expect(result).to be_a_success
+        end
+
+        it "marks existing notifications related to all messages in the thread as read" do
+          expect { result }.to change {
+            Notification.where(
+              notification_type: Notification.types[:chat_mention],
+              user: current_user,
+              read: false,
+            ).count
+          }.by(-2)
+        end
+
+        it "publishes new last read to clients" do
+          expect(messages.map(&:channel)).to include("/chat/user-tracking-state/#{current_user.id}")
+        end
+      end
+    end
+  end
+end
diff --git a/plugins/chat/spec/system/user_menu_notifications/sidebar_spec.rb b/plugins/chat/spec/system/user_menu_notifications/sidebar_spec.rb
index a9fe1464dea..3bfa6ad19ac 100644
--- a/plugins/chat/spec/system/user_menu_notifications/sidebar_spec.rb
+++ b/plugins/chat/spec/system/user_menu_notifications/sidebar_spec.rb
@@ -129,6 +129,31 @@ RSpec.describe "User menu notifications | sidebar", type: :system, js: true do
             href: "/chat/c/#{channel_1.slug}/#{channel_1.id}/#{message.id}",
           )
         end
+
+        it "shows a mention notification when the message is in a thread" do
+          Jobs.run_immediately!
+
+          message =
+            Chat::MessageCreator.create(
+              chat_channel: channel_1,
+              user: other_user,
+              content: "this is fine @#{current_user.username}",
+              thread_id: Fabricate(:chat_thread, channel: channel_1).id,
+            ).chat_message
+
+          visit("/")
+
+          find(".header-dropdown-toggle.current-user").click
+          within("#user-menu-button-chat-notifications") do |panel|
+            expect(panel).to have_content(1)
+            panel.click
+          end
+
+          expect(find("#quick-access-chat-notifications")).to have_link(
+            I18n.t("js.notifications.popup.chat_mention.direct", channel: channel_1.name),
+            href: "/chat/c/#{channel_1.slug}/#{channel_1.id}/t/#{message.thread_id}",
+          )
+        end
       end
 
       context "when @all" do