mirror of
https://github.com/runbear-io/beardrive.git
synced 2026-08-25 08:08:08 +02:00
fix(hub): a folder URL with a trailing slash is the same page as without (BEA-28) (#73)
* fix(hub): a folder URL with a trailing slash is the same page as without (BEA-28) * docs(architecture): Route.trailingSlash in the frontend diagram (BEA-28) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
feacbe3b3a
commit
6ae941683e
@@ -32,6 +32,7 @@ classDiagram
|
||||
+top-level routes orgs billing
|
||||
+parseRoute(url, mode) Route
|
||||
+Route.version ?v= sha, one past version
|
||||
+Route.trailingSlash notes/ resolves, then replaces to notes
|
||||
+urlForPath(path, projectId, version)
|
||||
+urlForView / encodePath / decodePath
|
||||
}
|
||||
|
||||
@@ -48,6 +48,22 @@ test("folder listing: counts, change feed, heat dot on a read file", async ({ pa
|
||||
await expect(page.locator('.dl-row[title="notes/readme.md"] .heatdot')).toBeVisible();
|
||||
});
|
||||
|
||||
// BEA-28: copying a folder URL hands you a trailing slash, and that URL used
|
||||
// to 404 while the sidebar showed the folder populated right next to it.
|
||||
test("folder URL with a trailing slash renders the listing and drops the slash", async ({ page }) => {
|
||||
await login(page);
|
||||
const pid = await wikiId(page);
|
||||
await page.goto(`/${pid}`);
|
||||
await page.goto(`/${pid}/notes/`);
|
||||
await expect(page.locator(".dl-title")).toContainText("notes");
|
||||
await expect(page.locator(".dl-history .dl-h3")).toHaveText("Recent changes");
|
||||
await expect(page).toHaveURL(`/${pid}/notes`);
|
||||
// Replaced, not pushed: Back leaves the folder instead of bouncing off the
|
||||
// slashed URL and landing right back here.
|
||||
await page.goBack();
|
||||
await expect(page).toHaveURL(`/${pid}`);
|
||||
});
|
||||
|
||||
// BEA-17: the kind glyph read as a disclosure toggle. It is now a text
|
||||
// badge, the row's only real expander is the note, and clicking the badge
|
||||
// navigates like the rest of the row — no dead zone, no second behavior.
|
||||
|
||||
@@ -2,7 +2,7 @@ import { useEffect, useMemo, useState } from "react";
|
||||
import { postJSON } from "../api/http";
|
||||
import type { InviteAccepted, Project, ServerConfig } from "../api/types";
|
||||
import { useOrgs, usePending, useProjects, useHubRefresh } from "../hooks/useHub";
|
||||
import { parseRoute, urlForView } from "../router";
|
||||
import { parseRoute, 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";
|
||||
@@ -214,6 +214,14 @@ export default function HubApp({ config }: { config: ServerConfig }) {
|
||||
return <Redirect to={urlForView(route.view, current.id, route.viewTarget)} />;
|
||||
}
|
||||
|
||||
// /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)} />;
|
||||
}
|
||||
|
||||
return (
|
||||
<Browser
|
||||
key={current.id} // fresh tree/fold state per project
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
import { useEffect, useMemo } from "react";
|
||||
import type { ServerConfig } from "../api/types";
|
||||
import { VaultHeader } from "../components/shell";
|
||||
import { parseRoute } from "../router";
|
||||
import { useLocationPath } from "../nav";
|
||||
import { parseRoute, urlForPath } from "../router";
|
||||
import { Redirect, useLocationPath } from "../nav";
|
||||
import Browser from "./Browser";
|
||||
|
||||
// Single-volume mode: one folder, no projects or orgs — but the full
|
||||
@@ -15,6 +15,11 @@ export default function VolumeApp({ config }: { config: ServerConfig }) {
|
||||
}, [config, name]);
|
||||
const route = useMemo(() => parseRoute(loc, "volume"), [loc]);
|
||||
|
||||
// /notes/ is the same page as /notes — see the same guard in HubApp.
|
||||
if (route.trailingSlash && route.path) {
|
||||
return <Redirect to={urlForPath(route.path)} />;
|
||||
}
|
||||
|
||||
return (
|
||||
<Browser
|
||||
config={config}
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
// 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 } 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.
|
||||
test("trailing slashes are stripped off project paths", () => {
|
||||
const a = parseRoute("/p-1/notes/", "hub");
|
||||
assert.equal(a.path, "notes");
|
||||
assert.equal(a.trailingSlash, true);
|
||||
|
||||
const b = parseRoute("/p-1/notes//", "hub");
|
||||
assert.equal(b.path, "notes");
|
||||
assert.equal(b.trailingSlash, true);
|
||||
|
||||
const c = parseRoute("/p-1/notes/deep/", "hub");
|
||||
assert.equal(c.path, "notes/deep");
|
||||
assert.equal(c.trailingSlash, true);
|
||||
|
||||
const f = parseRoute("/p-1/guide.md/", "hub");
|
||||
assert.equal(f.path, "guide.md");
|
||||
assert.equal(f.trailingSlash, true);
|
||||
});
|
||||
|
||||
// Nothing to strip means no flag — otherwise the project root would ask for
|
||||
// a redirect to itself, forever.
|
||||
test("the project root does not ask for a redirect", () => {
|
||||
const r = parseRoute("/p-1/", "hub");
|
||||
assert.equal(r.project, "p-1");
|
||||
assert.equal(r.path, "");
|
||||
assert.ok(!r.trailingSlash);
|
||||
|
||||
const bare = parseRoute("/p-1", "hub");
|
||||
assert.equal(bare.path, "");
|
||||
assert.ok(!bare.trailingSlash);
|
||||
});
|
||||
|
||||
test("volume mode strips too", () => {
|
||||
const r = parseRoute("/notes/", "volume");
|
||||
assert.equal(r.path, "notes");
|
||||
assert.equal(r.trailingSlash, true);
|
||||
|
||||
const root = parseRoute("/", "volume");
|
||||
assert.equal(root.path, "");
|
||||
assert.ok(!root.trailingSlash);
|
||||
});
|
||||
|
||||
test("the ?v= version survives the rewrite", () => {
|
||||
const r = parseRoute("/p-1/notes/?v=abc123", "hub");
|
||||
assert.equal(r.path, "notes");
|
||||
assert.equal(r.version, "abc123");
|
||||
assert.equal(r.trailingSlash, true);
|
||||
});
|
||||
|
||||
// View targets were already normalized at parse; this guards that.
|
||||
test("view routes still resolve", () => {
|
||||
const r = parseRoute("/p-1/history/notes/", "hub");
|
||||
assert.equal(r.view, "history");
|
||||
assert.equal(r.viewTarget, "notes");
|
||||
assert.equal(r.path, "");
|
||||
});
|
||||
|
||||
// Stripping happens on the still-encoded slice, so the redirect target
|
||||
// re-encodes to exactly one URL rather than another one that redirects.
|
||||
test("odd characters round-trip", () => {
|
||||
const r = parseRoute("/p-1/notes/a%20b/", "hub");
|
||||
assert.equal(r.path, "notes/a b");
|
||||
assert.equal(r.trailingSlash, true);
|
||||
});
|
||||
@@ -48,6 +48,10 @@ export interface Route {
|
||||
// The URL used a renamed segment (e.g. /insights): the app replaces it
|
||||
// with the canonical one instead of leaving two URLs for one page.
|
||||
legacyView?: boolean;
|
||||
// The URL carried a trailing separator (/notes/ — what a browser hands you
|
||||
// when you copy a folder URL). Same treatment as legacyView: it resolves,
|
||||
// then the app replaces it with the slash-free URL.
|
||||
trailingSlash?: boolean;
|
||||
// A past version of `path`, by content hash (?v=<sha>). Not a view route:
|
||||
// the first segment after the project id is reserved for view names, and a
|
||||
// version is the same page pinned to older bytes, so it rides as a query
|
||||
@@ -64,9 +68,20 @@ export function parseRoute(url: string, mode: "volume" | "hub"): Route {
|
||||
return r;
|
||||
}
|
||||
|
||||
// Trailing separators are stripped off the raw (still-encoded) slice so a
|
||||
// percent-encoded slash inside a segment survives, and flagged only when
|
||||
// stripping actually changed the string — a bare "/p-1/" has nothing to
|
||||
// strip, so it never asks for a redirect.
|
||||
function withPath(r: Route, raw: string): Route {
|
||||
const p = raw.replace(/\/+$/, "");
|
||||
if (p !== raw) r.trailingSlash = true;
|
||||
r.path = p ? decodePath(p) : "";
|
||||
return r;
|
||||
}
|
||||
|
||||
function parsePath(pathname: string, mode: "volume" | "hub"): Route {
|
||||
const raw = pathname.replace(/^\/+/, "");
|
||||
if (mode !== "hub") return { path: raw ? decodePath(raw) : "" };
|
||||
if (mode !== "hub") return withPath({ path: "" }, raw);
|
||||
if (raw === "orgs" || raw.startsWith("orgs/")) {
|
||||
return { org: raw.slice(5).replace(/\/+$/, ""), path: "" };
|
||||
}
|
||||
@@ -75,7 +90,7 @@ function parsePath(pathname: string, mode: "volume" | "hub"): Route {
|
||||
}
|
||||
const slash = raw.indexOf("/");
|
||||
if (slash === -1) return { project: raw, path: "" };
|
||||
const r: Route = { project: raw.slice(0, slash), path: decodePath(raw.slice(slash + 1)) };
|
||||
const r = withPath({ project: raw.slice(0, slash), path: "" }, raw.slice(slash + 1));
|
||||
const seg = r.path.indexOf("/");
|
||||
const head = seg === -1 ? r.path : r.path.slice(0, seg);
|
||||
if (VIEW_ROUTES.has(head) || LEGACY_VIEWS[head]) {
|
||||
|
||||
+12
-12
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-l8F9gvwJ.js"></script>
|
||||
<script type="module" crossorigin src="/assets/index-D0KboqaF.js"></script>
|
||||
<link rel="stylesheet" crossorigin href="/assets/index-B18K6amq.css">
|
||||
</head>
|
||||
<body>
|
||||
|
||||
Reference in New Issue
Block a user