From b6c5a2da08c08b4df45c0885ec2757a117d83dea Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Thu, 25 May 2023 09:56:19 +0200 Subject: [PATCH] FEATURE: Initial chat thread unread indicators (#21694) This commit adds the thread index and individual thread in the index list unread indicators, and wires up the message bus events to mark the threads as read/unread when: 1. People send a new message in the thread 2. The user marks a thread as read There are several hacky parts and TODOs to cover before this is more functional: 1. We need to flesh out the thread scrolling and message visibility behaviour. Currently if you scroll to the end of the thread it will just mark the whole thread read unconditionally. 2. We need to send down the thread current user membership along with the last read message ID to the client and update that with read state. 3. We need to handle the sidebar unread dot for when threads are unread in the channel and clear it based on when the channel was last viewed. 4. We need to show some indicator of thread unreads on the thread indicators on original messages. 5. UI improvements to make the experience nicer and more like the actual design rather than just placeholders. But, the basic premise around incrementing/decrementing the thread overview count and showing which thread is unread in the list is working as intended. --- .../chat/api/channel_threads_controller.rb | 1 + .../controllers/chat/api/reads_controller.rb | 2 +- .../app/controllers/chat/chat_controller.rb | 15 ++-- plugins/chat/app/models/chat/threads_view.rb | 5 +- plugins/chat/app/models/chat/view.rb | 6 +- .../app/queries/chat/thread_unreads_query.rb | 1 + .../chat/thread_list_serializer.rb | 6 +- .../app/serializers/chat/view_serializer.rb | 8 +- .../app/services/chat/channel_view_builder.rb | 12 +-- .../services/chat/lookup_channel_threads.rb | 10 +++ .../chat/mark_all_user_channels_read.rb | 2 +- plugins/chat/app/services/chat/publisher.rb | 68 +++++++++++++---- .../services/chat/update_user_last_read.rb | 22 +++--- .../chat/update_user_thread_last_read.rb | 6 +- .../discourse/components/chat-channel.js | 22 +++--- .../chat-drawer/header/right-actions.hbs | 2 +- .../chat-drawer/header/thread-list-button.hbs | 9 --- .../chat-drawer/header/thread-list-button.js | 11 --- .../components/chat-drawer/thread.js | 2 +- .../components/chat-full-page-header.hbs | 9 +-- .../discourse/components/chat-thread.hbs | 2 +- .../discourse/components/chat-thread.js | 63 +++++++++++++++- .../chat/thread/header-unread-indicator.hbs | 9 +++ .../chat/thread/header-unread-indicator.js | 18 +++++ .../components/chat/thread/list-item.hbs | 8 +- .../chat/thread/threads-list-button.hbs | 15 ++++ .../chat/thread/threads-list-button.js | 10 +++ .../discourse/lib/chat-threads-manager.js | 7 ++ .../discourse/models/chat-channel.js | 17 ++++- .../discourse/models/chat-thread.js | 4 + .../discourse/services/chat-drawer-router.js | 29 +++++-- .../services/chat-subscriptions-manager.js | 75 +++++++++++++++++-- .../services/chat-tracking-state-manager.js | 12 +++ .../javascripts/discourse/services/chat.js | 11 +++ .../stylesheets/common/chat-channel.scss | 51 +++++-------- .../stylesheets/common/chat-drawer.scss | 4 + .../common/chat-thread-header-button.scss | 53 +++++++++++++ .../stylesheets/common/chat-threads-list.scss | 4 + .../chat/assets/stylesheets/common/index.scss | 1 + .../chat/channel_view_builder_spec.rb | 2 +- .../chat/spec/services/chat/publisher_spec.rb | 14 +++- .../chat/update_user_last_read_spec.rb | 2 +- .../spec/system/page_objects/chat/chat.rb | 4 - .../system/page_objects/chat/chat_channel.rb | 20 +++++ .../page_objects/chat/chat_thread_list.rb | 8 ++ .../page_objects/chat_drawer/chat_drawer.rb | 20 +++++ .../spec/system/thread_list/drawer_spec.rb | 10 ++- .../spec/system/thread_list/full_page_spec.rb | 14 ++-- .../system/thread_tracking/drawer_spec.rb | 68 +++++++++++++++++ .../system/thread_tracking/full_page_spec.rb | 58 ++++++++++++++ 50 files changed, 665 insertions(+), 167 deletions(-) delete mode 100644 plugins/chat/assets/javascripts/discourse/components/chat-drawer/header/thread-list-button.hbs delete mode 100644 plugins/chat/assets/javascripts/discourse/components/chat-drawer/header/thread-list-button.js create mode 100644 plugins/chat/assets/javascripts/discourse/components/chat/thread/header-unread-indicator.hbs create mode 100644 plugins/chat/assets/javascripts/discourse/components/chat/thread/header-unread-indicator.js create mode 100644 plugins/chat/assets/javascripts/discourse/components/chat/thread/threads-list-button.hbs create mode 100644 plugins/chat/assets/javascripts/discourse/components/chat/thread/threads-list-button.js create mode 100644 plugins/chat/assets/stylesheets/common/chat-thread-header-button.scss create mode 100644 plugins/chat/spec/system/thread_tracking/drawer_spec.rb create mode 100644 plugins/chat/spec/system/thread_tracking/full_page_spec.rb diff --git a/plugins/chat/app/controllers/chat/api/channel_threads_controller.rb b/plugins/chat/app/controllers/chat/api/channel_threads_controller.rb index abec9aabd1b..94ec54ff72c 100644 --- a/plugins/chat/app/controllers/chat/api/channel_threads_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channel_threads_controller.rb @@ -9,6 +9,7 @@ class Chat::Api::ChannelThreadsController < Chat::ApiController user: guardian.user, threads: result.threads, channel: result.channel, + tracking: result.tracking, ), ::Chat::ThreadListSerializer, root: false, diff --git a/plugins/chat/app/controllers/chat/api/reads_controller.rb b/plugins/chat/app/controllers/chat/api/reads_controller.rb index ec80f2b7f67..99a8dff412b 100644 --- a/plugins/chat/app/controllers/chat/api/reads_controller.rb +++ b/plugins/chat/app/controllers/chat/api/reads_controller.rb @@ -8,7 +8,7 @@ class Chat::Api::ReadsController < Chat::ApiController on_failed_policy(:ensure_message_id_recency) do raise Discourse::InvalidParameters.new(:message_id) end - on_failed_policy(:ensure_message_exists) { raise Discourse::NotFound } + on_model_not_found(:message) { raise Discourse::NotFound } on_model_not_found(:active_membership) { raise Discourse::NotFound } on_model_not_found(:channel) { raise Discourse::NotFound } end diff --git a/plugins/chat/app/controllers/chat/chat_controller.rb b/plugins/chat/app/controllers/chat/chat_controller.rb index 8ef0d2e37f6..4e1e82f081b 100644 --- a/plugins/chat/app/controllers/chat/chat_controller.rb +++ b/plugins/chat/app/controllers/chat/chat_controller.rb @@ -141,17 +141,16 @@ module Chat end end - Chat::Publisher.publish_user_tracking_state( - current_user, - @chat_channel.id, + message = ( - if chat_message_creator.chat_message.thread_id.present? - @user_chat_channel_membership.last_read_message_id + if chat_message_creator.chat_message.in_thread? + Chat::Message.find(@user_chat_channel_membership.last_read_message_id) else - chat_message_creator.chat_message.id + chat_message_creator.chat_message end - ), - ) + ) + + Chat::Publisher.publish_user_tracking_state!(current_user, @chat_channel, message) render json: success_json end diff --git a/plugins/chat/app/models/chat/threads_view.rb b/plugins/chat/app/models/chat/threads_view.rb index 6c51ccbc7a3..d349956b47f 100644 --- a/plugins/chat/app/models/chat/threads_view.rb +++ b/plugins/chat/app/models/chat/threads_view.rb @@ -2,12 +2,13 @@ module Chat class ThreadsView - attr_reader :user, :channel, :threads + attr_reader :user, :channel, :threads, :tracking - def initialize(channel:, threads:, user:) + def initialize(channel:, threads:, user:, tracking:) @channel = channel @threads = threads @user = user + @tracking = tracking end end end diff --git a/plugins/chat/app/models/chat/view.rb b/plugins/chat/app/models/chat/view.rb index 62bc87e9127..2e6f1b18e0a 100644 --- a/plugins/chat/app/models/chat/view.rb +++ b/plugins/chat/app/models/chat/view.rb @@ -7,7 +7,7 @@ module Chat :chat_messages, :can_load_more_past, :can_load_more_future, - :thread_tracking_overview, + :unread_thread_ids, :threads, :tracking @@ -17,7 +17,7 @@ module Chat user:, can_load_more_past: nil, can_load_more_future: nil, - thread_tracking_overview: nil, + unread_thread_ids: nil, threads: nil, tracking: nil ) @@ -26,7 +26,7 @@ module Chat @user = user @can_load_more_past = can_load_more_past @can_load_more_future = can_load_more_future - @thread_tracking_overview = thread_tracking_overview + @unread_thread_ids = unread_thread_ids @threads = threads @tracking = tracking end diff --git a/plugins/chat/app/queries/chat/thread_unreads_query.rb b/plugins/chat/app/queries/chat/thread_unreads_query.rb index 6e32d3e3936..da1649c217d 100644 --- a/plugins/chat/app/queries/chat/thread_unreads_query.rb +++ b/plugins/chat/app/queries/chat/thread_unreads_query.rb @@ -45,6 +45,7 @@ module Chat INNER JOIN chat_threads ON chat_threads.id = chat_messages.thread_id AND chat_threads.channel_id = chat_messages.chat_channel_id INNER JOIN user_chat_thread_memberships ON user_chat_thread_memberships.thread_id = chat_threads.id AND 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) AND chat_messages.deleted_at IS NULL diff --git a/plugins/chat/app/serializers/chat/thread_list_serializer.rb b/plugins/chat/app/serializers/chat/thread_list_serializer.rb index edb53b03fcd..bdeedf92e79 100644 --- a/plugins/chat/app/serializers/chat/thread_list_serializer.rb +++ b/plugins/chat/app/serializers/chat/thread_list_serializer.rb @@ -2,7 +2,7 @@ module Chat class ThreadListSerializer < ApplicationSerializer - attributes :meta, :threads + attributes :meta, :threads, :tracking def threads ActiveModel::ArraySerializer.new( @@ -12,6 +12,10 @@ module Chat ) end + def tracking + object.tracking + end + def meta { channel_id: object.channel.id } end diff --git a/plugins/chat/app/serializers/chat/view_serializer.rb b/plugins/chat/app/serializers/chat/view_serializer.rb index c62913113bf..ca510465624 100644 --- a/plugins/chat/app/serializers/chat/view_serializer.rb +++ b/plugins/chat/app/serializers/chat/view_serializer.rb @@ -2,7 +2,7 @@ module Chat class ViewSerializer < ApplicationSerializer - attributes :meta, :chat_messages, :threads, :tracking, :thread_tracking_overview, :channel + attributes :meta, :chat_messages, :threads, :tracking, :unread_thread_ids, :channel def threads return [] if !object.threads @@ -18,15 +18,15 @@ module Chat object.tracking || {} end - def thread_tracking_overview - object.thread_tracking_overview || [] + def unread_thread_ids + object.unread_thread_ids || [] end def include_threads? include_thread_data? end - def include_thread_tracking_overview? + def include_unread_thread_ids? include_thread_data? end diff --git a/plugins/chat/app/services/chat/channel_view_builder.rb b/plugins/chat/app/services/chat/channel_view_builder.rb index 3e86f0e1f8c..e8243ac8d49 100644 --- a/plugins/chat/app/services/chat/channel_view_builder.rb +++ b/plugins/chat/app/services/chat/channel_view_builder.rb @@ -32,7 +32,7 @@ module Chat step :determine_threads_enabled step :determine_include_thread_messages step :fetch_messages - step :fetch_thread_tracking_overview + step :fetch_unread_thread_ids step :fetch_threads_for_messages step :fetch_tracking step :build_view @@ -120,11 +120,11 @@ module Chat # that have unread messages, only threads with unread messages # will be included in this array. This is a low-cost way to know # how many threads the user has unread across the entire channel. - def fetch_thread_tracking_overview(guardian:, channel:, threads_enabled:, **) + def fetch_unread_thread_ids(guardian:, channel:, threads_enabled:, **) if !threads_enabled - context.thread_tracking_overview = [] + context.unread_thread_ids = [] else - context.thread_tracking_overview = + context.unread_thread_ids = ::Chat::TrackingStateReportQuery .call( guardian: guardian, @@ -177,7 +177,7 @@ module Chat messages:, threads:, tracking:, - thread_tracking_overview:, + unread_thread_ids:, can_load_more_past:, can_load_more_future:, ** @@ -189,7 +189,7 @@ module Chat user: guardian.user, can_load_more_past: can_load_more_past, can_load_more_future: can_load_more_future, - thread_tracking_overview: thread_tracking_overview, + unread_thread_ids: unread_thread_ids, threads: threads, tracking: tracking, ) diff --git a/plugins/chat/app/services/chat/lookup_channel_threads.rb b/plugins/chat/app/services/chat/lookup_channel_threads.rb index 7bab283f5e1..e64a05c4eff 100644 --- a/plugins/chat/app/services/chat/lookup_channel_threads.rb +++ b/plugins/chat/app/services/chat/lookup_channel_threads.rb @@ -23,6 +23,7 @@ module Chat policy :threading_enabled_for_channel policy :can_view_channel model :threads + step :fetch_tracking # @!visibility private class Contract @@ -71,5 +72,14 @@ module Chat .order("last_posted_at DESC NULLS LAST") .limit(50) end + + def fetch_tracking(guardian:, threads:, **) + context.tracking = + ::Chat::TrackingStateReportQuery.call( + guardian: guardian, + thread_ids: threads.map(&:id), + include_threads: true, + ).thread_tracking + end end 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 9c6376fbc63..e59e1d651e9 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 @@ -63,7 +63,7 @@ module Chat membership_id: membership.membership_id, } end - Chat::Publisher.publish_bulk_user_tracking_state(guardian.user, data) + Chat::Publisher.publish_bulk_user_tracking_state!(guardian.user, data) end end end diff --git a/plugins/chat/app/services/chat/publisher.rb b/plugins/chat/app/services/chat/publisher.rb index 17512b6d312..3d93e088d6e 100644 --- a/plugins/chat/app/services/chat/publisher.rb +++ b/plugins/chat/app/services/chat/publisher.rb @@ -47,13 +47,26 @@ module Chat ), ) - # NOTE: This means that the read count is only updated in the client - # for new messages in the main channel stream, maybe in future we want to - # do this for thread messages as well? if !chat_message.thread_reply? || !allow_publish_to_thread?(chat_channel) MessageBus.publish( self.new_messages_message_bus_channel(chat_channel.id), { + type: "channel", + channel_id: chat_channel.id, + message_id: chat_message.id, + user_id: chat_message.user.id, + username: chat_message.user.username, + thread_id: chat_message.thread_id, + }, + permissions(chat_channel), + ) + end + + if chat_message.thread_reply? && allow_publish_to_thread?(chat_channel) + MessageBus.publish( + self.new_messages_message_bus_channel(chat_channel.id), + { + type: "thread", channel_id: chat_channel.id, message_id: chat_message.id, user_id: chat_message.user.id, @@ -253,23 +266,46 @@ module Chat "/chat/user-tracking-state/#{user_id}" end - def self.publish_user_tracking_state(user, chat_channel_id, chat_message_id) - tracking_data = - Chat::TrackingState.call( - guardian: Guardian.new(user), - channel_ids: [chat_channel_id], + def self.publish_user_tracking_state!(user, channel, message) + data = { + channel_id: channel.id, + last_read_message_id: message.id, + thread_id: message.thread_id, + } + + channel_tracking_data = + Chat::TrackingStateReportQuery.call( + guardian: user.guardian, + channel_ids: [channel.id], include_missing_memberships: true, - ) - if tracking_data.failure? - raise StandardError, - "Tracking service failed when trying to publish user tracking state:\n\n#{tracking_data.inspect_steps}" + ).find_channel(channel.id) + + data.merge!(channel_tracking_data) + + # Need the thread unread overview if channel has threading enabled + # and a message is sent in the thread. We also need to pass the actual + # thread tracking state. + if channel.threading_enabled && message.thread_reply? + data[:unread_thread_ids] = ::Chat::TrackingStateReportQuery + .call( + guardian: user.guardian, + channel_ids: [channel.id], + include_threads: true, + include_read: false, + ) + .find_channel_threads(channel.id) + .keys + + data[:thread_tracking] = ::Chat::TrackingStateReportQuery.call( + guardian: user.guardian, + thread_ids: [message.thread_id], + include_threads: true, + ).find_thread(message.thread_id) end MessageBus.publish( self.user_tracking_state_message_bus_channel(user.id), - { channel_id: chat_channel_id, last_read_message_id: chat_message_id }.merge( - tracking_data.report.find_channel(chat_channel_id), - ).as_json, + data.as_json, user_ids: [user.id], ) end @@ -278,7 +314,7 @@ module Chat "/chat/bulk-user-tracking-state/#{user_id}" end - def self.publish_bulk_user_tracking_state(user, channel_last_read_map) + def self.publish_bulk_user_tracking_state!(user, channel_last_read_map) tracking_data = Chat::TrackingState.call( guardian: Guardian.new(user), 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 3ba18cb0d4b..8327d167b33 100644 --- a/plugins/chat/app/services/chat/update_user_last_read.rb +++ b/plugins/chat/app/services/chat/update_user_last_read.rb @@ -19,7 +19,7 @@ module Chat model :channel model :active_membership policy :invalid_access - policy :ensure_message_exists + model :message policy :ensure_message_id_recency step :update_last_read_message_id step :mark_associated_mentions_as_read @@ -47,29 +47,29 @@ module Chat guardian.can_join_chat_channel?(active_membership.chat_channel) end - def ensure_message_exists(channel:, contract:, **) - ::Chat::Message.with_deleted.exists?(chat_channel_id: channel.id, id: contract.message_id) + def fetch_message(channel:, contract:, **) + ::Chat::Message.with_deleted.find_by(chat_channel_id: channel.id, id: contract.message_id) end - def ensure_message_id_recency(contract:, active_membership:, **) + def ensure_message_id_recency(message:, active_membership:, **) !active_membership.last_read_message_id || - contract.message_id >= active_membership.last_read_message_id + message.id >= active_membership.last_read_message_id end - def update_last_read_message_id(contract:, active_membership:, **) - active_membership.update!(last_read_message_id: contract.message_id) + def update_last_read_message_id(message:, active_membership:, **) + active_membership.update!(last_read_message_id: message.id) end - def mark_associated_mentions_as_read(active_membership:, contract:, **) + def mark_associated_mentions_as_read(active_membership:, message:, **) ::Chat::Action::MarkMentionsRead.call( active_membership.user, channel_ids: [active_membership.chat_channel.id], - message_id: contract.message_id, + message_id: message.id, ) end - def publish_new_last_read_to_clients(guardian:, channel:, contract:, **) - ::Chat::Publisher.publish_user_tracking_state(guardian.user, channel.id, contract.message_id) + def publish_new_last_read_to_clients(guardian:, channel:, message:, **) + ::Chat::Publisher.publish_user_tracking_state!(guardian.user, channel, message) end end end 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 index a44c0e73c76..4748a4ca01e 100644 --- a/plugins/chat/app/services/chat/update_user_thread_last_read.rb +++ b/plugins/chat/app/services/chat/update_user_thread_last_read.rb @@ -70,10 +70,10 @@ module Chat end def publish_new_last_read_to_clients(guardian:, thread:, **) - ::Chat::Publisher.publish_user_tracking_state( + ::Chat::Publisher.publish_user_tracking_state!( guardian.user, - thread.channel_id, - thread.replies.last.id, + thread.channel, + thread.replies.last, ) end end diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-channel.js b/plugins/chat/assets/javascripts/discourse/components/chat-channel.js index ce2ccc9aa68..2c2c2d5cbe3 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-channel.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-channel.js @@ -188,6 +188,16 @@ export default class ChatLivePane extends Component { this.args.channel.addMessages(messages); this.args.channel.details = meta; + if (result.threads) { + result.threads.forEach((thread) => { + this.args.channel.threadsManager.store(this.args.channel, thread); + }); + } + + if (result.unread_thread_ids) { + this.args.channel.unreadThreadIds = result.unread_thread_ids; + } + if (this.requestedTargetMessageId) { this.scrollToMessage(findArgs["targetMessageId"], { highlight: true, @@ -205,18 +215,6 @@ export default class ChatLivePane extends Component { } this.scrollToBottom(); - - if (result.threads) { - result.threads.forEach((thread) => { - this.args.channel.threadsManager.store(this.args.channel, thread); - }); - } - - if (result.thread_tracking_overview) { - this.args.channel.threadTrackingOverview.push( - ...result.thread_tracking_overview - ); - } }) .catch(this._handleErrors) .finally(() => { diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-drawer/header/right-actions.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-drawer/header/right-actions.hbs index 55708259585..ed1b1934e2e 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-drawer/header/right-actions.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-drawer/header/right-actions.hbs @@ -1,7 +1,7 @@
{{#if this.chat.activeChannel.threadingEnabled}} - + {{/if}} - {{d-icon "discourse-threads"}} - \ No newline at end of file diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-drawer/header/thread-list-button.js b/plugins/chat/assets/javascripts/discourse/components/chat-drawer/header/thread-list-button.js deleted file mode 100644 index b7a5d7d9ada..00000000000 --- a/plugins/chat/assets/javascripts/discourse/components/chat-drawer/header/thread-list-button.js +++ /dev/null @@ -1,11 +0,0 @@ -import Component from "@glimmer/component"; -import { inject as service } from "@ember/service"; -import { action } from "@ember/object"; -export default class ChatDrawerThreadListButton extends Component { - @service chat; - - @action - stopPropagation(event) { - event.stopPropagation(); - } -} diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-drawer/thread.js b/plugins/chat/assets/javascripts/discourse/components/chat-drawer/thread.js index c3b5e9f0e4e..1a230dee760 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-drawer/thread.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-drawer/thread.js @@ -14,7 +14,7 @@ export default class ChatDrawerThread extends Component { models: this.chat.activeChannel.routeModels, }; - if (this.chatDrawerRouter.previousRouteName === "chat.channel.threads") { + if (this.chatDrawerRouter.previousRoute?.name === "chat.channel.threads") { link.title = "chat.return_to_threads_list"; link.route = "chat.channel.threads"; } else { diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-full-page-header.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-full-page-header.hbs index 5c070e12547..7dcbb9369a2 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-full-page-header.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-full-page-header.hbs @@ -41,14 +41,7 @@ {{/if}} {{#if @channel.threadingEnabled}} - - {{d-icon "discourse-threads"}} - + {{/if}}
{{/if}} diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs index 0c9143b45f5..9564c628f0b 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs @@ -25,7 +25,7 @@
{ + if (this._selfDeleted) { + return; + } + + // TODO (martin) HACK: We don't have proper scroll visibility over + // what message we are looking at, don't have the lastReadMessageId + // for the thread, and this updateLastReadMessage function is only + // called when scrolling all the way to the bottom. + this.markThreadAsRead(); + }); + } + @action setScrollable(element) { this.scrollable = element; diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/thread/header-unread-indicator.hbs b/plugins/chat/assets/javascripts/discourse/components/chat/thread/header-unread-indicator.hbs new file mode 100644 index 00000000000..5282ed4a2f8 --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/components/chat/thread/header-unread-indicator.hbs @@ -0,0 +1,9 @@ +{{#if this.showUnreadIndicator}} +
+
+
{{this.unreadCountLabel}}
+
+
+{{/if}} \ No newline at end of file diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/thread/header-unread-indicator.js b/plugins/chat/assets/javascripts/discourse/components/chat/thread/header-unread-indicator.js new file mode 100644 index 00000000000..d8a4426762b --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/components/chat/thread/header-unread-indicator.js @@ -0,0 +1,18 @@ +import Component from "@glimmer/component"; +import { inject as service } from "@ember/service"; + +export default class ChatThreadHeaderUnreadIndicator extends Component { + @service chatTrackingStateManager; + + get showUnreadIndicator() { + return this.args.channel.unreadThreadCount > 0; + } + + get unreadCountLabel() { + if (this.args.channel.unreadThreadCount > 99) { + return "99+"; + } + + return this.args.channel.unreadThreadCount; + } +} diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/thread/list-item.hbs b/plugins/chat/assets/javascripts/discourse/components/chat/thread/list-item.hbs index 6d4135d75ab..de9ae221bd9 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/thread/list-item.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat/thread/list-item.hbs @@ -1,4 +1,10 @@ -
+
+ {{d-icon "discourse-threads"}} + + {{#unless this.currentUserInDnD}} + + {{/unless}} + \ No newline at end of file diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/thread/threads-list-button.js b/plugins/chat/assets/javascripts/discourse/components/chat/thread/threads-list-button.js new file mode 100644 index 00000000000..c468cf54838 --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/components/chat/thread/threads-list-button.js @@ -0,0 +1,10 @@ +import { inject as service } from "@ember/service"; +import Component from "@glimmer/component"; + +export default class ChatThreadsListButton extends Component { + @service currentUser; + + get currentUserInDnD() { + return this.currentUser.isInDoNotDisturb(); + } +} diff --git a/plugins/chat/assets/javascripts/discourse/lib/chat-threads-manager.js b/plugins/chat/assets/javascripts/discourse/lib/chat-threads-manager.js index fa228ad5182..3e198ab142f 100644 --- a/plugins/chat/assets/javascripts/discourse/lib/chat-threads-manager.js +++ b/plugins/chat/assets/javascripts/discourse/lib/chat-threads-manager.js @@ -15,6 +15,7 @@ import { TrackedObject } from "@ember-compat/tracked-built-ins"; export default class ChatThreadsManager { @service chatSubscriptionsManager; + @service chatTrackingStateManager; @service chatApi; @service chat; @service currentUser; @@ -43,6 +44,12 @@ export default class ChatThreadsManager { thread ); }); + + this.chatTrackingStateManager.setupChannelThreadState( + this.chat.activeChannel, + result.tracking + ); + return { threads, meta: result.meta }; }); } diff --git a/plugins/chat/assets/javascripts/discourse/models/chat-channel.js b/plugins/chat/assets/javascripts/discourse/models/chat-channel.js index a5728bf651c..ed8eadfbf50 100644 --- a/plugins/chat/assets/javascripts/discourse/models/chat-channel.js +++ b/plugins/chat/assets/javascripts/discourse/models/chat-channel.js @@ -1,5 +1,5 @@ import UserChatChannelMembership from "discourse/plugins/chat/discourse/models/user-chat-channel-membership"; -import { TrackedArray } from "@ember-compat/tracked-built-ins"; +import { TrackedSet } from "@ember-compat/tracked-built-ins"; import { ajax } from "discourse/lib/ajax"; import { escapeExpression } from "discourse/lib/utilities"; import { tracked } from "@glimmer/tracking"; @@ -90,11 +90,12 @@ export default class ChatChannel { @tracked archive; @tracked tracking; @tracked threadingEnabled = false; - @tracked threadTrackingOverview = new TrackedArray(); threadsManager = new ChatThreadsManager(getOwner(this)); messagesManager = new ChatMessagesManager(getOwner(this)); + @tracked _unreadThreadIds = new TrackedSet(); + constructor(args = {}) { this.id = args.id; this.chatableId = args.chatable_id; @@ -132,6 +133,18 @@ export default class ChatChannel { this.tracking = new ChatTrackingState(getOwner(this)); } + get unreadThreadCount() { + return this.unreadThreadIds.size; + } + + get unreadThreadIds() { + return this._unreadThreadIds; + } + + set unreadThreadIds(unreadThreadIds) { + this._unreadThreadIds = new TrackedSet(unreadThreadIds); + } + findIndexOfMessage(id) { return this.messagesManager.findIndexOfMessage(id); } diff --git a/plugins/chat/assets/javascripts/discourse/models/chat-thread.js b/plugins/chat/assets/javascripts/discourse/models/chat-thread.js index b01ee19f86b..9617cb8b6c9 100644 --- a/plugins/chat/assets/javascripts/discourse/models/chat-thread.js +++ b/plugins/chat/assets/javascripts/discourse/models/chat-thread.js @@ -4,6 +4,7 @@ import { escapeExpression } from "discourse/lib/utilities"; import { tracked } from "@glimmer/tracking"; import guid from "pretty-text/guid"; import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message"; +import ChatTrackingState from "discourse/plugins/chat/discourse/models/chat-tracking-state"; export const THREAD_STATUSES = { open: "open", @@ -26,6 +27,7 @@ export default class ChatThread { @tracked originalMessage; @tracked threadMessageBusLastId; @tracked replyCount; + @tracked tracking; messagesManager = new ChatMessagesManager(getOwner(this)); @@ -38,6 +40,8 @@ export default class ChatThread { this.staged = args.staged; this.replyCount = args.reply_count; this.originalMessage = ChatMessage.create(channel, args.original_message); + + this.tracking = new ChatTrackingState(getOwner(this)); } stageMessage(message) { diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-drawer-router.js b/plugins/chat/assets/javascripts/discourse/services/chat-drawer-router.js index 4c958d2a8d5..bddb9df34b7 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-drawer-router.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-drawer-router.js @@ -6,7 +6,7 @@ import ChatDrawerThread from "discourse/plugins/chat/discourse/components/chat-d import ChatDrawerThreads from "discourse/plugins/chat/discourse/components/chat-drawer/threads"; import ChatDrawerIndex from "discourse/plugins/chat/discourse/components/chat-drawer/index"; -const COMPONENTS_MAP = { +const ROUTES = { "chat.draft-channel": { name: ChatDrawerDraftChannel }, "chat.channel": { name: ChatDrawerChannel }, "chat.channel.thread": { @@ -50,23 +50,40 @@ const COMPONENTS_MAP = { export default class ChatDrawerRouter extends Service { @service router; @tracked component = null; + @tracked drawerRoute = null; @tracked params = null; @tracked history = []; - get previousRouteName() { + get previousRoute() { if (this.history.length > 1) { return this.history[this.history.length - 2]; } } + get currentRoute() { + if (this.history.length > 0) { + return this.history[this.history.length - 1]; + } + } + stateFor(route) { - this.history.push(route.name); + this.drawerRoute?.deactivate?.(this.currentRoute); + + this.history.push(route); if (this.history.length > 10) { this.history.shift(); } - const component = COMPONENTS_MAP[route.name]; - this.params = component?.extractParams?.(route) || route.params; - this.component = component?.name || ChatDrawerIndex; + this.drawerRoute = ROUTES[route.name]; + + if (!this.drawerRoute) { + // TODO (joffrey) maybe we should implement the equivalent of a 404 here? + return; + } + + this.params = this.drawerRoute?.extractParams?.(route) || route.params; + this.component = this.drawerRoute?.name || ChatDrawerIndex; + + this.drawerRoute.activate?.(route); } } diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js index 5f48c23f90f..2caa3b4c095 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js @@ -179,6 +179,17 @@ export default class ChatSubscriptionsManager extends Service { @bind _onNewMessages(busData) { + switch (busData.type) { + case "channel": + this._onNewChannelMessage(busData); + break; + case "thread": + this._onNewThreadMessage(busData); + break; + } + } + + _onNewChannelMessage(busData) { this.chatChannelsManager.find(busData.channel_id).then((channel) => { if (busData.user_id === this.currentUser.id) { // User sent message, update tracking state to no unread @@ -188,13 +199,18 @@ export default class ChatSubscriptionsManager extends Service { if (this.currentUser.ignored_users.includes(busData.username)) { channel.currentUserMembership.lastReadMessageId = busData.message_id; } else { - // Message from other user. Increment trackings state + // Message from other user. Increment unread for channel tracking state. if ( busData.message_id > (channel.currentUserMembership.lastReadMessageId || 0) ) { channel.tracking.unreadCount++; } + + // Thread should be considered unread if not already. + if (busData.thread_id) { + channel.unreadThreadIds.add(busData.thread_id); + } } } @@ -202,6 +218,35 @@ export default class ChatSubscriptionsManager extends Service { }); } + _onNewThreadMessage(busData) { + this.chatChannelsManager.find(busData.channel_id).then((channel) => { + channel.threadsManager + .find(busData.channel_id, busData.thread_id) + .then((thread) => { + if (busData.user_id === this.currentUser.id) { + // Thread should no longer be considered unread. + channel.unreadThreadIds.remove(busData.thread_id); + // TODO (martin) Update lastReadMessageId for thread membership on client. + } else { + if (this.currentUser.ignored_users.includes(busData.username)) { + // TODO (martin) Update lastReadMessageId for thread membership on client. + } else { + if ( + this.chat.activeChannel?.activeThread?.id === busData.thread_id + ) { + // TODO (martin) HACK: We don't yet have the lastReadMessageId on the client, + // so if the user is looking at the thread don't do anything to mark it unread. + } else { + // Message from other user. Thread should be considered unread if not already. + channel.unreadThreadIds.add(busData.thread_id); + thread.tracking.unreadCount += 1; + } + } + } + }); + }); + } + _startUserTrackingStateSubscription(lastId) { if (!this.currentUser) { return; @@ -248,13 +293,31 @@ export default class ChatSubscriptionsManager extends Service { } @bind - _updateChannelTrackingData(channelId, trackingData) { + _updateChannelTrackingData(channelId, busData) { this.chatChannelsManager.find(channelId).then((channel) => { - channel.currentUserMembership.lastReadMessageId = - trackingData.last_read_message_id; + if (busData.thread_id) { + // TODO (martin) Update thread membership last read message ID on client. + } else { + channel.currentUserMembership.lastReadMessageId = + busData.last_read_message_id; + } - channel.tracking.unreadCount = trackingData.unread_count; - channel.tracking.mentionCount = trackingData.mention_count; + channel.tracking.unreadCount = busData.unread_count; + channel.tracking.mentionCount = busData.mention_count; + + if (busData.hasOwnProperty("unread_thread_ids")) { + channel.unreadThreadIds = busData.unread_thread_ids; + } + + if (busData.thread_id && busData.hasOwnProperty("thread_tracking")) { + channel.threadsManager + .find(channelId, busData.thread_id) + .then((thread) => { + thread.tracking.unreadCount = busData.thread_tracking.unread_count; + thread.tracking.mentionCount = + busData.thread_tracking.mention_count; + }); + } }); } diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-tracking-state-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-tracking-state-manager.js index e8e8301b86b..1c5f4f4e0b7 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-tracking-state-manager.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-tracking-state-manager.js @@ -32,6 +32,18 @@ export default class ChatTrackingStateManager extends Service { }); } + setupChannelThreadState(channel, threadTracking) { + channel.threadsManager.threads.forEach((thread) => { + // TODO (martin) Since we didn't backfill data for thread membership, + // there are cases where we are getting threads the user "participated" + // in but don't have tracking state for them. We need a migration to + // backfill this data. + if (threadTracking[thread.id.toString()]) { + this.#setState(thread, threadTracking[thread.id.toString()]); + } + }); + } + get publicChannelUnreadCount() { return this.#publicChannels().reduce((unreadCount, channel) => { return unreadCount + channel.tracking.unreadCount; diff --git a/plugins/chat/assets/javascripts/discourse/services/chat.js b/plugins/chat/assets/javascripts/discourse/services/chat.js index f61df665d93..dcf145aa837 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat.js @@ -56,6 +56,10 @@ export default class Chat extends Service { this._activeMessage = null; } + if (this._activeChannel) { + this._activeChannel.activeThread = null; + } + this._activeChannel = channel; } @@ -123,6 +127,13 @@ export default class Chat extends Service { this.chatChannelsManager .find(channelObject.id, { fetchIfNotFound: false }) .then((channel) => { + // TODO (martin) We need to do something here for thread tracking + // state as well on presence change, otherwise we will be back in + // the same place as the channels were. + // + // At some point it would likely be better to just fetch an + // endpoint that gives you all channel tracking state and the + // thread tracking state for the current channel. if (channel) { channel.updateMembership(channelObject.current_user_membership); diff --git a/plugins/chat/assets/stylesheets/common/chat-channel.scss b/plugins/chat/assets/stylesheets/common/chat-channel.scss index f68d0d4e46c..c44031ab882 100644 --- a/plugins/chat/assets/stylesheets/common/chat-channel.scss +++ b/plugins/chat/assets/stylesheets/common/chat-channel.scss @@ -1,3 +1,19 @@ +@mixin chat-channel-header-button { + color: var(--primary-low-mid); + + &:visited { + color: var(--primary-low-mid); + } + + &:hover { + color: var(--primary); + } + + > * { + pointer-events: none; + } +} + .chat-channel { display: flex; flex-direction: column; @@ -9,39 +25,8 @@ min-width: 250px; @include chat-height; - .open-drawer-btn, - .open-thread-list-btn { - color: var(--primary-low-mid); - - &:visited { - color: var(--primary-low-mid); - } - - &:hover { - color: var(--primary); - } - - > * { - pointer-events: none; - } - } - - .open-thread-list-btn { - .d-icon { - margin-right: 0; - } - - &:hover { - .discourse-touch & { - background: none !important; - } - } - - &:active { - .discourse-touch & { - background: var(--secondary-very-high) !important; - } - } + .open-drawer-btn { + @include chat-channel-header-button; } .chat-messages-scroll { diff --git a/plugins/chat/assets/stylesheets/common/chat-drawer.scss b/plugins/chat/assets/stylesheets/common/chat-drawer.scss index b545f9b37dc..00f15c13a16 100644 --- a/plugins/chat/assets/stylesheets/common/chat-drawer.scss +++ b/plugins/chat/assets/stylesheets/common/chat-drawer.scss @@ -248,6 +248,10 @@ a.chat-drawer-header__title { } } } + + &__thread-list-btn.-has-unreads { + margin-right: 0.5rem; + } } .chat-drawer-content { diff --git a/plugins/chat/assets/stylesheets/common/chat-thread-header-button.scss b/plugins/chat/assets/stylesheets/common/chat-thread-header-button.scss new file mode 100644 index 00000000000..d101245c667 --- /dev/null +++ b/plugins/chat/assets/stylesheets/common/chat-thread-header-button.scss @@ -0,0 +1,53 @@ +.chat-threads-list-button { + position: relative; + display: flex; + align-items: center; + border: 1px solid var(--blend-primary-secondary-5); + border-radius: 5px; + + @include chat-channel-header-button; + + &.-has-unreads { + .d-icon { + color: var(--tertiary-med-or-tertiary); + } + } + + .d-icon { + margin-right: 0; + } + + .chat-thread-header-unread-indicator { + color: var(--tertiary); + padding-left: 0.25rem; + + &__number-wrap { + background-color: var(--tertiary-med-or-tertiary); + display: flex; + padding: 0.25rem 0.5rem; + border-radius: 20px; + width: 35px; + box-sizing: border-box; + flex-direction: column; + align-items: center; + } + + &__number { + color: var(--secondary); + font-size: var(--font-down-3); + font-weight: bold; + } + } + + &:hover { + .discourse-touch & { + background: none !important; + } + } + + &:active { + .discourse-touch & { + background: var(--secondary-very-high) !important; + } + } +} diff --git a/plugins/chat/assets/stylesheets/common/chat-threads-list.scss b/plugins/chat/assets/stylesheets/common/chat-threads-list.scss index 05d412fc8b2..af99745945b 100644 --- a/plugins/chat/assets/stylesheets/common/chat-threads-list.scss +++ b/plugins/chat/assets/stylesheets/common/chat-threads-list.scss @@ -16,6 +16,10 @@ .chat-thread-list-item { margin: 0.75rem 0.25rem 0.75rem 0.5rem; + &.-unread { + border-left: 2px solid var(--tertiary-medium); + } + & + .chat-thread-list-item { margin-top: 0; } diff --git a/plugins/chat/assets/stylesheets/common/index.scss b/plugins/chat/assets/stylesheets/common/index.scss index e71c7f1c21a..ecfb85129f2 100644 --- a/plugins/chat/assets/stylesheets/common/index.scss +++ b/plugins/chat/assets/stylesheets/common/index.scss @@ -52,3 +52,4 @@ @import "chat-threads-list"; @import "chat-thread-original-message"; @import "chat-composer-separator"; +@import "chat-thread-header-button"; diff --git a/plugins/chat/spec/services/chat/channel_view_builder_spec.rb b/plugins/chat/spec/services/chat/channel_view_builder_spec.rb index 15890292a43..b124888caf7 100644 --- a/plugins/chat/spec/services/chat/channel_view_builder_spec.rb +++ b/plugins/chat/spec/services/chat/channel_view_builder_spec.rb @@ -148,7 +148,7 @@ RSpec.describe Chat::ChannelViewBuilder do thread = Fabricate(:chat_thread, channel: channel) thread.add(current_user) message_1 = Fabricate(:chat_message, chat_channel: channel, thread: thread) - expect(subject.view.thread_tracking_overview).to eq([message_1.thread.id]) + expect(subject.view.unread_thread_ids).to eq([message_1.thread.id]) end it "fetches the tracking state of threads in the channel" do diff --git a/plugins/chat/spec/services/chat/publisher_spec.rb b/plugins/chat/spec/services/chat/publisher_spec.rb index 68a2c69633f..97455a6fd3a 100644 --- a/plugins/chat/spec/services/chat/publisher_spec.rb +++ b/plugins/chat/spec/services/chat/publisher_spec.rb @@ -219,6 +219,7 @@ describe Chat::Publisher do end expect(messages.first.data).to eq( { + type: "channel", channel_id: channel.id, message_id: message_1.id, user_id: message_1.user_id, @@ -270,12 +271,21 @@ describe Chat::Publisher do context "if threading_enabled is true for the channel" do before { channel.update!(threading_enabled: true) } - it "does not publish to the new_messages_message_bus_channel" do + it "does publish to the new_messages_message_bus_channel" do messages = MessageBus.track_publish( described_class.new_messages_message_bus_channel(channel.id), ) { described_class.publish_new!(channel, message_1, staged_id) } - expect(messages).to be_empty + expect(messages.first.data).to eq( + { + type: "thread", + channel_id: channel.id, + message_id: message_1.id, + user_id: message_1.user_id, + username: message_1.user.username, + thread_id: thread.id, + }, + ) end end end diff --git a/plugins/chat/spec/services/chat/update_user_last_read_spec.rb b/plugins/chat/spec/services/chat/update_user_last_read_spec.rb index 06b2c218af1..120834feeff 100644 --- a/plugins/chat/spec/services/chat/update_user_last_read_spec.rb +++ b/plugins/chat/spec/services/chat/update_user_last_read_spec.rb @@ -65,7 +65,7 @@ RSpec.describe Chat::UpdateUserLastRead do membership.update!(last_read_message_id: 1) end - it { is_expected.to fail_a_policy(:ensure_message_exists) } + it { is_expected.to fail_to_find_a_model(:message) } end context "when everything is fine" do diff --git a/plugins/chat/spec/system/page_objects/chat/chat.rb b/plugins/chat/spec/system/page_objects/chat/chat.rb index c7fa707187c..9a6d4dc1b5b 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat.rb @@ -52,10 +52,6 @@ module PageObjects find(".open-drawer-btn").click end - def open_thread_list - find(".open-thread-list-btn").click - end - def has_message?(message) container = find(".chat-message-container[data-id=\"#{message.id}\"") container.has_content?(message.message) diff --git a/plugins/chat/spec/system/page_objects/chat/chat_channel.rb b/plugins/chat/spec/system/page_objects/chat/chat_channel.rb index 569c32930d0..df94ad88058 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat_channel.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat_channel.rb @@ -214,6 +214,26 @@ module PageObjects find(message_thread_indicator_selector(message)) end + def open_thread_list + find(thread_list_button_selector).click + end + + def has_unread_thread_indicator?(count:) + has_css?("#{thread_list_button_selector}.-has-unreads") && + has_css?( + ".chat-thread-header-unread-indicator .chat-thread-header-unread-indicator__number-wrap", + text: count.to_s, + ) + end + + def has_no_unread_thread_indicator? + has_no_css?("#{thread_list_button_selector}.-has-unreads") + end + + def thread_list_button_selector + ".chat-threads-list-button" + end + private def message_thread_indicator_selector(message) diff --git a/plugins/chat/spec/system/page_objects/chat/chat_thread_list.rb b/plugins/chat/spec/system/page_objects/chat/chat_thread_list.rb index 30aaa341b04..71fec44e7f1 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat_thread_list.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat_thread_list.rb @@ -7,6 +7,14 @@ module PageObjects find(item_by_id_selector(id)) end + def has_unread_item?(id) + has_css?(item_by_id_selector(id) + ".-unread") + end + + def has_no_unread_item?(id) + has_no_css?(item_by_id_selector(id) + ".-unread") + end + def item_by_id_selector(id) ".chat-thread-list__items .chat-thread-list-item[data-thread-id=\"#{id}\"]" end 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 acf1184270f..92a921a9370 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 @@ -46,6 +46,26 @@ module PageObjects def has_open_thread_list? has_css?("#{VISIBLE_DRAWER} .chat-thread-list") end + + def open_thread_list + find(thread_list_button_selector).click + end + + def thread_list_button_selector + ".chat-threads-list-button" + end + + def has_unread_thread_indicator?(count:) + has_css?("#{thread_list_button_selector}.-has-unreads") && + has_css?( + ".chat-thread-header-unread-indicator .chat-thread-header-unread-indicator__number-wrap", + text: count.to_s, + ) + end + + def has_no_unread_thread_indicator? + has_no_css?("#{thread_list_button_selector}.-has-unreads") + end end end end diff --git a/plugins/chat/spec/system/thread_list/drawer_spec.rb b/plugins/chat/spec/system/thread_list/drawer_spec.rb index 217cd9c725f..4f2af8d96fc 100644 --- a/plugins/chat/spec/system/thread_list/drawer_spec.rb +++ b/plugins/chat/spec/system/thread_list/drawer_spec.rb @@ -25,7 +25,9 @@ describe "Thread list in side panel | drawer", type: :system, js: true do visit("/") chat_page.open_from_header drawer_page.open_channel(channel) - expect(find(".chat-drawer-header__right-actions")).not_to have_css(".open-thread-list-btn") + expect(find(".chat-drawer-header__right-actions")).not_to have_css( + drawer_page.thread_list_button_selector, + ) end end @@ -55,7 +57,7 @@ describe "Thread list in side panel | drawer", type: :system, js: true do visit("/") chat_page.open_from_header drawer_page.open_channel(channel) - find(".open-thread-list-btn").click + drawer_page.open_thread_list expect(drawer_page).to have_open_thread_list end @@ -63,7 +65,7 @@ describe "Thread list in side panel | drawer", type: :system, js: true do visit("/") chat_page.open_from_header drawer_page.open_channel(channel) - find(".open-thread-list-btn").click + drawer_page.open_thread_list expect(drawer_page).to have_open_thread_list expect(thread_list_page).to have_content(thread_1.title) expect(thread_list_page).to have_content(thread_2.title) @@ -73,7 +75,7 @@ describe "Thread list in side panel | drawer", type: :system, js: true do visit("/") chat_page.open_from_header drawer_page.open_channel(channel) - find(".open-thread-list-btn").click + drawer_page.open_thread_list expect(drawer_page).to have_open_thread_list thread_list_page.item_by_id(thread_1.id).click expect(drawer_page).to have_open_thread(thread_1) diff --git a/plugins/chat/spec/system/thread_list/full_page_spec.rb b/plugins/chat/spec/system/thread_list/full_page_spec.rb index 3943f33469f..5acfdd9a7fa 100644 --- a/plugins/chat/spec/system/thread_list/full_page_spec.rb +++ b/plugins/chat/spec/system/thread_list/full_page_spec.rb @@ -20,7 +20,7 @@ describe "Thread list in side panel | full page", type: :system, js: true do context "when there are no threads that the user is participating in" do it "shows a message" do chat_page.visit_channel(channel) - chat_page.open_thread_list + channel_page.open_thread_list expect(page).to have_content(I18n.t("js.chat.threads.none")) end end @@ -37,14 +37,14 @@ describe "Thread list in side panel | full page", type: :system, js: true do it "shows a default title for threads without a title" do chat_page.visit_channel(channel) - chat_page.open_thread_list + channel_page.open_thread_list expect(page).to have_content(I18n.t("js.chat.thread.default_title", thread_id: thread_1.id)) end it "shows the thread title with emoji" do thread_1.update!(title: "What is for dinner? :hamburger:") chat_page.visit_channel(channel) - chat_page.open_thread_list + channel_page.open_thread_list expect(thread_list_page.item_by_id(thread_1.id)).to have_content("What is for dinner?") expect(thread_list_page.item_by_id(thread_1.id)).to have_css("img.emoji[alt='hamburger']") end @@ -53,7 +53,7 @@ describe "Thread list in side panel | full page", type: :system, js: true do thread_1.original_message.update!(message: "This is a long message for the excerpt") thread_1.original_message.rebake! chat_page.visit_channel(channel) - chat_page.open_thread_list + channel_page.open_thread_list expect(thread_list_page.item_by_id(thread_1.id)).to have_content( "This is a long message for the excerpt", ) @@ -61,7 +61,7 @@ describe "Thread list in side panel | full page", type: :system, js: true do it "shows the thread original message user username and avatar" do chat_page.visit_channel(channel) - chat_page.open_thread_list + channel_page.open_thread_list expect(thread_list_page.item_by_id(thread_1.id)).to have_css( ".chat-thread-original-message__avatar .chat-user-avatar .chat-user-avatar-container img", ) @@ -72,7 +72,7 @@ describe "Thread list in side panel | full page", type: :system, js: true do it "opens a thread" do chat_page.visit_channel(channel) - chat_page.open_thread_list + channel_page.open_thread_list thread_list_page.item_by_id(thread_1.id).click expect(side_panel).to have_open_thread(thread_1) end @@ -82,7 +82,7 @@ describe "Thread list in side panel | full page", type: :system, js: true do def open_thread_list chat_page.visit_channel(channel) - chat_page.open_thread_list + channel_page.open_thread_list expect(side_panel).to have_open_thread_list end diff --git a/plugins/chat/spec/system/thread_tracking/drawer_spec.rb b/plugins/chat/spec/system/thread_tracking/drawer_spec.rb new file mode 100644 index 00000000000..bf357f28b03 --- /dev/null +++ b/plugins/chat/spec/system/thread_tracking/drawer_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +describe "Thread tracking state | drawer", type: :system, js: true do + fab!(:current_user) { Fabricate(:admin) } + fab!(:channel) { Fabricate(:chat_channel, threading_enabled: true) } + fab!(:other_user) { Fabricate(:user) } + fab!(:thread) { Fabricate(:chat_thread, channel: channel) } + + let(:chat_page) { PageObjects::Pages::Chat.new } + let(:channel_page) { PageObjects::Pages::ChatChannel.new } + let(:thread_page) { PageObjects::Pages::ChatThread.new } + let(:thread_list_page) { PageObjects::Pages::ChatThreadList.new } + let(:drawer_page) { PageObjects::Pages::ChatDrawer.new } + + before do + SiteSetting.enable_experimental_chat_threaded_discussions = true + chat_system_bootstrap(current_user, [channel]) + sign_in(current_user) + thread.add(current_user) + end + + context "when the user has unread messages for a thread" do + fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel, thread: thread) } + fab!(:message_2) do + Fabricate(:chat_message, chat_channel: channel, thread: thread, user: current_user) + end + + it "shows the count of threads with unread messages on the thread list button" do + visit("/") + chat_page.open_from_header + drawer_page.open_channel(channel) + expect(drawer_page).to have_unread_thread_indicator(count: 1) + end + + it "shows an indicator on the unread thread in the list" do + visit("/") + chat_page.open_from_header + drawer_page.open_channel(channel) + drawer_page.open_thread_list + expect(drawer_page).to have_open_thread_list + expect(thread_list_page).to have_unread_item(thread.id) + end + + it "marks the thread as read and removes both indicators when the user opens it" do + visit("/") + chat_page.open_from_header + drawer_page.open_channel(channel) + drawer_page.open_thread_list + thread_list_page.item_by_id(thread.id).click + expect(drawer_page).to have_no_unread_thread_indicator + drawer_page.open_thread_list + expect(thread_list_page).to have_no_unread_item(thread.id) + end + + it "shows unread indicators for the header icon and the list when a new unread arrives" do + message_1.trash! + visit("/") + chat_page.open_from_header + drawer_page.open_channel(channel) + drawer_page.open_thread_list + expect(drawer_page).to have_no_unread_thread_indicator + expect(thread_list_page).to have_no_unread_item(thread.id) + Fabricate(:chat_message, chat_channel: channel, thread: thread) + expect(drawer_page).to have_unread_thread_indicator(count: 1) + expect(thread_list_page).to have_unread_item(thread.id) + end + end +end diff --git a/plugins/chat/spec/system/thread_tracking/full_page_spec.rb b/plugins/chat/spec/system/thread_tracking/full_page_spec.rb new file mode 100644 index 00000000000..b941f9bea38 --- /dev/null +++ b/plugins/chat/spec/system/thread_tracking/full_page_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +describe "Thread tracking state | full page", type: :system, js: true do + fab!(:current_user) { Fabricate(:user) } + fab!(:channel) { Fabricate(:chat_channel, threading_enabled: true) } + fab!(:other_user) { Fabricate(:user) } + fab!(:thread) { Fabricate(:chat_thread, channel: channel) } + + let(:chat_page) { PageObjects::Pages::Chat.new } + let(:channel_page) { PageObjects::Pages::ChatChannel.new } + let(:thread_page) { PageObjects::Pages::ChatThread.new } + let(:thread_list_page) { PageObjects::Pages::ChatThreadList.new } + + before do + SiteSetting.enable_experimental_chat_threaded_discussions = true + chat_system_bootstrap(current_user, [channel]) + sign_in(current_user) + thread.add(current_user) + end + + context "when the user has unread messages for a thread" do + fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel, thread: thread) } + fab!(:message_2) do + Fabricate(:chat_message, chat_channel: channel, thread: thread, user: current_user) + end + + it "shows the count of threads with unread messages on the thread list button" do + chat_page.visit_channel(channel) + expect(channel_page).to have_unread_thread_indicator(count: 1) + end + + it "shows an indicator on the unread thread in the list" do + chat_page.visit_channel(channel) + channel_page.open_thread_list + expect(thread_list_page).to have_unread_item(thread.id) + end + + it "marks the thread as read and removes both indicators when the user opens it" do + chat_page.visit_channel(channel) + channel_page.open_thread_list + thread_list_page.item_by_id(thread.id).click + expect(channel_page).to have_no_unread_thread_indicator + channel_page.open_thread_list + expect(thread_list_page).to have_no_unread_item(thread.id) + end + + it "shows unread indicators for the header icon and the list when a new unread arrives" do + message_1.trash! + chat_page.visit_channel(channel) + channel_page.open_thread_list + expect(channel_page).to have_no_unread_thread_indicator + expect(thread_list_page).to have_no_unread_item(thread.id) + Fabricate(:chat_message, chat_channel: channel, thread: thread) + expect(channel_page).to have_unread_thread_indicator(count: 1) + expect(thread_list_page).to have_unread_item(thread.id) + end + end +end