From 06bbed69f9f75f9b345037841e44dd17d066064e Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Tue, 13 Feb 2024 11:37:15 +0100 Subject: [PATCH] DEV: allows a context when creating a message (#25647) The service `Chat::CreateMessage` will now accept `context_post_ids` and `context_topic_id` as params. These values represent the topic which might be visible when sending a message (for now, this is only possible when using the drawer). The `DiscourseEvent` `chat_message_created` will now have the following signature: ```ruby on(:chat_message_created) do | message, channel, user, meta| p meta[:context][:post_ids] end ``` --- .../chat/app/services/chat/create_message.rb | 6 +++ .../discourse/components/chat-channel.gjs | 10 +++- .../discourse/components/chat-thread.gjs | 17 +++--- .../lib/extract-current-topic-info.js | 29 +++++++++++ .../pre-initializers/chat-plugin-api.js | 2 + .../discourse/services/chat-api.js | 2 + plugins/chat/lib/chat/types/array.rb | 10 ++++ .../chat/spec/lib/chat/types/array_spec.rb | 16 ++++++ .../spec/services/chat/create_message_spec.rb | 30 ++++++++++- plugins/chat/spec/system/drawer_spec.rb | 52 +++++++++++++++++++ 10 files changed, 164 insertions(+), 10 deletions(-) create mode 100644 plugins/chat/assets/javascripts/discourse/lib/extract-current-topic-info.js diff --git a/plugins/chat/app/services/chat/create_message.rb b/plugins/chat/app/services/chat/create_message.rb index 4f1e5c7ef84..eeab69a0f8b 100644 --- a/plugins/chat/app/services/chat/create_message.rb +++ b/plugins/chat/app/services/chat/create_message.rb @@ -16,6 +16,8 @@ module Chat # @param in_reply_to_id [Integer] ID of a message to reply to # @param thread_id [Integer] ID of a thread to reply to # @param upload_ids [Array] IDs of uploaded documents + # @param context_topic_id [Integer] ID of the currently visible topic in drawer mode + # @param context_post_ids [Array] IDs of the currently visible posts in drawer mode # @param staged_id [String] arbitrary string that will be sent back to the client # @param incoming_chat_webhook [Chat::IncomingWebhook] @@ -50,6 +52,8 @@ module Chat class Contract attribute :chat_channel_id, :string attribute :in_reply_to_id, :string + attribute :context_topic_id, :integer + attribute :context_post_ids, :array attribute :message, :string attribute :staged_id, :string attribute :upload_ids, :array @@ -185,11 +189,13 @@ module Chat def process(channel:, message_instance:, contract:, **) ::Chat::Publisher.publish_new!(channel, message_instance, contract.staged_id) + DiscourseEvent.trigger( :chat_message_created, message_instance, channel, message_instance.user, + { context: { post_ids: contract.context_post_ids, topic_id: contract.context_topic_id } }, ) if contract.process_inline diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-channel.gjs b/plugins/chat/assets/javascripts/discourse/components/chat-channel.gjs index f93fe4c88e0..829cebb66b7 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-channel.gjs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-channel.gjs @@ -35,6 +35,7 @@ import { checkMessageTopVisibility, } from "discourse/plugins/chat/discourse/lib/check-message-visibility"; import DatesSeparatorsPositioner from "discourse/plugins/chat/discourse/lib/dates-separators-positioner"; +import { extractCurrentTopicInfo } from "discourse/plugins/chat/discourse/lib/extract-current-topic-info"; import { scrollListToBottom, scrollListToMessage, @@ -560,12 +561,17 @@ export default class ChatChannel extends Component { } try { - await this.chatApi.sendMessage(this.args.channel.id, { + const params = { message: message.message, in_reply_to_id: message.inReplyTo?.id, staged_id: message.id, upload_ids: message.uploads.map((upload) => upload.id), - }); + }; + + await this.chatApi.sendMessage( + this.args.channel.id, + Object.assign({}, params, extractCurrentTopicInfo(this)) + ); if (!this.capabilities.isIOS) { this.scrollToLatestMessage(); diff --git a/plugins/chat/assets/javascripts/discourse/components/chat-thread.gjs b/plugins/chat/assets/javascripts/discourse/components/chat-thread.gjs index 090ed48b0a7..1a924fe1e35 100644 --- a/plugins/chat/assets/javascripts/discourse/components/chat-thread.gjs +++ b/plugins/chat/assets/javascripts/discourse/components/chat-thread.gjs @@ -25,6 +25,7 @@ import { } from "discourse/plugins/chat/discourse/lib/chat-ios-hacks"; import ChatMessagesLoader from "discourse/plugins/chat/discourse/lib/chat-messages-loader"; import DatesSeparatorsPositioner from "discourse/plugins/chat/discourse/lib/dates-separators-positioner"; +import { extractCurrentTopicInfo } from "discourse/plugins/chat/discourse/lib/extract-current-topic-info"; import { scrollListToBottom, scrollListToMessage, @@ -429,15 +430,17 @@ export default class ChatThread extends Component { } try { + const params = { + message: message.message, + in_reply_to_id: null, + staged_id: message.id, + upload_ids: message.uploads.map((upload) => upload.id), + thread_id: message.thread.id, + }; + const response = await this.chatApi.sendMessage( this.args.thread.channel.id, - { - message: message.message, - in_reply_to_id: null, - staged_id: message.id, - upload_ids: message.uploads.map((upload) => upload.id), - thread_id: message.thread.id, - } + Object.assign({}, params, extractCurrentTopicInfo(this)) ); this.args.thread.currentUserMembership ??= diff --git a/plugins/chat/assets/javascripts/discourse/lib/extract-current-topic-info.js b/plugins/chat/assets/javascripts/discourse/lib/extract-current-topic-info.js new file mode 100644 index 00000000000..92c9e75350e --- /dev/null +++ b/plugins/chat/assets/javascripts/discourse/lib/extract-current-topic-info.js @@ -0,0 +1,29 @@ +import { getOwner } from "@ember/application"; + +export function extractCurrentTopicInfo(context) { + const topic = getOwner(context).lookup("controller:topic")?.get("model"); + + if (!topic) { + return; + } + + const info = { topic_id: topic.id }; + const currentPostNumber = parseInt(topic.current_post_number, 10); + const posts = topic.postStream.posts; + + const currentPost = posts.find( + (post) => post.post_number === currentPostNumber + ); + const previousPost = posts.findLast( + (post) => + !post.hidden && !post.deleted_at && post.post_number < currentPostNumber + ); + const nextPost = posts.find( + (post) => + !post.hidden && !post.deleted_at && post.post_number > currentPostNumber + ); + + info.post_ids = [previousPost?.id, currentPost?.id, nextPost?.id]; + + return info; +} diff --git a/plugins/chat/assets/javascripts/discourse/pre-initializers/chat-plugin-api.js b/plugins/chat/assets/javascripts/discourse/pre-initializers/chat-plugin-api.js index 17045e5e7ef..2049ace308b 100644 --- a/plugins/chat/assets/javascripts/discourse/pre-initializers/chat-plugin-api.js +++ b/plugins/chat/assets/javascripts/discourse/pre-initializers/chat-plugin-api.js @@ -163,6 +163,8 @@ export default { .lookup("service:chat-api") .sendMessage(channelId, { thread_id: options.threadId, + post_ids: options.postIds, + topic_id: options.topicId, message: options.message, uploads: options.uploads, }); diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-api.js b/plugins/chat/assets/javascripts/discourse/services/chat-api.js index 2cea38c9c2c..0fc0481df95 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-api.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-api.js @@ -180,6 +180,8 @@ export default class ChatApi extends Service { * @param {number} [data.in_reply_to_id] - The ID of the replied-to message. * @param {number} [data.staged_id] - The staged ID of the message before it was persisted. * @param {number} [data.thread_id] - The ID of the thread where this message should be posted. + * @param {number} [data.topic_id] - The ID of the currently visible topic in drawer mode. + * @param {number} [data.post_ids] - The ID of the currently visible posts in drawer mode. * @param {Array.} [data.upload_ids] - Array of upload ids linked to the message. * @returns {Promise} */ diff --git a/plugins/chat/lib/chat/types/array.rb b/plugins/chat/lib/chat/types/array.rb index a8491a868b1..81212f42cd0 100644 --- a/plugins/chat/lib/chat/types/array.rb +++ b/plugins/chat/lib/chat/types/array.rb @@ -11,10 +11,20 @@ module Chat case value when String value.split(",") + when ::Array + value.map { |item| convert_to_integer(item) } else ::Array.wrap(value) end end + + private + + def convert_to_integer(item) + Integer(item) + rescue ArgumentError + item + end end end end diff --git a/plugins/chat/spec/lib/chat/types/array_spec.rb b/plugins/chat/spec/lib/chat/types/array_spec.rb index c6e843d4a18..8850e18ae9e 100644 --- a/plugins/chat/spec/lib/chat/types/array_spec.rb +++ b/plugins/chat/spec/lib/chat/types/array_spec.rb @@ -24,6 +24,22 @@ RSpec.describe Chat::Types::Array do end end + context "when 'value' is an array of numbers as string" do + let(:value) { %w[1 2] } + + it "returns it with string casted as integer" do + expect(casted_value).to eq([1, 2]) + end + end + + context "when 'value' is an array of numbers" do + let(:value) { [1, 2] } + + it "returns it" do + expect(casted_value).to eq([1, 2]) + end + end + context "when 'value' is something else" do let(:value) { Time.current } diff --git a/plugins/chat/spec/services/chat/create_message_spec.rb b/plugins/chat/spec/services/chat/create_message_spec.rb index bceed7b2836..27151cef679 100644 --- a/plugins/chat/spec/services/chat/create_message_spec.rb +++ b/plugins/chat/spec/services/chat/create_message_spec.rb @@ -31,8 +31,17 @@ RSpec.describe Chat::CreateMessage do let(:guardian) { user.guardian } let(:content) { "A new message @#{other_user.username_lower}" } + let(:context_topic_id) { nil } + let(:context_post_ids) { nil } let(:params) do - { guardian: guardian, chat_channel_id: channel.id, message: content, upload_ids: [upload.id] } + { + guardian: guardian, + chat_channel_id: channel.id, + message: content, + upload_ids: [upload.id], + context_topic_id: context_topic_id, + context_post_ids: context_post_ids, + } end let(:message) { result[:message_instance].reload } @@ -91,10 +100,29 @@ RSpec.describe Chat::CreateMessage do instance_of(Chat::Message), channel, user, + anything, ) + result end + context "when context given" do + let(:context_post_ids) { [1, 2] } + let(:context_topic_id) { 3 } + + it "triggers a Discourse event with context if given" do + DiscourseEvent.expects(:trigger).with( + :chat_message_created, + instance_of(Chat::Message), + channel, + user, + { context: { post_ids: context_post_ids, topic_id: context_topic_id } }, + ) + + result + end + end + it "processes the direct message channel" do Chat::Action::PublishAndFollowDirectMessageChannel.expects(:call).with( channel_membership: membership, diff --git a/plugins/chat/spec/system/drawer_spec.rb b/plugins/chat/spec/system/drawer_spec.rb index a4cb27b9284..5248cd8af19 100644 --- a/plugins/chat/spec/system/drawer_spec.rb +++ b/plugins/chat/spec/system/drawer_spec.rb @@ -151,4 +151,56 @@ RSpec.describe "Drawer", type: :system do expect(drawer_page).to have_open_channel(channel) end end + + context "when sending a message from topic" do + fab!(:topic) + fab!(:posts) { Fabricate.times(5, :post, topic: topic) } + fab!(:channel) { Fabricate(:chat_channel) } + fab!(:membership) do + Fabricate(:user_chat_channel_membership, user: current_user, chat_channel: channel) + end + + let(:topic_page) { PageObjects::Pages::Topic.new } + + context "when on a channel" do + it "has context" do + ::Chat::CreateMessage + .expects(:call) + .with do |value| + value["topic_id"] === topic.id.to_s && + value["post_ids"] === [posts[1].id.to_s, posts[2].id.to_s, posts[3].id.to_s] + end + + topic_page.visit_topic(topic, post_number: 3) + chat_page.open_from_header + drawer_page.open_channel(channel) + channel_page.send_message + end + end + + context "when on a thread" do + before { channel.update!(threading_enabled: true) } + + fab!(:thread_1) { Fabricate(:chat_thread, channel: channel) } + + let(:thread_list_page) { PageObjects::Components::Chat::ThreadList.new } + let(:thread_page) { PageObjects::Pages::ChatThread.new } + + it "has context" do + ::Chat::CreateMessage + .expects(:call) + .with do |value| + value["topic_id"] === topic.id.to_s && + value["post_ids"] === [posts[1].id.to_s, posts[2].id.to_s, posts[3].id.to_s] + end + + topic_page.visit_topic(topic, post_number: 3) + chat_page.open_from_header + drawer_page.open_channel(channel) + drawer_page.open_thread_list + thread_list_page.open_thread(thread_1) + thread_page.send_message + end + end + end end