diff --git a/plugins/chat/app/models/chat/thread.rb b/plugins/chat/app/models/chat/thread.rb index f78fc05f02e..6b1eafedaea 100644 --- a/plugins/chat/app/models/chat/thread.rb +++ b/plugins/chat/app/models/chat/thread.rb @@ -34,6 +34,21 @@ module Chat original_message.excerpt(max_length: EXCERPT_LENGTH) end + def self.grouped_messages(thread_ids: nil, message_ids: nil, include_original_message: true) + DB.query(<<~SQL, message_ids: message_ids, thread_ids: thread_ids) + SELECT thread_id, + array_agg(chat_messages.id ORDER BY chat_messages.created_at, chat_messages.id) AS thread_message_ids, + chat_threads.original_message_id + FROM chat_messages + INNER JOIN chat_threads ON chat_threads.id = chat_messages.thread_id + WHERE thread_id IS NOT NULL + #{thread_ids ? "AND thread_id IN (:thread_ids)" : ""} + #{message_ids ? "AND chat_messages.id IN (:message_ids)" : ""} + #{include_original_message ? "" : "AND chat_messages.id != chat_threads.original_message_id"} + GROUP BY thread_id, chat_threads.original_message_id; + SQL + end + def self.ensure_consistency! update_counts end diff --git a/plugins/chat/app/services/chat/publisher.rb b/plugins/chat/app/services/chat/publisher.rb index 81e3fd4c736..6b847234dd3 100644 --- a/plugins/chat/app/services/chat/publisher.rb +++ b/plugins/chat/app/services/chat/publisher.rb @@ -167,11 +167,31 @@ module Chat end def self.publish_bulk_delete!(chat_channel, deleted_message_ids) - # TODO (martin) Handle sending this through for all the threads that - # may contain the deleted messages as well. + Chat::Thread + .grouped_messages(message_ids: deleted_message_ids) + .each do |group| + MessageBus.publish( + thread_message_bus_channel(chat_channel.id, group.thread_id), + { + type: "bulk_delete", + deleted_ids: group.thread_message_ids, + deleted_at: Time.zone.now, + }, + permissions(chat_channel), + ) + + # Don't need to publish to the main channel if the messages deleted + # were a part of the thread (except the original message ID, since + # that shows in the main channel). + deleted_message_ids = + deleted_message_ids - (group.thread_message_ids - [group.original_message_id]) + end + + return if deleted_message_ids.empty? + MessageBus.publish( root_message_bus_channel(chat_channel.id), - { typ: "bulk_delete", deleted_ids: deleted_message_ids, deleted_at: Time.zone.now }, + { type: "bulk_delete", deleted_ids: deleted_message_ids, deleted_at: Time.zone.now }, permissions(chat_channel), ) end diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-channel-pane-subscriptions-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-channel-pane-subscriptions-manager.js index 30d4aec8de3..e8d1080baa5 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-channel-pane-subscriptions-manager.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-channel-pane-subscriptions-manager.js @@ -26,15 +26,6 @@ export default class ChatChannelPaneSubscriptionsManager extends ChatPaneBaseSub return; } - handleBulkDeleteMessage(data) { - data.deleted_ids.forEach((deletedId) => { - this.handleDeleteMessage({ - deleted_id: deletedId, - deleted_at: data.deleted_at, - }); - }); - } - handleThreadCreated(data) { const message = this.messagesManager.findMessage(data.chat_message.id); if (message) { diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-pane-subscriptions-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-pane-subscriptions-manager.js index a3e897b028f..bee7089edc2 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-pane-subscriptions-manager.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-channel-thread-pane-subscriptions-manager.js @@ -39,11 +39,6 @@ export default class ChatChannelThreadPaneSubscriptionsManager extends ChatPaneB return; } - // TODO (martin) Hook this up correctly in Chat::Publisher for threads. - handleBulkDeleteMessage() { - return; - } - // NOTE: noop for now, later we may want to do scrolling or something like // we do in the channel pane. afterProcessedMessage() { diff --git a/plugins/chat/assets/javascripts/discourse/services/chat-pane-base-subscriptions-manager.js b/plugins/chat/assets/javascripts/discourse/services/chat-pane-base-subscriptions-manager.js index 3a09348d6a0..d02962207fb 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat-pane-base-subscriptions-manager.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat-pane-base-subscriptions-manager.js @@ -177,8 +177,13 @@ export default class ChatPaneBaseSubscriptionsManager extends Service { } } - handleBulkDeleteMessage() { - throw "not implemented"; + handleBulkDeleteMessage(data) { + data.deleted_ids.forEach((deletedId) => { + this.handleDeleteMessage({ + deleted_id: deletedId, + deleted_at: data.deleted_at, + }); + }); } handleDeleteMessage(data) { diff --git a/plugins/chat/spec/lib/chat/message_mover_spec.rb b/plugins/chat/spec/lib/chat/message_mover_spec.rb index 3c3781be65e..c694b246d9d 100644 --- a/plugins/chat/spec/lib/chat/message_mover_spec.rb +++ b/plugins/chat/spec/lib/chat/message_mover_spec.rb @@ -76,7 +76,7 @@ describe Chat::MessageMover do deleted_messages = Chat::Message.with_deleted.where(id: move_message_ids).order(:id) expect(deleted_messages.count).to eq(3) expect(messages.first.channel).to eq("/chat/#{source_channel.id}") - expect(messages.first.data[:typ]).to eq("bulk_delete") + expect(messages.first.data[:type]).to eq("bulk_delete") expect(messages.first.data[:deleted_ids]).to eq(deleted_messages.map(&:id)) expect(messages.first.data[:deleted_at]).not_to eq(nil) end diff --git a/plugins/chat/spec/models/chat/thread_spec.rb b/plugins/chat/spec/models/chat/thread_spec.rb index dbed433a1c3..f48a4f04f94 100644 --- a/plugins/chat/spec/models/chat/thread_spec.rb +++ b/plugins/chat/spec/models/chat/thread_spec.rb @@ -42,4 +42,107 @@ RSpec.describe Chat::Thread do end end end + + describe ".grouped_messages" do + fab!(:channel) { Fabricate(:category_channel) } + fab!(:thread_1) { Fabricate(:chat_thread, channel: channel) } + fab!(:thread_2) { Fabricate(:chat_thread, channel: channel) } + + fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel, thread: thread_1) } + fab!(:message_2) { Fabricate(:chat_message, chat_channel: channel, thread: thread_1) } + fab!(:message_3) { Fabricate(:chat_message, chat_channel: channel, thread: thread_2) } + + let(:result) { Chat::Thread.grouped_messages(**params) } + + context "when thread_ids provided" do + let(:params) { { thread_ids: [thread_1.id, thread_2.id] } } + + it "groups all the message ids in each thread by thread ID" do + expect(result.find { |res| res.thread_id == thread_1.id }.to_h).to eq( + { + thread_message_ids: [thread_1.original_message_id, message_1.id, message_2.id], + thread_id: thread_1.id, + original_message_id: thread_1.original_message_id, + }, + ) + expect(result.find { |res| res.thread_id == thread_2.id }.to_h).to eq( + { + thread_message_ids: [thread_2.original_message_id, message_3.id], + thread_id: thread_2.id, + original_message_id: thread_2.original_message_id, + }, + ) + end + + context "when include_original_message is false" do + let(:params) { { thread_ids: [thread_1.id, thread_2.id], include_original_message: false } } + + it "does not include the original message in the thread_message_ids" do + expect(result.find { |res| res.thread_id == thread_1.id }.to_h).to eq( + { + thread_message_ids: [message_1.id, message_2.id], + thread_id: thread_1.id, + original_message_id: thread_1.original_message_id, + }, + ) + end + end + end + + context "when message_ids provided" do + let(:params) do + { + message_ids: [ + thread_1.original_message_id, + thread_2.original_message_id, + message_1.id, + message_2.id, + message_3.id, + ], + } + end + + it "groups all the message ids in each thread by thread ID" do + expect(result.find { |res| res.thread_id == thread_1.id }.to_h).to eq( + { + thread_message_ids: [thread_1.original_message_id, message_1.id, message_2.id], + thread_id: thread_1.id, + original_message_id: thread_1.original_message_id, + }, + ) + expect(result.find { |res| res.thread_id == thread_2.id }.to_h).to eq( + { + thread_message_ids: [thread_2.original_message_id, message_3.id], + thread_id: thread_2.id, + original_message_id: thread_2.original_message_id, + }, + ) + end + + context "when include_original_message is false" do + let(:params) do + { + message_ids: [ + thread_1.original_message_id, + thread_2.original_message_id, + message_1.id, + message_2.id, + message_3.id, + ], + include_original_message: false, + } + end + + it "does not include the original message in the thread_message_ids" do + expect(result.find { |res| res.thread_id == thread_1.id }.to_h).to eq( + { + thread_message_ids: [message_1.id, message_2.id], + thread_id: thread_1.id, + original_message_id: thread_1.original_message_id, + }, + ) + end + end + end + end end diff --git a/plugins/chat/spec/system/deleted_message_spec.rb b/plugins/chat/spec/system/deleted_message_spec.rb index aa6d4f9237c..384c64d2c1a 100644 --- a/plugins/chat/spec/system/deleted_message_spec.rb +++ b/plugins/chat/spec/system/deleted_message_spec.rb @@ -24,4 +24,49 @@ RSpec.describe "Deleted message", type: :system, js: true do expect(page).to have_content(I18n.t("js.chat.deleted")) end end + + context "when bulk deleting messages across the channel and a thread" do + let(:side_panel) { PageObjects::Pages::ChatSidePanel.new } + let(:open_thread) { PageObjects::Pages::ChatThread.new } + + fab!(:other_user) { Fabricate(:user) } + fab!(:message_1) { Fabricate(:chat_message, chat_channel: channel_1, user: other_user) } + fab!(:message_2) { Fabricate(:chat_message, chat_channel: channel_1, user: other_user) } + fab!(:message_3) { Fabricate(:chat_message, chat_channel: channel_1, user: other_user) } + + fab!(:thread) { Fabricate(:chat_thread, channel: channel_1) } + fab!(:message_4) do + Fabricate(:chat_message, chat_channel: channel_1, user: other_user, thread: thread) + end + fab!(:message_5) do + Fabricate(:chat_message, chat_channel: channel_1, user: other_user, thread: thread) + end + + before do + channel_1.update!(threading_enabled: true) + SiteSetting.enable_experimental_chat_threaded_discussions = true + chat_system_user_bootstrap(user: other_user, channel: channel_1) + end + + it "hides the deleted messages" do + chat_page.visit_channel(channel_1) + channel_page.message_thread_indicator(thread.original_message).click + expect(side_panel).to have_open_thread(thread) + + expect(channel_page).to have_message(id: message_1.id) + expect(channel_page).to have_message(id: message_2.id) + expect(open_thread).to have_message(thread.id, id: message_4.id) + expect(open_thread).to have_message(thread.id, id: message_5.id) + + Chat::Publisher.publish_bulk_delete!( + channel_1, + [message_1.id, message_2.id, message_4.id, message_5.id].flatten, + ) + + expect(channel_page).to have_no_message(id: message_1.id) + expect(channel_page).to have_no_message(id: message_2.id) + expect(open_thread).to have_no_message(thread.id, id: message_4.id) + expect(open_thread).to have_no_message(thread.id, id: message_5.id) + end + end end diff --git a/plugins/chat/spec/system/page_objects/chat/chat_channel.rb b/plugins/chat/spec/system/page_objects/chat/chat_channel.rb index 8d0c8db253b..f2e193ade82 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat_channel.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat_channel.rb @@ -157,10 +157,19 @@ module PageObjects end def has_message?(text: nil, id: nil) + check_message_presence(exists: true, text: text, id: id) + end + + def has_no_message?(text: nil, id: nil) + check_message_presence(exists: false, text: text, id: id) + end + + def check_message_presence(exists: true, text: nil, id: nil) + css_method = exists ? :has_css? : :has_no_css? if text - has_css?(".chat-message-text", text: text) + send(css_method, ".chat-message-text", text: text, wait: 5) elsif id - has_css?(".chat-message-container[data-id=\"#{id}\"]", wait: 10) + send(css_method, ".chat-message-container[data-id=\"#{id}\"]", wait: 10) end end diff --git a/plugins/chat/spec/system/page_objects/chat/chat_thread.rb b/plugins/chat/spec/system/page_objects/chat/chat_thread.rb index e9ac6dd96b9..cf857d805fb 100644 --- a/plugins/chat/spec/system/page_objects/chat/chat_thread.rb +++ b/plugins/chat/spec/system/page_objects/chat/chat_thread.rb @@ -49,10 +49,25 @@ module PageObjects end def has_message?(thread_id, text: nil, id: nil) + check_message_presence(thread_id, exists: true, text: text, id: id) + end + + def has_no_message?(thread_id, text: nil, id: nil) + check_message_presence(thread_id, exists: false, text: text, id: id) + end + + def check_message_presence(thread_id, exists: true, text: nil, id: nil) + css_method = exists ? :has_css? : :has_no_css? if text - find(thread_selector_by_id(thread_id)).has_css?(".chat-message-text", text: text, wait: 5) + find(thread_selector_by_id(thread_id)).send( + css_method, + ".chat-message-text", + text: text, + wait: 5, + ) elsif id - find(thread_selector_by_id(thread_id)).has_css?( + find(thread_selector_by_id(thread_id)).send( + css_method, ".chat-message-container[data-id=\"#{id}\"]", wait: 10, )