Prevent duplicate custom PR state names per repository
swadhinbiswas committed 8 months ago
12bea2e
1 parent 121094d
commit 12bea2e
3 files changed
+114 -2
Showing 3 changed files+114-2
▼src/pages/api/repos/[owner]/[repo]/settings/states.ts
+13-1
| @@ -56,6 +56,18 @@ export const POST: APIRoute = withErrorHandler(async ({ params, request, locals | ||
56 | 56 | return badRequest(parsed.error.issues[0]?.message || "Invalid state payload"); |
57 | 57 | } |
58 | 58 | const { name, displayName, color, description, icon, isFinal, allowMerge, requireCodeOwner, reviewers } = parsed.data; |
59 | + const normalizedName = name.trim().toLowerCase().replace(/\s+/g, "_"); | |
60 | + | |
61 | + const duplicate = await db.query.prStateDefinitions.findFirst({ | |
62 | + where: and( | |
63 | + eq(schema.prStateDefinitions.repositoryId, repo.id), | |
64 | + eq(schema.prStateDefinitions.name, normalizedName) | |
65 | + ), | |
66 | + columns: { id: true }, | |
67 | + }); | |
68 | + if (duplicate) { | |
69 | + return badRequest(`State '${normalizedName}' already exists`); | |
70 | + } | |
59 | 71 | |
60 | 72 | const id = nanoid(); |
61 | 73 | |
| @@ -69,7 +81,7 @@ export const POST: APIRoute = withErrorHandler(async ({ params, request, locals | ||
69 | 81 | const [newState] = await (db as any).insert(schema.prStateDefinitions).values({ |
70 | 82 | id, |
71 | 83 | repositoryId: repo.id, |
72 | - name, | |
84 | + name: normalizedName, | |
73 | 85 | displayName, |
74 | 86 | color: color || "#6B7280", |
75 | 87 | description, |
76 | 88 | |
▼src/pages/api/repos/[owner]/[repo]/settings/states/[id].ts
+15-1
| @@ -45,6 +45,7 @@ export const PUT: APIRoute = withErrorHandler(async ({ params, request, locals } | ||
45 | 45 | return badRequest(parsed.error.issues[0]?.message || "Invalid state payload"); |
46 | 46 | } |
47 | 47 | const { name, displayName, color, description, icon, isFinal, allowMerge, requireCodeOwner, order, reviewers } = parsed.data; |
48 | + const normalizedName = name !== undefined ? name.trim().toLowerCase().replace(/\s+/g, "_") : undefined; | |
48 | 49 | |
49 | 50 | const existing = await db.query.prStateDefinitions.findFirst({ |
50 | 51 | where: and( |
| @@ -55,9 +56,22 @@ export const PUT: APIRoute = withErrorHandler(async ({ params, request, locals } | ||
55 | 56 | |
56 | 57 | if (!existing) return notFound("State not found"); |
57 | 58 | |
59 | + if (normalizedName !== undefined && normalizedName !== existing.name) { | |
60 | + const duplicate = await db.query.prStateDefinitions.findFirst({ | |
61 | + where: and( | |
62 | + eq(schema.prStateDefinitions.repositoryId, repo.id), | |
63 | + eq(schema.prStateDefinitions.name, normalizedName) | |
64 | + ), | |
65 | + columns: { id: true }, | |
66 | + }); | |
67 | + if (duplicate) { | |
68 | + return badRequest(`State '${normalizedName}' already exists`); | |
69 | + } | |
70 | + } | |
71 | + | |
58 | 72 | const [updated] = await (db as any).update(schema.prStateDefinitions) |
59 | 73 | .set({ |
60 | - name: name ?? existing.name, | |
74 | + name: normalizedName ?? existing.name, | |
61 | 75 | displayName: displayName ?? existing.displayName, |
62 | 76 | color: color ?? existing.color, |
63 | 77 | description: description ?? existing.description, |
64 | 78 | |
▼tests/integration/settings-states-duplicate-name.test.ts
+86-0
| @@ -0,0 +1,86 @@ | ||
1 | +import { beforeEach, describe, expect, it, vi } from "vitest"; | |
2 | + | |
3 | +const { getRepoPermissionMock, fakeSchema } = vi.hoisted(() => ({ | |
4 | + getRepoPermissionMock: vi.fn(async () => "admin"), | |
5 | + fakeSchema: { | |
6 | + prStateDefinitions: { repositoryId: {}, name: {}, id: {} }, | |
7 | + } as any, | |
8 | +})); | |
9 | + | |
10 | +let mockDb: any; | |
11 | + | |
12 | +vi.mock("@/db", () => ({ | |
13 | + getDatabase: () => mockDb, | |
14 | + schema: fakeSchema, | |
15 | +})); | |
16 | + | |
17 | +vi.mock("@/lib/permissions", () => ({ | |
18 | + getRepoPermission: getRepoPermissionMock, | |
19 | +})); | |
20 | + | |
21 | +import { POST as createState } from "@/pages/api/repos/[owner]/[repo]/settings/states"; | |
22 | +import { PUT as updateState } from "@/pages/api/repos/[owner]/[repo]/settings/states/[id]"; | |
23 | + | |
24 | +function makeDb() { | |
25 | + return { | |
26 | + query: { | |
27 | + prStateDefinitions: { | |
28 | + findFirst: vi.fn(async (_args?: any) => ({ id: "existing-state", name: "in_review" })), | |
29 | + }, | |
30 | + }, | |
31 | + insert: vi.fn(() => ({ | |
32 | + values: vi.fn(async () => ({ returning: vi.fn(async () => []) })), | |
33 | + })), | |
34 | + update: vi.fn(() => ({ | |
35 | + set: vi.fn(() => ({ | |
36 | + where: vi.fn(async () => []), | |
37 | + })), | |
38 | + })), | |
39 | + }; | |
40 | +} | |
41 | + | |
42 | +async function readJson(response: Response): Promise<any> { | |
43 | + return response.json(); | |
44 | +} | |
45 | + | |
46 | +describe("settings states duplicate protection", () => { | |
47 | + beforeEach(() => { | |
48 | + mockDb = makeDb(); | |
49 | + getRepoPermissionMock.mockResolvedValue("admin"); | |
50 | + }); | |
51 | + | |
52 | + it("rejects creating duplicate state name in repository", async () => { | |
53 | + const response = await createState({ | |
54 | + locals: { repo: { id: "repo-1" }, user: { id: "user-1" } }, | |
55 | + request: new Request("http://localhost/api/repos/owner/demo/settings/states", { | |
56 | + method: "POST", | |
57 | + headers: { "content-type": "application/json" }, | |
58 | + body: JSON.stringify({ name: "In Review", displayName: "In Review" }), | |
59 | + }), | |
60 | + } as any); | |
61 | + | |
62 | + const body = await readJson(response); | |
63 | + expect(response.status).toBe(400); | |
64 | + expect(body?.error?.code).toBe("BAD_REQUEST"); | |
65 | + }); | |
66 | + | |
67 | + it("rejects updating state to duplicate name in repository", async () => { | |
68 | + mockDb.query.prStateDefinitions.findFirst | |
69 | + .mockResolvedValueOnce({ id: "state-2", name: "todo" }) | |
70 | + .mockResolvedValueOnce({ id: "state-1", name: "in_review" }); | |
71 | + | |
72 | + const response = await updateState({ | |
73 | + params: { id: "state-2" }, | |
74 | + locals: { repo: { id: "repo-1" }, user: { id: "user-1" } }, | |
75 | + request: new Request("http://localhost/api/repos/owner/demo/settings/states/state-2", { | |
76 | + method: "PUT", | |
77 | + headers: { "content-type": "application/json" }, | |
78 | + body: JSON.stringify({ name: "In Review" }), | |
79 | + }), | |
80 | + } as any); | |
81 | + | |
82 | + const body = await readJson(response); | |
83 | + expect(response.status).toBe(400); | |
84 | + expect(body?.error?.code).toBe("BAD_REQUEST"); | |
85 | + }); | |
86 | +}); | |
0 | 87 | |