From 76953cc3561299d84931f0b77e5d99cd21f63f57 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Wed, 6 Mar 2024 12:03:42 +0100 Subject: [PATCH] FEATURE: allows to force a thread (#25987) Forcing a thread will work even in channel which don't have `threading_enabled` or in direct message channels. For now this feature is only available through the `ChatSDK`: ```ruby ChatSDK::Message.create(in_reply_to_id: 1, guardian: guardian, raw: "foo bar baz", channel_id: 2, force_thread: true) ``` --- lib/svg_sprite.rb | 1 - .../chat/api/channel_messages_controller.rb | 3 ++ plugins/chat/app/models/chat/message.rb | 2 +- .../chat/app/queries/chat/messages_query.rb | 40 +++++++++++++------ .../app/queries/chat/thread_unreads_query.rb | 2 +- .../serializers/chat/message_serializer.rb | 12 +++--- .../app/serializers/chat/thread_serializer.rb | 6 +-- .../chat/app/services/chat/create_message.rb | 4 +- .../services/chat/list_channel_messages.rb | 7 ++-- .../chat/list_channel_thread_messages.rb | 2 +- .../chat/app/services/chat/lookup_thread.rb | 2 +- plugins/chat/app/services/chat/publisher.rb | 11 ++--- .../discourse/components/chat-message.gjs | 3 +- .../components/chat/thread/header.gjs | 3 +- .../discourse/models/chat-thread.js | 2 + .../discourse/routes/chat-channel-thread.js | 31 +++++++------- .../discourse/routes/chat-channel.js | 7 +++- .../services/chat-subscriptions-manager.js | 2 +- .../stylesheets/common/chat-navbar.scss | 4 ++ .../20240301100413_add_force_to_threads.rb | 7 ++++ plugins/chat/lib/chat_sdk/message.rb | 2 + .../chat/spec/lib/chat_sdk/message_spec.rb | 10 +++++ .../api/channel_messages_controller_spec.rb | 19 +++++++++ .../spec/services/chat/create_message_spec.rb | 15 +++++++ .../chat/list_channel_messages_spec.rb | 25 ++++++++++-- .../chat/list_channel_thread_messages_spec.rb | 6 +++ .../spec/services/chat/lookup_thread_spec.rb | 6 +++ .../chat/spec/services/chat/publisher_spec.rb | 1 + .../chat/spec/system/single_thread_spec.rb | 13 ++++++ .../assets/svg-icons/discourse-additional.svg | 3 -- 30 files changed, 191 insertions(+), 60 deletions(-) create mode 100644 plugins/chat/db/migrate/20240301100413_add_force_to_threads.rb diff --git a/lib/svg_sprite.rb b/lib/svg_sprite.rb index e099fc736ec..63fe74c0195 100644 --- a/lib/svg_sprite.rb +++ b/lib/svg_sprite.rb @@ -77,7 +77,6 @@ module SvgSprite discourse-other-tab discourse-sparkles discourse-threads - discourse-sidebar download ellipsis-h ellipsis-v diff --git a/plugins/chat/app/controllers/chat/api/channel_messages_controller.rb b/plugins/chat/app/controllers/chat/api/channel_messages_controller.rb index 80f614ca3b5..c57b52820a9 100644 --- a/plugins/chat/app/controllers/chat/api/channel_messages_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channel_messages_controller.rb @@ -31,6 +31,9 @@ class Chat::Api::ChannelMessagesController < Chat::ApiController end def create + # users can't force a thread through JSON API + params[:force_thread] = false + Chat::MessageRateLimiter.run!(current_user) with_service(Chat::CreateMessage) do diff --git a/plugins/chat/app/models/chat/message.rb b/plugins/chat/app/models/chat/message.rb index 8fd52263378..bd8d5b96955 100644 --- a/plugins/chat/app/models/chat/message.rb +++ b/plugins/chat/app/models/chat/message.rb @@ -279,7 +279,7 @@ module Chat end def thread_om? - in_thread? && self.thread.original_message_id == self.id + in_thread? && self.thread&.original_message_id == self.id end def parsed_mentions diff --git a/plugins/chat/app/queries/chat/messages_query.rb b/plugins/chat/app/queries/chat/messages_query.rb index b96b2be24cd..ff4f48242d9 100644 --- a/plugins/chat/app/queries/chat/messages_query.rb +++ b/plugins/chat/app/queries/chat/messages_query.rb @@ -52,13 +52,23 @@ module Chat include_thread_messages = true messages = messages.where(thread_id: thread_id) end - messages = messages.where(<<~SQL, channel_id: channel.id) if !include_thread_messages - chat_messages.thread_id IS NULL OR chat_messages.id IN ( - SELECT original_message_id - FROM chat_threads - WHERE chat_threads.channel_id = :channel_id - ) - SQL + + if include_thread_messages + if !thread_id.present? + messages = + messages.left_joins(:thread).where( + "chat_threads.id IS NULL OR chat_threads.force = false OR chat_messages.id = chat_threads.original_message_id", + ) + end + else + messages = messages.where(<<~SQL, channel_id: channel.id) + chat_messages.thread_id IS NULL OR chat_messages.id IN ( + SELECT original_message_id + FROM chat_threads + WHERE chat_threads.channel_id = :channel_id + ) + SQL + end if target_message_id.present? && direction.blank? query_around_target(target_message_id, channel, messages) @@ -100,14 +110,14 @@ module Chat past_messages = messages - .where("created_at < ?", target_message.created_at) + .where("chat_messages.created_at < ?", target_message.created_at) .order(created_at: :desc) .limit(PAST_MESSAGE_LIMIT) .to_a future_messages = messages - .where("created_at > ?", target_message.created_at) + .where("chat_messages.created_at > ?", target_message.created_at) .order(created_at: :asc) .limit(FUTURE_MESSAGE_LIMIT) .to_a @@ -135,11 +145,15 @@ module Chat if target_message_id.present? condition = direction == PAST ? "<" : ">" - messages = messages.where("id #{condition} ?", target_message_id.to_i) + messages = messages.where("chat_messages.id #{condition} ?", target_message_id.to_i) end order = direction == FUTURE ? "ASC" : "DESC" - messages = messages.order("created_at #{order}, id #{order}").limit(page_size).to_a + messages = + messages + .order("chat_messages.created_at #{order}, chat_messages.id #{order}") + .limit(page_size) + .to_a if direction == FUTURE can_load_more_future = messages.size == page_size @@ -161,14 +175,14 @@ module Chat def self.query_by_date(target_date, channel, messages) past_messages = messages - .where("created_at <= ?", target_date.to_time.utc) + .where("chat_messages.created_at <= ?", target_date.to_time.utc) .order(created_at: :desc) .limit(PAST_MESSAGE_LIMIT) .to_a future_messages = messages - .where("created_at > ?", target_date.to_time.utc) + .where("chat_messages.created_at > ?", target_date.to_time.utc) .order(created_at: :asc) .limit(FUTURE_MESSAGE_LIMIT) .to_a diff --git a/plugins/chat/app/queries/chat/thread_unreads_query.rb b/plugins/chat/app/queries/chat/thread_unreads_query.rb index 503f63edfdb..73841f09721 100644 --- a/plugins/chat/app/queries/chat/thread_unreads_query.rb +++ b/plugins/chat/app/queries/chat/thread_unreads_query.rb @@ -53,7 +53,7 @@ module Chat AND chat_messages.deleted_at IS NULL AND chat_messages.thread_id IS NOT NULL AND chat_messages.id != chat_threads.original_message_id - AND chat_channels.threading_enabled + AND (chat_channels.threading_enabled OR chat_threads.force = true) AND user_chat_thread_memberships.notification_level NOT IN (:quiet_notification_levels) AND original_message.deleted_at IS NULL AND user_chat_channel_memberships.muted = false diff --git a/plugins/chat/app/serializers/chat/message_serializer.rb b/plugins/chat/app/serializers/chat/message_serializer.rb index acccb091432..5c43a6062f9 100644 --- a/plugins/chat/app/serializers/chat/message_serializer.rb +++ b/plugins/chat/app/serializers/chat/message_serializer.rb @@ -182,20 +182,20 @@ module Chat end end - def include_thread? - include_thread_id? && object.thread_om? && object.thread.present? + def include_thread_id? + channel.threading_enabled || object.thread&.force end - def include_thread_id? - channel.threading_enabled + def include_thread? + include_thread_id? && object.thread_om? && object.thread.present? end def thread Chat::ThreadSerializer.new( object.thread, scope: scope, - membership: @options[:thread_memberships]&.find { |m| m.thread_id == object.thread.id }, - participants: @options[:thread_participants]&.dig(object.thread.id), + membership: @options[:thread_memberships]&.find { |m| m.thread_id == object.thread&.id }, + participants: @options[:thread_participants]&.dig(object.thread&.id), include_thread_preview: true, include_thread_original_message: @options[:include_thread_original_message], root: false, diff --git a/plugins/chat/app/serializers/chat/thread_serializer.rb b/plugins/chat/app/serializers/chat/thread_serializer.rb index 77bae3375c3..b123a08c779 100644 --- a/plugins/chat/app/serializers/chat/thread_serializer.rb +++ b/plugins/chat/app/serializers/chat/thread_serializer.rb @@ -13,14 +13,14 @@ module Chat :reply_count, :current_user_membership, :preview, - :last_message_id + :last_message_id, + :force def initialize(object, opts) super(object, opts) @opts = opts - # Avoids an N1 to re-load the thread in the serializer for original_message. - object.original_message&.thread = object + object&.original_message&.thread = object @current_user_membership = opts[:membership] end diff --git a/plugins/chat/app/services/chat/create_message.rb b/plugins/chat/app/services/chat/create_message.rb index a0e06cd0437..8fcb13c26cc 100644 --- a/plugins/chat/app/services/chat/create_message.rb +++ b/plugins/chat/app/services/chat/create_message.rb @@ -62,6 +62,7 @@ module Chat attribute :enforce_membership, :boolean, default: false attribute :incoming_chat_webhook attribute :process_inline, :boolean, default: Rails.env.test? + attribute :force_thread, :boolean, default: false validates :chat_channel_id, presence: true validates :message, presence: true, if: -> { upload_ids.blank? } @@ -112,6 +113,7 @@ module Chat original_message: reply, original_message_user: reply.user, channel: channel, + force: contract.force_thread, ) end @@ -185,7 +187,7 @@ module Chat end def publish_new_thread(reply:, contract:, channel:, thread:, **) - return unless channel.threading_enabled? + return unless channel.threading_enabled? || thread&.force return unless reply&.thread_id_previously_changed?(from: nil) Chat::Publisher.publish_thread_created!(channel, reply, thread.id) end diff --git a/plugins/chat/app/services/chat/list_channel_messages.rb b/plugins/chat/app/services/chat/list_channel_messages.rb index e5faca0095e..30d5fcce1f0 100644 --- a/plugins/chat/app/services/chat/list_channel_messages.rb +++ b/plugins/chat/app/services/chat/list_channel_messages.rb @@ -115,7 +115,8 @@ module Chat context.target_message_id = messages_data[:target_message_id] messages_data[:target_message] = ( - if enabled_threads && messages_data[:target_message]&.thread_reply? + if messages_data[:target_message]&.thread_reply? && + (enabled_threads || messages_data[:target_message].thread&.force) [] else [messages_data[:target_message]] @@ -130,10 +131,10 @@ module Chat ].flatten.compact end - def fetch_tracking(guardian:, enabled_threads:, **) + def fetch_tracking(guardian:, **) context.tracking = {} - return if !enabled_threads || !context.thread_ids.present? + return if !context.thread_ids.present? context.tracking = ::Chat::TrackingStateReportQuery.call( diff --git a/plugins/chat/app/services/chat/list_channel_thread_messages.rb b/plugins/chat/app/services/chat/list_channel_thread_messages.rb index 5fedc020474..9aa02ef4ee5 100644 --- a/plugins/chat/app/services/chat/list_channel_thread_messages.rb +++ b/plugins/chat/app/services/chat/list_channel_thread_messages.rb @@ -61,7 +61,7 @@ module Chat end def ensure_thread_enabled(thread:, **) - thread.channel.threading_enabled + thread.channel.threading_enabled || thread.force end def can_view_thread(guardian:, thread:, **) diff --git a/plugins/chat/app/services/chat/lookup_thread.rb b/plugins/chat/app/services/chat/lookup_thread.rb index 789665ae588..b9fffe21769 100644 --- a/plugins/chat/app/services/chat/lookup_thread.rb +++ b/plugins/chat/app/services/chat/lookup_thread.rb @@ -46,7 +46,7 @@ module Chat end def threading_enabled_for_channel(thread:, **) - thread.channel.threading_enabled + thread.channel.threading_enabled || thread.force end def fetch_membership(thread:, guardian:, **) diff --git a/plugins/chat/app/services/chat/publisher.rb b/plugins/chat/app/services/chat/publisher.rb index f960f679abe..a368fa2f041 100644 --- a/plugins/chat/app/services/chat/publisher.rb +++ b/plugins/chat/app/services/chat/publisher.rb @@ -15,7 +15,7 @@ module Chat end def self.calculate_publish_targets(channel, message) - return [root_message_bus_channel(channel.id)] if !allow_publish_to_thread?(channel) + return [root_message_bus_channel(channel.id)] if !allow_publish_to_thread?(channel, message) if message.thread_om? [ @@ -30,8 +30,8 @@ module Chat end end - def self.allow_publish_to_thread?(channel) - channel.threading_enabled + def self.allow_publish_to_thread?(channel, message) + channel.threading_enabled || message.thread&.force end def self.publish_new!(chat_channel, chat_message, staged_id) @@ -42,7 +42,7 @@ module Chat serialize_message_with_type(chat_message, :sent).merge(staged_id: staged_id), ) - if !chat_message.thread_reply? || !allow_publish_to_thread?(chat_channel) + if !chat_message.thread_reply? || !allow_publish_to_thread?(chat_channel, chat_message) MessageBus.publish( self.new_messages_message_bus_channel(chat_channel.id), { @@ -59,13 +59,14 @@ module Chat ) end - if chat_message.thread_reply? && allow_publish_to_thread?(chat_channel) + if chat_message.thread_reply? && allow_publish_to_thread?(chat_channel, chat_message) MessageBus.publish( self.new_messages_message_bus_channel(chat_channel.id), { type: "thread", channel_id: chat_channel.id, thread_id: chat_message.thread_id, + force_thread: chat_message.thread&.force, message: Chat::MessageSerializer.new( chat_message, diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-message.gjs b/plugins/chat/assets/javascripts/discourse/components/chat-message.gjs index 8a127734b4e..085aee51338 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-message.gjs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-message.gjs @@ -466,7 +466,8 @@ export default class ChatMessage extends Component { get threadingEnabled() { return ( - this.args.message?.channel?.threadingEnabled && + (this.args.message?.channel?.threadingEnabled || + this.args.message?.thread?.force) && !!this.args.message?.thread ); } diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/thread/header.gjs b/plugins/chat/assets/javascripts/discourse/components/chat/thread/header.gjs index 8a7ac4fba5b..b1c2e2ac979 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat/thread/header.gjs +++ b/plugins/chat/assets/javascripts/discourse/components/chat/thread/header.gjs @@ -3,6 +3,7 @@ import { inject as service } from "@ember/service"; import replaceEmoji from "discourse/helpers/replace-emoji"; import icon from "discourse-common/helpers/d-icon"; import I18n from "discourse-i18n"; +import and from "truth-helpers/helpers/and"; import Navbar from "discourse/plugins/chat/discourse/components/chat/navbar"; import ChatThreadHeaderUnreadIndicator from "discourse/plugins/chat/discourse/components/chat/thread/header-unread-indicator"; @@ -54,7 +55,7 @@ export default class ChatThreadHeader extends Component {