DEV: allows chat thread sdk to query more messages (#32914)

jjaffeux · web-flow · commit cd4b6b8fab3c · 2025-06-02T09:29:24.000+02:00
The changes are:
- Chat Thread SDK can query up to 500 messages
- If you are above this number, we don't raise an error anymore but we
clamp it to 500
- The controller using this service is forced into the old limit of 50
to avoid abuses
diff --git a/plugins/chat/app/controllers/chat/api/channel_thread_messages_controller.rb b/plugins/chat/app/controllers/chat/api/channel_thread_messages_controller.rb
@@ -1,8 +1,12 @@
 # frozen_string_literal: true
 
 class Chat::Api::ChannelThreadMessagesController < Chat::ApiController
+  MAX_PAGE_SIZE = 50
+
   def index
-    ::Chat::ListChannelThreadMessages.call(service_params) do |result|
+    ::Chat::ListChannelThreadMessages.call(
+      service_params.deep_merge(params: { page_size: MAX_PAGE_SIZE }),
+    ) do |result|
       on_success do
         render_serialized(
           result,
diff --git a/plugins/chat/app/queries/chat/messages_query.rb b/plugins/chat/app/queries/chat/messages_query.rb
@@ -21,7 +21,7 @@ class MessagesQuery
     PAST = "past"
     FUTURE = "future"
     VALID_DIRECTIONS = [PAST, FUTURE]
-    MAX_PAGE_SIZE = 50
+    MAX_PAGE_SIZE = 500
 
     # @param channel [Chat::Channel] The channel to query messages within.
     # @param guardian [Guardian] The guardian to use for permission checks.
diff --git a/plugins/chat/app/services/chat/list_channel_thread_messages.rb b/plugins/chat/app/services/chat/list_channel_thread_messages.rb
@@ -34,12 +34,7 @@ class ListChannelThreadMessages
                   in: Chat::MessagesQuery::VALID_DIRECTIONS,
                 },
                 allow_nil: true
-      validates :page_size,
-                numericality: {
-                  less_than_or_equal_to: Chat::MessagesQuery::MAX_PAGE_SIZE,
-                  only_integer: true,
-                },
-                allow_nil: true
+      validates :page_size, numericality: { only_integer: true }, allow_nil: true
     end
 
     model :thread
@@ -94,7 +89,7 @@ def fetch_messages(thread:, guardian:, params:)
           guardian: guardian,
           target_message_id: context.target_message_id,
           thread_id: thread.id,
-          page_size: params.page_size || Chat::MessagesQuery::MAX_PAGE_SIZE,
+          page_size: params.page_size,
           direction: params.direction,
           target_date: params.target_date,
           include_target_message_id:
diff --git a/plugins/chat/spec/lib/chat_sdk/thread_spec.rb b/plugins/chat/spec/lib/chat_sdk/thread_spec.rb
@@ -126,14 +126,6 @@
       end
     end
 
-    context "when page_size is too large" do
-      it "fails" do
-        expect { described_class.messages(**params, page_size: 9999) }.to raise_error(
-          "Page size must be less than or equal to 50",
-        )
-      end
-    end
-
     context "when target_message doesn’t exist" do
       it "fails" do
         expect { described_class.messages(**params, target_message_id: -999) }.to raise_error(
diff --git a/plugins/chat/spec/queries/chat/messages_query_spec.rb b/plugins/chat/spec/queries/chat/messages_query_spec.rb
@@ -106,6 +106,12 @@
       stub_const(described_class, "MAX_PAGE_SIZE", 1) { expect(query[:messages].length).to eq(1) }
     end
 
+    it "limits results to MAX_PAGE_SIZE" do
+      options[:page_size] = 2
+      options[:target_message_id] = nil
+      stub_const(described_class, "MAX_PAGE_SIZE", 1) { expect(query[:messages].length).to eq(1) }
+    end
+
     describe "when some messages are in threads" do
       fab!(:thread) { Fabricate(:chat_thread, channel: channel) }
 
diff --git a/plugins/chat/spec/requests/chat/api/channel_thread_messages_controller_spec.rb b/plugins/chat/spec/requests/chat/api/channel_thread_messages_controller_spec.rb
@@ -60,9 +60,22 @@
       end
     end
 
+    context "when page_size is a above limit" do
+      fab!(:message_3) { Fabricate(:chat_message, thread: thread, chat_channel: thread.channel) }
+
+      it "clamps it to the max" do
+        stub_const(Chat::Api::ChannelThreadMessagesController, "MAX_PAGE_SIZE", 1) do
+          get "/chat/api/channels/#{thread.channel.id}/threads/#{thread.id}/messages?page_size=9999"
+
+          expect(response.status).to eq(200)
+          expect(response.parsed_body["messages"].length).to eq(1)
+        end
+      end
+    end
+
     context "when params are invalid" do
       it "returns a 400" do
-        get "/chat/api/channels/#{thread.channel.id}/threads/#{thread.id}/messages?page_size=9999"
+        get "/chat/api/channels/#{thread.channel.id}/threads/#{thread.id}/messages?direction=yolo"
 
         expect(response.status).to eq(400)
       end
diff --git a/plugins/chat/spec/services/chat/list_channel_thread_messages_spec.rb b/plugins/chat/spec/services/chat/list_channel_thread_messages_spec.rb
@@ -11,7 +11,6 @@
     it do
       is_expected.to allow_values(Chat::MessagesQuery::MAX_PAGE_SIZE, 1, "1", nil).for(:page_size)
     end
-    it { is_expected.not_to allow_values(Chat::MessagesQuery::MAX_PAGE_SIZE + 1).for(:page_size) }
   end
 
   describe ".call" do
diff --git a/plugins/chat/spec/system/bookmark_message_spec.rb b/plugins/chat/spec/system/bookmark_message_spec.rb
@@ -28,25 +28,26 @@
 
       expect(channel_page).to have_bookmarked_message(message_1)
     end
+    context "when in a long thread" do
+      it "supports linking to a bookmark in a long thread" do
+        category_channel_1.update!(threading_enabled: true)
+        category_channel_1.add(current_user)
 
-    it "supports linking to a bookmark in a long thread" do
-      category_channel_1.update!(threading_enabled: true)
-      category_channel_1.add(current_user)
+        thread =
+          chat_thread_chain_bootstrap(
+            channel: category_channel_1,
+            users: [current_user, Fabricate(:user)],
+            messages_count: 51,
+          )
 
-      thread =
-        chat_thread_chain_bootstrap(
-          channel: category_channel_1,
-          users: [current_user, Fabricate(:user)],
-          messages_count: Chat::MessagesQuery::MAX_PAGE_SIZE + 1,
-        )
-
-      first_message = thread.replies.first
+        first_message = thread.replies.first
 
-      bookmark = Bookmark.create!(bookmarkable: first_message, user: current_user)
+        bookmark = Bookmark.create!(bookmarkable: first_message, user: current_user)
 
-      visit bookmark.bookmarkable.url
+        visit bookmark.bookmarkable.url
 
-      expect(thread_page).to have_bookmarked_message(first_message)
+        expect(thread_page).to have_bookmarked_message(first_message)
+      end
     end
 
     context "in drawer mode" do
