diff --git a/plugins/chat/app/services/chat/publisher.rb b/plugins/chat/app/services/chat/publisher.rb index dd4013a1e81..a14bc288dcd 100644 --- a/plugins/chat/app/services/chat/publisher.rb +++ b/plugins/chat/app/services/chat/publisher.rb @@ -15,18 +15,22 @@ module Chat end def self.calculate_publish_targets(channel, message) - targets = - if message.thread_om? - [ - root_message_bus_channel(channel.id), - thread_message_bus_channel(channel.id, message.thread_id), - ] - elsif message.thread_reply? - [thread_message_bus_channel(channel.id, message.thread_id)] - else - [root_message_bus_channel(channel.id)] - end - targets + return [root_message_bus_channel(channel.id)] if !allow_publish_to_thread?(channel) + + if message.thread_om? + [ + root_message_bus_channel(channel.id), + thread_message_bus_channel(channel.id, message.thread_id), + ] + elsif message.thread_reply? + [thread_message_bus_channel(channel.id, message.thread_id)] + else + [root_message_bus_channel(channel.id)] + end + end + + def self.allow_publish_to_thread?(channel) + SiteSetting.enable_experimental_chat_threaded_discussions && channel.threading_enabled end def self.publish_new!(chat_channel, chat_message, staged_id) @@ -40,7 +44,7 @@ module Chat # NOTE: This means that the read count is only updated in the client # for new messages in the main channel stream, maybe in future we want to # do this for thread messages as well? - if !chat_message.thread_reply? + if !chat_message.thread_reply? || !allow_publish_to_thread?(chat_channel) MessageBus.publish( self.new_messages_message_bus_channel(chat_channel.id), { diff --git a/plugins/chat/spec/services/chat/publisher_spec.rb b/plugins/chat/spec/services/chat/publisher_spec.rb index d5cc2ec781f..78e3e3d15f3 100644 --- a/plugins/chat/spec/services/chat/publisher_spec.rb +++ b/plugins/chat/spec/services/chat/publisher_spec.rb @@ -16,19 +16,149 @@ describe Chat::Publisher do end describe ".calculate_publish_targets" do - context "when the chat message is the original message of a thread" do - fab!(:thread) { Fabricate(:chat_thread, original_message: message, channel: channel) } + context "when enable_experimental_chat_threaded_discussions is false" do + before { SiteSetting.enable_experimental_chat_threaded_discussions = false } - it "generates the correct targets" do - targets = described_class.calculate_publish_targets(channel, message) - expect(targets).to contain_exactly( - "/chat/#{channel.id}", - "/chat/#{channel.id}/thread/#{thread.id}", + context "when the message is the original message of a thread" do + fab!(:thread) { Fabricate(:chat_thread, original_message: message, channel: channel) } + + it "generates the correct targets" do + targets = described_class.calculate_publish_targets(channel, message) + expect(targets).to contain_exactly("/chat/#{channel.id}") + end + end + + context "when the message is a thread reply" do + fab!(:thread) do + Fabricate( + :chat_thread, + original_message: Fabricate(:chat_message, chat_channel: channel), + channel: channel, + ) + end + + before { message.update!(thread: thread) } + + it "generates the correct targets" do + targets = described_class.calculate_publish_targets(channel, message) + expect(targets).to contain_exactly("/chat/#{channel.id}") + end + end + + context "when the message is not part of a thread" do + it "generates the correct targets" do + targets = described_class.calculate_publish_targets(channel, message) + expect(targets).to contain_exactly("/chat/#{channel.id}") + end + end + end + + context "when threading_enabled is false for the channel" do + before do + SiteSetting.enable_experimental_chat_threaded_discussions = true + channel.update!(threading_enabled: false) + end + + context "when the message is the original message of a thread" do + fab!(:thread) { Fabricate(:chat_thread, original_message: message, channel: channel) } + + it "generates the correct targets" do + targets = described_class.calculate_publish_targets(channel, message) + expect(targets).to contain_exactly("/chat/#{channel.id}") + end + end + + context "when the message is a thread reply" do + fab!(:thread) do + Fabricate( + :chat_thread, + original_message: Fabricate(:chat_message, chat_channel: channel), + channel: channel, + ) + end + + before { message.update!(thread: thread) } + + it "generates the correct targets" do + targets = described_class.calculate_publish_targets(channel, message) + expect(targets).to contain_exactly("/chat/#{channel.id}") + end + end + + context "when the message is not part of a thread" do + it "generates the correct targets" do + targets = described_class.calculate_publish_targets(channel, message) + expect(targets).to contain_exactly("/chat/#{channel.id}") + end + end + end + + context "when enable_experimental_chat_threaded_discussions is true and threading_enabled is true for the channel" do + before do + channel.update!(threading_enabled: true) + SiteSetting.enable_experimental_chat_threaded_discussions = true + end + + context "when the message is the original message of a thread" do + fab!(:thread) { Fabricate(:chat_thread, original_message: message, channel: channel) } + + it "generates the correct targets" do + targets = described_class.calculate_publish_targets(channel, message) + expect(targets).to contain_exactly( + "/chat/#{channel.id}", + "/chat/#{channel.id}/thread/#{thread.id}", + ) + end + end + + context "when the message is a thread reply" do + fab!(:thread) do + Fabricate( + :chat_thread, + original_message: Fabricate(:chat_message, chat_channel: channel), + channel: channel, + ) + end + + before { message.update!(thread: thread) } + + it "generates the correct targets" do + targets = described_class.calculate_publish_targets(channel, message) + expect(targets).to contain_exactly("/chat/#{channel.id}/thread/#{thread.id}") + end + end + + context "when the message is not part of a thread" do + it "generates the correct targets" do + targets = described_class.calculate_publish_targets(channel, message) + expect(targets).to contain_exactly("/chat/#{channel.id}") + end + end + end + end + + describe ".publish_new!" do + let(:staged_id) { 999 } + + context "when the message is not a thread reply" do + it "publishes to the new_messages_message_bus_channel" do + messages = + MessageBus.track_publish(described_class.new_messages_message_bus_channel(channel.id)) do + described_class.publish_new!(channel, message, staged_id) + end + expect(messages.first.data).to eq( + { + channel_id: channel.id, + message_id: message.id, + user_id: message.user_id, + username: message.user.username, + thread_id: nil, + }, ) end end - context "when the chat message is a thread reply" do + context "when the message is a thread reply" do fab!(:thread) do Fabricate( :chat_thread, @@ -39,16 +169,44 @@ describe Chat::Publisher do before { message.update!(thread: thread) } - it "generates the correct targets" do - targets = described_class.calculate_publish_targets(channel, message) - expect(targets).to contain_exactly("/chat/#{channel.id}/thread/#{thread.id}") - end - end + context "if enable_experimental_chat_threaded_discussions is false" do + before { SiteSetting.enable_experimental_chat_threaded_discussions = false } - context "when the chat message is not part of a thread" do - it "generates the correct targets" do - targets = described_class.calculate_publish_targets(channel, message) - expect(targets).to contain_exactly("/chat/#{channel.id}") + it "publishes to the new_messages_message_bus_channel" do + messages = + MessageBus.track_publish( + described_class.new_messages_message_bus_channel(channel.id), + ) { described_class.publish_new!(channel, message, staged_id) } + expect(messages).not_to be_empty + end + end + + context "if enable_experimental_chat_threaded_discussions is true" do + before { SiteSetting.enable_experimental_chat_threaded_discussions = true } + + context "if threading_enabled is false for the channel" do + before { channel.update!(threading_enabled: false) } + + it "publishes to the new_messages_message_bus_channel" do + messages = + MessageBus.track_publish( + described_class.new_messages_message_bus_channel(channel.id), + ) { described_class.publish_new!(channel, message, staged_id) } + expect(messages).not_to be_empty + end + end + + context "if threading_enabled is true for the channel" do + before { channel.update!(threading_enabled: true) } + + it "does not publish to the new_messages_message_bus_channel" do + messages = + MessageBus.track_publish( + described_class.new_messages_message_bus_channel(channel.id), + ) { described_class.publish_new!(channel, message, staged_id) } + expect(messages).to be_empty + end + end end end end