From 79d2eb5beb926f804b40787583714d60b77e4643 Mon Sep 17 00:00:00 2001
From: David Battersby <info@davidbattersby.com>
Date: Fri, 11 Oct 2024 13:05:07 +0400
Subject: [PATCH] FEATURE: enable threading in chat DM channels (#29170)

Support threads in DMs and group chats so members can keep their conversations organized.

This change adds a new toggle switch for threads within the Chat Channel Settings screen. For new direct message channels threading is enabled by default.

We have made a decision to exclude direct message threads from the My Threads screen for now.
---
 .../chat/api/channels_controller.rb           |  8 ++---
 .../app/models/chat/direct_message_channel.rb |  1 +
 .../chat/routes/channel-info-settings.gjs     | 30 +++++++++++++++++--
 plugins/chat/lib/chat/guardian_extensions.rb  |  2 +-
 .../chat/spec/fabricators/chat_fabricator.rb  |  1 +
 .../spec/lib/chat/guardian_extensions_spec.rb |  4 +--
 .../chat/direct_message_channel_spec.rb       |  6 ++++
 .../spec/system/channel_settings_page_spec.rb | 17 +++++++++++
 8 files changed, 58 insertions(+), 11 deletions(-)

diff --git a/plugins/chat/app/controllers/chat/api/channels_controller.rb b/plugins/chat/app/controllers/chat/api/channels_controller.rb
index 6e6ca0ef379..56dc148ec8d 100644
--- a/plugins/chat/app/controllers/chat/api/channels_controller.rb
+++ b/plugins/chat/app/controllers/chat/api/channels_controller.rb
@@ -1,12 +1,8 @@
 # frozen_string_literal: true
 
 class Chat::Api::ChannelsController < Chat::ApiController
-  CHANNEL_EDITABLE_PARAMS ||= %i[name description slug]
-  CATEGORY_CHANNEL_EDITABLE_PARAMS ||= %i[
-    auto_join_users
-    allow_channel_wide_mentions
-    threading_enabled
-  ]
+  CHANNEL_EDITABLE_PARAMS ||= %i[name description slug threading_enabled]
+  CATEGORY_CHANNEL_EDITABLE_PARAMS ||= %i[auto_join_users allow_channel_wide_mentions]
 
   def index
     permitted = params.permit(:filter, :limit, :offset, :status)
diff --git a/plugins/chat/app/models/chat/direct_message_channel.rb b/plugins/chat/app/models/chat/direct_message_channel.rb
index 2e5a1b8e2a9..23eea91aa8b 100644
--- a/plugins/chat/app/models/chat/direct_message_channel.rb
+++ b/plugins/chat/app/models/chat/direct_message_channel.rb
@@ -3,6 +3,7 @@
 module Chat
   class DirectMessageChannel < Channel
     alias_method :direct_message, :chatable
+    before_validation(on: :create) { self.threading_enabled = true }
 
     def direct_message_channel?
       true
diff --git a/plugins/chat/assets/javascripts/discourse/components/chat/routes/channel-info-settings.gjs b/plugins/chat/assets/javascripts/discourse/components/chat/routes/channel-info-settings.gjs
index 292cfaf8098..75fea4dfed5 100644
--- a/plugins/chat/assets/javascripts/discourse/components/chat/routes/channel-info-settings.gjs
+++ b/plugins/chat/assets/javascripts/discourse/components/chat/routes/channel-info-settings.gjs
@@ -98,10 +98,14 @@ export default class ChatRouteChannelInfoSettings extends Component {
     return this.args.channel.isCategoryChannel && this.args.channel.isOpen;
   }
 
-  get toggleThreadingAvailable() {
+  get toggleThreadingCategoryChannel() {
     return this.args.channel.isCategoryChannel && this.args.channel.isOpen;
   }
 
+  get toggleThreadingDirectMessage() {
+    return this.args.channel.isDirectMessageChannel && this.args.channel.isOpen;
+  }
+
   get channelWideMentionsDescription() {
     return I18n.t("chat.settings.channel_wide_mentions_description", {
       channel: this.args.channel.title,
@@ -420,6 +424,28 @@ export default class ChatRouteChannelInfoSettings extends Component {
                   </:action>
                 </section.row>
               {{/if}}
+
+              {{#if this.toggleThreadingDirectMessage}}
+                <section.row @label={{this.toggleThreadingLabel}}>
+                  <:action>
+                    <DToggleSwitch
+                      @state={{@channel.threadingEnabled}}
+                      class="c-channel-settings__threading-switch"
+                      {{on
+                        "click"
+                        (fn
+                          this.onToggleThreadingEnabled
+                          @channel.threadingEnabled
+                        )
+                      }}
+                    />
+                  </:action>
+
+                  <:description>
+                    {{this.toggleThreadingDescription}}
+                  </:description>
+                </section.row>
+              {{/if}}
             </form.section>
           {{/if}}
 
@@ -482,7 +508,7 @@ export default class ChatRouteChannelInfoSettings extends Component {
                 </section.row>
               {{/if}}
 
-              {{#if this.toggleThreadingAvailable}}
+              {{#if this.toggleThreadingCategoryChannel}}
                 <section.row @label={{this.toggleThreadingLabel}}>
                   <:action>
                     <DToggleSwitch
diff --git a/plugins/chat/lib/chat/guardian_extensions.rb b/plugins/chat/lib/chat/guardian_extensions.rb
index 4fc03f8ef67..760bb6e9217 100644
--- a/plugins/chat/lib/chat/guardian_extensions.rb
+++ b/plugins/chat/lib/chat/guardian_extensions.rb
@@ -44,7 +44,7 @@ module Chat
     # name and description can be edited.
     def can_edit_chat_channel?(channel)
       if channel.direct_message_channel?
-        channel.chatable.group && (is_staff? || channel.chatable.user_can_access?(@user))
+        is_staff? || channel.chatable.user_can_access?(@user)
       elsif channel.category_channel?
         is_staff?
       end
diff --git a/plugins/chat/spec/fabricators/chat_fabricator.rb b/plugins/chat/spec/fabricators/chat_fabricator.rb
index 7aad2439486..0a26dabfc40 100644
--- a/plugins/chat/spec/fabricators/chat_fabricator.rb
+++ b/plugins/chat/spec/fabricators/chat_fabricator.rb
@@ -43,6 +43,7 @@ Fabricator(:direct_message_channel, from: :chat_channel) do
   end
   status { :open }
   name nil
+  threading_enabled true
   after_create do |channel, attrs|
     if attrs[:with_membership]
       channel.chatable.users.each do |user|
diff --git a/plugins/chat/spec/lib/chat/guardian_extensions_spec.rb b/plugins/chat/spec/lib/chat/guardian_extensions_spec.rb
index 3b60bc1766d..87ea025d789 100644
--- a/plugins/chat/spec/lib/chat/guardian_extensions_spec.rb
+++ b/plugins/chat/spec/lib/chat/guardian_extensions_spec.rb
@@ -87,9 +87,9 @@ RSpec.describe Chat::GuardianExtensions do
         end
 
         context "when not group" do
-          it "doesn’t allow  to edit the channel" do
+          it "allows to edit the channel" do
             Chat::DirectMessageUser.create(user: user, direct_message: dm_channel.chatable)
-            expect(user.guardian.can_edit_chat_channel?(dm_channel)).to eq(false)
+            expect(user.guardian.can_edit_chat_channel?(dm_channel)).to eq(true)
           end
         end
       end
diff --git a/plugins/chat/spec/models/chat/direct_message_channel_spec.rb b/plugins/chat/spec/models/chat/direct_message_channel_spec.rb
index 83a9642a1e2..2603678f648 100644
--- a/plugins/chat/spec/models/chat/direct_message_channel_spec.rb
+++ b/plugins/chat/spec/models/chat/direct_message_channel_spec.rb
@@ -52,6 +52,12 @@ RSpec.describe Chat::DirectMessageChannel do
     end
   end
 
+  describe "#threading_enabled" do
+    it "defaults to true" do
+      expect(channel.threading_enabled).to be(true)
+    end
+  end
+
   describe "#title" do
     subject(:title) { channel.title(user) }
 
diff --git a/plugins/chat/spec/system/channel_settings_page_spec.rb b/plugins/chat/spec/system/channel_settings_page_spec.rb
index 7c0cc3bab11..270896c5e4e 100644
--- a/plugins/chat/spec/system/channel_settings_page_spec.rb
+++ b/plugins/chat/spec/system/channel_settings_page_spec.rb
@@ -166,6 +166,23 @@ RSpec.describe "Channel - Info - Settings page", type: :system do
         ).to eq(false)
       end
     end
+
+    context "when direct message channel" do
+      fab!(:channel_1) do
+        Fabricate(:direct_message_channel, users: [current_user, Fabricate(:user)])
+      end
+
+      before { channel_1.add(current_user) }
+
+      it "can toggle threading" do
+        chat_page.visit_channel_settings(channel_1)
+
+        expect {
+          PageObjects::Components::DToggleSwitch.new(".c-channel-settings__threading-switch").toggle
+          expect(toasts).to have_success(I18n.t("js.saved"))
+        }.to change { channel_1.reload.threading_enabled }.from(true).to(false)
+      end
+    end
   end
 
   context "as staff" do