-
Notifications
You must be signed in to change notification settings - Fork 6
feat: add artist delete endpoint #421
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| import { NextRequest, NextResponse } from "next/server"; | ||
| import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; | ||
| import { deleteArtistHandler } from "@/lib/artists/deleteArtistHandler"; | ||
|
|
||
| /** | ||
| * OPTIONS handler for CORS preflight requests. | ||
| * | ||
| * @returns A NextResponse with CORS headers. | ||
| */ | ||
| export async function OPTIONS() { | ||
| return new NextResponse(null, { | ||
| status: 200, | ||
| headers: getCorsHeaders(), | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * DELETE /api/artists/{id} | ||
| * | ||
| * Removes the authenticated account's direct artist link and deletes the artist | ||
| * account if that link was the last remaining owner association. | ||
| * | ||
| * @param request - The request object | ||
| * @param options - Route options containing params | ||
| * @param options.params - Route params containing the artist account ID | ||
| * @returns A NextResponse with the delete result | ||
| */ | ||
| export async function DELETE(request: NextRequest, options: { params: Promise<{ id: string }> }) { | ||
| return deleteArtistHandler(request, options.params); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| import { describe, it, expect, vi, beforeEach } from "vitest"; | ||
| import { NextRequest, NextResponse } from "next/server"; | ||
|
|
||
| import { deleteArtistHandler } from "../deleteArtistHandler"; | ||
| import { validateDeleteArtistRequest } from "../validateDeleteArtistRequest"; | ||
| import { deleteArtist } from "../deleteArtist"; | ||
|
|
||
| vi.mock("@/lib/networking/getCorsHeaders", () => ({ | ||
| getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), | ||
| })); | ||
|
|
||
| vi.mock("../validateDeleteArtistRequest", () => ({ | ||
| validateDeleteArtistRequest: vi.fn(), | ||
| })); | ||
|
|
||
| vi.mock("../deleteArtist", () => ({ | ||
| deleteArtist: vi.fn(), | ||
| })); | ||
|
|
||
| describe("deleteArtistHandler", () => { | ||
| const artistId = "550e8400-e29b-41d4-a716-446655440000"; | ||
| const requesterAccountId = "660e8400-e29b-41d4-a716-446655440000"; | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it("returns the validation response when request validation fails", async () => { | ||
| const validationError = NextResponse.json({ error: "Unauthorized" }, { status: 401 }); | ||
| vi.mocked(validateDeleteArtistRequest).mockResolvedValue(validationError); | ||
|
|
||
| const request = new NextRequest(`http://localhost/api/artists/${artistId}`, { | ||
| method: "DELETE", | ||
| }); | ||
|
|
||
| const response = await deleteArtistHandler(request, Promise.resolve({ id: artistId })); | ||
|
|
||
| expect(response).toBe(validationError); | ||
| expect(deleteArtist).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("returns success when the artist is deleted", async () => { | ||
| vi.mocked(validateDeleteArtistRequest).mockResolvedValue({ | ||
| artistId, | ||
| requesterAccountId, | ||
| }); | ||
| vi.mocked(deleteArtist).mockResolvedValue(artistId); | ||
|
|
||
| const request = new NextRequest(`http://localhost/api/artists/${artistId}`, { | ||
| method: "DELETE", | ||
| }); | ||
|
|
||
| const response = await deleteArtistHandler(request, Promise.resolve({ id: artistId })); | ||
| const body = await response.json(); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| expect(body).toEqual({ | ||
| success: true, | ||
| artistId, | ||
| }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| import { describe, it, expect, vi, beforeEach } from "vitest"; | ||
| import { NextRequest, NextResponse } from "next/server"; | ||
|
|
||
| import { validateDeleteArtistRequest } from "../validateDeleteArtistRequest"; | ||
| import { validateAuthContext } from "@/lib/auth/validateAuthContext"; | ||
| import { selectAccounts } from "@/lib/supabase/accounts/selectAccounts"; | ||
| import { checkAccountArtistAccess } from "../checkAccountArtistAccess"; | ||
|
|
||
| vi.mock("@/lib/auth/validateAuthContext", () => ({ | ||
| validateAuthContext: vi.fn(), | ||
| })); | ||
|
|
||
| vi.mock("@/lib/supabase/accounts/selectAccounts", () => ({ | ||
| selectAccounts: vi.fn(), | ||
| })); | ||
|
|
||
| vi.mock("../checkAccountArtistAccess", () => ({ | ||
| checkAccountArtistAccess: vi.fn(), | ||
| })); | ||
|
|
||
| describe("validateDeleteArtistRequest", () => { | ||
| const validArtistId = "550e8400-e29b-41d4-a716-446655440000"; | ||
| const authenticatedAccountId = "660e8400-e29b-41d4-a716-446655440000"; | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it("returns a 400 response when the artist id is invalid", async () => { | ||
| const request = new NextRequest("http://localhost/api/artists/not-a-uuid", { | ||
| method: "DELETE", | ||
| headers: { | ||
| Authorization: "Bearer test-token", | ||
| }, | ||
| }); | ||
|
|
||
| const result = await validateDeleteArtistRequest(request, "not-a-uuid"); | ||
|
|
||
| expect(result).toBeInstanceOf(NextResponse); | ||
| expect((result as NextResponse).status).toBe(400); | ||
| expect(validateAuthContext).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("returns the auth error when authentication fails", async () => { | ||
| const authError = NextResponse.json({ error: "Unauthorized" }, { status: 401 }); | ||
| vi.mocked(validateAuthContext).mockResolvedValue(authError); | ||
|
|
||
| const request = new NextRequest(`http://localhost/api/artists/${validArtistId}`, { | ||
| method: "DELETE", | ||
| headers: { | ||
| Authorization: "Bearer test-token", | ||
| }, | ||
| }); | ||
|
|
||
| const result = await validateDeleteArtistRequest(request, validArtistId); | ||
|
|
||
| expect(result).toBe(authError); | ||
| expect(validateAuthContext).toHaveBeenCalledWith(request); | ||
| }); | ||
|
|
||
| it("returns 404 when the artist does not exist", async () => { | ||
| vi.mocked(validateAuthContext).mockResolvedValue({ | ||
| accountId: authenticatedAccountId, | ||
| authToken: "test-token", | ||
| orgId: null, | ||
| }); | ||
| vi.mocked(selectAccounts).mockResolvedValue([]); | ||
|
|
||
| const request = new NextRequest(`http://localhost/api/artists/${validArtistId}`, { | ||
| method: "DELETE", | ||
| headers: { | ||
| Authorization: "Bearer test-token", | ||
| }, | ||
| }); | ||
|
|
||
| const result = await validateDeleteArtistRequest(request, validArtistId); | ||
|
|
||
| expect(result).toBeInstanceOf(NextResponse); | ||
| expect((result as NextResponse).status).toBe(404); | ||
| expect(checkAccountArtistAccess).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("returns 403 when the requester cannot access the artist", async () => { | ||
| vi.mocked(validateAuthContext).mockResolvedValue({ | ||
| accountId: authenticatedAccountId, | ||
| authToken: "test-token", | ||
| orgId: null, | ||
| }); | ||
| vi.mocked(selectAccounts).mockResolvedValue([{ id: validArtistId }] as never); | ||
| vi.mocked(checkAccountArtistAccess).mockResolvedValue(false); | ||
|
|
||
| const request = new NextRequest(`http://localhost/api/artists/${validArtistId}`, { | ||
| method: "DELETE", | ||
| headers: { | ||
| Authorization: "Bearer test-token", | ||
| }, | ||
| }); | ||
|
|
||
| const result = await validateDeleteArtistRequest(request, validArtistId); | ||
|
|
||
| expect(result).toBeInstanceOf(NextResponse); | ||
| expect((result as NextResponse).status).toBe(403); | ||
| }); | ||
|
|
||
| it("returns the validated artist and requester account ids", async () => { | ||
| vi.mocked(validateAuthContext).mockResolvedValue({ | ||
| accountId: authenticatedAccountId, | ||
| authToken: "test-token", | ||
| orgId: null, | ||
| }); | ||
| vi.mocked(selectAccounts).mockResolvedValue([{ id: validArtistId }] as never); | ||
| vi.mocked(checkAccountArtistAccess).mockResolvedValue(true); | ||
|
|
||
| const request = new NextRequest(`http://localhost/api/artists/${validArtistId}`, { | ||
| method: "DELETE", | ||
| headers: { | ||
| Authorization: "Bearer test-token", | ||
| }, | ||
| }); | ||
|
|
||
| const result = await validateDeleteArtistRequest(request, validArtistId); | ||
|
|
||
| expect(result).toEqual({ | ||
| artistId: validArtistId, | ||
| requesterAccountId: authenticatedAccountId, | ||
| }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| import { deleteAccountArtistId } from "@/lib/supabase/account_artist_ids/deleteAccountArtistId"; | ||
| import { getAccountArtistIds } from "@/lib/supabase/account_artist_ids/getAccountArtistIds"; | ||
| import { deleteAccountById } from "@/lib/supabase/accounts/deleteAccountById"; | ||
|
|
||
| export interface DeleteArtistParams { | ||
| artistId: string; | ||
| requesterAccountId: string; | ||
| } | ||
|
|
||
| /** | ||
| * Deletes an artist for an already validated requester. | ||
| * | ||
| * The validator is responsible for existence and access checks. This helper | ||
| * only removes the direct owner link and deletes the artist account if that | ||
| * link was the last remaining association. | ||
| * | ||
| * @param params - Delete artist parameters | ||
| * @param params.artistId - Artist account ID to remove | ||
| * @param params.requesterAccountId - Authenticated account performing the delete | ||
| * @returns The deleted artist account ID | ||
| */ | ||
| export async function deleteArtist({ | ||
| artistId, | ||
| requesterAccountId, | ||
| }: DeleteArtistParams): Promise<string> { | ||
| const deletedLinks = await deleteAccountArtistId(requesterAccountId, artistId); | ||
| if (!deletedLinks.length) { | ||
| throw new Error("Failed to delete artist link"); | ||
| } | ||
|
|
||
| const remainingLinks = await getAccountArtistIds({ | ||
| artistIds: [artistId], | ||
| }); | ||
|
|
||
| if (remainingLinks.length === 0) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Prompt for AI agents |
||
| await deleteAccountById(artistId); | ||
| } | ||
|
Comment on lines
+31
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make orphan-check + account delete atomic. Line 31 and Line 35-36 perform a read-then-delete sequence without transaction boundaries. Under concurrency, duplicate or stale decisions are possible. Move this into one atomic DB operation (transaction/RPC) so “delete if no remaining links” is evaluated and executed together. 🤖 Prompt for AI Agents |
||
|
|
||
| return artistId; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| import { NextRequest, NextResponse } from "next/server"; | ||
| import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; | ||
| import { deleteArtist } from "@/lib/artists/deleteArtist"; | ||
| import { validateDeleteArtistRequest } from "@/lib/artists/validateDeleteArtistRequest"; | ||
|
|
||
| /** | ||
| * Handler for DELETE /api/artists/{id}. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Custom agent: API Design Consistency and Maintainability Rule 1 requires POST for actions/mutations, but this new handler is explicitly for a DELETE mutation ( Prompt for AI agents |
||
| * | ||
| * Removes the authenticated account's direct link to an artist. If that link | ||
| * was the last remaining owner link, the artist account is deleted as well. | ||
| * | ||
| * @param request - The incoming request | ||
| * @param params - Route params containing the artist account ID | ||
| * @returns A NextResponse with the delete result or an error | ||
| */ | ||
| export async function deleteArtistHandler( | ||
| request: NextRequest, | ||
| params: Promise<{ id: string }>, | ||
| ): Promise<NextResponse> { | ||
| try { | ||
| const { id } = await params; | ||
|
|
||
| const validated = await validateDeleteArtistRequest(request, id); | ||
| if (validated instanceof NextResponse) { | ||
| return validated; | ||
| } | ||
|
|
||
| const artistId = await deleteArtist(validated); | ||
|
|
||
| return NextResponse.json( | ||
| { | ||
| success: true, | ||
| artistId, | ||
| }, | ||
| { | ||
| status: 200, | ||
| headers: getCorsHeaders(), | ||
| }, | ||
| ); | ||
| } catch (error) { | ||
| console.error("[ERROR] deleteArtistHandler:", error); | ||
| return NextResponse.json( | ||
| { | ||
| status: "error", | ||
| error: error instanceof Error ? error.message : "Internal server error", | ||
| }, | ||
| { | ||
| status: 500, | ||
| headers: getCorsHeaders(), | ||
| }, | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| import { NextRequest, NextResponse } from "next/server"; | ||
| import { validateAccountParams } from "@/lib/accounts/validateAccountParams"; | ||
| import { validateAuthContext } from "@/lib/auth/validateAuthContext"; | ||
| import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; | ||
| import { checkAccountArtistAccess } from "@/lib/artists/checkAccountArtistAccess"; | ||
| import { selectAccounts } from "@/lib/supabase/accounts/selectAccounts"; | ||
|
|
||
| export interface DeleteArtistRequest { | ||
| artistId: string; | ||
| requesterAccountId: string; | ||
| } | ||
|
|
||
| /** | ||
| * Validates DELETE /api/artists/{id} path params and authentication. | ||
| * | ||
| * @param request - The incoming request | ||
| * @param id - The artist account ID from the route | ||
| * @returns The validated artist ID plus requester context, or a NextResponse error | ||
| */ | ||
| export async function validateDeleteArtistRequest( | ||
| request: NextRequest, | ||
| id: string, | ||
| ): Promise<DeleteArtistRequest | NextResponse> { | ||
| const validatedParams = validateAccountParams(id); | ||
| if (validatedParams instanceof NextResponse) { | ||
| return validatedParams; | ||
| } | ||
|
|
||
| const authResult = await validateAuthContext(request); | ||
| if (authResult instanceof NextResponse) { | ||
| return authResult; | ||
| } | ||
|
|
||
| const artistId = validatedParams.id; | ||
| const requesterAccountId = authResult.accountId; | ||
|
|
||
| const existingArtist = await selectAccounts(artistId); | ||
| if (!existingArtist.length) { | ||
| return NextResponse.json( | ||
| { | ||
| status: "error", | ||
| error: "Artist not found", | ||
| }, | ||
| { | ||
| status: 404, | ||
| headers: getCorsHeaders(), | ||
| }, | ||
| ); | ||
| } | ||
|
|
||
| const hasAccess = await checkAccountArtistAccess(requesterAccountId, artistId); | ||
| if (!hasAccess) { | ||
| return NextResponse.json( | ||
| { | ||
| status: "error", | ||
| error: "Unauthorized delete attempt", | ||
| }, | ||
| { | ||
| status: 403, | ||
| headers: getCorsHeaders(), | ||
| }, | ||
| ); | ||
| } | ||
|
Comment on lines
+51
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Enforce direct-link ownership here, not broad artist access.
Proposed change-import { checkAccountArtistAccess } from "@/lib/artists/checkAccountArtistAccess";
+import { selectAccountArtistId } from "@/lib/supabase/account_artist_ids/selectAccountArtistId";
@@
- const hasAccess = await checkAccountArtistAccess(requesterAccountId, artistId);
- if (!hasAccess) {
+ const directLink = await selectAccountArtistId(requesterAccountId, artistId);
+ if (!directLink) {
return NextResponse.json(
{
status: "error",
error: "Unauthorized delete attempt",
},🤖 Prompt for AI Agents |
||
|
|
||
| return { | ||
| artistId, | ||
| requesterAccountId, | ||
| }; | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Custom agent: Module should export a single primary function whose name matches the filename
This module exports multiple top-level functions (
OPTIONSandDELETE) instead of a single primary export matching the filenameroute, violating the single-export module rule. Split the handlers into separate files or rename the primary export to match the filename.Prompt for AI agents