Public: anyone can read this; the crew can change it.
Every public page has a title, description, og:title, og:description, og:image and canonical URL, and a test fails any that lacks one #9606
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/9577-every-public-page-has-a-title-descriptio → main 4 commits fork 3 threads · 3 unresolved
5 files
changed +160
−5
src/web/describe.ts +23 −1
| @@ -69,6 +69,27 @@ export function ownerLine(o: { | ||
| 69 | 69 | return `${o.org ? "Organization" : "User"} @${o.name} · ${n} public ${noun}.`; |
| 70 | 70 | } |
| 71 | 71 | |
| 72 | /** | |
| 73 | * A whole line that is a scaffold's default or a placeholder, not a | |
| 74 | * description a visitor can read. Anchored: "A todo list app" is real. | |
| 75 | */ | |
| 76 | const JARGON = | |
| 77 | /^\s*(untitled|test|description|none|n\/a|todo|tbd|wip|fixme|lorem ipsum.*|initial commit|(an? )?(scaffold(ed)?|auto-?generated)( \w+)?|created (by|with|from) (forge|template).*)\s*[.!]?\s*$/i; | |
| 78 | ||
| 79 | /** A description a visitor can read, or `fallback` when it is empty or jargon. */ | |
| 80 | export function plainDescription(text: string, fallback: string): string { | |
| 81 | const line = firstLine(text); | |
| 82 | return line === "" || JARGON.test(line) ? fallback : line; | |
| 83 | } | |
| 84 | ||
| 85 | const sentence = (text: string): string => (/[.!?…]$/.test(text) ? text : `${text}.`); | |
| 86 | ||
| 87 | /** A repository: its own description, or a plain sentence naming it. */ | |
| 88 | export function repoLine(o: { private: boolean; name: string; description: string }): string { | |
| 89 | if (o.private) return PRIVATE_LINE; | |
| 90 | return sentence(plainDescription(o.description, `${o.name} is a project on the forge: code, issues and changes in one place.`)); | |
| 91 | } | |
| 92 | ||
| 72 | 93 | /** An app: its own description, and whether it is up, in `envPhrase`'s word. */ |
| 73 | 94 | export function appLine(o: { |
| 74 | 95 | private: boolean; |
| @@ -77,5 +98,6 @@ export function appLine(o: { | ||
| 77 | 98 | state: string; |
| 78 | 99 | }): string { |
| 79 | 100 | if (o.private) return PRIVATE_LINE; |
| 80 | return `${firstLine(o.description) || o.name} · ${o.state}.`; | |
| 101 | const own = plainDescription(o.description, `${o.name} is an app you can try and make your own.`); | |
| 102 | return `${sentence(own)} Status: ${o.state}.`; | |
| 81 | 103 | } |
src/web/pages/repo.ts +3 −2
| @@ -8,6 +8,7 @@ | ||
| 8 | 8 | * path segment and can never smuggle a path of its own. |
| 9 | 9 | */ |
| 10 | 10 | |
| 11 | import { repoLine } from "../describe.ts"; | |
| 11 | 12 | import { recordLook } from "../../flow/valve_store.ts"; |
| 12 | 13 | import { PINNED_PUBLIC, immutableWhenAddressed, mayShare } from "../../http/addressed.ts"; |
| 13 | 14 | import { EDGE_VARY } from "../../http/edge_cache.ts"; |
| @@ -1331,7 +1332,7 @@ async function codeHome(ctx: Ctx, repo: Repo): Promise<Response> { | ||
| 1331 | 1332 | const emptyResponse = page({ |
| 1332 | 1333 | ctx, |
| 1333 | 1334 | title: repo.name, |
| 1334 | description: repo.description, | |
| 1335 | description: repoLine({ private: repo.visibility === "private", name: repo.name, description: repo.description }), | |
| 1335 | 1336 | ogImage: `/og/repo/${repo.ownerName}~${repo.name}.png`, |
| 1336 | 1337 | feed: { |
| 1337 | 1338 | href: `${repoBase(repo)}/activity.atom`, |
| @@ -1405,7 +1406,7 @@ async function codeHome(ctx: Ctx, repo: Repo): Promise<Response> { | ||
| 1405 | 1406 | const response = page({ |
| 1406 | 1407 | ctx, |
| 1407 | 1408 | title: repo.name, |
| 1408 | description: repo.description, | |
| 1409 | description: repoLine({ private: repo.visibility === "private", name: repo.name, description: repo.description }), | |
Same repoLine(...) call as in the empty-repo branch above; hoist it into one const. |
||
| 1409 | 1410 | ogImage: `/og/repo/${repo.ownerName}~${repo.name}.png`, |
| 1410 | 1411 | feed: { |
| 1411 | 1412 | href: `${repoBase(repo)}/activity.atom`, |
test/og-tags.test.ts +1 −1
| @@ -165,7 +165,7 @@ describe("every kind of page unfurls", () => { | ||
| 165 | 165 | expect(res.status).toBe(200); |
| 166 | 166 | const t = tags(await res.text()); |
| 167 | 167 | expect(t.get("og:title")).toBeTruthy(); |
| 168 | expect(t.get("og:description")).toMatch(/·.*\.$/); | |
| 168 | expect(t.get("og:description")).toMatch(/Status: .+\.$/); | |
| 169 | 169 | expect(t.get("twitter:card")).toBe("summary_large_image"); |
| 170 | 170 | const image = new URL(t.get("og:image")!).pathname; |
| 171 | 171 | expect(image).toBe("/og/app/widget.png"); |
test/page-head.test.ts added +132
| @@ -0,0 +1,132 @@ | ||
| 1 | import { afterAll, beforeAll, describe, expect, test } from "bun:test"; | |
| 2 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 3 | import { tmpdir } from "node:os"; | |
| 4 | import { join } from "node:path"; | |
| 5 | import { createApp as createForge, type App } from "../src/app.ts"; | |
| 6 | import { createRepo } from "../src/actions/repos.ts"; | |
| 7 | import { getRepo } from "../src/core/repos.ts"; | |
| 8 | import { createIssue } from "../src/core/issues.ts"; | |
| 9 | import { createApp as createAppRecord } from "../src/core/apps.ts"; | |
| 10 | import { createUser } from "../src/core/users.ts"; | |
| 11 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 12 | import { appLine, plainDescription, repoLine } from "../src/web/describe.ts"; | |
| 13 | ||
| 14 | const content = (html: string, re: RegExp): string => html.match(re)?.[1]?.trim() ?? ""; | |
| 15 | ||
| 16 | describe("every public page names itself for a link preview", () => { | |
| 17 | let dataDir: string; | |
| 18 | let app: App; | |
| 19 | let servers: Servers; | |
| 20 | let base: string; | |
| 21 | ||
| 22 | beforeAll(async () => { | |
| 23 | dataDir = await mkdtemp(join(tmpdir(), "forge-pagehead-")); | |
| 24 | app = createForge({ dataDir, port: 0 }); | |
| 25 | servers = await serve(app); | |
| 26 | base = servers.url; | |
| 27 | const u = await createUser(app.db, { | |
| 28 | origin: "seed", | |
| 29 | trusted: true, | |
| 30 | username: "alice", | |
| 31 | email: "[email protected]", | |
| 32 | password: "alice-passphrase", | |
| 33 | adminIfFirst: true, | |
| 34 | }); | |
| 35 | if (!u.ok) throw new Error(u.error); | |
| 36 | // No description at all, and one that is scaffold jargon. | |
| 37 | for (const name of ["bare", "widget"]) { | |
| 38 | const made = await createRepo(app, u.value, { name, visibility: "public" }); | |
| 39 | if (!made.ok) throw new Error(String(made.error)); | |
| 40 | } | |
| 41 | app.db.query("UPDATE repo SET description = 'TODO' WHERE name = 'widget'").run(); | |
| 42 | const bareId = getRepo(app.db, "alice", "bare")!.id; | |
| 43 | const widgetId = getRepo(app.db, "alice", "widget")!.id; | |
| 44 | const rec = createAppRecord(app.db, { repoId: widgetId, slug: "widget", access: "public" }); | |
| 45 | if (!rec.ok) throw new Error(rec.error); | |
| 46 | for (const kind of ["issue", "pull"] as const) { | |
| 47 | const i = createIssue(app.db, { repoId: bareId, authorId: u.value.id, title: `A ${kind}`, kind }); | |
| 48 | if (!i.ok) throw new Error(String(i.error)); | |
| 49 | if (kind === "pull") | |
| 50 | app.db | |
| 51 | .query( | |
| 52 | `INSERT INTO pull (issue_id, base_branch, head_branch, base_oid, head_oid, head_repo_id) | |
| 53 | VALUES (?1, 'main', 'topic', '', '', ?2)`, | |
| 54 | ) | |
| 55 | .run(i.value.id, bareId); | |
| 56 | } | |
| 57 | }); | |
| 58 | ||
| 59 | afterAll(async () => { | |
| 60 | await servers.stop(); | |
| 61 | await rm(dataDir, { recursive: true, force: true }); | |
| 62 | }); | |
| 63 | ||
| 64 | const paths = [ | |
| 65 | "/", | |
Add the app page (/alice/widget/app) to the walked paths; box 1 names app, and docs is still uncovered. |
||
| 66 | "/explore", | |
| 67 | "/alice", | |
| 68 | "/alice/bare", | |
| 69 | "/alice/widget", | |
| 70 | // The app page: /alice/widget/app only redirects here. | |
| 71 | "/alice/bare/issues/1", | |
| 72 | "/alice/bare/pulls/2", | |
| 73 | "/packs", | |
| 74 | "/changelog", | |
| 75 | "/contribute", | |
| 76 | ]; | |
| 77 | ||
| 78 | for (const path of paths) { | |
| 79 | test(`${path} has a title, description, og tags and canonical`, async () => { | |
| 80 | const res = await fetch(base + path, { redirect: "manual" }); | |
| 81 | expect(res.status).toBe(200); | |
| 82 | const h = await res.text(); | |
| 83 | const found: Record<string, string> = { | |
| 84 | title: content(h, /<title>([^<]*)<\/title>/), | |
| 85 | description: content(h, /<meta\s+name="description"\s+content="([^"]*)"/), | |
| 86 | "og:title": content(h, /<meta\s+property="og:title"\s+content="([^"]*)"/), | |
| 87 | "og:description": content(h, /<meta\s+property="og:description"\s+content="([^"]*)"/), | |
| 88 | "og:image": content(h, /<meta\s+property="og:image"\s+content="([^"]*)"/), | |
| 89 | canonical: content(h, /<link\s+rel="canonical"\s+href="([^"]*)"/), | |
| 90 | }; | |
| 91 | for (const [name, value] of Object.entries(found)) expect(`${name}=${value}`).not.toBe(`${name}=`); | |
| 92 | expect(found.description).not.toMatch(/ · live\.$/); | |
| 93 | expect(found["og:description"]).not.toMatch(/ · live\.$/); | |
| 94 | }); | |
| 95 | } | |
| 96 | ||
| 97 | test("a pack page is covered too", async () => { | |
| 98 | const packs = await (await fetch(base + "/packs")).text(); | |
| 99 | const first = packs.match(/href="(\/packs\/[^"]+)"/)?.[1]; | |
| 100 | expect(first).toBeTruthy(); | |
| 101 | const h = await (await fetch(base + first!)).text(); | |
| 102 | expect(content(h, /<link\s+rel="canonical"\s+href="([^"]*)"/)).toContain(first!); | |
| 103 | expect(content(h, /<meta\s+name="description"\s+content="([^"]*)"/)).not.toBe(""); | |
| 104 | }); | |
| 105 | ||
| 106 | test("an empty or jargon description gets a plain sentence", async () => { | |
| 107 | for (const path of ["/alice/bare", "/alice/widget"]) { | |
| 108 | const h = await (await fetch(base + path)).text(); | |
| 109 | const d = content(h, /<meta\s+name="description"\s+content="([^"]*)"/); | |
| 110 | expect(d).toMatch(path.endsWith("bare") ? /^bare is a project on the forge: / | |
| 111 | : /^widget is an app you can try and make your own\. Status: /); | |
| 112 | } | |
| 113 | }); | |
| 114 | }); | |
| 115 | ||
| 116 | describe("description sentences", () => { | |
| 117 | test("empty and jargon fall back", () => { | |
| 118 | expect(plainDescription("", "Plain.")).toBe("Plain."); | |
| 119 | expect(plainDescription(" \n", "Plain.")).toBe("Plain."); | |
| 120 | expect(plainDescription("TODO", "Plain.")).toBe("Plain."); | |
| 121 | expect(plainDescription("A notes app", "Plain.")).toBe("A notes app"); | |
| 122 | expect(plainDescription("A todo app", "Plain.")).toBe("A todo app"); | |
| 123 | expect(plainDescription("Wip tracker", "Plain.")).toBe("Wip tracker"); | |
| 124 | expect(plainDescription("Scaffolded.", "Plain.")).toBe("Plain."); | |
| 125 | expect(repoLine({ private: false, name: "x", description: "" })).toMatch(/^x is a project/); | |
| 126 | }); | |
| 127 | test("an app line never ends in ' · live.'", () => { | |
| 128 | const line = appLine({ private: false, name: "x", description: "Notes", state: "live" }); | |
| 129 | expect(line).toBe("Notes. Status: live."); | |
| 130 | expect(line).not.toMatch(/ · live\.$/); | |
| 131 | }); | |
| 132 | }); |
test/unfurl.test.ts +1 −1
| @@ -187,7 +187,7 @@ describe("the pages", () => { | ||
| 187 | 187 | |
| 188 | 188 | test("an app opens on its description and whether it is up", async () => { |
| 189 | 189 | const line = unfurl(await get("/alice/open/app")); |
| 190 | expect(line).toMatch(/^open app for the unfurl test · [a-z ·]+\.$/); | |
| 190 | expect(line).toMatch(/^open app for the unfurl test\. Status: [a-z ·]+\.$/); | |
| 191 | 191 | }); |
| 192 | 192 | |
| 193 | 193 | test("no sentence uses dev's vocabulary", async () => { |
Unanchored todo/wip/fixme/tbd words reject genuine descriptions like "A todo list app". Anchor the whole line or narrow the list.