From 2449434fef622ed14ecce06f622af918da403f15 Mon Sep 17 00:00:00 2001 From: Corey Alexander Date: Tue, 26 Apr 2022 23:49:37 -0400 Subject: [PATCH] feat: Allow Document to be fetched without Slug (#3453) * Allow Document to be fetched without Slug Fixes #3423 This PR refactors the `Document.findByPk` method to not require the `slug` portion of the urlID. Before this function accepted two different 'formats' for the ID. - The `uuid` ID of the Document - The full `urlID` which looked something like `some-document-1234567890` However the `some-document` slug portion of this identifier wasn't actually used when looking for a document. We now allow searching by JUST the postfix of the `urlID`, in the above example that is `1234567890`. We do this via a new Regex pattern to match on that just looks for the right looking id alone, without the prefix. This codepath looks the same as when we find it by the full `urlID` besides the different regex that we match on. The issue #3423 mentions that this should apply to all the API endpoints. I believe that this `findByPk` method is all that should be needed for that change. But if this is incorrect, OR you would like more test coverage on the API endpoints as a more 'end to end test' please let me know! * Change original regex to make the slug optional This has the, I believe to be good, side-effect of making the same logic apply to `Collection` as well. Since `Collection` was always doing the same stripping of the slug before the lookup I believe it should be just as safe to do there. We don't have to touch the code in Collections but we add a test of this behavior there as well. * No reason to rename this now that we aren't doing two matches --- server/models/Collection.test.ts | 7 +++++++ server/models/Document.test.ts | 7 +++++++ shared/utils/urlHelpers.ts | 2 +- 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/server/models/Collection.test.ts b/server/models/Collection.test.ts index 03999f7ef..8318bd3c9 100644 --- a/server/models/Collection.test.ts +++ b/server/models/Collection.test.ts @@ -372,6 +372,13 @@ describe("#findByPk", () => { expect(response!.id).toBe(collection.id); }); + test("should return collection when urlId is present, but missing slug", async () => { + const collection = await buildCollection(); + const id = collection.urlId; + const response = await Collection.findByPk(id); + expect(response!.id).toBe(collection.id); + }); + test("should return undefined when incorrect uuid type", async () => { const collection = await buildCollection(); const response = await Collection.findByPk(collection.id + "-incorrect"); diff --git a/server/models/Document.test.ts b/server/models/Document.test.ts index f06a37f27..f894844de 100644 --- a/server/models/Document.test.ts +++ b/server/models/Document.test.ts @@ -532,6 +532,13 @@ describe("#findByPk", () => { const response = await Document.findByPk(id); expect(response?.id).toBe(document.id); }); + + test("should return document when urlId is given without the slug prefix", async () => { + const { document } = await seed(); + const id = document.urlId; + const response = await Document.findByPk(id); + expect(response?.id).toBe(document.id); + }); }); describe("tasks", () => { diff --git a/shared/utils/urlHelpers.ts b/shared/utils/urlHelpers.ts index 1749c4f29..e665d0052 100644 --- a/shared/utils/urlHelpers.ts +++ b/shared/utils/urlHelpers.ts @@ -51,4 +51,4 @@ export function signin(service = "slack"): string { return `${process.env.URL}/auth/${service}`; } -export const SLUG_URL_REGEX = /^[0-9a-zA-Z-_~]*-([a-zA-Z0-9]{10,15})$/; +export const SLUG_URL_REGEX = /^(?:[0-9a-zA-Z-_~]*-)?([a-zA-Z0-9]{10,15})$/;