DEV: Refactor DiscourseURL redirects (#24372)

Previously this was being handled in two places:

1. As a monkey-patch to the Ember router. This would 'trick' the router into rendering a different route, but would leave the browser URL bar unchanged. Many possible bugs can come from this state

2. In the DiscourseURL.routeTo function. This functioned fine as a redirect, but wouldn't have any effect when the transition is handled by Ember

This commit refactors things so that the DiscourseURL redirects are handled the same as our permalinks. When the Ember 'unknown' route is hit, we check for a possible rewrite and redirect there. This is a supported way of doing things, and should be more robust going forwards.

app/assets/javascripts/discourse/app/instance-initializers/url-redirects.js

@@ -1,21 +1,10 @@
 import DiscourseURL from "discourse/lib/url";
 import { initializeDefaultHomepage } from "discourse/lib/utilities";
-import escapeRegExp from "discourse-common/utils/escape-regexp";
 
 export default {
   after: "inject-objects",
 
   initialize(owner) {
-    const currentUser = owner.lookup("service:current-user");
-    if (currentUser) {
-      const username = currentUser.get("username");
-      const escapedUsername = escapeRegExp(username);
-      DiscourseURL.rewrite(
-        new RegExp(`^/u/${escapedUsername}/?$`, "i"),
-        `/u/${username}/activity`
-      );
-    }
-
     // We are still using these for now
     DiscourseURL.rewrite(/^\/group\//, "/groups/");
     DiscourseURL.rewrite(/^\/groups$/, "/g");

app/assets/javascripts/discourse/app/lib/url.js

@@ -247,8 +247,6 @@ const DiscourseURL = EmberObject.extend({
       path = withoutPrefix(path);
     }
 
-    path = rewritePath(path);
-
     if (typeof opts.afterRouteComplete === "function") {
       schedule("afterRender", opts.afterRouteComplete);
     }

app/assets/javascripts/discourse/app/mapping-router.js

@@ -1,5 +1,4 @@
 import EmberRouter from "@ember/routing/router";
-import { rewritePath } from "discourse/lib/url";
 import { defaultHomepage } from "discourse/lib/utilities";
 import Site from "discourse/models/site";
 import { isTesting } from "discourse-common/config/environment";
@@ -9,7 +8,6 @@ const BareRouter = EmberRouter.extend({
   location: isTesting() ? "none" : "discourse-location",
 
   handleURL(url) {
-    url = rewritePath(url);
     const params = url.split("?");
 
     if (params[0] === "/" || params[0] === "") {

app/assets/javascripts/discourse/app/routes/unknown.js

@@ -1,38 +1,47 @@
+import { inject as service } from "@ember/service";
 import { ajax } from "discourse/lib/ajax";
-import DiscourseURL from "discourse/lib/url";
+import DiscourseURL, { rewritePath } from "discourse/lib/url";
 import DiscourseRoute from "discourse/routes/discourse";
 
-export default DiscourseRoute.extend({
-  model(_, transition) {
+export default class UnknownRoute extends DiscourseRoute {
+  @service router;
+
+  async model(_, transition) {
     const path = transition.intent.url;
 
     if (!this.currentUser && this.siteSettings.login_required) {
       return;
     }
 
-    return ajax("/permalink-check.json", {
+    const rewrittenPath = rewritePath(path);
+    if (rewrittenPath !== path) {
+      this.router.transitionTo(rewrittenPath);
+      return;
+    }
+
+    const permalinkResults = await ajax("/permalink-check.json", {
       data: { path },
-    }).then((results) => {
-      if (results.found) {
-        // Avoid polluting the history stack for external links
-        transition.abort();
-
-        let url = results.target_url;
-
-        if (transition._discourse_anchor) {
-          // Remove the anchor from the permalink if present
-          url = url.split("#")[0];
-
-          // Add the anchor from the transition
-          url += `#${transition._discourse_anchor}`;
-        }
-
-        DiscourseURL.routeTo(url);
-        return "";
-      } else {
-        // 404 body HTML
-        return results.html;
-      }
     });
-  },
-});
+
+    if (permalinkResults.found) {
+      // Avoid polluting the history stack for external links
+      transition.abort();
+
+      let url = permalinkResults.target_url;
+
+      if (transition._discourse_anchor) {
+        // Remove the anchor from the permalink if present
+        url = url.split("#")[0];
+
+        // Add the anchor from the transition
+        url += `#${transition._discourse_anchor}`;
+      }
+
+      DiscourseURL.routeTo(url);
+      return "";
+    } else {
+      // 404 body HTML
+      return permalinkResults.html;
+    }
+  }
+}

app/assets/javascripts/discourse/app/routes/user-index.js

@@ -34,7 +34,7 @@ export default class UserIndex extends DiscourseRoute {
 
     let destination;
     if (viewingMe) {
-      destination = "user.activity";
+      destination = "userActivity";
     } else {
       destination = this.viewingOtherUserDefaultRoute;
     }

app/assets/javascripts/discourse/tests/acceptance/group-index-test.js

@@ -1,4 +1,4 @@
-import { click, visit } from "@ember/test-helpers";
+import { click, currentURL, visit } from "@ember/test-helpers";
 import { test } from "qunit";
 import {
   acceptance,
@@ -145,3 +145,31 @@ acceptance("Group Members", function (needs) {
     );
   });
 });
+
+async function visitWithRedirects(url) {
+  try {
+    await visit(url);
+  } catch (error) {
+    const { message } = error;
+    if (message !== "TransitionAborted") {
+      throw error;
+    }
+  }
+}
+
+acceptance("Old group route redirections", function () {
+  test("/group/discourse is redirected", async function (assert) {
+    await visitWithRedirects("/group/discourse");
+    assert.strictEqual(currentURL(), "/g/discourse");
+  });
+
+  test("/groups/discourse is redirected", async function (assert) {
+    await visitWithRedirects("/groups/discourse");
+    assert.strictEqual(currentURL(), "/g/discourse");
+  });
+
+  test("/groups is redirected", async function (assert) {
+    await visitWithRedirects("/groups");
+    assert.strictEqual(currentURL(), "/g");
+  });
+});