Add required-reviewers status API for pull requests
swadhinbiswas committed 8 months ago
2c88b03
1 parent 249d4ee
commit 2c88b03
4 files changed
+201 -1
Showing 4 changed files+201-1
▼doc/feature_audit.md
+1-1
| @@ -50,7 +50,7 @@ | ||
50 | 50 | | Stack-level approvals | ⚠️ | Basic - individual PR approvals only | |
51 | 51 | | Bulk merge (merge stacks) | ⚠️ | Explicit bulk merge queue APIs now exist for stacks (`stacks/[stackId]/merge`) and selected PRs (`pulls/bulk-merge`); higher-level stack orchestration UX remains partial | |
52 | 52 | | Custom PR states | ⚠️ | State definitions plus dedicated PR state-transition API are implemented (`workflow/states`, `pulls/{number}/state`); richer UI guidance and automation-driven adoption remain partial | |
53 | -| Required reviewers per state | ⚠️ | Basic reviewer requirements exist; full state-specific enforcement remains incomplete | | |
53 | +| Required reviewers per state | ⚠️ | State reviewer requirements are enforced during transitions, and explicit required-reviewer status introspection is available (`pulls/{number}/required-reviewers`); richer state-specific policy UX remains incomplete | | |
54 | 54 | | PR merge queues | ✅ | `merge-queue.ts` (590+ lines) | |
55 | 55 | | Conflict detection before merge | ✅ | `mergeable` field in PR schema | |
56 | 56 | | Auto-merge rules | ⚠️ | Auto-merge rules engine plus explicit PR auto-merge control/status APIs now implemented (`pulls/{number}/auto-merge`); richer policy UX and rule introspection remain partial | |
57 | 57 | |
▼src/lib/openapi.ts
+18-0
| @@ -1332,6 +1332,24 @@ export const openApiSpec = { | ||
1332 | 1332 | }, |
1333 | 1333 | }, |
1334 | 1334 | }, |
1335 | + "/repos/{owner}/{repo}/pulls/{number}/required-reviewers": { | |
1336 | + get: { | |
1337 | + tags: ["Pull Requests"], | |
1338 | + summary: "Get required reviewer approval status", | |
1339 | + description: "Returns required reviewers for a PR and whether each required reviewer has an approving latest review.", | |
1340 | + security: [{ bearerAuth: [] }], | |
1341 | + parameters: [ | |
1342 | + { name: "owner", in: "path", required: true, schema: { type: "string" } }, | |
1343 | + { name: "repo", in: "path", required: true, schema: { type: "string" } }, | |
1344 | + { name: "number", in: "path", required: true, schema: { type: "integer" } }, | |
1345 | + ], | |
1346 | + responses: { | |
1347 | + 200: { description: "Required reviewer status returned" }, | |
1348 | + 401: { description: "Unauthorized", content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } }, | |
1349 | + 404: { description: "Repository or pull request not found", content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } }, | |
1350 | + }, | |
1351 | + }, | |
1352 | + }, | |
1335 | 1353 | "/repos/{owner}/{repo}/workflow/templates": { |
1336 | 1354 | get: { |
1337 | 1355 | tags: ["CI/CD"], |
1338 | 1356 | |
▼src/pages/api/repos/[owner]/[repo]/pulls/[number]/required-reviewers.ts
+92-0
| @@ -0,0 +1,92 @@ | ||
1 | +import type { APIRoute } from "astro"; | |
2 | +import type { NodePgDatabase } from "drizzle-orm/node-postgres"; | |
3 | +import { and, desc, eq } from "drizzle-orm"; | |
4 | +import { getDatabase, schema } from "@/db"; | |
5 | +import { badRequest, notFound, success, unauthorized } from "@/lib/api"; | |
6 | +import { withErrorHandler } from "@/lib/errors"; | |
7 | +import { canReadRepo } from "@/lib/permissions"; | |
8 | + | |
9 | +export const GET: APIRoute = withErrorHandler(async ({ params, locals }) => { | |
10 | + const owner = params.owner; | |
11 | + const repoName = params.repo; | |
12 | + const number = Number.parseInt(params.number || "", 10); | |
13 | + const user = locals.user; | |
14 | + | |
15 | + if (!user) return unauthorized(); | |
16 | + if (!owner || !repoName || Number.isNaN(number)) { | |
17 | + return badRequest("Missing or invalid parameters"); | |
18 | + } | |
19 | + | |
20 | + const db = getDatabase() as NodePgDatabase<typeof schema>; | |
21 | + const ownerUser = await db.query.users.findFirst({ | |
22 | + where: eq(schema.users.username, owner), | |
23 | + }); | |
24 | + if (!ownerUser) return notFound("Repository not found"); | |
25 | + | |
26 | + const repository = await db.query.repositories.findFirst({ | |
27 | + where: and( | |
28 | + eq(schema.repositories.ownerId, ownerUser.id), | |
29 | + eq(schema.repositories.name, repoName) | |
30 | + ), | |
31 | + }); | |
32 | + if (!repository) return notFound("Repository not found"); | |
33 | + | |
34 | + if (!(await canReadRepo(user.id, repository, { isAdmin: user.isAdmin }))) { | |
35 | + return notFound("Repository not found"); | |
36 | + } | |
37 | + | |
38 | + const pr = await db.query.pullRequests.findFirst({ | |
39 | + where: and( | |
40 | + eq(schema.pullRequests.repositoryId, repository.id), | |
41 | + eq(schema.pullRequests.number, number) | |
42 | + ), | |
43 | + columns: { id: true, number: true }, | |
44 | + }); | |
45 | + if (!pr) return notFound("Pull request not found"); | |
46 | + | |
47 | + const requiredReviewers = await db.query.pullRequestReviewers.findMany({ | |
48 | + where: and( | |
49 | + eq(schema.pullRequestReviewers.pullRequestId, pr.id), | |
50 | + eq(schema.pullRequestReviewers.isRequired, true) | |
51 | + ), | |
52 | + with: { | |
53 | + user: { | |
54 | + columns: { id: true, username: true, displayName: true }, | |
55 | + }, | |
56 | + }, | |
57 | + }); | |
58 | + | |
59 | + const allReviews = await db.query.pullRequestReviews.findMany({ | |
60 | + where: eq(schema.pullRequestReviews.pullRequestId, pr.id), | |
61 | + orderBy: [desc(schema.pullRequestReviews.submittedAt)], | |
62 | + }); | |
63 | + const latestReviewsByUser = new Map<string, typeof allReviews[number]>(); | |
64 | + for (const review of allReviews) { | |
65 | + if (!latestReviewsByUser.has(review.reviewerId)) { | |
66 | + latestReviewsByUser.set(review.reviewerId, review); | |
67 | + } | |
68 | + } | |
69 | + | |
70 | + const reviewers = requiredReviewers.map((required) => { | |
71 | + const latest = latestReviewsByUser.get(required.userId); | |
72 | + const reviewState = latest?.state || null; | |
73 | + const approved = reviewState === "approved"; | |
74 | + return { | |
75 | + userId: required.userId, | |
76 | + username: required.user?.username || null, | |
77 | + displayName: required.user?.displayName || null, | |
78 | + approved, | |
79 | + reviewState, | |
80 | + reviewedAt: latest?.submittedAt || latest?.createdAt || null, | |
81 | + }; | |
82 | + }); | |
83 | + | |
84 | + return success({ | |
85 | + pullRequestId: pr.id, | |
86 | + pullRequestNumber: pr.number, | |
87 | + totalRequired: reviewers.length, | |
88 | + approvedRequired: reviewers.filter((reviewer) => reviewer.approved).length, | |
89 | + missingRequired: reviewers.filter((reviewer) => !reviewer.approved).length, | |
90 | + reviewers, | |
91 | + }); | |
92 | +}); | |
0 | 93 | |
▼tests/integration/pr-required-reviewers-route.test.ts
+90-0
| @@ -0,0 +1,90 @@ | ||
1 | +import { beforeEach, describe, expect, it, vi } from "vitest"; | |
2 | + | |
3 | +const { | |
4 | + canReadRepoMock, | |
5 | + fakeSchema, | |
6 | +} = vi.hoisted(() => ({ | |
7 | + canReadRepoMock: vi.fn(async () => true), | |
8 | + fakeSchema: { | |
9 | + users: { username: {} }, | |
10 | + repositories: { ownerId: {}, name: {}, id: {} }, | |
11 | + pullRequests: { repositoryId: {}, number: {}, id: {} }, | |
12 | + pullRequestReviewers: { pullRequestId: {}, isRequired: {} }, | |
13 | + pullRequestReviews: { pullRequestId: {}, submittedAt: {} }, | |
14 | + } as any, | |
15 | +})); | |
16 | + | |
17 | +let mockDb: any; | |
18 | + | |
19 | +vi.mock("@/db", () => ({ | |
20 | + getDatabase: () => mockDb, | |
21 | + schema: fakeSchema, | |
22 | +})); | |
23 | + | |
24 | +vi.mock("@/lib/permissions", () => ({ | |
25 | + canReadRepo: canReadRepoMock, | |
26 | +})); | |
27 | + | |
28 | +import { GET as requiredReviewersGet } from "@/pages/api/repos/[owner]/[repo]/pulls/[number]/required-reviewers"; | |
29 | + | |
30 | +function makeDb() { | |
31 | + return { | |
32 | + query: { | |
33 | + users: { | |
34 | + findFirst: vi.fn(async () => ({ id: "owner-1" })), | |
35 | + }, | |
36 | + repositories: { | |
37 | + findFirst: vi.fn(async () => ({ id: "repo-1", ownerId: "owner-1", name: "demo" })), | |
38 | + }, | |
39 | + pullRequests: { | |
40 | + findFirst: vi.fn(async () => ({ id: "pr-1", number: 12 })), | |
41 | + }, | |
42 | + pullRequestReviewers: { | |
43 | + findMany: vi.fn(async () => ([ | |
44 | + { userId: "u1", user: { username: "alice", displayName: "Alice" } }, | |
45 | + { userId: "u2", user: { username: "bob", displayName: "Bob" } }, | |
46 | + ])), | |
47 | + }, | |
48 | + pullRequestReviews: { | |
49 | + findMany: vi.fn(async () => ([ | |
50 | + { reviewerId: "u1", state: "approved", submittedAt: new Date("2026-02-19T00:00:00Z"), createdAt: new Date("2026-02-19T00:00:00Z") }, | |
51 | + { reviewerId: "u2", state: "changes_requested", submittedAt: new Date("2026-02-18T00:00:00Z"), createdAt: new Date("2026-02-18T00:00:00Z") }, | |
52 | + ])), | |
53 | + }, | |
54 | + }, | |
55 | + }; | |
56 | +} | |
57 | + | |
58 | +async function readJson(response: Response): Promise<any> { | |
59 | + return response.json(); | |
60 | +} | |
61 | + | |
62 | +describe("required reviewers route", () => { | |
63 | + beforeEach(() => { | |
64 | + mockDb = makeDb(); | |
65 | + canReadRepoMock.mockResolvedValue(true); | |
66 | + }); | |
67 | + | |
68 | + it("returns 401 for unauthenticated users", async () => { | |
69 | + const response = await requiredReviewersGet({ | |
70 | + params: { owner: "owner-1", repo: "demo", number: "12" }, | |
71 | + locals: {}, | |
72 | + } as any); | |
73 | + | |
74 | + expect(response.status).toBe(401); | |
75 | + }); | |
76 | + | |
77 | + it("returns required reviewer approval status", async () => { | |
78 | + const response = await requiredReviewersGet({ | |
79 | + params: { owner: "owner-1", repo: "demo", number: "12" }, | |
80 | + locals: { user: { id: "user-1", isAdmin: false } }, | |
81 | + } as any); | |
82 | + | |
83 | + const body = await readJson(response); | |
84 | + expect(response.status).toBe(200); | |
85 | + expect(body?.data?.totalRequired).toBe(2); | |
86 | + expect(body?.data?.approvedRequired).toBe(1); | |
87 | + expect(body?.data?.missingRequired).toBe(1); | |
88 | + expect(body?.data?.reviewers?.[0]?.username).toBe("alice"); | |
89 | + }); | |
90 | +}); | |
0 | 91 | |