Public: anyone can read this; the crew can change it.
Write-access and gone-pull refusals live in one module, read by conflicts, edit and the pull page #5598
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/5590-write-access-and-gone-pull-refusals-live → main 1 commit fork
5 files
changed +59
−6
src/actions/finish-conflicts.ts +3 −2
| @@ -19,6 +19,7 @@ import { repoDir } from "../git/repos.ts"; | ||
| 19 | 19 | import { commitFiles } from "../git/write.ts"; |
| 20 | 20 | import { err, ok, type Result } from "../kernel/result.ts"; |
| 21 | 21 | import { requireWriter, type AccessError } from "./access.ts"; |
| 22 | import { NO_WRITE_ACCESS, PULL_GONE } from "./pull_words.ts"; | |
| 22 | 23 | |
| 23 | 24 | export type FinishError = |
| 24 | 25 | | AccessError |
| @@ -38,8 +39,8 @@ export type FinishError = | ||
| 38 | 39 | /** What each refusal says, on the page and in the terminal alike. */ |
| 39 | 40 | export const FINISH_REFUSALS: Record<FinishError, string> = { |
| 40 | 41 | unauthorized: "Sign in to finish a conflict.", |
| 41 | forbidden: "You do not have write access to this repository.", | |
| 42 | not_found: "That pull request is no longer here.", | |
| 42 | forbidden: NO_WRITE_ACCESS, | |
| 43 | not_found: PULL_GONE, | |
| 43 | 44 | closed: "This pull request is not open. Nothing was written.", |
| 44 | 45 | elsewhere: "This pull's branch lives elsewhere; resolve it there. Nothing was written.", |
| 45 | 46 | stale: |
src/actions/pull_words.ts added +11
| @@ -0,0 +1,11 @@ | ||
| 1 | /** | |
| 2 | * The refusals said wherever a pull is acted on — finishing a conflict, | |
| 3 | * editing a file, merging or approving on the pull page — written once so | |
| 4 | * every place says them the same way. | |
| 5 | */ | |
| 6 | ||
| 7 | /** Said to someone who may read the repository but not write it. */ | |
| 8 | export const NO_WRITE_ACCESS = "You do not have write access to this repository."; | |
| 9 | ||
| 10 | /** Said when the pull acted on was not found. */ | |
| 11 | export const PULL_GONE = "That pull request is no longer here."; |
src/web/pages/edit.ts +2 −1
| @@ -19,6 +19,7 @@ | ||
| 19 | 19 | |
| 20 | 20 | import { editFiles, MAX_FILE_BYTES } from "../../actions/contents.ts"; |
| 21 | 21 | import { contribute } from "../../actions/contribute.ts"; |
| 22 | import { NO_WRITE_ACCESS } from "../../actions/pull_words.ts"; | |
| 22 | 23 | import { canAdmin, getRepoById } from "../../core/repos.ts"; |
| 23 | 24 | import { notePhase, takeSessionTime, timePhase, timePhaseAsync } from "../../core/route_timing.ts"; |
| 24 | 25 | import { isSafeRef, isSafeTreePath } from "../../git/guard.ts"; |
| @@ -116,7 +117,7 @@ export function suggestBranch(username: string, path: string): string { | ||
| 116 | 117 | export function editErrorText(error: unknown): string { |
| 117 | 118 | if (typeof error === "string") { |
| 118 | 119 | if (error === "forbidden" || error === "unauthorized") |
| 119 | return "You do not have write access to this repository."; | |
| 120 | return NO_WRITE_ACCESS; | |
| 120 | 121 | if (error === "origin_owner_only") |
| 121 | 122 | return "This repository mirrors an origin, and only the account that connected it can write through to it."; |
| 122 | 123 | if (error === "origin_unreachable") |
src/web/pages/pull.ts +4 −3
| @@ -11,6 +11,7 @@ | ||
| 11 | 11 | import { askCheck } from "../../checks/again.ts"; |
| 12 | 12 | import { addCommentThrough } from "../../actions/issues.ts"; |
| 13 | 13 | import type { AccessError } from "../../actions/access.ts"; |
| 14 | import { NO_WRITE_ACCESS, PULL_GONE } from "../../actions/pull_words.ts"; | |
| 14 | 15 | import { |
| 15 | 16 | addReview, |
| 16 | 17 | mergePull, |
| @@ -1092,8 +1093,8 @@ export function mergeFailure(error: AccessError | MergePullError): string { | ||
| 1092 | 1093 | if (error === "checks_required") |
| 1093 | 1094 | return "This pull request needs a passing deploy of its current head, and does not have one yet. Nothing was merged."; |
| 1094 | 1095 | if (error === "forbidden" || error === "unauthorized") |
| 1095 | return "You do not have write access to this repository."; | |
| 1096 | return "That pull request is no longer here."; | |
| 1096 | return NO_WRITE_ACCESS; | |
| 1097 | return PULL_GONE; | |
| 1097 | 1098 | } |
| 1098 | 1099 | |
| 1099 | 1100 | // ---- verdicts and threads ---- |
| @@ -1165,7 +1166,7 @@ export async function pullApproveHereSubmit(ctx: Ctx): Promise<Response> { | ||
| 1165 | 1166 | ? "That pull request is closed." |
| 1166 | 1167 | : done.error === "not_foreign" |
| 1167 | 1168 | ? "That pull request needs no approval to run." |
| 1168 | : "That pull request is no longer here.", | |
| 1169 | : PULL_GONE, | |
| 1169 | 1170 | }); |
| 1170 | 1171 | return flashTo(ctx, dest, { |
| 1171 | 1172 | message: isProposed(ctx.app.db, found.pull) |
test/pull-words.test.ts added +39
| @@ -0,0 +1,39 @@ | ||
| 1 | /** | |
| 2 | * The write-access and gone-pull refusals are written once, in | |
| 3 | * `actions/pull_words.ts`, and every place that says them imports them: a | |
| 4 | * copy typed back into one of the readers fails here. | |
| 5 | */ | |
| 6 | ||
| 7 | import { describe, expect, test } from "bun:test"; | |
| 8 | import { readFileSync } from "node:fs"; | |
| 9 | import { join } from "node:path"; | |
| 10 | import * as words from "../src/actions/pull_words.ts"; | |
| 11 | ||
| 12 | const READERS = [ | |
| 13 | "src/actions/finish-conflicts.ts", | |
| 14 | "src/web/pages/edit.ts", | |
| 15 | "src/web/pages/pull.ts", | |
| 16 | ]; | |
| 17 | ||
| 18 | const SENTENCES = Object.entries(words) as [string, string][]; | |
| 19 | ||
| 20 | describe("pull words", () => { | |
| 21 | test("names both refusals", () => { | |
| 22 | expect(words.NO_WRITE_ACCESS).toBe("You do not have write access to this repository."); | |
| 23 | expect(words.PULL_GONE).toBe("That pull request is no longer here."); | |
| 24 | }); | |
| 25 | ||
| 26 | for (const [name, sentence] of SENTENCES) { | |
| 27 | test(`${name} is a sentence`, () => { | |
| 28 | expect(typeof sentence).toBe("string"); | |
| 29 | expect(sentence.trim().length).toBeGreaterThan(0); | |
| 30 | }); | |
| 31 | ||
| 32 | for (const file of READERS) { | |
| 33 | test(`${file} reads ${name} rather than typing it`, () => { | |
| 34 | const source = readFileSync(join(import.meta.dir, "..", file), "utf8"); | |
| 35 | expect(source.includes(sentence)).toBe(false); | |
| 36 | }); | |
| 37 | } | |
| 38 | } | |
| 39 | }); |
new above
offline