Public: anyone can read this; the crew can change it.
Every move on /next is recorded in the audit trail with who, what and from where #11911
Check · Review · Land · Live — Its check is queued.
torque/forge:torque/11754-every-move-on-next-is-recorded-in-the-au → main 1 commit fork
3 files
changed +103
−2
src/actions/next.ts +10 −2
| @@ -62,7 +62,10 @@ export function rankIssue( | ||
| 62 | 62 | } |
| 63 | 63 | // Checked before anything is written: a place that is not there, or a list |
| 64 | 64 | // too long, changes nothing. |
| 65 | const rest = nextUp(app.db, repo).rows.map((r) => r.issue).filter((i) => i.id !== issue.id); | |
| 65 | const listed = nextUp(app.db, repo).rows.map((r) => r.issue); | |
| 66 | const rest = listed.filter((i) => i.id !== issue.id); | |
| 67 | // Where it stood in the list as read, 1 at the top. | |
| 68 | const from = listed.findIndex((i) => i.id === issue.id) + 1; | |
| 66 | 69 | const at = place.to === "after" ? rest.findIndex((i) => i.number === place.number) + 1 : 0; |
| 67 | 70 | if (place.to === "after" && at === 0) return invalid(NEXT_NO_PLACE); |
| 68 | 71 | // The list is ranked down to where it was dropped, and whatever was ranked stays. |
| @@ -87,7 +90,12 @@ export function rankIssue( | ||
| 87 | 90 | kind: "issue.ranked", |
| 88 | 91 | repoId: repo.id, |
| 89 | 92 | actorId: actor.id, |
| 90 | payload: { number: issue.number, to: place.to }, | |
| 93 | // The audit entry carries these fields and nothing else: the issue, the | |
| 94 | // place (`after` only for `after #N`) and the position it left (0: not listed). | |
| 95 | payload: | |
| 96 | place.to === "after" | |
| 97 | ? { number: issue.number, to: place.to, after: place.number, from } | |
| 98 | : { number: issue.number, to: place.to, from }, | |
| 91 | 99 | }); |
| 92 | 100 | return ok(issue); |
| 93 | 101 | } |
src/design/activity.ts +14
| @@ -11,6 +11,7 @@ | ||
| 11 | 11 | |
| 12 | 12 | import { VIA_WORDS } from "../core/acting_via.ts"; |
| 13 | 13 | import { DETACH_RECEIPT } from "../actions/detach_words.ts"; |
| 14 | import { rankedWords, type Place } from "../actions/next.ts"; | |
| 14 | 15 | import type { ForgeEvent } from "../core/events.ts"; |
| 15 | 16 | |
| 16 | 17 | /** The pull number an environment name carries, for `pull-12`. */ |
| @@ -36,6 +37,19 @@ export function eventVerb(event: ForgeEvent): string { | ||
| 36 | 37 | if (state === "failed") return "ended a crew run without landing"; |
| 37 | 38 | return "had a crew run cancelled"; |
| 38 | 39 | } |
| 40 | if (kind === "issue.ranked") { | |
| 41 | const n = payload["number"]; | |
| 42 | const to = payload["to"]; | |
| 43 | const place: Place | undefined = | |
| 44 | to === "top" || to === "not_now" | |
| 45 | ? { to } | |
| 46 | : to === "after" && typeof payload["after"] === "number" | |
| 47 | ? { to, number: payload["after"] } | |
| 48 | : undefined; | |
| 49 | // The forge's own words for a move, `rankedWords`, as a verb phrase. | |
| 50 | if (typeof n === "number" && place !== undefined) | |
| 51 | return `ranked an issue: ${rankedWords(n, place)}`; | |
| 52 | } | |
| 39 | 53 | if (kind === "issue.assigned" || kind === "pull.assigned") { |
| 40 | 54 | const thing = kind === "issue.assigned" ? "an issue" : "a pull request"; |
| 41 | 55 | const to = payload["assignee"]; |
test/next-audit.test.ts added +79
| @@ -0,0 +1,79 @@ | ||
| 1 | /** Every move on /next is one audit entry: who, what, where to and where from; a refused move writes none (#11754). */ | |
| 2 | import { afterAll, beforeAll, expect, test } from "bun:test"; | |
| 3 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 4 | import { tmpdir } from "node:os"; | |
| 5 | import { join } from "node:path"; | |
| 6 | import { createApp, type App } from "../src/app.ts"; | |
| 7 | import { openIssue } from "../src/actions/issues.ts"; | |
| 8 | import { rankIssue } from "../src/actions/next.ts"; | |
| 9 | import { createRepo } from "../src/actions/repos.ts"; | |
| 10 | import { eventVerb } from "../src/design/activity.ts"; | |
| 11 | import { listEvents } from "../src/core/events.ts"; | |
| 12 | import type { Repo } from "../src/core/repos.ts"; | |
| 13 | import { createUser, type User } from "../src/core/users.ts"; | |
| 14 | ||
| 15 | let dir: string; | |
| 16 | let app: App; | |
| 17 | let alice: User; | |
| 18 | let bob: User; | |
| 19 | let repo: Repo; | |
| 20 | const n: number[] = []; | |
| 21 | ||
| 22 | const entries = () => listEvents(app.db, { repoId: repo.id, kind: "issue.ranked", limit: 100 }); | |
| 23 | ||
| 24 | beforeAll(async () => { | |
| 25 | dir = await mkdtemp(join(tmpdir(), "forge-next-audit-")); | |
| 26 | app = createApp({ dataDir: dir, port: 0 }); | |
| 27 | const a = await createUser(app.db, { origin: "seed", trusted: true, username: "alice", email: "[email protected]", password: "a-good-passphrase", adminIfFirst: true }); | |
| 28 | const b = await createUser(app.db, { origin: "seed", trusted: true, username: "bob", email: "[email protected]", password: "a-good-passphrase" }); | |
| 29 | if (!a.ok || !b.ok) throw new Error("users"); | |
| 30 | alice = a.value; | |
| 31 | bob = b.value; | |
| 32 | const r = await createRepo(app, alice, { name: "audit", visibility: "public" }); | |
| 33 | if (!r.ok) throw new Error(JSON.stringify(r.error)); | |
| 34 | repo = r.value; | |
| 35 | const gate = openIssue(app, alice, repo, { title: "Later gate: not now" }); | |
| 36 | if (!gate.ok) throw new Error(String(gate.error)); | |
| 37 | for (let i = 1; i <= 3; i++) { | |
| 38 | const o = openIssue(app, alice, repo, { title: `Issue ${i}` }); | |
| 39 | if (!o.ok) throw new Error(String(o.error)); | |
| 40 | n.push(o.value.number); | |
| 41 | } | |
| 42 | }); | |
| 43 | ||
| 44 | afterAll(async () => { | |
| 45 | app.close(); | |
| 46 | await rm(dir, { recursive: true, force: true }); | |
| 47 | }); | |
| 48 | ||
| 49 | test("a move writes exactly one entry with who, what, where to and where from, and nothing else", () => { | |
| 50 | const before = entries().length; | |
| 51 | expect(rankIssue(app, alice, repo, n[2]!, { to: "top" }).ok).toBe(true); | |
| 52 | const made = entries(); | |
| 53 | expect(made.length).toBe(before + 1); | |
| 54 | const e = made[0]!; | |
| 55 | expect(e.actorId).toBe(alice.id); | |
| 56 | expect(e.payload).toEqual({ number: n[2]!, to: "top", from: 3 }); | |
| 57 | expect(eventVerb(e)).toContain(`#${n[2]} is on top.`); | |
| 58 | }); | |
| 59 | ||
| 60 | test("a place under another row is named, and the position it left", () => { | |
| 61 | expect(rankIssue(app, alice, repo, n[2]!, { to: "after", number: n[1]! }).ok).toBe(true); | |
| 62 | const e = entries()[0]!; | |
| 63 | expect(e.payload).toEqual({ number: n[2]!, to: "after", after: n[1]!, from: 1 }); | |
| 64 | expect(eventVerb(e)).toContain(`#${n[2]} moved under #${n[1]}.`); | |
| 65 | }); | |
| 66 | ||
| 67 | test("not now is one entry too", () => { | |
| 68 | expect(rankIssue(app, alice, repo, n[0]!, { to: "not_now" }).ok).toBe(true); | |
| 69 | expect(entries()[0]!.payload).toMatchObject({ number: n[0]!, to: "not_now" }); | |
| 70 | }); | |
| 71 | ||
| 72 | test("a forbidden, missing or invalid move writes none", () => { | |
| 73 | const before = entries().length; | |
| 74 | expect(rankIssue(app, bob, repo, n[1]!, { to: "top" }).ok).toBe(false); | |
| 75 | expect(rankIssue(app, alice, repo, 9999, { to: "top" }).ok).toBe(false); | |
| 76 | expect(rankIssue(app, alice, repo, n[1]!, { to: "after", number: 9999 }).ok).toBe(false); | |
| 77 | expect(rankIssue(app, alice, repo, n[0]!, { to: "not_now" }).ok).toBe(false); | |
| 78 | expect(entries().length).toBe(before); | |
| 79 | }); |
new above
offline