Add merge gate item update and delete APIs
swadhinbiswas committed 8 months ago
c6a96ef
1 parent 8879d41
commit c6a96ef
5 files changed
+313 -1
Showing 5 changed files+313-1
▼doc/feature_audit.md
+1-1
| @@ -94,7 +94,7 @@ | ||
94 | 94 | | External CI integration | ⚠️ | Provider-aware integration APIs and normalized status ingestion are implemented, with repo-level integration health/status + token rotate/disable APIs (`external-ci`, `external-ci/checks`); provider-specific setup UX still maturing | |
95 | 95 | | Self-hosted runners | ✅ | `runner/` with Docker executor | |
96 | 96 | | Secrets management | ✅ | Schema + UI for repo/org secrets | |
97 | -| Merge checks & gates | ⚠️ | Required status checks + external CI gate readiness are enforced, with explicit PR gate introspection via `pulls/{number}/merge-readiness` and repository-level merge gate management via `/repos/{owner}/{repo}/merge-gates`; broader policy UX remains | | |
97 | +| Merge checks & gates | ⚠️ | Required status checks + external CI gate readiness are enforced, with explicit PR gate introspection via `pulls/{number}/merge-readiness` and repository-level merge gate management APIs (`/repos/{owner}/{repo}/merge-gates`, `/repos/{owner}/{repo}/merge-gates/{id}`); broader policy UX remains | | |
98 | 98 | | Automation rules engine | ⚠️ | Conditions/actions active with retry + dead-letter audit logging; advanced orchestration still maturing | |
99 | 99 | | Webhooks | ✅ | Full implementation with UI | |
100 | 100 | | Workflow templates | ⚠️ | Template library plus repo-level listing/apply APIs are now implemented (`workflow/templates`); broader UX/workflow adoption still maturing | |
101 | 101 | |
▼src/lib/ci-gates.ts
+16-0
| @@ -360,3 +360,19 @@ export async function removeRequiredCheck(checkId: string): Promise<boolean> { | ||
360 | 360 | return false; |
361 | 361 | } |
362 | 362 | } |
363 | + | |
364 | +/** | |
365 | + * Delete merge gate | |
366 | + */ | |
367 | +export async function removeMergeGate(gateId: string): Promise<boolean> { | |
368 | + const db = getDatabase(); | |
369 | + | |
370 | + try { | |
371 | + // @ts-expect-error - Drizzle multi-db union type issue | |
372 | + await db.delete(schema.mergeGates) | |
373 | + .where(eq(schema.mergeGates.id, gateId)); | |
374 | + return true; | |
375 | + } catch { | |
376 | + return false; | |
377 | + } | |
378 | +} | |
363 | 379 | |
▼src/lib/openapi.ts
+51-0
| @@ -1067,6 +1067,57 @@ export const openApiSpec = { | ||
1067 | 1067 | }, |
1068 | 1068 | }, |
1069 | 1069 | }, |
1070 | + "/repos/{owner}/{repo}/merge-gates/{id}": { | |
1071 | + patch: { | |
1072 | + tags: ["CI/CD"], | |
1073 | + summary: "Enable or disable a merge gate", | |
1074 | + security: [{ bearerAuth: [] }], | |
1075 | + parameters: [ | |
1076 | + { name: "owner", in: "path", required: true, schema: { type: "string" } }, | |
1077 | + { name: "repo", in: "path", required: true, schema: { type: "string" } }, | |
1078 | + { name: "id", in: "path", required: true, schema: { type: "string" } }, | |
1079 | + ], | |
1080 | + requestBody: { | |
1081 | + required: true, | |
1082 | + content: { | |
1083 | + "application/json": { | |
1084 | + schema: { | |
1085 | + type: "object", | |
1086 | + properties: { | |
1087 | + enabled: { type: "boolean" }, | |
1088 | + }, | |
1089 | + required: ["enabled"], | |
1090 | + }, | |
1091 | + }, | |
1092 | + }, | |
1093 | + }, | |
1094 | + responses: { | |
1095 | + 200: { description: "Merge gate updated" }, | |
1096 | + 400: { description: "Invalid payload or update failure", content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } }, | |
1097 | + 401: { description: "Unauthorized", content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } }, | |
1098 | + 403: { description: "Forbidden", content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } }, | |
1099 | + 404: { description: "Repository or merge gate not found", content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } }, | |
1100 | + }, | |
1101 | + }, | |
1102 | + delete: { | |
1103 | + tags: ["CI/CD"], | |
1104 | + summary: "Delete merge gate or required check", | |
1105 | + description: "Deletes a repository-scoped merge gate entry. If the id belongs to a required status check, that check is deleted.", | |
1106 | + security: [{ bearerAuth: [] }], | |
1107 | + parameters: [ | |
1108 | + { name: "owner", in: "path", required: true, schema: { type: "string" } }, | |
1109 | + { name: "repo", in: "path", required: true, schema: { type: "string" } }, | |
1110 | + { name: "id", in: "path", required: true, schema: { type: "string" } }, | |
1111 | + ], | |
1112 | + responses: { | |
1113 | + 200: { description: "Entry deleted" }, | |
1114 | + 400: { description: "Delete failed", content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } }, | |
1115 | + 401: { description: "Unauthorized", content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } }, | |
1116 | + 403: { description: "Forbidden", content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } }, | |
1117 | + 404: { description: "Repository or entry not found", content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } }, | |
1118 | + }, | |
1119 | + }, | |
1120 | + }, | |
1070 | 1121 | "/user/notification-digests/test": { |
1071 | 1122 | post: { |
1072 | 1123 | tags: ["Notifications"], |
1073 | 1124 | |
▼src/pages/api/repos/[owner]/[repo]/merge-gates/[id].ts
+106-0
| @@ -0,0 +1,106 @@ | ||
1 | +import type { APIRoute } from "astro"; | |
2 | +import type { NodePgDatabase } from "drizzle-orm/node-postgres"; | |
3 | +import { and, eq } from "drizzle-orm"; | |
4 | +import { z } from "zod"; | |
5 | +import { getDatabase, schema } from "@/db"; | |
6 | +import { badRequest, forbidden, notFound, success, unauthorized } from "@/lib/api"; | |
7 | +import { withErrorHandler } from "@/lib/errors"; | |
8 | +import { canAdminRepo } from "@/lib/permissions"; | |
9 | +import { removeMergeGate, removeRequiredCheck, toggleGate } from "@/lib/ci-gates"; | |
10 | + | |
11 | +const patchPayloadSchema = z.object({ | |
12 | + enabled: z.boolean(), | |
13 | +}); | |
14 | + | |
15 | +async function resolveRepository(owner: string, repoName: string) { | |
16 | + const db = getDatabase() as NodePgDatabase<typeof schema>; | |
17 | + const ownerUser = await db.query.users.findFirst({ | |
18 | + where: eq(schema.users.username, owner), | |
19 | + }); | |
20 | + if (!ownerUser) return null; | |
21 | + return db.query.repositories.findFirst({ | |
22 | + where: and( | |
23 | + eq(schema.repositories.ownerId, ownerUser.id), | |
24 | + eq(schema.repositories.name, repoName) | |
25 | + ), | |
26 | + }); | |
27 | +} | |
28 | + | |
29 | +export const PATCH: APIRoute = withErrorHandler(async ({ params, request, locals }) => { | |
30 | + const owner = params.owner; | |
31 | + const repoName = params.repo; | |
32 | + const id = params.id; | |
33 | + const user = locals.user; | |
34 | + | |
35 | + if (!user) return unauthorized(); | |
36 | + if (!owner || !repoName || !id) return badRequest("Missing parameters"); | |
37 | + | |
38 | + const repository = await resolveRepository(owner, repoName); | |
39 | + if (!repository) return notFound("Repository not found"); | |
40 | + | |
41 | + if (!(await canAdminRepo(user.id, repository, { isAdmin: user.isAdmin }))) { | |
42 | + return forbidden(); | |
43 | + } | |
44 | + | |
45 | + const db = getDatabase() as NodePgDatabase<typeof schema>; | |
46 | + const gate = await db.query.mergeGates.findFirst({ | |
47 | + where: and( | |
48 | + eq(schema.mergeGates.id, id), | |
49 | + eq(schema.mergeGates.repositoryId, repository.id) | |
50 | + ), | |
51 | + }); | |
52 | + if (!gate) return notFound("Merge gate not found"); | |
53 | + | |
54 | + const body = await request.json().catch(() => null); | |
55 | + const parsed = patchPayloadSchema.safeParse(body || {}); | |
56 | + if (!parsed.success) { | |
57 | + return badRequest(parsed.error.issues[0]?.message || "Invalid merge gate payload"); | |
58 | + } | |
59 | + | |
60 | + const ok = await toggleGate(gate.id, parsed.data.enabled); | |
61 | + if (!ok) return badRequest("Failed to update merge gate"); | |
62 | + | |
63 | + return success({ updated: true, id: gate.id, enabled: parsed.data.enabled }); | |
64 | +}); | |
65 | + | |
66 | +export const DELETE: APIRoute = withErrorHandler(async ({ params, locals }) => { | |
67 | + const owner = params.owner; | |
68 | + const repoName = params.repo; | |
69 | + const id = params.id; | |
70 | + const user = locals.user; | |
71 | + | |
72 | + if (!user) return unauthorized(); | |
73 | + if (!owner || !repoName || !id) return badRequest("Missing parameters"); | |
74 | + | |
75 | + const repository = await resolveRepository(owner, repoName); | |
76 | + if (!repository) return notFound("Repository not found"); | |
77 | + | |
78 | + if (!(await canAdminRepo(user.id, repository, { isAdmin: user.isAdmin }))) { | |
79 | + return forbidden(); | |
80 | + } | |
81 | + | |
82 | + const db = getDatabase() as NodePgDatabase<typeof schema>; | |
83 | + const requiredCheck = await db.query.requiredStatusChecks.findFirst({ | |
84 | + where: and( | |
85 | + eq(schema.requiredStatusChecks.id, id), | |
86 | + eq(schema.requiredStatusChecks.repositoryId, repository.id) | |
87 | + ), | |
88 | + }); | |
89 | + if (requiredCheck) { | |
90 | + const ok = await removeRequiredCheck(requiredCheck.id); | |
91 | + if (!ok) return badRequest("Failed to delete required check"); | |
92 | + return success({ deleted: true, kind: "required_check", id: requiredCheck.id }); | |
93 | + } | |
94 | + | |
95 | + const gate = await db.query.mergeGates.findFirst({ | |
96 | + where: and( | |
97 | + eq(schema.mergeGates.id, id), | |
98 | + eq(schema.mergeGates.repositoryId, repository.id) | |
99 | + ), | |
100 | + }); | |
101 | + if (!gate) return notFound("Merge gate or required check not found"); | |
102 | + | |
103 | + const ok = await removeMergeGate(gate.id); | |
104 | + if (!ok) return badRequest("Failed to delete merge gate"); | |
105 | + return success({ deleted: true, kind: "merge_gate", id: gate.id }); | |
106 | +}); | |
0 | 107 | |
▼tests/integration/merge-gate-item-route.test.ts
+139-0
| @@ -0,0 +1,139 @@ | ||
1 | +import { beforeEach, describe, expect, it, vi } from "vitest"; | |
2 | + | |
3 | +const { | |
4 | + canAdminRepoMock, | |
5 | + toggleGateMock, | |
6 | + removeRequiredCheckMock, | |
7 | + removeMergeGateMock, | |
8 | + fakeSchema, | |
9 | +} = vi.hoisted(() => ({ | |
10 | + canAdminRepoMock: vi.fn(async () => true), | |
11 | + toggleGateMock: vi.fn(async () => true), | |
12 | + removeRequiredCheckMock: vi.fn(async () => true), | |
13 | + removeMergeGateMock: vi.fn(async () => true), | |
14 | + fakeSchema: { | |
15 | + users: { username: {} }, | |
16 | + repositories: { ownerId: {}, name: {}, id: {} }, | |
17 | + mergeGates: { id: {}, repositoryId: {} }, | |
18 | + requiredStatusChecks: { id: {}, repositoryId: {} }, | |
19 | + } as any, | |
20 | +})); | |
21 | + | |
22 | +let mockDb: any; | |
23 | + | |
24 | +vi.mock("@/db", () => ({ | |
25 | + getDatabase: () => mockDb, | |
26 | + schema: fakeSchema, | |
27 | +})); | |
28 | + | |
29 | +vi.mock("@/lib/permissions", () => ({ | |
30 | + canAdminRepo: canAdminRepoMock, | |
31 | +})); | |
32 | + | |
33 | +vi.mock("@/lib/ci-gates", () => ({ | |
34 | + toggleGate: toggleGateMock, | |
35 | + removeRequiredCheck: removeRequiredCheckMock, | |
36 | + removeMergeGate: removeMergeGateMock, | |
37 | +})); | |
38 | + | |
39 | +import { | |
40 | + PATCH as mergeGatePatch, | |
41 | + DELETE as mergeGateDelete, | |
42 | +} from "@/pages/api/repos/[owner]/[repo]/merge-gates/[id]"; | |
43 | + | |
44 | +function makeDb() { | |
45 | + return { | |
46 | + query: { | |
47 | + users: { | |
48 | + findFirst: vi.fn(async () => ({ id: "owner-1" })), | |
49 | + }, | |
50 | + repositories: { | |
51 | + findFirst: vi.fn(async () => ({ id: "repo-1", ownerId: "owner-1", name: "demo" })), | |
52 | + }, | |
53 | + mergeGates: { | |
54 | + findFirst: vi.fn(async () => ({ id: "gate-1", repositoryId: "repo-1", isEnabled: true })), | |
55 | + }, | |
56 | + requiredStatusChecks: { | |
57 | + findFirst: vi.fn(async () => null), | |
58 | + }, | |
59 | + }, | |
60 | + }; | |
61 | +} | |
62 | + | |
63 | +describe("merge gate item route", () => { | |
64 | + beforeEach(() => { | |
65 | + vi.clearAllMocks(); | |
66 | + mockDb = makeDb(); | |
67 | + canAdminRepoMock.mockResolvedValue(true); | |
68 | + toggleGateMock.mockResolvedValue(true); | |
69 | + removeRequiredCheckMock.mockResolvedValue(true); | |
70 | + removeMergeGateMock.mockResolvedValue(true); | |
71 | + }); | |
72 | + | |
73 | + it("updates merge gate enabled state for admins", async () => { | |
74 | + const response = await mergeGatePatch({ | |
75 | + params: { owner: "owner-1", repo: "demo", id: "gate-1" }, | |
76 | + locals: { user: { id: "user-1", isAdmin: false } }, | |
77 | + request: new Request("http://localhost/api/repos/owner-1/demo/merge-gates/gate-1", { | |
78 | + method: "PATCH", | |
79 | + headers: { "content-type": "application/json" }, | |
80 | + body: JSON.stringify({ enabled: false }), | |
81 | + }), | |
82 | + } as any); | |
83 | + | |
84 | + expect(response.status).toBe(200); | |
85 | + expect(toggleGateMock).toHaveBeenCalledWith("gate-1", false); | |
86 | + }); | |
87 | + | |
88 | + it("returns 403 for non-admin updates", async () => { | |
89 | + canAdminRepoMock.mockResolvedValue(false); | |
90 | + const response = await mergeGatePatch({ | |
91 | + params: { owner: "owner-1", repo: "demo", id: "gate-1" }, | |
92 | + locals: { user: { id: "user-1", isAdmin: false } }, | |
93 | + request: new Request("http://localhost/api/repos/owner-1/demo/merge-gates/gate-1", { | |
94 | + method: "PATCH", | |
95 | + headers: { "content-type": "application/json" }, | |
96 | + body: JSON.stringify({ enabled: true }), | |
97 | + }), | |
98 | + } as any); | |
99 | + | |
100 | + expect(response.status).toBe(403); | |
101 | + expect(toggleGateMock).not.toHaveBeenCalled(); | |
102 | + }); | |
103 | + | |
104 | + it("deletes required check when id maps to required check", async () => { | |
105 | + mockDb.query.requiredStatusChecks.findFirst = vi.fn(async () => ({ | |
106 | + id: "check-1", | |
107 | + repositoryId: "repo-1", | |
108 | + })); | |
109 | + | |
110 | + const response = await mergeGateDelete({ | |
111 | + params: { owner: "owner-1", repo: "demo", id: "check-1" }, | |
112 | + locals: { user: { id: "user-1", isAdmin: false } }, | |
113 | + } as any); | |
114 | + | |
115 | + expect(response.status).toBe(200); | |
116 | + expect(removeRequiredCheckMock).toHaveBeenCalledWith("check-1"); | |
117 | + expect(removeMergeGateMock).not.toHaveBeenCalled(); | |
118 | + }); | |
119 | + | |
120 | + it("deletes merge gate when id maps to gate", async () => { | |
121 | + const response = await mergeGateDelete({ | |
122 | + params: { owner: "owner-1", repo: "demo", id: "gate-1" }, | |
123 | + locals: { user: { id: "user-1", isAdmin: false } }, | |
124 | + } as any); | |
125 | + | |
126 | + expect(response.status).toBe(200); | |
127 | + expect(removeMergeGateMock).toHaveBeenCalledWith("gate-1"); | |
128 | + }); | |
129 | + | |
130 | + it("returns 404 when entry is not found for delete", async () => { | |
131 | + mockDb.query.mergeGates.findFirst = vi.fn(async () => null); | |
132 | + const response = await mergeGateDelete({ | |
133 | + params: { owner: "owner-1", repo: "demo", id: "missing-1" }, | |
134 | + locals: { user: { id: "user-1", isAdmin: false } }, | |
135 | + } as any); | |
136 | + | |
137 | + expect(response.status).toBe(404); | |
138 | + }); | |
139 | +}); | |
0 | 140 | |