Public: anyone can read this; the crew can change it.
A repository's reports list answers `{ items, next_cursor }` #5229
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/5223-a-repository-s-reports-list-answers-item → main 2 commits fork 1 thread · 1 unresolved
2 files
changed +41
−1
src/http/api.ts +9 −1
| @@ -3290,12 +3290,20 @@ export function apiRoutes(router: Router<Handler>): void { | ||
| 3290 | 3290 | if (denied) return denied; |
| 3291 | 3291 | const repo = loadRepo(ctx); |
| 3292 | 3292 | if (!repo) return notFound(); |
| 3293 | const window = keyedWindow(ctx); | |
| 3294 | if (window instanceof Response) return window; | |
| 3293 | 3295 | const waiting = timePhase(ctx.request, "reports", () => |
| 3294 | 3296 | waitingReports(ctx.app, ctx.auth.user, repo), |
| 3295 | 3297 | ); |
| 3296 | 3298 | if (!waiting.ok) return respondAccess(waiting.error, 403); |
| 3297 | 3299 | return timePhase(ctx.request, "render", () => |
| 3298 | json({ items: waiting.value.map((r) => ({ ...r, from: "outside" })) }), | |
| 3300 | json( | |
| 3301 | keyedPage( | |
| 3302 | waiting.value.map((r) => ({ ...r, from: "outside" })), | |
| 3303 | (r) => r.id, | |
| 3304 | window, | |
| 3305 | ), | |
| 3306 | ), | |
| 3299 | 3307 | ); |
| 3300 | 3308 | }); |
| 3301 | 3309 |
test/reports.test.ts +32
| @@ -537,6 +537,38 @@ describe("reports from outside", () => { | ||
| 537 | 537 | expect(count("issue")).toBe(issues); |
| 538 | 538 | }); |
| 539 | 539 | |
| 540 | test("the reports list pages with limit and next_cursor, and refuses a bad window", async () => { | |
| 541 | app.db.run("DELETE FROM report"); | |
| 542 | const insert = app.db.query( | |
| 543 | "INSERT INTO report (id, repo_id, title, body) VALUES (?1, ?2, 't', '')", | |
| 544 | ); | |
| 545 | for (let i = 0; i < 5; i++) insert.run(`page${i}`, game.id); | |
| 546 | const path = "/api/v1/repos/alice/game/reports"; | |
| 547 | const whole = (await (await as(aliceToken, path)).json()) as any; | |
| 548 | expect(whole.items.length).toBe(5); | |
| 549 | expect(whole.next_cursor).toBeNull(); | |
| 550 | const seen: string[] = []; | |
| 551 | let cursor: string | null = null; | |
| 552 | let pages = 0; | |
| 553 | do { | |
| 554 | const res = await as(aliceToken, `${path}?limit=2${cursor ? `&cursor=${cursor}` : ""}`); | |
| 555 | expect(res.status).toBe(200); | |
| 556 | const page = (await res.json()) as any; | |
| 557 | expect(page.items.every((r: any) => r.from === "outside")).toBe(true); | |
| 558 | seen.push(...page.items.map((r: any) => r.id)); | |
| 559 | cursor = page.next_cursor; | |
| 560 | pages++; | |
| 561 | } while (cursor !== null && pages < 10); | |
| 562 | expect(pages).toBe(3); | |
| 563 | expect(seen).toEqual(whole.items.map((r: any) => r.id)); | |
| 564 | for (const bad of ["0", "201", "-1", "x", ""]) { | |
| 565 | const res = await as(aliceToken, `${path}?limit=${bad}`); | |
| 566 | expect(res.status).toBe(422); | |
| 567 | expect(await res.text()).toContain("invalid_limit"); | |
| 568 | } | |
| 569 | app.db.run("DELETE FROM report"); | |
| 570 | }); | |
| 571 | ||
| 540 | 572 | test("a repository holds a bounded number of waiting reports", async () => { |
| 541 | 573 | app.db.run("DELETE FROM report"); |
| 542 | 574 | const insert = app.db.query( |
new above
offline
Non-blocking: parsing the window before
waitingReportsmeans a non-decider with?limit=0sees a 422 before the 403. It leaks nothing, but moving this below the access check would make the error order match the rest of the route.