From 97f70edd93a55478eabe51d9bfd0904dd992da36 Mon Sep 17 00:00:00 2001 From: Apoorv Mishra Date: Thu, 8 Sep 2022 13:14:25 +0530 Subject: [PATCH] Permanently redirect to `/s/...` for share links (#4067) --- app/components/Editor.tsx | 3 ++- app/components/SearchListItem.tsx | 5 ++++- .../Sidebar/components/SharedDocumentLink.tsx | 3 ++- app/routes/index.tsx | 13 ++++++++++--- .../Document/components/PublicBreadcrumb.tsx | 7 ++++++- .../Document/components/ReferenceListItem.tsx | 5 ++++- app/utils/routeHelpers.test.ts | 14 ++++++++++++++ app/utils/routeHelpers.ts | 4 ++++ server/models/Share.ts | 2 +- server/presenters/share.ts | 2 +- server/routes/index.test.ts | 12 ++++++------ server/routes/index.ts | 14 +++++++++++--- 12 files changed, 65 insertions(+), 19 deletions(-) create mode 100644 app/utils/routeHelpers.test.ts diff --git a/app/components/Editor.tsx b/app/components/Editor.tsx index 1d6537bd7..88e12d791 100644 --- a/app/components/Editor.tsx +++ b/app/components/Editor.tsx @@ -25,6 +25,7 @@ import { NotFoundError } from "~/utils/errors"; import { uploadFile } from "~/utils/files"; import history from "~/utils/history"; import { isModKey } from "~/utils/keyboard"; +import { sharedDocumentPath } from "~/utils/routeHelpers"; import { isHash } from "~/utils/urls"; import DocumentBreadcrumb from "./DocumentBreadcrumb"; @@ -160,7 +161,7 @@ function Editor(props: Props, ref: React.RefObject | null) { } if (shareId) { - navigateTo = `/share/${shareId}${navigateTo}`; + navigateTo = sharedDocumentPath(shareId, navigateTo); } history.push(navigateTo); diff --git a/app/components/SearchListItem.tsx b/app/components/SearchListItem.tsx index fad5500e3..14f4c045e 100644 --- a/app/components/SearchListItem.tsx +++ b/app/components/SearchListItem.tsx @@ -7,6 +7,7 @@ import breakpoint from "styled-components-breakpoint"; import Document from "~/models/Document"; import Highlight, { Mark } from "~/components/Highlight"; import { hover } from "~/styles"; +import { sharedDocumentPath } from "~/utils/routeHelpers"; type Props = { document: Document; @@ -38,7 +39,9 @@ function DocumentListItem( ref={ref} dir={document.dir} to={{ - pathname: shareId ? `/share/${shareId}${document.url}` : document.url, + pathname: shareId + ? sharedDocumentPath(shareId, document.url) + : document.url, state: { title: document.titleWithDefault, }, diff --git a/app/components/Sidebar/components/SharedDocumentLink.tsx b/app/components/Sidebar/components/SharedDocumentLink.tsx index 90c7d6bb6..e379c2bcc 100644 --- a/app/components/Sidebar/components/SharedDocumentLink.tsx +++ b/app/components/Sidebar/components/SharedDocumentLink.tsx @@ -5,6 +5,7 @@ import Collection from "~/models/Collection"; import Document from "~/models/Document"; import useStores from "~/hooks/useStores"; import { NavigationNode } from "~/types"; +import { sharedDocumentPath } from "~/utils/routeHelpers"; import Disclosure from "./Disclosure"; import SidebarLink from "./SidebarLink"; @@ -92,7 +93,7 @@ function DocumentLink( <> - + + + + diff --git a/app/scenes/Document/components/PublicBreadcrumb.tsx b/app/scenes/Document/components/PublicBreadcrumb.tsx index 66134421e..027221b75 100644 --- a/app/scenes/Document/components/PublicBreadcrumb.tsx +++ b/app/scenes/Document/components/PublicBreadcrumb.tsx @@ -1,6 +1,7 @@ import * as React from "react"; import Breadcrumb from "~/components/Breadcrumb"; import { MenuInternalLink, NavigationNode } from "~/types"; +import { sharedDocumentPath } from "~/utils/routeHelpers"; type Props = { documentId: string; @@ -48,7 +49,11 @@ const PublicBreadcrumb: React.FC = ({ pathToDocument(sharedTree, documentId) .slice(0, -1) .map((item) => { - return { ...item, type: "route", to: `/share/${shareId}${item.url}` }; + return { + ...item, + type: "route", + to: sharedDocumentPath(shareId, item.url), + }; }), [sharedTree, shareId, documentId] ); diff --git a/app/scenes/Document/components/ReferenceListItem.tsx b/app/scenes/Document/components/ReferenceListItem.tsx index 166b527aa..808b78781 100644 --- a/app/scenes/Document/components/ReferenceListItem.tsx +++ b/app/scenes/Document/components/ReferenceListItem.tsx @@ -9,6 +9,7 @@ import EmojiIcon from "~/components/EmojiIcon"; import Flex from "~/components/Flex"; import { hover } from "~/styles"; import { NavigationNode } from "~/types"; +import { sharedDocumentPath } from "~/utils/routeHelpers"; type Props = { shareId?: string; @@ -64,7 +65,9 @@ function ReferenceListItem({ return ( { + test("should return share path for a document", () => { + const shareId = "1c922644-40d8-41fe-98f9-df2b67239d45"; + const docPath = "/doc/test-DjDlkBi77t"; + expect(sharedDocumentPath(shareId)).toBe( + "/s/1c922644-40d8-41fe-98f9-df2b67239d45" + ); + expect(sharedDocumentPath(shareId, docPath)).toBe( + "/s/1c922644-40d8-41fe-98f9-df2b67239d45/doc/test-DjDlkBi77t" + ); + }); +}); diff --git a/app/utils/routeHelpers.ts b/app/utils/routeHelpers.ts index 43d108fea..7a7486655 100644 --- a/app/utils/routeHelpers.ts +++ b/app/utils/routeHelpers.ts @@ -117,6 +117,10 @@ export function searchPath( return `${route}${search}`; } +export function sharedDocumentPath(shareId: string, docPath?: string) { + return docPath ? `/s/${shareId}${docPath}` : `/s/${shareId}`; +} + export function notFoundUrl(): string { return "/404"; } diff --git a/server/models/Share.ts b/server/models/Share.ts index 050c4938f..ffc0d3edc 100644 --- a/server/models/Share.ts +++ b/server/models/Share.ts @@ -92,7 +92,7 @@ class Share extends IdModel { } get canonicalUrl() { - return `${this.team.url}/share/${this.id}`; + return `${this.team.url}/s/${this.id}`; } // associations diff --git a/server/presenters/share.ts b/server/presenters/share.ts index 067ae2553..16ae15a25 100644 --- a/server/presenters/share.ts +++ b/server/presenters/share.ts @@ -8,7 +8,7 @@ export default function present(share: Share, isAdmin = false) { documentTitle: share.document?.title, documentUrl: share.document?.url, published: share.published, - url: `${share.team.url}/share/${share.id}`, + url: share.canonicalUrl, createdBy: presentUser(share.user), includeChildDocuments: share.includeChildDocuments, lastAccessedAt: share.lastAccessedAt || undefined, diff --git a/server/routes/index.test.ts b/server/routes/index.test.ts index 4883144bf..abd9a037c 100644 --- a/server/routes/index.test.ts +++ b/server/routes/index.test.ts @@ -8,19 +8,19 @@ afterAll(server.disconnect); beforeEach(db.flush); -describe("/share/:id", () => { +describe("/s/:id", () => { it("should return standard title in html when loading unpublished share", async () => { const share = await buildShare({ published: false, }); - const res = await server.get(`/share/${share.id}`); + const res = await server.get(`/s/${share.id}`); const body = await res.text(); expect(res.status).toEqual(404); expect(body).toContain("Outline"); }); it("should return standard title in html when share does not exist", async () => { - const res = await server.get(`/share/junk`); + const res = await server.get(`/s/junk`); const body = await res.text(); expect(res.status).toEqual(404); expect(body).toContain("Outline"); @@ -33,7 +33,7 @@ describe("/share/:id", () => { teamId: document.teamId, }); await document.destroy(); - const res = await server.get(`/share/${share.id}`); + const res = await server.get(`/s/${share.id}`); const body = await res.text(); expect(res.status).toEqual(404); expect(body).toContain("Outline"); @@ -45,7 +45,7 @@ describe("/share/:id", () => { documentId: document.id, teamId: document.teamId, }); - const res = await server.get(`/share/${share.id}`); + const res = await server.get(`/s/${share.id}`); const body = await res.text(); expect(res.status).toEqual(200); expect(body).toContain(`${document.title}`); @@ -57,7 +57,7 @@ describe("/share/:id", () => { documentId: document.id, teamId: document.teamId, }); - const res = await server.get(`/share/${share.id}/doc/${document.urlId}`); + const res = await server.get(`/s/${share.id}/doc/${document.urlId}`); const body = await res.text(); expect(res.status).toEqual(200); expect(body).toContain(`${document.title}`); diff --git a/server/routes/index.ts b/server/routes/index.ts index 73af3322a..7e14004a4 100644 --- a/server/routes/index.ts +++ b/server/routes/index.ts @@ -25,6 +25,14 @@ koa.use( koa.use(userAgent); +router.use( + ["/share/:shareId", "/share/:shareId/doc/:documentSlug", "/share/:shareId/*"], + (ctx) => { + ctx.redirect(ctx.path.replace(/^\/share/, "/s")); + ctx.status = 301; + } +); + if (isProduction) { router.get("/static/*", async (ctx) => { try { @@ -84,9 +92,9 @@ router.get("/opensearch.xml", (ctx) => { ctx.body = opensearchResponse(ctx.request.URL.origin); }); -router.get("/share/:shareId", renderShare); -router.get("/share/:shareId/doc/:documentSlug", renderShare); -router.get("/share/:shareId/*", renderShare); +router.get("/s/:shareId", renderShare); +router.get("/s/:shareId/doc/:documentSlug", renderShare); +router.get("/s/:shareId/*", renderShare); // catch all for application router.get("*", renderApp);