FIX: skip 1:1s when chat search returns users (#28464)

When we use CTRL/CMD + K to search, on the server we run 4 queries:

- 1 for users
- 1 for groups
- 1 for category channels
- 1 for direct message channels

The server returns up to 10 results per type and the client concatenate all the results (in the order I described) and then takes the first 10 or so.

Let's say you've had 1:1s with john1, john2, and john3. Searching for “john” would return all the johns as “users”. It doesn’t make sense to also return them as 1:1 dms.

That’s what this fixes. When the search returns some users, we skip all 1:1s because we assume they will show up as the results of the “users” query.

We’ll always return matching direct messages with more than 2 users (aka. “group chats”). All 3 other queries (users, groups, and category channels) are unaffected.

Internal ref - t/136079

plugins/chat/app/services/chat/search_chatable.rb

@@ -9,6 +9,8 @@ module Chat
   class SearchChatable
     include Service::Base
 
+    SEARCH_RESULT_LIMIT ||= 10
+
     # @!method call(term:, guardian:)
     #   @param [String] term
     #   @param [Guardian] guardian
@@ -35,7 +37,7 @@ module Chat
     private
 
     def clean_term(contract:)
-      context.term = contract.term.downcase&.gsub(/^#+/, "")&.gsub(/^@+/, "")&.strip
+      context.term = contract.term&.downcase&.strip&.gsub(/^[@#]+/, "")
     end
 
     def fetch_memberships(guardian:)
@@ -45,57 +47,29 @@ module Chat
     def fetch_users(guardian:, contract:)
       return unless contract.include_users
       return unless guardian.can_create_direct_message?
-      search_users(context, guardian, contract)
+      search_users(context, guardian)
     end
 
     def fetch_groups(guardian:, contract:)
       return unless contract.include_groups
       return unless guardian.can_create_direct_message?
-      search_groups(context, guardian, contract)
+      search_groups(context, guardian)
     end
 
     def fetch_category_channels(guardian:, contract:)
       return unless contract.include_category_channels
-      return if !SiteSetting.enable_public_channels
+      return unless SiteSetting.enable_public_channels
-
+      search_category_channels(context, guardian)
-      ::Chat::ChannelFetcher.secured_public_channel_search(
-        guardian,
-        filter_on_category_name: false,
-        match_filter_on_starts_with: false,
-        filter: context.term,
-        status: :open,
-        limit: 10,
-      )
     end
 
-    def fetch_direct_message_channels(guardian:, users:, contract:, **args)
+    def fetch_direct_message_channels(guardian:, contract:, users:, **args)
       return unless contract.include_direct_message_channels
-
+      return unless guardian.can_create_direct_message?
-      channels =
+      search_direct_message_channels(context, guardian, contract, users)
-        ::Chat::ChannelFetcher.secured_direct_message_channels_search(
-          guardian.user.id,
-          guardian,
-          limit: 10,
-          match_filter_on_starts_with: false,
-          filter: context.term,
-        ) || []
-
-      if users && contract.include_users
-        user_ids = users.map(&:id)
-        channels =
-          channels.reject do |channel|
-            channel_user_ids = channel.allowed_user_ids - [guardian.user.id]
-            channel.allowed_user_ids.length == 1 &&
-              user_ids.include?(channel.allowed_user_ids.first) ||
-              channel_user_ids.length == 1 && user_ids.include?(channel_user_ids.first)
-          end
-      end
-
-      channels
     end
 
-    def search_users(context, guardian, contract)
+    def search_users(context, guardian)
-      user_search = ::UserSearch.new(context.term, limit: 10)
+      user_search = ::UserSearch.new(context.term, limit: SEARCH_RESULT_LIMIT)
 
       if context.term.blank?
         user_search = user_search.scoped_users
@@ -112,11 +86,7 @@ module Chat
       if context.excluded_memberships_channel_id
         user_search =
           user_search.where(
-            "NOT EXISTS (
+            "NOT EXISTS (SELECT 1 FROM user_chat_channel_memberships WHERE user_id = users.id AND chat_channel_id = ?)",
-      SELECT 1
-      FROM user_chat_channel_memberships
-      WHERE user_chat_channel_memberships.user_id = users.id AND user_chat_channel_memberships.chat_channel_id = ?
-    )",
             context.excluded_memberships_channel_id,
           )
       end
@@ -124,7 +94,7 @@ module Chat
       user_search
     end
 
-    def search_groups(context, guardian, contract)
+    def search_groups(context, guardian)
       Group
         .visible_groups(guardian.user)
         .includes(users: :user_option)
@@ -132,6 +102,39 @@ module Chat
           "groups.name ILIKE :term_like OR groups.full_name ILIKE :term_like",
           term_like: "%#{context.term}%",
         )
+        .limit(SEARCH_RESULT_LIMIT)
+    end
+
+    def search_category_channels(context, guardian)
+      ::Chat::ChannelFetcher.secured_public_channel_search(
+        guardian,
+        status: :open,
+        filter: context.term,
+        filter_on_category_name: false,
+        match_filter_on_starts_with: false,
+        limit: SEARCH_RESULT_LIMIT,
+      )
+    end
+
+    def search_direct_message_channels(context, guardian, contract, users)
+      channels =
+        ::Chat::ChannelFetcher.secured_direct_message_channels_search(
+          guardian.user.id,
+          guardian,
+          filter: context.term,
+          match_filter_on_starts_with: false,
+          limit: SEARCH_RESULT_LIMIT,
+        ) || []
+
+      # skip 1:1s when search returns users
+      if contract.include_users && users.present?
+        channels.reject! do |channel|
+          other_user_ids = channel.allowed_user_ids - [guardian.user.id]
+          other_user_ids.size <= 1
+        end
+      end
+
+      channels
     end
   end
 end