FIX: load existing chat dm channel via url (#26998)

When users click a link that points to an existing group chat, we should reopen that chat instead of creating a new group chat so users can more easily continue ongoing conversations.

plugins/chat/app/models/chat/direct_message.rb

@@ -17,9 +17,9 @@ module Chat
     class << self
       def polymorphic_class_mapping = { "DirectMessage" => Chat::DirectMessage }
 
-      def for_user_ids(user_ids)
+      def for_user_ids(user_ids, group: false)
         joins(:users)
-          .where(group: false)
+          .where(group: group)
           .group("direct_message_channels.id")
           .having("ARRAY[?] = ARRAY_AGG(users.id ORDER BY users.id)", user_ids.sort)
           .first

plugins/chat/app/services/chat/create_direct_message_channel.rb

@@ -20,6 +20,7 @@ module Chat
     #   @param [Hash] params_to_create
     #   @option params_to_create [Array<String>] target_usernames
     #   @option params_to_create [Array<String>] target_groups
+    #   @option params_to_create [Boolean] upsert
     #   @return [Service::Base::Context]
 
     contract
@@ -42,6 +43,7 @@ module Chat
       attribute :name, :string
       attribute :target_usernames, :array
       attribute :target_groups, :array
+      attribute :upsert, :boolean, default: false
 
       validate :target_presence
 
@@ -78,11 +80,13 @@ module Chat
 
     def fetch_or_create_direct_message(target_users:, contract:)
       ids = target_users.map(&:id)
+      is_group = ids.size > 2 || contract.name.present?
 
-      if ids.size > 2 || contract.name.present?
+      if contract.upsert || !is_group
-        ::Chat::DirectMessage.create(user_ids: ids, group: true)
+        ::Chat::DirectMessage.for_user_ids(ids, group: is_group) ||
+          ::Chat::DirectMessage.create(user_ids: ids, group: is_group)
       else
-        ::Chat::DirectMessage.for_user_ids(ids) || ::Chat::DirectMessage.create(user_ids: ids)
+        ::Chat::DirectMessage.create(user_ids: ids, group: is_group)
       end
     end
 

plugins/chat/assets/javascripts/discourse/components/chat/message-creator/new-group.gjs

@@ -40,9 +40,9 @@ export default class NewGroup extends Component {
         .filter((member) => member.type === "group")
         .mapBy("model.name");
 
-      const channel = await this.chat.upsertDmChannel(
+      const channel = await this.chat.createDmChannel(
         { usernames, groups },
-        this.newGroupTitle
+        { name: this.newGroupTitle }
       );
 
       if (!channel) {

plugins/chat/assets/javascripts/discourse/services/chat.js

@@ -355,14 +355,17 @@ export default class Chat extends Service {
   // channel for. The current user will automatically be included in the channel when it is created.
   // @param {array} [targets.usernames] - The usernames to include in the direct message channel.
   // @param {array} [targets.groups] - The groups to include in the direct message channel.
-  // @param {string|null} [name=null] - Optional name for the direct message channel.
+  // @param {object} opts - Optional values when fetching or creating the direct message channel.
-  upsertDmChannel(targets, name = null) {
+  // @param {string|null} [opts.name] - Name for the direct message channel.
+  // @param {boolean} [opts.upsert] - Should we attempt to fetch existing channel before creating a new one.
+  createDmChannel(targets, opts = { name: null, upsert: false }) {
     return ajax("/chat/api/direct-message-channels.json", {
       method: "POST",
       data: {
         target_usernames: targets.usernames?.uniq(),
         target_groups: targets.groups?.uniq(),
-        name,
+        upsert: opts.upsert,
+        name: opts.name,
       },
     })
       .then((response) => {
@@ -373,6 +376,10 @@ export default class Chat extends Service {
       .catch(popupAjaxError);
   }
 
+  upsertDmChannel(targets, name = null) {
+    return this.createDmChannel(targets, { name, upsert: true });
+  }
+
   // @param {array} usernames - The usernames to fetch the direct message
   // channel for. The current user will automatically be included as a
   // participant to fetch the channel for.

plugins/chat/spec/requests/chat/api/direct_messages_controller_spec.rb

@@ -13,7 +13,7 @@ RSpec.describe Chat::Api::DirectMessagesController do
   end
 
   def create_dm_channel(user_ids)
-    direct_messages_channel = Chat::DirectMessage.create!
+    direct_messages_channel = Chat::DirectMessage.create!(group: user_ids.length > 2)
     user_ids.each do |user_id|
       direct_messages_channel.direct_message_users.create!(user_id: user_id)
     end
@@ -65,7 +65,7 @@ RSpec.describe Chat::Api::DirectMessagesController do
       end
     end
 
-    describe "dm with two other users" do
+    describe "dm with multiple users" do
       let(:usernames) { [user1, user2, user3].map(&:username) }
       let(:direct_message_user_ids) { [current_user.id, user1.id, user2.id, user3.id] }
 
@@ -78,13 +78,25 @@ RSpec.describe Chat::Api::DirectMessagesController do
         )
       end
 
-      it "createsa new dm channel" do
+      it "creates a new dm channel" do
         create_dm_channel(direct_message_user_ids)
 
         expect {
           post "/chat/api/direct-message-channels.json", params: { target_usernames: [usernames] }
         }.to change { Chat::DirectMessage.count }.by(1)
       end
+
+      it "returns existing dm channel when upsert is true" do
+        create_dm_channel(direct_message_user_ids)
+
+        expect {
+          post "/chat/api/direct-message-channels.json",
+               params: {
+                 target_usernames: [usernames],
+                 upsert: true,
+               }
+        }.not_to change { Chat::DirectMessage.count }
+      end
     end
 
     it "creates Chat::UserChatChannelMembership records" do

plugins/chat/spec/system/chat_new_message_spec.rb

@@ -35,12 +35,38 @@ RSpec.describe "Chat New Message from params", type: :system do
   end
 
   context "with multiple users" do
-    it "creates a dm channel with multiple users" do
+    fab!(:group_dm) do
-      chat_page.visit_new_message([user_1, user_2])
+      Fabricate(:direct_message_channel, users: [current_user, user_1, user_2], group: true)
+    end
+    fab!(:user_3) { Fabricate(:user) }
 
+    it "loads existing dm channel when one exists" do
+      expect { chat_page.visit_new_message([user_1, user_2]) }.not_to change { Chat::Channel.count }
       users = [user_1.username, user_2.username].permutation.map { |u| u.join("-") }.join("|")
 
-      expect(page).to have_current_path(%r{/chat/c/(#{users})/#{Chat::Channel.last.id}})
+      expect(page).to have_current_path(%r{/chat/c/(#{users})/#{group_dm.id}})
+    end
+
+    it "creates a dm channel when none exists" do
+      expect { chat_page.visit_new_message([user_1, user_3]) }.to change { Chat::Channel.count }.by(
+        1,
+      )
+
+      expect(page).to have_current_path(
+        "/chat/c/#{user_1.username}-#{user_3.username}/#{Chat::Channel.last.id}",
+      )
+    end
+
+    context "when user has chat disabled" do
+      before { user_3.user_option.update!(chat_enabled: false) }
+
+      it "loads channel without the chat disabled user" do
+        expect { chat_page.visit_new_message([user_1, user_3]) }.not_to change {
+          Chat::Channel.count
+        }
+
+        expect(page).to have_current_path("/chat/c/#{user_1.username}/#{user_1_channel.id}")
+      end
     end
   end
 end