Public: anyone can read this; the crew can change it.
A preview's forge toolbar names its pull and checks state, and links back to the pull in one tap, without shifting the app's layout #11252
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/11233-a-preview-s-forge-toolbar-names-its-pull → main 2 commits fork 3 threads · 3 unresolved
5 files
changed +220
−7
src/apps/pill_check.ts added +34
| @@ -0,0 +1,34 @@ | ||
| 1 | /** | |
| 2 | * The check state the forge pill names for a preview's pull, in plain words: | |
| 3 | * the newest check run of the pull's head. One word set, so the panel and the | |
| 4 | * tests read the same table. | |
| 5 | */ | |
| 6 | ||
| 7 | import type { Database } from "bun:sqlite"; | |
| 8 | ||
| 9 | export const CHECK_WORDS = { | |
| 10 | passed: "Checks passed", | |
| 11 | failed: "Checks failed", | |
| 12 | running: "Checks running", | |
| 13 | waiting: "Checks waiting", | |
| 14 | cancelled: "Checks cancelled", | |
| 15 | none: "No checks yet", | |
| 16 | } as const; | |
| 17 | ||
| 18 | export type PillCheck = keyof typeof CHECK_WORDS; | |
| 19 | ||
| 20 | /** The state of the newest check run of this commit of this pull. */ | |
| 21 | export function pillCheck(db: Database, issueId: string, headOid: string): PillCheck { | |
| 22 | const row = db | |
| 23 | .query<{ state: string }, [string, string]>( | |
| 24 | `SELECT state FROM run WHERE kind = 'check' AND issue_id = ?1 AND commit_oid = ?2 ORDER BY id DESC LIMIT 1`, | |
| 25 | ) | |
| 26 | .get(issueId, headOid); | |
| 27 | if (row === null) return "none"; | |
| 28 | if (row.state === "ok") return "passed"; | |
| 29 | if (row.state === "failed") return "failed"; | |
| 30 | if (row.state === "running") return "running"; | |
| 31 | if (row.state === "queued") return "waiting"; | |
| 32 | if (row.state === "cancelled") return "cancelled"; | |
| 33 | return "none"; | |
| 34 | } |
src/apps/proxy.ts +6 −4
| @@ -58,6 +58,7 @@ import { clientBucket } from "../http/peer.ts"; | ||
| 58 | 58 | import { DOOR_PULL_LIMIT, doorPage, type DoorPullLine } from "./door.ts"; |
| 59 | 59 | import { checkWord } from "./check_word.ts"; |
| 60 | 60 | import { servingWords } from "./serving_words.ts"; |
| 61 | import { pillCheck } from "./pill_check.ts"; | |
| 61 | 62 | import { openPulls } from "./door_pulls.ts"; |
| 62 | 63 | import { notePhase, startedAt, takePhases, timePhase } from "../core/route_timing.ts"; |
| 63 | 64 | import { readServing } from "./serving.ts"; |
| @@ -563,17 +564,18 @@ export function pillWhere( | ||
| 563 | 564 | if (pull === undefined) return where; |
| 564 | 565 | if (!reads || !mayView(db, repo, pull, user)) return where; |
| 565 | 566 | where.title = pull.title; |
| 567 | const head = getPull(db, target.app.repoId, where.pull); | |
| 568 | if (head !== undefined && head.headOid !== "") where.check = pillCheck(db, pull.id, head.headOid); | |
| 566 | 569 | // How current the preview is: the row's head against the commit it runs. A |
| 567 | 570 | // signed-in reader only, so a page an anonymous viewer may share never carries it. |
| 568 | 571 | if (user === undefined) return where; |
| 569 | const row = getPull(db, target.app.repoId, where.pull); | |
| 570 | if (row === undefined) return where; | |
| 571 | where.state = row.mergedAt !== null ? "merged" : row.state === "closed" ? "closed" : "open"; | |
| 572 | if (head === undefined) return where; | |
| 573 | where.state = head.mergedAt !== null ? "merged" : head.state === "closed" ? "closed" : "open"; | |
| 572 | 574 | // Only an open pull has a newer push to be behind. |
| 573 | 575 | if (where.state !== "open") return where; |
| 574 | 576 | const status = runtime.envStatus?.(target.app, target.env); |
| 575 | 577 | if (status?.state === "deploying") where.deploying = true; |
| 576 | where.behind = status?.state !== "deploying" && status?.commitOid !== undefined && status.commitOid !== row.headOid; | |
| 578 | where.behind = status?.state !== "deploying" && status?.commitOid !== undefined && status.commitOid !== head.headOid; | |
| 577 | 579 | return where; |
| 578 | 580 | } |
| 579 | 581 |
src/apps/toolbar.ts +6
| @@ -13,6 +13,7 @@ | ||
| 13 | 13 | |
| 14 | 14 | import { staticResponse } from "../web/asset.ts"; |
| 15 | 15 | import { sha256 } from "../kernel/hash.ts"; |
| 16 | import { CHECK_WORDS, type PillCheck } from "./pill_check.ts"; | |
| 16 | 17 | import { NOTE_EMPTY, NOTE_FAILED } from "./note_words.ts"; |
| 17 | 18 | |
| 18 | 19 | /** What the panel says about a preview's freshness (#9111): the pill's words, in one table. */ |
| @@ -96,6 +97,8 @@ const toolbarSource = `(function () { | ||
| 96 | 97 | panel.appendChild(pl); |
| 97 | 98 | } else if (d.pull) line("Preview of pull #" + d.pull); |
| 98 | 99 | if (d.title) line(d.title); |
| 100 | var CW = ${JSON.stringify(CHECK_WORDS)}; | |
| 101 | if (d.env !== "production" && d.check && Object.prototype.hasOwnProperty.call(CW, d.check)) line(CW[d.check]); | |
Logic in the injected script is only tested by grepping its source. Run it against a fake element/dataset and assert the line is drawn for pull-3 and absent for production and for an unknown key. |
||
| 99 | 102 | // Whether this is the pull as it stands now; an open, current preview says nothing. |
| 100 | 103 | var W = ${JSON.stringify(FRESHNESS_WORDS)}; |
| 101 | 104 | var freshness = d.state === "merged" ? W.merged |
| @@ -627,6 +630,8 @@ export type PillWhere = { | ||
| 627 | 630 | behind?: boolean; |
| 628 | 631 | /** A newer commit is being deployed now. */ |
| 629 | 632 | deploying?: boolean; |
| 633 | /** The newest check of the pull's head, a key of `CHECK_WORDS`. */ | |
| 634 | check?: PillCheck; | |
| 630 | 635 | }; |
| 631 | 636 | |
| 632 | 637 | /** `text` safe inside a double-quoted HTML attribute. */ |
| @@ -675,6 +680,7 @@ export function toolbarTag( | ||
| 675 | 680 | ["pull", where.pull === undefined ? undefined : String(where.pull)], |
| 676 | 681 | ["title", where.title], |
| 677 | 682 | ["state", where.state], |
| 683 | ["check", where.check], | |
| 678 | 684 | ["behind", where.behind === true ? "1" : undefined], |
| 679 | 685 | ["deploying", where.deploying === true ? "1" : undefined], |
| 680 | 686 | ] |
test/apps-pill-check.test.ts added +171
| @@ -0,0 +1,171 @@ | ||
| 1 | /** | |
| 2 | * The forge pill names the pull's check state in plain words (#11233): from | |
| 3 | * the newest check of the head, escaped as a data attribute, shown for a | |
| 4 | * preview and never for production; the pill stays fixed and reserves no space. | |
| 5 | */ | |
| 6 | ||
| 7 | import { afterAll, beforeAll, describe, expect, test } from "bun:test"; | |
| 8 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 9 | import { tmpdir } from "node:os"; | |
| 10 | import { join } from "node:path"; | |
| 11 | import { createApp, type App } from "../src/app.ts"; | |
| 12 | import { newApp } from "../src/actions/apps.ts"; | |
| 13 | import { createIssue } from "../src/core/issues.ts"; | |
| 14 | import { createUser } from "../src/core/users.ts"; | |
| 15 | import { CHECK_WORDS, pillCheck } from "../src/apps/pill_check.ts"; | |
| 16 | import { toolbarJs, toolbarTag } from "../src/apps/toolbar.ts"; | |
| 17 | import { pillWhere, type AppTarget } from "../src/apps/proxy.ts"; | |
| 18 | import type { AppsRuntime } from "../src/apps/runtime.ts"; | |
| 19 | import { getAppByRepo } from "../src/core/apps.ts"; | |
| 20 | import { boot } from "./helpers/toolbar-boot.ts"; | |
| 21 | import { quietLogs } from "./helpers/globals.ts"; | |
| 22 | ||
| 23 | quietLogs(); | |
| 24 | ||
| 25 | describe("pillCheck", () => { | |
| 26 | let dataDir: string; | |
| 27 | let app: App; | |
| 28 | let repoId: string; | |
| 29 | let issueId: string; | |
| 30 | ||
| 31 | beforeAll(async () => { | |
| 32 | dataDir = await mkdtemp(join(tmpdir(), "forge-pill-check-")); | |
| 33 | app = createApp({ dataDir, port: 0, host: "forge.test" }); | |
| 34 | const u = await createUser(app.db, { | |
| 35 | origin: "seed", trusted: true, username: "alice", email: "[email protected]", | |
| 36 | password: "a-good-passphrase", adminIfFirst: true, | |
| 37 | }); | |
| 38 | if (!u.ok) throw new Error(u.error); | |
| 39 | const made = await newApp(app, u.value, { name: "open", description: "x", visibility: "public" }); | |
| 40 | if (!made.ok) throw new Error(String(made.error)); | |
| 41 | repoId = made.value.repo.id; | |
| 42 | const pull = createIssue(app.db, { repoId, authorId: u.value.id, title: "p", kind: "pull" }); | |
| 43 | if (!pull.ok) throw new Error(pull.error); | |
| 44 | issueId = pull.value.id; | |
| 45 | }, 120_000); | |
| 46 | ||
| 47 | afterAll(async () => { | |
| 48 | app.close(); | |
| 49 | await rm(dataDir, { recursive: true, force: true }); | |
| 50 | }); | |
| 51 | ||
| 52 | const add = (id: string, oid: string, state: string) => | |
| 53 | app.db.run( | |
| 54 | `INSERT INTO run (id, repo_id, kind, issue_id, commit_oid, state) VALUES (?1, ?2, 'check', ?3, ?4, ?5)`, | |
| 55 | [id, repoId, issueId, oid, state], | |
| 56 | ); | |
| 57 | ||
| 58 | test("no check is said so; the newest check of this head decides", () => { | |
| 59 | expect(pillCheck(app.db, issueId, "aaa")).toBe("none"); | |
| 60 | add("01a", "aaa", "failed"); | |
| 61 | expect(pillCheck(app.db, issueId, "aaa")).toBe("failed"); | |
| 62 | add("01b", "aaa", "running"); | |
| 63 | expect(pillCheck(app.db, issueId, "aaa")).toBe("running"); | |
| 64 | add("01c", "aaa", "ok"); | |
| 65 | expect(pillCheck(app.db, issueId, "aaa")).toBe("passed"); | |
| 66 | add("01e", "ccc", "cancelled"); | |
| 67 | expect(pillCheck(app.db, issueId, "ccc")).toBe("cancelled"); | |
| 68 | add("01d", "bbb", "queued"); | |
| 69 | expect(pillCheck(app.db, issueId, "bbb")).toBe("waiting"); | |
| 70 | expect(pillCheck(app.db, issueId, "aaa")).toBe("passed"); | |
| 71 | }); | |
outdated
toContain on source strings is not a behaviour test; add pillWhere tests (readable vs not, empty head) and a production render that names the app. |
||
| 72 | }); | |
| 73 | ||
| 74 | describe("the pill's check line", () => { | |
| 75 | const drawn = (set: Record<string, string>) => | |
| 76 | boot(null, 400, 800, set).texts(); | |
| 77 | ||
| 78 | test("rides as an attribute; production's tag carries none", () => { | |
| 79 | expect(toolbarTag("http://f.test", { app: "a", env: "pull-3", pull: 3, check: "failed" })).toContain( | |
| 80 | ` data-check="failed"`, | |
| 81 | ); | |
| 82 | expect(toolbarTag("http://f.test", { app: "a", env: "production" })).not.toContain("data-check"); | |
| 83 | }); | |
| 84 | ||
| 85 | test("the panel script draws the line for a pull, never for production or an unknown key", () => { | |
| 86 | const pull = { app: "notes", env: "pull-3", pull: "3" }; | |
| 87 | expect(drawn({ ...pull, check: "failed" })).toContain(CHECK_WORDS.failed); | |
| 88 | expect(drawn({ ...pull, check: "cancelled" })).toContain("Checks cancelled"); | |
| 89 | expect(drawn({ ...pull, check: "bogus" }).filter((t) => t.startsWith("Checks"))).toEqual([]); | |
| 90 | expect(drawn({ ...pull, check: "constructor" }).filter((t) => t.startsWith("Checks"))).toEqual([]); | |
| 91 | expect(drawn(pull).some((t) => /^(Checks|No checks)/.test(t))).toBe(false); | |
| 92 | // Production, even handed a check, names the app and says no check. | |
| 93 | const prod = drawn({ app: "notes", env: "production", check: "passed" }); | |
| 94 | expect(prod).toContain("notes"); | |
| 95 | expect(prod).toContain("Production"); | |
| 96 | expect(prod.some((t) => /^(Checks|No checks)/.test(t))).toBe(false); | |
| 97 | }); | |
| 98 | ||
| 99 | test("the bar is fixed-position and takes no room in the page", () => { | |
| 100 | const b = boot(null, 360, 640, { app: "notes", env: "pull-3", pull: "3", check: "passed" }); | |
| 101 | expect(b.host.style.cssText).toContain("position:fixed"); | |
| 102 | expect(b.host.style.cssText).not.toMatch(/margin|padding/); | |
| 103 | expect(toolbarJs).not.toMatch(/document\.body\.style|documentElement\.style|body\.style\.padding/); | |
| 104 | }); | |
| 105 | }); | |
| 106 | ||
| 107 | describe("pillWhere's check", () => { | |
| 108 | let dataDir: string; | |
| 109 | let app: App; | |
| 110 | let repoId: string; | |
| 111 | let aliceId: string; | |
| 112 | let target: AppTarget; | |
| 113 | const runtime = () => ({ app }) as unknown as AppsRuntime; | |
| 114 | ||
| 115 | beforeAll(async () => { | |
| 116 | dataDir = await mkdtemp(join(tmpdir(), "forge-pill-check-where-")); | |
| 117 | app = createApp({ dataDir, port: 0, host: "forge.test" }); | |
| 118 | const a = await createUser(app.db, { | |
| 119 | origin: "seed", trusted: true, username: "alice", email: "[email protected]", | |
| 120 | password: "a-good-passphrase", adminIfFirst: true, | |
| 121 | }); | |
| 122 | if (!a.ok) throw new Error(a.error); | |
| 123 | aliceId = a.value.id; | |
| 124 | const made = await newApp(app, a.value, { name: "priv", description: "x", visibility: "private" }); | |
| 125 | if (!made.ok) throw new Error(String(made.error)); | |
| 126 | repoId = made.value.repo.id; | |
| 127 | target = { app: getAppByRepo(app.db, repoId)!, env: "production" } as AppTarget; | |
| 128 | }, 120_000); | |
| 129 | ||
| 130 | afterAll(async () => { | |
| 131 | app.close(); | |
| 132 | await rm(dataDir, { recursive: true, force: true }); | |
| 133 | }); | |
| 134 | ||
| 135 | const pull = (head: string, run?: string) => { | |
| 136 | const p = createIssue(app.db, { repoId, authorId: aliceId, title: "t", kind: "pull" }); | |
| 137 | if (!p.ok) throw new Error(p.error); | |
| 138 | app.db.run("INSERT INTO pull (issue_id, base_branch, head_branch, base_oid, head_oid) VALUES (?1, 'main', 'topic', ?2, ?2)", [p.value.id, head || "b".repeat(40)]); | |
| 139 | if (head === "") app.db.run("UPDATE pull SET head_oid='' WHERE issue_id=?1", [p.value.id]); | |
| 140 | if (run) | |
| 141 | app.db.run( | |
| 142 | `INSERT INTO run (id, repo_id, kind, issue_id, commit_oid, state) VALUES (?1, ?2, 'check', ?3, ?4, ?5)`, | |
| 143 | [`r${p.value.number}`, repoId, p.value.id, head, run], | |
| 144 | ); | |
| 145 | return { ...target, env: `pull-${p.value.number}` } as AppTarget; | |
| 146 | }; | |
| 147 | ||
| 148 | test("a reader gets the check of the head", () => { | |
| 149 | const t = pull("c".repeat(40), "ok"); | |
| 150 | expect(pillWhere(runtime(), t, { id: aliceId } as never).check).toBe("passed"); | |
| 151 | }); | |
| 152 | ||
| 153 | test("a head with no run says none", () => { | |
| 154 | const t = pull("d".repeat(40)); | |
| 155 | expect(pillWhere(runtime(), t, { id: aliceId } as never).check).toBe("none"); | |
| 156 | }); | |
| 157 | ||
| 158 | test("a viewer who cannot read the pull gets no check", () => { | |
| 159 | const t = pull("e".repeat(40), "failed"); | |
| 160 | const w = pillWhere(runtime(), t, undefined); | |
| 161 | expect(w.check).toBeUndefined(); | |
| 162 | expect(w.title).toBeUndefined(); | |
| 163 | }); | |
| 164 | ||
| 165 | test("a pull with an empty head gets no check", () => { | |
| 166 | const t = pull(""); | |
| 167 | const w = pillWhere(runtime(), t, { id: aliceId } as never); | |
| 168 | expect(w.title).toBe("t"); | |
| 169 | expect(w.check).toBeUndefined(); | |
| 170 | }); | |
| 171 | }); |
test/helpers/toolbar-boot.ts +3 −3
| @@ -28,11 +28,11 @@ const make = (tag: string): El => ({ | ||
| 28 | 28 | }); |
| 29 | 29 | const all = (el: El): El[] => [el, ...el.children.flatMap(all)]; |
| 30 | 30 | |
| 31 | export function boot(store: Record<string, string> | null, width = 400, height = 800) { | |
| 31 | export function boot(store: Record<string, string> | null, width = 400, height = 800, dataset: Record<string, string> = { app: "notes", env: "production" }) { | |
| 32 | 32 | let root!: El; |
| 33 | 33 | const host = { el: null as El | null }; |
| 34 | 34 | const doc = { |
| 35 | currentScript: { src: "https://forge.test/assets/t.js", dataset: { app: "notes", env: "production" } }, | |
| 35 | currentScript: { src: "https://forge.test/assets/t.js", dataset }, | |
| 36 | 36 | createElement: (tag: string) => { |
| 37 | 37 | const el = make(tag); |
| 38 | 38 | if (tag === "div" && !host.el) { |
| @@ -56,5 +56,5 @@ export function boot(store: Record<string, string> | null, width = 400, height = | ||
| 56 | 56 | const box = root.children[0]!; |
| 57 | 57 | const panel = els.find((e) => e.attrs.role === "region")!; |
| 58 | 58 | const fire = (el: El, t: string, e: object = {}) => el.listeners[t]!.forEach((f) => f({ preventDefault() {}, type: t, ...e })); |
| 59 | return { host: host.el!, pill, box, panel, fire }; | |
| 59 | return { host: host.el!, pill, box, panel, fire, texts: () => all(panel).map((e) => e.textContent).filter(Boolean) }; | |
| 60 | 60 | } |
const row = head;is a pointless alias; useheadbelow (or rename). Also line 568 (the new where.check path) has no test.