UX: update chat channel sorting to include unread threads (#29617)

Adds channels with unread threads (watching/tracking) to the sorting logic for both public and direct message channels.

Previously channels with unread threads could easily be missed as we didn't bump them to the top when new thread replies were created.

We are also adding a blue unread badge next to DM channels when there is an unread thread, as previously they weren't appearing as unread within the DMs tab (they only showed within the My Threads section).

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

@@ -11,44 +11,45 @@ export default class ChatChannelUnreadIndicator extends Component {
   get showUnreadIndicator() {
     return (
       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.args.channel.isCategoryChannel &&
-        this.chat.activeChannel?.id !== this.args.channel.id &&
-        this.args.channel.unreadThreadsCountSinceLastViewed > 0)
+      this.args.channel.unreadThreadsCountSinceLastViewed > 0
     );
   }
 
-  get unreadCount() {
-    if (this.#hasChannelMentions()) {
+  get urgentCount() {
+    if (this.hasChannelMentions) {
       return this.args.channel.tracking.mentionCount;
     }
-    if (this.#hasWatchedThreads()) {
+    if (this.hasWatchedThreads) {
       return this.args.channel.tracking.watchedThreadsUnreadCount;
     }
     return this.args.channel.tracking.unreadCount;
   }
 
   get isUrgent() {
-    if (this.#onlyMentions()) {
-      return this.#hasChannelMentions();
+    if (this.onlyMentions) {
+      return this.hasChannelMentions;
     }
     return (
-      this.args.channel.isDirectMessageChannel ||
-      this.#hasChannelMentions() ||
-      this.#hasWatchedThreads()
+      this.isDirectMessage || this.hasChannelMentions || this.hasWatchedThreads
     );
   }
 
-  #hasChannelMentions() {
+  get isDirectMessage() {
+    return (
+      this.args.channel.isDirectMessageChannel &&
+      this.args.channel.tracking.unreadCount > 0
+    );
+  }
+
+  get hasChannelMentions() {
     return this.args.channel.tracking.mentionCount > 0;
   }
 
-  #hasWatchedThreads() {
+  get hasWatchedThreads() {
     return this.args.channel.tracking.watchedThreadsUnreadCount > 0;
   }
 
-  #onlyMentions() {
+  get onlyMentions() {
     return hasChatIndicator(this.currentUser).ONLY_MENTIONS;
   }
 
@@ -62,7 +63,7 @@ export default class ChatChannelUnreadIndicator extends Component {
       >
         <div class="chat-channel-unread-indicator__number">{{#if
             this.isUrgent
-          }}{{this.unreadCount}}{{else}}&nbsp;{{/if}}</div>
+          }}{{this.urgentCount}}{{else}}&nbsp;{{/if}}</div>
       </div>
     {{/if}}
   </template>

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

@@ -198,24 +198,37 @@ export default class ChatChannelsManager extends Service {
 
   #sortChannelsByActivity(channels) {
     return channels.sort((a, b) => {
-      // if both channels have mention count, sort by slug
-      // otherwise prioritize channel with mention count
-      if (a.tracking.mentionCount > 0 && b.tracking.mentionCount > 0) {
+      const stats = {
+        a: {
+          urgent:
+            a.tracking.mentionCount + a.tracking.watchedThreadsUnreadCount,
+          unread: a.tracking.unreadCount + a.threadsManager.unreadThreadCount,
+        },
+        b: {
+          urgent:
+            b.tracking.mentionCount + b.tracking.watchedThreadsUnreadCount,
+          unread: b.tracking.unreadCount + b.threadsManager.unreadThreadCount,
+        },
+      };
+
+      // if both channels have urgent count, sort by slug
+      // otherwise prioritize channel with urgent count
+      if (stats.a.urgent > 0 && stats.b.urgent > 0) {
         return a.slug?.localeCompare?.(b.slug);
       }
 
-      if (a.tracking.mentionCount > 0 || b.tracking.mentionCount > 0) {
-        return a.tracking.mentionCount > b.tracking.mentionCount ? -1 : 1;
+      if (stats.a.urgent > 0 || stats.b.urgent > 0) {
+        return stats.a.urgent > stats.b.urgent ? -1 : 1;
       }
 
-      // if both channels have unread count, sort by slug
+      // if both channels have unread messages or threads, sort by slug
       // otherwise prioritize channel with unread count
-      if (a.tracking.unreadCount > 0 && b.tracking.unreadCount > 0) {
+      if (stats.a.unread > 0 && stats.b.unread > 0) {
         return a.slug?.localeCompare?.(b.slug);
       }
 
-      if (a.tracking.unreadCount > 0 || b.tracking.unreadCount > 0) {
-        return a.tracking.unreadCount > b.tracking.unreadCount ? -1 : 1;
+      if (stats.a.unread > 0 || stats.b.unread > 0) {
+        return stats.a.unread > stats.b.unread ? -1 : 1;
       }
 
       return a.slug?.localeCompare?.(b.slug);
@@ -232,14 +245,30 @@ export default class ChatChannelsManager extends Service {
         return -1;
       }
 
-      if (a.tracking.unreadCount === b.tracking.unreadCount) {
+      if (
+        a.tracking.unreadCount + a.tracking.watchedThreadsUnreadCount > 0 ||
+        b.tracking.unreadCount + b.tracking.watchedThreadsUnreadCount > 0
+      ) {
+        return a.tracking.unreadCount + a.tracking.watchedThreadsUnreadCount >
+          b.tracking.unreadCount + b.tracking.watchedThreadsUnreadCount
+          ? -1
+          : 1;
+      }
+
+      if (
+        a.threadsManager.unreadThreadCount > 0 ||
+        b.threadsManager.unreadThreadCount > 0
+      ) {
+        return a.threadsManager.unreadThreadCount >
+          b.threadsManager.unreadThreadCount
+          ? -1
+          : 1;
+      }
+
       return new Date(a.lastMessage.createdAt) >
         new Date(b.lastMessage.createdAt)
         ? -1
         : 1;
-      } else {
-        return a.tracking.unreadCount > b.tracking.unreadCount ? -1 : 1;
-      }
     });
   }
 }

plugins/chat/spec/fabricators/chat_fabricator.rb

@@ -220,7 +220,12 @@ Fabricator(:chat_thread, class_name: "Chat::Thread") do
     thread.channel = original_message.chat_channel
   end
 
-  transient :with_replies, :channel, :original_message_user, :old_om, use_service: false
+  transient :with_replies,
+            :channel,
+            :original_message_user,
+            :old_om,
+            use_service: false,
+            notification_level: :tracking
 
   original_message do |attrs|
     Fabricate(
@@ -239,7 +244,7 @@ Fabricator(:chat_thread, class_name: "Chat::Thread") do
     attrs[:created_at] = 1.week.ago if transients[:old_om]
 
     thread.original_message.update!(**attrs)
-    thread.add(thread.original_message_user)
+    thread.add(thread.original_message_user, notification_level: transients[:notification_level])
 
     if transients[:with_replies]
       Fabricate

plugins/chat/spec/system/list_channels/drawer_spec.rb

@@ -36,8 +36,20 @@ RSpec.describe "List channels | Drawer", type: :system do
     context "when multiple channels are present" do
       fab!(:channel_1) { Fabricate(:category_channel, name: "a channel") }
       fab!(:channel_2) { Fabricate(:category_channel, name: "b channel") }
-      fab!(:channel_3) { Fabricate(:category_channel, name: "c channel") }
+      fab!(:channel_3) { Fabricate(:category_channel, name: "c channel", threading_enabled: true) }
       fab!(:channel_4) { Fabricate(:category_channel, name: "d channel") }
+      fab!(:message) do
+        Fabricate(:chat_message, chat_channel: channel_3, user: current_user, use_service: true)
+      end
+      fab!(:thread) do
+        Fabricate(
+          :chat_thread,
+          channel: channel_3,
+          original_message: message,
+          with_replies: 2,
+          use_service: true,
+        )
+      end
 
       before do
         channel_1.add(current_user)
@@ -46,32 +58,31 @@ RSpec.describe "List channels | Drawer", type: :system do
         channel_4.add(current_user)
       end
 
-      it "sorts them by urgent, unread, then by slug" do
+      it "sorts them by urgent, unread messages or threads, then by slug" do
         drawer_page.visit_index
 
         Fabricate(
           :chat_message,
-          chat_channel: channel_1,
-          created_at: 5.minutes.ago,
-          use_service: true,
-        )
-        Fabricate(
-          :chat_message,
-          chat_channel: channel_2,
-          created_at: 2.minutes.ago,
-          use_service: true,
-        )
-        Fabricate(
-          :chat_message,
-          chat_channel: channel_3,
+          chat_channel: channel_4,
           message: "@#{current_user.username}",
           use_service: true,
         )
+
+        expect(drawer_page).to have_channel_at_position(channel_4, 1)
+        expect(drawer_page).to have_channel_at_position(channel_3, 2)
+        expect(drawer_page).to have_channel_at_position(channel_1, 3)
+        expect(drawer_page).to have_channel_at_position(channel_2, 4)
+      end
+
+      it "sorts by slug when multiple channels have the same unread count" do
+        drawer_page.visit_index
+        Fabricate(:chat_message, chat_channel: channel_2, use_service: true)
         Fabricate(:chat_message, chat_channel: channel_4, use_service: true)
 
-        expect(drawer_page).to have_channel_at_position(channel_3, 1)
-        expect(drawer_page).to have_channel_at_position(channel_1, 2)
-        expect(drawer_page).to have_channel_at_position(channel_2, 3)
+        expect(drawer_page).to have_channel_at_position(channel_2, 1)
+        expect(drawer_page).to have_channel_at_position(channel_3, 2)
+        expect(drawer_page).to have_channel_at_position(channel_4, 3)
+        expect(drawer_page).to have_channel_at_position(channel_1, 4)
       end
     end
   end
@@ -108,19 +119,71 @@ RSpec.describe "List channels | Drawer", type: :system do
 
     context "when multiple channels are present" do
       fab!(:user_1) { Fabricate(:user) }
+      fab!(:user_2) { Fabricate(:user) }
+      fab!(:user_3) { Fabricate(:user) }
       fab!(:dm_channel_1) { Fabricate(:direct_message_channel, users: [current_user]) }
       fab!(:dm_channel_2) { Fabricate(:direct_message_channel, users: [current_user, user_1]) }
-
-      before do
-        Fabricate(:chat_message, chat_channel: dm_channel_2, user: user_1, use_service: true)
-      end
+      fab!(:dm_channel_3) { Fabricate(:direct_message_channel, users: [current_user, user_2]) }
+      fab!(:dm_channel_4) { Fabricate(:direct_message_channel, users: [current_user, user_3]) }
 
       it "sorts them by latest activity" do
         drawer_page.visit_index
         drawer_page.click_direct_messages
 
+        Fabricate(:chat_message, chat_channel: dm_channel_2, user: user_1, use_service: true)
+
+        # last message was read but the created at date is used for sorting
+        message =
+          Fabricate(:chat_message, chat_channel: dm_channel_4, user: user_3, use_service: true)
+        dm_channel_4.membership_for(current_user).mark_read!(message.id)
+
         expect(drawer_page).to have_channel_at_position(dm_channel_2, 1)
-        expect(drawer_page).to have_channel_at_position(dm_channel_1, 2)
+        expect(drawer_page).to have_urgent_channel(dm_channel_2)
+
+        expect(drawer_page).to have_channel_at_position(dm_channel_4, 2)
+        expect(drawer_page).to have_channel_at_position(dm_channel_1, 3)
+        expect(drawer_page).to have_channel_at_position(dm_channel_3, 4)
+      end
+
+      it "sorts channels with threads by urgency" do
+        drawer_page.visit_index
+        drawer_page.click_direct_messages
+
+        Fabricate(
+          :chat_thread,
+          notification_level: :watching,
+          original_message:
+            Fabricate(
+              :chat_message,
+              chat_channel: dm_channel_4,
+              user: current_user,
+              use_service: true,
+            ),
+          with_replies: 2,
+          use_service: true,
+        )
+
+        Fabricate(
+          :chat_thread,
+          original_message:
+            Fabricate(
+              :chat_message,
+              chat_channel: dm_channel_3,
+              user: current_user,
+              use_service: true,
+            ),
+          with_replies: 2,
+          use_service: true,
+        )
+
+        expect(drawer_page).to have_channel_at_position(dm_channel_4, 1)
+        expect(drawer_page).to have_urgent_channel(dm_channel_4)
+
+        expect(drawer_page).to have_channel_at_position(dm_channel_3, 2)
+        expect(drawer_page).to have_unread_channel(dm_channel_3)
+
+        expect(drawer_page).to have_channel_at_position(dm_channel_1, 3)
+        expect(drawer_page).to have_channel_at_position(dm_channel_2, 4)
       end
     end
   end

plugins/chat/spec/system/page_objects/chat/components/channels_index.rb

@@ -37,9 +37,17 @@ module PageObjects
           has_no_css?(channel_row_selector(channel))
         end
 
-        def has_unread_channel?(channel, count: nil, wait: Capybara.default_max_wait_time)
+        def has_unread_channel?(
+          channel,
+          count: nil,
+          urgent: false,
+          wait: Capybara.default_max_wait_time
+        )
           unread_indicator_selector =
             "#{channel_row_selector(channel)} .chat-channel-unread-indicator"
+
+          unread_indicator_selector += ".-urgent" if urgent
+
           has_css?(unread_indicator_selector) &&
             if count
               has_css?(

plugins/chat/spec/system/page_objects/chat_drawer/chat_drawer.rb

@@ -70,6 +70,10 @@ module PageObjects
         channels_index.has_no_unread_channel?(channel)
       end
 
+      def has_urgent_channel?(channel)
+        channels_index.has_unread_channel?(channel, urgent: true)
+      end
+
       def has_user_threads_section?
         has_css?("#c-footer-threads")
       end