FIX: show chat thread notifications for direct message channels (#29414)

When adding threads to DM channels in #29170 we intentionally didn't add them to the My Threads section. However this makes it easy to miss notifications as we don't get the new thread badge on the sidebar and footer tabs (drawer/mobile). However they were also missing from the chat header and sidebar too, which is fixed with this PR.

When a new thread or a reply to an existing thread is created within a DM channel (either 1:1 or group), we now show the standard badges like we do for public channels.

We now also show the green dot in the sidebar for My Threads and public channels when they contain an unread watched thread.

plugins/chat/app/queries/chat/thread_unreads_query.rb

@@ -46,7 +46,7 @@ module Chat
           INNER JOIN user_chat_thread_memberships ON user_chat_thread_memberships.thread_id = chat_threads.id
           INNER JOIN user_chat_channel_memberships ON user_chat_channel_memberships.chat_channel_id = chat_messages.chat_channel_id
           INNER JOIN chat_messages AS original_message ON original_message.id = chat_threads.original_message_id
-          AND chat_messages.thread_id = memberships.thread_id
+          WHERE chat_messages.thread_id = memberships.thread_id
           AND chat_messages.user_id != :user_id
           AND user_chat_thread_memberships.user_id = :user_id
           AND chat_messages.id > COALESCE(user_chat_thread_memberships.last_read_message_id, 0)

plugins/chat/app/services/chat/list_user_channels.rb

@@ -24,9 +24,11 @@ module Chat
     end
 
     def inject_unread_thread_overview(structured:, guardian:)
+      channel_ids =
+        structured[:public_channels].map(&:id) + structured[:direct_message_channels].map(&:id)
       structured[:unread_thread_overview] = ::Chat::TrackingStateReportQuery.call(
         guardian: guardian,
-        channel_ids: structured[:public_channels].map(&:id),
+        channel_ids: channel_ids,
         include_threads: true,
         include_read: false,
         include_last_reply_details: true,

plugins/chat/app/services/chat/lookup_user_threads.rb

@@ -77,13 +77,7 @@ module Chat
             ::Chat::Channel
               .joins(:user_chat_channel_memberships)
               .where(user_chat_channel_memberships: { user_id: guardian.user.id, following: true })
-              .where(
+              .where({ threading_enabled: true, status: ::Chat::Channel.statuses[:open] })
-                {
-                  chatable_type: "Category",
-                  threading_enabled: true,
-                  status: ::Chat::Channel.statuses[:open],
-                },
-              )
               .select(:id),
         )
         .where("original_message.chat_channel_id = chat_threads.channel_id")

plugins/chat/assets/javascripts/discourse/components/chat-channel-unread-indicator.gjs

@@ -13,7 +13,8 @@ export default class ChatChannelUnreadIndicator extends Component {
       this.args.channel.tracking.unreadCount > 0 ||
       // We want to do this so we don't show a blue dot if the user is inside
       // the channel and a new unread thread comes in.
-      (this.chat.activeChannel?.id !== this.args.channel.id &&
+      (this.args.channel.isCategoryChannel &&
+        this.chat.activeChannel?.id !== this.args.channel.id &&
         this.args.channel.unreadThreadsCountSinceLastViewed > 0)
     );
   }

plugins/chat/assets/javascripts/discourse/initializers/chat-sidebar.js

@@ -63,7 +63,6 @@ export default {
             prefixType = "icon";
             prefixValue = "discourse-threads";
             suffixType = "icon";
-            suffixCSSClass = "unread";
 
             constructor() {
               super(...arguments);
@@ -74,12 +73,20 @@ export default {
             }
 
             get suffixValue() {
-              return chatChannelsManager.publicMessageChannels.some(
+              return chatChannelsManager.allChannels.some(
                 (channel) => channel.unreadThreadsCount > 0
               )
                 ? "circle"
                 : "";
             }
+
+            get suffixCSSClass() {
+              return chatChannelsManager.allChannels.some(
+                (channel) => channel.tracking.watchedThreadsUnreadCount > 0
+              )
+                ? "urgent"
+                : "unread";
+            }
           };
 
           const SidebarChatMyThreadsSection = class extends BaseCustomSidebarSection {
@@ -196,7 +203,8 @@ export default {
               }
 
               get suffixCSSClass() {
-                return this.channel.tracking.mentionCount > 0
+                return this.channel.tracking.mentionCount > 0 ||
+                  this.channel.tracking.watchedThreadsUnreadCount > 0
                   ? "urgent"
                   : "unread";
               }

plugins/chat/assets/javascripts/discourse/models/chat-channel.js

@@ -111,7 +111,7 @@ export default class ChatChannel {
   }
 
   get unreadThreadsCount() {
-    return Array.from(this.threadsManager.unreadThreadOverview.values()).length;
+    return this.threadsManager.unreadThreadCount;
   }
 
   get watchedThreadsUnreadCount() {

plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js

@@ -108,9 +108,7 @@ export default class ChatChannelsManager extends Service {
 
   @cached
   get hasThreadedChannels() {
-    return this.publicMessageChannels?.some(
+    return this.allChannels?.some((channel) => channel.threadingEnabled);
-      (channel) => channel.threadingEnabled
-    );
   }
 
   get allChannels() {

plugins/chat/assets/javascripts/discourse/services/chat-tracking-state-manager.js

@@ -78,13 +78,11 @@ export default class ChatTrackingStateManager extends Service {
   }
 
   get hasUnreadThreads() {
-    return this.#publicChannels.some(
+    return this.#allChannels.some((channel) => channel.unreadThreadsCount > 0);
-      (channel) => channel.unreadThreadsCount > 0
-    );
   }
 
   get watchedThreadsUnreadCount() {
-    return this.#publicChannels.reduce((unreadCount, channel) => {
+    return this.#allChannels.reduce((unreadCount, channel) => {
       return unreadCount + channel.tracking.watchedThreadsUnreadCount;
     }, 0);
   }
@@ -129,4 +127,8 @@ export default class ChatTrackingStateManager extends Service {
   get #directMessageChannels() {
     return this.chatChannelsManager.directMessageChannels;
   }
+
+  get #allChannels() {
+    return this.chatChannelsManager.allChannels;
+  }
 }

plugins/chat/spec/services/chat/create_thread_spec.rb

@@ -11,8 +11,11 @@ RSpec.describe Chat::CreateThread do
     subject(:result) { described_class.call(params:, **dependencies) }
 
     fab!(:current_user) { Fabricate(:user) }
+    fab!(:another_user) { Fabricate(:user) }
     fab!(:channel_1) { Fabricate(:chat_channel, threading_enabled: true) }
     fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1) }
+    fab!(:dm_channel) { Fabricate(:direct_message_channel, users: [current_user, another_user]) }
+    fab!(:dm_message) { Fabricate(:chat_message, chat_channel: dm_channel) }
 
     let(:guardian) { Guardian.new(current_user) }
     let(:title) { nil }
@@ -39,11 +42,20 @@ RSpec.describe Chat::CreateThread do
         expect(result.membership).to eq(result.thread.membership_for(current_user))
       end
 
-      it "publishes a `thread_created` MessageBus event" do
+      it "publishes a `thread_created` MessageBus event for public channels" do
         message = MessageBus.track_publish("/chat/#{channel_1.id}") { result }.first
         expect(message.data["type"]).to eq("thread_created")
       end
 
+      it "publishes a `thread_created` MessageBus event for DM channels" do
+        params[:channel_id] = dm_channel.id
+        params[:original_message_id] = dm_message.id
+        params[:guardian] = Guardian.new(another_user)
+        message = MessageBus.track_publish("/chat/#{dm_channel.id}") { result }.first
+
+        expect(message.data["type"]).to eq("thread_created")
+      end
+
       it "triggers a discourse event `chat_thread_created`" do
         event = DiscourseEvent.track_events(:chat_thread_created) { result }.first
 

plugins/chat/spec/system/chat_footer_spec.rb

@@ -133,36 +133,64 @@ RSpec.describe "Mobile Chat footer", type: :system, mobile: true do
     end
 
     context "for my threads" do
-      fab!(:thread) { Fabricate(:chat_thread, channel: channel, original_message: message) }
+      context "with public channels" do
-      fab!(:thread_message) { Fabricate(:chat_message, chat_channel: channel, thread: thread) }
+        fab!(:thread) { Fabricate(:chat_thread, channel: channel, original_message: message) }
+        fab!(:thread_message) { Fabricate(:chat_message, chat_channel: channel, thread: thread) }
 
-      before { SiteSetting.chat_threads_enabled = true }
+        before { SiteSetting.chat_threads_enabled = true }
 
-      it "is unread" do
+        it "is unread" do
-        visit("/")
+          visit("/")
-        chat_page.open_from_header
+          chat_page.open_from_header
 
-        expect(page).to have_css("#c-footer-threads .c-unread-indicator")
+          expect(page).to have_css("#c-footer-threads .c-unread-indicator")
+        end
+
+        it "is not unread when thread is from a muted channel" do
+          channel.membership_for(current_user).update!(muted: true)
+
+          visit("/")
+          chat_page.open_from_header
+
+          expect(page).to have_no_css("#c-footer-threads .c-unread-indicator")
+        end
+
+        it "is urgent for watched thread messages" do
+          thread.membership_for(current_user).update!(
+            notification_level: ::Chat::NotificationLevels.all[:watching],
+          )
+
+          visit("/")
+          chat_page.open_from_header
+
+          expect(page).to have_css("#c-footer-threads .c-unread-indicator.-urgent")
+        end
       end
 
-      it "is not unread when thread is from a muted channel" do
+      context "with direct messages" do
-        channel.membership_for(current_user).update!(muted: true)
+        fab!(:dm_channel) do
+          Fabricate(
+            :direct_message_channel,
+            threading_enabled: true,
+            users: [current_user, other_user],
+          )
+        end
+        fab!(:dm_message) { Fabricate(:chat_message, chat_channel: dm_channel, user: current_user) }
+        fab!(:dm_thread) do
+          Fabricate(:chat_thread, channel: dm_channel, original_message: dm_message)
+        end
+        fab!(:dm_thread_message) do
+          Fabricate(:chat_message, chat_channel: dm_channel, thread: dm_thread, user: other_user)
+        end
 
-        visit("/")
+        before { SiteSetting.chat_threads_enabled = true }
-        chat_page.open_from_header
 
-        expect(page).to have_no_css("#c-footer-threads .c-unread-indicator")
+        it "is unread" do
-      end
+          visit("/")
+          chat_page.open_from_header
 
-      it "is urgent for watched thread messages" do
+          expect(page).to have_css("#c-footer-threads .c-unread-indicator")
-        thread.membership_for(current_user).update!(
+        end
-          notification_level: ::Chat::NotificationLevels.all[:watching],
-        )
-
-        visit("/")
-        chat_page.open_from_header
-
-        expect(page).to have_css("#c-footer-threads .c-unread-indicator.-urgent")
       end
     end
   end