From dc509592217bbe4edc9bb8cfc2db726d0878e7fd Mon Sep 17 00:00:00 2001 From: Arpit Gupta Date: Fri, 10 Apr 2026 05:23:14 +0530 Subject: [PATCH 1/2] feat: add artist delete endpoint --- app/api/artists/[id]/route.ts | 30 +++++ lib/artists/__tests__/deleteArtist.test.ts | 126 ++++++++++++++++++ .../__tests__/deleteArtistHandler.test.ts | 107 +++++++++++++++ .../validateDeleteArtistRequest.test.ts | 72 ++++++++++ lib/artists/deleteArtist.ts | 68 ++++++++++ lib/artists/deleteArtistHandler.ts | 79 +++++++++++ lib/artists/validateDeleteArtistRequest.ts | 35 +++++ .../deleteAccountArtistId.ts | 27 ++++ lib/supabase/accounts/deleteAccountById.ts | 19 +++ 9 files changed, 563 insertions(+) create mode 100644 app/api/artists/[id]/route.ts create mode 100644 lib/artists/__tests__/deleteArtist.test.ts create mode 100644 lib/artists/__tests__/deleteArtistHandler.test.ts create mode 100644 lib/artists/__tests__/validateDeleteArtistRequest.test.ts create mode 100644 lib/artists/deleteArtist.ts create mode 100644 lib/artists/deleteArtistHandler.ts create mode 100644 lib/artists/validateDeleteArtistRequest.ts create mode 100644 lib/supabase/account_artist_ids/deleteAccountArtistId.ts create mode 100644 lib/supabase/accounts/deleteAccountById.ts diff --git a/app/api/artists/[id]/route.ts b/app/api/artists/[id]/route.ts new file mode 100644 index 00000000..7376882d --- /dev/null +++ b/app/api/artists/[id]/route.ts @@ -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); +} diff --git a/lib/artists/__tests__/deleteArtist.test.ts b/lib/artists/__tests__/deleteArtist.test.ts new file mode 100644 index 00000000..15bae573 --- /dev/null +++ b/lib/artists/__tests__/deleteArtist.test.ts @@ -0,0 +1,126 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +import { deleteArtist } from "../deleteArtist"; +import { selectAccounts } from "@/lib/supabase/accounts/selectAccounts"; +import { checkAccountArtistAccess } from "../checkAccountArtistAccess"; +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"; + +vi.mock("@/lib/supabase/accounts/selectAccounts", () => ({ + selectAccounts: vi.fn(), +})); + +vi.mock("../checkAccountArtistAccess", () => ({ + checkAccountArtistAccess: vi.fn(), +})); + +vi.mock("@/lib/supabase/account_artist_ids/deleteAccountArtistId", () => ({ + deleteAccountArtistId: vi.fn(), +})); + +vi.mock("@/lib/supabase/account_artist_ids/getAccountArtistIds", () => ({ + getAccountArtistIds: vi.fn(), +})); + +vi.mock("@/lib/supabase/accounts/deleteAccountById", () => ({ + deleteAccountById: vi.fn(), +})); + +describe("deleteArtist", () => { + const requesterAccountId = "660e8400-e29b-41d4-a716-446655440000"; + const artistId = "550e8400-e29b-41d4-a716-446655440000"; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("returns not_found when the artist account does not exist", async () => { + vi.mocked(selectAccounts).mockResolvedValue([]); + + const result = await deleteArtist({ + artistId, + requesterAccountId, + }); + + expect(result).toEqual({ + ok: false, + code: "not_found", + }); + expect(checkAccountArtistAccess).not.toHaveBeenCalled(); + }); + + it("returns forbidden when the requester cannot access the artist", async () => { + vi.mocked(selectAccounts).mockResolvedValue([{ id: artistId }] as never); + vi.mocked(checkAccountArtistAccess).mockResolvedValue(false); + + const result = await deleteArtist({ + artistId, + requesterAccountId, + }); + + expect(result).toEqual({ + ok: false, + code: "forbidden", + }); + expect(deleteAccountArtistId).not.toHaveBeenCalled(); + }); + + it("returns forbidden when the requester has access but no direct artist link to delete", async () => { + vi.mocked(selectAccounts).mockResolvedValue([{ id: artistId }] as never); + vi.mocked(checkAccountArtistAccess).mockResolvedValue(true); + vi.mocked(deleteAccountArtistId).mockResolvedValue([]); + + const result = await deleteArtist({ + artistId, + requesterAccountId, + }); + + expect(result).toEqual({ + ok: false, + code: "forbidden", + }); + expect(getAccountArtistIds).not.toHaveBeenCalled(); + }); + + it("removes only the requester link when other links still exist", async () => { + vi.mocked(selectAccounts).mockResolvedValue([{ id: artistId }] as never); + vi.mocked(checkAccountArtistAccess).mockResolvedValue(true); + vi.mocked(deleteAccountArtistId).mockResolvedValue([ + { account_id: requesterAccountId, artist_id: artistId }, + ] as never); + vi.mocked(getAccountArtistIds).mockResolvedValue([{ artist_id: artistId }] as never); + + const result = await deleteArtist({ + artistId, + requesterAccountId, + }); + + expect(result).toEqual({ + ok: true, + artistId, + }); + expect(deleteAccountById).not.toHaveBeenCalled(); + }); + + it("deletes the artist account when no links remain", async () => { + vi.mocked(selectAccounts).mockResolvedValue([{ id: artistId }] as never); + vi.mocked(checkAccountArtistAccess).mockResolvedValue(true); + vi.mocked(deleteAccountArtistId).mockResolvedValue([ + { account_id: requesterAccountId, artist_id: artistId }, + ] as never); + vi.mocked(getAccountArtistIds).mockResolvedValue([]); + vi.mocked(deleteAccountById).mockResolvedValue(true); + + const result = await deleteArtist({ + artistId, + requesterAccountId, + }); + + expect(result).toEqual({ + ok: true, + artistId, + }); + expect(deleteAccountById).toHaveBeenCalledWith(artistId); + }); +}); diff --git a/lib/artists/__tests__/deleteArtistHandler.test.ts b/lib/artists/__tests__/deleteArtistHandler.test.ts new file mode 100644 index 00000000..c132163b --- /dev/null +++ b/lib/artists/__tests__/deleteArtistHandler.test.ts @@ -0,0 +1,107 @@ +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 404 when the artist does not exist", async () => { + vi.mocked(validateDeleteArtistRequest).mockResolvedValue({ + artistId, + requesterAccountId, + }); + vi.mocked(deleteArtist).mockResolvedValue({ + ok: false, + code: "not_found", + }); + + 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(404); + expect(body.error).toBe("Artist not found"); + }); + + it("returns 403 when the delete is not authorized", async () => { + vi.mocked(validateDeleteArtistRequest).mockResolvedValue({ + artistId, + requesterAccountId, + }); + vi.mocked(deleteArtist).mockResolvedValue({ + ok: false, + code: "forbidden", + }); + + 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(403); + expect(body.error).toBe("Unauthorized delete attempt"); + }); + + it("returns success when the artist is deleted", async () => { + vi.mocked(validateDeleteArtistRequest).mockResolvedValue({ + artistId, + requesterAccountId, + }); + vi.mocked(deleteArtist).mockResolvedValue({ + ok: true, + 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, + }); + }); +}); diff --git a/lib/artists/__tests__/validateDeleteArtistRequest.test.ts b/lib/artists/__tests__/validateDeleteArtistRequest.test.ts new file mode 100644 index 00000000..fe2175b2 --- /dev/null +++ b/lib/artists/__tests__/validateDeleteArtistRequest.test.ts @@ -0,0 +1,72 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; + +import { validateDeleteArtistRequest } from "../validateDeleteArtistRequest"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; + +vi.mock("@/lib/auth/validateAuthContext", () => ({ + validateAuthContext: 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 the validated artist and requester account ids", async () => { + vi.mocked(validateAuthContext).mockResolvedValue({ + accountId: authenticatedAccountId, + authToken: "test-token", + orgId: null, + }); + + 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, + }); + }); +}); diff --git a/lib/artists/deleteArtist.ts b/lib/artists/deleteArtist.ts new file mode 100644 index 00000000..7313108d --- /dev/null +++ b/lib/artists/deleteArtist.ts @@ -0,0 +1,68 @@ +import { checkAccountArtistAccess } from "@/lib/artists/checkAccountArtistAccess"; +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"; +import { selectAccounts } from "@/lib/supabase/accounts/selectAccounts"; + +export interface DeleteArtistParams { + artistId: string; + requesterAccountId: string; +} + +export type DeleteArtistResult = + | { ok: true; artistId: string } + | { ok: false; code: "forbidden" | "not_found" }; + +/** + * Deletes an artist for the authenticated requester. + * + * The requester must be able to access the artist and must also have a direct + * account_artist_ids link to remove. If the deleted link was the last remaining + * artist link, the artist account itself is deleted as well. + * + * @param params - Delete artist parameters + * @param params.artistId - Artist account ID to remove + * @param params.requesterAccountId - Authenticated account performing the delete + * @returns Delete result describing whether the artist was removed + */ +export async function deleteArtist({ + artistId, + requesterAccountId, +}: DeleteArtistParams): Promise { + const existingArtist = await selectAccounts(artistId); + if (!existingArtist.length) { + return { + ok: false, + code: "not_found", + }; + } + + const hasAccess = await checkAccountArtistAccess(requesterAccountId, artistId); + if (!hasAccess) { + return { + ok: false, + code: "forbidden", + }; + } + + const deletedLinks = await deleteAccountArtistId(requesterAccountId, artistId); + if (!deletedLinks.length) { + return { + ok: false, + code: "forbidden", + }; + } + + const remainingLinks = await getAccountArtistIds({ + artistIds: [artistId], + }); + + if (remainingLinks.length === 0) { + await deleteAccountById(artistId); + } + + return { + ok: true, + artistId, + }; +} diff --git a/lib/artists/deleteArtistHandler.ts b/lib/artists/deleteArtistHandler.ts new file mode 100644 index 00000000..d5692cca --- /dev/null +++ b/lib/artists/deleteArtistHandler.ts @@ -0,0 +1,79 @@ +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}. + * + * 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 { + try { + const { id } = await params; + + const validated = await validateDeleteArtistRequest(request, id); + if (validated instanceof NextResponse) { + return validated; + } + + const result = await deleteArtist(validated); + + if (!result.ok && result.code === "not_found") { + return NextResponse.json( + { + status: "error", + error: "Artist not found", + }, + { + status: 404, + headers: getCorsHeaders(), + }, + ); + } + + if (!result.ok && result.code === "forbidden") { + return NextResponse.json( + { + status: "error", + error: "Unauthorized delete attempt", + }, + { + status: 403, + headers: getCorsHeaders(), + }, + ); + } + + return NextResponse.json( + { + success: true, + artistId: result.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(), + }, + ); + } +} diff --git a/lib/artists/validateDeleteArtistRequest.ts b/lib/artists/validateDeleteArtistRequest.ts new file mode 100644 index 00000000..75289ded --- /dev/null +++ b/lib/artists/validateDeleteArtistRequest.ts @@ -0,0 +1,35 @@ +import { NextRequest, NextResponse } from "next/server"; +import { validateAccountParams } from "@/lib/accounts/validateAccountParams"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; + +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 { + const validatedParams = validateAccountParams(id); + if (validatedParams instanceof NextResponse) { + return validatedParams; + } + + const authResult = await validateAuthContext(request); + if (authResult instanceof NextResponse) { + return authResult; + } + + return { + artistId: validatedParams.id, + requesterAccountId: authResult.accountId, + }; +} diff --git a/lib/supabase/account_artist_ids/deleteAccountArtistId.ts b/lib/supabase/account_artist_ids/deleteAccountArtistId.ts new file mode 100644 index 00000000..294763fa --- /dev/null +++ b/lib/supabase/account_artist_ids/deleteAccountArtistId.ts @@ -0,0 +1,27 @@ +import supabase from "../serverClient"; +import type { Tables } from "@/types/database.types"; + +/** + * Deletes a direct account-artist relationship. + * + * @param accountId - The requester account ID + * @param artistId - The artist account ID + * @returns Deleted account_artist_ids rows + */ +export async function deleteAccountArtistId( + accountId: string, + artistId: string, +): Promise[]> { + const { data, error } = await supabase + .from("account_artist_ids") + .delete() + .eq("account_id", accountId) + .eq("artist_id", artistId) + .select("*"); + + if (error) { + throw new Error(`Failed to delete account-artist relationship: ${error.message}`); + } + + return data || []; +} diff --git a/lib/supabase/accounts/deleteAccountById.ts b/lib/supabase/accounts/deleteAccountById.ts new file mode 100644 index 00000000..c5b513a6 --- /dev/null +++ b/lib/supabase/accounts/deleteAccountById.ts @@ -0,0 +1,19 @@ +import supabase from "../serverClient"; + +/** + * Deletes an account by ID. + * + * Related rows are removed by the database's foreign key cascade rules. + * + * @param accountId - The account ID to delete + * @returns True when the delete succeeds + */ +export async function deleteAccountById(accountId: string): Promise { + const { error } = await supabase.from("accounts").delete().eq("id", accountId); + + if (error) { + throw new Error(`Failed to delete account: ${error.message}`); + } + + return true; +} From 79fba4078bc76a4feb1d9370cbc0e91bcf7dee77 Mon Sep 17 00:00:00 2001 From: Arpit Gupta Date: Fri, 10 Apr 2026 05:54:08 +0530 Subject: [PATCH 2/2] refactor: validate artist delete access in request validator --- lib/artists/__tests__/deleteArtist.test.ts | 126 ------------------ .../__tests__/deleteArtistHandler.test.ts | 47 +------ .../validateDeleteArtistRequest.test.ts | 56 ++++++++ lib/artists/deleteArtist.ts | 44 ++---- lib/artists/deleteArtistHandler.ts | 30 +---- lib/artists/validateDeleteArtistRequest.ts | 38 +++++- 6 files changed, 103 insertions(+), 238 deletions(-) delete mode 100644 lib/artists/__tests__/deleteArtist.test.ts diff --git a/lib/artists/__tests__/deleteArtist.test.ts b/lib/artists/__tests__/deleteArtist.test.ts deleted file mode 100644 index 15bae573..00000000 --- a/lib/artists/__tests__/deleteArtist.test.ts +++ /dev/null @@ -1,126 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; - -import { deleteArtist } from "../deleteArtist"; -import { selectAccounts } from "@/lib/supabase/accounts/selectAccounts"; -import { checkAccountArtistAccess } from "../checkAccountArtistAccess"; -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"; - -vi.mock("@/lib/supabase/accounts/selectAccounts", () => ({ - selectAccounts: vi.fn(), -})); - -vi.mock("../checkAccountArtistAccess", () => ({ - checkAccountArtistAccess: vi.fn(), -})); - -vi.mock("@/lib/supabase/account_artist_ids/deleteAccountArtistId", () => ({ - deleteAccountArtistId: vi.fn(), -})); - -vi.mock("@/lib/supabase/account_artist_ids/getAccountArtistIds", () => ({ - getAccountArtistIds: vi.fn(), -})); - -vi.mock("@/lib/supabase/accounts/deleteAccountById", () => ({ - deleteAccountById: vi.fn(), -})); - -describe("deleteArtist", () => { - const requesterAccountId = "660e8400-e29b-41d4-a716-446655440000"; - const artistId = "550e8400-e29b-41d4-a716-446655440000"; - - beforeEach(() => { - vi.clearAllMocks(); - }); - - it("returns not_found when the artist account does not exist", async () => { - vi.mocked(selectAccounts).mockResolvedValue([]); - - const result = await deleteArtist({ - artistId, - requesterAccountId, - }); - - expect(result).toEqual({ - ok: false, - code: "not_found", - }); - expect(checkAccountArtistAccess).not.toHaveBeenCalled(); - }); - - it("returns forbidden when the requester cannot access the artist", async () => { - vi.mocked(selectAccounts).mockResolvedValue([{ id: artistId }] as never); - vi.mocked(checkAccountArtistAccess).mockResolvedValue(false); - - const result = await deleteArtist({ - artistId, - requesterAccountId, - }); - - expect(result).toEqual({ - ok: false, - code: "forbidden", - }); - expect(deleteAccountArtistId).not.toHaveBeenCalled(); - }); - - it("returns forbidden when the requester has access but no direct artist link to delete", async () => { - vi.mocked(selectAccounts).mockResolvedValue([{ id: artistId }] as never); - vi.mocked(checkAccountArtistAccess).mockResolvedValue(true); - vi.mocked(deleteAccountArtistId).mockResolvedValue([]); - - const result = await deleteArtist({ - artistId, - requesterAccountId, - }); - - expect(result).toEqual({ - ok: false, - code: "forbidden", - }); - expect(getAccountArtistIds).not.toHaveBeenCalled(); - }); - - it("removes only the requester link when other links still exist", async () => { - vi.mocked(selectAccounts).mockResolvedValue([{ id: artistId }] as never); - vi.mocked(checkAccountArtistAccess).mockResolvedValue(true); - vi.mocked(deleteAccountArtistId).mockResolvedValue([ - { account_id: requesterAccountId, artist_id: artistId }, - ] as never); - vi.mocked(getAccountArtistIds).mockResolvedValue([{ artist_id: artistId }] as never); - - const result = await deleteArtist({ - artistId, - requesterAccountId, - }); - - expect(result).toEqual({ - ok: true, - artistId, - }); - expect(deleteAccountById).not.toHaveBeenCalled(); - }); - - it("deletes the artist account when no links remain", async () => { - vi.mocked(selectAccounts).mockResolvedValue([{ id: artistId }] as never); - vi.mocked(checkAccountArtistAccess).mockResolvedValue(true); - vi.mocked(deleteAccountArtistId).mockResolvedValue([ - { account_id: requesterAccountId, artist_id: artistId }, - ] as never); - vi.mocked(getAccountArtistIds).mockResolvedValue([]); - vi.mocked(deleteAccountById).mockResolvedValue(true); - - const result = await deleteArtist({ - artistId, - requesterAccountId, - }); - - expect(result).toEqual({ - ok: true, - artistId, - }); - expect(deleteAccountById).toHaveBeenCalledWith(artistId); - }); -}); diff --git a/lib/artists/__tests__/deleteArtistHandler.test.ts b/lib/artists/__tests__/deleteArtistHandler.test.ts index c132163b..ac452668 100644 --- a/lib/artists/__tests__/deleteArtistHandler.test.ts +++ b/lib/artists/__tests__/deleteArtistHandler.test.ts @@ -39,57 +39,12 @@ describe("deleteArtistHandler", () => { expect(deleteArtist).not.toHaveBeenCalled(); }); - it("returns 404 when the artist does not exist", async () => { - vi.mocked(validateDeleteArtistRequest).mockResolvedValue({ - artistId, - requesterAccountId, - }); - vi.mocked(deleteArtist).mockResolvedValue({ - ok: false, - code: "not_found", - }); - - 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(404); - expect(body.error).toBe("Artist not found"); - }); - - it("returns 403 when the delete is not authorized", async () => { - vi.mocked(validateDeleteArtistRequest).mockResolvedValue({ - artistId, - requesterAccountId, - }); - vi.mocked(deleteArtist).mockResolvedValue({ - ok: false, - code: "forbidden", - }); - - 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(403); - expect(body.error).toBe("Unauthorized delete attempt"); - }); - it("returns success when the artist is deleted", async () => { vi.mocked(validateDeleteArtistRequest).mockResolvedValue({ artistId, requesterAccountId, }); - vi.mocked(deleteArtist).mockResolvedValue({ - ok: true, - artistId, - }); + vi.mocked(deleteArtist).mockResolvedValue(artistId); const request = new NextRequest(`http://localhost/api/artists/${artistId}`, { method: "DELETE", diff --git a/lib/artists/__tests__/validateDeleteArtistRequest.test.ts b/lib/artists/__tests__/validateDeleteArtistRequest.test.ts index fe2175b2..c0585f9f 100644 --- a/lib/artists/__tests__/validateDeleteArtistRequest.test.ts +++ b/lib/artists/__tests__/validateDeleteArtistRequest.test.ts @@ -3,11 +3,21 @@ 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"; @@ -48,12 +58,58 @@ describe("validateDeleteArtistRequest", () => { 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", diff --git a/lib/artists/deleteArtist.ts b/lib/artists/deleteArtist.ts index 7313108d..330826b3 100644 --- a/lib/artists/deleteArtist.ts +++ b/lib/artists/deleteArtist.ts @@ -1,56 +1,31 @@ -import { checkAccountArtistAccess } from "@/lib/artists/checkAccountArtistAccess"; 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"; -import { selectAccounts } from "@/lib/supabase/accounts/selectAccounts"; export interface DeleteArtistParams { artistId: string; requesterAccountId: string; } -export type DeleteArtistResult = - | { ok: true; artistId: string } - | { ok: false; code: "forbidden" | "not_found" }; - /** - * Deletes an artist for the authenticated requester. + * Deletes an artist for an already validated requester. * - * The requester must be able to access the artist and must also have a direct - * account_artist_ids link to remove. If the deleted link was the last remaining - * artist link, the artist account itself is deleted as well. + * 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 Delete result describing whether the artist was removed + * @returns The deleted artist account ID */ export async function deleteArtist({ artistId, requesterAccountId, -}: DeleteArtistParams): Promise { - const existingArtist = await selectAccounts(artistId); - if (!existingArtist.length) { - return { - ok: false, - code: "not_found", - }; - } - - const hasAccess = await checkAccountArtistAccess(requesterAccountId, artistId); - if (!hasAccess) { - return { - ok: false, - code: "forbidden", - }; - } - +}: DeleteArtistParams): Promise { const deletedLinks = await deleteAccountArtistId(requesterAccountId, artistId); if (!deletedLinks.length) { - return { - ok: false, - code: "forbidden", - }; + throw new Error("Failed to delete artist link"); } const remainingLinks = await getAccountArtistIds({ @@ -61,8 +36,5 @@ export async function deleteArtist({ await deleteAccountById(artistId); } - return { - ok: true, - artistId, - }; + return artistId; } diff --git a/lib/artists/deleteArtistHandler.ts b/lib/artists/deleteArtistHandler.ts index d5692cca..10aad609 100644 --- a/lib/artists/deleteArtistHandler.ts +++ b/lib/artists/deleteArtistHandler.ts @@ -25,38 +25,12 @@ export async function deleteArtistHandler( return validated; } - const result = await deleteArtist(validated); - - if (!result.ok && result.code === "not_found") { - return NextResponse.json( - { - status: "error", - error: "Artist not found", - }, - { - status: 404, - headers: getCorsHeaders(), - }, - ); - } - - if (!result.ok && result.code === "forbidden") { - return NextResponse.json( - { - status: "error", - error: "Unauthorized delete attempt", - }, - { - status: 403, - headers: getCorsHeaders(), - }, - ); - } + const artistId = await deleteArtist(validated); return NextResponse.json( { success: true, - artistId: result.artistId, + artistId, }, { status: 200, diff --git a/lib/artists/validateDeleteArtistRequest.ts b/lib/artists/validateDeleteArtistRequest.ts index 75289ded..a69832e4 100644 --- a/lib/artists/validateDeleteArtistRequest.ts +++ b/lib/artists/validateDeleteArtistRequest.ts @@ -1,6 +1,9 @@ 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; @@ -28,8 +31,39 @@ export async function validateDeleteArtistRequest( 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(), + }, + ); + } + return { - artistId: validatedParams.id, - requesterAccountId: authResult.accountId, + artistId, + requesterAccountId, }; }