FIX: Better handling of deleted thread original messages (#22402)

This commit includes several fixes and improvements to thread
original message handling:

1. When a thread's original message is deleted, the thread no longer
   counts as unread for a user
2. When a thread original message is deleted and the user is looking
   at the thread list, it will be removed from the list
3. When a thread original message is restored and the user is looking
   at the thread list, it will be added back to the list if it was
   previously loaded

plugins/chat/app/queries/chat/thread_unreads_query.rb

@@ -44,6 +44,7 @@ module Chat
           INNER JOIN chat_channels ON chat_channels.id = chat_messages.chat_channel_id
           INNER JOIN chat_threads ON chat_threads.id = chat_messages.thread_id AND chat_threads.channel_id = chat_messages.chat_channel_id
           INNER JOIN user_chat_thread_memberships ON user_chat_thread_memberships.thread_id = chat_threads.id
+          INNER JOIN chat_messages AS original_message ON original_message.id = chat_threads.original_message_id
           AND chat_messages.thread_id = memberships.thread_id
           AND chat_messages.user_id != :user_id
           AND user_chat_thread_memberships.user_id = :user_id
@@ -53,6 +54,7 @@ module Chat
           AND chat_messages.id != chat_threads.original_message_id
           AND chat_channels.threading_enabled
           AND user_chat_thread_memberships.notification_level NOT IN (:quiet_notification_levels)
+          AND original_message.deleted_at IS NULL
         ) AS unread_count,
         0 AS mention_count,
         chat_threads.channel_id,

plugins/chat/app/services/chat/lookup_channel_threads.rb

@@ -146,6 +146,10 @@ module Chat
               ON tracked_threads_subquery.thread_id = chat_threads.id",
         )
         .joins(:user_chat_thread_memberships)
+        .joins(
+          "LEFT JOIN chat_messages original_messages ON chat_threads.original_message_id = original_messages.id",
+        )
+        .where("original_messages.deleted_at IS NULL")
         .where(user_chat_thread_memberships_chat_threads: { user_id: guardian.user.id })
     end
 

plugins/chat/assets/javascripts/discourse/components/chat/thread-list.hbs

@@ -1,8 +1,10 @@
 {{#if this.shouldRender}}
   <div
     class="chat-thread-list"
+    {{did-insert this.subscribe}}
     {{did-insert this.loadThreads}}
     {{did-update this.loadThreads @channel}}
+    {{did-update this.subscribe @channel}}
     {{will-destroy this.teardown}}
   >
     {{#if @includeHeader}}

plugins/chat/assets/javascripts/discourse/components/chat/thread-list.js

@@ -1,10 +1,12 @@
 import Component from "@glimmer/component";
+import { bind } from "discourse-common/utils/decorators";
 import { tracked } from "@glimmer/tracking";
 import { action } from "@ember/object";
 import { inject as service } from "@ember/service";
 
 export default class ChatThreadList extends Component {
   @service chat;
+  @service messageBus;
 
   @tracked loading = true;
 
@@ -16,9 +18,30 @@ export default class ChatThreadList extends Component {
       return [];
     }
 
-    return this.args.channel.threadsManager.threads.sort((threadA, threadB) => {
-      // If both are unread we just want to sort by last reply date + time descending.
-      if (threadA.tracking.unreadCount && threadB.tracking.unreadCount) {
+    return this.args.channel.threadsManager.threads
+      .sort((threadA, threadB) => {
+        // If both are unread we just want to sort by last reply date + time descending.
+        if (threadA.tracking.unreadCount && threadB.tracking.unreadCount) {
+          if (
+            threadA.preview.lastReplyCreatedAt >
+            threadB.preview.lastReplyCreatedAt
+          ) {
+            return -1;
+          } else {
+            return 1;
+          }
+        }
+
+        // If one is unread and the other is not, we want to sort the unread one first.
+        if (threadA.tracking.unreadCount) {
+          return -1;
+        }
+
+        if (threadB.tracking.unreadCount) {
+          return 1;
+        }
+
+        // If both are read, we want to sort by last reply date + time descending.
         if (
           threadA.preview.lastReplyCreatedAt >
           threadB.preview.lastReplyCreatedAt
@@ -27,32 +50,65 @@ export default class ChatThreadList extends Component {
         } else {
           return 1;
         }
-      }
-
-      // If one is unread and the other is not, we want to sort the unread one first.
-      if (threadA.tracking.unreadCount) {
-        return -1;
-      }
-
-      if (threadB.tracking.unreadCount) {
-        return 1;
-      }
-
-      // If both are read, we want to sort by last reply date + time descending.
-      if (
-        threadA.preview.lastReplyCreatedAt > threadB.preview.lastReplyCreatedAt
-      ) {
-        return -1;
-      } else {
-        return 1;
-      }
-    });
+      })
+      .filter((thread) => !thread.originalMessage.deletedAt);
   }
 
   get shouldRender() {
     return !!this.args.channel;
   }
 
+  @action
+  subscribe() {
+    this.#unsubscribe();
+
+    this.messageBus.subscribe(
+      `/chat/${this.args.channel.id}`,
+      this.onMessageBus,
+      this.args.channel.messageBusLastId
+    );
+  }
+
+  @bind
+  onMessageBus(busData) {
+    switch (busData.type) {
+      case "delete":
+        this.handleDeleteMessage(busData);
+        break;
+      case "restore":
+        this.handleRestoreMessage(busData);
+        break;
+    }
+  }
+
+  handleDeleteMessage(data) {
+    const deletedOriginalMessageThread =
+      this.args.channel.threadsManager.threads.findBy(
+        "originalMessage.id",
+        data.deleted_id
+      );
+
+    if (!deletedOriginalMessageThread) {
+      return;
+    }
+
+    deletedOriginalMessageThread.originalMessage.deletedAt = new Date();
+  }
+
+  handleRestoreMessage(data) {
+    const restoredOriginalMessageThread =
+      this.args.channel.threadsManager.threads.findBy(
+        "originalMessage.id",
+        data.chat_message.id
+      );
+
+    if (!restoredOriginalMessageThread) {
+      return;
+    }
+
+    restoredOriginalMessageThread.originalMessage.deletedAt = null;
+  }
+
   @action
   loadThreads() {
     this.loading = true;
@@ -64,5 +120,13 @@ export default class ChatThreadList extends Component {
   @action
   teardown() {
     this.loading = true;
+    this.#unsubscribe();
+  }
+
+  #unsubscribe() {
+    this.messageBus.unsubscribe(
+      `/chat/${this.args.channel.id}`,
+      this.onMessageBus
+    );
   }
 }

plugins/chat/spec/queries/chat/thread_unreads_query_spec.rb

@@ -93,6 +93,13 @@ describe Chat::ThreadUnreadsQuery do
         )
       end
 
+      it "does not count the thread as unread if the original message is deleted" do
+        thread_1.original_message.destroy
+        expect(query.map(&:to_h).find { |tracking| tracking[:thread_id] == thread_1.id }).to eq(
+          { channel_id: channel_1.id, mention_count: 0, thread_id: thread_1.id, unread_count: 0 },
+        )
+      end
+
       context "when include_read is false" do
         let(:include_read) { false }
 

plugins/chat/spec/system/page_objects/chat/chat_channel.rb

@@ -58,6 +58,10 @@ module PageObjects
         has_css?(".chat-selection-management")
       end
 
+      def expand_deleted_message(message)
+        message_by_id(message.id).find(".chat-message-expand").click
+      end
+
       def expand_message_actions(message)
         hover_message(message)
         click_more_button

plugins/chat/spec/system/page_objects/chat/chat_thread.rb

@@ -132,10 +132,29 @@ module PageObjects
         end
       end
 
+      def expand_deleted_message(message)
+        message_by_id(message.id).find(".chat-message-expand").click
+      end
+
       def copy_link(message)
+        expand_message_actions(message)
+        find("[data-value='copyLink']").click
+      end
+
+      def delete_message(message)
+        expand_message_actions(message)
+        find("[data-value='delete']").click
+      end
+
+      def restore_message(message)
+        expand_deleted_message(message)
+        expand_message_actions(message)
+        find("[data-value='restore']").click
+      end
+
+      def expand_message_actions(message)
         hover_message(message)
         click_more_button
-        find("[data-value='copyLink']").click
       end
 
       def click_more_button

plugins/chat/spec/system/page_objects/chat/components/thread_list.rb

@@ -23,6 +23,10 @@ module PageObjects
           item_by_id(thread.id)
         end
 
+        def has_no_thread?(thread)
+          component.has_no_css?(item_by_id_selector(thread.id))
+        end
+
         def item_by_id(id)
           component.find(item_by_id_selector(id))
         end

plugins/chat/spec/system/thread_list/full_page_spec.rb

@@ -5,6 +5,7 @@ describe "Thread list in side panel | full page", type: :system do
   fab!(:channel) { Fabricate(:chat_channel, threading_enabled: true) }
   fab!(:other_user) { Fabricate(:user) }
 
+  let(:side_panel_page) { PageObjects::Pages::ChatSidePanel.new }
   let(:chat_page) { PageObjects::Pages::Chat.new }
   let(:channel_page) { PageObjects::Pages::ChatChannel.new }
   let(:side_panel) { PageObjects::Pages::ChatSidePanel.new }
@@ -88,6 +89,53 @@ describe "Thread list in side panel | full page", type: :system do
       expect(side_panel).to have_open_thread(thread_1)
     end
 
+    describe "deleting and restoring the original message of the thread" do
+      before do
+        thread_1.update!(original_message_user: other_user)
+        thread_1.original_message.update!(user: other_user)
+      end
+
+      it "hides the thread in the list when another user deletes the original message" do
+        chat_page.visit_channel(channel)
+        channel_page.open_thread_list
+        expect(thread_list_page).to have_thread(thread_1)
+
+        using_session(:tab_2) do |session|
+          sign_in(other_user)
+          chat_page.visit_thread(thread_1)
+          expect(side_panel_page).to have_open_thread(thread_1)
+          thread_page.delete_message(thread_1.original_message)
+          session.quit
+        end
+
+        expect(thread_list_page).to have_no_thread(thread_1)
+      end
+
+      it "shows the thread in the list when another user restores the original message" do
+        # This is necessary because normal users can't see deleted messages
+        other_user.update!(admin: true)
+        current_user.update!(admin: true)
+
+        thread_1.original_message.trash!
+        chat_page.visit_channel(channel)
+        channel_page.open_thread_list
+        expect(thread_list_page).to have_no_thread(thread_1)
+
+        using_session(:tab_2) do |session|
+          sign_in(other_user)
+          chat_page.visit_channel(channel)
+          expect(channel_page).to have_no_loading_skeleton
+          channel_page.expand_deleted_message(thread_1.original_message)
+          channel_page.message_thread_indicator(thread_1.original_message).click
+          expect(side_panel_page).to have_open_thread(thread_1)
+          thread_page.restore_message(thread_1.original_message)
+          session.quit
+        end
+
+        expect(thread_list_page).to have_thread(thread_1)
+      end
+    end
+
     describe "updating the title of the thread" do
       let(:new_title) { "wow new title" }
 

plugins/chat/spec/system/thread_tracking/full_page_spec.rb

@@ -29,6 +29,12 @@ describe "Thread tracking state | full page", type: :system do
       expect(channel_page).to have_unread_thread_indicator(count: 1)
     end
 
+    it "does not include threads with deleted original messages in the count of threads with unread messages" do
+      thread.original_message.trash!
+      chat_page.visit_channel(channel)
+      expect(thread_page).to have_no_unread_list_indicator
+    end
+
     it "shows an indicator on the unread thread in the list" do
       chat_page.visit_channel(channel)
       channel_page.open_thread_list