diff --git a/plugins/chat/app/controllers/chat/api/channel_threads_controller.rb b/plugins/chat/app/controllers/chat/api/channel_threads_controller.rb index 2a25950a475..b21f6c58618 100644 --- a/plugins/chat/app/controllers/chat/api/channel_threads_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channel_threads_controller.rb @@ -18,7 +18,6 @@ class Chat::Api::ChannelThreadsController < Chat::ApiController root: false, ) end - on_failed_policy(:threaded_discussions_enabled) { raise Discourse::NotFound } on_failed_policy(:threading_enabled_for_channel) { raise Discourse::NotFound } on_failed_policy(:can_view_channel) { raise Discourse::InvalidAccess } on_model_not_found(:channel) { raise Discourse::NotFound } @@ -39,7 +38,7 @@ class Chat::Api::ChannelThreadsController < Chat::ApiController participants: result.participants, ) end - on_failed_policy(:threaded_discussions_enabled) { raise Discourse::NotFound } + on_failed_policy(:threading_enabled_for_channel) { raise Discourse::NotFound } on_model_not_found(:thread) { raise Discourse::NotFound } end @@ -47,7 +46,6 @@ class Chat::Api::ChannelThreadsController < Chat::ApiController def update with_service(::Chat::UpdateThread) do - on_failed_policy(:threaded_discussions_enabled) { raise Discourse::NotFound } on_failed_policy(:threading_enabled_for_channel) { raise Discourse::NotFound } on_failed_policy(:can_view_channel) { raise Discourse::InvalidAccess } on_failed_policy(:can_edit_thread) { raise Discourse::InvalidAccess } @@ -57,4 +55,24 @@ class Chat::Api::ChannelThreadsController < Chat::ApiController end end end + + def create + with_service(::Chat::CreateThread) do + on_success do + render_serialized( + result.thread, + ::Chat::ThreadSerializer, + root: false, + membership: result.membership, + include_thread_original_message: true, + ) + end + on_failed_policy(:threading_enabled_for_channel) { raise Discourse::NotFound } + on_failed_policy(:can_view_channel) { raise Discourse::InvalidAccess } + on_failed_step(:create_thread) do + render json: failed_json.merge(errors: [result["result.step.create_thread"].error]), + status: 422 + end + end + end end diff --git a/plugins/chat/app/services/chat/create_message.rb b/plugins/chat/app/services/chat/create_message.rb index 1d131446aad..2c13c0ec8b2 100644 --- a/plugins/chat/app/services/chat/create_message.rb +++ b/plugins/chat/app/services/chat/create_message.rb @@ -9,7 +9,7 @@ module Chat class CreateMessage include Service::Base - # @!method call(chat_channel_id:, guardian:, in_reply_to_id:, message:, staged_id:, upload_ids:, thread_id:, staged_thread_id:, incoming_chat_webhook:) + # @!method call(chat_channel_id:, guardian:, in_reply_to_id:, message:, staged_id:, upload_ids:, thread_id:, incoming_chat_webhook:) # @param guardian [Guardian] # @param chat_channel_id [Integer] # @param message [String] @@ -17,7 +17,6 @@ module Chat # @param thread_id [Integer] ID of a thread to reply to # @param upload_ids [Array] IDs of uploaded documents # @param staged_id [String] arbitrary string that will be sent back to the client - # @param staged_thread_id [String] arbitrary string that will be sent back to the client (for a new thread) # @param incoming_chat_webhook [Chat::IncomingWebhook] policy :no_silenced_user @@ -53,7 +52,6 @@ module Chat attribute :staged_id, :string attribute :upload_ids, :array attribute :thread_id, :string - attribute :staged_thread_id, :string attribute :incoming_chat_webhook validates :chat_channel_id, presence: true @@ -167,16 +165,11 @@ module Chat def publish_new_thread(reply:, contract:, channel:, thread:, **) return unless channel.threading_enabled? return unless reply&.thread_id_previously_changed?(from: nil) - Chat::Publisher.publish_thread_created!(channel, reply, thread.id, contract.staged_thread_id) + Chat::Publisher.publish_thread_created!(channel, reply, thread.id) end def publish_new_message_events(channel:, message:, contract:, guardian:, **) - Chat::Publisher.publish_new!( - channel, - message, - contract.staged_id, - staged_thread_id: contract.staged_thread_id, - ) + Chat::Publisher.publish_new!(channel, message, contract.staged_id) Jobs.enqueue(Jobs::Chat::ProcessMessage, { chat_message_id: message.id }) Chat::Notifier.notify_new(chat_message: message, timestamp: message.created_at) DiscourseEvent.trigger(:chat_message_created, message, channel, guardian.user) diff --git a/plugins/chat/app/services/chat/create_thread.rb b/plugins/chat/app/services/chat/create_thread.rb new file mode 100644 index 00000000000..3b83352ac37 --- /dev/null +++ b/plugins/chat/app/services/chat/create_thread.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +module Chat + # Creates a thread. + # + # @example + # Chat::CreateThread.call(channel_id: 2, original_message_id: 3, guardian: guardian, title: "Restaurant for Saturday") + # + class CreateThread + include Service::Base + + # @!method call(thread_id:, channel_id:, guardian:, **params_to_edit) + # @param [Integer] original_message_id + # @param [Integer] channel_id + # @param [Guardian] guardian + # @option params_to_create [String,nil] title + # @return [Service::Base::Context] + + contract + model :channel + policy :can_view_channel + policy :threading_enabled_for_channel + model :original_message + transaction do + step :create_thread + step :associate_thread_to_message + step :fetch_membership + step :publish_new_thread + end + + # @!visibility private + class Contract + attribute :original_message_id, :integer + attribute :channel_id, :integer + attribute :title, :string + + validates :original_message_id, :channel_id, presence: true + validates :title, length: { maximum: Chat::Thread::MAX_TITLE_LENGTH } + end + + private + + def fetch_channel(contract:, **) + ::Chat::Channel.find_by(id: contract.channel_id) + end + + def fetch_original_message(channel:, contract:, **) + ::Chat::Message.find_by( + id: contract.original_message_id, + chat_channel_id: contract.channel_id, + ) + end + + def can_view_channel(guardian:, channel:, **) + guardian.can_preview_chat_channel?(channel) + end + + def threading_enabled_for_channel(channel:, **) + channel.threading_enabled + end + + def create_thread(channel:, original_message:, contract:, **) + context.thread = + channel.threads.create( + title: contract.title, + original_message: original_message, + original_message_user: original_message.user, + ) + fail!(context.thread.errors.full_messages.join(", ")) if context.thread.invalid? + end + + def associate_thread_to_message(original_message:, **) + original_message.update(thread: context.thread) + end + + def fetch_membership(guardian:, **) + context.membership = context.thread.membership_for(guardian.user) + end + + def publish_new_thread(channel:, original_message:, **) + ::Chat::Publisher.publish_thread_created!(channel, original_message, context.thread.id) + end + end +end diff --git a/plugins/chat/app/services/chat/publisher.rb b/plugins/chat/app/services/chat/publisher.rb index 9eb41d9dd0b..af0f56b644b 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, staged_thread_id: nil) + def self.calculate_publish_targets(channel, message) return [root_message_bus_channel(channel.id)] if !allow_publish_to_thread?(channel) if message.thread_om? @@ -22,9 +22,8 @@ module Chat root_message_bus_channel(channel.id), thread_message_bus_channel(channel.id, message.thread_id), ] - elsif staged_thread_id || message.thread_reply? + elsif 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)] @@ -35,16 +34,12 @@ module Chat channel.threading_enabled end - 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) + def self.publish_new!(chat_channel, chat_message, staged_id) + message_bus_targets = calculate_publish_targets(chat_channel, chat_message) publish_to_targets!( message_bus_targets, chat_channel, - serialize_message_with_type(chat_message, :sent).merge( - staged_id: staged_id, - staged_thread_id: staged_thread_id, - ), + serialize_message_with_type(chat_message, :sent).merge(staged_id: staged_id), ) if !chat_message.thread_reply? || !allow_publish_to_thread?(chat_channel) @@ -100,14 +95,10 @@ module Chat ) end - def self.publish_thread_created!(chat_channel, chat_message, thread_id, staged_thread_id) + def self.publish_thread_created!(chat_channel, chat_message, 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 }, - ), + serialize_message_with_type(chat_message, :thread_created, { thread_id: thread_id }), ) end diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-desktop.gjs b/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-desktop.gjs new file mode 100644 index 00000000000..c845e6319dd --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-desktop.gjs @@ -0,0 +1,195 @@ +import Component from "@glimmer/component"; +import { inject as service } from "@ember/service"; +import ChatMessageInteractor from "discourse/plugins/chat/discourse/lib/chat-message-interactor"; +import { getOwner } from "@ember/application"; +import { schedule } from "@ember/runloop"; +import { createPopper } from "@popperjs/core"; +import chatMessageContainer from "discourse/plugins/chat/discourse/lib/chat-message-container"; +import { action } from "@ember/object"; +import { tracked } from "@glimmer/tracking"; +import didInsert from "@ember/render-modifiers/modifiers/did-insert"; +import didUpdate from "@ember/render-modifiers/modifiers/did-update"; +import willDestroy from "@ember/render-modifiers/modifiers/will-destroy"; +import concatClass from "discourse/helpers/concat-class"; +import BookmarkIcon from "discourse/components/bookmark-icon"; +import ChatMessageReaction from "discourse/plugins/chat/discourse/components/chat-message-reaction"; +import DButton from "discourse/components/d-button"; +import DropdownSelectBox from "select-kit/components/dropdown-select-box"; +import { on } from "@ember/modifier"; +import and from "truth-helpers/helpers/and"; +import { concat, hash } from "@ember/helper"; + +const MSG_ACTIONS_VERTICAL_PADDING = -10; +const FULL = "full"; +const REDUCED = "reduced"; +const REDUCED_WIDTH_THRESHOLD = 500; + +export default class ChatMessageActionsDesktop extends Component { + + + @service chat; + @service chatEmojiPickerManager; + @service site; + + @tracked size = FULL; + + popper = null; + + get message() { + return this.chat.activeMessage.model; + } + + get context() { + return this.chat.activeMessage.context; + } + + get messageInteractor() { + return new ChatMessageInteractor( + getOwner(this), + this.message, + this.context + ); + } + + get shouldRenderFavoriteReactions() { + return this.size === FULL; + } + + @action + onWheel() { + // prevents menu to stop scroll on the list of messages + this.chat.activeMessage = null; + } + + @action + setup(element) { + this.popper?.destroy(); + + schedule("afterRender", () => { + const messageContainer = chatMessageContainer( + this.message.id, + this.context + ); + + if (!messageContainer) { + return; + } + + const viewport = messageContainer.closest(".popper-viewport"); + this.size = + viewport.clientWidth < REDUCED_WIDTH_THRESHOLD ? REDUCED : FULL; + + if (!messageContainer) { + return; + } + + this.popper = createPopper(messageContainer, element, { + placement: "top-end", + strategy: "fixed", + modifiers: [ + { + name: "flip", + enabled: true, + options: { + boundary: viewport, + fallbackPlacements: ["bottom-end"], + }, + }, + { name: "hide", enabled: true }, + { name: "eventListeners", options: { scroll: false } }, + { + name: "offset", + options: { offset: [-2, MSG_ACTIONS_VERTICAL_PADDING] }, + }, + ], + }); + }); + } + + @action + teardown() { + this.popper?.destroy(); + this.popper = null; + } +} 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 deleted file mode 100644 index 6fdde4039a0..00000000000 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-desktop.hbs +++ /dev/null @@ -1,79 +0,0 @@ -{{#if (and this.site.desktopView this.chat.activeMessage.model.id)}} -
-
- {{#if this.shouldRenderFavoriteReactions}} - {{#each - this.messageInteractor.emojiReactions key="emoji" - as |reaction| - }} - - {{/each}} - {{/if}} - - {{#if this.messageInteractor.canInteractWithMessage}} - - {{/if}} - - {{#if this.messageInteractor.canBookmark}} - - - - {{/if}} - - {{#if this.messageInteractor.canReply}} - - {{/if}} - - {{#if - (and - this.messageInteractor.message - this.messageInteractor.secondaryActions.length - ) - }} - - {{/if}} -
-
-{{/if}} \ No newline at end of file diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-desktop.js b/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-desktop.js deleted file mode 100644 index d2fad1c7c4c..00000000000 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-desktop.js +++ /dev/null @@ -1,101 +0,0 @@ -import Component from "@glimmer/component"; -import { inject as service } from "@ember/service"; -import ChatMessageInteractor from "discourse/plugins/chat/discourse/lib/chat-message-interactor"; -import { getOwner } from "@ember/application"; -import { schedule } from "@ember/runloop"; -import { createPopper } from "@popperjs/core"; -import chatMessageContainer from "discourse/plugins/chat/discourse/lib/chat-message-container"; -import { action } from "@ember/object"; -import { tracked } from "@glimmer/tracking"; - -const MSG_ACTIONS_VERTICAL_PADDING = -10; -const FULL = "full"; -const REDUCED = "reduced"; -const REDUCED_WIDTH_THRESHOLD = 500; - -export default class ChatMessageActionsDesktop extends Component { - @service chat; - @service chatEmojiPickerManager; - @service site; - - @tracked size = FULL; - - popper = null; - - get message() { - return this.chat.activeMessage.model; - } - - get context() { - return this.chat.activeMessage.context; - } - - get messageInteractor() { - return new ChatMessageInteractor( - getOwner(this), - this.message, - this.context - ); - } - - get shouldRenderFavoriteReactions() { - return this.size === FULL; - } - - @action - onWheel() { - // prevents menu to stop scroll on the list of messages - this.chat.activeMessage = null; - } - - @action - setup(element) { - this.popper?.destroy(); - - schedule("afterRender", () => { - const messageContainer = chatMessageContainer( - this.message.id, - this.context - ); - - if (!messageContainer) { - return; - } - - const viewport = messageContainer.closest(".popper-viewport"); - this.size = - viewport.clientWidth < REDUCED_WIDTH_THRESHOLD ? REDUCED : FULL; - - if (!messageContainer) { - return; - } - - this.popper = createPopper(messageContainer, element, { - placement: "top-end", - strategy: "fixed", - modifiers: [ - { - name: "flip", - enabled: true, - options: { - boundary: viewport, - fallbackPlacements: ["bottom-end"], - }, - }, - { name: "hide", enabled: true }, - { name: "eventListeners", options: { scroll: false } }, - { - name: "offset", - options: { offset: [-2, MSG_ACTIONS_VERTICAL_PADDING] }, - }, - ], - }); - }); - } - - @action - teardown() { - this.popper?.destroy(); - this.popper = null; - } -} diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-mobile.gjs b/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-mobile.gjs new file mode 100644 index 00000000000..31baed3481f --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-mobile.gjs @@ -0,0 +1,197 @@ +import Component from "@glimmer/component"; +import ChatMessageInteractor from "discourse/plugins/chat/discourse/lib/chat-message-interactor"; +import { getOwner } from "discourse-common/lib/get-owner"; +import { tracked } from "@glimmer/tracking"; +import discourseLater from "discourse-common/lib/later"; +import { action } from "@ember/object"; +import { isTesting } from "discourse-common/config/environment"; +import { inject as service } from "@ember/service"; +import and from "truth-helpers/helpers/and"; +import didInsert from "@ember/render-modifiers/modifiers/did-insert"; +import concatClass from "discourse/helpers/concat-class"; +import DButton from "discourse/components/d-button"; +import { on } from "@ember/modifier"; +import ChatUserAvatar from "discourse/plugins/chat/discourse/components/chat/user-avatar"; +import ChatMessageReaction from "discourse/plugins/chat/discourse/components/chat-message-reaction"; +import { fn } from "@ember/helper"; +import or from "truth-helpers/helpers/or"; +import BookmarkIcon from "discourse/components/bookmark-icon"; + +export default class ChatMessageActionsMobile extends Component { + + + @service chat; + @service site; + @service capabilities; + + @tracked hasExpandedReply = false; + @tracked showFadeIn = false; + + get message() { + return this.chat.activeMessage.model; + } + + get context() { + return this.chat.activeMessage.context; + } + + get messageInteractor() { + return new ChatMessageInteractor( + getOwner(this), + this.message, + this.context + ); + } + + @action + fadeAndVibrate() { + discourseLater(this.#addFadeIn.bind(this)); + + if (this.capabilities.canVibrate && !isTesting()) { + navigator.vibrate(5); + } + } + + @action + expandReply(event) { + event.stopPropagation(); + this.hasExpandedReply = true; + } + + @action + collapseMenu(event) { + event.preventDefault(); + this.#onCloseMenu(); + } + + @action + actAndCloseMenu(fnId) { + this.messageInteractor[fnId](); + this.#onCloseMenu(); + } + + @action + openEmojiPicker(_, event) { + this.messageInteractor.openEmojiPicker(_, event); + this.#onCloseMenu(); + } + + #onCloseMenu() { + this.#removeFadeIn(); + + // we don't want to remove the component right away as it's animating + // 200 is equal to the duration of the css animation + discourseLater(() => { + if (this.isDestroying || this.isDestroyed) { + return; + } + + // by ensuring we are not hovering any message anymore + // we also ensure the menu is fully removed + this.chat.activeMessage = null; + }, 200); + } + + #addFadeIn() { + this.showFadeIn = true; + } + + #removeFadeIn() { + this.showFadeIn = false; + } +} 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 deleted file mode 100644 index 887156208fa..00000000000 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-mobile.hbs +++ /dev/null @@ -1,93 +0,0 @@ -{{#if (and this.site.mobileView this.chat.activeMessage)}} -
-
-
- -
-
-
- - - {{this.message.message}} - -
-
- -
    - {{#each this.messageInteractor.secondaryActions as |button|}} -
  • - -
    • - {{/each}} -
- - {{#if - (or this.messageInteractor.canReact this.messageInteractor.canReply) - }} -
- {{#if this.messageInteractor.canReact}} - {{#each this.messageInteractor.emojiReactions as |reaction|}} - - {{/each}} - - - {{/if}} - - {{#if this.messageInteractor.canBookmark}} - - - - {{/if}} - - {{#if this.messageInteractor.canReply}} - - {{/if}} -
- {{/if}} -
-
-{{/if}} \ No newline at end of file diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-mobile.js b/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-mobile.js deleted file mode 100644 index 43c2373c121..00000000000 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message-actions-mobile.js +++ /dev/null @@ -1,90 +0,0 @@ -import Component from "@glimmer/component"; -import ChatMessageInteractor from "discourse/plugins/chat/discourse/lib/chat-message-interactor"; -import { getOwner } from "discourse-common/lib/get-owner"; -import { tracked } from "@glimmer/tracking"; -import discourseLater from "discourse-common/lib/later"; -import { action } from "@ember/object"; -import { isTesting } from "discourse-common/config/environment"; -import { inject as service } from "@ember/service"; - -export default class ChatMessageActionsMobile extends Component { - @service chat; - @service site; - @service capabilities; - - @tracked hasExpandedReply = false; - @tracked showFadeIn = false; - - get message() { - return this.chat.activeMessage.model; - } - - get context() { - return this.chat.activeMessage.context; - } - - get messageInteractor() { - return new ChatMessageInteractor( - getOwner(this), - this.message, - this.context - ); - } - - @action - fadeAndVibrate() { - discourseLater(this.#addFadeIn.bind(this)); - - if (this.capabilities.canVibrate && !isTesting()) { - navigator.vibrate(5); - } - } - - @action - expandReply(event) { - event.stopPropagation(); - this.hasExpandedReply = true; - } - - @action - collapseMenu(event) { - event.preventDefault(); - this.#onCloseMenu(); - } - - @action - actAndCloseMenu(fnId) { - this.messageInteractor[fnId](); - this.#onCloseMenu(); - } - - @action - openEmojiPicker(_, event) { - this.messageInteractor.openEmojiPicker(_, event); - this.#onCloseMenu(); - } - - #onCloseMenu() { - this.#removeFadeIn(); - - // we don't want to remove the component right away as it's animating - // 200 is equal to the duration of the css animation - discourseLater(() => { - if (this.isDestroying || this.isDestroyed) { - return; - } - - // by ensuring we are not hovering any message anymore - // we also ensure the menu is fully removed - this.chat.activeMessage = null; - }, 200); - } - - #addFadeIn() { - this.showFadeIn = true; - } - - #removeFadeIn() { - this.showFadeIn = false; - } -} diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message.gjs b/plugins/chat/assets/javascripts/discourse/components/chat-message.gjs index c93132059b5..ba0922ebfbe 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message.gjs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-message.gjs @@ -460,7 +460,7 @@ export default class ChatMessage extends Component { @action onLongPressStart(element, event) { - if (!this.args.message.expanded) { + if (!this.args.message.expanded || !this.args.message.persisted) { return; } diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs b/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs index 0d5fb60712d..8446960a181 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-thread.hbs @@ -2,7 +2,6 @@ class={{concat-class "chat-thread" (if this.messagesLoader.loading "loading") - (if @thread.staged "staged") }} data-id={{@thread.id}} {{did-insert this.setUploadDropZone}} diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-thread.js b/plugins/chat/assets/javascripts/discourse/components/chat-thread.js index 98261307bcb..7908b0b526e 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-thread.js +++ b/plugins/chat/assets/javascripts/discourse/components/chat-thread.js @@ -177,14 +177,6 @@ export default class ChatThread extends Component { } async fetchMessages(findArgs = {}) { - if (this.args.thread.staged) { - const message = this.args.thread.originalMessage; - message.thread = this.args.thread; - message.manager = this.messagesManager; - this.messagesManager.addMessages([message]); - return; - } - if (this.messagesLoader.loading) { return; } @@ -326,7 +318,7 @@ export default class ChatThread extends Component { // and scrolling; for now it's enough to do it when the thread panel // opens/messages are loaded since we have no pagination for threads. markThreadAsRead() { - if (!this.args.thread || this.args.thread.staged) { + if (!this.args.thread) { return; } @@ -374,13 +366,10 @@ export default class ChatThread extends Component { this.args.thread.channel.id, { message: message.message, - in_reply_to_id: message.thread.staged - ? message.thread.originalMessage?.id - : null, + in_reply_to_id: null, staged_id: message.id, upload_ids: message.uploads.map((upload) => upload.id), - thread_id: message.thread.staged ? null : message.thread.id, - staged_thread_id: message.thread.staged ? message.thread.id : null, + thread_id: message.thread.id, } ); diff --git a/plugins/chat/assets/javascripts/discourse/models/chat-channel.js b/plugins/chat/assets/javascripts/discourse/models/chat-channel.js index 056c0356133..40600163531 100644 --- a/plugins/chat/assets/javascripts/discourse/models/chat-channel.js +++ b/plugins/chat/assets/javascripts/discourse/models/chat-channel.js @@ -7,7 +7,6 @@ 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"; import ChatDirectMessage from "discourse/plugins/chat/discourse/models/chat-direct-message"; import ChatChannelArchive from "discourse/plugins/chat/discourse/models/chat-channel-archive"; import Category from "discourse/models/category"; @@ -190,23 +189,6 @@ export default class ChatChannel { return this.meta.can_join_chat_channel; } - 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; - clonedMessage.manager = thread.messagesManager; - thread.messagesManager.addMessages([clonedMessage]); - - return thread; - } - async stageMessage(message) { message.id = guid(); message.staged = true; diff --git a/plugins/chat/assets/javascripts/discourse/models/chat-message.js b/plugins/chat/assets/javascripts/discourse/models/chat-message.js index 71c033306c6..f9c0290e018 100644 --- a/plugins/chat/assets/javascripts/discourse/models/chat-message.js +++ b/plugins/chat/assets/javascripts/discourse/models/chat-message.js @@ -57,7 +57,6 @@ export default class ChatMessage { @tracked _thread; constructor(channel, args = {}) { - // when modifying constructor, be sure to update duplicate function accordingly this.id = args.id; this.channel = channel; this.manager = args.manager; @@ -99,35 +98,8 @@ export default class ChatMessage { } } - 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, - 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.reactions = this.reactions; - message.user = this.user; - message.inReplyTo = this.inReplyTo; - message.bookmark = this.bookmark; - message.uploads = this.uploads; - - return message; + get persisted() { + return !!this.id && !this.staged; } get replyable() { 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 5a04de8e906..dc358406ac6 100644 --- a/plugins/chat/assets/javascripts/discourse/routes/chat-channel-thread.js +++ b/plugins/chat/assets/javascripts/discourse/routes/chat-channel-thread.js @@ -6,7 +6,6 @@ export default class ChatChannelThread extends DiscourseRoute { @service router; @service chatStateManager; @service chat; - @service chatStagedThreadMapping; @service chatThreadPane; model(params, transition) { @@ -46,24 +45,6 @@ export default class ChatChannelThread extends DiscourseRoute { 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 { threadId } = this.paramsFor(this.routeName); - - if (threadId?.startsWith("staged-thread-")) { - const mapping = this.chatStagedThreadMapping.getMapping(); - - if (mapping[threadId]) { - transition.abort(); - return this.router.transitionTo( - this.routeName, - ...[...channel.routeModels, mapping[threadId]] - ); - } - } - const { messageId } = this.paramsFor(this.routeName + ".near-message"); if ( !messageId && diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-api.js b/plugins/chat/assets/javascripts/discourse/services/chat-api.js index e8677485470..daae4cc8b19 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-api.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-api.js @@ -153,7 +153,6 @@ export default class ChatApi extends Service { * @param {number} [data.in_reply_to_id] - The ID of the replied-to message. * @param {number} [data.staged_id] - The staged ID of the message before it was persisted. * @param {number} [data.thread_id] - The ID of the thread where this message should be posted. - * @param {number} [data.staged_thread_id] - The staged ID of the thread before it was persisted. * @param {Array.} [data.upload_ids] - Array of upload ids linked to the message. * @returns {Promise} */ @@ -204,6 +203,21 @@ export default class ChatApi extends Service { return this.#putRequest(`/channels/${channelId}`, { channel: data }); } + /** + * Creates a thread. + * @param {number} channelId - The ID of the channel. + * @param {number} originalMessageId - The ID of the original message. + * @param {object} data - Params of the thread. + * @param {string} [data.title] - Title of the thread. + * @returns {Promise} + */ + createThread(channelId, originalMessageId, data = {}) { + return this.#postRequest(`/channels/${channelId}/threads`, { + title: data.title, + original_message_id: originalMessageId, + }); + } + /** * Updates the status of a channel. * @param {number} channelId - The ID of the channel. 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 b745b153dff..fdf3823ae37 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-channel-composer.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-channel-composer.js @@ -5,9 +5,11 @@ import { tracked } from "@glimmer/tracking"; export default class ChatChannelComposer extends Service { @service chat; + @service chatApi; @service currentUser; @service router; @service("chat-thread-composer") threadComposer; + @service loadingSlider; @tracked message; @tracked textarea; @@ -54,7 +56,17 @@ export default class ChatChannelComposer extends Service { if (message.channel.threadingEnabled) { if (!message.thread?.id) { - message.thread = message.channel.createStagedThread(message); + this.loadingSlider.transitionStarted(); + const threadObject = await this.chatApi.createThread( + message.channel.id, + message.id + ); + this.loadingSlider.transitionEnded(); + + message.thread = message.channel.threadsManager.add( + message.channel, + threadObject + ); } this.reset(message.channel); 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 daa1e79b315..09d9365f07f 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 @@ -36,7 +36,6 @@ export function handleStagedMessage(channel, messagesManager, data) { export default class ChatPaneBaseSubscriptionsManager extends Service { @service chat; @service currentUser; - @service chatStagedThreadMapping; messageBusChannel = null; messageBusLastId = null; @@ -215,40 +214,14 @@ export default class ChatPaneBaseSubscriptionsManager extends Service { 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.thread.preview.replyCount ??= 1; + .find(this.model.id, data.thread_id, { fetchIfNotFound: true }) + .then((thread) => { + const channelOriginalMessage = this.model.messagesManager.findMessage( + thread.originalMessage.id + ); - // We have to do this because the thread manager cache is keyed by - // staged_thread_id, but the thread_id is what we want to use to - // look up the thread, otherwise calls to .find() will not return - // the thread by its actual ID, and we will end up with double-ups - // in places like the thread list when .add() is called. - this.model.threadsManager.remove({ id: data.staged_thread_id }); - this.model.threadsManager.add(this.model, stagedThread, { - replace: true, - }); - } 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; - } - }); + if (channelOriginalMessage) { + channelOriginalMessage.thread = thread; } }); } 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 deleted file mode 100644 index 747e92e3df0..00000000000 --- a/plugins/chat/assets/javascripts/discourse/services/chat-staged-thread-mapping.js +++ /dev/null @@ -1,34 +0,0 @@ -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/config/routes.rb b/plugins/chat/config/routes.rb index a4a8c8e3432..7404c0a59d4 100644 --- a/plugins/chat/config/routes.rb +++ b/plugins/chat/config/routes.rb @@ -30,6 +30,7 @@ Chat::Engine.routes.draw do get "/mentions/groups" => "hints#check_group_mentions", :format => :json get "/channels/:channel_id/threads" => "channel_threads#index" + post "/channels/:channel_id/threads" => "channel_threads#create" put "/channels/:channel_id/threads/:thread_id" => "channel_threads#update" get "/channels/:channel_id/threads/:thread_id" => "channel_threads#show" get "/channels/:channel_id/threads/:thread_id/messages" => "channel_thread_messages#index" diff --git a/plugins/chat/spec/requests/chat/api/channel_threads_controller_spec.rb b/plugins/chat/spec/requests/chat/api/channel_threads_controller_spec.rb index 31ca9760f5a..cad5329c68a 100644 --- a/plugins/chat/spec/requests/chat/api/channel_threads_controller_spec.rb +++ b/plugins/chat/spec/requests/chat/api/channel_threads_controller_spec.rb @@ -234,4 +234,63 @@ RSpec.describe Chat::Api::ChannelThreadsController do end end end + + describe "create" do + fab!(:channel_1) { Fabricate(:chat_channel, threading_enabled: true) } + fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1) } + + let(:title) { "a very nice cat" } + let(:params) { { title: title, original_message_id: message_1.id } } + let(:channel_id) { channel_1.id } + + context "when channel does not exist" do + it "returns 404" do + channel_1.destroy! + post "/chat/api/channels/#{channel_id}", params: params + + expect(response.status).to eq(404) + end + end + + context "when channel exists" do + it "creates the thread" do + post "/chat/api/channels/#{channel_id}/threads", params: params + + expect(response.status).to eq(200) + expect(response.parsed_body["title"]).to eq(title) + end + + context "when user cannot view the channel" do + let(:channel_id) { Fabricate(:private_category_channel).id } + + it "returns 403" do + post "/chat/api/channels/#{channel_id}/threads", params: params + + expect(response.status).to eq(403) + end + end + + context "when the title is too long" do + let(:title) { "x" * Chat::Thread::MAX_TITLE_LENGTH + "x" } + + it "returns 400" do + post "/chat/api/channels/#{channel_id}/threads", params: params + + expect(response.status).to eq(400) + expect(response.parsed_body["errors"]).to eq( + ["Title is too long (maximum is #{Chat::Thread::MAX_TITLE_LENGTH} characters)"], + ) + end + end + end + + context "when channel does not have threading enabled" do + fab!(:channel_1) { Fabricate(:chat_channel, threading_enabled: false) } + + it "returns 404" do + post "/chat/api/channels/#{channel_id}/threads", params: params + expect(response.status).to eq(404) + end + end + end end diff --git a/plugins/chat/spec/requests/chat/api/messages_controller_spec.rb b/plugins/chat/spec/requests/chat/api/messages_controller_spec.rb index 274a8493922..e60204b42c6 100644 --- a/plugins/chat/spec/requests/chat/api/messages_controller_spec.rb +++ b/plugins/chat/spec/requests/chat/api/messages_controller_spec.rb @@ -301,32 +301,6 @@ RSpec.describe Chat::Api::ChannelMessagesController 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) - chat_channel.update!(threading_enabled: true) - - 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/create_message_spec.rb b/plugins/chat/spec/services/chat/create_message_spec.rb index 57f4268d618..41a668176f6 100644 --- a/plugins/chat/spec/services/chat/create_message_spec.rb +++ b/plugins/chat/spec/services/chat/create_message_spec.rb @@ -69,12 +69,7 @@ RSpec.describe Chat::CreateMessage do end it "publishes the new message" do - Chat::Publisher.expects(:publish_new!).with( - channel, - instance_of(Chat::Message), - nil, - staged_thread_id: nil, - ) + Chat::Publisher.expects(:publish_new!).with(channel, instance_of(Chat::Message), nil) result end diff --git a/plugins/chat/spec/services/chat/create_thread_spec.rb b/plugins/chat/spec/services/chat/create_thread_spec.rb new file mode 100644 index 00000000000..f687c67e5b8 --- /dev/null +++ b/plugins/chat/spec/services/chat/create_thread_spec.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +RSpec.describe Chat::CreateThread do + describe Chat::CreateThread::Contract, type: :model do + it { is_expected.to validate_presence_of :channel_id } + it { is_expected.to validate_presence_of :original_message_id } + end + + describe ".call" do + subject(:result) { described_class.call(params) } + + fab!(:current_user) { Fabricate(:user) } + fab!(:channel_1) { Fabricate(:chat_channel, threading_enabled: true) } + fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1) } + + let(:guardian) { Guardian.new(current_user) } + let(:title) { nil } + let(:params) do + { + guardian: guardian, + original_message_id: message_1.id, + channel_id: channel_1.id, + title: title, + } + end + + context "when all steps pass" do + it "sets the service result as successful" do + expect(result).to be_a_success + end + + it "creates a thread" do + result + expect(result.thread).to be_persisted + end + + it "associates the original message to the thread" do + expect { + result + message_1.reload + }.to change { message_1.thread_id }.from(nil).to(result.thread.id) + end + + it "fetches the membership" do + result + expect(result.membership).to eq(result.thread.membership_for(current_user)) + end + + it "publishes a `thread_created` MessageBus event" do + message = MessageBus.track_publish("/chat/#{channel_1.id}") { result }.first + expect(message.data["type"]).to eq("thread_created") + end + + it "sets the title when existing" do + params[:title] = "Restaurant for Saturday" + result + expect(result.thread.title).to eq(params[:title]) + end + end + + context "when params are not valid" do + before { params.delete(:original_message_id) } + + it { is_expected.to fail_a_contract } + end + + context "when title is too long" do + let(:title) { "a" * Chat::Thread::MAX_TITLE_LENGTH + "a" } + + it { is_expected.to fail_a_contract } + end + + context "when original message is not found" do + fab!(:channel_2) { Fabricate(:chat_channel, threading_enabled: true) } + + before { params[:channel_id] = channel_2.id } + + it { is_expected.to fail_to_find_a_model(:original_message) } + end + + context "when original message is not found" do + before { message_1.destroy! } + + it { is_expected.to fail_to_find_a_model(:original_message) } + end + + context "when user cannot see channel" do + fab!(:private_channel_1) { Fabricate(:private_category_channel, group: Fabricate(:group)) } + + before { params[:channel_id] = private_channel_1.id } + + it { is_expected.to fail_a_policy(:can_view_channel) } + end + + context "when threading is not enabled for the channel" do + before { channel_1.update!(threading_enabled: false) } + + it { is_expected.to fail_a_policy(:threading_enabled_for_channel) } + end + end +end diff --git a/plugins/chat/spec/services/chat/publisher_spec.rb b/plugins/chat/spec/services/chat/publisher_spec.rb index 0385cb0e95d..27bbd016dd3 100644 --- a/plugins/chat/spec/services/chat/publisher_spec.rb +++ b/plugins/chat/spec/services/chat/publisher_spec.rb @@ -181,32 +181,6 @@ 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_1.update!(thread: thread) } - - it "generates the correct targets" do - targets = - described_class.calculate_publish_targets( - channel, - message_1, - 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/test/javascripts/unit/models/chat-message-test.js b/plugins/chat/test/javascripts/unit/models/chat-message-test.js new file mode 100644 index 00000000000..091d5bfd4e4 --- /dev/null +++ b/plugins/chat/test/javascripts/unit/models/chat-message-test.js @@ -0,0 +1,23 @@ +import { module, test } from "qunit"; +import fabricators from "discourse/plugins/chat/discourse/lib/fabricators"; +import ChatMessage from "discourse/plugins/chat/discourse/models/chat-message"; + +module("Discourse Chat | Unit | Models | chat-message", function () { + test(".persisted", function (assert) { + const channel = fabricators.channel(); + let message = ChatMessage.create(channel, { id: null }); + assert.strictEqual(message.persisted, false); + + message = ChatMessage.create(channel, { + id: 1, + staged: true, + }); + assert.strictEqual(message.persisted, false); + + message = ChatMessage.create(channel, { + id: 1, + staged: false, + }); + assert.strictEqual(message.persisted, true); + }); +});