mirror of
https://github.com/runbear-io/beardrive.git
synced 2026-08-25 08:08:08 +02:00
fix(webapp): a project URL by name opens the project (BEA-140) (#180)
Two personas guessed /wiki independently — the project name is what the
sidebar shows them, and the id (4c400e3f-…) never appears in the UI as
something to copy. Both got a page that argued with itself: the correct
project's chrome, breadcrumb and full file tree, with a body reading
"Project not found. This project doesn't exist, or you're no longer a
member." One of them owned the project.
Both halves land in the same place, and the net effect is one conditional
removed. `projectMissing` becomes an early return:
- A first segment that names exactly one of your projects, matched
case-insensitively on the DECODED segment (route.project is the
still-encoded slice, so a name with a space would never have
matched), redirects to /<id> with the rest of the URL — path, view,
target, filters, version — carried along. Exactly one: ProjectDB
names are scoped per organization, so a viewer in two orgs can hold
two projects called "wiki", and the not-found page is the honest
answer there.
- Anything else renders the not-found body in a shell with NO tree —
the same shape the `!current` branch already uses — so no other
project's files sit beside a body denying the one that was asked
for. The copy names the segment and drops the "no longer a member"
claim, which was told to readers who may never have been members.
The `current` fallback chain is untouched: "Back to <project>" still
points at it. With the early return in place, none of the four redirects
below can be reached on a missing project, so the `if (!projectMissing)`
wrapper is gone and its reason now lives in the return.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
1868228d29
commit
52156f1b21
@@ -54,6 +54,7 @@ classDiagram
|
||||
+historyFilterQuery(filters) / hasHistoryFilters
|
||||
+urlForPath(path, projectId, version)
|
||||
+urlForView(view, projectId, target, filters) / encodePath / decodePath
|
||||
+projectByName(projects, seg) id, only when exactly one name matches
|
||||
}
|
||||
class nav {
|
||||
+navigate(url)
|
||||
@@ -62,6 +63,7 @@ classDiagram
|
||||
+Redirect
|
||||
}
|
||||
note for router "Two lookups on peer-authored path segments are now prototype-safe and one is throw-safe: legacyView() goes through Object.hasOwn, because LEGACY_VIEWS['constructor'] is truthy and turned a folder of that name into a view whose name was a FUNCTION; decodePath falls back to the raw segment instead of letting decodeURIComponent throw URIError out of a useMemo during render. Same shape as ProjectIcon's PROJECT_ICONS lookup in shell.tsx"
|
||||
note for router "projectByName is what makes /wiki reach the project called wiki: the id never appears in the UI as something to copy, so a hand-typed first segment is the NAME the sidebar shows. It decodes the segment (route.project is the still-encoded slice) and returns an id only on EXACTLY one case-insensitive match — ProjectDB names are scoped per organization, so a viewer in two orgs can hold two projects named wiki and guessing between them is worse than the not-found page (BEA-140)"
|
||||
note for nav "nav.ts + router.ts — deliberately NOT a router library (react-router v7 startTransition left stale views); History-API path routing, slashes literal, every user-facing page owns a URL path. A version is not a view route (the first segment after the project id is reserved for view names) — it rides as ?v=, so useLocationPath must snapshot the search too or the URL changes and nothing re-renders"
|
||||
|
||||
class api {
|
||||
|
||||
@@ -31,6 +31,33 @@ test("unknown project id says so instead of swapping projects", async ({ page })
|
||||
await expect(page.locator("#content .empty")).toContainText("Project not found");
|
||||
await expect(page).toHaveURL("/p-00000000");
|
||||
await expect(page.locator("#project-select")).toContainText(/.+/);
|
||||
// BEA-140. The page may not argue with itself: no other project's file tree
|
||||
// beside a body denying the requested one, and no claim of lost membership
|
||||
// to a reader who may never have been a member.
|
||||
await expect(page.locator("#tree .row")).toHaveCount(0);
|
||||
await expect(page.locator("#content .empty")).toContainText("p-00000000");
|
||||
await expect(page.locator("#content .empty")).not.toContainText("no longer a member");
|
||||
});
|
||||
|
||||
// BEA-140. Two personas guessed /wiki independently — the name is what the
|
||||
// sidebar shows them, and the id never appears as something to copy.
|
||||
test("a project name in the URL resolves to its id", async ({ page }) => {
|
||||
await login(page);
|
||||
const pid = await wikiId(page);
|
||||
|
||||
await page.goto("/wiki");
|
||||
await page.waitForURL("/" + pid);
|
||||
await expect(page.locator("#project-select")).toContainText("wiki");
|
||||
|
||||
// The rest of the URL rides along rather than being thrown away.
|
||||
await page.goto("/wiki/index.md");
|
||||
await page.waitForURL("/" + pid + "/index.md");
|
||||
await expect(page.locator("#content")).toContainText(/.+/);
|
||||
|
||||
// A view segment survives the hop too, and matching ignores case.
|
||||
await page.goto("/WIKI/dashboard");
|
||||
await page.waitForURL("/" + pid + "/dashboard");
|
||||
await expect(page.locator("#content")).toContainText(/.+/);
|
||||
});
|
||||
|
||||
test("account menu: admin gets hub admin entry; member does not", async ({ page, browser }) => {
|
||||
|
||||
@@ -2,7 +2,7 @@ import { useEffect, useMemo, useState } from "react";
|
||||
import { postJSON } from "../api/http";
|
||||
import type { InviteAccepted, Project, ProjectCreated, ServerConfig } from "../api/types";
|
||||
import { useOrgs, usePending, useProjects, useHubRefresh } from "../hooks/useHub";
|
||||
import { parseRoute, urlForPath, urlForView } from "../router";
|
||||
import { decodePath, parseRoute, projectByName, urlForPath, urlForView } from "../router";
|
||||
import { linkProps, navigate, Redirect, useLocationPath } from "../nav";
|
||||
import { AppShell, Page, Topbar, VaultHeader, closeSidebarOnMobile } from "../components/shell";
|
||||
import { OrgAdmin } from "../components/OrgAdmin";
|
||||
@@ -219,22 +219,59 @@ export default function HubApp({ config }: { config: ServerConfig }) {
|
||||
|
||||
// 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.
|
||||
// above is untouched) — it is where "back" points.
|
||||
const projectMissing = !!route.project && !projects.some((p) => p.id === route.project);
|
||||
const projectPage = projectMissing
|
||||
? {
|
||||
crumb: "Project",
|
||||
body: (
|
||||
if (projectMissing) {
|
||||
// The id never appears in the UI as something to copy, so a hand-typed
|
||||
// first segment is almost always the project NAME the sidebar shows. One
|
||||
// unambiguous match resolves to its id, carrying the rest of the URL —
|
||||
// path, view, target, filters, version — along with it.
|
||||
const named = projectByName(projects, route.project!);
|
||||
if (named) {
|
||||
return (
|
||||
<Redirect
|
||||
to={
|
||||
route.view
|
||||
? urlForView(route.view, named, route.viewTarget, route.filters)
|
||||
: urlForPath(route.path, named, route.version)
|
||||
}
|
||||
/>
|
||||
);
|
||||
}
|
||||
// Nothing resolved, so this returns early rather than falling through:
|
||||
// every redirect below rewrites the address bar off `current.id`, and all
|
||||
// of them are wrong for a segment that named nothing — /bad-id,
|
||||
// /bad-id/insights and /bad-id/notes/ would each swap in another project
|
||||
// and drop the path. The URL stays as typed.
|
||||
//
|
||||
// And it renders with NO tree. `current` still resolves, but painting its
|
||||
// sidebar beside a "not found" body is the page arguing with itself: two
|
||||
// personas read the full file tree of a project they were looking at,
|
||||
// one of them its owner, next to a line telling them they were no longer
|
||||
// a member of it.
|
||||
return (
|
||||
<AppShell
|
||||
vault={vault}
|
||||
projectsNav={<ProjectNav projects={projects} onNew={() => setCreating(true)} />}
|
||||
orgBar={accountBar}
|
||||
topbar={<Topbar />}
|
||||
>
|
||||
<Page>
|
||||
<div className="empty">
|
||||
<h3>Project not found</h3>
|
||||
<p>This project doesn't exist, or you're no longer a member.</p>
|
||||
<p>
|
||||
There's no project called “{decodePath(route.project!)}” in your account. It may
|
||||
have been renamed or deleted, or the link may be wrong.
|
||||
</p>
|
||||
<p>
|
||||
<a {...linkProps("/" + current.id)}>Back to {current.name}</a>
|
||||
</p>
|
||||
</div>
|
||||
),
|
||||
}
|
||||
: null;
|
||||
</Page>
|
||||
{newProjectDialog}
|
||||
</AppShell>
|
||||
);
|
||||
}
|
||||
|
||||
// Billing is hub-level (the managed deployment's surface), not
|
||||
// project-scoped — like the org route it borrows whichever project the
|
||||
@@ -283,38 +320,36 @@ export default function HubApp({ config }: { config: ServerConfig }) {
|
||||
}
|
||||
: null;
|
||||
|
||||
// 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} />;
|
||||
}
|
||||
// Everything below rewrites the address bar off `current.id`. A missing
|
||||
// project can no longer reach here — it returned above — so these need no
|
||||
// guard of their own.
|
||||
|
||||
// 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)} />;
|
||||
}
|
||||
// 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} />;
|
||||
}
|
||||
|
||||
// /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)} />;
|
||||
}
|
||||
// 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)} />;
|
||||
}
|
||||
|
||||
// /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)} />;
|
||||
}
|
||||
// /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.
|
||||
if (route.trailingSlash && route.path) {
|
||||
return <Redirect to={urlForPath(route.path, current.id, route.version)} />;
|
||||
}
|
||||
|
||||
return (
|
||||
@@ -376,7 +411,7 @@ export default function HubApp({ config }: { config: ServerConfig }) {
|
||||
),
|
||||
orgBar: accountBar,
|
||||
}}
|
||||
panel={activePanel || orgPage || projectPage || billingPage || routePage}
|
||||
panel={activePanel || orgPage || billingPage || routePage}
|
||||
onClosePanel={() => setPanel(null)}
|
||||
/>
|
||||
{newProjectDialog}
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
// Run with `npm test` (node's built-in runner; node ≥ 23 strips the types).
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { parseRoute, urlForView, historyFilterQuery } from "./router.ts";
|
||||
import { parseRoute, projectByName, urlForView, historyFilterQuery } from "./router.ts";
|
||||
|
||||
// A trailing slash is what a browser hands you when you copy a folder URL,
|
||||
// so /notes/ has to be the same page as /notes.
|
||||
@@ -162,3 +162,53 @@ test("an encoded separator in ?path= round-trips", () => {
|
||||
assert.equal(r.queryTarget, true);
|
||||
assert.equal(urlForView("history", "p-1", r.viewTarget), "/p-1/history/a/b.md");
|
||||
});
|
||||
|
||||
// BEA-140. The id never appears in the UI as something to copy, so readers
|
||||
// type the name the sidebar shows them. Resolving it is the whole redirect.
|
||||
const PROJECTS = [
|
||||
{ id: "4c400e3f", name: "wiki" },
|
||||
{ id: "aa11", name: "Design Docs" },
|
||||
];
|
||||
|
||||
test("a project name resolves to its id", () => {
|
||||
assert.equal(projectByName(PROJECTS, "wiki"), "4c400e3f");
|
||||
});
|
||||
|
||||
// The sidebar shows "Design Docs"; nobody types the capitals back exactly.
|
||||
test("name matching is case-insensitive", () => {
|
||||
assert.equal(projectByName(PROJECTS, "WIKI"), "4c400e3f");
|
||||
assert.equal(projectByName(PROJECTS, "design docs"), "aa11");
|
||||
});
|
||||
|
||||
// route.project is the still-encoded segment (parsePath slices `raw` before
|
||||
// decodePath runs), so a name with a space matches only if it is decoded
|
||||
// here — and a space is exactly what a hand-typed name is likely to carry.
|
||||
test("an encoded segment is decoded before matching", () => {
|
||||
assert.equal(projectByName(PROJECTS, "Design%20Docs"), "aa11");
|
||||
assert.equal(parseRoute("/Design%20Docs/index.md", "hub").project, "Design%20Docs");
|
||||
});
|
||||
|
||||
test("a name nobody has resolves to nothing", () => {
|
||||
assert.equal(projectByName(PROJECTS, "nope"), undefined);
|
||||
assert.equal(projectByName([], "wiki"), undefined);
|
||||
});
|
||||
|
||||
// Names are scoped per organization (ProjectDB create-or-join-by-name), so a
|
||||
// viewer in two orgs can hold two projects called "wiki". Guessing between
|
||||
// them would open the wrong one silently; the not-found page is the honest
|
||||
// answer, so a collision must never relax to "first match".
|
||||
test("two projects with one name resolve to neither", () => {
|
||||
const dupes = [
|
||||
{ id: "org-a-wiki", name: "wiki" },
|
||||
{ id: "org-b-wiki", name: "wiki" },
|
||||
];
|
||||
assert.equal(projectByName(dupes, "wiki"), undefined);
|
||||
assert.equal(projectByName(dupes, "WiKi"), undefined);
|
||||
});
|
||||
|
||||
// No id-shape check guards the matcher, on purpose: it only ever runs on a
|
||||
// segment that already failed to match every id, so an id-shaped segment can
|
||||
// resolve only if some project is literally named that string.
|
||||
test("an id-shaped segment matches nothing unless a project is named it", () => {
|
||||
assert.equal(projectByName(PROJECTS, "4c400e3f"), undefined);
|
||||
});
|
||||
|
||||
@@ -212,3 +212,29 @@ export function urlForView(
|
||||
if (target) s += "/" + encodePath(target.replace(/\/+$/, ""));
|
||||
return s + (view === "history" ? historyFilterQuery(filters) : "");
|
||||
}
|
||||
|
||||
// A first segment that names no project id is, nine times out of ten, the
|
||||
// project NAME: that is what the sidebar shows, and the id never appears in
|
||||
// the UI as something to copy. Resolve it only when it is unambiguous —
|
||||
// ProjectDB's names are scoped per organization (create-or-join-by-name), so
|
||||
// a viewer who belongs to two orgs can hold two projects called "wiki", and
|
||||
// guessing between them is worse than the not-found page.
|
||||
//
|
||||
// The segment arrives still-encoded (parsePath slices `raw` before decodePath
|
||||
// runs), so it is decoded here — without that, any name with a space or a
|
||||
// non-ASCII character silently fails to match, which is exactly the set of
|
||||
// names most likely to be typed by hand in the first place.
|
||||
//
|
||||
// No id-shape check is needed: this only ever runs on a segment that already
|
||||
// failed to match every id, and it compares against real names, so an
|
||||
// id-shaped segment can only resolve if some project is literally called
|
||||
// that. A UUID regex here would be a second rule to keep in sync with
|
||||
// parsePath for zero change in behaviour.
|
||||
export function projectByName(
|
||||
projects: { id: string; name: string }[],
|
||||
seg: string,
|
||||
): string | undefined {
|
||||
const want = decodePath(seg).toLowerCase();
|
||||
const hit = projects.filter((p) => p.name.toLowerCase() === want);
|
||||
return hit.length === 1 ? hit[0].id : undefined;
|
||||
}
|
||||
|
||||
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
@@ -5,7 +5,7 @@
|
||||
<meta name="viewport" content="width=device-width, initial-scale=1">
|
||||
<title>BearDrive</title>
|
||||
<link rel="icon" href="data:image/svg+xml,<svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 32 32' fill='%23f5a623'><rect x='4' y='4' width='5.6' height='24'/><rect x='11.2' y='4' width='14.4' height='11.2'/><rect x='11.2' y='16.8' width='16.8' height='11.2'/></svg>">
|
||||
<script type="module" crossorigin src="/assets/index-DU4-OSS9.js"></script>
|
||||
<script type="module" crossorigin src="/assets/index-seXZWqX2.js"></script>
|
||||
<link rel="modulepreload" crossorigin href="/assets/_commonjsHelpers-CqkleIqs.js">
|
||||
<link rel="modulepreload" crossorigin href="/assets/mermaid-DQuCJ8Gi.js">
|
||||
<link rel="stylesheet" crossorigin href="/assets/index-Bvo9-TXr.css">
|
||||
|
||||
Reference in New Issue
Block a user