diff --git a/web/src/hooks.test.ts b/web/src/hooks.test.ts new file mode 100644 index 00000000..440ffa3f --- /dev/null +++ b/web/src/hooks.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from "vitest"; +import { reroute } from "./hooks"; + +const at = (path: string) => reroute({ url: new URL(`https://x${path}`), fetch }); + +describe("reroute", () => { + const encoded = encodeURIComponent("at://did:plc:abc/sh.tangled.repo.pull/3xyz"); + + it("encodes a raw at-uri", () => { + expect(at("/alice/repo/pulls/at://did:plc:abc/sh.tangled.repo.pull/3xyz/latest")).toBe( + `/alice/repo/pulls/${encoded}/latest` + ); + }); + + it("encodes a collapsed at-uri", () => { + expect(at("/alice/repo/pulls/at:/did:plc:abc/sh.tangled.repo.pull/3xyz")).toBe( + `/alice/repo/pulls/${encoded}` + ); + }); + + it("leaves encoded and unrelated urls alone", () => { + expect(at(`/alice/repo/pulls/${encoded}/1/a..b`)).toBe( + `/alice/repo/pulls/${encoded}/1/a..b` + ); + expect(at("/alice/repo/pulls")).toBe("/alice/repo/pulls"); + }); +}); diff --git a/web/src/hooks.ts b/web/src/hooks.ts new file mode 100644 index 00000000..5b0af2dc --- /dev/null +++ b/web/src/hooks.ts @@ -0,0 +1,12 @@ +import type { Reroute } from "@sveltejs/kit"; + +// small hack to support raw at-uri as path segment: +// `/issues/at://did:example:repo/org.tangled.repo.ticket/123` +const RAW_ATURI = /\/(pulls|issues)\/at:\/\/?([^/]+)\/([^/]+)\/([^/]+)/; + +export const reroute: Reroute = ({ url }) => + url.pathname.replace( + RAW_ATURI, + (_match, section, authority, collection, rkey) => + `/${section}/${encodeURIComponent(`at://${authority}/${collection}/${rkey}`)}` + ); diff --git a/web/src/lib/api/pullRoute.test.ts b/web/src/lib/api/pullRoute.test.ts index 075fd176..bf6e9a07 100644 --- a/web/src/lib/api/pullRoute.test.ts +++ b/web/src/lib/api/pullRoute.test.ts @@ -6,7 +6,6 @@ import { parseDiffRoute, parseInterdiffRoute, parseRange, - pullHref, resolveChange, seeAll, type PullView @@ -150,15 +149,3 @@ describe("resolveChange", () => { }); }); }); - -describe("pullHref", () => { - const uri = "at://did:plc:alice/sh.tangled.repo.pull/3lzg7tkxvq222"; - - it("encodes the at-uri into a single segment", () => { - expect(pullHref("alice.test/core", uri)).toBe( - "/alice.test/core/pulls/at%3A%2F%2Fdid%3Aplc%3Aalice%2Fsh.tangled.repo.pull%2F3lzg7tkxvq222" - ); - expect(pullHref("alice.test/core", uri, 1, "c1..c2")).toMatch(/\/1\/c1\.\.c2$/); - expect(pullHref("alice.test/core", uri, "latest")).toMatch(/\/latest$/); - }); -}); diff --git a/web/src/lib/api/pullRoute.ts b/web/src/lib/api/pullRoute.ts index 09cbdb66..4d306c11 100644 --- a/web/src/lib/api/pullRoute.ts +++ b/web/src/lib/api/pullRoute.ts @@ -1,4 +1,5 @@ -// the pull page's url grammar: +// the pull page's url grammar, where goes in raw and `reroute` in +// src/hooks.ts folds its segments back together before routing: // // /pulls//latest diff of the newest version // /pulls//1 diff of version 1 @@ -161,17 +162,3 @@ export const resolveChange = ( ? { kind: "interdiff", from: spec(from, bases.from), to: spec(to, bases.to) } : { kind: "diff", spec: spec(to, bases.to) }; }; - -// `///pulls//[/]`, the only place the at-uri -// gets re-encoded. generated links use numbers, `latest` is only ever typed -export const pullHref = ( - repoBase: string, - uri: string, - version?: number | string, - range?: string -): string => { - const parts = [repoBase, "pulls", encodeURIComponent(uri)]; - if (version !== undefined) parts.push(String(version)); - if (range) parts.push(range); - return `/${parts.join("/")}`; -}; diff --git a/web/src/lib/components/repo/issues/IssueCardContent.svelte b/web/src/lib/components/repo/issues/IssueCardContent.svelte index d105793d..b9ec4f70 100644 --- a/web/src/lib/components/repo/issues/IssueCardContent.svelte +++ b/web/src/lib/components/repo/issues/IssueCardContent.svelte @@ -14,9 +14,8 @@ let { ownerHandle, repoName, issue }: Props = $props(); - const href = $derived( - resolve(`/${ownerHandle}/${repoName}/issues/${encodeURIComponent(issue.uri)}` as "/") - ); + // the at-uri is appended after `resolve`, which would drop the empty segment in `at://` + const href = $derived(`${resolve(`/${ownerHandle}/${repoName}/issues` as "/")}/${issue.uri}`);
diff --git a/web/src/lib/components/repo/pulls/PullCardContent.svelte b/web/src/lib/components/repo/pulls/PullCardContent.svelte index a834c97b..93cfa39a 100644 --- a/web/src/lib/components/repo/pulls/PullCardContent.svelte +++ b/web/src/lib/components/repo/pulls/PullCardContent.svelte @@ -14,8 +14,9 @@ let { ownerHandle, repoName, pull }: Props = $props(); + // the at-uri is appended after `resolve`, which would drop the empty segment in `at://` const href = $derived( - resolve(`/${ownerHandle}/${repoName}/pulls/${encodeURIComponent(pull.uri)}/latest` as "/") + `${resolve(`/${ownerHandle}/${repoName}/pulls` as "/")}/${pull.uri}/latest` ); diff --git a/web/src/routes/[handle]/[repo]/issues/[aturi]/+page.ts b/web/src/routes/[handle]/[repo]/issues/[aturi]/+page.ts index d2c92ccb..8e0e4a94 100644 --- a/web/src/routes/[handle]/[repo]/issues/[aturi]/+page.ts +++ b/web/src/routes/[handle]/[repo]/issues/[aturi]/+page.ts @@ -14,7 +14,7 @@ import type { PageLoad } from "./$types"; export const load: PageLoad = async (event) => { const parent = await event.parent(); - const uri = decodeURIComponent(event.params.aturi); + const uri = event.params.aturi; if (!uri.startsWith("at://")) error(404, "Issue not found"); diff --git a/web/src/routes/[handle]/[repo]/pulls/[aturi]/+page.ts b/web/src/routes/[handle]/[repo]/pulls/[aturi]/+page.ts index 5ed9d3e3..2b5db5b2 100644 --- a/web/src/routes/[handle]/[repo]/pulls/[aturi]/+page.ts +++ b/web/src/routes/[handle]/[repo]/pulls/[aturi]/+page.ts @@ -1,12 +1,11 @@ import { error, redirect } from "@sveltejs/kit"; -import { pullHref } from "$lib/api/pullRoute"; import type { PageLoad } from "./$types"; // every diff lives under a version segment, so the bare pull url picks one export const load: PageLoad = (event) => { - const uri = decodeURIComponent(event.params.aturi); + const uri = event.params.aturi; if (!uri.startsWith("at://")) error(404, "Pull request not found"); - redirect(307, pullHref(`${event.params.handle}/${event.params.repo}`, uri, "latest")); + redirect(307, `/${event.params.handle}/${event.params.repo}/pulls/${uri}/latest`); }; diff --git a/web/src/routes/[handle]/[repo]/pulls/[aturi]/[version]/[[range]]/+page.svelte b/web/src/routes/[handle]/[repo]/pulls/[aturi]/[version]/[[range]]/+page.svelte index 1f380336..6d864e06 100644 --- a/web/src/routes/[handle]/[repo]/pulls/[aturi]/[version]/[[range]]/+page.svelte +++ b/web/src/routes/[handle]/[repo]/pulls/[aturi]/[version]/[[range]]/+page.svelte @@ -5,7 +5,7 @@ import { mergeCheck } from "$lib/api/gitmirror"; import { getBranch } from "$lib/api/knotmirror"; import { editPull } from "$lib/api/pull"; - import { activeCommitId, activeVersion, pullHref, seeAll } from "$lib/api/pullRoute"; + import { activeCommitId, activeVersion, seeAll } from "$lib/api/pullRoute"; import { createLoad } from "$lib/action.svelte"; import { resizable } from "$lib/actions/resizable"; import { getAuth } from "$lib/auth.svelte"; @@ -22,6 +22,7 @@ import PullInfoBar from "$lib/components/repo/pulls/PullInfoBar.svelte"; import PullMetaPanel from "$lib/components/repo/pulls/PullMetaPanel.svelte"; import TabPanel from "$lib/components/ui/TabPanel.svelte"; + import type { ResolvedPathname } from "$app/types"; import type { PullState } from "$lib/api/records"; import type { Did } from "@atcute/lexicons/syntax"; import type { CommentView, ThreadInput } from "$lib/components/comment/comments"; @@ -138,10 +139,11 @@ const short = (hash: string) => hash.slice(0, 8); - let href = $derived((version?: number | string, range?: string) => - resolve( - pullHref(`${data.repo.ownerHandle}/${data.repo.name}`, data.uri, version, range) as "/" - ) + // the at-uri sits outside `resolve`, which would drop the empty segment in `at://` + let pullsBase = $derived(resolve(`/${data.repo.ownerHandle}/${data.repo.name}/pulls` as "/")); + let href = $derived( + (version: number | string, range?: string) => + `${pullsBase}/${data.uri}/${version}${range ? `/${range}` : ""}` as ResolvedPathname ); // the appview's interdiff button always steps back exactly one version let interdiffRange = $derived(`${active - 1}..${active}`); diff --git a/web/src/routes/[handle]/[repo]/pulls/[aturi]/[version]/[[range]]/+page.ts b/web/src/routes/[handle]/[repo]/pulls/[aturi]/[version]/[[range]]/+page.ts index 8d5672cd..7a61ad9f 100644 --- a/web/src/routes/[handle]/[repo]/pulls/[aturi]/[version]/[[range]]/+page.ts +++ b/web/src/routes/[handle]/[repo]/pulls/[aturi]/[version]/[[range]]/+page.ts @@ -3,7 +3,7 @@ import { createBobbinClient } from "$lib/api/client"; import { enrich, handleOf, TYPE_MINIDOC } from "$lib/api/enrich"; import { gitTarget } from "$lib/api/gitclient"; import { prepareDiff, prepareInterdiff, type PullPageDeps } from "$lib/api/pullPage"; -import { isRedirect, parseDiffRoute, parseInterdiffRoute, pullHref } from "$lib/api/pullRoute"; +import { isRedirect, parseDiffRoute, parseInterdiffRoute } from "$lib/api/pullRoute"; import { getPullView, type CommentListPage, @@ -21,7 +21,7 @@ import type { PageLoad } from "./$types"; export const load: PageLoad = async (event) => { const parent = await event.parent(); - const uri = decodeURIComponent(event.params.aturi); + const uri = event.params.aturi; if (!uri.startsWith("at://")) error(404, "Pull request not found"); @@ -85,7 +85,7 @@ export const load: PageLoad = async (event) => { if (isRedirect(route)) { // a bad version falls back to the newest one, a bad range only drops itself const target = route.redirect === "latest" ? "latest" : event.params.version; - redirect(307, pullHref(`${event.params.handle}/${event.params.repo}`, uri, target)); + redirect(307, `/${event.params.handle}/${event.params.repo}/pulls/${uri}/${target}`); } const diffStyle: DiffStyle = event.url.searchParams.get("diff") === "split" ? "split" : "unified";