FIX: Improve chat route cleanup (#20557)

1. `this.chat.activeChannel = null` was being done in twice
2. using `willTransition()` and checking transition.to.name prefix for route cleanup rather than using `deactivate()` was unnecessarily verbose and could be premature (if something aborted the transition you'd end up in a broken state)
3. `activeChannel` on Chat service can be null, check for that before accessing

plugins/chat/assets/javascripts/discourse/routes/chat-channel-decorator.js

@@ -23,6 +23,8 @@ export default function withChatChannel(extendedClass) {
     }
 
     afterModel(model) {
+      super.afterModel?.(...arguments);
+
       this.controllerFor("chat-channel").set("targetMessageId", null);
       this.chat.activeChannel = model;
 

plugins/chat/assets/javascripts/discourse/routes/chat-channel.js

@@ -1,32 +1,21 @@
 import DiscourseRoute from "discourse/routes/discourse";
 import withChatChannel from "./chat-channel-decorator";
 import { inject as service } from "@ember/service";
-import { action } from "@ember/object";
 
 @withChatChannel
 export default class ChatChannelRoute extends DiscourseRoute {
   @service chat;
   @service chatStateManager;
 
-  @action
-  willTransition(transition) {
+  beforeModel() {
+    if (this.chat.activeChannel) {
+      // When moving between channels
+      this.#closeThread();
+    }
+  }
+
+  deactivate() {
     this.#closeThread();
-
-    if (transition?.to?.name === "chat.channel.index") {
-      const targetChannelId = transition?.to?.parent?.params?.channelId;
-      if (
-        targetChannelId &&
-        parseInt(targetChannelId, 10) !== this.chat.activeChannel.id
-      ) {
-        this.chat.activeChannel.messagesManager.clearMessages();
-      }
-    }
-
-    if (!transition?.to?.name?.startsWith("chat.")) {
-      this.chatStateManager.storeChatURL();
-      this.chat.activeChannel = null;
-      this.chat.updatePresence();
-    }
   }
 
   #closeThread() {

plugins/chat/assets/javascripts/discourse/routes/chat.js

@@ -4,7 +4,6 @@ import { defaultHomepage } from "discourse/lib/utilities";
 import { inject as service } from "@ember/service";
 import { scrollTop } from "discourse/mixins/scroll-top";
 import { schedule } from "@ember/runloop";
-import { action } from "@ember/object";
 
 export default class ChatRoute extends DiscourseRoute {
   @service chat;
@@ -65,23 +64,17 @@ export default class ChatRoute extends DiscourseRoute {
     });
   }
 
-  deactivate() {
+  deactivate(transition) {
+    const url = this.router.urlFor(transition.from.name);
+    this.chatStateManager.storeChatURL(url);
+
+    this.chat.activeChannel = null;
+    this.chat.updatePresence();
+
     schedule("afterRender", () => {
       document.body.classList.remove("has-full-page-chat");
       document.documentElement.classList.remove("has-full-page-chat");
       scrollTop();
     });
   }
-
-  @action
-  willTransition(transition) {
-    if (!transition?.to?.name?.startsWith("chat.channel")) {
-      this.chat.activeChannel = null;
-    }
-
-    if (!transition?.to?.name?.startsWith("chat.")) {
-      this.chatStateManager.storeChatURL();
-      this.chat.updatePresence();
-    }
-  }
 }

plugins/chat/assets/javascripts/discourse/services/chat-state-manager.js

@@ -113,11 +113,17 @@ export default class ChatStateManager extends Service {
   }
 
   storeAppURL(url = null) {
-    this._appURL = url || this.router.currentURL;
+    if (url) {
+      this._appURL = url;
+    } else if (this.router.currentURL?.startsWith("/chat")) {
+      this._appURL = "/";
+    } else {
+      this._appURL = this.router.currentURL;
+    }
   }
 
-  storeChatURL(url = null) {
-    this._chatURL = url || this.router.currentURL;
+  storeChatURL(url) {
+    this._chatURL = url;
   }
 
   get lastKnownAppURL() {

plugins/chat/test/javascripts/unit/services/chat-state-manager-test.js

@@ -53,11 +53,6 @@ module(
     test("lastKnownChatURL", function (assert) {
       assert.strictEqual(this.subject.lastKnownChatURL, "/chat");
 
-      sinon.stub(this.subject.router, "currentURL").value("/foo");
-      this.subject.storeChatURL();
-
-      assert.strictEqual(this.subject.lastKnownChatURL, "/foo");
-
       this.subject.storeChatURL("/bar");
 
       assert.strictEqual(this.subject.lastKnownChatURL, "/bar");