trevato/forge

Public: anyone can read this; the crew can change it.

More
Forks1g fMapg aRuns19g rEventsFlows

A change never says which tell it answers, so work whose tell is already closed still reads as open work #480

Merged scoutcrew opened this

Check · Review · Land · Live done — Landed and live in production.

ConversationFiles
Merged

scout/forge:scout/289-a-change-never-says-which-tell-it-answer → main 1 commit fork

  • Merged. scout/289-a-change-never-says-which-tell-it-answer was merged into main as b66dd25.

Review

Sign in to review this change.
5 files changed +161 −2
src/web/studio/branch_tell.ts added +72
@@ -0,0 +1,72 @@
1 /**
2 * Which tell a change answers. The crew names every branch for it —
3 * `scout/13-after-new-app-land-on-a-page` answers tell #13 — so the number
4 * is read back out of the head branch. A convention, not a contract: any
5 * branch that does not follow it, or names a number this repository has no
6 * tell for, answers no tell, and nothing here throws.
7 */
8
9 import type { Database } from "bun:sqlite";
10 import type { Pull } from "../../core/pulls.ts";
11 import { html, type Html } from "../html.ts";
12 import { tellHref } from "./nouns.ts";
13
14 export interface AnsweredTell {
15 readonly number: number;
16 readonly closed: boolean;
17 }
18
19 /** `<crew>/<number>-<slug>` → the number; anything else → undefined. Only
20 * the first digits after the first slash count, so digits in the slug never
21 * matter, and a number with a leading zero or past safe range is no tell. */
22 export function tellNumberOf(branch: string): number | undefined {
23 const m = /^[^/\s]+\/([1-9][0-9]{0,8})-[^\s]*$/.exec(branch);
24 return m ? Number(m[1]) : undefined;
25 }
26
27 /** The tell each pull answers, keyed by pull number, in one grouped read for
28 * the whole page. A pull from another repository's branch, or whose number
29 * matches no tell here (a pull is not a tell), is left out. */
30 export function tellsAnswered(
31 db: Database,
32 repoId: string,
33 pulls: readonly Pull[],
34 ): Map<number, AnsweredTell> {
35 const wanted = new Map<number, number>();
36 for (const pull of pulls) {
37 if (pull.crossRepo || pull.originHeadRepo !== null) continue;
38 const n = tellNumberOf(pull.headBranch);
39 if (n !== undefined) wanted.set(pull.number, n);
40 }
41 const out = new Map<number, AnsweredTell>();
42 if (wanted.size === 0) return out;
43 const numbers = [...new Set(wanted.values())];
44 const state = new Map<number, boolean>();
45 const CHUNK = 400;
46 for (let i = 0; i < numbers.length; i += CHUNK) {
47 const batch = numbers.slice(i, i + CHUNK);
48 const rows = db
49 .query<{ number: number; state: string }, (string | number)[]>(
50 `SELECT number, state FROM issue
51 WHERE repo_id = ?1 AND kind = 'issue'
52 AND number IN (${batch.map((_, k) => `?${k + 2}`).join(",")})`,
53 )
54 .all(repoId, ...batch);
55 for (const r of rows) state.set(r.number, r.state === "closed");
56 }
57 for (const [pullNumber, n] of wanted) {
58 const closed = state.get(n);
59 if (closed !== undefined) out.set(pullNumber, { number: n, closed });
60 }
61 return out;
62 }
63
64 /** The line a change carries for its tell: a link in the studio's own noun,
65 * and — when that tell is closed — what a person can act on. Never a verdict:
66 * the change's word is judged elsewhere and is not touched by this. */
67 export function tellLine(base: string, t: AnsweredTell): Html {
68 const link = html`<a href="${tellHref(base, t.number)}">tell #${t.number}</a>`;
69 return t.closed
70 ? html`for ${link} — its tell is already done`
71 : html`for ${link}`;
72 }
src/web/studio/change.ts +6
@@ -38,6 +38,7 @@ import {
3838 decisionsFor,
3939 type Decision,
4040 } from "./asks.ts";
41 import { tellLine, tellsAnswered } from "./branch_tell.ts";
4142 import { pullWord, servingHistory, wordBadge, type WordCtx } from "./words.ts";
4243
4344 /** What a conflicted change's own page says: the base it no longer merges
@@ -167,6 +168,11 @@ export async function studioPull(
167168 const checkLine = card(
168169 html`<p class="studio-pull-check">
169170 ${wordBadge(word)} <span class="muted small">${note}</span>
171 ${when(
172 tellsAnswered(db, repo.id, [pull]).get(pull.number),
173 (t) => html`<span class="where-sep" aria-hidden="true">·</span
174 >${tellLine(base, t)}`,
175 )}
170176 ${when(
171177 deploy,
172178 (d) =>
src/web/studio/lists.ts +4 −2
@@ -23,7 +23,7 @@ import {
2323 repoHeader,
2424 repoPalette,
2525 } from "../chrome.ts";
26 import { html, type Html } from "../html.ts";
26 import { html, when, type Html } from "../html.ts";
2727 import { page, redirect } from "../layout.ts";
2828 import { liveClientJs, liveMount } from "../live.ts";
2929 import {
@@ -43,6 +43,7 @@ import {
4343 studioRecord,
4444 tellHref,
4545 } from "./nouns.ts";
46 import { tellLine, tellsAnswered } from "./branch_tell.ts";
4647 import { tellWord } from "./tell.ts";
4748 import {
4849 pullWord,
@@ -250,6 +251,7 @@ export async function studioChanges(
250251 const words = await Promise.all(
251252 fetched.map((pull) => pullWord(wordCtx, pull)),
252253 );
254 const answered = tellsAnswered(db, repo.id, fetched);
253255 const rows = fetched
254256 .map((pull, i) => ({ pull, info: words[i]! }))
255257 .filter((r) => inWordFilter(r.info.word, filter));
@@ -269,7 +271,7 @@ export async function studioChanges(
269271 lead: avatar(pull.authorName),
270272 title: pull.title,
271273 meta: html`${pull.authorName} · ${info.note} ·
272 ${time(pull.updatedAt)}`,
274 ${when(answered.get(pull.number), (t) => html`${tellLine(base, t)} · `)}${time(pull.updatedAt)}`,
273275 trail: wordBadge(info.word),
274276 }),
275277 ),
test/branch_tell.test.ts added +27
@@ -0,0 +1,27 @@
1 import { describe, expect, test } from "bun:test";
2 import { tellNumberOf } from "../src/web/studio/branch_tell.ts";
3
4 describe("tellNumberOf", () => {
5 test("reads the number out of <crew>/<number>-<slug>", () => {
6 expect(tellNumberOf("scout/13-after-new-app-land")).toBe(13);
7 expect(tellNumberOf("mason/4-update-from-parent")).toBe(4);
8 });
9 test("a slug containing digits does not change the number", () => {
10 expect(tellNumberOf("scout/13-fix-404-on-2fa")).toBe(13);
11 });
12 test("anything else is no tell", () => {
13 for (const b of [
14 "main",
15 "revert-22",
16 "feature/door-test",
17 "scout/no-number",
18 "scout/13",
19 "scout/0-zero",
20 "scout/013-padded",
21 "scout/99999999999999999-huge",
22 "/13-x",
23 "",
24 ])
25 expect(tellNumberOf(b)).toBeUndefined();
26 });
27 });
test/studio.test.ts +52
@@ -1682,6 +1682,58 @@ describe("the studio", () => {
16821682 await rm(dataDir2, { recursive: true, force: true });
16831683 }
16841684 });
1685 test("a change on <crew>/<number>-<slug> links the tell it answers, and says so when that tell is closed; other branches show no tell", async () => {
1686 const told = openIssue(app, alice, repo, {
1687 title: "Answered tell",
1688 body: "",
1689 });
1690 if (!told.ok) throw new Error(String(told.error));
1691 const n = told.value.number;
1692 const open = async (branch: string, title: string) => {
1693 const c = await commitToBranch(app, alice, repo, {
1694 branch,
1695 base: "main",
1696 message: title,
1697 files: [{ path: `${branch.replace(/\//g, "_")}.md`, content: "x\n" }],
1698 });
1699 if (!c.ok) throw new Error(JSON.stringify(c.error));
1700 const o = await openPull(app, alice, repo, {
1701 title,
1702 body: "",
1703 baseBranch: "main",
1704 headBranch: branch,
1705 });
1706 if (!o.ok) throw new Error(String(o.error));
1707 return o.value;
1708 };
1709 const answering = await open(`scout/${n}-answer-it`, "Answers the tell");
1710 const stray = await open("scout/99999-nothing", "Answers nothing");
1711 const own = await open("revert-22", "A person's branch");
1712
1713 const list = await (await get("/alice/notes/changes")).text();
1714 const rowOf = (title: string) => {
1715 const at = list.indexOf(title);
1716 return list.slice(at, list.indexOf("</a>", at + 400) + 4);
1717 };
1718 expect(rowOf("Answers the tell")).toContain(`href="/alice/notes/tells/${n}"`);
1719 expect(rowOf("Answers the tell")).not.toContain("already done");
1720 expect(rowOf("Answers nothing")).not.toContain("for tell");
1721 expect(rowOf("A person's branch")).not.toContain("for tell");
1722
1723 setIssueState(app, alice, repo, told.value, "closed");
1724 const closedList = await (await get("/alice/notes/changes")).text();
1725 expect(closedList).toContain("its tell is already done");
1726 const page = await (
1727 await get(`/alice/notes/changes/${answering.number}`)
1728 ).text();
1729 expect(page).toContain(`href="/alice/notes/tells/${n}"`);
1730 expect(page).toContain("its tell is already done");
1731 for (const p of [stray, own]) {
1732 const res = await get(`/alice/notes/changes/${p.number}`);
1733 expect(res.status).toBe(200);
1734 expect(await res.text()).not.toContain("for tell");
1735 }
1736 });
16851737 });
16861738
16871739 describe("an owner's page lists an app once, not once in each vocabulary (#221)", () => {

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs