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 @@