From c57ae992b337016e2b7f68f4c6bce4f5320f84b6 Mon Sep 17 00:00:00 2001
From: Martin Brennan <martin@discourse.org>
Date: Mon, 29 May 2023 20:12:01 +0200
Subject: [PATCH] FIX: Page size edge case for null last_read_message_id
 (#21811)

Followup 55ef2d0698627348e4f340f079504419c0939677.

In the cases where the user has no last_read_message_id for
a channel, we want to make sure that a page_size is set for
the ChannelViewBuilder + MessagesQuery, otherwise we end up
loading way more messages than needed (the additional message
loading was fixed in the last commit).
---
 .../app/services/chat/channel_view_builder.rb |  9 +++++++++
 .../chat/channel_view_builder_spec.rb         | 19 +++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/plugins/chat/app/services/chat/channel_view_builder.rb b/plugins/chat/app/services/chat/channel_view_builder.rb
index bf519c3ece1..ce15a4e905f 100644
--- a/plugins/chat/app/services/chat/channel_view_builder.rb
+++ b/plugins/chat/app/services/chat/channel_view_builder.rb
@@ -84,6 +84,15 @@ module Chat
     def determine_target_message_id(contract:, channel:, guardian:, **)
       if contract.fetch_from_last_read
         contract.target_message_id = channel.membership_for(guardian.user)&.last_read_message_id
+
+        # We need to force a page size here because we don't want to
+        # load all messages in the channel (since starting from 0
+        # makes them all unread). When the target_message_id is provided
+        # page size is not required since we load N messages either side of
+        # the target.
+        if contract.target_message_id.blank?
+          contract.page_size = contract.page_size || Chat::MessagesQuery::MAX_PAGE_SIZE
+        end
       end
     end
 
diff --git a/plugins/chat/spec/services/chat/channel_view_builder_spec.rb b/plugins/chat/spec/services/chat/channel_view_builder_spec.rb
index 60e4efc9417..d61d6ee0138 100644
--- a/plugins/chat/spec/services/chat/channel_view_builder_spec.rb
+++ b/plugins/chat/spec/services/chat/channel_view_builder_spec.rb
@@ -93,6 +93,12 @@ RSpec.describe Chat::ChannelViewBuilder do
       it { is_expected.to fail_a_contract }
     end
 
+    context "when page_size is too big" do
+      let(:page_size) { Chat::MessagesQuery::MAX_PAGE_SIZE + 1 }
+
+      it { is_expected.to fail_a_contract }
+    end
+
     context "when channel has threading_enabled and enable_experimental_chat_threaded_discussions is true" do
       before do
         channel.update!(threading_enabled: true)
@@ -247,6 +253,19 @@ RSpec.describe Chat::ChannelViewBuilder do
           it "does not error and still returns messages" do
             expect(subject.view.chat_messages).to eq([past_message_2, past_message_1, message])
           end
+
+          context "if page_size is nil" do
+            let(:page_size) { nil }
+
+            it "calls the messages query with the default page size" do
+              ::Chat::MessagesQuery
+                .expects(:call)
+                .with(has_entries(page_size: Chat::MessagesQuery::MAX_PAGE_SIZE))
+                .once
+                .returns({ messages: [] })
+              subject
+            end
+          end
         end
       end
     end