Public: anyone can read this; the crew can change it.
Each /review row says what the crew advises and why, or that it has not advised yet #4736
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/4730-each-review-row-says-what-the-crew-advis → main 1 commit fork 2 threads · 2 unresolved
3 files
changed +28
−4
README.md +1 −1
| @@ -449,7 +449,7 @@ only, shows a guest nothing, and names nothing of a placement in a repository | ||
| 449 | 449 | you cannot read. |
| 450 | 450 | |
| 451 | 451 | Review: `/review` lists the asks waiting on you, one row each (title, repository, |
| 452 | age, why), newest first. It only reads; the buttons stay on `/asks`, which links | |
| 452 | age, why, and the crew's advice or that it has not advised yet), newest first. It only reads; the buttons stay on `/asks`, which links | |
| 453 | 453 | to it, and signed out it sends you to sign-in. The terminal has no page of its |
| 454 | 454 | own for it: the same asks are the dashboard's On you card. |
| 455 | 455 |
src/web/studio/review_page.ts +15 −2
| @@ -1,11 +1,12 @@ | ||
| 1 | 1 | /** |
| 2 | 2 | * `/review`: every ask waiting on the viewer as one row — the title, the |
| 3 | * repository, how long it has waited and why a person must decide — newest | |
| 3 | * repository, how long it has waited and why a person must decide, and what the crew advises — newest | |
| 4 | 4 | * decision first. Read from the same candidates the asks page reads |
| 5 | 5 | * (`waitingOn`); no polling, and nothing here changes anything. |
| 6 | 6 | */ |
| 7 | 7 | |
| 8 | 8 | import { notePhase, takeSessionTime, timePhase } from "../../core/route_timing.ts"; |
| 9 | import { recommendationsFor } from "../../core/pull_wait.ts"; | |
| 9 | 10 | import { listReposVisibleTo } from "../../core/repos.ts"; |
| 10 | 11 | import type { Ctx } from "../../http/context.ts"; |
| 11 | 12 | import { repoBase, signInFirst } from "../chrome.ts"; |
| @@ -23,6 +24,13 @@ export function reviewPage(ctx: Ctx): Response { | ||
| 23 | 24 | return waitingOn(ctx.app.db, user, repos).sort((a, b) => Date.parse(b.since) - Date.parse(a.since)); |
| 24 | 25 | }); |
| 25 | 26 | const rendering = performance.now(); |
| 27 | const advice = recommendationsFor( | |
| 28 | ctx.app.db, | |
| 29 | asks.flatMap((a) => { | |
| 30 | const id = a.pull?.id ?? a.issue?.id; | |
| 31 | return id === undefined ? [] : [id]; | |
| 32 | }), | |
| 33 | ); | |
| 26 | 34 | const rows = asks.map((a) => { |
| 27 | 35 | const base = repoBase(a.repo); |
| 28 | 36 | const href = |
| @@ -31,12 +39,17 @@ export function reviewPage(ctx: Ctx): Response { | ||
| 31 | 39 | : a.issue !== undefined |
| 32 | 40 | ? `${base}/issues/${a.issue.number}` |
| 33 | 41 | : base; |
| 42 | const rec = advice.get((a.pull?.id ?? a.issue?.id) ?? ""); | |
| 43 | const advised = | |
Same id expression as the flatMap above; build one list of [ask, id] or a small helper so they cannot drift. |
||
| 44 | rec === undefined | |
| 45 | ? "The crew has not advised yet" | |
| 46 | : html`The crew advises: ${rec.verdict}${rec.reason === "" ? "" : html` — ${rec.reason}`}`; | |
| 34 | 47 | return row({ |
| 35 | 48 | href, |
| 36 | 49 | nav: true, |
| 37 | 50 | title: a.pull?.title ?? a.issue?.title ?? a.proposal?.title ?? `${a.repo.name} is waiting on you`, |
| 38 | 51 | titleAttr: `${a.repo.ownerName}/${a.repo.name}: ${a.why ?? a.consequence.why}`, |
| 39 | meta: html`${a.repo.ownerName}/${a.repo.name} · ${time(a.since)} · ${a.why ?? a.consequence.why}`, | |
| 52 | meta: html`${a.repo.ownerName}/${a.repo.name} · ${time(a.since)} · ${a.why ?? a.consequence.why} · ${advised}`, | |
| 40 | 53 | }); |
| 41 | 54 | }); |
| 42 | 55 | const body = html`<h1>Review</h1> |
test/review-page.test.ts +12 −1
| @@ -5,6 +5,7 @@ import { tmpdir } from "node:os"; | ||
| 5 | 5 | import { join } from "node:path"; |
| 6 | 6 | import { createApp, type App } from "../src/app.ts"; |
| 7 | 7 | import { addComment, openIssue } from "../src/actions/issues.ts"; |
| 8 | import { addComment as addRawComment } from "../src/core/issues.ts"; | |
| 8 | 9 | import { addCrew, createAgentRow, setCrewRole } from "../src/core/agents.ts"; |
| 9 | 10 | import { createRepo as createRepoRow } from "../src/core/repos.ts"; |
| 10 | 11 | import { submitReview } from "../src/core/reviews.ts"; |
| @@ -76,7 +77,8 @@ beforeAll(async () => { | ||
| 76 | 77 | const handed = pull(builder.id, "Handed over pull", 3); |
| 77 | 78 | for (let r = 0; r < 3; r++) |
| 78 | 79 | submitReview(app.db, { issueId: handed, authorId: reviewer.id, state: "changes_requested", body: `Round ${r}.`, commitOid: ZERO }); |
| 79 | pull(alice.id, "Stale pull", 30); | |
| 80 | const stale = pull(alice.id, "Stale pull", 30); | |
| 81 | addRawComment(app.db, { issueId: stale, authorId: reviewer.id, body: "Recommendation: change — rename the <flag>." }); | |
| 80 | 82 | pull(builder.id, "Young crew pull", 2); |
| 81 | 83 | const issue = openIssue(app, alice, repo, { title: "Crew asked about this" }); |
| 82 | 84 | if (!issue.ok) throw new Error(String(issue.error)); |
| @@ -124,3 +126,12 @@ test("the asks page links to /review", async () => { | ||
| 124 | 126 | const res = await fetch(`${servers.url}/asks`, { headers: { cookie } }); |
| 125 | 127 | expect(await res.text()).toContain('href="/review"'); |
| 126 | 128 | }); |
| 129 | ||
| 130 | test("each row says what the crew advises and why, or that it has not advised yet", async () => { | |
| 131 | const body = await (await fetch(`${servers.url}/review`, { headers: { cookie } })).text(); | |
| 132 | const rows = body.split("<li").filter((r) => r.includes('class="row"')); | |
| 133 | const find = (t: string) => rows.find((r) => r.includes(t))!; | |
| 134 | expect(find("Stale pull")).toContain("The crew advises: change — rename the <flag>."); | |
| 135 | expect(find("Handed over pull")).toContain("The crew has not advised yet"); | |
| 136 | expect(find("Crew asked about this")).toContain("The crew has not advised yet"); | |
| 137 | }); |
new above
offline
Re-wrap this line to the paragraph width.