From 187b59d37610e9c0c4d0cd2f5d4d80a07312a582 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Fri, 5 May 2023 08:55:55 +0200 Subject: [PATCH] UX: implements draft threads (#21361) This commit implements all the necessary logic to create thread seamlessly. For this it relies on the same logic used for messages and generates a `staged-id`(using the format: `staged-thread-CHANNEL_ID-MESSAGE_ID` which is used to re-conciliate state client sides once the thread has been persisted on the backend. Part of this change the client side is now always using real thread and channel objects instead of sometimes relying on a flat `threadId` or `channelId`. This PR also brings three UX changes: - thread starts from top - number of buttons on message actions is dependent of the width of the enclosing container - shift + ArrowUp will reply to the last message --- .../app/controllers/chat/chat_controller.rb | 1 + plugins/chat/app/services/chat/publisher.rb | 32 +++-- .../discourse/components/chat-channel.js | 15 ++- .../discourse/components/chat-composer.hbs | 2 +- .../discourse/components/chat-composer.js | 34 +++++- .../components/chat-drawer/thread.hbs | 2 +- .../chat-message-actions-desktop.hbs | 20 +++- .../chat-message-actions-desktop.js | 21 +++- .../chat-message-actions-mobile.hbs | 3 + .../chat-message-in-reply-to-indicator.js | 4 +- .../chat-message-thread-indicator.hbs | 2 +- .../discourse/components/chat-message.hbs | 2 +- .../discourse/components/chat-message.js | 8 +- .../discourse/components/chat-thread.hbs | 6 +- .../discourse/components/chat-thread.js | 108 ++++++----------- .../components/chat/composer/channel.js | 14 --- .../components/chat/composer/thread.js | 30 ++--- .../discourse/helpers/format-chat-date.js | 4 +- .../discourse/lib/chat-message-interactor.js | 12 +- .../discourse/lib/chat-threads-manager.js | 17 ++- .../discourse/models/chat-channel.js | 27 ++++- .../discourse/models/chat-message.js | 54 ++++++--- .../discourse/models/chat-thread.js | 22 +++- .../discourse/routes/chat-channel-thread.js | 51 ++++++-- .../discourse/services/chat-api.js | 6 +- .../services/chat-channel-composer.js | 79 +++++-------- ...chat-channel-pane-subscriptions-manager.js | 8 -- .../discourse/services/chat-channel-pane.js | 4 + .../services/chat-channel-thread-composer.js | 15 ++- ...annel-thread-pane-subscriptions-manager.js | 16 +-- .../services/chat-channel-thread-pane.js | 13 +++ .../discourse/services/chat-composer.js | 52 +++++++++ .../chat-pane-base-subscriptions-manager.js | 51 +++++--- .../services/chat-staged-thread-mapping.js | 34 ++++++ .../discourse/services/chat-state-manager.js | 31 ++--- .../templates/chat-channel-thread.hbs | 3 +- .../stylesheets/common/chat-thread.scss | 3 +- plugins/chat/lib/chat/message_creator.rb | 24 +++- .../components/chat/message_creator_spec.rb | 24 ++++ .../spec/requests/chat_controller_spec.rb | 25 ++++ .../chat/spec/services/chat/publisher_spec.rb | 26 +++++ .../chat/spec/system/archive_channel_spec.rb | 7 +- plugins/chat/spec/system/browse_page_spec.rb | 6 +- .../channel_thread_message_echoing_spec.rb | 4 +- .../spec/system/chat_message/channel_spec.rb | 2 +- .../spec/system/chat_message/thread_spec.rb | 8 +- .../chat/spec/system/deleted_message_spec.rb | 9 +- .../system/message_thread_indicator_spec.rb | 6 +- .../spec/system/page_objects/chat/chat.rb | 1 + .../system/page_objects/chat/chat_channel.rb | 30 +++-- .../page_objects/chat/chat_side_panel.rb | 8 +- .../system/page_objects/chat/chat_thread.rb | 38 ++---- .../page_objects/chat_drawer/chat_drawer.rb | 8 +- .../system/reply_to_message/drawer_spec.rb | 99 ++++++++++++++++ .../system/reply_to_message/full_page_spec.rb | 103 +++++++++++++++++ .../system/reply_to_message/mobile_spec.rb | 109 ++++++++++++++++++ .../system/reply_to_message/smoke_spec.rb | 77 +++++++++++++ .../system/shortcuts/chat_composer_spec.rb | 13 ++- .../chat/spec/system/single_thread_spec.rb | 20 ++-- 59 files changed, 1075 insertions(+), 378 deletions(-) create mode 100644 plugins/chat/assets/javascripts/discourse/services/chat-composer.js create mode 100644 plugins/chat/assets/javascripts/discourse/services/chat-staged-thread-mapping.js create mode 100644 plugins/chat/spec/system/reply_to_message/drawer_spec.rb create mode 100644 plugins/chat/spec/system/reply_to_message/full_page_spec.rb create mode 100644 plugins/chat/spec/system/reply_to_message/mobile_spec.rb create mode 100644 plugins/chat/spec/system/reply_to_message/smoke_spec.rb diff --git a/plugins/chat/app/controllers/chat/chat_controller.rb b/plugins/chat/app/controllers/chat/chat_controller.rb index 0a5ec268eb8..df19c534fa3 100644 --- a/plugins/chat/app/controllers/chat/chat_controller.rb +++ b/plugins/chat/app/controllers/chat/chat_controller.rb @@ -107,6 +107,7 @@ module Chat staged_id: params[:staged_id], upload_ids: params[:upload_ids], thread_id: params[:thread_id], + staged_thread_id: params[:staged_thread_id], ) return render_json_error(chat_message_creator.error) if chat_message_creator.failed? diff --git a/plugins/chat/app/services/chat/publisher.rb b/plugins/chat/app/services/chat/publisher.rb index a14bc288dcd..10ec97971eb 100644 --- a/plugins/chat/app/services/chat/publisher.rb +++ b/plugins/chat/app/services/chat/publisher.rb @@ -14,7 +14,7 @@ module Chat "#{root_message_bus_channel(chat_channel_id)}/thread/#{thread_id}" end - def self.calculate_publish_targets(channel, message) + def self.calculate_publish_targets(channel, message, staged_thread_id: nil) return [root_message_bus_channel(channel.id)] if !allow_publish_to_thread?(channel) if message.thread_om? @@ -22,8 +22,10 @@ module Chat 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)] + elsif staged_thread_id || message.thread_reply? + targets = [thread_message_bus_channel(channel.id, message.thread_id)] + targets << thread_message_bus_channel(channel.id, staged_thread_id) if staged_thread_id + targets else [root_message_bus_channel(channel.id)] end @@ -33,12 +35,16 @@ module Chat SiteSetting.enable_experimental_chat_threaded_discussions && channel.threading_enabled end - def self.publish_new!(chat_channel, chat_message, staged_id) - message_bus_targets = calculate_publish_targets(chat_channel, chat_message) + def self.publish_new!(chat_channel, chat_message, staged_id, staged_thread_id: nil) + message_bus_targets = + calculate_publish_targets(chat_channel, chat_message, staged_thread_id: staged_thread_id) publish_to_targets!( message_bus_targets, chat_channel, - serialize_message_with_type(chat_message, :sent).merge(staged_id: staged_id), + serialize_message_with_type(chat_message, :sent).merge( + staged_id: staged_id, + staged_thread_id: staged_thread_id, + ), ) # NOTE: This means that the read count is only updated in the client @@ -70,8 +76,15 @@ module Chat ) end - def self.publish_thread_created!(chat_channel, chat_message) - publish_to_channel!(chat_channel, serialize_message_with_type(chat_message, :thread_created)) + def self.publish_thread_created!(chat_channel, chat_message, thread_id, staged_thread_id) + publish_to_channel!( + chat_channel, + serialize_message_with_type( + chat_message, + :thread_created, + { thread_id: thread_id, staged_thread_id: staged_thread_id }, + ), + ) end def self.publish_processed!(chat_message) @@ -215,11 +228,12 @@ module Chat end end - def self.serialize_message_with_type(chat_message, type) + def self.serialize_message_with_type(chat_message, type, options = {}) Chat::MessageSerializer .new(chat_message, { scope: anonymous_guardian, root: :chat_message }) .as_json .merge(type: type) + .merge(options) end def self.user_tracking_state_message_bus_channel(user_id) diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-channel.js b/plugins/chat/assets/javascripts/discourse/components/chat-channel.js index d02aae4fe48..64ce8abe0b5 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-channel.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-channel.js @@ -1,5 +1,6 @@ import { capitalize } from "@ember/string"; import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message"; +import ChatThread from "discourse/plugins/chat/discourse/models/chat-thread"; import Component from "@glimmer/component"; import { bind, debounce } from "discourse-common/utils/decorators"; import { action } from "@ember/object"; @@ -352,7 +353,13 @@ export default class ChatLivePane extends Component { messageData.newest = true; } - messages.push(ChatMessage.create(channel, messageData)); + const message = ChatMessage.create(channel, messageData); + + if (messageData.thread_id) { + message.thread = new ChatThread(channel, { id: messageData.thread_id }); + } + + messages.push(message); }); return [messages, results.meta]; @@ -548,7 +555,11 @@ export default class ChatLivePane extends Component { } if (data.chat_message.user.id === this.currentUser.id && data.staged_id) { - const stagedMessage = handleStagedMessage(this.#messagesManager, data); + const stagedMessage = handleStagedMessage( + this.args.channel, + this.#messagesManager, + data + ); if (stagedMessage) { return; } diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-composer.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-composer.hbs index f81add49728..6ef7b22d5bc 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-composer.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-composer.hbs @@ -25,7 +25,7 @@ {{did-update this.didUpdateMessage this.currentMessage}} {{did-update this.didUpdateInReplyTo this.currentMessage.inReplyTo}} {{did-insert this.setupAppEvents}} - {{will-destroy this.teardownAppEvents}} + {{will-destroy this.teardown}} {{will-destroy this.cancelPersistDraft}} >
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-composer.js b/plugins/chat/assets/javascripts/discourse/components/chat-composer.js index d8c9c1d6b04..b0a43a662a3 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-composer.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-composer.js @@ -17,6 +17,8 @@ import I18n from "I18n"; import { translations } from "pretty-text/emoji/data"; import { setupHashtagAutocomplete } from "discourse/lib/hashtag-autocomplete"; import { isEmpty, isPresent } from "@ember/utils"; +import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message"; +import { Promise } from "rsvp"; export default class ChatComposer extends Component { @service capabilities; @@ -39,7 +41,10 @@ export default class ChatComposer extends Component { } get shouldRenderMessageDetails() { - return this.currentMessage?.editing || this.currentMessage?.inReplyTo; + return ( + this.currentMessage?.editing || + (this.context === "channel" && this.currentMessage?.inReplyTo) + ); } get inlineButtons() { @@ -70,6 +75,18 @@ export default class ChatComposer extends Component { ); } + @action + sendMessage(raw) { + const message = ChatMessage.createDraftMessage(this.args.channel, { + user: this.currentUser, + message: raw, + }); + + this.args.onSendMessage(message); + + return Promise.resolve(); + } + @action persistDraft() {} @@ -133,13 +150,14 @@ export default class ChatComposer extends Component { } @action - teardownAppEvents() { + teardown() { this.appEvents.off("chat:modify-selection", this, "modifySelection"); this.appEvents.off( "chat:open-insert-link-modal", this, "openInsertLinkModal" ); + this.pane.sending = false; } @action @@ -221,7 +239,7 @@ export default class ChatComposer extends Component { } reportReplyingPresence() { - if (!this.args.channel) { + if (!this.args.channel || !this.currentMessage) { return; } @@ -305,9 +323,13 @@ export default class ChatComposer extends Component { !this.hasContent && !this.currentMessage.editing ) { - const editableMessage = this.pane?.lastCurrentUserMessage; - if (editableMessage) { - this.composer.editMessage(editableMessage); + if (event.shiftKey) { + this.composer.replyTo(this.pane?.lastMessage); + } else { + const editableMessage = this.pane?.lastCurrentUserMessage; + if (editableMessage) { + this.composer.editMessage(editableMessage); + } } } diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-drawer/thread.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-drawer/thread.hbs index b448a84a32f..4aa95ff443b 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-drawer/thread.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-drawer/thread.hbs @@ -24,7 +24,7 @@ {{did-update this.fetchChannelAndThread @params.threadId}} > {{#if this.chat.activeChannel.activeThread}} - + {{/if}}
{{/if}} \ No newline at end of file diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-desktop.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-desktop.hbs index e33f7a03852..2de4491b18f 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-desktop.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-desktop.hbs @@ -1,13 +1,16 @@ {{#if (and this.site.desktopView this.chat.activeMessage.model.id)}}
- {{#if this.chatStateManager.isFullPageActive}} + {{#if this.shouldRenderFavoriteReactions}} {{#each this.messageInteractor.emojiReactions key="emoji" @@ -50,7 +53,12 @@ /> {{/if}} - {{#if this.messageInteractor.secondaryButtons.length}} + {{#if + (and + this.messageInteractor.message + this.messageInteractor.secondaryButtons.length + ) + }} { @@ -45,6 +54,10 @@ export default class ChatMessageActionsDesktop extends Component { this.context ); + const viewport = messageContainer.closest(".popper-viewport"); + this.size = + viewport.clientWidth < REDUCED_WIDTH_THRESHOLD ? REDUCED : FULL; + if (!messageContainer) { return; } @@ -57,7 +70,7 @@ export default class ChatMessageActionsDesktop extends Component { name: "flip", enabled: true, options: { - boundary: messageContainer.closest(".popper-viewport"), + boundary: viewport, fallbackPlacements: ["bottom-end"], }, }, @@ -73,7 +86,7 @@ export default class ChatMessageActionsDesktop extends Component { } @action - teardownPopper() { + teardown() { this.popper?.destroy(); } } diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-mobile.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-mobile.hbs index c91d4d181f3..b232493a669 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-mobile.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-mobile.hbs @@ -64,6 +64,7 @@ @icon="discourse-emojis" @title="chat.react" @forwardEvent={{true}} + data-id="react" /> {{/if}} @@ -71,6 +72,7 @@ @@ -82,6 +84,7 @@ @action={{action this.actAndCloseMenu "reply"}} @icon="reply" @title="chat.reply" + data-id="reply" /> {{/if}}
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message-in-reply-to-indicator.js b/plugins/chat/assets/javascripts/discourse/components/chat-message-in-reply-to-indicator.js index 4601fbcc37f..75f2a714c41 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message-in-reply-to-indicator.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-message-in-reply-to-indicator.js @@ -16,7 +16,7 @@ export default class ChatMessageInReplyToIndicator extends Component { if (this.hasThread) { return [ ...this.args.message.channel.routeModels, - this.args.message.threadId, + this.args.message.thread.id, ]; } else { return [ @@ -29,7 +29,7 @@ export default class ChatMessageInReplyToIndicator extends Component { get hasThread() { return ( this.args.message?.channel?.threadingEnabled && - this.args.message?.threadId + this.args.message?.thread?.id ); } } diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message-thread-indicator.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-message-thread-indicator.hbs index de986b952ac..2412c1192b4 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message-thread-indicator.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-message-thread-indicator.hbs @@ -1,6 +1,6 @@ diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-message.hbs index 53a5baab681..b122566906b 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-message.hbs @@ -22,7 +22,7 @@ (if @message.highlighted "highlighted") }} data-id={{@message.id}} - data-thread-id={{@message.threadId}} + data-thread-id={{@message.thread.id}} {{chat/track-message (hash didEnterViewport=(fn @messageDidEnterViewport @message) diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message.js b/plugins/chat/assets/javascripts/discourse/components/chat-message.js index 4d86113e891..a399c0db713 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-message.js @@ -282,15 +282,15 @@ export default class ChatMessage extends Component { } get threadingEnabled() { - return this.args.channel?.threadingEnabled && this.args.message?.threadId; + return this.args.channel?.threadingEnabled && !!this.args.message?.thread; } get showThreadIndicator() { return ( this.args.context !== MESSAGE_CONTEXT_THREAD && this.threadingEnabled && - this.args.message?.threadId !== - this.args.message?.previousMessage?.threadId + this.args.message?.thread && + this.args.message?.threadReplyCount > 0 ); } @@ -352,7 +352,7 @@ export default class ChatMessage extends Component { inviteMentioned() { const userIds = this.mentionWarning.without_membership.mapBy("id"); - ajax(`/chat/${this.args.message.channelId}/invite`, { + ajax(`/chat/${this.args.message.channel.id}/invite`, { method: "PUT", data: { user_ids: userIds, chat_message_id: this.args.message.id }, }).then(() => { diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs index e2bae197268..5be4543e540 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs @@ -3,9 +3,9 @@ data-id={{this.thread.id}} {{did-insert this.setUploadDropZone}} {{did-insert this.subscribeToUpdates}} - {{did-insert this.loadMessages}} {{did-update this.subscribeToUpdates this.thread.id}} - {{did-update this.loadMessages this.thread.id}} + {{did-insert this.loadMessages}} + {{did-update this.loadMessages this.thread}} {{did-insert this.setupMessage}} {{will-destroy this.unsubscribeFromUpdates}} > @@ -42,7 +42,7 @@ @context="thread" /> {{/each}} - {{#if (or this.loading this.loadingMoreFuture)}} + {{#if this.loading}} {{/if}}
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-thread.js b/plugins/chat/assets/javascripts/discourse/components/chat-thread.js index baf84060e22..2b5d6e6e74c 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-thread.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-thread.js @@ -8,6 +8,7 @@ import { bind, debounce } from "discourse-common/utils/decorators"; import { inject as service } from "@ember/service"; import { schedule } from "@ember/runloop"; import discourseLater from "discourse-common/lib/later"; +import { resetIdle } from "discourse/lib/desktop-notifications"; const PAGE_SIZE = 50; @@ -25,22 +26,16 @@ export default class ChatThreadPanel extends Component { @service capabilities; @tracked loading; - @tracked loadingMorePast; @tracked uploadDropZone; scrollable = null; get thread() { - return this.channel.activeThread; + return this.args.thread; } get channel() { - return this.chat.activeChannel; - } - - @action - subscribeToUpdates() { - this.chatChannelThreadPaneSubscriptionsManager.subscribe(this.thread); + return this.thread?.channel; } @action @@ -56,6 +51,11 @@ export default class ChatThreadPanel extends Component { ); } + @action + subscribeToUpdates() { + this.chatChannelThreadPaneSubscriptionsManager.subscribe(this.thread); + } + @action unsubscribeFromUpdates() { this.chatChannelThreadPaneSubscriptionsManager.unsubscribe(); @@ -69,17 +69,7 @@ export default class ChatThreadPanel extends Component { @action loadMessages() { this.thread.messagesManager.clearMessages(); - - if (this.args.targetMessageId) { - this.requestedTargetMessageId = parseInt(this.args.targetMessageId, 10); - } - - // TODO (martin) Loading/scrolling to selected message - // this.highlightOrFetchMessage(this.requestedTargetMessageId); - // if (this.requestedTargetMessageId) { - // } else { this.fetchMessages(); - // } } @action @@ -97,21 +87,14 @@ export default class ChatThreadPanel extends Component { return Promise.resolve(); } - this.loadingMorePast = true; + if (this.thread.staged) { + this.thread.messagesManager.addMessages([this.thread.originalMessage]); + return Promise.resolve(); + } + this.loading = true; - const findArgs = { pageSize: PAGE_SIZE }; - - // TODO (martin) Find arguments for last read etc. - // const fetchingFromLastRead = !options.fetchFromLastMessage; - // if (this.requestedTargetMessageId) { - // findArgs["targetMessageId"] = this.requestedTargetMessageId; - // } else if (fetchingFromLastRead) { - // findArgs["targetMessageId"] = this._getLastReadId(); - // } - // - findArgs.threadId = this.thread.id; - + const findArgs = { pageSize: PAGE_SIZE, threadId: this.thread.id }; return this.chatApi .messages(this.channel.id, findArgs) .then((results) => { @@ -123,23 +106,13 @@ export default class ChatThreadPanel extends Component { ); } - const [messages, meta] = this.afterFetchCallback(this.channel, results); + const [messages, meta] = this.afterFetchCallback( + this.channel, + this.thread, + results + ); this.thread.messagesManager.addMessages(messages); - - // TODO (martin) details needed for thread?? this.thread.details = meta; - - // TODO (martin) Scrolling to particular messages - // if (this.requestedTargetMessageId) { - // this.scrollToMessage(findArgs["targetMessageId"], { - // highlight: true, - // }); - // } else if (fetchingFromLastRead) { - // this.scrollToMessage(findArgs["targetMessageId"]); - // } else if (messages.length) { - // this.scrollToMessage(messages.lastObject.id); - // } - // this.markThreadAsRead(); }) .catch(this.#handleErrors) @@ -148,16 +121,12 @@ export default class ChatThreadPanel extends Component { return; } - this.requestedTargetMessageId = null; this.loading = false; - this.loadingMorePast = false; - - // this.fillPaneAttempt(); }); } @bind - afterFetchCallback(channel, results) { + afterFetchCallback(channel, thread, results) { const messages = []; results.chat_messages.forEach((messageData) => { @@ -170,13 +139,10 @@ export default class ChatThreadPanel extends Component { ); } - if (this.requestedTargetMessageId === messageData.id) { - messageData.expanded = !messageData.hidden; - } else { - messageData.expanded = !(messageData.hidden || messageData.deleted_at); - } - - messages.push(ChatMessage.create(channel, messageData)); + messageData.expanded = !(messageData.hidden || messageData.deleted_at); + const message = ChatMessage.create(channel, messageData); + message.thread = thread; + messages.push(message); }); return [messages, results.meta]; @@ -191,6 +157,8 @@ export default class ChatThreadPanel extends Component { @action onSendMessage(message) { + resetIdle(); + if (message.editing) { this.#sendEditMessage(message); } else { @@ -200,7 +168,7 @@ export default class ChatThreadPanel extends Component { @action resetComposer() { - this.chatChannelThreadComposer.reset(this.channel); + this.chatChannelThreadComposer.reset(this.channel, this.thread); } @action @@ -209,34 +177,27 @@ export default class ChatThreadPanel extends Component { } #sendNewMessage(message) { - // TODO (martin) For desktop notifications - // resetIdle() + message.thread = this.thread; + if (this.chatChannelThreadPane.sending) { return; } this.chatChannelThreadPane.sending = true; - // TODO (martin) Handling case when channel is not followed???? IDK if we - // even let people send messages in threads without this, seems weird. - this.thread.stageMessage(message); const stagedMessage = message; this.resetComposer(); this.thread.messagesManager.addMessages([stagedMessage]); - // TODO (martin) Scrolling!! - // if (!this.channel.canLoadMoreFuture) { - // this.scrollToBottom(); - // } - return this.chatApi .sendMessage(this.channel.id, { message: stagedMessage.message, in_reply_to_id: stagedMessage.inReplyTo?.id, staged_id: stagedMessage.id, upload_ids: stagedMessage.uploads.map((upload) => upload.id), - thread_id: stagedMessage.threadId, + thread_id: this.thread.staged ? null : stagedMessage.thread.id, + staged_thread_id: this.thread.staged ? stagedMessage.thread.id : null, }) .then(() => { this.scrollToBottom(); @@ -264,7 +225,7 @@ export default class ChatThreadPanel extends Component { this.resetComposer(); return this.chatApi - .editMessage(message.channelId, message.id, data) + .editMessage(message.channel.id, message.id, data) .catch(popupAjaxError) .finally(() => { this.chatChannelThreadPane.sending = false; @@ -280,9 +241,9 @@ export default class ChatThreadPanel extends Component { return; } - this.scrollable.scrollTop = -1; + this.scrollable.scrollTop = this.scrollable.scrollHeight + 1; this.forceRendering(() => { - this.scrollable.scrollTop = 0; + this.scrollable.scrollTop = this.scrollable.scrollHeight; }); } @@ -319,7 +280,6 @@ export default class ChatThreadPanel extends Component { @action resendStagedMessage() {} - // resendStagedMessage(stagedMessage) {} @action messageDidEnterViewport(message) { diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/composer/channel.js b/plugins/chat/assets/javascripts/discourse/components/chat/composer/channel.js index 1116a0155cb..ab17c22186d 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/composer/channel.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat/composer/channel.js @@ -3,8 +3,6 @@ import { inject as service } from "@ember/service"; import I18n from "I18n"; import discourseDebounce from "discourse-common/lib/debounce"; import { action } from "@ember/object"; -import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message"; -import { Promise } from "rsvp"; export default class ChatComposerChannel extends ChatComposer { @service("chat-channel-composer") composer; @@ -14,18 +12,6 @@ export default class ChatComposerChannel extends ChatComposer { composerId = "channel-composer"; - @action - sendMessage(raw) { - const message = ChatMessage.createDraftMessage(this.args.channel, { - user: this.currentUser, - message: raw, - }); - - this.args.onSendMessage(message); - - return Promise.resolve(); - } - @action persistDraft() { if (this.args.channel?.isDraft) { diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/composer/thread.js b/plugins/chat/assets/javascripts/discourse/components/chat/composer/thread.js index ea46f97046b..e1a1086035e 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/composer/thread.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat/composer/thread.js @@ -1,32 +1,32 @@ import ChatComposer from "../../chat-composer"; import { inject as service } from "@ember/service"; import I18n from "I18n"; -import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message"; -import { Promise } from "rsvp"; import { action } from "@ember/object"; export default class ChatComposerThread extends ChatComposer { @service("chat-channel-thread-composer") composer; + @service("chat-channel-composer") channelComposer; @service("chat-channel-thread-pane") pane; + @service router; context = "thread"; composerId = "thread-composer"; - @action - sendMessage(raw) { - const message = ChatMessage.createDraftMessage(this.args.channel, { - user: this.currentUser, - message: raw, - thread_id: this.args.channel.activeThread.id, - }); - - this.args.onSendMessage(message); - - return Promise.resolve(); - } - get placeholder() { return I18n.t("chat.placeholder_thread"); } + + @action + onKeyDown(event) { + if (event.key === "Escape") { + this.router.transitionTo( + "chat.channel", + ...this.args.channel.routeModels + ); + return; + } + + super.onKeyDown(event); + } } diff --git a/plugins/chat/assets/javascripts/discourse/helpers/format-chat-date.js b/plugins/chat/assets/javascripts/discourse/helpers/format-chat-date.js index cd68b634d5c..0281c252480 100644 --- a/plugins/chat/assets/javascripts/discourse/helpers/format-chat-date.js +++ b/plugins/chat/assets/javascripts/discourse/helpers/format-chat-date.js @@ -17,12 +17,12 @@ registerUnbound("format-chat-date", function (message, mode) { if (message.staged) { return htmlSafe( - `${display}` + `${display}` ); } else { const url = getURL(`/chat/c/-/${message.channel.id}/${message.id}`); return htmlSafe( - `${display}` + `${display}` ); } }); 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 d65a3e532ae..239119787fe 100644 --- a/plugins/chat/assets/javascripts/discourse/lib/chat-message-interactor.js +++ b/plugins/chat/assets/javascripts/discourse/lib/chat-message-interactor.js @@ -229,8 +229,8 @@ export default class ChatMessageInteractor { copyLink() { const { protocol, host } = window.location; - const channelId = this.message.channelId; - const threadId = this.message.threadId; + const channelId = this.message.channel.id; + const threadId = this.message.thread?.id; let url; if (threadId) { @@ -276,7 +276,7 @@ export default class ChatMessageInteractor { return this.chatApi .publishReaction( - this.message.channelId, + this.message.channel.id, this.message.id, emoji, reactAction @@ -329,21 +329,21 @@ export default class ChatMessageInteractor { @action delete() { return this.chatApi - .trashMessage(this.message.channelId, this.message.id) + .trashMessage(this.message.channel.id, this.message.id) .catch(popupAjaxError); } @action restore() { return this.chatApi - .restoreMessage(this.message.channelId, this.message.id) + .restoreMessage(this.message.channel.id, this.message.id) .catch(popupAjaxError); } @action rebake() { return this.chatApi - .rebakeMessage(this.message.channelId, this.message.id) + .rebakeMessage(this.message.channel.id, this.message.id) .catch(popupAjaxError); } 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 a83a34c87c1..fb806415c9b 100644 --- a/plugins/chat/assets/javascripts/discourse/lib/chat-threads-manager.js +++ b/plugins/chat/assets/javascripts/discourse/lib/chat-threads-manager.js @@ -4,7 +4,6 @@ import Promise from "rsvp"; import ChatThread from "discourse/plugins/chat/discourse/models/chat-thread"; import { tracked } from "@glimmer/tracking"; import { TrackedObject } from "@ember-compat/tracked-built-ins"; -import { popupAjaxError } from "discourse/lib/ajax-error"; /* The ChatThreadsManager is responsible for managing the loaded chat threads @@ -39,11 +38,16 @@ export default class ChatThreadsManager { return Object.values(this._cached); } - store(threadObject) { + store(channel, threadObject) { let model = this.#findStale(threadObject.id); if (!model) { - model = new ChatThread(threadObject); + if (threadObject instanceof ChatThread) { + model = threadObject; + } else { + model = new ChatThread(channel, threadObject); + } + this.#cache(model); } @@ -59,12 +63,7 @@ export default class ChatThreadsManager { } async #find(channelId, threadId) { - return this.chatApi - .thread(channelId, threadId) - .catch(popupAjaxError) - .then((thread) => { - return thread; - }); + return this.chatApi.thread(channelId, threadId); } #cache(thread) { diff --git a/plugins/chat/assets/javascripts/discourse/models/chat-channel.js b/plugins/chat/assets/javascripts/discourse/models/chat-channel.js index 8bdc0d26180..c67862f1f4f 100644 --- a/plugins/chat/assets/javascripts/discourse/models/chat-channel.js +++ b/plugins/chat/assets/javascripts/discourse/models/chat-channel.js @@ -9,6 +9,7 @@ import ChatThreadsManager from "discourse/plugins/chat/discourse/lib/chat-thread import ChatMessagesManager from "discourse/plugins/chat/discourse/lib/chat-messages-manager"; import { getOwner } from "discourse-common/lib/get-owner"; import guid from "pretty-text/guid"; +import ChatThread from "discourse/plugins/chat/discourse/models/chat-thread"; export const CHATABLE_TYPES = { directMessageChannel: "DirectMessage", @@ -71,6 +72,10 @@ export default class ChatChannel extends RestModel { return this.messages.findIndex((m) => m.id === message.id); } + findMessage(id) { + return this.messagesManager.findMessage(id); + } + get messages() { return this.messagesManager.messages; } @@ -155,6 +160,23 @@ export default class ChatChannel extends RestModel { this.channelMessageBusLastId = details.channel_message_bus_last_id; } + createStagedThread(message) { + const clonedMessage = message.duplicate(); + + const thread = new ChatThread(this, { + id: `staged-thread-${message.channel.id}-${message.id}`, + original_message: message, + staged: true, + created_at: moment.utc().format(), + }); + + clonedMessage.thread = thread; + this.threadsManager.store(this, thread); + thread.messagesManager.addMessages([clonedMessage]); + + return thread; + } + stageMessage(message) { message.id = guid(); message.staged = true; @@ -163,11 +185,6 @@ export default class ChatChannel extends RestModel { message.cook(); if (message.inReplyTo) { - if (!message.inReplyTo.threadId) { - message.inReplyTo.threadId = guid(); - message.inReplyTo.threadReplyCount = 1; - } - if (!this.threadingEnabled) { this.messagesManager.addMessages([message]); } diff --git a/plugins/chat/assets/javascripts/discourse/models/chat-message.js b/plugins/chat/assets/javascripts/discourse/models/chat-message.js index 5c4355dcf76..23a64dceebd 100644 --- a/plugins/chat/assets/javascripts/discourse/models/chat-message.js +++ b/plugins/chat/assets/javascripts/discourse/models/chat-message.js @@ -31,8 +31,6 @@ export default class ChatMessage { @tracked deletedAt; @tracked uploads; @tracked excerpt; - @tracked threadId; - @tracked threadReplyCount; @tracked reactions; @tracked reviewableId; @tracked user; @@ -50,11 +48,12 @@ export default class ChatMessage { @tracked highlighted = false; @tracked firstOfResults = false; @tracked message; - + @tracked thread; + @tracked threadReplyCount; @tracked _cooked; constructor(channel, args = {}) { - this.channel = channel; + // when modifying constructor, be sure to update duplicate function accordingly this.id = args.id; this.newest = args.newest; this.firstOfResults = args.firstOfResults; @@ -62,23 +61,23 @@ export default class ChatMessage { this.edited = args.edited; this.availableFlags = args.availableFlags || args.available_flags; this.hidden = args.hidden; - this.threadId = args.threadId || args.thread_id; this.threadReplyCount = args.threadReplyCount || args.thread_reply_count; - this.channelId = args.channelId || args.chat_channel_id; this.chatWebhookEvent = args.chatWebhookEvent || args.chat_webhook_event; this.createdAt = args.createdAt || args.created_at; this.deletedAt = args.deletedAt || args.deleted_at; this.excerpt = args.excerpt; this.reviewableId = args.reviewableId || args.reviewable_id; this.userFlagStatus = args.userFlagStatus || args.user_flag_status; + this.draft = args.draft; + this.message = args.message || ""; + this._cooked = args.cooked || ""; + this.thread = args.thread; this.inReplyTo = args.inReplyTo || (args.in_reply_to || args.replyToMsg ? ChatMessage.create(channel, args.in_reply_to || args.replyToMsg) : null); - this.draft = args.draft; - this.message = args.message || ""; - this._cooked = args.cooked || ""; + this.channel = channel; this.reactions = this.#initChatMessageReactionModel( args.id, args.reactions @@ -88,6 +87,39 @@ export default class ChatMessage { this.bookmark = args.bookmark ? Bookmark.create(args.bookmark) : null; } + duplicate() { + // This is important as a message can exist in the context of a channel or a thread + // The current strategy is to have a different message object in each cases to avoid + // side effects + const message = new ChatMessage(this.channel, { + id: this.id, + newest: this.newest, + staged: this.staged, + edited: this.edited, + availableFlags: this.availableFlags, + hidden: this.hidden, + threadReplyCount: this.threadReplyCount, + chatWebhookEvent: this.chatWebhookEvent, + createdAt: this.createdAt, + deletedAt: this.deletedAt, + excerpt: this.excerpt, + reviewableId: this.reviewableId, + userFlagStatus: this.userFlagStatus, + draft: this.draft, + message: this.message, + cooked: this.cooked, + }); + + message.thread = this.thread; + message.reactions = this.reactions; + message.user = this.user; + message.inReplyTo = this.inReplyTo; + message.bookmark = this.bookmark; + message.uploads = this.uploads; + + return message; + } + get cooked() { return this._cooked; } @@ -131,10 +163,6 @@ export default class ChatMessage { } } - get threadRouteModels() { - return [...this.channel.routeModels, this.threadId]; - } - get read() { return this.channel.currentUserMembership?.last_read_message_id >= this.id; } diff --git a/plugins/chat/assets/javascripts/discourse/models/chat-thread.js b/plugins/chat/assets/javascripts/discourse/models/chat-thread.js index e44b4406780..ee4926842cc 100644 --- a/plugins/chat/assets/javascripts/discourse/models/chat-thread.js +++ b/plugins/chat/assets/javascripts/discourse/models/chat-thread.js @@ -4,6 +4,7 @@ import User from "discourse/models/user"; import { escapeExpression } from "discourse/lib/utilities"; import { tracked } from "@glimmer/tracking"; import guid from "pretty-text/guid"; +import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message"; export const THREAD_STATUSES = { open: "open", @@ -13,20 +14,25 @@ export const THREAD_STATUSES = { }; export default class ChatThread { + @tracked id; @tracked title; @tracked status; + @tracked draft; + @tracked staged; + @tracked channel; + @tracked originalMessage; + @tracked threadMessageBusLastId; messagesManager = new ChatMessagesManager(getOwner(this)); - constructor(args = {}) { + constructor(channel, args = {}) { this.title = args.title; this.id = args.id; - this.channelId = args.channel_id; + this.channel = channel; this.status = args.status; - - this.originalMessageUser = this.#initUserModel(args.original_message_user); - this.originalMessage = args.original_message; - this.originalMessage.user = this.originalMessageUser; + this.draft = args.draft; + this.staged = args.staged; + this.originalMessage = ChatMessage.create(channel, args.original_message); } stageMessage(message) { @@ -39,6 +45,10 @@ export default class ChatThread { this.messagesManager.addMessages([message]); } + get routeModels() { + return [...this.channel.routeModels, this.id]; + } + get messages() { return this.messagesManager.messages; } diff --git a/plugins/chat/assets/javascripts/discourse/routes/chat-channel-thread.js b/plugins/chat/assets/javascripts/discourse/routes/chat-channel-thread.js index 04151d5250f..54ce405ee3b 100644 --- a/plugins/chat/assets/javascripts/discourse/routes/chat-channel-thread.js +++ b/plugins/chat/assets/javascripts/discourse/routes/chat-channel-thread.js @@ -5,24 +5,57 @@ export default class ChatChannelThread extends DiscourseRoute { @service router; @service chatStateManager; @service chat; + @service chatStagedThreadMapping; + @service chatChannelThreadPane; - async model(params) { + model(params, transition) { const channel = this.modelFor("chat.channel"); - return channel.threadsManager.find(channel.id, params.threadId); + + return channel.threadsManager + .find(channel.id, params.threadId) + .catch(() => { + transition.abort(); + this.router.transitionTo("chat.channel", ...channel.routeModels); + return; + }); } deactivate() { - this.#closeThread(); + this.chatChannelThreadPane.close(); } - #closeThread() { - this.chat.activeChannel.activeThread?.messagesManager?.clearMessages(); - this.chat.activeChannel.activeThread = null; - this.chatStateManager.closeSidePanel(); + beforeModel(transition) { + const channel = this.modelFor("chat.channel"); + + if (!channel.threadingEnabled) { + transition.abort(); + this.router.transitionTo("chat.channel", ...channel.routeModels); + return; + } + + // This is a very special logic to attempt to reconciliate a staged thread id + // it happens after creating a new thread and having a temp ID in the URL + // if users presses reload at this moment, we would have a 404 + // replacing the ID in the URL sooner would also cause a reload + const params = this.paramsFor("chat.channel.thread"); + const threadId = params.threadId; + + if (threadId?.startsWith("staged-thread-")) { + const mapping = this.chatStagedThreadMapping.getMapping(); + + if (mapping[threadId]) { + transition.abort(); + + this.router.transitionTo( + "chat.channel.thread", + ...[...channel.routeModels, mapping[threadId]] + ); + return; + } + } } afterModel(model) { - this.chat.activeChannel.activeThread = model; - this.chatStateManager.openSidePanel(); + this.chatChannelThreadPane.open(model); } } diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-api.js b/plugins/chat/assets/javascripts/discourse/services/chat-api.js index d28d92663fc..6f15734b4b6 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-api.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-api.js @@ -40,7 +40,11 @@ export default class ChatApi extends Service { */ thread(channelId, threadId) { return this.#getRequest(`/channels/${channelId}/threads/${threadId}`).then( - (result) => this.chat.activeChannel.threadsManager.store(result.thread) + (result) => + this.chat.activeChannel.threadsManager.store( + this.chat.activeChannel, + result.thread + ) ); } 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 6afa1a96a76..58a580bce2c 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-channel-composer.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-channel-composer.js @@ -1,60 +1,41 @@ -import { tracked } from "@glimmer/tracking"; -import Service, { inject as service } from "@ember/service"; +import { inject as service } from "@ember/service"; import { action } from "@ember/object"; -import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message"; +import ChatComposer from "./chat-composer"; +import { next } from "@ember/runloop"; -export default class ChatChannelComposer extends Service { +export default class ChatChannelComposer extends ChatComposer { @service chat; - @service chatApi; - @service chatComposerPresenceManager; - @service currentUser; - - @tracked _message; - - @action - cancel() { - if (this.message.editing) { - this.reset(); - } else if (this.message.inReplyTo) { - this.message.inReplyTo = null; - } - } - - @action - reset(channel) { - this.message = ChatMessage.createDraftMessage(channel, { - user: this.currentUser, - }); - } - - @action - clear() { - this.message.message = ""; - } - - @action - editMessage(message) { - this.chat.activeMessage = null; - message.editing = true; - this.message = message; - } - - @action - onCancelEditing() { - this.reset(); - } + @service chatChannelThreadComposer; + @service router; @action replyTo(message) { this.chat.activeMessage = null; - this.message.inReplyTo = message; + const channel = message.channel; + + if ( + this.siteSettings.enable_experimental_chat_threaded_discussions && + channel.threadingEnabled + ) { + let thread; + if (message.thread?.id) { + thread = message.thread; + } else { + thread = channel.createStagedThread(message); + message.thread = thread; + } + + this.router + .transitionTo("chat.channel.thread", ...thread.routeModels) + .finally(() => this._setReplyToAfterTransition(message)); + } else { + this.message.inReplyTo = message; + } } - get message() { - return this._message; - } - - set message(message) { - this._message = message; + _setReplyToAfterTransition(message) { + next(() => { + this.chatChannelThreadComposer.replyTo(message); + }); } } 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 index 50d73097e65..1d8a7fbe950 100644 --- 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 @@ -20,14 +20,6 @@ export default class ChatChannelPaneSubscriptionsManager extends ChatPaneBaseSub return; } - 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) { diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-channel-pane.js b/plugins/chat/assets/javascripts/discourse/services/chat-channel-pane.js index 86b29e08290..5ed5bee8d1f 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-channel-pane.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-channel-pane.js @@ -55,4 +55,8 @@ export default class ChatChannelPane extends Service { return lastCurrentUserMessage; } + + get lastMessage() { + return this.chat.activeChannel.messages.lastObject; + } } 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 81749d4e725..6de4b84b6f8 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,13 +1,20 @@ -import ChatChannelComposer from "./chat-channel-composer"; +import ChatComposer from "./chat-composer"; import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message"; import { action } from "@ember/object"; -export default class extends ChatChannelComposer { +export default class ChatChannelThreadComposer extends ChatComposer { @action - reset(channel) { + reset(channel, thread) { this.message = ChatMessage.createDraftMessage(channel, { user: this.currentUser, - thread_id: channel.activeThread.id, }); + this.message.thread = thread; + } + + @action + replyTo(message) { + this.chat.activeMessage = null; + this.message.thread = message.thread; + this.message.inReplyTo = message; } } 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 index cd636f31c75..c0ffc6aa17d 100644 --- 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 @@ -3,7 +3,7 @@ import ChatPaneBaseSubscriptionsManager from "./chat-pane-base-subscriptions-man export default class ChatChannelThreadPaneSubscriptionsManager extends ChatPaneBaseSubscriptionsManager { get messageBusChannel() { - return `/chat/${this.model.channelId}/thread/${this.model.id}`; + return `/chat/${this.model.channel.id}/thread/${this.model.id}`; } get messageBusLastId() { @@ -12,7 +12,10 @@ export default class ChatChannelThreadPaneSubscriptionsManager extends ChatPaneB handleSentMessage(data) { if (data.chat_message.user.id === this.currentUser.id && data.staged_id) { - const stagedMessage = this.handleStagedMessageInternal(data); + const stagedMessage = this.handleStagedMessageInternal( + this.model.channel, + data + ); if (stagedMessage) { return; } @@ -23,15 +26,6 @@ export default class ChatChannelThreadPaneSubscriptionsManager extends ChatPaneB data.chat_message ); this.messagesManager.addMessages([message]); - - // TODO (martin) All the scrolling and new message indicator shenanigans, - // as well as handling marking the thread as read. - } - - // 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 diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-pane.js b/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-pane.js index dd39dc6dffc..2e03a27da6c 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-pane.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-pane.js @@ -3,6 +3,19 @@ import { inject as service } from "@ember/service"; export default class ChatChannelThreadPane extends ChatChannelPane { @service chatChannelThreadComposer; + @service chat; + @service chatStateManager; + + close() { + this.chat.activeChannel.activeThread?.messagesManager?.clearMessages(); + this.chat.activeChannel.activeThread = null; + this.chatStateManager.closeSidePanel(); + } + + open(thread) { + this.chat.activeChannel.activeThread = thread; + this.chatStateManager.openSidePanel(); + } get selectedMessageIds() { return this.chat.activeChannel.activeThread.selectedMessages.mapBy("id"); diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-composer.js b/plugins/chat/assets/javascripts/discourse/services/chat-composer.js new file mode 100644 index 00000000000..74f01157f04 --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/services/chat-composer.js @@ -0,0 +1,52 @@ +import { tracked } from "@glimmer/tracking"; +import Service, { inject as service } from "@ember/service"; +import { action } from "@ember/object"; +import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message"; + +export default class ChatComposer extends Service { + @service chat; + @service currentUser; + + @tracked _message; + + @action + cancel() { + if (this.message.editing) { + this.reset(); + } else if (this.message.inReplyTo) { + this.message.inReplyTo = null; + } + } + + @action + reset(channel) { + this.message = ChatMessage.createDraftMessage(channel, { + user: this.currentUser, + }); + } + + @action + clear() { + this.message.message = ""; + } + + @action + editMessage(message) { + this.chat.activeMessage = null; + message.editing = true; + this.message = message; + } + + @action + onCancelEditing() { + this.reset(); + } + + get message() { + return this._message; + } + + set message(message) { + this._message = message; + } +} 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 index 12cff6a7556..180946c26d6 100644 --- 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 @@ -6,7 +6,7 @@ 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) { +export function handleStagedMessage(channel, messagesManager, data) { const stagedMessage = messagesManager.findStagedMessage(data.staged_id); if (!stagedMessage) { @@ -17,17 +17,8 @@ export function handleStagedMessage(messagesManager, data) { 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.channel = channel; 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; - } - stagedMessage.cooked = data.chat_message.cooked; return stagedMessage; @@ -48,6 +39,7 @@ export function handleStagedMessage(messagesManager, data) { export default class ChatPaneBaseSubscriptionsManager extends Service { @service chat; @service currentUser; + @service chatStagedThreadMapping; get messageBusChannel() { throw "not implemented"; @@ -75,14 +67,15 @@ export default class ChatPaneBaseSubscriptionsManager extends Service { 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); + handleStagedMessageInternal(channel, data) { + return handleStagedMessage(channel, this.messagesManager, data); } @bind @@ -122,7 +115,7 @@ export default class ChatPaneBaseSubscriptionsManager extends Service { this.handleFlaggedMessage(busData); break; case "thread_created": - this.handleThreadCreated(busData); + this.handleNewThreadCreated(busData); break; case "update_thread_original_message": this.handleThreadOriginalMessageUpdate(busData); @@ -223,8 +216,34 @@ export default class ChatPaneBaseSubscriptionsManager extends Service { } } - handleThreadCreated() { - throw "not implemented"; + handleNewThreadCreated(data) { + this.model.threadsManager + .find(this.model.id, data.staged_thread_id, { fetchIfNotFound: false }) + .then((stagedThread) => { + if (stagedThread) { + this.chatStagedThreadMapping.setMapping( + data.thread_id, + stagedThread.id + ); + stagedThread.staged = false; + stagedThread.id = data.thread_id; + stagedThread.originalMessage.thread = stagedThread; + stagedThread.originalMessage.threadReplyCount ??= 1; + } else if (data.thread_id) { + this.model.threadsManager + .find(this.model.id, data.thread_id, { fetchIfNotFound: true }) + .then((thread) => { + const channelOriginalMessage = + this.model.messagesManager.findMessage( + thread.originalMessage.id + ); + + if (channelOriginalMessage) { + channelOriginalMessage.thread = thread; + } + }); + } + }); } handleThreadOriginalMessageUpdate() { diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-staged-thread-mapping.js b/plugins/chat/assets/javascripts/discourse/services/chat-staged-thread-mapping.js new file mode 100644 index 00000000000..747e92e3df0 --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/services/chat-staged-thread-mapping.js @@ -0,0 +1,34 @@ +import KeyValueStore from "discourse/lib/key-value-store"; +import Service from "@ember/service"; + +export default class ChatStagedThreadMapping extends Service { + STORE_NAMESPACE = "discourse_chat_"; + KEY = "staged_thread"; + + store = new KeyValueStore(this.STORE_NAMESPACE); + + constructor() { + super(...arguments); + + if (!this.store.getObject(this.USER_EMOJIS_STORE_KEY)) { + this.storedFavorites = []; + } + } + + getMapping() { + return JSON.parse(this.store.getObject(this.KEY) || "{}"); + } + + setMapping(id, stagedId) { + const mapping = {}; + mapping[stagedId] = id; + this.store.setObject({ + key: this.KEY, + value: JSON.stringify(mapping), + }); + } + + reset() { + this.store.setObject({ key: this.KEY, value: "{}" }); + } +} diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-state-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-state-manager.js index 6dd753c1692..2aee412c77b 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-state-manager.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-state-manager.js @@ -21,9 +21,10 @@ export function resetChatDrawerStateCallbacks() { export default class ChatStateManager extends Service { @service chat; @service router; - isDrawerExpanded = false; - isDrawerActive = false; - isSidePanelExpanded = false; + + @tracked isSidePanelExpanded = false; + @tracked isDrawerExpanded = false; + @tracked isDrawerActive = false; @tracked _chatURL = null; @tracked _appURL = null; @@ -44,16 +45,16 @@ export default class ChatStateManager extends Service { } openSidePanel() { - this.set("isSidePanelExpanded", true); + this.isSidePanelExpanded = true; } closeSidePanel() { - this.set("isSidePanelExpanded", false); + this.isSidePanelExpanded = false; } didOpenDrawer(url = null) { - this.set("isDrawerActive", true); - this.set("isDrawerExpanded", true); + this.isDrawerActive = true; + this.isDrawerExpanded = true; if (url) { this.storeChatURL(url); @@ -64,27 +65,27 @@ export default class ChatStateManager extends Service { } didCloseDrawer() { - this.set("isDrawerActive", false); - this.set("isDrawerExpanded", false); + this.isDrawerActive = false; + this.isDrawerExpanded = false; this.chat.updatePresence(); this.#publishStateChange(); } didExpandDrawer() { - this.set("isDrawerActive", true); - this.set("isDrawerExpanded", true); + this.isDrawerActive = true; + this.isDrawerExpanded = true; this.chat.updatePresence(); } didCollapseDrawer() { - this.set("isDrawerActive", true); - this.set("isDrawerExpanded", false); + this.isDrawerActive = true; + this.isDrawerExpanded = false; this.#publishStateChange(); } didToggleDrawer() { - this.set("isDrawerExpanded", !this.isDrawerExpanded); - this.set("isDrawerActive", true); + this.isDrawerExpanded = !this.isDrawerExpanded; + this.isDrawerActive = true; this.#publishStateChange(); } diff --git a/plugins/chat/assets/javascripts/discourse/templates/chat-channel-thread.hbs b/plugins/chat/assets/javascripts/discourse/templates/chat-channel-thread.hbs index 6a0689fc057..b68ebf99aa6 100644 --- a/plugins/chat/assets/javascripts/discourse/templates/chat-channel-thread.hbs +++ b/plugins/chat/assets/javascripts/discourse/templates/chat-channel-thread.hbs @@ -1,2 +1 @@ -{{! ChatThreadList will go here later }} - \ No newline at end of file + \ No newline at end of file diff --git a/plugins/chat/assets/stylesheets/common/chat-thread.scss b/plugins/chat/assets/stylesheets/common/chat-thread.scss index a3dbdcf780d..d254fae9b73 100644 --- a/plugins/chat/assets/stylesheets/common/chat-thread.scss +++ b/plugins/chat/assets/stylesheets/common/chat-thread.scss @@ -25,8 +25,7 @@ flex-grow: 1; overscroll-behavior: contain; display: flex; - flex-direction: column-reverse; - will-change: transform; + flex-direction: column; } .chat-composer__wrapper { diff --git a/plugins/chat/lib/chat/message_creator.rb b/plugins/chat/lib/chat/message_creator.rb index 9a608bd51e4..7acc4aceff9 100644 --- a/plugins/chat/lib/chat/message_creator.rb +++ b/plugins/chat/lib/chat/message_creator.rb @@ -13,6 +13,7 @@ module Chat chat_channel:, in_reply_to_id: nil, thread_id: nil, + staged_thread_id: nil, user:, content:, staged_id: nil, @@ -31,6 +32,7 @@ module Chat @incoming_chat_webhook = incoming_chat_webhook @upload_ids = upload_ids || [] @thread_id = thread_id + @staged_thread_id = staged_thread_id @error = nil @chat_message = @@ -57,7 +59,12 @@ module Chat create_thread @chat_message.attach_uploads(uploads) Chat::Draft.where(user_id: @user.id, chat_channel_id: @chat_channel.id).destroy_all - Chat::Publisher.publish_new!(@chat_channel, @chat_message, @staged_id) + Chat::Publisher.publish_new!( + @chat_channel, + @chat_message, + @staged_id, + staged_thread_id: @staged_thread_id, + ) resolved_thread&.increment_replies_count_cache Jobs.enqueue(Jobs::Chat::ProcessMessage, { chat_message_id: @chat_message.id }) Chat::Notifier.notify_new(chat_message: @chat_message, timestamp: @chat_message.created_at) @@ -123,6 +130,8 @@ module Chat end def validate_existing_thread! + return if @staged_thread_id.present? && @thread_id.blank? + return if @thread_id.blank? @existing_thread = Chat::Thread.find(@thread_id) @@ -165,7 +174,7 @@ module Chat def create_thread return if @in_reply_to_id.blank? - return if @chat_message.in_thread? + return if @chat_message.in_thread? && !@staged_thread_id.present? if @original_message.thread thread = @original_message.thread @@ -177,12 +186,15 @@ module Chat channel: @chat_message.chat_channel, ) @chat_message.in_reply_to.thread_id = thread.id - Chat::Publisher.publish_thread_created!( - @chat_message.chat_channel, - @chat_message.in_reply_to, - ) end + Chat::Publisher.publish_thread_created!( + @chat_message.chat_channel, + @chat_message.in_reply_to, + thread.id, + @staged_thread_id, + ) + @chat_message.thread_id = thread.id # NOTE: We intentionally do not try to correct thread IDs within the chain diff --git a/plugins/chat/spec/components/chat/message_creator_spec.rb b/plugins/chat/spec/components/chat/message_creator_spec.rb index a94399320dc..f1d390ea27d 100644 --- a/plugins/chat/spec/components/chat/message_creator_spec.rb +++ b/plugins/chat/spec/components/chat/message_creator_spec.rb @@ -451,6 +451,30 @@ describe Chat::MessageCreator do expect(thread_created_message.channel).to eq("/chat/#{public_chat_channel.id}") end + context "when a staged_thread_id is provided" do + fab!(:existing_thread) { Fabricate(:chat_thread, channel: public_chat_channel) } + + it "creates a thread and publishes with the staged id" do + messages = + MessageBus.track_publish do + described_class.create( + chat_channel: public_chat_channel, + user: user1, + content: "this is a message", + in_reply_to_id: reply_message.id, + staged_thread_id: "stagedthreadid", + ).chat_message + end + + thread_event = messages.find { |m| m.data["type"] == "thread_created" } + expect(thread_event.data["staged_thread_id"]).to eq("stagedthreadid") + expect(Chat::Thread.find(thread_event.data["thread_id"])).to be_persisted + + send_event = messages.find { |m| m.data["type"] == "sent" } + expect(send_event.data["staged_thread_id"]).to eq("stagedthreadid") + end + end + context "when the thread_id is provided" do fab!(:existing_thread) { Fabricate(:chat_thread, channel: public_chat_channel) } diff --git a/plugins/chat/spec/requests/chat_controller_spec.rb b/plugins/chat/spec/requests/chat_controller_spec.rb index 573474f8a5d..ce50fb698db 100644 --- a/plugins/chat/spec/requests/chat_controller_spec.rb +++ b/plugins/chat/spec/requests/chat_controller_spec.rb @@ -385,6 +385,31 @@ RSpec.describe Chat::ChatController do expect(messages.first.data["last_read_message_id"]).to eq(Chat::Message.last.id) end + context "when sending a message in a staged thread" do + it "creates the thread and publishes with the staged id" do + sign_in(user) + + messages = + MessageBus.track_publish do + post "/chat/#{chat_channel.id}.json", + params: { + message: message, + in_reply_to_id: message_1.id, + staged_thread_id: "stagedthreadid", + } + end + + expect(response.status).to eq(200) + + thread_event = messages.find { |m| m.data["type"] == "thread_created" } + expect(thread_event.data["staged_thread_id"]).to eq("stagedthreadid") + expect(Chat::Thread.find(thread_event.data["thread_id"])).to be_persisted + + sent_event = messages.find { |m| m.data["type"] == "sent" } + expect(sent_event.data["staged_thread_id"]).to eq("stagedthreadid") + end + end + context "when sending a message in a thread" do fab!(:thread) do Fabricate(:chat_thread, channel: chat_channel, original_message: message_1) diff --git a/plugins/chat/spec/services/chat/publisher_spec.rb b/plugins/chat/spec/services/chat/publisher_spec.rb index 78e3e3d15f3..6a70614974c 100644 --- a/plugins/chat/spec/services/chat/publisher_spec.rb +++ b/plugins/chat/spec/services/chat/publisher_spec.rb @@ -111,6 +111,32 @@ describe Chat::Publisher do end end + context "when a staged thread has been provided" 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, + staged_thread_id: "stagedthreadid", + ) + + expect(targets).to contain_exactly( + "/chat/#{channel.id}/thread/#{thread.id}", + "/chat/#{channel.id}/thread/stagedthreadid", + ) + end + end + context "when the message is a thread reply" do fab!(:thread) do Fabricate( diff --git a/plugins/chat/spec/system/archive_channel_spec.rb b/plugins/chat/spec/system/archive_channel_spec.rb index 292a0595e6d..63228a646b3 100644 --- a/plugins/chat/spec/system/archive_channel_spec.rb +++ b/plugins/chat/spec/system/archive_channel_spec.rb @@ -61,11 +61,12 @@ RSpec.describe "Archive channel", type: :system, js: true do find("#split-topic-name").fill_in(with: "An interesting topic for cats") click_button(I18n.t("js.chat.channel_archive.title")) - expect(page).to have_content(I18n.t("js.chat.channel_archive.process_started")) - expect(page).to have_css(".chat-channel-archive-status") + expect(page).to have_css(".chat-channel-archive-status", wait: 15) end it "shows an error when the topic is invalid" do + Jobs.run_immediately! + chat.visit_channel_settings(channel_1) click_button(I18n.t("js.chat.channel_settings.archive_channel")) find("#split-topic-name").fill_in( @@ -73,7 +74,7 @@ RSpec.describe "Archive channel", type: :system, js: true do ) click_button(I18n.t("js.chat.channel_archive.title")) - expect(page).not_to have_content(I18n.t("js.chat.channel_archive.process_started")) + expect(page).to have_no_content(I18n.t("js.chat.channel_archive.process_started")) expect(page).to have_content("Title can't have more than 1 emoji") end diff --git a/plugins/chat/spec/system/browse_page_spec.rb b/plugins/chat/spec/system/browse_page_spec.rb index 31392066c38..3cd3cfc7ee0 100644 --- a/plugins/chat/spec/system/browse_page_spec.rb +++ b/plugins/chat/spec/system/browse_page_spec.rb @@ -79,7 +79,7 @@ RSpec.describe "Browse page", type: :system, js: true do context "when results are found" do it "lists expected results" do visit("/chat/browse") - find(".dc-filter-input").fill_in(with: category_channel_1.name) + find(".chat-browse-view .dc-filter-input").fill_in(with: category_channel_1.name) expect(browse_view).to have_content(category_channel_1.name) expect(browse_view).to have_no_content(category_channel_2.name) @@ -89,14 +89,14 @@ RSpec.describe "Browse page", type: :system, js: true do context "when results are not found" do it "displays the correct message" do visit("/chat/browse") - find(".dc-filter-input").fill_in(with: "x") + find(".chat-browse-view .dc-filter-input").fill_in(with: "x") expect(browse_view).to have_content(I18n.t("js.chat.empty_state.title")) end it "doesn’t display any channel" do visit("/chat/browse") - find(".dc-filter-input").fill_in(with: "x") + find(".chat-browse-view .dc-filter-input").fill_in(with: "x") expect(browse_view).to have_no_content(category_channel_1.name) expect(browse_view).to have_no_content(category_channel_2.name) diff --git a/plugins/chat/spec/system/channel_thread_message_echoing_spec.rb b/plugins/chat/spec/system/channel_thread_message_echoing_spec.rb index 1e3f4274045..035ed2abdd5 100644 --- a/plugins/chat/spec/system/channel_thread_message_echoing_spec.rb +++ b/plugins/chat/spec/system/channel_thread_message_echoing_spec.rb @@ -69,8 +69,8 @@ describe "Channel thread message echoing", type: :system, js: true 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") + open_thread.send_message("new thread message") + expect(open_thread).to have_message(thread_id: thread.id, text: "new thread message") new_message = thread.reload.replies.last expect(channel_page).not_to have_css(channel_page.message_by_id_selector(new_message.id)) end diff --git a/plugins/chat/spec/system/chat_message/channel_spec.rb b/plugins/chat/spec/system/chat_message/channel_spec.rb index a91c6b94511..3027f515eff 100644 --- a/plugins/chat/spec/system/chat_message/channel_spec.rb +++ b/plugins/chat/spec/system/chat_message/channel_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.describe "Chat message", type: :system, js: true do +RSpec.describe "Chat message - channel", 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) } diff --git a/plugins/chat/spec/system/chat_message/thread_spec.rb b/plugins/chat/spec/system/chat_message/thread_spec.rb index 187a6aef2cc..15f6e639308 100644 --- a/plugins/chat/spec/system/chat_message/thread_spec.rb +++ b/plugins/chat/spec/system/chat_message/thread_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.describe "Chat message - channel", type: :system, js: true do +RSpec.describe "Chat message - thread", type: :system, js: true do fab!(:current_user) { Fabricate(:user) } fab!(:other_user) { Fabricate(:user) } fab!(:channel_1) { Fabricate(:chat_channel) } @@ -25,13 +25,13 @@ RSpec.describe "Chat message - channel", type: :system, js: true do context "when hovering a message" do it "adds an active class" do - last_message = thread_1.chat_messages.last + first_message = thread_1.chat_messages.first chat.visit_thread(thread_1) - thread.hover_message(last_message) + thread.hover_message(first_message) expect(page).to have_css( - ".chat-thread[data-id='#{thread_1.id}'] [data-id='#{last_message.id}'] .chat-message.is-active", + ".chat-thread[data-id='#{thread_1.id}'] [data-id='#{first_message.id}'] .chat-message.is-active", ) end end diff --git a/plugins/chat/spec/system/deleted_message_spec.rb b/plugins/chat/spec/system/deleted_message_spec.rb index 384c64d2c1a..918a1939e32 100644 --- a/plugins/chat/spec/system/deleted_message_spec.rb +++ b/plugins/chat/spec/system/deleted_message_spec.rb @@ -46,6 +46,7 @@ RSpec.describe "Deleted message", type: :system, js: true do channel_1.update!(threading_enabled: true) SiteSetting.enable_experimental_chat_threaded_discussions = true chat_system_user_bootstrap(user: other_user, channel: channel_1) + Chat::Thread.update_counts end it "hides the deleted messages" do @@ -55,8 +56,8 @@ RSpec.describe "Deleted message", type: :system, js: true do expect(channel_page).to have_message(id: message_1.id) expect(channel_page).to have_message(id: message_2.id) - expect(open_thread).to have_message(thread.id, id: message_4.id) - expect(open_thread).to have_message(thread.id, id: message_5.id) + expect(open_thread).to have_message(thread_id: thread.id, id: message_4.id) + expect(open_thread).to have_message(thread_id: thread.id, id: message_5.id) Chat::Publisher.publish_bulk_delete!( channel_1, @@ -65,8 +66,8 @@ RSpec.describe "Deleted message", type: :system, js: true do expect(channel_page).to have_no_message(id: message_1.id) expect(channel_page).to have_no_message(id: message_2.id) - expect(open_thread).to have_no_message(thread.id, id: message_4.id) - expect(open_thread).to have_no_message(thread.id, id: message_5.id) + expect(open_thread).to have_no_message(thread_id: thread.id, id: message_4.id) + expect(open_thread).to have_no_message(thread_id: thread.id, id: message_5.id) 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 ca73c23c0e7..92ae18a221f 100644 --- a/plugins/chat/spec/system/message_thread_indicator_spec.rb +++ b/plugins/chat/spec/system/message_thread_indicator_spec.rb @@ -86,8 +86,8 @@ describe "Thread indicator for chat messages", type: :system, js: true do message_without_thread = Fabricate(:chat_message, chat_channel: channel, user: other_user) chat_page.visit_channel(channel) channel_page.reply_to(message_without_thread) - channel_page.fill_composer("this is a reply to make a new thread") - channel_page.click_send_message + open_thread.fill_composer("this is a reply to make a new thread") + open_thread.click_send_message expect(channel_page).to have_thread_indicator(message_without_thread) @@ -108,7 +108,7 @@ describe "Thread indicator for chat messages", type: :system, js: true do ) 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") + open_thread.send_message("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), diff --git a/plugins/chat/spec/system/page_objects/chat/chat.rb b/plugins/chat/spec/system/page_objects/chat/chat.rb index 8fc07f6479b..a3bc3a90c58 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat.rb @@ -25,6 +25,7 @@ module PageObjects def visit_thread(thread) visit(thread.url) + has_no_css?(".chat-skeleton") end def visit_channel_settings(channel) 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 395c4b2c955..7daa59c4672 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat_channel.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat_channel.rb @@ -3,6 +3,10 @@ module PageObjects module Pages class ChatChannel < PageObjects::Pages::Base + def replying_to?(message) + find(".chat-channel .chat-reply", text: message.message) + end + def type_in_composer(input) find(".chat-channel .chat-composer__input").click # makes helper more reliable by ensuring focus is not lost find(".chat-channel .chat-composer__input").send_keys(input) @@ -49,7 +53,7 @@ module PageObjects def click_message_action_mobile(message, message_action) expand_message_actions_mobile(message, delay: 0.5) wait_for_animation(find(".chat-message-actions"), timeout: 5) - find(".chat-message-action-item[data-id=\"#{message_action}\"] button").click + find(".chat-message-actions [data-id=\"#{message_action}\"]").click end def hover_message(message) @@ -114,8 +118,12 @@ module PageObjects end def reply_to(message) - hover_message(message) - find(".reply-btn").click + if page.has_css?("html.mobile-view", wait: 0) + click_message_action_mobile(message, "reply") + else + hover_message(message) + find(".reply-btn").click + end end def has_bookmarked_message?(message) @@ -167,18 +175,22 @@ module PageObjects def check_message_presence(exists: true, text: nil, id: nil) css_method = exists ? :has_css? : :has_no_css? if text - send(css_method, ".chat-message-text", text: text, wait: 5) + find(".chat-channel").send(css_method, ".chat-message-text", text: text, wait: 5) elsif id - send(css_method, ".chat-message-container[data-id=\"#{id}\"]", wait: 10) + find(".chat-channel").send( + css_method, + ".chat-message-container[data-id=\"#{id}\"]", + wait: 10, + ) end end - def has_thread_indicator?(message) - has_css?(message_thread_indicator_selector(message)) + def has_thread_indicator?(message, text: nil) + has_css?(message_thread_indicator_selector(message), text: text) end - def has_no_thread_indicator?(message) - has_no_css?(message_thread_indicator_selector(message)) + def has_no_thread_indicator?(message, text: nil) + has_no_css?(message_thread_indicator_selector(message), text: text) end def message_thread_indicator(message) diff --git a/plugins/chat/spec/system/page_objects/chat/chat_side_panel.rb b/plugins/chat/spec/system/page_objects/chat/chat_side_panel.rb index fa6c15b6859..78d932244e3 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat_side_panel.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat_side_panel.rb @@ -3,8 +3,12 @@ module PageObjects module Pages class ChatSidePanel < PageObjects::Pages::Base - def has_open_thread?(thread) - has_css?(".chat-side-panel .chat-thread[data-id='#{thread.id}']") + def has_open_thread?(thread = nil) + if thread + has_css?(".chat-side-panel .chat-thread[data-id='#{thread.id}']") + else + has_css?(".chat-side-panel .chat-thread") + end end def has_no_open_thread? 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 ed6016b9f42..2ef08a9fa35 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat_thread.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat_thread.rb @@ -19,10 +19,6 @@ module PageObjects header.has_content?(content) end - def thread_selector_by_id(id) - ".chat-thread[data-id=\"#{id}\"]" - end - def has_no_loading_skeleton? has_no_css?(".chat-thread__messages .chat-skeleton") end @@ -41,42 +37,32 @@ module PageObjects find(".chat-thread .chat-composer__input").click # ensures autocomplete is closed and not masking anything end - def send_message(id, text = nil) + def send_message(text = nil) text = text.chomp if text.present? # having \n on the end of the string counts as an Enter keypress fill_composer(text) - click_send_message(id) + click_send_message click_composer end - def click_send_message(id) - find(thread_selector_by_id(id)).find( - ".chat-composer.is-send-enabled .chat-composer__send-btn", - ).click + def click_send_message + find(".chat-thread .chat-composer.is-send-enabled .chat-composer__send-btn").click end - def has_message?(thread_id, text: nil, id: nil) - check_message_presence(thread_id, exists: true, text: text, id: id) + def has_message?(text: nil, id: nil, thread_id: nil) + check_message_presence(exists: true, text: text, id: id, thread_id: thread_id) end - def has_no_message?(thread_id, text: nil, id: nil) - check_message_presence(thread_id, exists: false, text: text, id: id) + def has_no_message?(text: nil, id: nil, thread_id: nil) + check_message_presence(exists: false, text: text, id: id, thread_id: thread_id) end - def check_message_presence(thread_id, exists: true, text: nil, id: nil) + def check_message_presence(exists: true, text: nil, id: nil, thread_id: nil) css_method = exists ? :has_css? : :has_no_css? + selector = thread_id ? ".chat-thread[data-id=\"#{thread_id}\"]" : ".chat-thread" if text - find(thread_selector_by_id(thread_id)).send( - css_method, - ".chat-message-text", - text: text, - wait: 5, - ) + find(selector).send(css_method, ".chat-message-text", text: text, wait: 5) elsif id - find(thread_selector_by_id(thread_id)).send( - css_method, - ".chat-message-container[data-id=\"#{id}\"]", - wait: 10, - ) + find(selector).send(css_method, ".chat-message-container[data-id=\"#{id}\"]", wait: 10) end end diff --git a/plugins/chat/spec/system/page_objects/chat_drawer/chat_drawer.rb b/plugins/chat/spec/system/page_objects/chat_drawer/chat_drawer.rb index a6befde3676..8d07c4dd16f 100644 --- a/plugins/chat/spec/system/page_objects/chat_drawer/chat_drawer.rb +++ b/plugins/chat/spec/system/page_objects/chat_drawer/chat_drawer.rb @@ -31,8 +31,12 @@ module PageObjects find("#{VISIBLE_DRAWER} .chat-drawer-header__full-screen-btn").click end - def has_open_thread?(thread) - has_css?("#{VISIBLE_DRAWER} .chat-thread[data-id='#{thread.id}']") + def has_open_thread?(thread = nil) + if thread + has_css?("#{VISIBLE_DRAWER} .chat-thread[data-id='#{thread.id}']") + else + has_css?("#{VISIBLE_DRAWER} .chat-thread") + end end def has_open_channel?(channel) diff --git a/plugins/chat/spec/system/reply_to_message/drawer_spec.rb b/plugins/chat/spec/system/reply_to_message/drawer_spec.rb new file mode 100644 index 00000000000..8678063e512 --- /dev/null +++ b/plugins/chat/spec/system/reply_to_message/drawer_spec.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +RSpec.describe "Reply to message - channel - drawer", type: :system, js: true do + let(:chat_page) { PageObjects::Pages::Chat.new } + let(:channel_page) { PageObjects::Pages::ChatChannel.new } + let(:thread_page) { PageObjects::Pages::ChatThread.new } + let(:drawer_page) { PageObjects::Pages::ChatDrawer.new } + + fab!(:current_user) { Fabricate(:user) } + fab!(:channel_1) { Fabricate(:category_channel) } + fab!(:original_message) do + Fabricate(:chat_message, chat_channel: channel_1, user: Fabricate(:user)) + end + + before do + SiteSetting.enable_experimental_chat_threaded_discussions = true + chat_system_bootstrap + channel_1.update!(threading_enabled: true) + channel_1.add(current_user) + sign_in(current_user) + end + + context "when the message has not current thread" do + it "starts a thread" do + visit("/") + chat_page.open_from_header + drawer_page.open_channel(channel_1) + channel_page.reply_to(original_message) + expect(drawer_page).to have_open_thread + + thread_page.fill_composer("reply to message") + thread_page.click_send_message + + expect(thread_page).to have_message(text: "reply to message") + + drawer_page.back + + expect(channel_page).to have_thread_indicator(original_message) + end + end + + context "when the message has an existing thread" do + fab!(:message_1) do + creator = + Chat::MessageCreator.new( + chat_channel: channel_1, + in_reply_to_id: original_message.id, + user: Fabricate(:user), + content: Faker::Lorem.paragraph, + ) + creator.create + creator.chat_message + end + + it "replies to the existing thread" do + visit("/") + chat_page.open_from_header + drawer_page.open_channel(channel_1) + + expect(channel_page).to have_thread_indicator(original_message, text: "1") + + channel_page.reply_to(original_message) + + expect(drawer_page).to have_open_thread + + thread_page.fill_composer("reply to message") + thread_page.click_send_message + + expect(thread_page).to have_message(text: message_1.message) + expect(thread_page).to have_message(text: "reply to message") + + drawer_page.back + + expect(channel_page).to have_thread_indicator(original_message, text: "2") + expect(channel_page).to have_no_message(text: "reply to message") + end + end + + context "with threading disabled" do + before { channel_1.update!(threading_enabled: false) } + + it "makes a reply in the channel" do + visit("/") + chat_page.open_from_header + drawer_page.open_channel(channel_1) + channel_page.reply_to(original_message) + + expect(page).to have_selector( + ".chat-channel .chat-reply__excerpt", + text: original_message.message, + ) + + channel_page.fill_composer("reply to message") + channel_page.click_send_message + + expect(channel_page).to have_message(text: "reply to message") + end + end +end diff --git a/plugins/chat/spec/system/reply_to_message/full_page_spec.rb b/plugins/chat/spec/system/reply_to_message/full_page_spec.rb new file mode 100644 index 00000000000..1ec1101be0a --- /dev/null +++ b/plugins/chat/spec/system/reply_to_message/full_page_spec.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +RSpec.describe "Reply to message - channel - full page", type: :system, js: true do + let(:chat_page) { PageObjects::Pages::Chat.new } + let(:channel_page) { PageObjects::Pages::ChatChannel.new } + let(:thread_page) { PageObjects::Pages::ChatThread.new } + let(:side_panel_page) { PageObjects::Pages::ChatSidePanel.new } + + fab!(:current_user) { Fabricate(:user) } + fab!(:channel_1) { Fabricate(:category_channel) } + fab!(:original_message) do + Fabricate(:chat_message, chat_channel: channel_1, user: Fabricate(:user)) + end + + before do + SiteSetting.enable_experimental_chat_threaded_discussions = true + chat_system_bootstrap + channel_1.add(current_user) + sign_in(current_user) + channel_1.update!(threading_enabled: true) + end + + context "when the message has not current thread" do + it "starts a thread" do + chat_page.visit_channel(channel_1) + channel_page.reply_to(original_message) + + expect(side_panel_page).to have_open_thread + + thread_page.fill_composer("reply to message") + thread_page.click_send_message + + expect(thread_page).to have_message(text: "reply to message") + expect(channel_page).to have_thread_indicator(original_message) + end + + context "when reloading after creating thread" do + it "correctly loads the thread" do + chat_page.visit_channel(channel_1) + channel_page.reply_to(original_message) + thread_page.fill_composer("reply to message") + thread_page.click_send_message + + expect(thread_page).to have_message(text: "reply to message") + + refresh + + expect(thread_page).to have_message(text: "reply to message") + end + end + end + + context "when the message has an existing thread" do + fab!(:message_1) do + creator = + Chat::MessageCreator.new( + chat_channel: channel_1, + in_reply_to_id: original_message.id, + user: Fabricate(:user), + content: Faker::Lorem.paragraph, + ) + creator.create + creator.chat_message + end + + it "replies to the existing thread" do + chat_page.visit_channel(channel_1) + + expect(channel_page).to have_thread_indicator(original_message, text: "1") + + channel_page.reply_to(original_message) + + expect(side_panel_page).to have_open_thread + + thread_page.fill_composer("reply to message") + thread_page.click_send_message + + expect(thread_page).to have_message(text: message_1.message) + expect(thread_page).to have_message(text: "reply to message") + expect(channel_page).to have_thread_indicator(original_message, text: "2") + expect(channel_page).to have_no_message(text: "reply to message") + end + end + + context "with threading disabled" do + before { channel_1.update!(threading_enabled: false) } + + it "makes a reply in the channel" do + chat_page.visit_channel(channel_1) + channel_page.reply_to(original_message) + + expect(page).to have_selector( + ".chat-channel .chat-reply__excerpt", + text: original_message.message, + ) + + channel_page.fill_composer("reply to message") + channel_page.click_send_message + + expect(channel_page).to have_message(text: "reply to message") + end + end +end diff --git a/plugins/chat/spec/system/reply_to_message/mobile_spec.rb b/plugins/chat/spec/system/reply_to_message/mobile_spec.rb new file mode 100644 index 00000000000..9a1b1fc9cf3 --- /dev/null +++ b/plugins/chat/spec/system/reply_to_message/mobile_spec.rb @@ -0,0 +1,109 @@ +# frozen_string_literal: true + +RSpec.describe "Reply to message - channel - mobile", type: :system, js: true, mobile: true do + let(:chat_page) { PageObjects::Pages::Chat.new } + let(:channel_page) { PageObjects::Pages::ChatChannel.new } + let(:thread_page) { PageObjects::Pages::ChatThread.new } + let(:side_panel_page) { PageObjects::Pages::ChatSidePanel.new } + + fab!(:current_user) { Fabricate(:user) } + fab!(:channel_1) { Fabricate(:category_channel) } + fab!(:original_message) do + Fabricate(:chat_message, chat_channel: channel_1, user: Fabricate(:user)) + end + + before do + SiteSetting.enable_experimental_chat_threaded_discussions = true + chat_system_bootstrap + channel_1.update!(threading_enabled: true) + channel_1.add(current_user) + sign_in(current_user) + end + + context "when the message has not current thread" do + it "starts a thread" do + chat_page.visit_channel(channel_1) + channel_page.reply_to(original_message) + + expect(side_panel_page).to have_open_thread + + thread_page.fill_composer("reply to message") + thread_page.click_send_message + + expect(thread_page).to have_message(text: "reply to message") + + thread_page.close + + expect(channel_page).to have_thread_indicator(original_message) + end + + context "when reloading after creating thread" do + it "correctly loads the thread" do + chat_page.visit_channel(channel_1) + channel_page.reply_to(original_message) + thread_page.fill_composer("reply to message") + thread_page.click_send_message + + expect(thread_page).to have_message(text: "reply to message") + + refresh + + expect(thread_page).to have_message(text: "reply to message") + end + end + end + + context "when the message has an existing thread" do + fab!(:message_1) do + creator = + Chat::MessageCreator.new( + chat_channel: channel_1, + in_reply_to_id: original_message.id, + user: Fabricate(:user), + content: Faker::Lorem.paragraph, + ) + creator.create + creator.chat_message + end + + it "replies to the existing thread" do + chat_page.visit_channel(channel_1) + + expect(channel_page).to have_thread_indicator(original_message, text: "1") + + channel_page.reply_to(original_message) + + expect(side_panel_page).to have_open_thread + + thread_page.fill_composer("reply to message") + thread_page.click_send_message + + expect(thread_page).to have_message(text: message_1.message) + expect(thread_page).to have_message(text: "reply to message") + + thread_page.close + + expect(channel_page).to have_thread_indicator(original_message, text: "2") + expect(channel_page).to have_no_message(text: "reply to message") + end + end + + context "with threading disabled" do + before { channel_1.update!(threading_enabled: false) } + + it "makes a reply in the channel" do + chat_page.visit_channel(channel_1) + channel_page.reply_to(original_message) + + expect(page).to have_selector( + ".chat-channel .chat-reply__excerpt", + text: original_message.message, + ) + + channel_page.fill_composer("reply to message") + channel_page.click_send_message + + expect(channel_page).to have_message(text: "reply to message") + end + end +end diff --git a/plugins/chat/spec/system/reply_to_message/smoke_spec.rb b/plugins/chat/spec/system/reply_to_message/smoke_spec.rb new file mode 100644 index 00000000000..f2c5ef66134 --- /dev/null +++ b/plugins/chat/spec/system/reply_to_message/smoke_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +RSpec.describe "Reply to message - smoke", type: :system, js: true do + let(:chat_page) { PageObjects::Pages::Chat.new } + let(:channel_page) { PageObjects::Pages::ChatChannel.new } + let(:thread_page) { PageObjects::Pages::ChatThread.new } + + fab!(:user_1) { Fabricate(:user) } + fab!(:user_2) { Fabricate(:user) } + fab!(:channel_1) { Fabricate(:category_channel) } + fab!(:original_message) { Fabricate(:chat_message, chat_channel: channel_1) } + + before do + SiteSetting.enable_experimental_chat_threaded_discussions = true + chat_system_bootstrap + channel_1.add(user_1) + channel_1.add(user_2) + channel_1.update!(threading_enabled: true) + end + + context "when two users create a thread on the same message" do + it "works" do + using_session(:user_1) do + sign_in(user_1) + chat_page.visit_channel(channel_1) + channel_page.reply_to(original_message) + end + + using_session(:user_2) do + sign_in(user_2) + chat_page.visit_channel(channel_1) + channel_page.reply_to(original_message) + end + + using_session(:user_1) do + thread_page.fill_composer("user1reply") + thread_page.click_send_message + + expect(channel_page).to have_thread_indicator(original_message, text: 1) + + expect(thread_page).to have_message(text: "user1reply") + end + + using_session(:user_2) do |session| + expect(thread_page).to have_message(text: "user1reply") + expect(channel_page).to have_thread_indicator(original_message, text: 1) + + thread_page.fill_composer("user2reply") + thread_page.click_send_message + + expect(thread_page).to have_message(text: "user2reply") + expect(channel_page).to have_thread_indicator(original_message, text: 2) + + refresh + + expect(thread_page).to have_message(text: "user1reply") + expect(thread_page).to have_message(text: "user2reply") + expect(channel_page).to have_thread_indicator(original_message, text: 2) + + session.quit + end + + using_session(:user_1) do |session| + expect(thread_page).to have_message(text: "user2reply") + expect(channel_page).to have_thread_indicator(original_message, text: 2) + + refresh + + expect(thread_page).to have_message(text: "user1reply") + expect(thread_page).to have_message(text: "user2reply") + expect(channel_page).to have_thread_indicator(original_message, text: 2) + + session.quit + end + end + end +end diff --git a/plugins/chat/spec/system/shortcuts/chat_composer_spec.rb b/plugins/chat/spec/system/shortcuts/chat_composer_spec.rb index 457b1e0d3f9..624851f28da 100644 --- a/plugins/chat/spec/system/shortcuts/chat_composer_spec.rb +++ b/plugins/chat/spec/system/shortcuts/chat_composer_spec.rb @@ -92,11 +92,10 @@ RSpec.describe "Shortcuts | chat composer", type: :system, js: true do fab!(:message_1) do Fabricate(:chat_message, message: "message 1", chat_channel: channel_1, user: current_user) end - before { Fabricate(:chat_message, message: "message 2", chat_channel: channel_1) } + fab!(:message_2) { Fabricate(:chat_message, message: "message 2", chat_channel: channel_1) } it "edits last editable message" do chat.visit_channel(channel_1) - expect(channel_page).to have_message(id: message_1.id) find(".chat-composer__input").send_keys(:arrow_up) @@ -116,5 +115,15 @@ RSpec.describe "Shortcuts | chat composer", type: :system, js: true do page.driver.browser.network_conditions = { offline: false } end end + + context "with shift" do + it "starts replying to the last message" do + chat.visit_channel(channel_1) + + find(".chat-composer__input").send_keys(%i[shift arrow_up]) + + expect(channel_page).to be_replying_to(message_2) + end + end end end diff --git a/plugins/chat/spec/system/single_thread_spec.rb b/plugins/chat/spec/system/single_thread_spec.rb index cf6978a42fa..3300e83a7c5 100644 --- a/plugins/chat/spec/system/single_thread_spec.rb +++ b/plugins/chat/spec/system/single_thread_spec.rb @@ -123,8 +123,8 @@ describe "Single thread in side panel", type: :system, js: true do chat_page.visit_channel(channel) channel_page.message_thread_indicator(thread.original_message).click expect(side_panel).to have_open_thread(thread) - thread_page.send_message(thread.id, "new thread message") - expect(thread_page).to have_message(thread.id, text: "new thread message") + thread_page.send_message("new thread message") + expect(thread_page).to have_message(thread_id: 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) @@ -134,8 +134,8 @@ describe "Single thread in side panel", type: :system, js: true do chat_page.visit_channel(channel) channel_page.message_thread_indicator(thread.original_message).click expect(side_panel).to have_open_thread(thread) - thread_page.send_message(thread.id, "new thread message") - expect(thread_page).to have_message(thread.id, text: "new thread message") + thread_page.send_message("new thread message") + expect(thread_page).to have_message(thread_id: 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 @@ -157,19 +157,19 @@ describe "Single thread in side panel", type: :system, js: true do using_session(:tab_2) do expect(side_panel).to have_open_thread(thread) - thread_page.send_message(thread.id, "the other user message") - expect(thread_page).to have_message(thread.id, text: "the other user message") + thread_page.send_message("the other user message") + expect(thread_page).to have_message(thread_id: thread.id, text: "the other user message") end using_session(:tab_1) do expect(side_panel).to have_open_thread(thread) - expect(thread_page).to have_message(thread.id, text: "the other user message") - thread_page.send_message(thread.id, "this is a test message") - expect(thread_page).to have_message(thread.id, text: "this is a test message") + expect(thread_page).to have_message(thread_id: thread.id, text: "the other user message") + thread_page.send_message("this is a test message") + expect(thread_page).to have_message(thread_id: thread.id, text: "this is a test message") end using_session(:tab_2) do - expect(thread_page).to have_message(thread.id, text: "this is a test message") + expect(thread_page).to have_message(thread_id: thread.id, text: "this is a test message") end end