From 0012d9626f9f8379e267fe055c3b3d76f34cdd60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Wed, 22 May 2024 17:14:38 +0200 Subject: [PATCH] FIX: chat activity indicator wasn't working for threads When a user had the chat option "Show activity indicator in header" set to "all new messages", and they would get a reply to a thread they're part of, the chat icon in the header would not show the unread bubble indicator. In order to fix this, the `ChatHeaderIconUnreadIndicator` component will now `showUnreadIndicator` whenever there is either one unread public channel or there are unread threads. I only added a system spec for this very specific path because I don't want to slow down the whole suite to test for all the various combination of the `chat_header_indicator_preference` values. Internal ref - t/128874 --- .../chat/user_chat_channel_membership.rb | 4 ++ .../chat/header/icon/unread-indicator.gjs | 8 +++- ...message_notifications_with_sidebar_spec.rb | 45 ++++++++++++++++++- .../system/message_thread_indicator_spec.rb | 2 - 4 files changed, 54 insertions(+), 5 deletions(-) diff --git a/plugins/chat/app/models/chat/user_chat_channel_membership.rb b/plugins/chat/app/models/chat/user_chat_channel_membership.rb index c2e59caa7b2..d2c63451ac9 100644 --- a/plugins/chat/app/models/chat/user_chat_channel_membership.rb +++ b/plugins/chat/app/models/chat/user_chat_channel_membership.rb @@ -13,6 +13,10 @@ module Chat enum :desktop_notification_level, NOTIFICATION_LEVELS, prefix: :desktop_notifications enum :mobile_notification_level, NOTIFICATION_LEVELS, prefix: :mobile_notifications enum :join_mode, { manual: 0, automatic: 1 } + + def mark_read!(new_last_read_id = nil) + update!(last_read_message_id: new_last_read_id || chat_channel.last_message_id) + end end end diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/header/icon/unread-indicator.gjs b/plugins/chat/assets/javascripts/discourse/components/chat/header/icon/unread-indicator.gjs index 4ca6f86bd6c..2a6a55cb0b2 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/header/icon/unread-indicator.gjs +++ b/plugins/chat/assets/javascripts/discourse/components/chat/header/icon/unread-indicator.gjs @@ -34,6 +34,12 @@ export default class ChatHeaderIconUnreadIndicator extends Component { ); } + get hasUnreads() { + return ( + this.unreadCount > 0 || this.chatTrackingStateManager.hasUnreadThreads + ); + } + get indicatorPreference() { return ( this.args.indicatorPreference || @@ -57,7 +63,7 @@ export default class ChatHeaderIconUnreadIndicator extends Component { get showUnreadIndicator() { return ( - this.unreadCount > 0 && + this.hasUnreads && this.#hasAnyIndicatorPreference([HEADER_INDICATOR_PREFERENCE_ALL_NEW]) ); } diff --git a/plugins/chat/spec/system/message_notifications_with_sidebar_spec.rb b/plugins/chat/spec/system/message_notifications_with_sidebar_spec.rb index 731be1b3de3..00edb689a88 100644 --- a/plugins/chat/spec/system/message_notifications_with_sidebar_spec.rb +++ b/plugins/chat/spec/system/message_notifications_with_sidebar_spec.rb @@ -5,14 +5,21 @@ RSpec.describe "Message notifications - with sidebar", type: :system do let!(:chat_page) { PageObjects::Pages::Chat.new } let!(:channel_page) { PageObjects::Pages::ChatChannel.new } + let!(:thread_page) { PageObjects::Pages::ChatThread.new } before do SiteSetting.navigation_menu = "sidebar" chat_system_bootstrap end - def create_message(text: nil, channel:, creator: Fabricate(:user)) - Fabricate(:chat_message_with_service, chat_channel: channel, message: text, user: creator) + def create_message(text: nil, channel: nil, thread: nil, creator: Fabricate(:user)) + Fabricate( + :chat_message_with_service, + chat_channel: channel, + thread: thread, + message: text, + user: creator, + ) end context "as a user" do @@ -225,6 +232,40 @@ RSpec.describe "Message notifications - with sidebar", type: :system do end end end + + context "with a thread" do + fab!(:channel) { Fabricate(:category_channel, threading_enabled: true) } + fab!(:other_user) { Fabricate(:user) } + fab!(:thread) do + chat_thread_chain_bootstrap(channel: channel, users: [current_user, other_user]) + end + + before do + channel.membership_for(current_user).mark_read! + thread.membership_for(current_user).mark_read! + + visit("/") + end + + context "when chat_header_indicator_preference is 'all_new'" do + before do + current_user.user_option.update!( + chat_header_indicator_preference: + UserOption.chat_header_indicator_preferences[:all_new], + ) + end + + context "when a reply is created" do + it "shows the unread indicator in the header" do + expect(page).to have_no_css(".chat-header-icon .chat-channel-unread-indicator") + + create_message(thread: thread, creator: other_user) + + expect(page).to have_css(".chat-header-icon .chat-channel-unread-indicator") + end + end + end + end end end end diff --git a/plugins/chat/spec/system/message_thread_indicator_spec.rb b/plugins/chat/spec/system/message_thread_indicator_spec.rb index 2d8a4ea06a9..d2a3c9d8d8a 100644 --- a/plugins/chat/spec/system/message_thread_indicator_spec.rb +++ b/plugins/chat/spec/system/message_thread_indicator_spec.rb @@ -6,10 +6,8 @@ describe "Thread indicator for chat messages", type: :system do let(:chat_page) { PageObjects::Pages::Chat.new } let(:channel_page) { PageObjects::Pages::ChatChannel.new } - let(:thread_page) { PageObjects::Pages::ChatThread.new } let(:side_panel) { PageObjects::Pages::ChatSidePanel.new } let(:open_thread) { PageObjects::Pages::ChatThread.new } - let(:chat_drawer_page) { PageObjects::Pages::ChatDrawer.new } before do chat_system_bootstrap(current_user, [channel])