diff --git a/plugins/chat/app/models/chat/channel.rb b/plugins/chat/app/models/chat/channel.rb index ef0f0507beb..f002185a8a6 100644 --- a/plugins/chat/app/models/chat/channel.rb +++ b/plugins/chat/app/models/chat/channel.rb @@ -18,6 +18,7 @@ module Chat has_many :user_chat_channel_memberships, class_name: "Chat::UserChatChannelMembership", foreign_key: :chat_channel_id + has_many :threads, class_name: "Chat::Thread", foreign_key: :channel_id has_one :chat_channel_archive, class_name: "Chat::ChannelArchive", foreign_key: :chat_channel_id enum :status, { open: 0, read_only: 1, closed: 2, archived: 3 }, scopes: false diff --git a/plugins/chat/app/models/chat/thread.rb b/plugins/chat/app/models/chat/thread.rb index 64e41265b98..3c245be26ce 100644 --- a/plugins/chat/app/models/chat/thread.rb +++ b/plugins/chat/app/models/chat/thread.rb @@ -28,6 +28,14 @@ module Chat validates :title, length: { maximum: Chat::Thread::MAX_TITLE_LENGTH } + def add(user) + Chat::UserChatThreadMembership.find_or_create_by!(user: user, thread: self) + end + + def remove(user) + Chat::UserChatThreadMembership.find_by(user: user, thread: self)&.destroy + end + def replies self.chat_messages.where.not(id: self.original_message_id) end 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 c28b2524871..23a64c1e02e 100644 --- a/plugins/chat/app/models/chat/user_chat_channel_membership.rb +++ b/plugins/chat/app/models/chat/user_chat_channel_membership.rb @@ -13,9 +13,6 @@ 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 } - - attribute :unread_count, default: 0 - attribute :unread_mentions, default: 0 end end diff --git a/plugins/chat/app/queries/chat/channel_unreads_query.rb b/plugins/chat/app/queries/chat/channel_unreads_query.rb index 90184ad8736..7ba42370e38 100644 --- a/plugins/chat/app/queries/chat/channel_unreads_query.rb +++ b/plugins/chat/app/queries/chat/channel_unreads_query.rb @@ -7,12 +7,16 @@ module Chat # channels that the user is a member of will be counted and returned in # the result. class ChannelUnreadsQuery + # NOTE: This is arbitrary at this point in time, we may want to increase + # or decrease this as we find performance issues. + MAX_CHANNELS = 1000 + ## # @param channel_ids [Array] The IDs of the channels to count. # @param user_id [Integer] The ID of the user to count for. - # @param include_no_membership_channels [Boolean] Whether to include channels + # @param include_missing_memberships [Boolean] Whether to include channels # that the user is not a member of. These counts will always be 0. - def self.call(channel_ids:, user_id:, include_no_membership_channels: false) + def self.call(channel_ids:, user_id:, include_missing_memberships: false) sql = <<~SQL SELECT ( SELECT COUNT(*) AS unread_count @@ -21,30 +25,34 @@ module Chat INNER JOIN user_chat_channel_memberships ON user_chat_channel_memberships.chat_channel_id = chat_channels.id LEFT JOIN chat_threads ON chat_threads.id = chat_messages.thread_id WHERE chat_channels.id = memberships.chat_channel_id - AND chat_messages.user_id != :user_id AND user_chat_channel_memberships.user_id = :user_id AND chat_messages.id > COALESCE(user_chat_channel_memberships.last_read_message_id, 0) AND chat_messages.deleted_at IS NULL AND (chat_messages.thread_id IS NULL OR chat_messages.id = chat_threads.original_message_id) + AND NOT user_chat_channel_memberships.muted ) AS unread_count, ( SELECT COUNT(*) AS mention_count FROM notifications INNER JOIN user_chat_channel_memberships ON user_chat_channel_memberships.user_id = :user_id + INNER JOIN chat_messages ON (data::json->>'chat_message_id')::bigint = chat_messages.id + LEFT JOIN chat_threads ON chat_threads.id = chat_messages.thread_id WHERE NOT read AND user_chat_channel_memberships.chat_channel_id = memberships.chat_channel_id AND notifications.user_id = :user_id AND notifications.notification_type = :notification_type AND (data::json->>'chat_message_id')::bigint > COALESCE(user_chat_channel_memberships.last_read_message_id, 0) AND (data::json->>'chat_channel_id')::bigint = memberships.chat_channel_id + AND (chat_messages.thread_id IS NULL OR chat_messages.id = chat_threads.original_message_id) ) AS mention_count, memberships.chat_channel_id AS channel_id FROM user_chat_channel_memberships AS memberships WHERE memberships.user_id = :user_id AND memberships.chat_channel_id IN (:channel_ids) GROUP BY memberships.chat_channel_id + #{include_missing_memberships ? "" : "LIMIT :limit"} SQL - sql += <<~SQL if include_no_membership_channels + sql += <<~SQL if include_missing_memberships UNION ALL SELECT 0 AS unread_count, 0 AS mention_count, chat_channels.id AS channel_id FROM chat_channels @@ -52,6 +60,7 @@ module Chat AND user_chat_channel_memberships.user_id = :user_id WHERE chat_channels.id IN (:channel_ids) AND user_chat_channel_memberships.id IS NULL GROUP BY chat_channels.id + LIMIT :limit SQL DB.query( @@ -59,6 +68,7 @@ module Chat channel_ids: channel_ids, user_id: user_id, notification_type: Notification.types[:chat_mention], + limit: MAX_CHANNELS, ) end end diff --git a/plugins/chat/app/queries/chat/thread_unreads_query.rb b/plugins/chat/app/queries/chat/thread_unreads_query.rb new file mode 100644 index 00000000000..65f810eeed7 --- /dev/null +++ b/plugins/chat/app/queries/chat/thread_unreads_query.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +module Chat + ## + # Handles counting unread messages and mentions scoped to threads for a list + # of channels. A list of thread IDs can be provided to further focus the query. + # Alternatively, a list of thread IDs can be provided by itself to only get + # specific threads regardless of channel. + # + # This is used for unread indicators in the chat UI. By default only the + # threads that the user is a member of will be counted and returned in + # the result. Only threads inside a channel that has threading_enabled + # will be counted. + class ThreadUnreadsQuery + # NOTE: This is arbitrary at this point in time, we may want to increase + # or decrease this as we find performance issues. + MAX_THREADS = 3000 + + ## + # @param channel_ids [Array] (Optional) The IDs of the channels to count threads for. + # If only this is provided, all threads across the channels provided will be counted. + # @param thread_ids [Array] (Optional) The IDs of the threads to count. If this + # is used in tandem with channel_ids, it will just further filter the results of + # the thread counts from those channels. + # @param user_id [Integer] The ID of the user to count for. + # @param include_missing_memberships [Boolean] Whether to include threads + # that the user is not a member of. These counts will always be 0. + def self.call(channel_ids: [], thread_ids: [], user_id:, include_missing_memberships: false) + return [] if channel_ids.empty? && thread_ids.empty? + + sql = <<~SQL + SELECT ( + SELECT COUNT(*) AS unread_count + FROM chat_messages + INNER JOIN chat_channels ON chat_channels.id = chat_messages.chat_channel_id + 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 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 + AND chat_messages.thread_id IS NOT NULL + AND chat_messages.id != chat_threads.original_message_id + AND chat_channels.threading_enabled + AND user_chat_thread_memberships.notification_level != :muted_notification_level + ) AS unread_count, + 0 AS mention_count, + chat_threads.channel_id, + memberships.thread_id + FROM user_chat_thread_memberships AS memberships + INNER JOIN chat_threads ON chat_threads.id = memberships.thread_id + WHERE memberships.user_id = :user_id + #{channel_ids.present? ? "AND chat_threads.channel_id IN (:channel_ids)" : ""} + #{thread_ids.present? ? "AND chat_threads.id IN (:thread_ids)" : ""} + GROUP BY memberships.thread_id, chat_threads.channel_id + #{include_missing_memberships ? "" : "LIMIT :limit"} + SQL + + sql += <<~SQL if include_missing_memberships + UNION ALL + SELECT 0 AS unread_count, 0 AS mention_count, chat_threads.channel_id, chat_threads.id AS thread_id + FROM chat_channels + INNER JOIN chat_threads ON chat_threads.channel_id = chat_channels.id + LEFT JOIN user_chat_thread_memberships ON user_chat_thread_memberships.thread_id = chat_threads.id + AND user_chat_thread_memberships.user_id = :user_id + WHERE user_chat_thread_memberships.id IS NULL + #{channel_ids.present? ? "AND chat_threads.channel_id IN (:channel_ids)" : ""} + #{thread_ids.present? ? "AND chat_threads.id IN (:thread_ids)" : ""} + GROUP BY chat_threads.id + LIMIT :limit + SQL + + DB.query( + sql, + channel_ids: channel_ids, + thread_ids: thread_ids, + user_id: user_id, + notification_type: ::Notification.types[:chat_mention], + limit: MAX_THREADS, + muted_notification_level: ::Chat::UserChatThreadMembership.notification_levels[:muted], + ) + end + end +end diff --git a/plugins/chat/app/serializers/chat/base_channel_membership_serializer.rb b/plugins/chat/app/serializers/chat/base_channel_membership_serializer.rb index 1aef99fd33e..6ceb0ee522c 100644 --- a/plugins/chat/app/serializers/chat/base_channel_membership_serializer.rb +++ b/plugins/chat/app/serializers/chat/base_channel_membership_serializer.rb @@ -7,8 +7,6 @@ module Chat :desktop_notification_level, :mobile_notification_level, :chat_channel_id, - :last_read_message_id, - :unread_count, - :unread_mentions + :last_read_message_id end end diff --git a/plugins/chat/app/serializers/chat/structured_channel_serializer.rb b/plugins/chat/app/serializers/chat/structured_channel_serializer.rb index 1ac9c6dceeb..af86baeea99 100644 --- a/plugins/chat/app/serializers/chat/structured_channel_serializer.rb +++ b/plugins/chat/app/serializers/chat/structured_channel_serializer.rb @@ -2,7 +2,11 @@ module Chat class StructuredChannelSerializer < ApplicationSerializer - attributes :public_channels, :direct_message_channels, :meta + attributes :public_channels, :direct_message_channels, :tracking, :meta + + def tracking + object[:tracking] + end def public_channels object[:public_channels].map do |channel| diff --git a/plugins/chat/app/services/chat/publisher.rb b/plugins/chat/app/services/chat/publisher.rb index c34b7ee9f1c..a1348177316 100644 --- a/plugins/chat/app/services/chat/publisher.rb +++ b/plugins/chat/app/services/chat/publisher.rb @@ -242,20 +242,22 @@ module Chat end def self.publish_user_tracking_state(user, chat_channel_id, chat_message_id) - data = { - channel_id: chat_channel_id, - last_read_message_id: chat_message_id, - # TODO (martin) Remove old chat_channel_id and chat_message_id keys here once deploys have cycled, - # this will prevent JS errors from clients that are looking for the old payload. - chat_channel_id: chat_channel_id, - chat_message_id: chat_message_id, - }.merge( - Chat::ChannelUnreadsQuery.call(channel_ids: [chat_channel_id], user_id: user.id).first.to_h, - ) + tracking_data = + Chat::TrackingState.call( + guardian: Guardian.new(user), + channel_ids: [chat_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}" + end MessageBus.publish( self.user_tracking_state_message_bus_channel(user.id), - data.as_json, + { channel_id: chat_channel_id, last_read_message_id: chat_message_id }.merge( + tracking_data.report.find_channel(chat_channel_id), + ).as_json, user_ids: [user.id], ) end @@ -265,16 +267,19 @@ module Chat end def self.publish_bulk_user_tracking_state(user, channel_last_read_map) - unread_data = - Chat::ChannelUnreadsQuery.call( + tracking_data = + Chat::TrackingState.call( + guardian: Guardian.new(user), channel_ids: channel_last_read_map.keys, - user_id: user.id, - ).map(&:to_h) + include_missing_memberships: true, + ) + if tracking_data.failure? + raise StandardError, + "Tracking service failed when trying to publish bulk tracking state:\n\n#{tracking_data.inspect_steps}" + end channel_last_read_map.each do |key, value| - channel_last_read_map[key] = value.merge( - unread_data.find { |data| data[:channel_id] == key }.except(:channel_id), - ) + channel_last_read_map[key] = value.merge(tracking_data.report.find_channel(key)) end MessageBus.publish( diff --git a/plugins/chat/app/services/chat/tracking_state.rb b/plugins/chat/app/services/chat/tracking_state.rb new file mode 100644 index 00000000000..e77baf8cf13 --- /dev/null +++ b/plugins/chat/app/services/chat/tracking_state.rb @@ -0,0 +1,151 @@ +# frozen_string_literal: true + +module Chat + class TrackingStateReport + attr_accessor :channel_tracking, :thread_tracking + + class TrackingStateInfo + attr_accessor :unread_count, :mention_count + + def initialize(info) + @unread_count = info[:unread_count] + @mention_count = info[:mention_count] + end + + def to_hash + to_h + end + + def to_h + { unread_count: unread_count, mention_count: mention_count } + end + end + + def initialize + @channel_tracking = {} + @thread_tracking = {} + end + + def find_channel(channel_id) + TrackingStateInfo.new(channel_tracking[channel_id]) + end + + def find_thread(thread_id) + TrackingStateInfo.new(thread_tracking[thread_id]) + end + + def find_channel_threads(channel_id) + thread_tracking + .select { |_, thread| thread[:channel_id] == channel_id } + .map { |_, thread| TrackingStateInfo.new(thread) } + end + end + + # Produces the current tracking state for a user for one or more + # chat channels. This can be further filtered by providing one or + # more thread IDs for the channel. + # + # The goal of this class is to provide an easy way to get + # tracking state for: + # + # * A single channel + # * A single thread + # * Multiple channels and threads + # + # This is limited to 500 channels and 2000 threads by default, + # over time we can re-examine this if we find the need to. + # + # The user must be a member of these channels -- any channels + # they are not a member of will always return 0 for unread/mention + # counts at all times. + # + # Only channels with threads enabled will return thread tracking state. + # + # @example + # Chat::TrackingState.call(channel_ids: [2, 3], thread_ids: [6, 7], guardian: guardian) + # + class TrackingState + include Service::Base + + # @!method call(thread_ids:, channel_ids:, guardian:) + # @param [Integer] thread_ids + # @param [Integer] channel_ids + # @param [Guardian] guardian + # @return [Service::Base::Context] + + contract + policy :threaded_discussions_settings_ok + step :cast_thread_and_channel_ids_to_integer + model :report + + # @!visibility private + class Contract + attribute :channel_ids, default: [] + attribute :thread_ids, default: [] + attribute :include_missing_memberships, default: false + attribute :include_threads, default: false + end + + private + + def threaded_discussions_settings_ok(contract:, **) + return true if !contract.include_threads + SiteSetting.enable_experimental_chat_threaded_discussions + end + + def cast_thread_and_channel_ids_to_integer(contract:, **) + contract.thread_ids = contract.thread_ids.map(&:to_i) + contract.channel_ids = contract.channel_ids.map(&:to_i) + end + + def fetch_report(contract:, guardian:, **) + report = TrackingStateReport.new + + if contract.channel_ids.empty? + report.channel_tracking = {} + else + report.channel_tracking = + ::Chat::ChannelUnreadsQuery + .call( + channel_ids: contract.channel_ids, + user_id: guardian.user.id, + include_missing_memberships: contract.include_missing_memberships, + ) + .map do |ct| + [ct.channel_id, { mention_count: ct.mention_count, unread_count: ct.unread_count }] + end + .to_h + end + + if contract.include_threads + if contract.thread_ids.empty? && contract.channel_ids.empty? + report.thread_tracking = {} + else + report.thread_tracking = + ::Chat::ThreadUnreadsQuery + .call( + channel_ids: contract.channel_ids, + thread_ids: contract.thread_ids, + user_id: guardian.user.id, + include_missing_memberships: contract.include_missing_memberships, + ) + .map do |tt| + [ + tt.thread_id, + { + channel_id: tt.channel_id, + mention_count: tt.mention_count, + unread_count: tt.unread_count, + }, + ] + end + .to_h + end + else + report.thread_tracking = {} + end + + report + end + end +end diff --git a/plugins/chat/app/services/service/base.rb b/plugins/chat/app/services/service/base.rb index 8efe51837d8..983f2130cc3 100644 --- a/plugins/chat/app/services/service/base.rb +++ b/plugins/chat/app/services/service/base.rb @@ -58,6 +58,10 @@ module Service self end + def inspect_steps + Chat::StepsInspector.new(self) + end + private def self.build(context = {}) diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-channel-row.js b/plugins/chat/assets/javascripts/discourse/components/chat-channel-row.js index 9d697bfa30d..efd003a89ab 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-channel-row.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-channel-row.js @@ -19,7 +19,7 @@ export default class ChatChannelRow extends Component { } get channelHasUnread() { - return this.args.channel.currentUserMembership.unreadCount > 0; + return this.args.channel.tracking.unreadCount > 0; } get #firstDirectMessageUser() { diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-channel-unread-indicator.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-channel-unread-indicator.hbs index 208dc3d7674..d4c16d2432b 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-channel-unread-indicator.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-channel-unread-indicator.hbs @@ -1,16 +1,15 @@ -{{#if (gt @channel.currentUserMembership.unreadCount 0)}} +{{#if (gt @channel.tracking.unreadCount 0)}}
-
{{@channel.currentUserMembership.unreadCount}}
+
{{@channel.tracking.unreadCount}}
{{/if}} \ No newline at end of file diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-drawer/header/channel-title.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-drawer/header/channel-title.hbs index 3aad57058b3..876e4f87358 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-drawer/header/channel-title.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-drawer/header/channel-title.hbs @@ -21,9 +21,9 @@ >
- {{#if @channel.currentUserMembership.unreadCount}} + {{#if @channel.tracking.unreadCount}} - {{@channel.currentUserMembership.unreadCount}} + {{@channel.tracking.unreadCount}} {{/if}} diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-header-icon-unread-indicator.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-header-icon-unread-indicator.hbs index 93c6e38793a..237acb5b5b4 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-header-icon-unread-indicator.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-header-icon-unread-indicator.hbs @@ -1,7 +1,9 @@ {{#if this.showUrgentIndicator}}
-
{{this.chatChannelsManager.unreadUrgentCount}}
+
{{this.chatTrackingStateManager.allChannelUrgentCount}}
{{else if this.showUnreadIndicator}} diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-header-icon-unread-indicator.js b/plugins/chat/assets/javascripts/discourse/components/chat-header-icon-unread-indicator.js index 61410f92562..be7e160b058 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-header-icon-unread-indicator.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-header-icon-unread-indicator.js @@ -7,12 +7,12 @@ import { } from "../controllers/preferences-chat"; export default class ChatHeaderIconUnreadIndicator extends Component { - @service chatChannelsManager; + @service chatTrackingStateManager; @service currentUser; get showUrgentIndicator() { return ( - this.chatChannelsManager.unreadUrgentCount > 0 && + this.chatTrackingStateManager.allChannelUrgentCount > 0 && this.#hasAnyIndicatorPreference([ HEADER_INDICATOR_PREFERENCE_ALL_NEW, HEADER_INDICATOR_PREFERENCE_DM_AND_MENTIONS, @@ -22,7 +22,7 @@ export default class ChatHeaderIconUnreadIndicator extends Component { get showUnreadIndicator() { return ( - this.chatChannelsManager.unreadCount > 0 && + this.chatTrackingStateManager.publicChannelUnreadCount > 0 && this.#hasAnyIndicatorPreference([HEADER_INDICATOR_PREFERENCE_ALL_NEW]) ); } diff --git a/plugins/chat/assets/javascripts/discourse/initializers/chat-setup.js b/plugins/chat/assets/javascripts/discourse/initializers/chat-setup.js index ff336af6d7d..a23082bcbb0 100644 --- a/plugins/chat/assets/javascripts/discourse/initializers/chat-setup.js +++ b/plugins/chat/assets/javascripts/discourse/initializers/chat-setup.js @@ -122,6 +122,9 @@ export default { document.body.classList.add("chat-enabled"); const currentUser = api.getCurrentUser(); + + // NOTE: chat_channels is more than a simple array, it also contains + // tracking and membership data, see Chat::StructuredChannelSerializer if (currentUser?.chat_channels) { this.chatService.setupWithPreloadedChannels(currentUser.chat_channels); } diff --git a/plugins/chat/assets/javascripts/discourse/initializers/chat-sidebar.js b/plugins/chat/assets/javascripts/discourse/initializers/chat-sidebar.js index 440c89643cd..21bd2528ded 100644 --- a/plugins/chat/assets/javascripts/discourse/initializers/chat-sidebar.js +++ b/plugins/chat/assets/javascripts/discourse/initializers/chat-sidebar.js @@ -88,13 +88,11 @@ export default { } get suffixValue() { - return this.channel.currentUserMembership.unreadCount > 0 - ? "circle" - : ""; + return this.channel.tracking.unreadCount > 0 ? "circle" : ""; } get suffixCSSClass() { - return this.channel.currentUserMembership.unreadMentions > 0 + return this.channel.tracking.mentionCount > 0 ? "urgent" : "unread"; } @@ -282,9 +280,7 @@ export default { } get suffixValue() { - return this.channel.currentUserMembership.unreadCount > 0 - ? "circle" - : ""; + return this.channel.tracking.unreadCount > 0 ? "circle" : ""; } get suffixCSSClass() { diff --git a/plugins/chat/assets/javascripts/discourse/models/chat-channel.js b/plugins/chat/assets/javascripts/discourse/models/chat-channel.js index 1f7999b6054..4735b366dd8 100644 --- a/plugins/chat/assets/javascripts/discourse/models/chat-channel.js +++ b/plugins/chat/assets/javascripts/discourse/models/chat-channel.js @@ -11,6 +11,7 @@ import ChatThread from "discourse/plugins/chat/discourse/models/chat-thread"; import ChatDirectMessage from "discourse/plugins/chat/discourse/models/chat-direct-message"; import ChatChannelArchive from "discourse/plugins/chat/discourse/models/chat-channel-archive"; import Category from "discourse/models/category"; +import ChatTrackingState from "discourse/plugins/chat/discourse/models/chat-tracking-state"; export const CHATABLE_TYPES = { directMessageChannel: "DirectMessage", @@ -87,6 +88,7 @@ export default class ChatChannel { @tracked allowChannelWideMentions = true; @tracked membershipsCount = 0; @tracked archive; + @tracked tracking; threadsManager = new ChatThreadsManager(getOwner(this)); messagesManager = new ChatMessagesManager(getOwner(this)); @@ -121,6 +123,8 @@ export default class ChatChannel { if (args.archive_completed || args.archive_failed) { this.archive = ChatChannelArchive.create(args); } + + this.tracking = new ChatTrackingState(getOwner(this)); } findIndexOfMessage(id) { @@ -288,8 +292,6 @@ export default class ChatChannel { membership.desktop_notification_level; this.currentUserMembership.mobileNotificationLevel = membership.mobile_notification_level; - this.currentUserMembership.unreadCount = membership.unread_count; - this.currentUserMembership.unreadMentions = membership.unread_mentions; this.currentUserMembership.muted = membership.muted; } diff --git a/plugins/chat/assets/javascripts/discourse/models/chat-tracking-state.js b/plugins/chat/assets/javascripts/discourse/models/chat-tracking-state.js new file mode 100644 index 00000000000..ff16ac54c4a --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/models/chat-tracking-state.js @@ -0,0 +1,45 @@ +import { setOwner } from "@ember/application"; +import { inject as service } from "@ember/service"; +import { tracked } from "@glimmer/tracking"; + +export default class ChatTrackingState { + @service chatTrackingStateManager; + + @tracked _unreadCount = 0; + @tracked _mentionCount = 0; + + constructor(owner, params = {}) { + setOwner(this, owner); + this._unreadCount = params.unreadCount || 0; + this._mentionCount = params.mentionCount || 0; + } + + reset() { + this._unreadCount = 0; + this._mentionCount = 0; + } + + get unreadCount() { + return this._unreadCount; + } + + set unreadCount(value) { + const valueChanged = this._unreadCount !== value; + if (valueChanged) { + this._unreadCount = value; + this.chatTrackingStateManager.triggerNotificationsChanged(); + } + } + + get mentionCount() { + return this._mentionCount; + } + + set mentionCount(value) { + const valueChanged = this._mentionCount !== value; + if (valueChanged) { + this._mentionCount = value; + this.chatTrackingStateManager.triggerNotificationsChanged(); + } + } +} diff --git a/plugins/chat/assets/javascripts/discourse/models/user-chat-channel-membership.js b/plugins/chat/assets/javascripts/discourse/models/user-chat-channel-membership.js index ece9dd5d6cc..e50ec02d769 100644 --- a/plugins/chat/assets/javascripts/discourse/models/user-chat-channel-membership.js +++ b/plugins/chat/assets/javascripts/discourse/models/user-chat-channel-membership.js @@ -8,8 +8,6 @@ export default class UserChatChannelMembership { @tracked following = false; @tracked muted = false; - @tracked unreadCount = 0; - @tracked unreadMentions = 0; @tracked desktopNotificationLevel = null; @tracked mobileNotificationLevel = null; @tracked lastReadMessageId = null; @@ -18,8 +16,6 @@ export default class UserChatChannelMembership { constructor(args = {}) { this.following = args.following; this.muted = args.muted; - this.unreadCount = args.unread_count; - this.unreadMentions = args.unread_mentions; this.desktopNotificationLevel = args.desktop_notification_level; this.mobileNotificationLevel = args.mobile_notification_level; this.lastReadMessageId = args.last_read_message_id; 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 f81b3fbf7f7..e6aab87124f 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-channels-manager.js @@ -93,25 +93,6 @@ export default class ChatChannelsManager extends Service { delete this._cached[model.id]; } - get unreadCount() { - let count = 0; - this.publicMessageChannels.forEach((channel) => { - count += channel.currentUserMembership.unreadCount || 0; - }); - return count; - } - - get unreadUrgentCount() { - let count = 0; - this.channels.forEach((channel) => { - if (channel.isDirectMessageChannel) { - count += channel.currentUserMembership.unreadCount || 0; - } - count += channel.currentUserMembership.unreadMentions || 0; - }); - return count; - } - get publicMessageChannels() { return this.channels .filter( @@ -158,14 +139,12 @@ export default class ChatChannelsManager extends Service { #sortDirectMessageChannels(channels) { return channels.sort((a, b) => { - const unreadCountA = a.currentUserMembership.unreadCount || 0; - const unreadCountB = b.currentUserMembership.unreadCount || 0; - if (unreadCountA === unreadCountB) { + if (a.tracking.unreadCount === b.tracking.unreadCount) { return new Date(a.lastMessageSentAt) > new Date(b.lastMessageSentAt) ? -1 : 1; } else { - return unreadCountA > unreadCountB ? -1 : 1; + return a.tracking.unreadCount > b.tracking.unreadCount ? -1 : 1; } }); } 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 07a96d4854c..154f4d5fac9 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-subscriptions-manager.js @@ -7,6 +7,7 @@ import ChatChannelArchive from "../models/chat-channel-archive"; export default class ChatSubscriptionsManager extends Service { @service store; @service chatChannelsManager; + @service chatTrackingStateManager; @service currentUser; @service appEvents; @service chat; @@ -135,7 +136,7 @@ export default class ChatSubscriptionsManager extends Service { this.chatChannelsManager.find(busData.channel_id).then((channel) => { const membership = channel.currentUserMembership; if (busData.message_id > membership?.lastReadMessageId) { - membership.unreadMentions = (membership.unreadMentions || 0) + 1; + channel.tracking.mentionCount++; } }); } @@ -192,7 +193,7 @@ export default class ChatSubscriptionsManager extends Service { busData.message_id > (channel.currentUserMembership.lastReadMessageId || 0) ) { - channel.currentUserMembership.unreadCount++; + channel.tracking.unreadCount++; } } } @@ -251,8 +252,9 @@ export default class ChatSubscriptionsManager extends Service { this.chatChannelsManager.find(channelId).then((channel) => { channel.currentUserMembership.lastReadMessageId = trackingData.last_read_message_id; - channel.currentUserMembership.unreadCount = trackingData.unread_count; - channel.currentUserMembership.unreadMentions = trackingData.mention_count; + + channel.tracking.unreadCount = trackingData.unread_count; + channel.tracking.mentionCount = trackingData.mention_count; }); } @@ -281,7 +283,7 @@ export default class ChatSubscriptionsManager extends Service { channel.isDirectMessageChannel && !channel.currentUserMembership.following ) { - channel.currentUserMembership.unreadCount = 1; + channel.tracking.unreadCount = 1; } this.chatChannelsManager.follow(channel); @@ -360,8 +362,7 @@ export default class ChatSubscriptionsManager extends Service { // been deleted. we don't want them seeing the blue dot anymore so // just completely reset the unreads if (busData.status === CHANNEL_STATUSES.archived) { - channel.currentUserMembership.unreadCount = 0; - channel.currentUserMembership.unreadMentions = 0; + channel.tracking.reset(); } }); } 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 new file mode 100644 index 00000000000..e8e8301b86b --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/services/chat-tracking-state-manager.js @@ -0,0 +1,97 @@ +import Service, { inject as service } from "@ember/service"; +import discourseDebounce from "discourse-common/lib/debounce"; +import { cancel } from "@ember/runloop"; +import ChatTrackingState from "discourse/plugins/chat/discourse/models/chat-tracking-state"; +import { getOwner } from "discourse-common/lib/get-owner"; + +/** + * This service is used to provide a global interface to tracking individual + * channels and threads. In many places in the app, we need to know the global + * unread count for channels, threads, etc. + * + * The individual tracking state of each channel and thread is stored in + * a ChatTrackingState class instance and changed via the getters/setters + * provided there. + * + * This service is also used to preload bulk tracking state for channels + * and threads, which is used when the user first loads the app, and in + * certain cases where we need to set the state for many items at once. + */ +export default class ChatTrackingStateManager extends Service { + @service chatChannelsManager; + @service appEvents; + + // NOTE: In future, we may want to preload some thread tracking state + // as well, but for now we do that on demand when the user opens a channel, + // to avoid having to load all the threads across all channels into memory at once. + setupWithPreloadedState({ channel_tracking = {} }) { + this.chatChannelsManager.channels.forEach((channel) => { + if (channel_tracking[channel.id.toString()]) { + this.#setState(channel, channel_tracking[channel.id.toString()]); + } + }); + } + + get publicChannelUnreadCount() { + return this.#publicChannels().reduce((unreadCount, channel) => { + return unreadCount + channel.tracking.unreadCount; + }, 0); + } + + get allChannelUrgentCount() { + let publicChannelMentionCount = this.#publicChannels().reduce( + (mentionCount, channel) => { + return mentionCount + channel.tracking.mentionCount; + }, + 0 + ); + + let dmChannelUnreadCount = this.#directMessageChannels().reduce( + (unreadCount, channel) => { + return unreadCount + channel.tracking.unreadCount; + }, + 0 + ); + + return publicChannelMentionCount + dmChannelUnreadCount; + } + + willDestroy() { + super.willDestroy(...arguments); + cancel(this._onTriggerNotificationDebounceHandler); + } + + /** + * Some reactivity in the app such as the document title + * updates are only done via appEvents -- rather than + * sprinkle this appEvent call everywhere we just define + * it here so it can be changed as required. + */ + triggerNotificationsChanged() { + this._onTriggerNotificationDebounceHandler = discourseDebounce( + this, + this.#triggerNotificationsChanged, + 100 + ); + } + + #triggerNotificationsChanged() { + this.appEvents.trigger("notifications:changed"); + } + + #setState(model, state) { + if (!model.tracking) { + model.tracking = new ChatTrackingState(getOwner(this)); + } + model.tracking.unreadCount = state.unread_count; + model.tracking.mentionCount = state.mention_count; + } + + #publicChannels() { + return this.chatChannelsManager.publicMessageChannels; + } + + #directMessageChannels() { + return this.chatChannelsManager.directMessageChannels; + } +} diff --git a/plugins/chat/assets/javascripts/discourse/services/chat.js b/plugins/chat/assets/javascripts/discourse/services/chat.js index a49416a4130..e5425832561 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat.js @@ -34,6 +34,7 @@ export default class Chat extends Service { @service chatChannelsManager; @service chatChannelPane; @service chatChannelThreadPane; + @service chatTrackingStateManager; cook = null; presenceChannel = null; @@ -177,6 +178,8 @@ export default class Chat extends Service { return this.chatChannelsManager.follow(channel); } ); + + this.chatTrackingStateManager.setupWithPreloadedState(channels.tracking); } willDestroy() { @@ -208,7 +211,7 @@ export default class Chat extends Service { getDocumentTitleCount() { return this.chatNotificationManager.shouldCountChatInDocTitle() - ? this.chatChannelsManager.unreadUrgentCount + ? this.chatTrackingStateManager.allChannelUrgentCount : 0; } @@ -283,7 +286,7 @@ export default class Chat extends Service { const membership = channel.currentUserMembership; if (channel.isDirectMessageChannel) { - if (!dmChannelWithUnread && membership.unreadCount > 0) { + if (!dmChannelWithUnread && channel.tracking.unreadCount > 0) { dmChannelWithUnread = channel.id; } else if (!dmChannel) { dmChannel = channel.id; @@ -292,7 +295,10 @@ export default class Chat extends Service { if (membership.unread_mentions > 0) { publicChannelWithMention = channel.id; return; // <- We have a public channel with a mention. Break and return this. - } else if (!publicChannelWithUnread && membership.unreadCount > 0) { + } else if ( + !publicChannelWithUnread && + channel.tracking.unreadCount > 0 + ) { publicChannelWithUnread = channel.id; } else if ( !defaultChannel && diff --git a/plugins/chat/lib/chat/channel_fetcher.rb b/plugins/chat/lib/chat/channel_fetcher.rb index e1c821b855e..5a36cf599b7 100644 --- a/plugins/chat/lib/chat/channel_fetcher.rb +++ b/plugins/chat/lib/chat/channel_fetcher.rb @@ -4,14 +4,22 @@ module Chat class ChannelFetcher MAX_PUBLIC_CHANNEL_RESULTS = 50 - def self.structured(guardian) + def self.structured(guardian, include_threads: false) memberships = Chat::ChannelMembershipManager.all_for_user(guardian.user) + public_channels = + secured_public_channels(guardian, memberships, status: :open, following: true) + direct_message_channels = + secured_direct_message_channels(guardian.user.id, memberships, guardian) { - public_channels: - secured_public_channels(guardian, memberships, status: :open, following: true), - direct_message_channels: - secured_direct_message_channels(guardian.user.id, memberships, guardian), + public_channels: public_channels, + direct_message_channels: direct_message_channels, memberships: memberships, + tracking: + tracking_state( + public_channels.map(&:id) + direct_message_channels.map(&:id), + guardian, + include_threads: include_threads, + ), } end @@ -151,7 +159,6 @@ module Chat options.merge(include_archives: true, filter_on_category_name: true), ) - decorate_memberships_with_tracking_data(guardian, channels, memberships) channels = channels.to_a preload_custom_fields_for(channels) channels @@ -184,32 +191,17 @@ module Chat User.allowed_user_custom_fields(guardian) + UserField.all.pluck(:id).map { |fid| "#{User::USER_FIELD_PREFIX}#{fid}" } User.preload_custom_fields(channels.map { |c| c.chatable.users }.flatten, preload_fields) - - decorate_memberships_with_tracking_data(guardian, channels, memberships) + channels end - def self.decorate_memberships_with_tracking_data(guardian, channels, memberships) - unread_counts_per_channel = unread_counts(channels, guardian.user.id) - - channels.each do |channel| - membership = memberships.find { |m| m.chat_channel_id == channel.id } - - if membership - channel_unread_counts = - unread_counts_per_channel.find { |uc| uc.channel_id == channel.id } - - membership.unread_mentions = channel_unread_counts.mention_count - membership.unread_count = channel_unread_counts.unread_count if !membership.muted - end - end - end - - def self.unread_counts(channels, user_id) - Chat::ChannelUnreadsQuery.call( - channel_ids: channels.map(&:id), - user_id: user_id, - include_no_membership_channels: true, - ) + def self.tracking_state(channel_ids, guardian, include_threads: false) + Chat::TrackingState.call( + channel_ids: channel_ids, + guardian: guardian, + include_missing_memberships: true, + include_threads: + SiteSetting.enable_experimental_chat_threaded_discussions && include_threads, + ).report end def self.find_with_access_check(channel_id_or_name, guardian) diff --git a/plugins/chat/spec/fabricators/chat_fabricator.rb b/plugins/chat/spec/fabricators/chat_fabricator.rb index a62be7b22d5..b860f3f4e11 100644 --- a/plugins/chat/spec/fabricators/chat_fabricator.rb +++ b/plugins/chat/spec/fabricators/chat_fabricator.rb @@ -155,4 +155,12 @@ Fabricator(:chat_thread, class_name: "Chat::Thread") do after_create { |thread| thread.original_message.update!(thread_id: thread.id) } end -Fabricator(:user_chat_thread_membership, class_name: "Chat::UserChatThreadMembership") { user } +Fabricator(:user_chat_thread_membership, class_name: "Chat::UserChatThreadMembership") do + user + after_create do |membership| + Chat::UserChatChannelMembership.find_or_create_by!( + user: membership.user, + chat_channel: membership.thread.channel, + ).update!(following: true) + end +end diff --git a/plugins/chat/spec/lib/chat/channel_fetcher_spec.rb b/plugins/chat/spec/lib/chat/channel_fetcher_spec.rb index 9a2c32e5476..39bc3971dc0 100644 --- a/plugins/chat/spec/lib/chat/channel_fetcher_spec.rb +++ b/plugins/chat/spec/lib/chat/channel_fetcher_spec.rb @@ -45,7 +45,7 @@ describe Chat::ChannelFetcher do end end - describe ".unread_counts" do + describe ".tracking_state" do context "when user is member of the channel" do before do Fabricate(:user_chat_channel_membership, chat_channel: category_channel, user: user1) @@ -58,19 +58,17 @@ describe Chat::ChannelFetcher do end it "returns the correct count" do - unread_counts = described_class.unread_counts([category_channel], user1) - expect( - unread_counts.find { |uc| uc.channel_id == category_channel.id }.unread_count, - ).to eq(2) + tracking_state = + described_class.tracking_state([category_channel.id], Guardian.new(user1)) + expect(tracking_state.find_channel(category_channel.id).unread_count).to eq(2) end end context "with no unread messages" do it "returns the correct count" do - unread_counts = described_class.unread_counts([category_channel], user1) - expect( - unread_counts.find { |uc| uc.channel_id == category_channel.id }.unread_count, - ).to eq(0) + tracking_state = + described_class.tracking_state([category_channel.id], Guardian.new(user1)) + expect(tracking_state.find_channel(category_channel.id).unread_count).to eq(0) end end @@ -82,10 +80,9 @@ describe Chat::ChannelFetcher do before { last_unread.update!(deleted_at: Time.zone.now) } it "returns the correct count" do - unread_counts = described_class.unread_counts([category_channel], user1) - expect( - unread_counts.find { |uc| uc.channel_id == category_channel.id }.unread_count, - ).to eq(0) + tracking_state = + described_class.tracking_state([category_channel.id], Guardian.new(user1)) + expect(tracking_state.find_channel(category_channel.id).unread_count).to eq(0) end end end @@ -97,10 +94,9 @@ describe Chat::ChannelFetcher do end it "returns the correct count" do - unread_counts = described_class.unread_counts([category_channel], user1) - expect( - unread_counts.find { |uc| uc.channel_id == category_channel.id }.unread_count, - ).to eq(0) + tracking_state = + described_class.tracking_state([category_channel.id], Guardian.new(user1)) + expect(tracking_state.find_channel(category_channel.id).unread_count).to eq(0) end end end @@ -336,32 +332,17 @@ describe Chat::ChannelFetcher do Fabricate(:chat_message, user: user2, chat_channel: category_channel) resolved_memberships = memberships - described_class.secured_public_channels( - guardian, - resolved_memberships, - following: following, - ) + result = + described_class.tracking_state(resolved_memberships.map(&:chat_channel_id), guardian) - expect( - resolved_memberships - .find { |membership| membership.chat_channel_id == category_channel.id } - .unread_count, - ).to eq(2) - - resolved_memberships.last.update!(muted: true) + expect(result.channel_tracking[category_channel.id][:unread_count]).to eq(2) resolved_memberships = memberships - described_class.secured_public_channels( - guardian, - resolved_memberships, - following: following, - ) + resolved_memberships.last.update!(muted: true) + result = + described_class.tracking_state(resolved_memberships.map(&:chat_channel_id), guardian) - expect( - resolved_memberships - .find { |membership| membership.chat_channel_id == category_channel.id } - .unread_count, - ).to eq(0) + expect(result.channel_tracking[category_channel.id][:unread_count]).to eq(0) end end end @@ -422,17 +403,17 @@ describe Chat::ChannelFetcher do Fabricate(:chat_message, user: user2, chat_channel: direct_message_channel1) resolved_memberships = memberships - described_class.secured_direct_message_channels(user1.id, resolved_memberships, guardian) target_membership = resolved_memberships.find { |mem| mem.chat_channel_id == direct_message_channel1.id } - expect(target_membership.unread_count).to eq(2) + result = described_class.tracking_state([direct_message_channel1.id], guardian) + expect(result.channel_tracking[target_membership.chat_channel_id][:unread_count]).to eq(2) resolved_memberships = memberships target_membership = resolved_memberships.find { |mem| mem.chat_channel_id == direct_message_channel1.id } target_membership.update!(muted: true) - described_class.secured_direct_message_channels(user1.id, resolved_memberships, guardian) - expect(target_membership.unread_count).to eq(0) + result = described_class.tracking_state([direct_message_channel1.id], guardian) + expect(result.channel_tracking[target_membership.chat_channel_id][:unread_count]).to eq(0) end end diff --git a/plugins/chat/spec/queries/chat/channel_unreads_query_spec.rb b/plugins/chat/spec/queries/chat/channel_unreads_query_spec.rb index a1e2f7eef6e..1389c912377 100644 --- a/plugins/chat/spec/queries/chat/channel_unreads_query_spec.rb +++ b/plugins/chat/spec/queries/chat/channel_unreads_query_spec.rb @@ -21,6 +21,21 @@ describe Chat::ChannelUnreadsQuery do ).to eq({ mention_count: 0, unread_count: 1, channel_id: channel_1.id }) end + context "when the membership has been muted" do + before do + channel_1 + .user_chat_channel_memberships + .find_by(user_id: current_user.id) + .update!(muted: true) + end + + it "returns a zeroed unread count" do + expect( + described_class.call(channel_ids: [channel_1.id], user_id: current_user.id).first.to_h, + ).to eq({ mention_count: 0, unread_count: 0, channel_id: channel_1.id }) + end + end + context "for unread messages in a thread" do fab!(:thread_om) { Fabricate(:chat_message, chat_channel: channel_1) } fab!(:thread) { Fabricate(:chat_thread, channel: channel_1, original_message: thread_om) } @@ -80,13 +95,13 @@ describe Chat::ChannelUnreadsQuery do ).to match_array([{ mention_count: 0, unread_count: 1, channel_id: channel_1.id }]) end - context "when include_no_membership_channels is true" do + context "when include_missing_memberships is true" do it "does return zeroed counts for the channels" do expect( described_class.call( channel_ids: [channel_1.id, channel_2.id], user_id: current_user.id, - include_no_membership_channels: true, + include_missing_memberships: true, ).map(&:to_h), ).to match_array( [ @@ -122,6 +137,28 @@ describe Chat::ChannelUnreadsQuery do ).to eq({ mention_count: 1, unread_count: 1, channel_id: channel_1.id }) end + context "for unread mentions in a thread" do + fab!(:thread_om) { Fabricate(:chat_message, chat_channel: channel_1) } + fab!(:thread) { Fabricate(:chat_thread, channel: channel_1, original_message: thread_om) } + + it "does include the original message in the mention count" do + create_mention(thread_om, channel_1) + expect( + described_class.call(channel_ids: [channel_1.id], user_id: current_user.id).first.to_h, + ).to eq({ mention_count: 1, unread_count: 1, channel_id: channel_1.id }) + end + + it "does not include other thread messages in the mention count" do + thread_message_1 = Fabricate(:chat_message, chat_channel: channel_1, thread: thread) + thread_message_2 = Fabricate(:chat_message, chat_channel: channel_1, thread: thread) + create_mention(thread_message_1, channel_1) + create_mention(thread_message_2, channel_1) + expect( + described_class.call(channel_ids: [channel_1.id], user_id: current_user.id).first.to_h, + ).to eq({ mention_count: 0, unread_count: 1, channel_id: channel_1.id }) + end + end + context "for multiple channels" do fab!(:channel_2) { Fabricate(:category_channel) } diff --git a/plugins/chat/spec/queries/chat/thread_unreads_query_spec.rb b/plugins/chat/spec/queries/chat/thread_unreads_query_spec.rb new file mode 100644 index 00000000000..9e2610a279f --- /dev/null +++ b/plugins/chat/spec/queries/chat/thread_unreads_query_spec.rb @@ -0,0 +1,176 @@ +# frozen_string_literal: true + +require "rails_helper" + +describe Chat::ThreadUnreadsQuery do + fab!(:channel_1) { Fabricate(:category_channel, threading_enabled: true) } + fab!(:channel_2) { Fabricate(:category_channel, threading_enabled: true) } + fab!(:thread_1) { Fabricate(:chat_thread, channel: channel_1) } + fab!(:thread_2) { Fabricate(:chat_thread, channel: channel_1) } + fab!(:thread_3) { Fabricate(:chat_thread, channel: channel_2) } + fab!(:thread_4) { Fabricate(:chat_thread, channel: channel_2) } + fab!(:current_user) { Fabricate(:user) } + + let(:params) { { user_id: current_user.id, channel_ids: channel_ids, thread_ids: thread_ids } } + let(:include_missing_memberships) { false } + let(:channel_ids) { [] } + let(:thread_ids) { [] } + let(:subject) do + described_class.call( + channel_ids: channel_ids, + thread_ids: thread_ids, + user_id: current_user.id, + include_missing_memberships: include_missing_memberships, + ) + end + + before do + SiteSetting.chat_enabled = true + SiteSetting.enable_experimental_chat_threaded_discussions = true + SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone] + channel_1.add(current_user) + channel_2.add(current_user) + thread_1.add(current_user) + thread_2.add(current_user) + thread_3.add(current_user) + thread_4.add(current_user) + end + + context "with unread messages across multiple threads" do + fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1, thread: thread_1) } + fab!(:message_2) { Fabricate(:chat_message, chat_channel: channel_2, thread: thread_3) } + fab!(:message_3) { Fabricate(:chat_message, chat_channel: channel_2, thread: thread_4) } + + context "when only the channel ids are provided" do + let(:channel_ids) { [channel_1.id, channel_2.id] } + + it "gets a count of all the thread unreads across the channels" do + expect(subject.map(&:to_h)).to match_array( + [ + { channel_id: channel_1.id, mention_count: 0, thread_id: thread_1.id, unread_count: 1 }, + { channel_id: channel_1.id, mention_count: 0, thread_id: thread_2.id, unread_count: 0 }, + { channel_id: channel_2.id, mention_count: 0, thread_id: thread_3.id, unread_count: 1 }, + { channel_id: channel_2.id, mention_count: 0, thread_id: thread_4.id, unread_count: 1 }, + ], + ) + end + + it "does not count deleted messages" do + message_1.trash! + expect(subject.map(&:to_h).find { |tracking| tracking[:thread_id] == thread_1.id }).to eq( + { channel_id: channel_1.id, mention_count: 0, thread_id: thread_1.id, unread_count: 0 }, + ) + end + + it "does not messages in threads where threading_enabled is false on the channel" do + channel_1.update!(threading_enabled: false) + expect(subject.map(&:to_h).find { |tracking| tracking[:thread_id] == thread_1.id }).to eq( + { channel_id: channel_1.id, mention_count: 0, thread_id: thread_1.id, unread_count: 0 }, + ) + expect(subject.map(&:to_h).find { |tracking| tracking[:thread_id] == thread_2.id }).to eq( + { channel_id: channel_1.id, mention_count: 0, thread_id: thread_2.id, unread_count: 0 }, + ) + end + + it "does not count as unread if the last_read_message_id is greater than or equal to the message id" do + thread_1 + .user_chat_thread_memberships + .find_by(user: current_user) + .update!(last_read_message_id: message_1.id) + expect(subject.map(&:to_h).find { |tracking| tracking[:thread_id] == thread_1.id }).to eq( + { channel_id: channel_1.id, mention_count: 0, thread_id: thread_1.id, unread_count: 0 }, + ) + end + + it "does not count the original message ID as unread" do + thread_1.original_message.destroy + thread_1.update!(original_message: message_1) + expect(subject.map(&:to_h).find { |tracking| tracking[:thread_id] == thread_1.id }).to eq( + { channel_id: channel_1.id, mention_count: 0, thread_id: thread_1.id, unread_count: 0 }, + ) + end + end + + context "when only the thread_ids are provided" do + let(:thread_ids) { [thread_1.id, thread_3.id] } + + it "gets a count of all the thread unreads for the specified threads" do + expect(subject.map(&:to_h)).to match_array( + [ + { channel_id: channel_1.id, mention_count: 0, thread_id: thread_1.id, unread_count: 1 }, + { channel_id: channel_2.id, mention_count: 0, thread_id: thread_3.id, unread_count: 1 }, + ], + ) + end + + context "when the notification_level for the thread is muted" do + before do + thread_1 + .user_chat_thread_memberships + .find_by(user: current_user) + .update!(notification_level: :muted) + end + + it "gets a zeroed out count for the thread" do + expect(subject.map(&:to_h)).to include( + { channel_id: channel_1.id, mention_count: 0, thread_id: thread_1.id, unread_count: 0 }, + ) + end + end + + context "when the user is not a member of a thread" do + before { thread_1.user_chat_thread_memberships.find_by(user: current_user).destroy! } + + it "does not get that thread unread count by default" do + expect(subject.map(&:to_h)).to match_array( + [ + { + channel_id: channel_2.id, + mention_count: 0, + thread_id: thread_3.id, + unread_count: 1, + }, + ], + ) + end + + context "when include_missing_memberships is true" do + let(:include_missing_memberships) { true } + + it "includes the thread that the user is not a member of with zeroed out counts" do + expect(subject.map(&:to_h)).to match_array( + [ + { + channel_id: channel_1.id, + mention_count: 0, + thread_id: thread_1.id, + unread_count: 0, + }, + { + channel_id: channel_2.id, + mention_count: 0, + thread_id: thread_3.id, + unread_count: 1, + }, + ], + ) + end + end + end + end + + context "when channel_ids and thread_ids are provided" do + let(:channel_ids) { [channel_1.id, channel_2.id] } + let(:thread_ids) { [thread_1.id, thread_3.id] } + + it "gets a count of all the thread unreads across the channels filtered by thread id" do + expect(subject.map(&:to_h)).to match_array( + [ + { channel_id: channel_1.id, mention_count: 0, thread_id: thread_1.id, unread_count: 1 }, + { channel_id: channel_2.id, mention_count: 0, thread_id: thread_3.id, unread_count: 1 }, + ], + ) + end + end + end +end diff --git a/plugins/chat/spec/services/chat/tracking_state_spec.rb b/plugins/chat/spec/services/chat/tracking_state_spec.rb new file mode 100644 index 00000000000..6fedc57737a --- /dev/null +++ b/plugins/chat/spec/services/chat/tracking_state_spec.rb @@ -0,0 +1,184 @@ +# frozen_string_literal: true + +RSpec.describe ::Chat::TrackingState do + describe ".call" do + subject(:result) { described_class.call(params) } + + fab!(:current_user) { Fabricate(:user) } + fab!(:channel_1) { Fabricate(:chat_channel, threading_enabled: true) } + fab!(:channel_2) { Fabricate(:chat_channel, threading_enabled: true) } + fab!(:thread_1) { Fabricate(:chat_thread, channel: channel_1) } + fab!(:thread_2) { Fabricate(:chat_thread, channel: channel_1) } + fab!(:thread_3) { Fabricate(:chat_thread, channel: channel_2) } + fab!(:thread_4) { Fabricate(:chat_thread, channel: channel_2) } + + let(:guardian) { Guardian.new(current_user) } + let(:id_params) { { channel_ids: [channel_1.id], thread_ids: [thread_1.id] } } + let(:include_threads) { nil } + let(:include_missing_memberships) { nil } + + let(:params) do + id_params.merge(guardian: guardian).merge( + include_threads: include_threads, + include_missing_memberships: include_missing_memberships, + ) + end + + context "when enable_experimental_chat_threaded_discussions is disabled" do + before { SiteSetting.enable_experimental_chat_threaded_discussions = false } + + context "when include_threads is true" do + let(:include_threads) { true } + it { is_expected.to fail_a_policy(:threaded_discussions_settings_ok) } + end + + context "when include_threads is false" do + let(:include_threads) { false } + it { is_expected.not_to fail_a_policy(:threaded_discussions_settings_ok) } + end + end + + context "when enable_experimental_chat_threaded_discussions is enabled" do + before { SiteSetting.enable_experimental_chat_threaded_discussions = true } + + let(:include_threads) { true } + fab!(:channel_1_membership) do + Fabricate(:user_chat_channel_membership, chat_channel: channel_1, user: current_user) + end + fab!(:thread_1_membership) do + Fabricate(:user_chat_thread_membership, thread: thread_1, user: current_user) + end + fab!(:thread_2_membership) do + Fabricate(:user_chat_thread_membership, thread: thread_2, user: current_user) + end + + context "when not including channels and threads where the user is not a member" do + context "when only channel_ids are provided" do + let(:id_params) { { channel_ids: [channel_1.id, channel_2.id] } } + + it "gets the tracking state of the channels" do + generate_tracking_state + expect(result.report.channel_tracking).to eq( + channel_1.id => { + unread_count: 4, # 2 messages + 2 thread original messages + mention_count: 0, + }, + ) + end + + it "gets the tracking state of the threads in the channels" do + generate_tracking_state + expect(result.report.thread_tracking).to eq( + thread_1.id => { + channel_id: channel_1.id, + unread_count: 1, + mention_count: 0, + }, + thread_2.id => { + channel_id: channel_1.id, + unread_count: 2, + mention_count: 0, + }, + ) + end + + context "when include_threads is false" do + let(:include_threads) { false } + + it "only gets channel tracking state and no thread tracking state" do + generate_tracking_state + expect(result.report.thread_tracking).to eq({}) + expect(result.report.channel_tracking).to eq( + channel_1.id => { + unread_count: 4, # 2 messages + 2 thread original messages + mention_count: 0, + }, + ) + end + end + end + + context "when thread_ids and channel_ids are provided" do + let(:id_params) do + { channel_ids: [channel_1.id, channel_2.id], thread_ids: [thread_2.id] } + end + + it "gets the tracking state of the channels" do + generate_tracking_state + expect(result.report.channel_tracking).to eq( + channel_1.id => { + unread_count: 4, # 2 messages + 2 thread original messages + mention_count: 0, + }, + ) + end + + it "only gets the tracking state of the specified threads in the channels" do + generate_tracking_state + expect(result.report.thread_tracking).to eq( + thread_2.id => { + channel_id: channel_1.id, + unread_count: 2, + mention_count: 0, + }, + ) + end + end + end + + context "when including channels and threads where the user is not a member" do + let(:id_params) { { channel_ids: [channel_1.id, channel_2.id] } } + let(:include_missing_memberships) { true } + let(:include_threads) { true } + + it "gets the tracking state of all channels including the ones where the user is not a member" do + generate_tracking_state + expect(result.report.channel_tracking).to eq( + channel_1.id => { + unread_count: 4, # 2 messages + 2 thread original messages + mention_count: 0, + }, + channel_2.id => { + unread_count: 0, + mention_count: 0, + }, + ) + end + + it "gets the tracking state of all the threads in the channels including the ones where the user is not a member" do + generate_tracking_state + expect(result.report.thread_tracking).to eq( + thread_1.id => { + channel_id: channel_1.id, + unread_count: 1, + mention_count: 0, + }, + thread_2.id => { + channel_id: channel_1.id, + unread_count: 2, + mention_count: 0, + }, + thread_3.id => { + channel_id: channel_2.id, + unread_count: 0, + mention_count: 0, + }, + thread_4.id => { + channel_id: channel_2.id, + unread_count: 0, + mention_count: 0, + }, + ) + end + end + end + end + + def generate_tracking_state + Fabricate(:chat_message, chat_channel: channel_1) + Fabricate(:chat_message, chat_channel: channel_1) + Fabricate(:chat_message, chat_channel: channel_1, thread: thread_1) + Fabricate(:chat_message, chat_channel: channel_1, thread: thread_2) + Fabricate(:chat_message, chat_channel: channel_1, thread: thread_2) + end +end diff --git a/plugins/chat/test/javascripts/acceptance/hashtag-css-generator-test.js b/plugins/chat/test/javascripts/acceptance/hashtag-css-generator-test.js index a122b4ea0c1..cc39cd47a22 100644 --- a/plugins/chat/test/javascripts/acceptance/hashtag-css-generator-test.js +++ b/plugins/chat/test/javascripts/acceptance/hashtag-css-generator-test.js @@ -39,6 +39,14 @@ acceptance("Chat | Hashtag CSS Generator", function (needs) { ], direct_message_channels: [], meta: { message_bus_last_ids: {} }, + tracking: { + channel_tracking: { + 44: { unread_count: 0, mention_count: 0 }, + 74: { unread_count: 0, mention_count: 0 }, + 88: { unread_count: 0, mention_count: 0 }, + }, + thread_tracking: {}, + }, }, }); needs.site({ diff --git a/plugins/chat/test/javascripts/chat-fixtures.js b/plugins/chat/test/javascripts/chat-fixtures.js index df68bc1597a..edaad47b023 100644 --- a/plugins/chat/test/javascripts/chat-fixtures.js +++ b/plugins/chat/test/javascripts/chat-fixtures.js @@ -27,7 +27,6 @@ export const directMessageChannels = [ id: 75, title: "@hawk", current_user_membership: { - unread_count: 0, muted: false, following: true, }, @@ -63,7 +62,6 @@ export const directMessageChannels = [ id: 76, title: "@eviltrout, @markvanlan", current_user_membership: { - unread_count: 0, muted: false, following: true, }, @@ -112,7 +110,6 @@ export const chatChannels = { allow_channel_wide_mentions: true, last_message_sent_at: "2021-07-24T08:14:16.950Z", current_user_membership: { - unread_count: 0, muted: false, following: true, }, @@ -132,7 +129,6 @@ export const chatChannels = { allow_channel_wide_mentions: true, last_message_sent_at: "2021-07-15T08:14:16.950Z", current_user_membership: { - unread_count: 0, muted: false, following: true, }, @@ -152,7 +148,6 @@ export const chatChannels = { allow_channel_wide_mentions: true, last_message_sent_at: "2021-07-14T08:14:16.950Z", current_user_membership: { - unread_count: 0, muted: false, following: true, }, @@ -172,7 +167,6 @@ export const chatChannels = { allow_channel_wide_mentions: true, last_message_sent_at: "2021-07-10T08:14:16.950Z", current_user_membership: { - unread_count: 0, muted: false, following: true, }, @@ -192,7 +186,6 @@ export const chatChannels = { allow_channel_wide_mentions: true, last_message_sent_at: "2021-07-21T08:14:16.950Z", current_user_membership: { - unread_count: 0, muted: false, following: true, }, @@ -212,7 +205,6 @@ export const chatChannels = { allow_channel_wide_mentions: true, last_message_sent_at: "2021-07-25T08:14:16.950Z", current_user_membership: { - unread_count: 0, muted: false, following: true, }, @@ -232,7 +224,6 @@ export const chatChannels = { allow_channel_wide_mentions: true, last_message_sent_at: "2021-07-02T08:14:16.950Z", current_user_membership: { - unread_count: 0, muted: false, following: true, }, @@ -242,6 +233,20 @@ export const chatChannels = { }, }, ], + tracking: { + channel_tracking: { + 4: { unread_count: 0, mention_count: 0 }, + 5: { unread_count: 0, mention_count: 0 }, + 6: { unread_count: 0, mention_count: 0 }, + 7: { unread_count: 0, mention_count: 0 }, + 9: { unread_count: 0, mention_count: 0 }, + 10: { unread_count: 0, mention_count: 0 }, + 11: { unread_count: 0, mention_count: 0 }, + 75: { unread_count: 0, mention_count: 0 }, + 76: { unread_count: 0, mention_count: 0 }, + }, + thread_tracking: {}, + }, direct_message_channels: directMessageChannels.mapBy("chat_channel"), message_bus_last_ids: { channel_metadata: 0, diff --git a/plugins/chat/test/javascripts/components/chat-channel-metadata-test.js b/plugins/chat/test/javascripts/components/chat-channel-metadata-test.js index 4ed5253b727..d9420e6f3e2 100644 --- a/plugins/chat/test/javascripts/components/chat-channel-metadata-test.js +++ b/plugins/chat/test/javascripts/components/chat-channel-metadata-test.js @@ -29,7 +29,7 @@ module("Discourse Chat | Component | chat-channel-metadata", function (hooks) { test("unreadIndicator", async function (assert) { this.channel = fabricators.directMessageChatChannel(); - this.channel.currentUserMembership.unreadCount = 1; + this.channel.tracking.unreadCount = 1; this.unreadIndicator = true; await render( diff --git a/plugins/chat/test/javascripts/components/chat-channel-row-test.js b/plugins/chat/test/javascripts/components/chat-channel-row-test.js index e4e0891971a..7920e2fe7d5 100644 --- a/plugins/chat/test/javascripts/components/chat-channel-row-test.js +++ b/plugins/chat/test/javascripts/components/chat-channel-row-test.js @@ -131,7 +131,7 @@ module("Discourse Chat | Component | chat-channel-row", function (hooks) { assert.dom(".chat-channel-row").doesNotHaveClass("has-unread"); - this.categoryChatChannel.currentUserMembership.unreadCount = 1; + this.categoryChatChannel.tracking.unreadCount = 1; await render(hbs``); diff --git a/plugins/chat/test/javascripts/helpers/chat-pretenders.js b/plugins/chat/test/javascripts/helpers/chat-pretenders.js index 8045130e842..9d46961fc05 100644 --- a/plugins/chat/test/javascripts/helpers/chat-pretenders.js +++ b/plugins/chat/test/javascripts/helpers/chat-pretenders.js @@ -138,7 +138,6 @@ export function directMessageChannelPretender( opts = { unread_count: 0, muted: false } ) { let copy = cloneJSON(directMessageChannels[0]); - copy.chat_channel.currentUserMembership.unreadCount = opts.unread_count; copy.chat_channel.currentUserMembership.muted = opts.muted; server.get("/chat/chat_channels/75.json", () => helper.response(copy)); } @@ -150,13 +149,11 @@ export function chatChannelPretender(server, helper, changes = []) { let found; found = copy.public_channels.find((c) => c.id === change.id); if (found) { - found.currentUserMembership.unreadCount = change.unread_count; found.currentUserMembership.muted = change.muted; } if (!found) { found = copy.direct_message_channels.find((c) => c.id === change.id); if (found) { - found.currentUserMembership.unreadCount = change.unread_count; found.currentUserMembership.muted = change.muted; } }