DEV: adds first_messages/last_messages to thread SDK (#26861)

This commit introduces several enhancements to the ChatSDK module, aiming to improve the functionality and usability of chat thread interactions. Here's what has been changed and added:

1. **New Method: `first_messages`:**
   - Added a method to retrieve the first set of messages from a specified chat thread.
   - This method is particularly useful for fetching initial messages when entering a chat thread.
   - Parameters include `thread_id`, `guardian`, and an optional `page_size` which defaults to 10.
   - Usage example added to demonstrate fetching the first 15 messages from a thread.

2. **New Method: `last_messages`:**
   - Added a method to retrieve the last set of messages from a specified chat thread.
   - This method supports reverse pagination, where the user may want to see the most recent messages first.
   - Similar to `first_messages`, it accepts `thread_id`, `guardian`, and an optional `page_size` parameter, defaulting to 10.
   - Usage example provided to illustrate fetching the last 20 messages from a thread.

plugins/chat/app/queries/chat/messages_query.rb

@@ -36,6 +36,12 @@ module Chat
     # @param direction [String] (optional) The direction to fetch messages in when not
     #   using the target_message_id param. Must be valid. If not provided, only the
     #   latest messages for the channel are loaded.
+    # @param include_target_message_id [Boolean] (optional) Specifies whether the target message specified by
+    #   target_message_id should be included in the results. This parameter modifies the behavior when querying messages:
+    #   - When true and the direction is set to "past", the query will include messages up to and including the target message.
+    #   - When true and the direction is set to "future", the query will include messages starting from and including the target message.
+    #   - When false, the query will exclude the target message, fetching only those messages strictly before or after it, depending on the direction.
+
     def self.call(
       channel:,
       guardian:,
@@ -44,7 +50,8 @@ module Chat
       include_thread_messages: false,
       page_size: PAST_MESSAGE_LIMIT + FUTURE_MESSAGE_LIMIT,
       direction: nil,
-      target_date: nil
+      target_date: nil,
+      include_target_message_id: false
     )
       messages = base_query(channel: channel)
       messages = messages.with_deleted if guardian.can_moderate_chat?(channel.chatable)
@@ -76,7 +83,14 @@ module Chat
         if target_date.present?
           query_by_date(target_date, channel, messages)
         else
-          query_paginated_messages(direction, page_size, channel, messages, target_message_id)
+          query_paginated_messages(
+            direction,
+            page_size,
+            channel,
+            messages,
+            target_message_id: target_message_id,
+            include_target_message_id: include_target_message_id,
+          )
         end
       end
     end
@@ -139,16 +153,25 @@ module Chat
       page_size,
       channel,
       messages,
-      target_message_id = nil
+      target_message_id: nil,
+      include_target_message_id: false
     )
       page_size = [page_size || MAX_PAGE_SIZE, MAX_PAGE_SIZE].min
 
       if target_message_id.present?
+        condition = nil
+
+        if include_target_message_id
+          condition = direction == PAST ? "<=" : ">="
+        else
           condition = direction == PAST ? "<" : ">"
+        end
+
         messages = messages.where("chat_messages.id #{condition} ?", target_message_id.to_i)
       end
 
       order = direction == FUTURE ? "ASC" : "DESC"
+
       messages =
         messages
           .order("chat_messages.created_at #{order}, chat_messages.id #{order}")

plugins/chat/app/services/chat/list_channel_thread_messages.rb

@@ -35,6 +35,8 @@ module Chat
       attribute :direction, :string # (optional)
       attribute :page_size, :integer # (optional)
       attribute :fetch_from_last_read, :boolean # (optional)
+      attribute :fetch_from_last_message, :boolean # (optional)
+      attribute :fetch_from_first_message, :boolean # (optional)
       attribute :target_date, :string # (optional)
 
       validates :direction,
@@ -65,11 +67,15 @@ module Chat
     end
 
     def can_view_thread(guardian:, thread:)
-      guardian.can_preview_chat_channel?(thread.channel)
+      guardian.user == Discourse.system_user || guardian.can_preview_chat_channel?(thread.channel)
     end
 
-    def determine_target_message_id(contract:, membership:, guardian:)
+    def determine_target_message_id(contract:, membership:, guardian:, thread:)
-      if contract.fetch_from_last_read || !contract.target_message_id
+      if contract.fetch_from_last_message
+        context.target_message_id = thread.last_message_id
+      elsif contract.fetch_from_first_message
+        context.target_message_id = thread.original_message_id
+      elsif contract.fetch_from_last_read || !contract.target_message_id
         context.target_message_id = membership&.last_read_message_id
       elsif contract.target_message_id
         context.target_message_id = contract.target_message_id
@@ -98,6 +104,8 @@ module Chat
           page_size: contract.page_size || Chat::MessagesQuery::MAX_PAGE_SIZE,
           direction: contract.direction,
           target_date: contract.target_date,
+          include_target_message_id:
+            contract.fetch_from_first_message || contract.fetch_from_last_message,
         )
 
       context.can_load_more_past = messages_data[:can_load_more_past]

plugins/chat/lib/chat_sdk/thread.rb

@@ -13,12 +13,9 @@ module ChatSDK
     #
     # @example Updating the title of a chat thread
     #   ChatSDK::Thread.update_title(title: "New Thread Title", thread_id: 1, guardian: Guardian.new)
-    def self.update_title(**params)
+    #
-      new.update(title: params[:title], thread_id: params[:thread_id], guardian: params[:guardian])
+    def self.update_title(thread_id:, guardian:, title:)
-    end
+      new.update(title: title, thread_id: thread_id, guardian: guardian)
-
-    def self.update(**params)
-      new.update(**params)
     end
 
     # Retrieves messages from a specified thread.
@@ -34,13 +31,57 @@ module ChatSDK
       new.messages(thread_id: thread_id, guardian: guardian, **params)
     end
 
-    def messages(thread_id:, guardian:, **params)
+    # Fetches the first messages from a specified chat thread, starting from the first available message.
+    #
+    # @param thread_id [Integer] The ID of the chat thread from which to fetch messages.
+    # @param guardian [Guardian] The guardian object representing the user's permissions.
+    # @param page_size [Integer] (optional) The number of messages to fetch, defaults to 10.
+    # @return [Array<Chat::Message>] An array of message objects representing the first messages in the thread.
+    #
+    # @example Fetching the first 15 messages from a thread
+    #   ChatSDK::Thread.first_messages(thread_id: 1, guardian: Guardian.new, page_size: 15)
+    #
+    def self.first_messages(thread_id:, guardian:, page_size: 10)
+      new.messages(
+        thread_id: thread_id,
+        guardian: guardian,
+        page_size: page_size,
+        direction: "future",
+        fetch_from_first_message: true,
+      )
+    end
+
+    # Fetches the last messages from a specified chat thread, starting from the last available message.
+    #
+    # @param thread_id [Integer] The ID of the chat thread from which to fetch messages.
+    # @param guardian [Guardian] The guardian object representing the user's permissions.
+    # @param page_size [Integer] (optional) The number of messages to fetch, defaults to 10.
+    # @return [Array<Chat::Message>] An array of message objects representing the last messages in the thread.
+    #
+    # @example Fetching the last 20 messages from a thread
+    #   ChatSDK::Thread.last_messages(thread_id: 2, guardian: Guardian.new, page_size: 20)
+    #
+    def self.last_messages(thread_id:, guardian:, page_size: 10)
+      new.messages(
+        thread_id: thread_id,
+        guardian: guardian,
+        page_size: page_size,
+        direction: "past",
+        fetch_from_last_message: true,
+      )
+    end
+
+    def self.update(**params)
+      new.update(**params)
+    end
+
+    def messages(thread_id:, guardian:, direction: "future", **params)
       with_service(
         Chat::ListChannelThreadMessages,
         thread_id: thread_id,
         guardian: guardian,
+        direction: direction,
         **params,
-        direction: "future",
       ) do
         on_success { result.messages }
         on_failed_policy(:can_view_thread) { raise "Guardian can't view thread" }

plugins/chat/spec/lib/chat_sdk/thread_spec.rb

@@ -5,12 +5,7 @@ describe ChatSDK::Thread do
     fab!(:thread_1) { Fabricate(:chat_thread) }
 
     let(:params) do
-      {
+      { title: "New Title", thread_id: thread_1.id, guardian: Discourse.system_user.guardian }
-        title: "New Title",
-        channel_id: thread_1.channel_id,
-        thread_id: thread_1.id,
-        guardian: Discourse.system_user.guardian,
-      }
     end
 
     it "changes the title" do
@@ -23,7 +18,9 @@ describe ChatSDK::Thread do
       it "fails" do
         params.delete(:thread_id)
 
-        expect { described_class.update_title(**params) }.to raise_error("Thread can't be blank")
+        expect { described_class.update_title(**params) }.to raise_error(
+          "missing keyword: :thread_id",
+        )
       end
     end
 
@@ -67,6 +64,67 @@ describe ChatSDK::Thread do
         )
       end
     end
+  end
+
+  describe ".first_messages" do
+    fab!(:thread_1) { Fabricate(:chat_thread) }
+    fab!(:messages) do
+      Fabricate.times(5, :chat_message, thread: thread_1, chat_channel: thread_1.channel)
+    end
+
+    let(:params) { { thread_id: thread_1.id, guardian: Discourse.system_user.guardian } }
+
+    it "returns messages" do
+      expect(described_class.first_messages(**params)).to eq([thread_1.original_message, *messages])
+    end
+  end
+
+  describe ".last_messages" do
+    fab!(:thread_1) { Fabricate(:chat_thread) }
+    fab!(:messages) do
+      Fabricate.times(
+        5,
+        :chat_message,
+        thread: thread_1,
+        chat_channel: thread_1.channel,
+        use_service: true,
+      )
+    end
+
+    let(:params) do
+      { thread_id: thread_1.id, guardian: Discourse.system_user.guardian, page_size: 5 }
+    end
+
+    it "returns messages" do
+      expect(described_class.last_messages(**params)).to eq([*messages])
+    end
+  end
+
+  describe ".messages" do
+    fab!(:thread_1) { Fabricate(:chat_thread) }
+    fab!(:messages) do
+      Fabricate.times(
+        5,
+        :chat_message,
+        thread: thread_1,
+        chat_channel: thread_1.channel,
+        use_service: true,
+      )
+    end
+
+    let(:params) { { thread_id: thread_1.id, guardian: Discourse.system_user.guardian } }
+
+    it "returns messages" do
+      expect(described_class.messages(**params)).to eq([thread_1.original_message, *messages])
+    end
+
+    describe "page_size:" do
+      before { params[:page_size] = 2 }
+
+      it "limits returned messages" do
+        expect(described_class.messages(**params)).to eq([thread_1.original_message, messages[0]])
+      end
+    end
 
     context "when target_message doesn’t exist" do
       it "fails" do

plugins/chat/spec/queries/chat/messages_query_spec.rb

@@ -26,21 +26,49 @@ RSpec.describe Chat::MessagesQuery do
   end
 
   fab!(:message_1) do
-    message = Fabricate(:chat_message, chat_channel: channel)
+    message = Fabricate(:chat_message, chat_channel: channel, use_service: true)
     message.update!(created_at: 2.days.ago)
     message
   end
   fab!(:message_2) do
-    message = Fabricate(:chat_message, chat_channel: channel)
+    message = Fabricate(:chat_message, chat_channel: channel, use_service: true)
     message.update!(created_at: 6.hours.ago)
     message
   end
-  fab!(:message_3) { Fabricate(:chat_message, chat_channel: channel) }
+  fab!(:message_3) { Fabricate(:chat_message, chat_channel: channel, use_service: true) }
 
   context "when target_message_id provided" do
     let(:target_message) { message_2 }
     let(:target_message_id) { target_message.id }
 
+    context "when include_target_message_id is true" do
+      context "when querying future" do
+        it "includes the target message in the query" do
+          options[:direction] = "future"
+          options[:include_target_message_id] = true
+
+          expect(query).to eq(
+            messages: [target_message, message_3],
+            can_load_more_past: nil,
+            can_load_more_future: false,
+          )
+        end
+      end
+
+      context "when querying past" do
+        it "includes the target message in the query" do
+          options[:direction] = "past"
+          options[:include_target_message_id] = true
+
+          expect(query).to eq(
+            messages: [message_1, target_message],
+            can_load_more_past: false,
+            can_load_more_future: nil,
+          )
+        end
+      end
+    end
+
     it "queries messages in the channel and finds the past and future messages" do
       expect(query).to eq(
         past_messages: [message_1],

plugins/chat/spec/services/chat/list_channel_thread_messages_spec.rb

@@ -67,10 +67,30 @@ RSpec.describe Chat::ListChannelThreadMessages do
       end
 
       it { is_expected.to fail_a_policy(:can_view_thread) }
+
+      context "with system user" do
+        fab!(:user) { Discourse.system_user }
+
+        it { is_expected.to be_a_success }
+      end
     end
   end
 
   context "when determine_target_message_id" do
+    let(:optional_params) { { fetch_from_last_message: true } }
+
+    context "when fetch_from_last_message is true" do
+      it "sets target_message_id to last thread message id" do
+        expect(result.target_message_id).to eq(thread.chat_messages.last.id)
+      end
+    end
+
+    context "when fetch_from_first_message is true" do
+      it "sets target_message_id to first thread message id" do
+        expect(result.target_message_id).to eq(thread.chat_messages.first.id)
+      end
+    end
+
     context "when fetch_from_last_read is true" do
       let(:optional_params) { { fetch_from_last_read: true } }