diff --git a/lib/artists/__tests__/createArtistPostHandler.test.ts b/lib/artists/__tests__/createArtistPostHandler.test.ts index e63d244d..475f4fde 100644 --- a/lib/artists/__tests__/createArtistPostHandler.test.ts +++ b/lib/artists/__tests__/createArtistPostHandler.test.ts @@ -3,11 +3,11 @@ import { NextRequest, NextResponse } from "next/server"; import { createArtistPostHandler } from "../createArtistPostHandler"; -const mockCreateArtistInDb = vi.fn(); +const mockResolveOrCreateArtist = vi.fn(); const mockValidateAuthContext = vi.fn(); -vi.mock("@/lib/artists/createArtistInDb", () => ({ - createArtistInDb: (...args: unknown[]) => mockCreateArtistInDb(...args), +vi.mock("@/lib/artists/resolveOrCreateArtist", () => ({ + resolveOrCreateArtist: (...args: unknown[]) => mockResolveOrCreateArtist(...args), })); vi.mock("@/lib/auth/validateAuthContext", () => ({ @@ -45,7 +45,7 @@ describe("createArtistPostHandler", () => { account_info: [{ image: null }], account_socials: [], }; - mockCreateArtistInDb.mockResolvedValue(mockArtist); + mockResolveOrCreateArtist.mockResolvedValue({ artist: mockArtist, created: true }); const request = createRequest({ name: "Test Artist" }); const response = await createArtistPostHandler(request); @@ -53,10 +53,8 @@ describe("createArtistPostHandler", () => { expect(response.status).toBe(201); expect(data.artist).toEqual(mockArtist); - expect(mockCreateArtistInDb).toHaveBeenCalledWith( - "Test Artist", - "api-key-account-id", - undefined, + expect(mockResolveOrCreateArtist).toHaveBeenCalledWith( + expect.objectContaining({ name: "Test Artist", accountId: "api-key-account-id" }), ); }); @@ -74,7 +72,7 @@ describe("createArtistPostHandler", () => { account_info: [{ image: null }], account_socials: [], }; - mockCreateArtistInDb.mockResolvedValue(mockArtist); + mockResolveOrCreateArtist.mockResolvedValue({ artist: mockArtist, created: true }); const request = createRequest({ name: "Test Artist", @@ -82,10 +80,11 @@ describe("createArtistPostHandler", () => { }); const response = await createArtistPostHandler(request); - expect(mockCreateArtistInDb).toHaveBeenCalledWith( - "Test Artist", - "550e8400-e29b-41d4-a716-446655440000", - undefined, + expect(mockResolveOrCreateArtist).toHaveBeenCalledWith( + expect.objectContaining({ + name: "Test Artist", + accountId: "550e8400-e29b-41d4-a716-446655440000", + }), ); expect(response.status).toBe(201); }); @@ -115,7 +114,7 @@ describe("createArtistPostHandler", () => { account_info: [{ image: null }], account_socials: [], }; - mockCreateArtistInDb.mockResolvedValue(mockArtist); + mockResolveOrCreateArtist.mockResolvedValue({ artist: mockArtist, created: true }); const request = createRequest({ name: "Test Artist", @@ -124,10 +123,8 @@ describe("createArtistPostHandler", () => { await createArtistPostHandler(request); - expect(mockCreateArtistInDb).toHaveBeenCalledWith( - "Test Artist", - "api-key-account-id", - "660e8400-e29b-41d4-a716-446655440001", + expect(mockResolveOrCreateArtist).toHaveBeenCalledWith( + expect.objectContaining({ name: "Test Artist", accountId: "api-key-account-id" }), ); }); @@ -178,7 +175,7 @@ describe("createArtistPostHandler", () => { }); it("returns 500 when artist creation fails", async () => { - mockCreateArtistInDb.mockResolvedValue(null); + mockResolveOrCreateArtist.mockResolvedValue({ artist: null, created: true }); const request = createRequest({ name: "Test Artist" }); const response = await createArtistPostHandler(request); @@ -189,7 +186,7 @@ describe("createArtistPostHandler", () => { }); it("returns 500 with error message when exception thrown", async () => { - mockCreateArtistInDb.mockRejectedValue(new Error("Database error")); + mockResolveOrCreateArtist.mockRejectedValue(new Error("Database error")); const request = createRequest({ name: "Test Artist" }); const response = await createArtistPostHandler(request); @@ -198,4 +195,32 @@ describe("createArtistPostHandler", () => { expect(response.status).toBe(500); expect(data.error).toBe("Database error"); }); + // Row 8 (chat#1889): 200 = existing canonical linked, 201 = created. + it("returns 200 when an existing canonical was linked instead of created", async () => { + mockResolveOrCreateArtist.mockResolvedValue({ + artist: { id: "canonical-1", account_id: "canonical-1", name: "Del Water Gap" }, + created: false, + }); + const response = await createArtistPostHandler( + createRequest({ name: "Del Water Gap", spotify_artist_id: "0xPoVNPnxIIUS1vrxAYV00" }), + ); + + expect(response.status).toBe(200); + const json = await response.json(); + expect(json.artist.account_id).toBe("canonical-1"); + }); + + it("passes the spotify id through to resolveOrCreateArtist", async () => { + mockResolveOrCreateArtist.mockResolvedValue({ + artist: { id: "new-1", account_id: "new-1", name: "X" }, + created: true, + }); + await createArtistPostHandler( + createRequest({ name: "X", spotify_artist_id: "0xPoVNPnxIIUS1vrxAYV00" }), + ); + + expect(mockResolveOrCreateArtist).toHaveBeenCalledWith( + expect.objectContaining({ spotifyArtistId: "0xPoVNPnxIIUS1vrxAYV00" }), + ); + }); }); diff --git a/lib/artists/__tests__/resolveOrCreateArtist.test.ts b/lib/artists/__tests__/resolveOrCreateArtist.test.ts new file mode 100644 index 00000000..b34e9723 --- /dev/null +++ b/lib/artists/__tests__/resolveOrCreateArtist.test.ts @@ -0,0 +1,107 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { resolveOrCreateArtist } from "@/lib/artists/resolveOrCreateArtist"; +import { createArtistInDb } from "@/lib/artists/createArtistInDb"; +import { findCanonicalArtistBySpotifyId } from "@/lib/valuation/findCanonicalArtistBySpotifyId"; +import { selectAccountArtistId } from "@/lib/supabase/account_artist_ids/selectAccountArtistId"; +import { insertAccountArtistId } from "@/lib/supabase/account_artist_ids/insertAccountArtistId"; +import { selectAccountWithSocials } from "@/lib/supabase/accounts/selectAccountWithSocials"; +import { updateArtistSocials } from "@/lib/artist/updateArtistSocials"; + +vi.mock("@/lib/artists/createArtistInDb", () => ({ createArtistInDb: vi.fn() })); +vi.mock("@/lib/valuation/findCanonicalArtistBySpotifyId", () => ({ + findCanonicalArtistBySpotifyId: vi.fn(), +})); +vi.mock("@/lib/supabase/account_artist_ids/selectAccountArtistId", () => ({ + selectAccountArtistId: vi.fn(), +})); +vi.mock("@/lib/supabase/account_artist_ids/insertAccountArtistId", () => ({ + insertAccountArtistId: vi.fn(), +})); +vi.mock("@/lib/supabase/accounts/selectAccountWithSocials", () => ({ + selectAccountWithSocials: vi.fn(), +})); +vi.mock("@/lib/artist/updateArtistSocials", () => ({ updateArtistSocials: vi.fn() })); + +const SPOTIFY_ID = "0xPoVNPnxIIUS1vrxAYV00"; +const created = { id: "new-1", account_id: "new-1", name: "Del Water Gap" }; + +describe("resolveOrCreateArtist", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(createArtistInDb).mockResolvedValue(created as never); + vi.mocked(findCanonicalArtistBySpotifyId).mockResolvedValue(null); + vi.mocked(selectAccountArtistId).mockResolvedValue(null as never); + vi.mocked(selectAccountWithSocials).mockResolvedValue({ + id: "canonical-1", + name: "Del Water Gap", + } as never); + }); + + it("creates and attaches the Spotify social when no canonical exists", async () => { + const result = await resolveOrCreateArtist({ + name: "Del Water Gap", + accountId: "acct-1", + spotifyArtistId: SPOTIFY_ID, + }); + + expect(createArtistInDb).toHaveBeenCalledWith("Del Water Gap", "acct-1", undefined); + expect(updateArtistSocials).toHaveBeenCalledWith("new-1", { + SPOTIFY: `https://open.spotify.com/artist/${SPOTIFY_ID}`, + }); + expect(result).toEqual({ artist: created, created: true }); + }); + + // One canonical artist per Spotify id (chat#1889, decision 2026-07-29): + // when it exists, link it to the account — never mint a second row. + it("links and returns the existing canonical instead of creating", async () => { + vi.mocked(findCanonicalArtistBySpotifyId).mockResolvedValue("canonical-1"); + + const result = await resolveOrCreateArtist({ + name: "Del Water Gap", + accountId: "acct-1", + spotifyArtistId: SPOTIFY_ID, + }); + + expect(createArtistInDb).not.toHaveBeenCalled(); + expect(updateArtistSocials).not.toHaveBeenCalled(); + expect(insertAccountArtistId).toHaveBeenCalledWith("acct-1", "canonical-1"); + expect(result.created).toBe(false); + expect(result.artist).toMatchObject({ id: "canonical-1", account_id: "canonical-1" }); + }); + + it("does not re-link a canonical the account already rosters", async () => { + vi.mocked(findCanonicalArtistBySpotifyId).mockResolvedValue("canonical-1"); + vi.mocked(selectAccountArtistId).mockResolvedValue({ id: "link-1" } as never); + + const result = await resolveOrCreateArtist({ + name: "Del Water Gap", + accountId: "acct-1", + spotifyArtistId: SPOTIFY_ID, + }); + + expect(insertAccountArtistId).not.toHaveBeenCalled(); + expect(result.created).toBe(false); + }); + + it("plain create path is untouched when no spotify id is given", async () => { + const result = await resolveOrCreateArtist({ name: "X", accountId: "acct-1" }); + + expect(findCanonicalArtistBySpotifyId).not.toHaveBeenCalled(); + expect(updateArtistSocials).not.toHaveBeenCalled(); + expect(result).toEqual({ artist: created, created: true }); + }); + + // Enrichment is non-fatal (same contract as chat#1892): the row exists by + // the time the social attach runs, so a failed attach must not fail the add. + it("returns the created artist when the social attach fails", async () => { + vi.mocked(updateArtistSocials).mockRejectedValue(new Error("nope")); + + const result = await resolveOrCreateArtist({ + name: "Del Water Gap", + accountId: "acct-1", + spotifyArtistId: SPOTIFY_ID, + }); + + expect(result).toEqual({ artist: created, created: true }); + }); +}); diff --git a/lib/artists/__tests__/validateCreateArtistBody.test.ts b/lib/artists/__tests__/validateCreateArtistBody.test.ts index 4de5562b..91b49a01 100644 --- a/lib/artists/__tests__/validateCreateArtistBody.test.ts +++ b/lib/artists/__tests__/validateCreateArtistBody.test.ts @@ -253,4 +253,26 @@ describe("validateCreateArtistBody", () => { }); }); }); + // Row 8 (chat#1889): spotify_artist_id rides the create so the server can + // resolve-or-create the canonical instead of minting a duplicate per signup. + it("passes spotify_artist_id through when provided", async () => { + const request = createRequest( + { name: "Del Water Gap", spotify_artist_id: "0xPoVNPnxIIUS1vrxAYV00" }, + { "x-api-key": "k" }, + ); + const result = await validateCreateArtistBody(request); + + expect(result).not.toBeInstanceOf(NextResponse); + if (!(result instanceof NextResponse)) { + expect(result.spotifyArtistId).toBe("0xPoVNPnxIIUS1vrxAYV00"); + } + }); + + it("rejects an empty spotify_artist_id", async () => { + const request = createRequest({ name: "X", spotify_artist_id: "" }, { "x-api-key": "k" }); + const result = await validateCreateArtistBody(request); + + expect(result).toBeInstanceOf(NextResponse); + if (result instanceof NextResponse) expect(result.status).toBe(400); + }); }); diff --git a/lib/artists/createArtistPostHandler.ts b/lib/artists/createArtistPostHandler.ts index 80cb3adf..439945ee 100644 --- a/lib/artists/createArtistPostHandler.ts +++ b/lib/artists/createArtistPostHandler.ts @@ -1,7 +1,7 @@ import { NextRequest, NextResponse } from "next/server"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { validateCreateArtistBody } from "@/lib/artists/validateCreateArtistBody"; -import { createArtistInDb } from "@/lib/artists/createArtistInDb"; +import { resolveOrCreateArtist } from "@/lib/artists/resolveOrCreateArtist"; /** * Handler for POST /api/artists. @@ -26,11 +26,12 @@ export async function createArtistPostHandler(request: NextRequest): Promise { + const { name, accountId, organizationId, spotifyArtistId } = params; + + if (spotifyArtistId) { + const canonicalId = await findCanonicalArtistBySpotifyId(spotifyArtistId); + if (canonicalId) { + const alreadyRostered = await selectAccountArtistId(accountId, canonicalId); + if (!alreadyRostered) { + await insertAccountArtistId(accountId, canonicalId); + } + const artist = await selectAccountWithSocials(canonicalId); + return { + artist: artist ? { ...artist, account_id: artist.id } : null, + created: false, + }; + } + } + + const created = await createArtistInDb(name, accountId, organizationId); + + if (created && spotifyArtistId) { + try { + await updateArtistSocials(created.account_id, { + SPOTIFY: `https://open.spotify.com/artist/${spotifyArtistId}`, + }); + } catch { + // Non-fatal by design — see docstring. + } + } + + return { artist: created, created: true }; +} diff --git a/lib/artists/validateCreateArtistBody.ts b/lib/artists/validateCreateArtistBody.ts index 2515d116..c8b787e8 100644 --- a/lib/artists/validateCreateArtistBody.ts +++ b/lib/artists/validateCreateArtistBody.ts @@ -6,6 +6,7 @@ import { z } from "zod"; export const createArtistBodySchema = z.object({ name: z.string({ message: "name is required" }).min(1, "name cannot be empty"), + spotify_artist_id: z.string().min(1, "spotify_artist_id cannot be empty").optional(), account_id: z.uuid({ message: "account_id must be a valid UUID" }).optional(), organization_id: z.uuid({ message: "organization_id must be a valid UUID" }).optional(), }); @@ -16,6 +17,8 @@ export type ValidatedCreateArtistRequest = { name: string; accountId: string; organizationId?: string; + /** Optional Spotify artist id: the handler resolves-or-creates the canonical for it (chat#1889 row 8). */ + spotifyArtistId?: string; }; /** @@ -61,5 +64,6 @@ export async function validateCreateArtistBody( name: result.data.name, accountId: authContext.accountId, organizationId: result.data.organization_id, + spotifyArtistId: result.data.spotify_artist_id, }; }