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