Public: anyone can read this; the crew can change it.
GET …/issues/{number}?comments=N answers only the newest N comments and the total count #4104
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/4097-get-issues-number-comments-n-answers-onl → main 1 commit fork
3 files
changed +84
−2
src/contract/openapi.ts +1 −1
| @@ -73,7 +73,7 @@ export const ROUTES: readonly Route[] = [ | ||
| 73 | 73 | auth: true, |
| 74 | 74 | body: { title: "string", body: "string" }, |
| 75 | 75 | }, |
| 76 | { method: "get", path: `${REPO}/issues/{number}`, summary: "Read one issue with its comments", auth: false }, | |
| 76 | { method: "get", path: `${REPO}/issues/{number}`, summary: "Read one issue with its comments; comments=N (0 to 200) answers only the newest N and commentCount, any other value is a 422 invalid_comments", auth: false, query: { comments: "integer" } }, | |
| 77 | 77 | { |
| 78 | 78 | method: "patch", |
| 79 | 79 | path: `${REPO}/issues/{number}`, |
src/http/api.ts +11 −1
| @@ -3226,6 +3226,15 @@ export function apiRoutes(router: Router<Handler>): void { | ||
| 3226 | 3226 | const found = loadConversation(ctx); |
| 3227 | 3227 | if ("error" in found) return found.error; |
| 3228 | 3228 | const { repo, issue } = found; |
| 3229 | // comments=N answers the newest N, oldest first, and the total (#4097). | |
| 3230 | const asked = ctx.url.searchParams.get("comments"); | |
| 3231 | let tail: number | null = null; | |
| 3232 | if (asked !== null) { | |
| 3233 | if (!/^\d{1,3}$/.test(asked) || Number(asked) > 200) | |
| 3234 | return apiError("invalid_comments", 422, "comments must be a whole number from 0 to 200"); | |
| 3235 | tail = Number(asked); | |
| 3236 | } | |
| 3237 | const all = listComments(ctx.app.db, found.issue.id); | |
| 3229 | 3238 | return json({ |
| 3230 | 3239 | ...withLabels(ctx, issue), |
| 3231 | 3240 | ...(issue.kind === "pull" |
| @@ -3236,8 +3245,9 @@ export function apiRoutes(router: Router<Handler>): void { | ||
| 3236 | 3245 | comments: readerMarked( |
| 3237 | 3246 | ctx, |
| 3238 | 3247 | found.repo.id, |
| 3239 | listComments(ctx.app.db, found.issue.id), | |
| 3248 | tail === null ? all : tail === 0 ? [] : all.slice(-tail), | |
| 3240 | 3249 | ), |
| 3250 | ...(tail === null ? {} : { commentCount: all.length }), | |
| 3241 | 3251 | }); |
| 3242 | 3252 | }); |
| 3243 | 3253 |
test/api-issue-comments-tail.test.ts added +72
| @@ -0,0 +1,72 @@ | ||
| 1 | /** GET …/issues/{number}?comments=N answers the newest N comments and the total (#4097). */ | |
| 2 | import { afterAll, beforeAll, expect, test } from "bun:test"; | |
| 3 | import { mkdtemp, rm } from "node:fs/promises"; | |
| 4 | import { tmpdir } from "node:os"; | |
| 5 | import { join } from "node:path"; | |
| 6 | import { createApp, type App } from "../src/app.ts"; | |
| 7 | import { createRepo } from "../src/actions/repos.ts"; | |
| 8 | import { addComment, createIssue } from "../src/core/issues.ts"; | |
| 9 | import { serve, type Servers } from "../src/http/server.ts"; | |
| 10 | import { seedAdmin } from "./helpers/admin.ts"; | |
| 11 | ||
| 12 | let dataDir = ""; | |
| 13 | let app: App, servers: Servers; | |
| 14 | let number = 0; | |
| 15 | ||
| 16 | beforeAll(async () => { | |
| 17 | dataDir = await mkdtemp(join(tmpdir(), "forge-commentstail-")); | |
| 18 | app = createApp({ dataDir, port: 0 }); | |
| 19 | servers = await serve(app); | |
| 20 | const admin = await seedAdmin(app, { username: "alice", email: "[email protected]", password: "correct-horse-battery" }); | |
| 21 | if (!admin.ok) throw new Error(admin.error); | |
| 22 | const made = await createRepo(app, admin.value, { name: "notes", visibility: "public", description: "" }); | |
| 23 | if (!made.ok) throw new Error(String(made.error)); | |
| 24 | const issue = createIssue(app.db, { repoId: made.value.id, title: "long", body: "b", authorId: admin.value.id }); | |
| 25 | if (!issue.ok) throw new Error(String(issue.error)); | |
| 26 | number = issue.value.number; | |
| 27 | for (let i = 1; i <= 30; i++) | |
| 28 | addComment(app.db, { issueId: issue.value.id, authorId: admin.value.id, body: `c${i}` }); | |
| 29 | }); | |
| 30 | ||
| 31 | afterAll(async () => { | |
| 32 | servers.stop(); | |
| 33 | app.close(); | |
| 34 | await rm(dataDir, { recursive: true, force: true }); | |
| 35 | }); | |
| 36 | ||
| 37 | const read = (q: string) => fetch(`${servers.url}/api/v1/repos/alice/notes/issues/${number}${q}`); | |
| 38 | type Body = { comments: { body: string }[]; commentCount?: number }; | |
| 39 | ||
| 40 | test("comments=5 answers the newest five, oldest first, with the total", async () => { | |
| 41 | const res = await read("?comments=5"); | |
| 42 | expect(res.status).toBe(200); | |
| 43 | const body = (await res.json()) as Body; | |
| 44 | expect(body.comments.map((c) => c.body)).toEqual(["c26", "c27", "c28", "c29", "c30"]); | |
| 45 | expect(body.commentCount).toBe(30); | |
| 46 | }); | |
| 47 | ||
| 48 | test("comments=0 answers no comments and the count", async () => { | |
| 49 | const body = (await (await read("?comments=0")).json()) as Body; | |
| 50 | expect(body.comments).toEqual([]); | |
| 51 | expect(body.commentCount).toBe(30); | |
| 52 | }); | |
| 53 | ||
| 54 | test("comments=200 on a shorter thread answers all of it", async () => { | |
| 55 | const body = (await (await read("?comments=200")).json()) as Body; | |
| 56 | expect(body.comments.length).toBe(30); | |
| 57 | expect(body.commentCount).toBe(30); | |
| 58 | }); | |
| 59 | ||
| 60 | test("anything but 0 to 200 is a 422 invalid_comments", async () => { | |
| 61 | for (const q of ["abc", "201", "-1", "1.5", ""]) { | |
| 62 | const res = await read(`?comments=${q}`); | |
| 63 | expect(res.status).toBe(422); | |
| 64 | expect(((await res.json()) as { error: string }).error).toBe("invalid_comments"); | |
| 65 | } | |
| 66 | }); | |
| 67 | ||
| 68 | test("without the parameter all comments come back and no commentCount", async () => { | |
| 69 | const body = (await (await read("")).json()) as Body; | |
| 70 | expect(body.comments.length).toBe(30); | |
| 71 | expect("commentCount" in body).toBe(false); | |
| 72 | }); |
new above
offline