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

Merged opened this

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

Merged

scout/forge:scout/4398-a-pull-page-s-merge-box-draws-5-s-card-r → main 1 commit fork

  • Merged. scout/4398-a-pull-page-s-merge-box-draws-5-s-card-r was merged into main as e5dfc69.

Review

Sign in to review this change.
2 files changed +227 −131
src/web/pages/pull.ts +110 −131
@@ -86,8 +86,10 @@ import {
8686 icon,
8787 labelCss,
8888 queuedPhrase,
89 row,
8990 sha,
9091 time,
92 type Tone,
9193 } from "../ui.ts";
9294 import {
9395 assigneeCard,
@@ -373,6 +375,28 @@ function pullChecksLine(
373375 return html`${line}${found === null ? "" : specFindings(found)}`;
374376 }
375377
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
376400 /**
377401 * Six states, six cards. Every one names the situation, gives the reason,
378402 * and offers exactly the actions that are actually available.
@@ -399,37 +423,26 @@ function mergeBox(
399423 </form>`;
400424
401425 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,
415436 );
416437 }
417438
418439 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),
433446 );
434447 }
435448
@@ -439,31 +452,23 @@ function mergeBox(
439452 // required checks exactly. So the box says where the button is instead.
440453 if (repo.origin !== null || pull.originHeadRepo !== null) {
441454 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 ),
467472 );
468473 }
469474
@@ -483,14 +488,12 @@ function mergeBox(
483488 ? html` <a href="${thread}/resolve">Resolve</a> it here.`
484489 : ""}`
485490 : 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,
494497 );
495498 }
496499
@@ -509,26 +512,19 @@ function mergeBox(
509512 : html`This repository requires an approving review of the commit that
510513 is head right now. A verdict passed before the latest push does not
511514 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 ),
532528 );
533529 }
534530
@@ -552,60 +548,43 @@ function mergeBox(
552548 or push again.`
553549 : html`There is no deploy of this head yet. Nothing merges until one
554550 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 ),
575564 );
576565 }
577566
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>`,
609588 );
610589 }
611590
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 });

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs