FEATURE: Hook up chat bulk delete for threads (#21109)

Followup to bd5c5c4b5f,
this commit hooks up the bulk delete events for chat
messages inside the thread panel, by fanning out the
deleted message IDs based on whether they belong to
a thread or not.

Also adds a system spec to cover this case, as previously
the bulk delete event would have been broken with an incorrect
`typ` rather than `type` hash key.

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

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

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) {

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() {

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) {

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

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

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

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
 

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,
           )