Public: anyone can read this; the crew can change it.
A comment's refusals live in one module, read by issues, pulls and the forge toolbar #5603
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/5588-a-comment-s-refusals-live-in-one-module → main 1 commit fork
5 files
changed +44
−6
src/actions/comment_words.ts added +10
| @@ -0,0 +1,10 @@ | ||
| 1 | /** | |
| 2 | * What a comment's refusal says, wherever it is said: the issue page, the | |
| 3 | * pull page and the forge toolbar read these so the words cannot drift. | |
| 4 | */ | |
| 5 | ||
| 6 | /** A comment with nothing in it. */ | |
| 7 | export const COMMENT_EMPTY = "A comment needs something in it."; | |
| 8 | ||
| 9 | /** A comment the forge would not post, when the origin gave no reason. */ | |
| 10 | export const COMMENT_NOT_POSTED = "That comment could not be posted."; |
src/web/pages/issues.ts +3 −2
| @@ -13,6 +13,7 @@ import { | ||
| 13 | 13 | import type { Issue } from "../../core/issues.ts"; |
| 14 | 14 | import { canRead, type Repo } from "../../core/repos.ts"; |
| 15 | 15 | import { privateRepoForSealed } from "../../actions/repos.ts"; |
| 16 | import { COMMENT_EMPTY, COMMENT_NOT_POSTED } from "../../actions/comment_words.ts"; | |
| 16 | 17 | import { notePhase, takeSessionTime, timePhase } from "../../core/route_timing.ts"; |
| 17 | 18 | import type { Ctx } from "../../http/context.ts"; |
| 18 | 19 | import { |
| @@ -287,7 +288,7 @@ export async function issueCommentSubmit(ctx: Ctx): Promise<Response> { | ||
| 287 | 288 | if (body.trim() === "") |
| 288 | 289 | return flashTo(ctx, dest, { |
| 289 | 290 | tone: "danger", |
| 290 | message: "A comment needs something in it.", | |
| 291 | message: COMMENT_EMPTY, | |
| 291 | 292 | }); |
| 292 | 293 | const added = await addCommentThrough( |
| 293 | 294 | ctx.app, |
| @@ -299,7 +300,7 @@ export async function issueCommentSubmit(ctx: Ctx): Promise<Response> { | ||
| 299 | 300 | if (!added.ok) |
| 300 | 301 | return flashTo(ctx, dest, { |
| 301 | 302 | tone: "danger", |
| 302 | message: originError(added.error) ?? "That comment could not be posted.", | |
| 303 | message: originError(added.error) ?? COMMENT_NOT_POSTED, | |
| 303 | 304 | }); |
| 304 | 305 | return back === undefined |
| 305 | 306 | ? flashTo(ctx, `${dest}#conversation`, { message: "Comment posted." }) |
src/web/pages/pull.ts +3 −2
| @@ -10,6 +10,7 @@ | ||
| 10 | 10 | |
| 11 | 11 | import { askCheck } from "../../checks/again.ts"; |
| 12 | 12 | import { addCommentThrough } from "../../actions/issues.ts"; |
| 13 | import { COMMENT_EMPTY, COMMENT_NOT_POSTED } from "../../actions/comment_words.ts"; | |
| 13 | 14 | import type { AccessError } from "../../actions/access.ts"; |
| 14 | 15 | import { |
| 15 | 16 | addReview, |
| @@ -991,7 +992,7 @@ export async function pullCommentSubmit(ctx: Ctx): Promise<Response> { | ||
| 991 | 992 | if (bodyText.trim() === "") |
| 992 | 993 | return flashTo(ctx, dest, { |
| 993 | 994 | tone: "danger", |
| 994 | message: "A comment needs something in it.", | |
| 995 | message: COMMENT_EMPTY, | |
| 995 | 996 | }); |
| 996 | 997 | const added = await addCommentThrough( |
| 997 | 998 | ctx.app, |
| @@ -1003,7 +1004,7 @@ export async function pullCommentSubmit(ctx: Ctx): Promise<Response> { | ||
| 1003 | 1004 | if (!added.ok) |
| 1004 | 1005 | return flashTo(ctx, dest, { |
| 1005 | 1006 | tone: "danger", |
| 1006 | message: originError(added.error) ?? "That comment could not be posted.", | |
| 1007 | message: originError(added.error) ?? COMMENT_NOT_POSTED, | |
| 1007 | 1008 | }); |
| 1008 | 1009 | return flashTo(ctx, `${dest}`, { message: "Comment posted." }); |
| 1009 | 1010 | } |
src/web/studio/toolbar.ts +3 −2
| @@ -16,6 +16,7 @@ | ||
| 16 | 16 | */ |
| 17 | 17 | |
| 18 | 18 | import { addCommentThrough, openIssueThrough } from "../../actions/issues.ts"; |
| 19 | import { COMMENT_EMPTY, COMMENT_NOT_POSTED } from "../../actions/comment_words.ts"; | |
| 19 | 20 | import { reportTitle } from "../../actions/page-report.ts"; |
| 20 | 21 | import { getAppByRepo } from "../../core/apps.ts"; |
| 21 | 22 | import { getIssue, MAX_BODY } from "../../core/issues.ts"; |
| @@ -273,7 +274,7 @@ export async function toolbarComment(ctx: Ctx): Promise<Response> { | ||
| 273 | 274 | }, |
| 274 | 275 | status, |
| 275 | 276 | ); |
| 276 | if (text === "") return refuse("A comment needs something in it."); | |
| 277 | if (text === "") return refuse(COMMENT_EMPTY); | |
| 277 | 278 | const said = address === "" ? text : `${text}\n\nPage: <${address}>`; |
| 278 | 279 | if (said.length > MAX_BODY) return refuse("That is too long to send in one go."); |
| 279 | 280 | const target = pull ? `pull #${pull.number}` : "an issue on this app"; |
| @@ -302,7 +303,7 @@ export async function toolbarComment(ctx: Ctx): Promise<Response> { | ||
| 302 | 303 | if (pull) { |
| 303 | 304 | const added = await addCommentThrough(ctx.app, user, repo, pull, said); |
| 304 | 305 | if (!added.ok) |
| 305 | return refuse(originError(added.error) ?? "That comment could not be posted.", 403); | |
| 306 | return refuse(originError(added.error) ?? COMMENT_NOT_POSTED, 403); | |
| 306 | 307 | return receipt(`${base}/pulls/${pull.number}`, "Comment posted"); |
| 307 | 308 | } |
| 308 | 309 | // The `…Through` twin, as every door that opens an issue: on a mirror the |
test/comment-words.test.ts added +25
| @@ -0,0 +1,25 @@ | ||
| 1 | /** | |
| 2 | * A comment's refusals are said from one place (#5588): the module owns | |
| 3 | * each sentence and no page types its own copy. | |
| 4 | */ | |
| 5 | ||
| 6 | import { expect, test } from "bun:test"; | |
| 7 | import { readFileSync } from "node:fs"; | |
| 8 | import { join } from "node:path"; | |
| 9 | import * as words from "../src/actions/comment_words.ts"; | |
| 10 | ||
| 11 | const sentences = Object.values(words); | |
| 12 | const readers = ["src/web/pages/issues.ts", "src/web/pages/pull.ts", "src/web/studio/toolbar.ts"]; | |
| 13 | ||
| 14 | test("each sentence is non-empty", () => { | |
| 15 | expect(sentences).toEqual(["A comment needs something in it.", "That comment could not be posted."]); | |
| 16 | for (const s of sentences) expect(s.trim().length).toBeGreaterThan(0); | |
| 17 | }); | |
| 18 | ||
| 19 | test("no reader types a copy of a sentence", () => { | |
| 20 | for (const file of readers) { | |
| 21 | const source = readFileSync(join(import.meta.dir, "..", file), "utf8"); | |
| 22 | expect(source).toContain("actions/comment_words.ts"); | |
| 23 | for (const s of sentences) expect(source.includes(s)).toBe(false); | |
| 24 | } | |
| 25 | }); |
new above
offline