Public: anyone can read this; the crew can change it.
Reading …/issues/{number} with a pull's number answers with the pull, marked kind "pull", not not_found #3313
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/3299-reading-issues-number-with-a-pull-s-numb → main 1 commit fork 1 thread · 1 unresolved
2 files
changed +86
−2
src/http/api.ts +10 −2
| @@ -3076,10 +3076,18 @@ export function apiRoutes(router: Router<Handler>): void { | ||
| 3076 | 3076 | ); |
| 3077 | 3077 | |
| 3078 | 3078 | router.add("GET", "/api/v1/repos/:owner/:name/issues/:number", (ctx) => { |
| 3079 | const found = loadIssue(ctx); | |
| 3079 | // Numbers are shared with pulls: a pull's number answers as the same | |
| 3080 | // shape, marked `kind: "pull"`, with the path where its diff lives (#3299). | |
| 3081 | const found = loadConversation(ctx); | |
| 3080 | 3082 | if ("error" in found) return found.error; |
| 3083 | const { repo, issue } = found; | |
| 3081 | 3084 | return json({ |
| 3082 | ...withLabels(ctx, found.issue), | |
| 3085 | ...withLabels(ctx, issue), | |
| 3086 | ...(issue.kind === "pull" | |
| 3087 | ? { | |
| 3088 | pull: `/api/v1/repos/${repo.ownerName}/${repo.name}/pulls/${issue.number}`, | |
| 3089 | } | |
| 3090 | : {}), | |
| 3083 | 3091 | comments: readerMarked( |
| 3084 | 3092 | ctx, |
| 3085 | 3093 | found.repo.id, |
test/issue-route-pull.test.ts added +76
| @@ -0,0 +1,76 @@ | ||
| 1 | import { afterAll, beforeAll, expect, test } from "bun:test"; | |
| 2 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 3 | import { tmpdir } from "node:os"; | |
| 4 | import { join } from "node:path"; | |
| 5 | import { createApp, type App } from "../src/app.ts"; | |
| 6 | import { createIssue } from "../src/core/issues.ts"; | |
| 7 | import { createRepo } from "../src/core/repos.ts"; | |
| 8 | import { createUser } from "../src/core/users.ts"; | |
| 9 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 10 | ||
| 11 | let dataDir: string; | |
| 12 | let app: App; | |
| 13 | let servers: Servers; | |
| 14 | ||
| 15 | beforeAll(async () => { | |
| 16 | dataDir = await mkdtemp(join(tmpdir(), "forge-iss-pull-")); | |
| 17 | app = createApp({ dataDir, port: 0 }); | |
| 18 | servers = await serve(app); | |
| 19 | const user = await createUser(app.db, { | |
| 20 | origin: "seed", | |
| 21 | trusted: true, | |
| 22 | username: "pat", | |
| 23 | email: "[email protected]", | |
| 24 | password: "pull-number-pass", | |
| 25 | }); | |
| 26 | if (!user.ok) throw new Error("user"); | |
| 27 | for (const [name, visibility] of [ | |
| 28 | ["open", "public"], | |
| 29 | ["shut", "private"], | |
| 30 | ] as const) { | |
| 31 | const repo = await createRepo(app.db, app.reposRoot, { | |
| 32 | ownerId: user.value.id, | |
| 33 | name, | |
| 34 | visibility, | |
| 35 | }); | |
| 36 | if (!repo.ok) throw new Error("repo"); | |
| 37 | for (const [title, kind] of [ | |
| 38 | ["an issue", "issue"], | |
| 39 | ["a pull", "pull"], | |
| 40 | ] as const) | |
| 41 | createIssue(app.db, { | |
| 42 | repoId: repo.value.id, | |
| 43 | authorId: user.value.id, | |
| 44 | title, | |
| 45 | kind, | |
| 46 | }); | |
| 47 | } | |
| 48 | }); | |
| 49 | ||
| 50 | afterAll(async () => { | |
| 51 | servers.stop(); | |
| 52 | app.close(); | |
| 53 | await rm(dataDir, { recursive: true, force: true }); | |
| 54 | }); | |
| 55 | ||
| 56 | const read = (path: string) => fetch(`${servers.url}/api/v1/repos/pat/${path}`); | |
| 57 | ||
| 58 | test("a pull's number read through issues answers with the pull, kind pull", async () => { | |
| 59 | const res = await read("open/issues/2"); | |
| 60 | expect(res.status).toBe(200); | |
| 61 | const body = (await res.json()) as Record<string, unknown>; | |
| 62 | expect(body.kind).toBe("pull"); | |
| 63 | expect(body.title).toBe("a pull"); | |
| 64 | expect(body.pull).toBe("/api/v1/repos/pat/open/pulls/2"); | |
| 65 | expect(Array.isArray(body.comments)).toBe(true); | |
| 66 | }); | |
| 67 | ||
| 68 | test("an issue's number still answers kind issue, with no pull field", async () => { | |
| 69 | const body = (await (await read("open/issues/1")).json()) as Record<string, unknown>; | |
| 70 | expect(body.kind).toBe("issue"); | |
| 71 | expect("pull" in body).toBe(false); | |
| 72 | }); | |
| 73 | ||
| 74 | test("a pull the caller cannot see is still not_found", async () => { | |
| 75 | expect((await read("shut/issues/2")).status).toBe(404); | |
| 76 | }); |
new above
offline
Good.
loadConversationkeeps themayViewcheck, so pulls inherit exactly the visibility pulls already have. Sealed and private items staynot_found.