Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions internal/webapp/frontend/e2e/browse.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,24 @@ test("an old version of an extensionless file previews the same way", async ({ p
await expect(page.locator("#content .empty")).toContainText("That version isn't available.");
});

// BEA-83. A deep link to a project id you can't see used to swap in another
// project and throw the path away, with nothing on screen to say so.
test("a bogus project deep link says so and keeps the URL", async ({ page }) => {
await login(page);
await page.goto("/no-such-project-xyz/some/file.md");
await expect(page.locator("#content .empty")).toContainText("Project not found");
expect(page.url()).toContain("/no-such-project-xyz/some/file.md");
await expect(page.locator("#sidebar")).toBeVisible();
await page.reload(); // no bounce, no loop
await expect(page.locator("#content .empty")).toContainText("Project not found");
expect(page.url()).toContain("/no-such-project-xyz/some/file.md");
// The two other URL rewrites off current.id must not undo the fix either.
await page.goto("/no-such-project-xyz/insights");
expect(page.url()).toContain("/no-such-project-xyz/insights");
await page.goto("/no-such-project-xyz/notes/");
expect(page.url()).toContain("/no-such-project-xyz/notes/");
});

// BEA-81: an old URL for a file that has since been renamed or dragged into
// a folder still lands on the file, rewrites itself, and says what happened.
test("a moved file's old URL redirects and says so", async ({ page }) => {
Expand Down
8 changes: 6 additions & 2 deletions internal/webapp/frontend/e2e/hub.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,14 @@ test("deep link to a project resolves after reload", async ({ page }) => {
await expect(page).toHaveURL("/" + pid);
});

test("unknown project id falls back to a real project", async ({ page }) => {
// BEA-83 replaced the silent fallback with a not-found page: the sidebar
// still shows a real project (the fallback chain is unchanged), but the URL
// you typed stays put and the content pane says the id resolved to nothing.
test("unknown project id says so instead of swapping projects", async ({ page }) => {
await login(page);
await page.goto("/p-00000000");
await page.waitForURL(/\/[0-9a-f-]{36}$/);
await expect(page.locator("#content .empty")).toContainText("Project not found");
await expect(page).toHaveURL("/p-00000000");
await expect(page.locator("#project-select")).toContainText(/.+/);
});

Expand Down
76 changes: 49 additions & 27 deletions internal/webapp/frontend/src/apps/HubApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,25 @@ export default function HubApp({ config }: { config: ServerConfig }) {
}
: null;

// Same rule as orgMissing, for the project id: a deep link to an id that is
// not yours is not a landing. `current` still resolves (the fallback chain
// above is untouched) — it is what the sidebar shows and where "back" points.
const projectMissing = !!route.project && !projects.some((p) => p.id === route.project);
const projectPage = projectMissing
? {
crumb: "Project",
body: (
<div className="empty">
<h3>Project not found</h3>
<p>This project doesn't exist, or you're no longer a member.</p>
<p>
<a {...linkProps("/" + current.id)}>Back to {current.name}</a>
</p>
</div>
),
}
: null;

// Billing is hub-level (the managed deployment's surface), not
// project-scoped — like the org route it borrows whichever project the
// sidebar is showing. An OSS hub has no billing block; a hand-typed
Expand Down Expand Up @@ -249,35 +268,38 @@ export default function HubApp({ config }: { config: ServerConfig }) {
}
: null;

// Landing ("/") and unknown project ids both resolve to a real project
// URL; replace so back/forward never bounces through the redirect. The
// org route is not project-scoped, so it is exempt — it borrows whichever
// project the sidebar is showing.
if (!route.org && !route.billing && route.project !== current.id) {
return <Redirect to={"/" + current.id} />;
}
// Every redirect below rewrites the address bar off `current.id`, so all of
// them are wrong for a bogus deep link: /bad-id, /bad-id/insights and
// /bad-id/notes/ would each swap in another project and drop the path.
// projectMissing renders instead (projectPage above) at the URL as typed.
if (!projectMissing) {
// Landing ("/") resolves to a real project URL; replace so back/forward
// never bounces through the redirect. The org route is not project-scoped,
// so it is exempt — it borrows whichever project the sidebar is showing.
if (!route.org && !route.billing && route.project !== current.id) {
return <Redirect to={"/" + current.id} />;
}

// A renamed view URL (/insights) still resolves; swap it for the current
// one so there is one live URL per page. Filters ride along: the hop is a
// rename, not a reset, and dropping them would silently widen the feed.
if (route.legacyView && route.view) {
return <Redirect to={urlForView(route.view, current.id, route.viewTarget, route.filters)} />;
}
// A renamed view URL (/insights) still resolves; swap it for the current
// one so there is one live URL per page. Filters ride along: the hop is a
// rename, not a reset, and dropping them would silently widen the feed.
if (route.legacyView && route.view) {
return <Redirect to={urlForView(route.view, current.id, route.viewTarget, route.filters)} />;
}

// /history?path=guide.md resolved to guide.md's feed (the query form is
// what the History API teaches); put the canonical path URL in the address
// bar. Below the unknown-project redirect for the same reason the trailing
// slash one is: normalizing on a bad project id would pin the wrong project.
if (route.queryTarget && route.view) {
return <Redirect to={urlForView(route.view, current.id, route.viewTarget, route.filters)} />;
}
// /history?path=guide.md resolved to guide.md's feed (the query form is
// what the History API teaches); put the canonical path URL in the address
// bar.
if (route.queryTarget && route.view) {
return <Redirect to={urlForView(route.view, current.id, route.viewTarget, route.filters)} />;
}

// /notes/ is the same page as /notes — resolve it, then take the slash off
// the address bar. After the rewrite the flag is false, so there is no
// second hop. Must stay below the unknown-project redirect above, or a bad
// project id would be normalized on the path and keep the wrong project.
if (route.trailingSlash && route.path) {
return <Redirect to={urlForPath(route.path, current.id, route.version)} />;
// /notes/ is the same page as /notes — resolve it, then take the slash off
// the address bar. After the rewrite the flag is false, so there is no
// second hop.
if (route.trailingSlash && route.path) {
return <Redirect to={urlForPath(route.path, current.id, route.version)} />;
}
}

return (
Expand Down Expand Up @@ -339,7 +361,7 @@ export default function HubApp({ config }: { config: ServerConfig }) {
),
orgBar: accountBar,
}}
panel={activePanel || orgPage || billingPage || routePage}
panel={activePanel || orgPage || projectPage || billingPage || routePage}
onClosePanel={() => setPanel(null)}
/>
{newProjectDialog}
Expand Down
Loading
Loading