FIX: Doubled up or not tracked threads in thread list (#22631)

This commit fixes two issues with the thread list:

1. All threads were being shown regardless of whether the user had
   a membership in the thread. This was happening because the list
   and the channel share the same thread store, so if the channel
   had OMs with threads we would load them and they showed in the list.
2. Threads created by the user from a staged thread would double up.
   This is because the _cache in the channel threadsManager would use
   the staged thread ID even after we'd replaced the object's ID with
   the actual thread from the DB. The answer to this is to remove and
   re-add the thread to the local cache with the actual ID.

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

@@ -19,7 +19,10 @@ export default class ChatThreadList extends Component {
   // are sent to the thread, and we want the list to react in realtime to this.
   get sortedThreads() {
     return this.threadsManager.threads
-      .filter((thread) => !thread.originalMessage.deletedAt)
+      .filter(
+        (thread) =>
+          thread.currentUserMembership && !thread.originalMessage.deletedAt
+      )
       .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) {

plugins/chat/assets/javascripts/discourse/lib/chat-threads-manager.js

@@ -66,6 +66,10 @@ export default class ChatThreadsManager {
     }
   }
 
+  remove(threadObject) {
+    delete this._cached[threadObject.id];
+  }
+
   add(channel, threadObject, options = {}) {
     let model;
 

plugins/chat/assets/javascripts/discourse/services/chat-pane-base-subscriptions-manager.js

@@ -231,6 +231,16 @@ export default class ChatPaneBaseSubscriptionsManager extends Service {
           stagedThread.id = data.thread_id;
           stagedThread.originalMessage.thread = stagedThread;
           stagedThread.originalMessage.thread.preview.replyCount ??= 1;
+
+          // We have to do this because the thread manager cache is keyed by
+          // staged_thread_id, but the thread_id is what we want to use to
+          // look up the thread, otherwise calls to .find() will not return
+          // the thread by its actual ID, and we will end up with double-ups
+          // in places like the thread list when .add() is called.
+          this.model.threadsManager.remove({ id: data.staged_thread_id });
+          this.model.threadsManager.add(this.model, stagedThread, {
+            replace: true,
+          });
         } else if (data.thread_id) {
           this.model.threadsManager
             .find(this.model.id, data.thread_id, { fetchIfNotFound: true })

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

@@ -26,6 +26,50 @@ describe "Thread list in side panel | full page", type: :system do
     end
   end
 
+  context "for threads the user is not a participant in" do
+    fab!(:thread_om) { Fabricate(:chat_message, chat_channel: channel) }
+
+    before { chat_system_user_bootstrap(user: other_user, channel: channel) }
+
+    it "does not show existing threads in the channel if the user is not tracking them" do
+      Fabricate(:chat_thread, original_message: thread_om, channel: channel)
+      chat_page.visit_channel(channel)
+      channel_page.open_thread_list
+      expect(page).to have_content(I18n.t("js.chat.threads.none"))
+    end
+
+    it "does not show new threads in the channel in the thread list if the user is not tracking them" do
+      chat_page.visit_channel(channel)
+
+      using_session(:other_user) do |session|
+        sign_in(other_user)
+        chat_page.visit_channel(channel)
+        channel_page.reply_to(thread_om)
+        thread_page.send_message("hey everyone!")
+        expect(channel_page).to have_thread_indicator(thread_om)
+        session.quit
+      end
+
+      channel_page.open_thread_list
+      expect(page).to have_content(I18n.t("js.chat.threads.none"))
+    end
+
+    describe "when the user creates a new thread" do
+      it "does not double up the staged thread and the actual thread in the list" do
+        chat_page.visit_channel(channel)
+        channel_page.reply_to(thread_om)
+        thread_page.send_message("hey everyone!")
+        expect(channel_page).to have_thread_indicator(thread_om)
+        thread_page.close
+        channel_page.open_thread_list
+        expect(page).to have_css(
+          thread_list_page.item_by_id_selector(thread_om.reload.thread_id),
+          count: 1,
+        )
+      end
+    end
+  end
+
   context "when there are threads that the user is participating in" do
     fab!(:thread_1) do
       chat_thread_chain_bootstrap(channel: channel, users: [current_user, other_user])