Public: anyone can read this; the crew can change it.
The studio says "Close pull request" #182
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/171-the-studio-says-close-pull-request → main 1 commit fork
6 files
changed +66
−20
src/web/pages/issues.ts +1 −1
| @@ -520,7 +520,7 @@ export async function issuePage(ctx: Ctx): Promise<Response> { | ||
| 520 | 520 | action: `${thread}/comments`, |
| 521 | 521 | stateAction: `${thread}/state`, |
| 522 | 522 | state: issue.state, |
| 523 | kind: "issue", | |
| 523 | closeLabel: "Close issue", | |
| 524 | 524 | })} |
| 525 | 525 | </div> |
| 526 | 526 | <aside> |
src/web/pages/pull.ts +1 −1
| @@ -800,7 +800,7 @@ export async function pullPage(ctx: Ctx): Promise<Response> { | ||
| 800 | 800 | stateAction: `${thread}/state`, |
| 801 | 801 | state: pull.state, |
| 802 | 802 | locked: pull.mergedAt !== null, |
| 803 | kind: "pull", | |
| 803 | closeLabel: "Close pull request", | |
| 804 | 804 | })} |
| 805 | 805 | ${reviewCard(ctx, repo, pull, `${thread}/review-state`)} |
| 806 | 806 | </div> |
src/web/pages/thread.ts +17 −12
| @@ -47,6 +47,7 @@ import { | ||
| 47 | 47 | type Tone, |
| 48 | 48 | } from "../ui.ts"; |
| 49 | 49 | import { loadRepo, notFoundPage, repoBase, signInFirst } from "../chrome.ts"; |
| 50 | import { studioRecord } from "../studio/nouns.ts"; | |
| 50 | 51 | |
| 51 | 52 | /** |
| 52 | 53 | * The four ways a write can fail *because of the origin*, in words. |
| @@ -294,7 +295,10 @@ export interface ComposerOptions { | ||
| 294 | 295 | state: "open" | "closed"; |
| 295 | 296 | /** A merged pull can never reopen. */ |
| 296 | 297 | locked?: boolean; |
| 297 | kind: "issue" | "pull"; | |
| 298 | /** What the close button calls the thing here — dev's own noun | |
| 299 | * ("pull request", "issue") or the studio's ("change", "tell"). The | |
| 300 | * caller knows which page it is drawing; the composer does not have to. */ | |
| 301 | closeLabel: string; | |
| 298 | 302 | } |
| 299 | 303 | |
| 300 | 304 | /** The comment box. Close and reopen are their own form, not an `intent` |
| @@ -321,16 +325,11 @@ export function composer(ctx: Ctx, o: ComposerOptions): Html { | ||
| 321 | 325 | : null; |
| 322 | 326 | const closeButton = |
| 323 | 327 | o.stateAction !== undefined && !o.locked |
| 324 | ? btn( | |
| 325 | o.state === "open" | |
| 326 | ? `Close ${o.kind === "pull" ? "pull request" : "issue"}` | |
| 327 | : "Reopen", | |
| 328 | { | |
| 329 | size: "sm", | |
| 330 | icon: o.state === "open" ? "issueClosed" : "issue", | |
| 331 | form: "thread-state", | |
| 332 | }, | |
| 333 | ) | |
| 328 | ? btn(o.state === "open" ? o.closeLabel : "Reopen", { | |
| 329 | size: "sm", | |
| 330 | icon: o.state === "open" ? "issueClosed" : "issue", | |
| 331 | form: "thread-state", | |
| 332 | }) | |
| 334 | 333 | : null; |
| 335 | 334 | return html`${stateForm} |
| 336 | 335 | <form class="card comment-form" method="post" action="${o.action}"> |
| @@ -580,10 +579,16 @@ export async function stateSubmit( | ||
| 580 | 579 | wanted, |
| 581 | 580 | ); |
| 582 | 581 | if (!changed.ok) { |
| 582 | // The same noun the redirect target is about to render with | |
| 583 | // (`studioRecord`), so a flash never says dev's word on a studio page. | |
| 584 | const merged = | |
| 585 | studioRecord(ctx, found.repo) !== undefined | |
| 586 | ? "A merged change cannot be reopened." | |
| 587 | : "A merged pull request cannot be reopened."; | |
| 583 | 588 | const message = |
| 584 | 589 | originError(changed.error) ?? |
| 585 | 590 | (changed.error === "already_merged" |
| 586 | ? "A merged pull request cannot be reopened." | |
| 591 | ? merged | |
| 587 | 592 | : changed.error === "forbidden" |
| 588 | 593 | ? "Only the author or someone with write access can do that." |
| 589 | 594 | : "That could not be changed."); |
src/web/studio/change.ts +1 −1
| @@ -179,7 +179,7 @@ export async function studioPull( | ||
| 179 | 179 | stateAction: `${thread}/state`, |
| 180 | 180 | state: pull.state, |
| 181 | 181 | locked: pull.mergedAt !== null, |
| 182 | kind: "pull", | |
| 182 | closeLabel: "Close change", | |
| 183 | 183 | })} |
| 184 | 184 | </div> |
| 185 | 185 | <aside>${previewCard(ctx, repo, pull)}</aside> |
src/web/studio/tell.ts +1 −1
| @@ -279,7 +279,7 @@ export async function studioTell( | ||
| 279 | 279 | action: `${thread}/comments`, |
| 280 | 280 | stateAction: `${thread}/state`, |
| 281 | 281 | state: issue.state, |
| 282 | kind: "issue", | |
| 282 | closeLabel: "Close tell", | |
| 283 | 283 | })} |
| 284 | 284 | </div> |
| 285 | 285 | <aside> |
test/studio-words.test.ts +45 −4
| @@ -78,6 +78,18 @@ const DEV_WORDS: readonly { pattern: RegExp; because: string }[] = [ | ||
| 78 | 78 | pattern: /\bmerge\b/i, |
| 79 | 79 | because: "the studio's own word for a merge is shipped", |
| 80 | 80 | }, |
| 81 | { | |
| 82 | pattern: /pull request/i, | |
| 83 | because: "the studio's own word for a pull is change", | |
| 84 | }, | |
| 85 | { | |
| 86 | // Bare "issue" also rides the topbar's own live-event wiring | |
| 87 | // (`data-live="run.*,pull.*,issue.*,check.*"`) on every page, which is | |
| 88 | // not a word a person reads — so only the composer's own button text is | |
| 89 | // checked here, the shape #171 actually leaked. | |
| 90 | pattern: /Close issue\b/, | |
| 91 | because: "the studio's own word for a tell is tell", | |
| 92 | }, | |
| 81 | 93 | ]; |
| 82 | 94 | |
| 83 | 95 | /** |
| @@ -447,7 +459,10 @@ describe("dev's vocabulary never reaches a studio page", () => { | ||
| 447 | 459 | { |
| 448 | 460 | word: "tell", |
| 449 | 461 | path: `${repoPath}/issues/${tellNumber}`, |
| 450 | dev: ["<h2>Labels</h2>"], | |
| 462 | // #171: the composer's close button used to say "Close issue" here, | |
| 463 | // dev's own noun on the studio's own page. | |
| 464 | shows: ["Close tell"], | |
| 465 | dev: ["<h2>Labels</h2>", "Close issue"], | |
| 451 | 466 | }, |
| 452 | 467 | // The studio's own nouns for the same two pages (#158): one handler |
| 453 | 468 | // each, so what is walked here is the very page above under the |
| @@ -457,18 +472,21 @@ describe("dev's vocabulary never reaches a studio page", () => { | ||
| 457 | 472 | { |
| 458 | 473 | word: "tell (the studio's noun)", |
| 459 | 474 | path: `${repoPath}/tells/${tellNumber}`, |
| 460 | shows: [tellTitle], | |
| 475 | shows: [tellTitle, "Close tell"], | |
| 461 | 476 | }, |
| 462 | 477 | { word: "working", path: `${repoPath}/runs/${runId}` }, |
| 463 | 478 | { |
| 464 | 479 | word: "look", |
| 465 | 480 | path: `${repoPath}/pulls/${openPullNumber}`, |
| 466 | dev: ["diff-table", openBranch], | |
| 481 | // #171: the composer's close button used to say "Close pull | |
| 482 | // request" here, dev's own noun on the studio's own page. | |
| 483 | shows: ["Close change"], | |
| 484 | dev: ["diff-table", openBranch, "Close pull request"], | |
| 467 | 485 | }, |
| 468 | 486 | { |
| 469 | 487 | word: "look (the studio's noun)", |
| 470 | 488 | path: `${repoPath}/changes/${openPullNumber}`, |
| 471 | shows: [openPullTitle], | |
| 489 | shows: [openPullTitle, "Close change"], | |
| 472 | 490 | }, |
| 473 | 491 | { |
| 474 | 492 | word: "shipped", |
| @@ -681,4 +699,27 @@ describe("dev's vocabulary never reaches a studio page", () => { | ||
| 681 | 699 | // proving itself once it exists. |
| 682 | 700 | expect(proved).toBe(pages.filter((p) => p.dev !== undefined).length); |
| 683 | 701 | }); |
| 702 | ||
| 703 | test("reopening a merged change refuses in the reader's own noun — dev's word never rides the flash on a studio page (#171)", async () => { | |
| 704 | const reopen = (mode: "studio" | "dev") => | |
| 705 | fetch(`${base}/alice/vocab/pulls/${mergedPullNumber}/state`, { | |
| 706 | method: "POST", | |
| 707 | headers: { | |
| 708 | cookie: `${cookie}; forge_mode=${mode}`, | |
| 709 | origin: base, | |
| 710 | accept: "application/json", | |
| 711 | }, | |
| 712 | body: new URLSearchParams({ state: "open" }), | |
| 713 | }); | |
| 714 | const studio = await reopen("studio"); | |
| 715 | expect(studio.status).toBe(422); | |
| 716 | expect(((await studio.json()) as { error: string }).error).toBe( | |
| 717 | "A merged change cannot be reopened.", | |
| 718 | ); | |
| 719 | const dev = await reopen("dev"); | |
| 720 | expect(dev.status).toBe(422); | |
| 721 | expect(((await dev.json()) as { error: string }).error).toBe( | |
| 722 | "A merged pull request cannot be reopened.", | |
| 723 | ); | |
| 724 | }); | |
| 684 | 725 | }); |
new above
offline