DEV: General code cleanup (#29)

Author: markvanlan

assets/javascripts/discourse/api-initializers/nested-view-redirect.js

@@ -35,7 +35,7 @@ export default apiInitializer((api) => {
   const topicTrackingState = api.container.lookup(
     "service:topic-tracking-state"
   );
-  let composerSavedFromNested = false;
+  let composerSaveInfo = null;
 
   appEvents.on("composer:saved", () => {
     if (!siteSettings.nested_replies_enabled) {
@@ -44,7 +44,11 @@ export default apiInitializer((api) => {
 
     const route = router.currentRouteName;
     if (route?.startsWith("nested")) {
-      composerSavedFromNested = true;
+      const nestedController = api.container.lookup("controller:nested");
+      composerSaveInfo = {
+        topicId: nestedController?.topic?.id,
+        time: Date.now(),
+      };
     }
   });
 
@@ -100,10 +104,17 @@ export default apiInitializer((api) => {
     }
 
     // After composer save on nested route, suppress the redirect to flat view.
-    // Returning null tells routeTo to abort navigation.
-    if (composerSavedFromNested && /^\/t\//.test(path)) {
-      composerSavedFromNested = false;
-      return null;
+    // Returning null tells routeTo to abort navigation. Scoped to the same
+    // topic and expires after 5 seconds to prevent stale flags from
+    // suppressing unrelated navigations.
+    if (composerSaveInfo && /^\/t\//.test(path)) {
+      const match = TOPIC_URL_RE.exec(path);
+      const elapsed = Date.now() - composerSaveInfo.time;
+      const savedTopicId = composerSaveInfo.topicId;
+      composerSaveInfo = null;
+      if (match && parseInt(match[2], 10) === savedTopicId && elapsed < 5000) {
+        return null;
+      }
     }
 
     // If already in flat view, don't redirect to nested (e.g. timeline navigation).
@@ -152,7 +163,8 @@ export default apiInitializer((api) => {
   // state yet), intercept before the topic route's model hook runs.
   // When the topic isn't in tracking state, abort and fetch topic info
   // to determine the category before deciding which view to load.
-  const checkedTopicIds = new Set();
+  const checkedTopicIds = new Map();
+  const CHECKED_TOPIC_TTL_MS = 60_000;
 
   router.on("routeWillChange", (transition) => {
     if (!siteSettings.nested_replies_enabled) {
@@ -205,15 +217,32 @@ export default apiInitializer((api) => {
 
     // Topic not in tracking state (e.g. direct URL entry). Abort, look
     // up the category via a lightweight request, then redirect or resume.
-    if (checkedTopicIds.has(topicId)) {
+    const checkedAt = checkedTopicIds.get(topicId);
+    if (checkedAt && Date.now() - checkedAt < CHECKED_TOPIC_TTL_MS) {
       return;
     }
-    checkedTopicIds.add(topicId);
+    checkedTopicIds.set(topicId, Date.now());
+
+    // Evict stale entries to prevent unbounded growth
+    if (checkedTopicIds.size > 100) {
+      const now = Date.now();
+      for (const [id, time] of checkedTopicIds) {
+        if (now - time > CHECKED_TOPIC_TTL_MS) {
+          checkedTopicIds.delete(id);
+        }
+      }
+    }
 
+    const fromRoute = router.currentRouteName;
     transition.abort();
 
     ajax(`/t/${topicId}.json`, { data: { track_visit: false } })
       .then((data) => {
+        // Bail if user navigated away during the async lookup
+        if (router.currentRouteName !== fromRoute) {
+          return;
+        }
+
         if (isNestedDefault(siteSettings, data.category_id)) {
           const queryParams = {};
           const nearPost = transition.to?.params?.nearPost;
@@ -226,7 +255,9 @@ export default apiInitializer((api) => {
         }
       })
       .catch(() => {
-        // On error, let the normal topic route handle it
+        if (router.currentRouteName !== fromRoute) {
+          return;
+        }
         transition.retry();
       });
   });

assets/javascripts/discourse/controllers/nested.js

@@ -324,8 +324,9 @@ export default class NestedController extends Controller {
     }
 
     if (this.topic?.id && this.messageBusLastId != null) {
+      this._messageBusChannel = `/topic/${this.topic.id}`;
       this.messageBus.subscribe(
-        `/topic/${this.topic.id}`,
+        this._messageBusChannel,
         this._onMessage,
         this.messageBusLastId
       );
@@ -346,7 +347,10 @@ export default class NestedController extends Controller {
       );
       this._postEventsSubscribed = false;
     }
-    this.messageBus.unsubscribe("/topic/*", this._onMessage);
+    if (this._messageBusChannel) {
+      this.messageBus.unsubscribe(this._messageBusChannel, this._onMessage);
+      this._messageBusChannel = null;
+    }
     this.postRegistry.clear();
   }
 
@@ -406,14 +410,31 @@ export default class NestedController extends Controller {
   }
 
   async _handlePostChanged(data) {
+    if (data.type === "deleted") {
+      this._markPostDeletedLocally(data.id);
+      return;
+    }
+
     try {
       const postData = await ajax(`/posts/${data.id}.json`);
-      this.store.createRecord("post", postData);
+      const post = this.store.createRecord("post", postData);
+      post.topic = this.topic;
     } catch {
       // Post may not be visible
     }
   }
 
+  _markPostDeletedLocally(postId) {
+    for (const post of this.postRegistry.values()) {
+      if (post.id === postId) {
+        post.set("deleted", true);
+        post.set("deleted_post_placeholder", true);
+        post.set("cooked", "");
+        break;
+      }
+    }
+  }
+
   @action
   async loadNewRoots() {
     const ids = [...this.newRootPostIds];

assets/javascripts/discourse/lib/nested-post-url.js

@@ -1,3 +1,5 @@
+import getURL from "discourse/lib/get-url";
+
 export default function nestedPostUrl(topic, postNumber) {
-  return `/nested/${topic.slug}/${topic.id}?post_number=${postNumber}`;
+  return getURL(`/nested/${topic.slug}/${topic.id}?post_number=${postNumber}`);
 }

lib/discourse_nested_replies/post_preloader.rb

@@ -62,6 +62,35 @@ def pluck(*columns)
           map { |record| columns.map { |col| record.public_send(col) } }
         end
       end
+
+      def where(conditions = nil, *rest)
+        return self if conditions.nil?
+        result =
+          select do |record|
+            if conditions.is_a?(Hash)
+              conditions.all? { |k, v| record.public_send(k) == v }
+            else
+              true
+            end
+          end
+        PostsArray.new(result)
+      end
+
+      def limit(_n)
+        self
+      end
+
+      def order(*_args)
+        self
+      end
+
+      def not(*_args)
+        self
+      end
+
+      def reorder(*_args)
+        self
+      end
     end
 
     # Batch-preload associations that plugin serializer extensions access per-post.