Public: anyone can read this; the crew can change it.
A pull page's merge box draws §5's card, row and btn, and the merge-* classes leave src/web/pages/pull.ts #4422
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/4398-a-pull-page-s-merge-box-draws-5-s-card-r → main 1 commit fork
2 files
changed +227
−131
src/web/pages/pull.ts +110 −131
| @@ -86,8 +86,10 @@ import { | ||
| 86 | 86 | icon, |
| 87 | 87 | labelCss, |
| 88 | 88 | queuedPhrase, |
| 89 | row, | |
| 89 | 90 | sha, |
| 90 | 91 | time, |
| 92 | type Tone, | |
| 91 | 93 | } from "../ui.ts"; |
| 92 | 94 | import { |
| 93 | 95 | assigneeCard, |
| @@ -373,6 +375,28 @@ function pullChecksLine( | ||
| 373 | 375 | return html`${line}${found === null ? "" : specFindings(found)}`; |
| 374 | 376 | } |
| 375 | 377 | |
| 378 | /** One §5 row in a toned card; `.setting-list` wraps the sentence and drops the action under it on a phone. */ | |
| 379 | function box( | |
| 380 | tone: Tone | undefined, | |
| 381 | lead: Html, | |
| 382 | /** A fixed phrase of ours, never a person's text. */ | |
| 383 | title: Html | string, | |
| 384 | meta: Html, | |
| 385 | trail: Html | null, | |
| 386 | ): Html { | |
| 387 | return card( | |
| 388 | html`<ul class="list setting-list"> | |
| 389 | ${row({ | |
| 390 | lead, | |
| 391 | title: typeof title === "string" ? raw(title) : title, | |
| 392 | meta, | |
| 393 | ...(trail ? { trail } : {}), | |
| 394 | })} | |
| 395 | </ul>`, | |
| 396 | { ...(tone ? { tone } : {}), pad: false, extra: "mt" }, | |
| 397 | ); | |
| 398 | } | |
| 399 | ||
| 376 | 400 | /** |
| 377 | 401 | * Six states, six cards. Every one names the situation, gives the reason, |
| 378 | 402 | * and offers exactly the actions that are actually available. |
| @@ -399,37 +423,26 @@ function mergeBox( | ||
| 399 | 423 | </form>`; |
| 400 | 424 | |
| 401 | 425 | if (pull.mergedAt !== null) { |
| 402 | return card( | |
| 403 | html`${icon("merge")} | |
| 404 | <div class="grow"> | |
| 405 | <div class="merge-headline">Merged.</div> | |
| 406 | <p class="merge-sub"> | |
| 407 | <span class="mono">${pull.headBranch}</span> was merged into | |
| 408 | <span class="mono">${pull.baseBranch}</span>${when( | |
| 409 | pull.mergedOid, | |
| 410 | (oid) => html` as ${sha(oid, `${repoBase(repo)}/commit/${oid}`)}`, | |
| 411 | )}. | |
| 412 | </p> | |
| 413 | </div>`, | |
| 414 | { tone: "merged", extra: "merge-box" }, | |
| 426 | return box( | |
| 427 | "merged", | |
| 428 | icon("merge"), | |
| 429 | "Merged.", | |
| 430 | html`<span class="mono">${pull.headBranch}</span> was merged into | |
| 431 | <span class="mono">${pull.baseBranch}</span>${when( | |
| 432 | pull.mergedOid, | |
| 433 | (oid) => html` as ${sha(oid, `${repoBase(repo)}/commit/${oid}`)}`, | |
| 434 | )}.`, | |
| 435 | null, | |
| 415 | 436 | ); |
| 416 | 437 | } |
| 417 | 438 | |
| 418 | 439 | if (pull.state === "closed") { |
| 419 | return card( | |
| 420 | html`${icon("x")} | |
| 421 | <div class="grow"> | |
| 422 | <div class="merge-headline">Closed without merging.</div> | |
| 423 | <p class="merge-sub"> | |
| 424 | Nothing was applied to | |
| 425 | <span class="mono">${pull.baseBranch}</span>. | |
| 426 | </p> | |
| 427 | </div> | |
| 428 | ${when( | |
| 429 | ctx.auth.user !== undefined, | |
| 430 | () => html`<span class="merge-form">${reopen}</span>`, | |
| 431 | )}`, | |
| 432 | { extra: "merge-box" }, | |
| 440 | return box( | |
| 441 | undefined, | |
| 442 | icon("x"), | |
| 443 | "Closed without merging.", | |
| 444 | html`Nothing was applied to <span class="mono">${pull.baseBranch}</span>.`, | |
| 445 | when(ctx.auth.user !== undefined, () => reopen), | |
| 433 | 446 | ); |
| 434 | 447 | } |
| 435 | 448 | |
| @@ -439,31 +452,23 @@ function mergeBox( | ||
| 439 | 452 | // required checks exactly. So the box says where the button is instead. |
| 440 | 453 | if (repo.origin !== null || pull.originHeadRepo !== null) { |
| 441 | 454 | const at = repo.origin; |
| 442 | return card( | |
| 443 | html`${icon(at?.kind ?? "globe")} | |
| 444 | <div class="grow"> | |
| 445 | <div class="merge-headline">Merge on the origin.</div> | |
| 446 | <p class="merge-sub"> | |
| 447 | This repository mirrors | |
| 448 | ${at === null || at.fullName === "" | |
| 449 | ? html`another forge` | |
| 450 | : html`<span class="mono">${at.fullName}</span>`}, | |
| 451 | which is truth for it. Merge there and the next sync brings back the | |
| 452 | merge commit it actually made. | |
| 453 | </p> | |
| 454 | </div> | |
| 455 | ${when( | |
| 456 | pull.originUrl, | |
| 457 | (url) => | |
| 458 | html`<span class="merge-form" | |
| 459 | >${btn("Open on the origin", { | |
| 460 | href: url, | |
| 461 | variant: "primary", | |
| 462 | icon: "external", | |
| 463 | })}</span | |
| 464 | >`, | |
| 465 | )}`, | |
| 466 | { tone: "attn", extra: "merge-box" }, | |
| 455 | return box( | |
| 456 | "attn", | |
| 457 | icon(at?.kind ?? "globe"), | |
| 458 | "Merge on the origin.", | |
| 459 | html`This repository mirrors | |
| 460 | ${at === null || at.fullName === "" | |
| 461 | ? html`another forge` | |
| 462 | : html`<span class="mono">${at.fullName}</span>`}, | |
| 463 | which is truth for it. Merge there and the next sync brings back the | |
| 464 | merge commit it actually made.`, | |
| 465 | when(pull.originUrl, (url) => | |
| 466 | btn("Open on the origin", { | |
| 467 | href: url, | |
| 468 | variant: "primary", | |
| 469 | icon: "external", | |
| 470 | }), | |
| 471 | ), | |
| 467 | 472 | ); |
| 468 | 473 | } |
| 469 | 474 | |
| @@ -483,14 +488,12 @@ function mergeBox( | ||
| 483 | 488 | ? html` <a href="${thread}/resolve">Resolve</a> it here.` |
| 484 | 489 | : ""}` |
| 485 | 490 | : html`This branch cannot be applied automatically.`; |
| 486 | return card( | |
| 487 | html`${icon("alert")} | |
| 488 | <div class="grow"> | |
| 489 | <div class="merge-headline">Can't merge.</div> | |
| 490 | <p class="merge-sub">${reason}</p> | |
| 491 | ${reviewsChip(status.reviews, by)} ${checks} | |
| 492 | </div>`, | |
| 493 | { tone: "danger", extra: "merge-box" }, | |
| 491 | return box( | |
| 492 | "danger", | |
| 493 | icon("alert"), | |
| 494 | "Can't merge.", | |
| 495 | html`${reason} ${reviewsChip(status.reviews, by)} ${checks}`, | |
| 496 | null, | |
| 494 | 497 | ); |
| 495 | 498 | } |
| 496 | 499 | |
| @@ -509,26 +512,19 @@ function mergeBox( | ||
| 509 | 512 | : html`This repository requires an approving review of the commit that |
| 510 | 513 | is head right now. A verdict passed before the latest push does not |
| 511 | 514 | count.`; |
| 512 | return card( | |
| 513 | html`${icon("eye")} | |
| 514 | <div class="grow"> | |
| 515 | <div class="merge-headline">Waiting on review.</div> | |
| 516 | <p class="merge-sub">${why}</p> | |
| 517 | ${reviewsChip(status.reviews, by)} ${checks} | |
| 518 | </div> | |
| 519 | ${when( | |
| 520 | mayMerge, | |
| 521 | () => | |
| 522 | html`<span class="merge-form" | |
| 523 | >${btn("Merge pull request", { | |
| 524 | variant: "primary", | |
| 525 | icon: "merge", | |
| 526 | disabled: true, | |
| 527 | title: "An approving review of this head is required first", | |
| 528 | })}</span | |
| 529 | >`, | |
| 530 | )}`, | |
| 531 | { tone: "attn", extra: "merge-box" }, | |
| 515 | return box( | |
| 516 | "attn", | |
| 517 | icon("eye"), | |
| 518 | "Waiting on review.", | |
| 519 | html`${why} ${reviewsChip(status.reviews, by)} ${checks}`, | |
| 520 | when(mayMerge, () => | |
| 521 | btn("Merge pull request", { | |
| 522 | variant: "primary", | |
| 523 | icon: "merge", | |
| 524 | disabled: true, | |
| 525 | title: "An approving review of this head is required first", | |
| 526 | }), | |
| 527 | ), | |
| 532 | 528 | ); |
| 533 | 529 | } |
| 534 | 530 | |
| @@ -552,60 +548,43 @@ function mergeBox( | ||
| 552 | 548 | or push again.` |
| 553 | 549 | : html`There is no deploy of this head yet. Nothing merges until one |
| 554 | 550 | passes.`; |
| 555 | return card( | |
| 556 | html`${icon("loader")} | |
| 557 | <div class="grow"> | |
| 558 | <div class="merge-headline">Waiting on checks.</div> | |
| 559 | <p class="merge-sub">${why}</p> | |
| 560 | ${reviewsChip(status.reviews, by)} ${checks} | |
| 561 | </div> | |
| 562 | ${when( | |
| 563 | mayMerge, | |
| 564 | () => | |
| 565 | html`<span class="merge-form" | |
| 566 | >${btn("Merge pull request", { | |
| 567 | variant: "primary", | |
| 568 | icon: "merge", | |
| 569 | disabled: true, | |
| 570 | title: "A passing deploy of this head is required first", | |
| 571 | })}</span | |
| 572 | >`, | |
| 573 | )}`, | |
| 574 | { tone: "attn", extra: "merge-box" }, | |
| 551 | return box( | |
| 552 | "attn", | |
| 553 | icon("loader"), | |
| 554 | "Waiting on checks.", | |
| 555 | html`${why} ${reviewsChip(status.reviews, by)} ${checks}`, | |
| 556 | when(mayMerge, () => | |
| 557 | btn("Merge pull request", { | |
| 558 | variant: "primary", | |
| 559 | icon: "merge", | |
| 560 | disabled: true, | |
| 561 | title: "A passing deploy of this head is required first", | |
| 562 | }), | |
| 563 | ), | |
| 575 | 564 | ); |
| 576 | 565 | } |
| 577 | 566 | |
| 578 | return card( | |
| 579 | html`${icon("check")} | |
| 580 | <div class="grow"> | |
| 581 | <div class="merge-headline"> | |
| 582 | Ready to merge — no conflicts with | |
| 583 | <span class="mono">${pull.baseBranch}</span>. | |
| 584 | </div> | |
| 585 | <p class="merge-sub"> | |
| 586 | ${status.ahead} commit${status.ahead === 1 ? "" : "s"} will land on | |
| 587 | <span class="mono">${pull.baseBranch}</span>. | |
| 588 | </p> | |
| 589 | ${reviewsChip(status.reviews, by)} ${checks} | |
| 590 | </div> | |
| 591 | ${mayMerge | |
| 592 | ? html`<span class="merge-form"> | |
| 593 | <form method="post" action="${thread}/merge" class="merge-form"> | |
| 594 | <label class="sr-only" for="strategy">Merge strategy</label> | |
| 595 | <select class="input" id="strategy" name="strategy"> | |
| 596 | <option value="merge">Merge commit</option> | |
| 597 | <option value="squash">Squash and merge</option> | |
| 598 | </select> | |
| 599 | ${btn("Merge pull request", { | |
| 600 | variant: "primary", | |
| 601 | icon: "merge", | |
| 602 | })} | |
| 603 | </form> | |
| 604 | </span>` | |
| 605 | : html`<span class="muted small" | |
| 606 | >You need write access to merge.</span | |
| 607 | >`}`, | |
| 608 | { tone: "success", extra: "merge-box" }, | |
| 567 | return box( | |
| 568 | "success", | |
| 569 | icon("check"), | |
| 570 | html`Ready to merge — no conflicts with | |
| 571 | <span class="mono">${pull.baseBranch}</span>.`, | |
| 572 | html`${status.ahead} commit${status.ahead === 1 ? "" : "s"} will land on | |
| 573 | <span class="mono">${pull.baseBranch}</span>. ${reviewsChip(status.reviews, by)} | |
| 574 | ${checks}`, | |
| 575 | mayMerge | |
| 576 | ? html`<form method="post" action="${thread}/merge" class="row-fields"> | |
| 577 | <label class="sr-only" for="strategy">Merge strategy</label> | |
| 578 | <select class="input" id="strategy" name="strategy"> | |
| 579 | <option value="merge">Merge commit</option> | |
| 580 | <option value="squash">Squash and merge</option> | |
| 581 | </select> | |
| 582 | ${btn("Merge pull request", { | |
| 583 | variant: "primary", | |
| 584 | icon: "merge", | |
| 585 | })} | |
| 586 | </form>` | |
| 587 | : html`<span class="muted small">You need write access to merge.</span>`, | |
| 609 | 588 | ); |
| 610 | 589 | } |
| 611 | 590 |
test/pull-merge-box.test.ts added +117
| @@ -0,0 +1,117 @@ | ||
| 1 | /** | |
| 2 | * The pull page's merge box is §5's card and row: no merge-* class, the same | |
| 3 | * sentences, and the Merge form posting the same fields to the same action. | |
| 4 | */ | |
| 5 | import { afterAll, beforeAll, expect, test } from "bun:test"; | |
| 6 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 7 | import { tmpdir } from "node:os"; | |
| 8 | import { join } from "node:path"; | |
| 9 | import { createApp, type App } from "../src/app.ts"; | |
| 10 | import { mergePull, openPull } from "../src/actions/pulls.ts"; | |
| 11 | import { commitToBranch, createRepo } from "../src/actions/repos.ts"; | |
| 12 | import { createSession } from "../src/core/sessions.ts"; | |
| 13 | import { createUser, type User } from "../src/core/users.ts"; | |
| 14 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 15 | import { quietLogs } from "./helpers/globals.ts"; | |
| 16 | ||
| 17 | quietLogs(); | |
| 18 | ||
| 19 | let dataDir: string; | |
| 20 | let app: App; | |
| 21 | let servers: Servers; | |
| 22 | let alice: User; | |
| 23 | const numbers: Record<string, number> = {}; | |
| 24 | const cells = process.env["FORGE_CELLS"]; | |
| 25 | ||
| 26 | const boxOf = async (repo: string): Promise<string> => { | |
| 27 | const res = await fetch( | |
| 28 | `${servers.url}/alice/${repo}/pulls/${numbers[repo]}/files`, | |
| 29 | { headers: { cookie: `forge_session=${createSession(app.db, alice.id).token}` } }, | |
| 30 | ); | |
| 31 | const body = await res.text(); | |
| 32 | for (const name of ["merge-box", "merge-form", "merge-headline", "merge-sub"]) | |
| 33 | expect(body).not.toContain(name); | |
| 34 | const at = body.search(/<div\s+class="card mt"[^>]*data-tone=/); | |
| 35 | expect(at).toBeGreaterThan(-1); | |
| 36 | return body.slice(at, body.indexOf("</ul>", at)).replace(/\s+/g, " "); | |
| 37 | }; | |
| 38 | ||
| 39 | beforeAll(async () => { | |
| 40 | process.env["FORGE_CELLS"] = "off"; | |
| 41 | dataDir = await mkdtemp(join(tmpdir(), "forge-merge-box-")); | |
| 42 | app = createApp({ dataDir, port: 0 }); | |
| 43 | servers = await serve(app); | |
| 44 | const made = await createUser(app.db, { | |
| 45 | origin: "seed", | |
| 46 | trusted: true, | |
| 47 | username: "alice", | |
| 48 | email: "[email protected]", | |
| 49 | password: "a-good-passphrase", | |
| 50 | adminIfFirst: true, | |
| 51 | }); | |
| 52 | if (!made.ok) throw new Error(String(made.error)); | |
| 53 | alice = made.value; | |
| 54 | for (const name of ["ready", "checks", "merged"]) { | |
| 55 | const repo = await createRepo(app, alice, { name, visibility: "public" }); | |
| 56 | if (!repo.ok) throw new Error(String(repo.error)); | |
| 57 | for (const [branch, file] of [[undefined, "forge.json"], ["topic", "t.txt"]] as const) { | |
| 58 | const done = await commitToBranch(app, alice, repo.value, { | |
| 59 | ...(branch === undefined ? {} : { branch }), | |
| 60 | message: "change", | |
| 61 | files: [{ path: file, content: file === "forge.json" ? "{}" : "x" }], | |
| 62 | }); | |
| 63 | if (!done.ok) throw new Error(JSON.stringify(done.error)); | |
| 64 | } | |
| 65 | const pull = await openPull(app, alice, repo.value, { | |
| 66 | title: name, | |
| 67 | body: "", | |
| 68 | baseBranch: "main", | |
| 69 | headBranch: "topic", | |
| 70 | }); | |
| 71 | if (!pull.ok) throw new Error(JSON.stringify(pull.error)); | |
| 72 | numbers[name] = pull.value.number; | |
| 73 | if (name === "checks") | |
| 74 | app.db.run("UPDATE repo SET require_checks = 1 WHERE id = ?", [repo.value.id]); | |
| 75 | if (name === "merged") { | |
| 76 | const done = await mergePull(app, alice, repo.value, pull.value, "merge"); | |
| 77 | if (!done.ok) throw new Error(JSON.stringify(done.error)); | |
| 78 | } | |
| 79 | } | |
| 80 | }); | |
| 81 | ||
| 82 | afterAll(async () => { | |
| 83 | await servers.stop(); | |
| 84 | app.close(); | |
| 85 | if (cells === undefined) delete process.env["FORGE_CELLS"]; | |
| 86 | else process.env["FORGE_CELLS"] = cells; | |
| 87 | await rm(dataDir, { recursive: true, force: true }); | |
| 88 | }); | |
| 89 | ||
| 90 | test("ready: a toned card and row holding the Merge form's action and fields", async () => { | |
| 91 | const box = await boxOf("ready"); | |
| 92 | expect(box).toContain('data-tone="success"'); | |
| 93 | expect(box).toContain('<ul class="list setting-list">'); | |
| 94 | expect(box).toContain("Ready to merge — no conflicts with"); | |
| 95 | expect(box).toContain(`action="/alice/ready/pulls/${numbers["ready"]}/merge"`); | |
| 96 | expect(box).toContain('method="post"'); | |
| 97 | expect(box).toContain('name="strategy"'); | |
| 98 | expect(box).toContain('<option value="merge">'); | |
| 99 | expect(box).toContain('<option value="squash">'); | |
| 100 | expect(box).toContain("Merge pull request"); | |
| 101 | }); | |
| 102 | ||
| 103 | test("waiting on checks: the sentence stays and the Merge button is disabled", async () => { | |
| 104 | const box = await boxOf("checks"); | |
| 105 | expect(box).toContain('data-tone="attn"'); | |
| 106 | expect(box).toContain("Waiting on checks."); | |
| 107 | expect(box).toContain("There is no deploy of this head yet."); | |
| 108 | expect(box).toContain("disabled"); | |
| 109 | expect(box).not.toContain("/pulls/" + numbers["checks"] + "/merge"); | |
| 110 | }); | |
| 111 | ||
| 112 | test("merged: the merged tone and the sentence", async () => { | |
| 113 | const box = await boxOf("merged"); | |
| 114 | expect(box).toContain('data-tone="merged"'); | |
| 115 | expect(box).toContain("Merged."); | |
| 116 | expect(box).toContain("was merged into"); | |
| 117 | }); |
new above
offline