From c0471f0c287317cdfbc94f427e5868461bd4d881 Mon Sep 17 00:00:00 2001
From: Chris Raible <chris@ghost.org>
Date: Wed, 21 Aug 2024 13:45:59 -0700
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20frontend=20routing=20pri?=
 =?UTF-8?q?oritizing=20collections=20over=20built=20in=20routes=20(#20765)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

ref
https://linear.app/tryghost/issue/ONC-242/frontend-routing-prioritizes-collections-over-taxonomies

- Under a fairly specific edge case with a collection route that conflicts with a default, built-in route ("taxonomy" — like tags, authors, etc), the frontend routing would prioritize the collection over the taxonomy.

- For example, with the following in a custom `routes.yaml`:
```
collections:
  /:
    permalink: /{primary_tag}/{slug}/
    template: index
```

If a post exists with the same slug as its primary tag's slug, the frontend routing would redirect the `/tag/{slug}/` route to the post in the collection, rather than serving the tag itself.

- This commit changes that, so if a collection's route conflicts with e.g. a `/tag/{slug}/` default route, Ghost will still return the built in route, rather than the collection.
---
 .../services/routing/RouterManager.js         | 14 ++++----
 .../test/e2e-frontend/custom_routes.test.js   | 33 +++++++++++++++++++
 .../fixtures/settings/edgecaseroutes.yaml     | 10 ++++++
 ghost/core/test/utils/index.js                |  7 ++++
 4 files changed, 57 insertions(+), 7 deletions(-)
 create mode 100644 ghost/core/test/utils/fixtures/settings/edgecaseroutes.yaml

diff --git a/ghost/core/core/frontend/services/routing/RouterManager.js b/ghost/core/core/frontend/services/routing/RouterManager.js
index 7960be6117..1e92eaa2fa 100644
--- a/ghost/core/core/frontend/services/routing/RouterManager.js
+++ b/ghost/core/core/frontend/services/routing/RouterManager.js
@@ -126,6 +126,13 @@ class RouterManager {
             this.registry.setRouter(staticRoutesRouter.identifier, staticRoutesRouter);
         });
 
+        _.each(routerSettings.taxonomies, (value, key) => {
+            const taxonomyRouter = new TaxonomyRouter(key, value, RESOURCE_CONFIG, this.routerCreated.bind(this));
+            this.siteRouter.mountRouter(taxonomyRouter.router());
+
+            this.registry.setRouter(taxonomyRouter.identifier, taxonomyRouter);
+        });
+
         _.each(routerSettings.collections, (value, key) => {
             const collectionRouter = new CollectionRouter(key, value, RESOURCE_CONFIG, this.routerCreated.bind(this));
             this.siteRouter.mountRouter(collectionRouter.router());
@@ -137,13 +144,6 @@ class RouterManager {
 
         this.registry.setRouter('staticPagesRouter', staticPagesRouter);
 
-        _.each(routerSettings.taxonomies, (value, key) => {
-            const taxonomyRouter = new TaxonomyRouter(key, value, RESOURCE_CONFIG, this.routerCreated.bind(this));
-            this.siteRouter.mountRouter(taxonomyRouter.router());
-
-            this.registry.setRouter(taxonomyRouter.identifier, taxonomyRouter);
-        });
-
         const appRouter = new ParentRouter('AppsRouter');
         this.siteRouter.mountRouter(appRouter.router());
 
diff --git a/ghost/core/test/e2e-frontend/custom_routes.test.js b/ghost/core/test/e2e-frontend/custom_routes.test.js
index 7c4977662c..fa1d1eab06 100644
--- a/ghost/core/test/e2e-frontend/custom_routes.test.js
+++ b/ghost/core/test/e2e-frontend/custom_routes.test.js
@@ -80,3 +80,36 @@ describe('Custom Frontend routing', function () {
             .expect(assertCorrectFrontendHeaders);
     });
 });
+
+describe('Custom frontend routing - edge cases', function () {
+    let request;
+
+    before(async function () {
+        const routesFilePath = path.join(configUtils.config.get('paths:appRoot'), 'test/utils/fixtures/settings/edgecaseroutes.yaml');
+        await configUtils.restore();
+        await testUtils.stopGhost();
+
+        await testUtils.startGhost({
+            forceStart: true,
+            routesFilePath,
+            subdir: false
+        });
+
+        request = supertest.agent(configUtils.config.get('url'));
+    });
+
+    after(async function () {
+        await testUtils.stopGhost();
+    });
+
+    it('should prioritize taxonomies over collections', async function () {
+        // Create a tag and a post with the same slug
+        const slug = 'cheese';
+        await testUtils.createTag({tag: {slug}});
+        await testUtils.createPost({post: {tags: [{slug}], slug}});
+
+        // Test that the tag route takes precedence
+        await request.get(`/tag/${slug}/`)
+            .expect(200);
+    });
+});
diff --git a/ghost/core/test/utils/fixtures/settings/edgecaseroutes.yaml b/ghost/core/test/utils/fixtures/settings/edgecaseroutes.yaml
new file mode 100644
index 0000000000..d4a8a4b133
--- /dev/null
+++ b/ghost/core/test/utils/fixtures/settings/edgecaseroutes.yaml
@@ -0,0 +1,10 @@
+routes:
+
+collections:
+ /:
+   permalink: /{primary_tag}/{slug}/
+   template: index
+
+taxonomies:
+ tag: /tag/{slug}/
+ author: /author/{slug}/
\ No newline at end of file
diff --git a/ghost/core/test/utils/index.js b/ghost/core/test/utils/index.js
index b43d59bc6f..6e4fd34cfa 100644
--- a/ghost/core/test/utils/index.js
+++ b/ghost/core/test/utils/index.js
@@ -79,6 +79,12 @@ const createPost = function createPost(options) {
     return models.Post.add(post, context.internal);
 };
 
+const createTag = function createTag(options) {
+    const tag = DataGenerator.forKnex.createTag(options.tag);
+
+    return models.Tag.add(tag, context.internal);
+};
+
 const createEmail = function createEmail(options) {
     const email = DataGenerator.forKnex.createEmail(options.email);
     return models.Email.add(email, context.internal);
@@ -103,6 +109,7 @@ module.exports = {
     setup: setup,
     createUser: createUser,
     createPost: createPost,
+    createTag: createTag,
     createEmailedPost,
 
     /**