From bd5c5c4b5f7b33a64cc12e2ba13e81767ac00edc Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Thu, 13 Apr 2023 22:45:50 +1000 Subject: [PATCH] FEATURE: Reacting to MessageBus in chat thread panel (#21070) This commit introduces a ChatChannelPaneSubscriptionsManager and a ChatChannelThreadPaneSubscriptionsManager that inherits from the first service that handle MessageBus subscriptions for the main channel and the thread panel respectively. This necessitated a change to Chat::Publisher to be able to send MessageBus messages to multiple channels based on whether a message was an OM for a thread, a thread reply, or a regular channel message. An initial change to update the thread indicator with new replies has been done too, but that will be improved in future as we have more data to update on the indicators. Still remaining is to fully move over the handleSentMessage functionality which includes scrolling and new message indicator things. Co-authored-by: Joffrey JAFFEUX --- plugins/chat/app/models/chat/message.rb | 6 +- .../app/serializers/chat/thread_serializer.rb | 18 +- plugins/chat/app/services/chat/publisher.rb | 173 ++++++++----- .../discourse/components/chat-live-pane.js | 224 ++--------------- .../discourse/components/chat-thread.hbs | 5 +- .../discourse/components/chat-thread.js | 15 +- .../discourse/lib/chat-message-interactor.js | 11 +- .../discourse/lib/chat-threads-manager.js | 8 + .../routes/chat-channel-decorator.js | 14 +- .../services/chat-channel-composer.js | 35 +-- ...chat-channel-pane-subscriptions-manager.js | 57 +++++ .../services/chat-channel-thread-composer.js | 2 +- ...annel-thread-pane-subscriptions-manager.js | 52 ++++ .../chat-pane-base-subscriptions-manager.js | 238 ++++++++++++++++++ .../stylesheets/common/chat-thread.scss | 10 +- plugins/chat/spec/plugin_helper.rb | 7 + .../chat/spec/services/chat/publisher_spec.rb | 38 +++ .../spec/system/chat_message/channel_spec.rb | 41 +++ .../spec/system/chat_message/thread_spec.rb | 48 ++++ plugins/chat/spec/system/chat_message_spec.rb | 26 -- .../system/message_thread_indicator_spec.rb | 15 ++ .../spec/system/page_objects/chat/chat.rb | 4 + .../system/page_objects/chat/chat_channel.rb | 8 +- .../system/page_objects/chat/chat_thread.rb | 2 +- .../chat/spec/system/single_thread_spec.rb | 56 +++++ plugins/chat/spec/system/transcript_spec.rb | 29 +-- spec/system/page_objects/cdp.rb | 31 +++ 27 files changed, 818 insertions(+), 355 deletions(-) create mode 100644 plugins/chat/assets/javascripts/discourse/services/chat-channel-pane-subscriptions-manager.js create mode 100644 plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-pane-subscriptions-manager.js create mode 100644 plugins/chat/assets/javascripts/discourse/services/chat-pane-base-subscriptions-manager.js create mode 100644 plugins/chat/spec/system/chat_message/channel_spec.rb create mode 100644 plugins/chat/spec/system/chat_message/thread_spec.rb delete mode 100644 plugins/chat/spec/system/chat_message_spec.rb create mode 100644 spec/system/page_objects/cdp.rb diff --git a/plugins/chat/app/models/chat/message.rb b/plugins/chat/app/models/chat/message.rb index f82a2ec1ce6..b8c18249e41 100644 --- a/plugins/chat/app/models/chat/message.rb +++ b/plugins/chat/app/models/chat/message.rb @@ -312,11 +312,11 @@ module Chat end def thread_reply? - in_thread? && !is_thread_om? + in_thread? && !thread_om? end - def is_thread_om? - self.thread.original_message_id == self.id + def thread_om? + in_thread? && self.thread.original_message_id == self.id end private diff --git a/plugins/chat/app/serializers/chat/thread_serializer.rb b/plugins/chat/app/serializers/chat/thread_serializer.rb index 0408387c0f2..d2f37bd72f5 100644 --- a/plugins/chat/app/serializers/chat/thread_serializer.rb +++ b/plugins/chat/app/serializers/chat/thread_serializer.rb @@ -5,6 +5,22 @@ module Chat has_one :original_message_user, serializer: BasicUserWithStatusSerializer, embed: :objects has_one :original_message, serializer: Chat::ThreadOriginalMessageSerializer, embed: :objects - attributes :id, :title, :status + attributes :id, :title, :status, :channel_id, :meta + + def initialize(object, opts) + super(object, opts) + @opts = opts + end + + def meta + { message_bus_last_ids: { thread_message_bus_last_id: thread_message_bus_last_id } } + end + + private + + def thread_message_bus_last_id + @opts[:thread_message_bus_last_id] || + MessageBus.last_id(Chat::Publisher.thread_message_bus_channel(object.channel_id, object.id)) + end end end diff --git a/plugins/chat/app/services/chat/publisher.rb b/plugins/chat/app/services/chat/publisher.rb index f2964828d92..5aa87e9ad05 100644 --- a/plugins/chat/app/services/chat/publisher.rb +++ b/plugins/chat/app/services/chat/publisher.rb @@ -10,8 +10,27 @@ module Chat "/chat/#{chat_channel_id}" end + def self.thread_message_bus_channel(chat_channel_id, thread_id) + "#{root_message_bus_channel(chat_channel_id)}/thread/#{thread_id}" + end + + def self.calculate_publish_targets(channel, message) + targets = + if message.thread_om? + [ + root_message_bus_channel(channel.id), + thread_message_bus_channel(channel.id, message.thread_id), + ] + elsif message.thread_reply? + [thread_message_bus_channel(channel.id, message.thread_id)] + else + [root_message_bus_channel(channel.id)] + end + targets + end + def self.publish_new!(chat_channel, chat_message, staged_id) - return if chat_message.thread_reply? + message_bus_targets = calculate_publish_targets(chat_channel, chat_message) content = Chat::MessageSerializer.new( @@ -22,19 +41,38 @@ module Chat content[:staged_id] = staged_id permissions = permissions(chat_channel) - MessageBus.publish(root_message_bus_channel(chat_channel.id), content.as_json, permissions) + message_bus_targets.each do |message_bus_channel| + MessageBus.publish(message_bus_channel, content.as_json, permissions) + end - MessageBus.publish( - self.new_messages_message_bus_channel(chat_channel.id), - { - 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, - ) + if chat_message.thread_reply? + MessageBus.publish( + root_message_bus_channel(chat_channel.id), + { + type: :update_thread_original_message, + original_message_id: chat_message.thread.original_message_id, + action: :increment_reply_count, + }.as_json, + permissions, + ) + end + + # 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? + MessageBus.publish( + self.new_messages_message_bus_channel(chat_channel.id), + { + 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, + ) + end end def self.publish_thread_created!(chat_channel, chat_message) @@ -50,7 +88,7 @@ module Chat end def self.publish_processed!(chat_message) - return if chat_message.thread_reply? + message_bus_targets = calculate_publish_targets(chat_channel, chat_message) chat_channel = chat_message.chat_channel content = { @@ -60,15 +98,14 @@ module Chat cooked: chat_message.cooked, }, } - MessageBus.publish( - root_message_bus_channel(chat_channel.id), - content.as_json, - permissions(chat_channel), - ) + + message_bus_targets.each do |message_bus_channel| + MessageBus.publish(message_bus_channel, content.as_json, permissions(chat_channel)) + end end def self.publish_edit!(chat_channel, chat_message) - return if chat_message.thread_reply? + message_bus_targets = calculate_publish_targets(chat_channel, chat_message) content = Chat::MessageSerializer.new( @@ -76,15 +113,14 @@ module Chat { scope: anonymous_guardian, root: :chat_message }, ).as_json content[:type] = :edit - MessageBus.publish( - root_message_bus_channel(chat_channel.id), - content.as_json, - permissions(chat_channel), - ) + + message_bus_targets.each do |message_bus_channel| + MessageBus.publish(message_bus_channel, content.as_json, permissions(chat_channel)) + end end def self.publish_refresh!(chat_channel, chat_message) - return if chat_message.thread_reply? + message_bus_targets = calculate_publish_targets(chat_channel, chat_message) content = Chat::MessageSerializer.new( @@ -92,15 +128,14 @@ module Chat { scope: anonymous_guardian, root: :chat_message }, ).as_json content[:type] = :refresh - MessageBus.publish( - root_message_bus_channel(chat_channel.id), - content.as_json, - permissions(chat_channel), - ) + + message_bus_targets.each do |message_bus_channel| + MessageBus.publish(message_bus_channel, content.as_json, permissions(chat_channel)) + end end def self.publish_reaction!(chat_channel, chat_message, action, user, emoji) - return if chat_message.thread_reply? + message_bus_targets = calculate_publish_targets(chat_channel, chat_message) content = { action: action, @@ -109,11 +144,10 @@ module Chat type: :reaction, chat_message_id: chat_message.id, } - MessageBus.publish( - root_message_bus_channel(chat_channel.id), - content.as_json, - permissions(chat_channel), - ) + + message_bus_targets.each do |message_bus_channel| + MessageBus.publish(message_bus_channel, content.as_json, permissions(chat_channel)) + end end def self.publish_presence!(chat_channel, user, typ) @@ -121,16 +155,20 @@ module Chat end def self.publish_delete!(chat_channel, chat_message) - return if chat_message.thread_reply? + message_bus_targets = calculate_publish_targets(chat_channel, chat_message) - MessageBus.publish( - root_message_bus_channel(chat_channel.id), - { type: "delete", deleted_id: chat_message.id, deleted_at: chat_message.deleted_at }, - permissions(chat_channel), - ) + message_bus_targets.each do |message_bus_channel| + MessageBus.publish( + message_bus_channel, + { type: "delete", deleted_id: chat_message.id, deleted_at: chat_message.deleted_at }, + permissions(chat_channel), + ) + end end def self.publish_bulk_delete!(chat_channel, deleted_message_ids) + # TODO (martin) Handle sending this through for all the threads that + # may contain the deleted messages as well. MessageBus.publish( root_message_bus_channel(chat_channel.id), { typ: "bulk_delete", deleted_ids: deleted_message_ids, deleted_at: Time.zone.now }, @@ -139,7 +177,7 @@ module Chat end def self.publish_restore!(chat_channel, chat_message) - return if chat_message.thread_reply? + message_bus_targets = calculate_publish_targets(chat_channel, chat_message) content = Chat::MessageSerializer.new( @@ -147,33 +185,36 @@ module Chat { scope: anonymous_guardian, root: :chat_message }, ).as_json content[:type] = :restore - MessageBus.publish( - root_message_bus_channel(chat_channel.id), - content.as_json, - permissions(chat_channel), - ) + + message_bus_targets.each do |message_bus_channel| + MessageBus.publish(message_bus_channel, content.as_json, permissions(chat_channel)) + end end def self.publish_flag!(chat_message, user, reviewable, score) - return if chat_message.thread_reply? + message_bus_targets = calculate_publish_targets(chat_message.chat_channel, chat_message) - # Publish to user who created flag - MessageBus.publish( - "/chat/#{chat_message.chat_channel_id}", - { - type: "self_flagged", - user_flag_status: score.status_for_database, - chat_message_id: chat_message.id, - }.as_json, - user_ids: [user.id], - ) + message_bus_targets.each do |message_bus_channel| + # Publish to user who created flag + MessageBus.publish( + message_bus_channel, + { + type: "self_flagged", + user_flag_status: score.status_for_database, + chat_message_id: chat_message.id, + }.as_json, + user_ids: [user.id], + ) + end - # Publish flag with link to reviewable to staff - MessageBus.publish( - "/chat/#{chat_message.chat_channel_id}", - { type: "flag", chat_message_id: chat_message.id, reviewable_id: reviewable.id }.as_json, - group_ids: [Group::AUTO_GROUPS[:staff]], - ) + message_bus_targets.each do |message_bus_channel| + # Publish flag with link to reviewable to staff + MessageBus.publish( + message_bus_channel, + { type: "flag", chat_message_id: chat_message.id, reviewable_id: reviewable.id }.as_json, + group_ids: [Group::AUTO_GROUPS[:staff]], + ) + end end def self.user_tracking_state_message_bus_channel(user_id) diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js b/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js index bb1408d99f8..37b91e116b9 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-live-pane.js @@ -4,7 +4,10 @@ import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message"; import ChatMessageDraft from "discourse/plugins/chat/discourse/models/chat-message-draft"; import Component from "@glimmer/component"; import { bind, debounce } from "discourse-common/utils/decorators"; -import EmberObject, { action } from "@ember/object"; +import { action } from "@ember/object"; +// TODO (martin) Remove this when the handleSentMessage logic inside chatChannelPaneSubscriptionsManager +// is moved over from this file completely. +import { handleStagedMessage } from "discourse/plugins/chat/discourse/services/chat-pane-base-subscriptions-manager"; import { ajax } from "discourse/lib/ajax"; import { popupAjaxError } from "discourse/lib/ajax-error"; import { cancel, schedule, throttle } from "@ember/runloop"; @@ -34,6 +37,7 @@ export default class ChatLivePane extends Component { @service chatStateManager; @service chatChannelComposer; @service chatChannelPane; + @service chatChannelPaneSubscriptionsManager; @service chatApi; @service currentUser; @service appEvents; @@ -108,7 +112,7 @@ export default class ChatLivePane extends Component { } this.loadMessages(); - this._subscribeToUpdates(this.args.channel?.id); + this._subscribeToUpdates(this.args.channel); } @action @@ -209,8 +213,8 @@ export default class ChatLivePane extends Component { const loadingMoreKey = `loadingMore${capitalize(direction)}`; const canLoadMore = loadingPast - ? this.args.channel.messagesManager.canLoadMorePast - : this.args.channel.messagesManager.canLoadMoreFuture; + ? this.#messagesManager.canLoadMorePast + : this.#messagesManager.canLoadMoreFuture; if ( !canLoadMore || @@ -261,7 +265,7 @@ export default class ChatLivePane extends Component { } this.args.channel.details = meta; - this.args.channel.messagesManager.addMessages(messages); + this.#messagesManager.addMessages(messages); // Edge case for IOS to avoid blank screens // and/or scrolling to bottom losing track of scroll position @@ -508,9 +512,9 @@ export default class ChatLivePane extends Component { } removeMessage(msgData) { - const message = this.args.channel.messagesManager.findMessage(msgData.id); + const message = this.#messagesManager.findMessage(msgData.id); if (message) { - this.args.channel.messagesManager.removeMessage(message); + this.#messagesManager.removeMessage(message); } } @@ -520,72 +524,6 @@ export default class ChatLivePane extends Component { case "sent": this.handleSentMessage(data); break; - case "processed": - this.handleProcessedMessage(data); - break; - case "edit": - this.handleEditMessage(data); - break; - case "refresh": - this.handleRefreshMessage(data); - break; - case "delete": - this.handleDeleteMessage(data); - break; - case "bulk_delete": - this.handleBulkDeleteMessage(data); - break; - case "reaction": - this.handleReactionMessage(data); - break; - case "restore": - this.handleRestoreMessage(data); - break; - case "mention_warning": - this.handleMentionWarning(data); - break; - case "self_flagged": - this.handleSelfFlaggedMessage(data); - break; - case "flag": - this.handleFlaggedMessage(data); - break; - case "thread_created": - this.handleThreadCreated(data); - break; - } - } - - handleThreadCreated(data) { - const message = this.args.channel.messagesManager.findMessage( - data.chat_message.id - ); - if (message) { - message.threadId = data.chat_message.thread_id; - message.threadReplyCount = 1; - } - } - - _handleStagedMessage(stagedMessage, data) { - stagedMessage.error = null; - stagedMessage.id = data.chat_message.id; - stagedMessage.staged = false; - stagedMessage.excerpt = data.chat_message.excerpt; - stagedMessage.threadId = data.chat_message.thread_id; - stagedMessage.channelId = data.chat_message.chat_channel_id; - stagedMessage.createdAt = data.chat_message.created_at; - - const inReplyToMsg = this.args.channel.messagesManager.findMessage( - data.chat_message.in_reply_to?.id - ); - if (inReplyToMsg && !inReplyToMsg.threadId) { - inReplyToMsg.threadId = data.chat_message.thread_id; - } - - // some markdown is cooked differently on the server-side, e.g. - // quotes, avatar images etc. - if (data.chat_message?.cooked !== stagedMessage.cooked) { - stagedMessage.cooked = data.chat_message.cooked; } } @@ -595,139 +533,30 @@ export default class ChatLivePane extends Component { } if (data.chat_message.user.id === this.currentUser.id && data.staged_id) { - const stagedMessage = this.args.channel.messagesManager.findStagedMessage( - data.staged_id - ); + const stagedMessage = handleStagedMessage(this.#messagesManager, data); if (stagedMessage) { - return this._handleStagedMessage(stagedMessage, data); + return; } } - if (this.args.channel.messagesManager.canLoadMoreFuture) { + if (this.#messagesManager.canLoadMoreFuture) { // If we can load more messages, we just notice the user of new messages this.hasNewMessages = true; } else if (this.#isTowardsBottom()) { // If we are at the bottom, we append the message and scroll to it const message = ChatMessage.create(this.args.channel, data.chat_message); - this.args.channel.messagesManager.addMessages([message]); + this.#messagesManager.addMessages([message]); this.scrollToLatestMessage(); this.updateLastReadMessage(); } else { // If we are almost at the bottom, we append the message and notice the user const message = ChatMessage.create(this.args.channel, data.chat_message); - this.args.channel.messagesManager.addMessages([message]); + this.#messagesManager.addMessages([message]); this.hasNewMessages = true; } } - handleProcessedMessage(data) { - const message = this.args.channel.messagesManager.findMessage( - data.chat_message.id - ); - if (message) { - message.cooked = data.chat_message.cooked; - this.scrollToLatestMessage(); - } - } - - handleRefreshMessage(data) { - const message = this.args.channel.messagesManager.findMessage( - data.chat_message.id - ); - if (message) { - message.incrementVersion(); - } - } - - handleEditMessage(data) { - const message = this.args.channel.messagesManager.findMessage( - data.chat_message.id - ); - if (message) { - message.message = data.chat_message.message; - message.cooked = data.chat_message.cooked; - message.excerpt = data.chat_message.excerpt; - message.uploads = cloneJSON(data.chat_message.uploads || []); - message.edited = true; - message.incrementVersion(); - } - } - - handleBulkDeleteMessage(data) { - data.deleted_ids.forEach((deletedId) => { - this.handleDeleteMessage({ - deleted_id: deletedId, - deleted_at: data.deleted_at, - }); - }); - } - - handleDeleteMessage(data) { - const deletedId = data.deleted_id; - const targetMsg = this.args.channel.messagesManager.findMessage(deletedId); - - if (!targetMsg) { - return; - } - - if (this.currentUser.staff || this.currentUser.id === targetMsg.user.id) { - targetMsg.deletedAt = data.deleted_at; - targetMsg.expanded = false; - } else { - this.args.channel.messagesManager.removeMessage(targetMsg); - } - } - - handleReactionMessage(data) { - const message = this.args.channel.messagesManager.findMessage( - data.chat_message_id - ); - if (message) { - message.react(data.emoji, data.action, data.user, this.currentUser.id); - } - } - - handleRestoreMessage(data) { - const message = this.args.channel.messagesManager.findMessage( - data.chat_message.id - ); - if (message) { - message.deletedAt = null; - } else { - this.args.channel.messagesManager.addMessages([ - ChatMessage.create(this.args.channel, data.chat_message), - ]); - } - } - - handleMentionWarning(data) { - const message = this.args.channel.messagesManager.findMessage( - data.chat_message_id - ); - if (message) { - message.mentionWarning = EmberObject.create(data); - } - } - - handleSelfFlaggedMessage(data) { - const message = this.args.channel.messagesManager.findMessage( - data.chat_message_id - ); - if (message) { - message.userFlagStatus = data.user_flag_status; - } - } - - handleFlaggedMessage(data) { - const message = this.args.channel.messagesManager.findMessage( - data.chat_message_id - ); - if (message) { - message.reviewableId = data.reviewable_id; - } - } - // TODO (martin) Maybe change this to public, since its referred to by // livePanel.linkedComponent at the moment. get _selfDeleted() { @@ -788,13 +617,13 @@ export default class ChatLivePane extends Component { if (stagedMessage.inReplyTo) { if (!this.args.channel.threadingEnabled) { - this.args.channel.messagesManager.addMessages([stagedMessage]); + this.#messagesManager.addMessages([stagedMessage]); } } else { - this.args.channel.messagesManager.addMessages([stagedMessage]); + this.#messagesManager.addMessages([stagedMessage]); } - if (!this.args.channel.messagesManager.canLoadMoreFuture) { + if (!this.#messagesManager.canLoadMoreFuture) { this.scrollToLatestMessage(); } @@ -844,8 +673,7 @@ export default class ChatLivePane extends Component { } _onSendError(id, error) { - const stagedMessage = - this.args.channel.messagesManager.findStagedMessage(id); + const stagedMessage = this.#messagesManager.findStagedMessage(id); if (stagedMessage) { if (error.jqXHR?.responseJSON?.errors?.length) { // only network errors are retryable @@ -910,20 +738,22 @@ export default class ChatLivePane extends Component { return; } + this.chatChannelPaneSubscriptionsManager.unsubscribe(); this.messageBus.unsubscribe(`/chat/${channelId}`, this.onMessage); } - _subscribeToUpdates(channelId) { - if (!channelId) { + _subscribeToUpdates(channel) { + if (!channel) { return; } - this._unsubscribeToUpdates(channelId); + this._unsubscribeToUpdates(channel.id); this.messageBus.subscribe( - `/chat/${channelId}`, + `/chat/${channel.id}`, this.onMessage, - this.args.channel.channelMessageBusLastId + channel.channelMessageBusLastId ); + this.chatChannelPaneSubscriptionsManager.subscribe(channel); } @bind diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs index baaa67e3f9c..1c00da944af 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs @@ -1,14 +1,17 @@
{{#if @includeHeader}}
{{i18n "chat.thread.label"}} diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-thread.js b/plugins/chat/assets/javascripts/discourse/components/chat-thread.js index 9b147442f55..ee63ef2ed39 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-thread.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-thread.js @@ -21,6 +21,7 @@ export default class ChatThreadPanel extends Component { @service chatComposerPresenceManager; @service chatChannelThreadComposer; @service chatChannelThreadPane; + @service chatChannelThreadPaneSubscriptionsManager; @service appEvents; @service capabilities; @@ -37,6 +38,16 @@ export default class ChatThreadPanel extends Component { return this.chat.activeChannel; } + @action + subscribeToUpdates() { + this.chatChannelThreadPaneSubscriptionsManager.subscribe(this.thread); + } + + @action + unsubscribeFromUpdates() { + this.chatChannelThreadPaneSubscriptionsManager.unsubscribe(); + } + @action setScrollable(element) { this.scrollable = element; @@ -189,7 +200,7 @@ export default class ChatThreadPanel extends Component { .sendMessage(this.channel.id, { message: stagedMessage.message, in_reply_to_id: stagedMessage.inReplyTo?.id, - staged_id: stagedMessage.stagedId, + staged_id: stagedMessage.id, upload_ids: stagedMessage.uploads.map((upload) => upload.id), thread_id: stagedMessage.threadId, }) @@ -197,7 +208,7 @@ export default class ChatThreadPanel extends Component { this.scrollToBottom(); }) .catch((error) => { - this.#onSendError(stagedMessage.stagedId, error); + this.#onSendError(stagedMessage.id, error); }) .finally(() => { if (this._selfDeleted) { diff --git a/plugins/chat/assets/javascripts/discourse/lib/chat-message-interactor.js b/plugins/chat/assets/javascripts/discourse/lib/chat-message-interactor.js index 9c1b6a7d2dc..349b83f577e 100644 --- a/plugins/chat/assets/javascripts/discourse/lib/chat-message-interactor.js +++ b/plugins/chat/assets/javascripts/discourse/lib/chat-message-interactor.js @@ -229,7 +229,16 @@ export default class ChatMessageInteractor { copyLink() { const { protocol, host } = window.location; - let url = getURL(`/chat/c/-/${this.message.channelId}/${this.message.id}`); + const channelId = this.message.channelId; + const threadId = this.message.threadId; + + let url; + if (threadId) { + url = getURL(`/chat/c/-/${channelId}/t/${threadId}`); + } else { + url = getURL(`/chat/c/-/${channelId}/${this.message.id}`); + } + url = url.indexOf("/") === 0 ? protocol + "//" + host + url : url; clipboardCopy(url); } 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 0fdd45d1bcf..a83a34c87c1 100644 --- a/plugins/chat/assets/javascripts/discourse/lib/chat-threads-manager.js +++ b/plugins/chat/assets/javascripts/discourse/lib/chat-threads-manager.js @@ -47,6 +47,14 @@ export default class ChatThreadsManager { this.#cache(model); } + if ( + threadObject.meta?.message_bus_last_ids?.thread_message_bus_last_id !== + undefined + ) { + model.threadMessageBusLastId = + threadObject.meta.message_bus_last_ids.thread_message_bus_last_id; + } + return model; } diff --git a/plugins/chat/assets/javascripts/discourse/routes/chat-channel-decorator.js b/plugins/chat/assets/javascripts/discourse/routes/chat-channel-decorator.js index 383dcbc5a36..25433ed2d4d 100644 --- a/plugins/chat/assets/javascripts/discourse/routes/chat-channel-decorator.js +++ b/plugins/chat/assets/javascripts/discourse/routes/chat-channel-decorator.js @@ -30,12 +30,20 @@ export default function withChatChannel(extendedClass) { } if (channelTitle && channelTitle !== model.slugifiedTitle) { - const nearMessageParams = this.paramsFor("chat.channel.near-message"); - if (nearMessageParams.messageId) { + messageId = this.paramsFor("chat.channel.near-message").messageId; + const threadId = this.paramsFor("chat.channel.thread").threadId; + + if (threadId) { + this.router.replaceWith( + "chat.channel.thread", + ...model.routeModels, + threadId + ); + } else if (messageId) { this.router.replaceWith( "chat.channel.near-message", ...model.routeModels, - nearMessageParams.messageId + messageId ); } else { this.router.replaceWith("chat.channel", ...model.routeModels); diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-channel-composer.js b/plugins/chat/assets/javascripts/discourse/services/chat-channel-composer.js index f809c07caff..e06ed71af0b 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-channel-composer.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-channel-composer.js @@ -16,7 +16,7 @@ export default class ChatChannelComposer extends Service { this.replyToMsg = null; } - get #model() { + get model() { return this.chat.activeChannel; } @@ -26,7 +26,7 @@ export default class ChatChannelComposer extends Service { const message = typeof messageOrId === "number" - ? this.#model.messagesManager.findMessage(messageOrId) + ? this.model.messagesManager.findMessage(messageOrId) : messageOrId; this.replyToMsg = message; this.focusComposer(); @@ -38,7 +38,7 @@ export default class ChatChannelComposer extends Service { } editButtonClicked(messageId) { - const message = this.#model.messagesManager.findMessage(messageId); + const message = this.model.messagesManager.findMessage(messageId); this.editingMessage = message; // TODO (martin) Move scrollToLatestMessage to live panel. @@ -53,13 +53,13 @@ export default class ChatChannelComposer extends Service { replyToMsg, inProgressUploadsCount, }) { - if (!this.#model) { + if (!this.model) { return; } - if (!this.editingMessage && !this.#model.isDraft) { - if (typeof value !== "undefined") { - this.#model.draft.message = value; + if (!this.editingMessage && !this.model.isDraft) { + if (typeof value !== "undefined" && this.model.draft) { + this.model.draft.message = value; } // only save the uploads to the draft if we are not still uploading other @@ -69,17 +69,18 @@ export default class ChatChannelComposer extends Service { if ( typeof uploads !== "undefined" && inProgressUploadsCount !== "undefined" && - inProgressUploadsCount === 0 + inProgressUploadsCount === 0 && + this.model.draft ) { - this.#model.draft.uploads = uploads; + this.model.draft.uploads = uploads; } - if (typeof replyToMsg !== "undefined") { - this.#model.draft.replyToMsg = replyToMsg; + if (typeof replyToMsg !== "undefined" && this.model.draft) { + this.model.draft.replyToMsg = replyToMsg; } } - if (!this.#model.isDraft) { + if (!this.model.isDraft) { this.#reportReplyingPresence(value); } @@ -103,25 +104,25 @@ export default class ChatChannelComposer extends Service { return; } - if (this.#model.isDraft) { + if (this.model.isDraft) { return; } const replying = !this.editingMessage && !!composerValue; - this.chatComposerPresenceManager.notifyState(this.#model.id, replying); + this.chatComposerPresenceManager.notifyState(this.model.id, replying); } @debounce(2000) _persistDraft() { - if (this.#componentDeleted || !this.#model) { + if (this.#componentDeleted || !this.model) { return; } - if (!this.#model.draft) { + if (!this.model.draft) { return; } - return this.chatApi.saveDraft(this.#model.id, this.#model.draft.toJSON()); + return this.chatApi.saveDraft(this.model.id, this.model.draft.toJSON()); } get #componentDeleted() { diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-channel-pane-subscriptions-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-channel-pane-subscriptions-manager.js new file mode 100644 index 00000000000..30d4aec8de3 --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/services/chat-channel-pane-subscriptions-manager.js @@ -0,0 +1,57 @@ +import { inject as service } from "@ember/service"; +import ChatPaneBaseSubscriptionsManager from "./chat-pane-base-subscriptions-manager"; + +export default class ChatChannelPaneSubscriptionsManager extends ChatPaneBaseSubscriptionsManager { + @service chat; + @service currentUser; + + get messageBusChannel() { + return `/chat/${this.model.id}`; + } + + get messageBusLastId() { + return this.model.channelMessageBusLastId; + } + + // TODO (martin) Implement this for the channel, since it involves a bunch + // of scrolling and pane-specific logic. Will leave the existing sub inside + // ChatLivePane for now. + handleSentMessage() { + return; + } + + // TODO (martin) Move scrolling functionality to pane from ChatLivePane? + afterProcessedMessage() { + // this.scrollToLatestMessage(); + return; + } + + handleBulkDeleteMessage(data) { + data.deleted_ids.forEach((deletedId) => { + this.handleDeleteMessage({ + deleted_id: deletedId, + deleted_at: data.deleted_at, + }); + }); + } + + handleThreadCreated(data) { + const message = this.messagesManager.findMessage(data.chat_message.id); + if (message) { + message.threadId = data.chat_message.thread_id; + message.threadReplyCount = 0; + } + } + + handleThreadOriginalMessageUpdate(data) { + const message = this.messagesManager.findMessage(data.original_message_id); + if (message) { + if (data.action === "increment_reply_count") { + // TODO (martin) In future we should use a replies_count delivered + // from the server and simply update the message accordingly, for + // now we don't have an accurate enough count for this. + message.threadReplyCount += 1; + } + } + } +} diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-composer.js b/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-composer.js index 904644a3971..bd691b7ce44 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-composer.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-composer.js @@ -1,7 +1,7 @@ import ChatChannelComposer from "./chat-channel-composer"; export default class extends ChatChannelComposer { - get #model() { + get model() { return this.chat.activeChannel.activeThread; } diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-pane-subscriptions-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-pane-subscriptions-manager.js new file mode 100644 index 00000000000..a3e897b028f --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-pane-subscriptions-manager.js @@ -0,0 +1,52 @@ +import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message"; +import ChatPaneBaseSubscriptionsManager from "./chat-pane-base-subscriptions-manager"; + +export default class ChatChannelThreadPaneSubscriptionsManager extends ChatPaneBaseSubscriptionsManager { + get messageBusChannel() { + return `/chat/${this.model.channelId}/thread/${this.model.id}`; + } + + get messageBusLastId() { + return this.model.threadMessageBusLastId; + } + + handleSentMessage(data) { + if (data.chat_message.user.id === this.currentUser.id && data.staged_id) { + const stagedMessage = this.handleStagedMessageInternal(data); + if (stagedMessage) { + return; + } + } + + const message = ChatMessage.create( + this.chat.activeChannel, + data.chat_message + ); + this.messagesManager.addMessages([message]); + + // TODO (martin) All the scrolling and new message indicator shenanigans. + } + + // NOTE: noop, there is nothing to do when a thread is created + // inside the thread panel. + handleThreadCreated() { + return; + } + + // NOTE: noop, there is nothing to do when a thread original message + // is updated inside the thread panel (for now). + handleThreadOriginalMessageUpdate() { + return; + } + + // TODO (martin) Hook this up correctly in Chat::Publisher for threads. + handleBulkDeleteMessage() { + return; + } + + // NOTE: noop for now, later we may want to do scrolling or something like + // we do in the channel pane. + afterProcessedMessage() { + return; + } +} diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-pane-base-subscriptions-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-pane-base-subscriptions-manager.js new file mode 100644 index 00000000000..6e102f9db5b --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/services/chat-pane-base-subscriptions-manager.js @@ -0,0 +1,238 @@ +import Service, { inject as service } from "@ember/service"; +import EmberObject from "@ember/object"; +import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message"; +import { cloneJSON } from "discourse-common/lib/object"; +import { bind } from "discourse-common/utils/decorators"; + +// TODO (martin) This export can be removed once we move the handleSentMessage +// code completely out of ChatLivePane +export function handleStagedMessage(messagesManager, data) { + const stagedMessage = messagesManager.findStagedMessage(data.staged_id); + + if (!stagedMessage) { + return; + } + + stagedMessage.error = null; + stagedMessage.id = data.chat_message.id; + stagedMessage.staged = false; + stagedMessage.excerpt = data.chat_message.excerpt; + stagedMessage.threadId = data.chat_message.thread_id; + stagedMessage.channelId = data.chat_message.chat_channel_id; + stagedMessage.createdAt = data.chat_message.created_at; + + const inReplyToMsg = messagesManager.findMessage( + data.chat_message.in_reply_to?.id + ); + if (inReplyToMsg && !inReplyToMsg.threadId) { + inReplyToMsg.threadId = data.chat_message.thread_id; + } + + // some markdown is cooked differently on the server-side, e.g. + // quotes, avatar images etc. + if (data.chat_message?.cooked !== stagedMessage.cooked) { + stagedMessage.cooked = data.chat_message.cooked; + } + + return stagedMessage; +} + +/** + * Handles subscriptions for MessageBus messages sent from Chat::Publisher + * to the channel and thread panes. There are individual services for + * each (ChatChannelPaneSubscriptionsManager and ChatChannelThreadPaneSubscriptionsManager) + * that implement their own logic where necessary. Functions which will + * always be different between the two raise a "not implemented" error in + * the base class, and the child class must define the associated function, + * even if it is a noop in that context. + * + * For example, in the thread context there is no need to handle the thread + * creation event, because the panel will not be open in that case. + */ +export default class ChatPaneBaseSubscriptionsManager extends Service { + @service chat; + @service currentUser; + + get messageBusChannel() { + throw "not implemented"; + } + + get messageBusLastId() { + throw "not implemented"; + } + + get messagesManager() { + return this.model.messagesManager; + } + + subscribe(model) { + this.unsubscribe(); + this.model = model; + this.messageBus.subscribe( + this.messageBusChannel, + this.onMessage, + this.messageBusLastId + ); + } + + unsubscribe() { + if (!this.model) { + return; + } + this.messageBus.unsubscribe(this.messageBusChannel, this.onMessage); + this.model = null; + } + + // TODO (martin) This can be removed once we move the handleSentMessage + // code completely out of ChatLivePane + handleStagedMessageInternal(data) { + return handleStagedMessage(this.messagesManager, data); + } + + @bind + onMessage(busData) { + switch (busData.type) { + case "sent": + this.handleSentMessage(busData); + break; + case "reaction": + this.handleReactionMessage(busData); + break; + case "processed": + this.handleProcessedMessage(busData); + break; + case "edit": + this.handleEditMessage(busData); + break; + case "refresh": + this.handleRefreshMessage(busData); + break; + case "delete": + this.handleDeleteMessage(busData); + break; + case "bulk_delete": + this.handleBulkDeleteMessage(busData); + break; + case "restore": + this.handleRestoreMessage(busData); + break; + case "mention_warning": + this.handleMentionWarning(busData); + break; + case "self_flagged": + this.handleSelfFlaggedMessage(busData); + break; + case "flag": + this.handleFlaggedMessage(busData); + break; + case "thread_created": + this.handleThreadCreated(busData); + break; + case "update_thread_original_message": + this.handleThreadOriginalMessageUpdate(busData); + break; + } + } + + handleSentMessage() { + throw "not implemented"; + } + + handleProcessedMessage(data) { + const message = this.messagesManager.findMessage(data.chat_message.id); + if (message) { + message.cooked = data.chat_message.cooked; + this.afterProcessedMessage(message); + } + } + + afterProcessedMessage() { + throw "not implemented"; + } + + handleReactionMessage(data) { + const message = this.messagesManager.findMessage(data.chat_message_id); + if (message) { + message.react(data.emoji, data.action, data.user, this.currentUser.id); + } + } + + handleEditMessage(data) { + const message = this.messagesManager.findMessage(data.chat_message.id); + if (message) { + message.message = data.chat_message.message; + message.cooked = data.chat_message.cooked; + message.excerpt = data.chat_message.excerpt; + message.uploads = cloneJSON(data.chat_message.uploads || []); + message.edited = true; + message.incrementVersion(); + } + } + + handleRefreshMessage(data) { + const message = this.messagesManager.findMessage(data.chat_message.id); + if (message) { + message.incrementVersion(); + } + } + + handleBulkDeleteMessage() { + throw "not implemented"; + } + + handleDeleteMessage(data) { + const deletedId = data.deleted_id; + const targetMsg = this.messagesManager.findMessage(deletedId); + + if (!targetMsg) { + return; + } + + if (this.currentUser.staff || this.currentUser.id === targetMsg.user.id) { + targetMsg.deletedAt = data.deleted_at; + targetMsg.expanded = false; + } else { + this.messagesManager.removeMessage(targetMsg); + } + } + + handleRestoreMessage(data) { + const message = this.messagesManager.findMessage(data.chat_message.id); + if (message) { + message.deletedAt = null; + } else { + this.messagesManager.addMessages([ + ChatMessage.create(this.args.channel, data.chat_message), + ]); + } + } + + handleMentionWarning(data) { + const message = this.messagesManager.findMessage(data.chat_message_id); + if (message) { + message.mentionWarning = EmberObject.create(data); + } + } + + handleSelfFlaggedMessage(data) { + const message = this.messagesManager.findMessage(data.chat_message_id); + if (message) { + message.userFlagStatus = data.user_flag_status; + } + } + + handleFlaggedMessage(data) { + const message = this.messagesManager.findMessage(data.chat_message_id); + if (message) { + message.reviewableId = data.reviewable_id; + } + } + + handleThreadCreated() { + throw "not implemented"; + } + + handleThreadOriginalMessageUpdate() { + throw "not implemented"; + } +} diff --git a/plugins/chat/assets/stylesheets/common/chat-thread.scss b/plugins/chat/assets/stylesheets/common/chat-thread.scss index e998b7de1af..4204eeead4b 100644 --- a/plugins/chat/assets/stylesheets/common/chat-thread.scss +++ b/plugins/chat/assets/stylesheets/common/chat-thread.scss @@ -12,7 +12,7 @@ display: flex; align-items: center; justify-content: space-between; - padding-inline: 1.5rem; + padding-inline: 1rem; } &__body { @@ -27,12 +27,4 @@ flex-direction: column-reverse; will-change: transform; } - - &__close { - color: var(--primary-medium); - - &:visited { - color: var(--primary-medium); - } - } } diff --git a/plugins/chat/spec/plugin_helper.rb b/plugins/chat/spec/plugin_helper.rb index aed5274fe98..6dfd5c66c3b 100644 --- a/plugins/chat/spec/plugin_helper.rb +++ b/plugins/chat/spec/plugin_helper.rb @@ -20,6 +20,13 @@ module ChatSystemHelpers Group.refresh_automatic_groups! end + def chat_system_user_bootstrap(user:, channel:) + user.activate + user.user_option.update!(chat_enabled: true) + Group.refresh_automatic_group!("trust_level_#{user.trust_level}".to_sym) + Fabricate(:user_chat_channel_membership, chat_channel: channel, user: user) + end + def chat_thread_chain_bootstrap(channel:, users:, messages_count: 4) last_user = nil last_message = nil diff --git a/plugins/chat/spec/services/chat/publisher_spec.rb b/plugins/chat/spec/services/chat/publisher_spec.rb index ec22a9a05f1..d5cc2ec781f 100644 --- a/plugins/chat/spec/services/chat/publisher_spec.rb +++ b/plugins/chat/spec/services/chat/publisher_spec.rb @@ -14,4 +14,42 @@ describe Chat::Publisher do expect(data["type"]).to eq("refresh") end end + + describe ".calculate_publish_targets" do + context "when the chat message is the original message of a thread" do + fab!(:thread) { Fabricate(:chat_thread, original_message: message, channel: channel) } + + it "generates the correct targets" do + targets = described_class.calculate_publish_targets(channel, message) + expect(targets).to contain_exactly( + "/chat/#{channel.id}", + "/chat/#{channel.id}/thread/#{thread.id}", + ) + end + end + + context "when the chat message is a thread reply" do + fab!(:thread) do + Fabricate( + :chat_thread, + original_message: Fabricate(:chat_message, chat_channel: channel), + channel: channel, + ) + end + + before { message.update!(thread: thread) } + + it "generates the correct targets" do + targets = described_class.calculate_publish_targets(channel, message) + expect(targets).to contain_exactly("/chat/#{channel.id}/thread/#{thread.id}") + end + end + + context "when the chat message is not part of a thread" do + it "generates the correct targets" do + targets = described_class.calculate_publish_targets(channel, message) + expect(targets).to contain_exactly("/chat/#{channel.id}") + end + end + end end diff --git a/plugins/chat/spec/system/chat_message/channel_spec.rb b/plugins/chat/spec/system/chat_message/channel_spec.rb new file mode 100644 index 00000000000..39d9556bd06 --- /dev/null +++ b/plugins/chat/spec/system/chat_message/channel_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +RSpec.describe "Chat message", type: :system, js: true do + fab!(:current_user) { Fabricate(:user) } + fab!(:channel_1) { Fabricate(:chat_channel) } + fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1) } + + let(:cdp) { PageObjects::CDP.new } + let(:chat) { PageObjects::Pages::Chat.new } + let(:channel) { PageObjects::Pages::ChatChannel.new } + + before do + chat_system_bootstrap + channel_1.add(current_user) + sign_in(current_user) + end + + context "when hovering a message" do + it "adds an active class" do + chat.visit_channel(channel_1) + + channel.hover_message(message_1) + + expect(page).to have_css( + ".chat-live-pane[data-id='#{channel_1.id}'] [data-id='#{message_1.id}'] .chat-message.is-active", + ) + end + end + + context "when copying link to a message" do + before { cdp.allow_clipboard } + + it "copies the link to the message" do + chat.visit_channel(channel_1) + + channel.copy_link(message_1) + + expect(cdp.read_clipboard).to include("/chat/c/-/#{channel_1.id}/#{message_1.id}") + end + end +end diff --git a/plugins/chat/spec/system/chat_message/thread_spec.rb b/plugins/chat/spec/system/chat_message/thread_spec.rb new file mode 100644 index 00000000000..aaa8aa1287a --- /dev/null +++ b/plugins/chat/spec/system/chat_message/thread_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +RSpec.describe "Chat message - channel", type: :system, js: true do + fab!(:current_user) { Fabricate(:user) } + fab!(:other_user) { Fabricate(:user) } + fab!(:channel_1) { Fabricate(:chat_channel) } + fab!(:thread_1) do + chat_thread_chain_bootstrap(channel: channel_1, users: [current_user, other_user]) + end + + let(:cdp) { PageObjects::CDP.new } + let(:chat) { PageObjects::Pages::Chat.new } + let(:channel) { PageObjects::Pages::ChatChannel.new } + let(:message_1) { thread_1.chat_messages.first } + + before do + chat_system_bootstrap + channel_1.update!(threading_enabled: true) + channel_1.add(current_user) + channel_1.add(other_user) + SiteSetting.enable_experimental_chat_threaded_discussions = true + sign_in(current_user) + end + + context "when hovering a message" do + it "adds an active class" do + chat.visit_thread(thread_1) + + channel.hover_message(message_1) + + expect(page).to have_css( + ".chat-thread[data-id='#{thread_1.id}'] [data-id='#{message_1.id}'] .chat-message.is-active", + ) + end + end + + context "when copying link to a message" do + before { cdp.allow_clipboard } + + it "copies the link to the thread" do + chat.visit_thread(thread_1) + + channel.copy_link(message_1) + + expect(cdp.read_clipboard).to include("/chat/c/-/#{channel_1.id}/t/#{thread_1.id}") + end + end +end diff --git a/plugins/chat/spec/system/chat_message_spec.rb b/plugins/chat/spec/system/chat_message_spec.rb deleted file mode 100644 index c8cbf2dedcd..00000000000 --- a/plugins/chat/spec/system/chat_message_spec.rb +++ /dev/null @@ -1,26 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe "Chat message", type: :system, js: true do - fab!(:current_user) { Fabricate(:user) } - fab!(:channel_1) { Fabricate(:chat_channel) } - fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1) } - - let(:chat) { PageObjects::Pages::Chat.new } - let(:channel) { PageObjects::Pages::ChatChannel.new } - - before { chat_system_bootstrap } - - context "when hovering a message" do - before do - channel_1.add(current_user) - sign_in(current_user) - end - - it "adds an active class" do - chat.visit_channel(channel_1) - channel.hover_message(message_1) - - expect(page).to have_css("[data-id='#{message_1.id}'] .chat-message.is-active") - end - end -end diff --git a/plugins/chat/spec/system/message_thread_indicator_spec.rb b/plugins/chat/spec/system/message_thread_indicator_spec.rb index 531054addac..8135a1023fa 100644 --- a/plugins/chat/spec/system/message_thread_indicator_spec.rb +++ b/plugins/chat/spec/system/message_thread_indicator_spec.rb @@ -92,5 +92,20 @@ describe "Thread indicator for chat messages", type: :system, js: true do new_thread = message_without_thread.reload.thread expect(page).not_to have_css(channel_page.message_by_id_selector(new_thread.replies.first)) end + + it "increments the indicator when a new reply is sent in the thread" do + chat_page.visit_channel(channel) + expect(channel_page.message_thread_indicator(thread_1.original_message)).to have_css( + ".chat-message-thread-indicator__replies-count", + text: I18n.t("js.chat.thread.replies", count: 3), + ) + channel_page.message_thread_indicator(thread_1.original_message).click + expect(side_panel).to have_open_thread(thread_1) + open_thread.send_message(thread_1.id, "new thread message") + expect(channel_page.message_thread_indicator(thread_1.original_message)).to have_css( + ".chat-message-thread-indicator__replies-count", + text: I18n.t("js.chat.thread.replies", count: 4), + ) + end end end diff --git a/plugins/chat/spec/system/page_objects/chat/chat.rb b/plugins/chat/spec/system/page_objects/chat/chat.rb index 4ed41f580a5..0e80ff632d1 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat.rb @@ -23,6 +23,10 @@ module PageObjects has_no_css?(".chat-skeleton") end + def visit_thread(thread) + visit(thread.url) + end + def visit_channel_settings(channel) visit(channel.url + "/info/settings") end 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 1abdd866436..8d0c8db253b 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat_channel.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat_channel.rb @@ -22,7 +22,7 @@ module PageObjects end def message_by_id_selector(id) - ".chat-message-container[data-id=\"#{id}\"]" + ".chat-live-pane .chat-messages-container .chat-message-container[data-id=\"#{id}\"]" end def message_by_id(id) @@ -71,6 +71,12 @@ module PageObjects find("[data-value='flag']").click end + def copy_link(message) + hover_message(message) + click_more_button + find("[data-value='copyLink']").click + end + def flag_message(message) hover_message(message) click_more_button diff --git a/plugins/chat/spec/system/page_objects/chat/chat_thread.rb b/plugins/chat/spec/system/page_objects/chat/chat_thread.rb index e047edd0c69..e9ac6dd96b9 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat_thread.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat_thread.rb @@ -50,7 +50,7 @@ module PageObjects def has_message?(thread_id, text: nil, id: nil) if text - find(thread_selector_by_id(thread_id)).has_css?(".chat-message-text", text: text) + find(thread_selector_by_id(thread_id)).has_css?(".chat-message-text", text: text, wait: 5) elsif id find(thread_selector_by_id(thread_id)).has_css?( ".chat-message-container[data-id=\"#{id}\"]", diff --git a/plugins/chat/spec/system/single_thread_spec.rb b/plugins/chat/spec/system/single_thread_spec.rb index c5e56cb0b69..561423a0904 100644 --- a/plugins/chat/spec/system/single_thread_spec.rb +++ b/plugins/chat/spec/system/single_thread_spec.rb @@ -89,6 +89,62 @@ describe "Single thread in side panel", type: :system, js: true do expect(open_thread.omu).to have_content(thread.original_message_user.username) end + describe "sending a message" do + it "shows the message in the thread pane and links it to the correct channel" do + chat_page.visit_channel(channel) + channel_page.message_thread_indicator(thread.original_message).click + expect(side_panel).to have_open_thread(thread) + open_thread.send_message(thread.id, "new thread message") + expect(open_thread).to have_message(thread.id, text: "new thread message") + thread_message = thread.replies.last + expect(thread_message.chat_channel_id).to eq(channel.id) + expect(thread_message.thread.channel_id).to eq(channel.id) + end + + it "does not echo the message in the channel pane" do + chat_page.visit_channel(channel) + channel_page.message_thread_indicator(thread.original_message).click + expect(side_panel).to have_open_thread(thread) + open_thread.send_message(thread.id, "new thread message") + expect(open_thread).to have_message(thread.id, text: "new thread message") + thread_message = thread.reload.replies.last + expect(channel_page).not_to have_css(channel_page.message_by_id_selector(thread_message.id)) + end + + it "handles updates from multiple users sending messages in the thread" do + using_session(:tab_1) do + sign_in(current_user) + chat_page.visit_channel(channel) + channel_page.message_thread_indicator(thread.original_message).click + end + + other_user = Fabricate(:user) + chat_system_user_bootstrap(user: other_user, channel: channel) + using_session(:tab_2) do + sign_in(other_user) + chat_page.visit_channel(channel) + channel_page.message_thread_indicator(thread.original_message).click + end + + using_session(:tab_2) do + expect(side_panel).to have_open_thread(thread) + open_thread.send_message(thread.id, "the other user message") + expect(open_thread).to have_message(thread.id, text: "the other user message") + end + + using_session(:tab_1) do + expect(side_panel).to have_open_thread(thread) + expect(open_thread).to have_message(thread.id, text: "the other user message") + open_thread.send_message(thread.id, "this is a test message") + expect(open_thread).to have_message(thread.id, text: "this is a test message") + end + + using_session(:tab_2) do + expect(open_thread).to have_message(thread.id, text: "this is a test message") + end + end + end + context "when using mobile" do it "opens the side panel for a single thread using the indicator", mobile: true do chat_page.visit_channel(channel) diff --git a/plugins/chat/spec/system/transcript_spec.rb b/plugins/chat/spec/system/transcript_spec.rb index 2f53d751f1a..953f570f4ac 100644 --- a/plugins/chat/spec/system/transcript_spec.rb +++ b/plugins/chat/spec/system/transcript_spec.rb @@ -4,6 +4,7 @@ RSpec.describe "Quoting chat message transcripts", type: :system, js: true do fab!(:current_user) { Fabricate(:user) } fab!(:chat_channel_1) { Fabricate(:chat_channel) } + let(:cdp) { PageObjects::CDP.new } let(:chat_page) { PageObjects::Pages::Chat.new } let(:chat_channel_page) { PageObjects::Pages::ChatChannel.new } let(:topic_page) { PageObjects::Pages::Topic.new } @@ -25,30 +26,6 @@ RSpec.describe "Quoting chat message transcripts", type: :system, js: true do end end - def cdp_allow_clipboard_access! - cdp_params = { - origin: page.server_url, - permission: { - name: "clipboard-read", - }, - setting: "granted", - } - page.driver.browser.execute_cdp("Browser.setPermission", **cdp_params) - - cdp_params = { - origin: page.server_url, - permission: { - name: "clipboard-write", - }, - setting: "granted", - } - page.driver.browser.execute_cdp("Browser.setPermission", **cdp_params) - end - - def read_clipboard - page.evaluate_async_script("navigator.clipboard.readText().then(arguments[0])") - end - def click_selection_button(button) selector = case button @@ -70,7 +47,7 @@ RSpec.describe "Quoting chat message transcripts", type: :system, js: true do expect(chat_channel_page).to have_selection_management click_selection_button("copy") expect(page).to have_selector(".chat-copy-success") - clip_text = read_clipboard + clip_text = cdp.read_clipboard expect(clip_text.chomp).to eq(generate_transcript(messages, current_user)) clip_text end @@ -84,7 +61,7 @@ RSpec.describe "Quoting chat message transcripts", type: :system, js: true do end describe "copying quote transcripts with the clipboard" do - before { cdp_allow_clipboard_access! } + before { cdp.allow_clipboard } context "when quoting a single message into a topic" do fab!(:post_1) { Fabricate(:post) } diff --git a/spec/system/page_objects/cdp.rb b/spec/system/page_objects/cdp.rb new file mode 100644 index 00000000000..56bf4922f26 --- /dev/null +++ b/spec/system/page_objects/cdp.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module PageObjects + class CDP + include Capybara::DSL + + def allow_clipboard + cdp_params = { + origin: page.server_url, + permission: { + name: "clipboard-read", + }, + setting: "granted", + } + page.driver.browser.execute_cdp("Browser.setPermission", **cdp_params) + + cdp_params = { + origin: page.server_url, + permission: { + name: "clipboard-write", + }, + setting: "granted", + } + page.driver.browser.execute_cdp("Browser.setPermission", **cdp_params) + end + + def read_clipboard + page.evaluate_async_script("navigator.clipboard.readText().then(arguments[0])") + end + end +end