diff --git a/plugins/chat/app/queries/chat/messages_query.rb b/plugins/chat/app/queries/chat/messages_query.rb index ff4f48242d9..5b1d2b82d9b 100644 --- a/plugins/chat/app/queries/chat/messages_query.rb +++ b/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 = direction == PAST ? "<" : ">" + 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}") diff --git a/plugins/chat/app/services/chat/list_channel_thread_messages.rb b/plugins/chat/app/services/chat/list_channel_thread_messages.rb index 6096d8a8ca2..81d5dcf00fa 100644 --- a/plugins/chat/app/services/chat/list_channel_thread_messages.rb +++ b/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:) - if contract.fetch_from_last_read || !contract.target_message_id + def determine_target_message_id(contract:, membership:, guardian:, thread:) + 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] diff --git a/plugins/chat/lib/chat_sdk/thread.rb b/plugins/chat/lib/chat_sdk/thread.rb index f7aabbd895d..bf242339485 100644 --- a/plugins/chat/lib/chat_sdk/thread.rb +++ b/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]) - end - - def self.update(**params) - new.update(**params) + # + def self.update_title(thread_id:, guardian:, title:) + new.update(title: title, thread_id: thread_id, guardian: guardian) 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] 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] 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" } diff --git a/plugins/chat/spec/lib/chat_sdk/thread_spec.rb b/plugins/chat/spec/lib/chat_sdk/thread_spec.rb index 570c21836f7..a6ab8a74363 100644 --- a/plugins/chat/spec/lib/chat_sdk/thread_spec.rb +++ b/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", - channel_id: thread_1.channel_id, - thread_id: thread_1.id, - guardian: Discourse.system_user.guardian, - } + { title: "New Title", 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 diff --git a/plugins/chat/spec/queries/chat/messages_query_spec.rb b/plugins/chat/spec/queries/chat/messages_query_spec.rb index fb63f90f458..fbd36de1409 100644 --- a/plugins/chat/spec/queries/chat/messages_query_spec.rb +++ b/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], 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 index 8533f97b479..e42f121e685 100644 --- a/plugins/chat/spec/services/chat/list_channel_thread_messages_spec.rb +++ b/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 } }