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)
```
This commit is contained in:
Joffrey JAFFEUX 2024-03-06 12:03:42 +01:00 committed by GitHub
parent 898b71da88
commit 76953cc356
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
30 changed files with 191 additions and 60 deletions

View File

@ -77,7 +77,6 @@ module SvgSprite
discourse-other-tab
discourse-sparkles
discourse-threads
discourse-sidebar
download
ellipsis-h
ellipsis-v

View File

@ -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

View File

@ -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

View File

@ -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
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

View File

@ -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

View File

@ -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,

View File

@ -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

View File

@ -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

View File

@ -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(

View File

@ -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:, **)

View File

@ -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:, **)

View File

@ -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,

View File

@ -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
);
}

View File

@ -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 {
<template>
<Navbar @showFullTitle={{@showFullTitle}} as |navbar|>
{{#if @thread}}
{{#if (and this.channel.threadingEnabled @thread)}}
<navbar.BackButton
@route={{this.backLink.route}}
@routeModels={{this.backLink.models}}

View File

@ -32,6 +32,7 @@ export default class ChatThread {
@tracked tracking;
@tracked currentUserMembership;
@tracked preview;
@tracked force;
messagesManager = new ChatMessagesManager(getOwnerWithFallback(this));
@ -41,6 +42,7 @@ export default class ChatThread {
this.status = args.status;
this.staged = args.staged;
this.replyCount = args.reply_count;
this.force = args.force;
this.originalMessage = args.original_message
? ChatMessage.create(channel, args.original_message)

View File

@ -8,20 +8,31 @@ export default class ChatChannelThread extends DiscourseRoute {
@service chat;
@service chatThreadPane;
redirectToChannel(channel, transition) {
transition.abort();
this.chatStateManager.closeSidePanel();
this.router.transitionTo("chat.channel", ...channel.routeModels);
}
model(params, transition) {
const channel = this.modelFor("chat.channel");
return channel.threadsManager
.find(channel.id, params.threadId)
.catch(() => {
transition.abort();
this.chatStateManager.closeSidePanel();
this.router.transitionTo("chat.channel", ...channel.routeModels);
this.redirectToChannel(channel, transition);
return;
});
}
afterModel(model) {
this.chat.activeChannel.activeThread = model;
afterModel(thread, transition) {
const channel = this.modelFor("chat.channel");
if (!channel.threadingEnabled && !thread.force) {
this.redirectToChannel(channel, transition);
return;
}
this.chat.activeChannel.activeThread = thread;
}
@action
@ -36,15 +47,7 @@ export default class ChatChannelThread extends DiscourseRoute {
}
}
beforeModel(transition) {
const channel = this.modelFor("chat.channel");
if (!channel.threadingEnabled) {
transition.abort();
this.router.transitionTo("chat.channel", ...channel.routeModels);
return;
}
beforeModel() {
const { messageId } = this.paramsFor(this.routeName + ".near-message");
if (
!messageId &&

View File

@ -15,7 +15,12 @@ export default class ChatChannelRoute extends DiscourseRoute {
const messageId = this.paramsFor("chat.channel.near-message").messageId;
const threadId = this.paramsFor("chat.channel.thread").threadId;
if (!messageId && !threadId && model.threadsManager.unreadThreadCount > 0) {
if (
model.threadingEnabled &&
!messageId &&
!threadId &&
model.threadsManager.unreadThreadCount > 0
) {
this.router.transitionTo("chat.channel.threads", ...model.routeModels);
}
}

View File

@ -226,7 +226,7 @@ export default class ChatSubscriptionsManager extends Service {
_onNewThreadMessage(busData) {
this.chatChannelsManager.find(busData.channel_id).then((channel) => {
if (!channel.threadingEnabled) {
if (!channel.threadingEnabled && !busData.force_thread) {
return;
}

View File

@ -36,6 +36,10 @@
.single-select-header {
padding: 0.3675rem 0.584rem;
}
> .c-navbar__title:first-child {
margin-left: 1rem;
}
}
.c-navbar__back-button {

View File

@ -0,0 +1,7 @@
# frozen_string_literal: true
class AddForceToThreads < ActiveRecord::Migration[7.0]
def change
add_column :chat_threads, :force, :boolean, null: false, default: false
end
end

View File

@ -113,6 +113,7 @@ module ChatSDK
upload_ids: nil,
streaming: false,
enforce_membership: false,
force_thread: false,
&block
)
message =
@ -126,6 +127,7 @@ module ChatSDK
upload_ids: upload_ids,
streaming: streaming,
enforce_membership: enforce_membership,
force_thread: force_thread,
) do
on_model_not_found(:channel) { raise "Couldn't find channel with id: `#{channel_id}`" }
on_model_not_found(:channel_membership) do

View File

@ -27,6 +27,16 @@ describe ChatSDK::Message do
end
end
context "when force_thread is present" do
it "creates the message in a thread" do
message_1 = described_class.create(**params)
message_2 =
described_class.create(**params, in_reply_to_id: message_1.id, force_thread: true)
expect(message_2.thread.force).to eq(true)
end
end
context "when channel doesnt exist" do
it "fails" do
expect { described_class.create(**params, channel_id: -999) }.to raise_error(

View File

@ -67,4 +67,23 @@ RSpec.describe Chat::Api::ChannelMessagesController do
end
end
end
describe "#create" do
context "when force_thread param is given" do
it "removes it from params" do
sign_in(current_user)
message_1 = Fabricate(:chat_message, chat_channel: channel)
expect {
post "/chat/#{channel.id}.json",
params: {
in_reply_to_id: message_1.id,
message: "test",
force_thread: true,
}
}.to change { Chat::Thread.where(force: false).count }.by(1)
end
end
end
end

View File

@ -42,6 +42,7 @@ RSpec.describe Chat::CreateMessage do
upload_ids: [upload.id],
context_topic_id: context_topic_id,
context_post_ids: context_post_ids,
force_thread: false,
}
end
let(:message) { result[:message_instance].reload }
@ -323,6 +324,20 @@ RSpec.describe Chat::CreateMessage do
Chat::Publisher.expects(:publish_thread_created!).never
result
end
context "when thread is forced" do
before { params[:force_thread] = true }
it "publishes the new thread" do
Chat::Publisher.expects(:publish_thread_created!).with(
channel,
reply_to,
instance_of(Integer),
nil,
)
result
end
end
end
end
end

View File

@ -159,10 +159,29 @@ RSpec.describe Chat::ListChannelMessages do
context "when threads are disabled" do
fab!(:thread_1) { Fabricate(:chat_thread, channel: channel) }
before { channel.update!(threading_enabled: false) }
before do
channel.update!(threading_enabled: false)
thread_1.add(user)
end
it "returns empty tracking" do
expect(result.tracking).to eq({})
it "returns tracking" do
Fabricate(:chat_message, chat_channel: channel, thread: thread_1)
expect(result.tracking.thread_tracking).to eq(
{ thread_1.id => { channel_id: channel.id, mention_count: 0, unread_count: 0 } },
)
end
context "when thread is forced" do
before { thread_1.update!(force: true) }
it "returns tracking" do
Fabricate(:chat_message, chat_channel: channel, thread: thread_1)
expect(result.tracking.thread_tracking).to eq(
{ thread_1.id => { channel_id: channel.id, mention_count: 0, unread_count: 1 } },
)
end
end
end

View File

@ -42,6 +42,12 @@ RSpec.describe Chat::ListChannelThreadMessages do
before { thread.channel.update!(threading_enabled: false) }
it { is_expected.to fail_a_policy(:ensure_thread_enabled) }
context "when the thread is forced" do
before { thread.update!(force: true) }
it { is_expected.to be_a_success }
end
end
context "when channel and site setting are enabling threading" do

View File

@ -56,6 +56,12 @@ RSpec.describe Chat::LookupThread do
before { channel.update!(threading_enabled: false) }
it { is_expected.to fail_a_policy(:threading_enabled_for_channel) }
context "when thread is forced" do
before { thread.update!(force: true) }
it { is_expected.to be_a_success }
end
end
end
end

View File

@ -281,6 +281,7 @@ describe Chat::Publisher do
type: "thread",
channel_id: channel.id,
thread_id: thread.id,
force_thread: false,
message:
Chat::MessageSerializer.new(
message_1,

View File

@ -82,6 +82,19 @@ describe "Single thread in side panel", type: :system do
expect(chat_drawer_page).to have_open_channel(channel)
end
context "when thread is forced and threading disabled" do
before do
channel.update!(threading_enabled: false)
thread.update!(force: true)
end
it "doesnt show back button " do
chat_page.visit_thread(thread)
expect(page).to have_no_css(".c-routes-channel-thread .c-navbar__back-button")
end
end
it "highlights the message in the channel when clicking original message link" do
chat_page.visit_thread(thread)

View File

@ -73,7 +73,4 @@ Additional SVG icons
<path d="M26 46.5156C37.3305 46.5156 46.5156 37.3305 46.5156 26C46.5156 14.6695 37.3305 5.48438 26 5.48438C14.6695 5.48438 5.48438 14.6695 5.48438 26C5.48438 37.3305 14.6695 46.5156 26 46.5156ZM26 52C40.3594 52 52 40.3594 52 26C52 11.6406 40.3594 0 26 0C11.6406 0 0 11.6406 0 26C0 40.3594 11.6406 52 26 52Z"/>
<path d="M33.3125 22.75H18.6875C16.8926 22.75 15.4375 24.2051 15.4375 26C15.4375 27.7949 16.8926 29.25 18.6875 29.25H33.3125C35.1074 29.25 36.5625 27.7949 36.5625 26C36.5625 24.2051 35.1074 22.75 33.3125 22.75Z"/>
</symbol>
<symbol id="discourse-sidebar" viewBox="0 0 513 513">
<path fill-rule="evenodd" clip-rule="evenodd" d="M64.125 0C28.0547 0 0 29.0566 0 64.125V399.75C0 435.82 28.0547 463.875 64.125 463.875H448.875C483.943 463.875 513 435.82 513 399.75V64.125C513 29.0566 483.943 0 448.875 0H64.125ZM219.375 407.75H56.125V56.1875H219.375V407.75ZM272.5 56.1875H455.875V407.75H272.5V56.1875ZM96 96C91.5817 96 88 99.5817 88 104V128C88 132.418 91.5817 136 96 136H184C188.418 136 192 132.418 192 128V104C192 99.5817 188.418 96 184 96H96ZM88 184C88 179.582 91.5817 176 96 176H184C188.418 176 192 179.582 192 184V208C192 212.418 188.418 216 184 216H96C91.5817 216 88 212.418 88 208V184ZM96 256C91.5817 256 88 259.582 88 264V288C88 292.418 91.5817 296 96 296H184C188.418 296 192 292.418 192 288V264C192 259.582 188.418 256 184 256H96Z"/>
</symbol>
</svg>

Before

Width:  |  Height:  |  Size: 12 KiB

After

Width:  |  Height:  |  Size: 11 KiB