From ce76b88eb2a6e3a2cab4ee1c0b11e3dd5dfa5b58 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Mon, 4 Nov 2024 06:14:35 +0900 Subject: [PATCH] DEV: start/stop reply implementation (#29542) * DEV: join/leave presence chat-reply when streaming This commit ensures that starting/stopping a chat message with the streaming option will automatically make the creator of the message as present in the chat-reply channel. * implements start/stop reply * not needed --- plugins/chat/app/services/chat/start_reply.rb | 50 +++++++++++ plugins/chat/app/services/chat/stop_reply.rb | 46 ++++++++++ plugins/chat/lib/chat_sdk/channel.rb | 65 ++++++++++++++ plugins/chat/plugin.rb | 12 ++- .../chat/spec/lib/chat_sdk/channel_spec.rb | 86 +++++++++++++++++++ .../spec/services/chat/start_reply_spec.rb | 53 ++++++++++++ .../spec/services/chat/stop_reply_spec.rb | 51 +++++++++++ 7 files changed, 361 insertions(+), 2 deletions(-) create mode 100644 plugins/chat/app/services/chat/start_reply.rb create mode 100644 plugins/chat/app/services/chat/stop_reply.rb create mode 100644 plugins/chat/spec/services/chat/start_reply_spec.rb create mode 100644 plugins/chat/spec/services/chat/stop_reply_spec.rb diff --git a/plugins/chat/app/services/chat/start_reply.rb b/plugins/chat/app/services/chat/start_reply.rb new file mode 100644 index 00000000000..5362fac0796 --- /dev/null +++ b/plugins/chat/app/services/chat/start_reply.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +module Chat + # Service responsible for joining the reply presence channel of a chat channel. + # The client_id set in the context should be stored to be able to call Chat::StopReply later. + # + # @example + # Chat::StartReply.call(params: { channel_id: 3 }, guardian: guardian) + # + class StartReply + include ::Service::Base + + # @!method self.call(guardian:, params:) + # @param [Guardian] guardian + # @param [Hash] params + # @option params [Integer] :channel_id + # @option params [Integer] :thread_id + # @return [Service::Base::Context] + params do + attribute :channel_id, :integer + validates :channel_id, presence: true + + attribute :thread_id, :integer + end + + model :presence_channel + step :generate_client_id + step :join_chat_reply_presence_channel + + private + + def fetch_presence_channel(params:) + name = "/chat-reply/#{params.channel_id}" + name += "/thread/#{params.thread_id}" if params.thread_id + PresenceChannel.new(name) + rescue PresenceChannel::NotFound + nil + end + + def generate_client_id + context[:client_id] = SecureRandom.hex + end + + def join_chat_reply_presence_channel(presence_channel:, guardian:) + presence_channel.present(user_id: guardian.user.id, client_id: context.client_id) + rescue PresenceChannel::InvalidAccess + fail!("Presence channel not accessible by the user: #{guardian.user.id}") + end + end +end diff --git a/plugins/chat/app/services/chat/stop_reply.rb b/plugins/chat/app/services/chat/stop_reply.rb new file mode 100644 index 00000000000..ef986777175 --- /dev/null +++ b/plugins/chat/app/services/chat/stop_reply.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +module Chat + # Service responsible for leaving the reply presence channel of a chat channel. + # + # @example + # Chat::StopReply.call(params: { client_id: "xxx", channel_id: 3 }, guardian: guardian) + # + class StopReply + include ::Service::Base + + # @!method self.call(guardian:, params:) + # @param [Guardian] guardian + # @param [Hash] params + # @option params [Integer] :client_id + # @option params [Integer] :channel_id + # @option params [Integer] :thread_id + # @return [Service::Base::Context] + params do + attribute :channel_id, :integer + validates :channel_id, presence: true + + attribute :client_id, :string + validates :client_id, presence: true + + attribute :thread_id, :integer + end + + model :presence_channel + step :leave_chat_reply_presence_channel + + private + + def fetch_presence_channel(params:) + name = "/chat-reply/#{params.channel_id}" + name += "/thread/#{params.thread_id}" if params.thread_id + PresenceChannel.new(name) + rescue PresenceChannel::NotFound + nil + end + + def leave_chat_reply_presence_channel(presence_channel:, params:, guardian:) + presence_channel.leave(user_id: guardian.user.id, client_id: params.client_id) + end + end +end diff --git a/plugins/chat/lib/chat_sdk/channel.rb b/plugins/chat/lib/chat_sdk/channel.rb index 169f492dffb..a2862212609 100644 --- a/plugins/chat/lib/chat_sdk/channel.rb +++ b/plugins/chat/lib/chat_sdk/channel.rb @@ -11,6 +11,9 @@ module ChatSDK # @example Fetching messages from a channel with additional parameters # ChatSDK::Channel.messages(channel_id: 1, guardian: Guardian.new) # + # @raise [RuntimeError] Raises an "Unexpected error" if the message retrieval fails for an unspecified reason. + # @raise [RuntimeError] Raises "Guardian can't view channel" if the user's permissions are insufficient to view the channel. + # @raise [RuntimeError] Raises "Target message doesn't exist" if the specified target message cannot be found in the channel. def self.messages(...) new.messages(...) end @@ -30,5 +33,67 @@ module ChatSDK on_failed_policy(:target_message_exists) { raise "Target message doesn't exist" } end end + + # Initiates a reply in a specified channel or thread. + # + # @param channel_id [Integer] The ID of the channel where the reply is started. + # @param thread_id [Integer, nil] (optional) The ID of the thread within the channel where the reply is started. + # @param guardian [Guardian] The guardian object representing the user's permissions. + # @return [String] The client ID associated with the initiated reply. + # + # @example Starting a reply in a channel + # ChatSDK::Channel.start_reply(channel_id: 1, guardian: Guardian.new) + # + # @example Starting a reply in a specific thread + # ChatSDK::Channel.start_reply(channel_id: 1, thread_id: 34, guardian: Guardian.new) + # + # @raise [RuntimeError] Raises an error if the specified channel or thread is not found. + def self.start_reply(...) + new.start_reply(...) + end + + def start_reply(channel_id:, thread_id: nil, guardian:) + Chat::StartReply.call( + guardian: guardian, + params: { + channel_id: channel_id, + thread_id: thread_id, + }, + ) do + on_success { |client_id:| client_id } + on_model_not_found(:presence_channel) { raise "Chat::Channel or Chat::Thread not found." } + end + end + + # Ends an ongoing reply in a specified channel or thread. + # + # @param channel_id [Integer] The ID of the channel where the reply is being stopped. + # @param thread_id [Integer, nil] (optional) The ID of the thread within the channel where the reply is being stopped. + # @param client_id [String] The client ID associated with the reply to stop. + # @param guardian [Guardian] The guardian object representing the user's permissions. + # + # @example Stopping a reply in a channel + # ChatSDK::Channel.stop_reply(channel_id: 1, client_id: "abc123", guardian: Guardian.new) + # + # @example Stopping a reply in a specific thread + # ChatSDK::Channel.stop_reply(channel_id: 1, thread_id: 34, client_id: "abc123", guardian: Guardian.new) + # + # @raise [RuntimeError] Raises an error if the specified channel or thread is not found. + def self.stop_reply(...) + new.stop_reply(...) + end + + def stop_reply(channel_id:, thread_id: nil, client_id:, guardian:) + Chat::StopReply.call( + guardian: guardian, + params: { + client_id: client_id, + channel_id: channel_id, + thread_id: thread_id, + }, + ) do + on_model_not_found(:presence_channel) { raise "Chat::Channel or Chat::Thread not found." } + end + end end end diff --git a/plugins/chat/plugin.rb b/plugins/chat/plugin.rb index 314b49a1fcf..c293627dce1 100644 --- a/plugins/chat/plugin.rb +++ b/plugins/chat/plugin.rb @@ -316,8 +316,16 @@ after_initialize do end register_presence_channel_prefix("chat-reply") do |channel_name| - if chat_channel_id = channel_name[%r{/chat-reply/(\d+)}, 1] - chat_channel = Chat::Channel.find(chat_channel_id) + if ( + channel_id, thread_id = + channel_name.match(%r{^/chat-reply/(\d+)(?:/thread/(\d+))?$})&.captures + ) + chat_channel = nil + if thread_id + chat_channel = Chat::Thread.find_by!(id: thread_id, channel_id: channel_id).channel + else + chat_channel = Chat::Channel.find(channel_id) + end PresenceChannel::Config.new.tap do |config| config.allowed_group_ids = chat_channel.allowed_group_ids diff --git a/plugins/chat/spec/lib/chat_sdk/channel_spec.rb b/plugins/chat/spec/lib/chat_sdk/channel_spec.rb index 8bcac2f7484..7af2f51104d 100644 --- a/plugins/chat/spec/lib/chat_sdk/channel_spec.rb +++ b/plugins/chat/spec/lib/chat_sdk/channel_spec.rb @@ -38,4 +38,90 @@ describe ChatSDK::Channel do end end end + + describe ".start_reply" do + fab!(:channel_1) { Fabricate(:chat_channel, threading_enabled: true) } + fab!(:thread_1) { Fabricate(:chat_thread, channel: channel_1) } + + let(:params) do + { channel_id: channel_1.id, thread_id: thread_1.id, guardian: Discourse.system_user.guardian } + end + + it "starts a reply" do + client_id = nil + expect { client_id = described_class.start_reply(**params) }.to change { + PresenceChannel.new("/chat-reply/#{channel_1.id}/thread/#{thread_1.id}").count + }.by(1) + + expect(client_id).to be_present + end + + context "when the channel doesn't exist" do + it "fails" do + params[:channel_id] = -999 + + expect { described_class.start_reply(**params) }.to raise_error( + "Chat::Channel or Chat::Thread not found.", + ) + end + end + + context "when the thread doesn't exist" do + it "fails" do + params[:thread_id] = -999 + + expect { described_class.start_reply(**params) }.to raise_error( + "Chat::Channel or Chat::Thread not found.", + ) + end + end + end + + describe ".stop_reply" do + fab!(:user) { Fabricate(:user) } + fab!(:channel_1) { Fabricate(:chat_channel, threading_enabled: true) } + fab!(:thread_1) { Fabricate(:chat_thread, channel: channel_1) } + fab!(:client_id) do + described_class.start_reply( + channel_id: channel_1.id, + thread_id: thread_1.id, + guardian: user.guardian, + ) + end + + let(:params) do + { + channel_id: channel_1.id, + thread_id: thread_1.id, + client_id: client_id, + guardian: user.guardian, + } + end + + it "stops a reply" do + expect { described_class.stop_reply(**params) }.to change { + PresenceChannel.new("/chat-reply/#{channel_1.id}/thread/#{thread_1.id}").count + }.by(-1) + end + + context "when the channel doesn't exist" do + it "fails" do + params[:channel_id] = -999 + + expect { described_class.stop_reply(**params) }.to raise_error( + "Chat::Channel or Chat::Thread not found.", + ) + end + end + + context "when the thread doesn't exist" do + it "fails" do + params[:thread_id] = -999 + + expect { described_class.stop_reply(**params) }.to raise_error( + "Chat::Channel or Chat::Thread not found.", + ) + end + end + end end diff --git a/plugins/chat/spec/services/chat/start_reply_spec.rb b/plugins/chat/spec/services/chat/start_reply_spec.rb new file mode 100644 index 00000000000..fef057c43be --- /dev/null +++ b/plugins/chat/spec/services/chat/start_reply_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +RSpec.describe Chat::StartReply do + describe described_class::Contract, type: :model do + subject(:contract) { described_class.new } + + it { is_expected.to validate_presence_of :channel_id } + end + + describe ".call" do + subject(:result) { described_class.call(params:, **dependencies) } + + fab!(:user) + fab!(:channel) { Fabricate(:chat_channel, threading_enabled: true) } + fab!(:thread) { Fabricate(:chat_thread, channel: channel) } + + let(:guardian) { user.guardian } + let(:params) { { channel_id: channel.id, thread_id: thread.id } } + let(:dependencies) { { guardian: } } + + before { channel.add(guardian.user) } + + context "when the channel is not found" do + before { params[:channel_id] = 999 } + + it { is_expected.to fail_to_find_a_model(:presence_channel) } + end + + context "when the thread is not found" do + before { params[:thread_id] = 999 } + + it { is_expected.to fail_to_find_a_model(:presence_channel) } + end + + it "generates a client id" do + expect(result.client_id).to be_present + end + + it "joins the presence channel" do + expect { result }.to change { + PresenceChannel.new("/chat-reply/#{channel.id}/thread/#{thread.id}").count + }.by(1) + end + + context "when the user is not part of the channel" do + fab!(:channel) { Fabricate(:private_category_channel, threading_enabled: true) } + + before { params[:thread_id] = nil } + + it { is_expected.to fail_a_step(:join_chat_reply_presence_channel) } + end + end +end diff --git a/plugins/chat/spec/services/chat/stop_reply_spec.rb b/plugins/chat/spec/services/chat/stop_reply_spec.rb new file mode 100644 index 00000000000..fd6cc1bb104 --- /dev/null +++ b/plugins/chat/spec/services/chat/stop_reply_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +RSpec.describe Chat::StopReply do + describe described_class::Contract, type: :model do + subject(:contract) { described_class.new } + + it { is_expected.to validate_presence_of :channel_id } + it { is_expected.to validate_presence_of :client_id } + end + + describe ".call" do + subject(:result) { described_class.call(params:, **dependencies) } + + fab!(:user) + fab!(:channel) { Fabricate(:chat_channel, threading_enabled: true) } + fab!(:thread) { Fabricate(:chat_thread, channel: channel) } + fab!(:client_id) do + Chat::StartReply.call( + params: { + channel_id: channel.id, + thread_id: thread.id, + }, + guardian: user.guardian, + ).client_id + end + + let(:guardian) { user.guardian } + let(:params) { { client_id: client_id, channel_id: channel.id, thread_id: thread.id } } + let(:dependencies) { { guardian: } } + + before { channel.add(guardian.user) } + + context "when the channel is not found" do + before { params[:channel_id] = 999 } + + it { is_expected.to fail_to_find_a_model(:presence_channel) } + end + + context "when the thread is not found" do + before { params[:thread_id] = 999 } + + it { is_expected.to fail_to_find_a_model(:presence_channel) } + end + + it "leaves the presence channel" do + presence_channel = PresenceChannel.new("/chat-reply/#{channel.id}/thread/#{thread.id}") + + expect { result }.to change { presence_channel.count }.by(-1) + end + end +end