diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-channel-unread-indicator.gjs b/plugins/chat/assets/javascripts/discourse/components/chat-channel-unread-indicator.gjs index 86178c1f789..02a6a6d3036 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-channel-unread-indicator.gjs +++ b/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 { >
{{#if this.isUrgent - }}{{this.unreadCount}}{{else}} {{/if}}
+ }}{{this.urgentCount}}{{else}} {{/if}} {{/if}} diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js index 56a3a8e4ca4..7c5106402f3 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js +++ b/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) { - return new Date(a.lastMessage.createdAt) > - new Date(b.lastMessage.createdAt) + 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; - } else { - return a.tracking.unreadCount > b.tracking.unreadCount ? -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; }); } } diff --git a/plugins/chat/spec/fabricators/chat_fabricator.rb b/plugins/chat/spec/fabricators/chat_fabricator.rb index ee32819a44a..4e1cd9d4b91 100644 --- a/plugins/chat/spec/fabricators/chat_fabricator.rb +++ b/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 diff --git a/plugins/chat/spec/system/list_channels/drawer_spec.rb b/plugins/chat/spec/system/list_channels/drawer_spec.rb index 1a27e4a27fb..757d76baad4 100644 --- a/plugins/chat/spec/system/list_channels/drawer_spec.rb +++ b/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 diff --git a/plugins/chat/spec/system/page_objects/chat/components/channels_index.rb b/plugins/chat/spec/system/page_objects/chat/components/channels_index.rb index 342daa5ee88..f61b651ce86 100644 --- a/plugins/chat/spec/system/page_objects/chat/components/channels_index.rb +++ b/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?( diff --git a/plugins/chat/spec/system/page_objects/chat_drawer/chat_drawer.rb b/plugins/chat/spec/system/page_objects/chat_drawer/chat_drawer.rb index 3109b5160d9..a10d8503381 100644 --- a/plugins/chat/spec/system/page_objects/chat_drawer/chat_drawer.rb +++ b/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