From c2c0b636af9096ddf6ceee0f55c0bddd99744678 Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Thu, 18 Jun 2026 08:13:48 -0500 Subject: [PATCH 1/4] feat: POST /api/catalogs create + materialize from valuation snapshot Creates a catalog owned by the authenticated account (account derived from credentials via validateAuthContext, never the body). With from.snapshot_id, materializes the catalog from a completed valuation snapshot: creates the catalogs row, links account_catalogs, adds the snapshot's measured ISRCs as catalog_songs, and records the catalog on the snapshot. Re-claiming the same snapshot is idempotent. TDD: validateCreateCatalogBody (6 tests) + createCatalogHandler (8 tests), red->green. New supabase wrappers: insertCatalog, selectCatalogById, insertAccountCatalog, updateSnapshotCatalog. Implements recoupable/chat#1801 Phase 2. Matches docs contract recoupable/docs#243. Co-Authored-By: Claude Opus 4.8 (1M context) --- app/api/catalogs/route.ts | 26 +++ .../__tests__/createCatalogHandler.test.ts | 160 ++++++++++++++++++ .../validateCreateCatalogBody.test.ts | 50 ++++++ lib/catalog/createCatalogHandler.ts | 83 +++++++++ lib/catalog/materializeSnapshotCatalog.ts | 41 +++++ lib/catalog/validateCreateCatalogBody.ts | 50 ++++++ .../account_catalogs/insertAccountCatalog.ts | 21 +++ lib/supabase/catalogs/insertCatalog.ts | 19 +++ lib/supabase/catalogs/selectCatalogById.ts | 19 +++ .../updateSnapshotCatalog.ts | 24 +++ 10 files changed, 493 insertions(+) create mode 100644 app/api/catalogs/route.ts create mode 100644 lib/catalog/__tests__/createCatalogHandler.test.ts create mode 100644 lib/catalog/__tests__/validateCreateCatalogBody.test.ts create mode 100644 lib/catalog/createCatalogHandler.ts create mode 100644 lib/catalog/materializeSnapshotCatalog.ts create mode 100644 lib/catalog/validateCreateCatalogBody.ts create mode 100644 lib/supabase/account_catalogs/insertAccountCatalog.ts create mode 100644 lib/supabase/catalogs/insertCatalog.ts create mode 100644 lib/supabase/catalogs/selectCatalogById.ts create mode 100644 lib/supabase/playcount_snapshots/updateSnapshotCatalog.ts diff --git a/app/api/catalogs/route.ts b/app/api/catalogs/route.ts new file mode 100644 index 000000000..5ab6c6692 --- /dev/null +++ b/app/api/catalogs/route.ts @@ -0,0 +1,26 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { createCatalogHandler } from "@/lib/catalog/createCatalogHandler"; + +/** + * OPTIONS handler for CORS preflight requests. + * + * @returns A NextResponse with CORS headers. + */ +export async function OPTIONS() { + return new NextResponse(null, { + status: 200, + headers: getCorsHeaders(), + }); +} + +/** + * POST handler for creating a catalog (optionally materialized from a + * valuation snapshot). + * + * @param request - The request object containing the catalog body. + * @returns A NextResponse with the created catalog. + */ +export async function POST(request: NextRequest) { + return createCatalogHandler(request); +} diff --git a/lib/catalog/__tests__/createCatalogHandler.test.ts b/lib/catalog/__tests__/createCatalogHandler.test.ts new file mode 100644 index 000000000..669d1f1b2 --- /dev/null +++ b/lib/catalog/__tests__/createCatalogHandler.test.ts @@ -0,0 +1,160 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { NextRequest, NextResponse } from "next/server"; + +import { createCatalogHandler } from "../createCatalogHandler"; +import { validateCreateCatalogBody } from "../validateCreateCatalogBody"; +import { materializeSnapshotCatalog } from "../materializeSnapshotCatalog"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import { selectPlaycountSnapshots } from "@/lib/supabase/playcount_snapshots/selectPlaycountSnapshots"; +import { selectCatalogById } from "@/lib/supabase/catalogs/selectCatalogById"; +import { insertCatalog } from "@/lib/supabase/catalogs/insertCatalog"; +import { insertAccountCatalog } from "@/lib/supabase/account_catalogs/insertAccountCatalog"; + +vi.mock("@/lib/networking/getCorsHeaders", () => ({ + getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), +})); +vi.mock("../validateCreateCatalogBody", () => ({ validateCreateCatalogBody: vi.fn() })); +vi.mock("../materializeSnapshotCatalog", () => ({ materializeSnapshotCatalog: vi.fn() })); +vi.mock("@/lib/auth/validateAuthContext", () => ({ validateAuthContext: vi.fn() })); +vi.mock("@/lib/supabase/playcount_snapshots/selectPlaycountSnapshots", () => ({ + selectPlaycountSnapshots: vi.fn(), +})); +vi.mock("@/lib/supabase/catalogs/selectCatalogById", () => ({ selectCatalogById: vi.fn() })); +vi.mock("@/lib/supabase/catalogs/insertCatalog", () => ({ insertCatalog: vi.fn() })); +vi.mock("@/lib/supabase/account_catalogs/insertAccountCatalog", () => ({ + insertAccountCatalog: vi.fn(), +})); + +const accountId = "550e8400-e29b-41d4-a716-446655440000"; +const otherAccountId = "550e8400-e29b-41d4-a716-446655440999"; +const snapshotId = "11111111-2222-3333-4444-555555555555"; +const catalogId = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"; + +const catalog = { + id: catalogId, + name: "Bad Bunny — Catalog", + created_at: "2026-06-18T00:00:00Z", + updated_at: "2026-06-18T00:00:00Z", +}; + +const makeRequest = () => new NextRequest("http://localhost/api/catalogs", { method: "POST" }); + +const okAuth = () => + vi.mocked(validateAuthContext).mockResolvedValue({ accountId, orgId: null, authToken: "t" }); + +describe("createCatalogHandler", () => { + beforeEach(() => vi.clearAllMocks()); + + it("short-circuits with the validator error and never authenticates", async () => { + const err = NextResponse.json({ status: "error" }, { status: 400 }); + vi.mocked(validateCreateCatalogBody).mockReturnValue(err); + + const res = await createCatalogHandler(makeRequest()); + + expect(res).toBe(err); + expect(validateAuthContext).not.toHaveBeenCalled(); + }); + + it("short-circuits with the auth error when unauthenticated", async () => { + vi.mocked(validateCreateCatalogBody).mockReturnValue({ name: "X" }); + const authErr = NextResponse.json({ status: "error" }, { status: 401 }); + vi.mocked(validateAuthContext).mockResolvedValue(authErr); + + const res = await createCatalogHandler(makeRequest()); + + expect(res).toBe(authErr); + expect(insertCatalog).not.toHaveBeenCalled(); + }); + + it("creates an empty catalog from a name-only body and links the account", async () => { + vi.mocked(validateCreateCatalogBody).mockReturnValue({ name: "Bad Bunny — Catalog" }); + okAuth(); + vi.mocked(insertCatalog).mockResolvedValue(catalog); + + const res = await createCatalogHandler(makeRequest()); + const body = await res.json(); + + expect(res.status).toBe(200); + expect(insertCatalog).toHaveBeenCalledWith("Bad Bunny — Catalog"); + expect(insertAccountCatalog).toHaveBeenCalledWith({ account: accountId, catalog: catalogId }); + expect(body).toEqual({ status: "success", catalog, songs_added: 0 }); + expect(selectPlaycountSnapshots).not.toHaveBeenCalled(); + }); + + it("materializes a catalog from an owned, not-yet-claimed snapshot", async () => { + vi.mocked(validateCreateCatalogBody).mockReturnValue({ + name: "Bad Bunny — Catalog", + from: { snapshot_id: snapshotId }, + }); + okAuth(); + vi.mocked(selectPlaycountSnapshots).mockResolvedValue([ + { id: snapshotId, account: accountId, catalog: null, isrcs: ["A", "B"] } as never, + ]); + vi.mocked(materializeSnapshotCatalog).mockResolvedValue({ catalog, songsAdded: 2 }); + + const res = await createCatalogHandler(makeRequest()); + const body = await res.json(); + + expect(res.status).toBe(200); + expect(materializeSnapshotCatalog).toHaveBeenCalledWith({ + accountId, + snapshot: expect.objectContaining({ id: snapshotId }), + name: "Bad Bunny — Catalog", + }); + expect(body).toEqual({ status: "success", catalog, songs_added: 2 }); + }); + + it("returns 404 when the snapshot does not exist", async () => { + vi.mocked(validateCreateCatalogBody).mockReturnValue({ from: { snapshot_id: snapshotId } }); + okAuth(); + vi.mocked(selectPlaycountSnapshots).mockResolvedValue([]); + + const res = await createCatalogHandler(makeRequest()); + + expect(res.status).toBe(404); + expect(materializeSnapshotCatalog).not.toHaveBeenCalled(); + }); + + it("returns 403 when the snapshot belongs to a different account", async () => { + vi.mocked(validateCreateCatalogBody).mockReturnValue({ from: { snapshot_id: snapshotId } }); + okAuth(); + vi.mocked(selectPlaycountSnapshots).mockResolvedValue([ + { id: snapshotId, account: otherAccountId, catalog: null, isrcs: ["A"] } as never, + ]); + + const res = await createCatalogHandler(makeRequest()); + + expect(res.status).toBe(403); + expect(materializeSnapshotCatalog).not.toHaveBeenCalled(); + }); + + it("is idempotent: returns the existing catalog when the snapshot is already claimed", async () => { + vi.mocked(validateCreateCatalogBody).mockReturnValue({ from: { snapshot_id: snapshotId } }); + okAuth(); + vi.mocked(selectPlaycountSnapshots).mockResolvedValue([ + { id: snapshotId, account: accountId, catalog: catalogId, isrcs: ["A", "B"] } as never, + ]); + vi.mocked(selectCatalogById).mockResolvedValue(catalog); + + const res = await createCatalogHandler(makeRequest()); + const body = await res.json(); + + expect(res.status).toBe(200); + expect(selectCatalogById).toHaveBeenCalledWith(catalogId); + expect(materializeSnapshotCatalog).not.toHaveBeenCalled(); + expect(body).toEqual({ status: "success", catalog, songs_added: 0 }); + }); + + it("returns a generic 500 without leaking the underlying error", async () => { + vi.mocked(validateCreateCatalogBody).mockReturnValue({ name: "X" }); + okAuth(); + vi.mocked(insertCatalog).mockRejectedValue(new Error("db down at 10.0.0.1:5432")); + + const res = await createCatalogHandler(makeRequest()); + const body = await res.json(); + + expect(res.status).toBe(500); + expect(body.status).toBe("error"); + expect(JSON.stringify(body)).not.toContain("10.0.0.1"); + }); +}); diff --git a/lib/catalog/__tests__/validateCreateCatalogBody.test.ts b/lib/catalog/__tests__/validateCreateCatalogBody.test.ts new file mode 100644 index 000000000..ea02b1857 --- /dev/null +++ b/lib/catalog/__tests__/validateCreateCatalogBody.test.ts @@ -0,0 +1,50 @@ +import { describe, it, expect } from "vitest"; +import { NextResponse } from "next/server"; + +import { validateCreateCatalogBody } from "../validateCreateCatalogBody"; + +const snapshotId = "550e8400-e29b-41d4-a716-446655440000"; + +describe("validateCreateCatalogBody", () => { + it("accepts a name-only body", () => { + const result = validateCreateCatalogBody({ name: "My Catalog" }); + expect(result).toEqual({ name: "My Catalog" }); + }); + + it("accepts a from-only body and parses snapshot_id", () => { + const result = validateCreateCatalogBody({ from: { snapshot_id: snapshotId } }); + expect(result).toEqual({ from: { snapshot_id: snapshotId } }); + }); + + it("accepts name and from together", () => { + const result = validateCreateCatalogBody({ + name: "Bad Bunny — Catalog", + from: { snapshot_id: snapshotId }, + }); + expect(result).toEqual({ + name: "Bad Bunny — Catalog", + from: { snapshot_id: snapshotId }, + }); + }); + + it("rejects an empty body (neither name nor from) with 400", async () => { + const result = validateCreateCatalogBody({}); + expect(result).toBeInstanceOf(NextResponse); + const res = result as NextResponse; + expect(res.status).toBe(400); + const body = await res.json(); + expect(body.status).toBe("error"); + }); + + it("rejects a non-uuid snapshot_id with 400", async () => { + const result = validateCreateCatalogBody({ from: { snapshot_id: "not-a-uuid" } }); + expect(result).toBeInstanceOf(NextResponse); + expect((result as NextResponse).status).toBe(400); + }); + + it("rejects a null/non-object body with 400", async () => { + const result = validateCreateCatalogBody(null); + expect(result).toBeInstanceOf(NextResponse); + expect((result as NextResponse).status).toBe(400); + }); +}); diff --git a/lib/catalog/createCatalogHandler.ts b/lib/catalog/createCatalogHandler.ts new file mode 100644 index 000000000..ab59c9502 --- /dev/null +++ b/lib/catalog/createCatalogHandler.ts @@ -0,0 +1,83 @@ +import { NextRequest, NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { errorResponse } from "@/lib/networking/errorResponse"; +import { validateAuthContext } from "@/lib/auth/validateAuthContext"; +import { Tables } from "@/types/database.types"; +import { validateCreateCatalogBody } from "./validateCreateCatalogBody"; +import { materializeSnapshotCatalog } from "./materializeSnapshotCatalog"; +import { selectPlaycountSnapshots } from "@/lib/supabase/playcount_snapshots/selectPlaycountSnapshots"; +import { selectCatalogById } from "@/lib/supabase/catalogs/selectCatalogById"; +import { insertCatalog } from "@/lib/supabase/catalogs/insertCatalog"; +import { insertAccountCatalog } from "@/lib/supabase/account_catalogs/insertAccountCatalog"; + +const DEFAULT_CATALOG_NAME = "Valuation Catalog"; + +function success(catalog: Tables<"catalogs">, songsAdded: number): NextResponse { + return NextResponse.json( + { status: "success", catalog, songs_added: songsAdded }, + { status: 200, headers: getCorsHeaders() }, + ); +} + +/** + * POST /api/catalogs + * + * Creates a catalog owned by the authenticated account. The owning account is + * resolved from credentials (Privy bearer or x-api-key), never from the body. + * + * With `from.snapshot_id`, materializes the catalog from a completed valuation + * snapshot: the snapshot must be owned by the caller, and re-claiming the same + * snapshot is idempotent (returns the catalog already created for that run). + * + * @param request - The request object + * @returns A NextResponse with `{ status, catalog, songs_added }` + */ +export async function createCatalogHandler(request: NextRequest): Promise { + try { + const body = await request.json().catch(() => null); + + const validated = validateCreateCatalogBody(body); + if (validated instanceof NextResponse) { + return validated; + } + + const authResult = await validateAuthContext(request); + if (authResult instanceof NextResponse) { + return authResult; + } + const { accountId } = authResult; + + if (!validated.from) { + const catalog = await insertCatalog(validated.name ?? DEFAULT_CATALOG_NAME); + await insertAccountCatalog({ account: accountId, catalog: catalog.id }); + return success(catalog, 0); + } + + const snapshotId = validated.from.snapshot_id; + const [snapshot] = await selectPlaycountSnapshots({ id: snapshotId }); + if (!snapshot) { + return errorResponse("Snapshot not found", 404); + } + if (snapshot.account !== accountId) { + return errorResponse("Snapshot belongs to a different account", 403); + } + + // Idempotent re-claim: the run already produced a catalog. + if (snapshot.catalog) { + const existing = await selectCatalogById(snapshot.catalog); + if (existing) { + return success(existing, 0); + } + } + + const { catalog, songsAdded } = await materializeSnapshotCatalog({ + accountId, + snapshot, + name: validated.name, + }); + return success(catalog, songsAdded); + } catch (error) { + console.error("Error creating catalog:", error); + return errorResponse("Internal server error", 500); + } +} diff --git a/lib/catalog/materializeSnapshotCatalog.ts b/lib/catalog/materializeSnapshotCatalog.ts new file mode 100644 index 000000000..31ca5e3c3 --- /dev/null +++ b/lib/catalog/materializeSnapshotCatalog.ts @@ -0,0 +1,41 @@ +import { Tables } from "@/types/database.types"; +import { insertCatalog } from "@/lib/supabase/catalogs/insertCatalog"; +import { insertAccountCatalog } from "@/lib/supabase/account_catalogs/insertAccountCatalog"; +import { insertCatalogSongs } from "@/lib/supabase/catalog_songs/insertCatalogSongs"; +import { updateSnapshotCatalog } from "@/lib/supabase/playcount_snapshots/updateSnapshotCatalog"; + +const DEFAULT_CATALOG_NAME = "Valuation Catalog"; + +/** + * Materializes a valuation snapshot into an account-linked catalog: + * creates the `catalogs` row, links it to the account via `account_catalogs`, + * adds the snapshot's measured ISRCs as `catalog_songs`, and records the new + * catalog on the snapshot (the idempotency key for re-claims). + * + * Callers must first confirm the snapshot is owned by `accountId` and not yet + * claimed (`snapshot.catalog` is null). + * + * @param params.accountId - Owning account (already authorized) + * @param params.snapshot - The owned, unclaimed snapshot row + * @param params.name - Optional catalog name; falls back to a default + * @returns The created catalog and the number of songs added + */ +export async function materializeSnapshotCatalog(params: { + accountId: string; + snapshot: Tables<"playcount_snapshots">; + name?: string; +}): Promise<{ catalog: Tables<"catalogs">; songsAdded: number }> { + const { accountId, snapshot, name } = params; + + const catalog = await insertCatalog(name ?? DEFAULT_CATALOG_NAME); + await insertAccountCatalog({ account: accountId, catalog: catalog.id }); + + const isrcs = snapshot.isrcs ?? []; + if (isrcs.length > 0) { + await insertCatalogSongs(isrcs.map(isrc => ({ catalog: catalog.id, song: isrc }))); + } + + await updateSnapshotCatalog({ snapshotId: snapshot.id, catalogId: catalog.id }); + + return { catalog, songsAdded: isrcs.length }; +} diff --git a/lib/catalog/validateCreateCatalogBody.ts b/lib/catalog/validateCreateCatalogBody.ts new file mode 100644 index 000000000..be6b72e59 --- /dev/null +++ b/lib/catalog/validateCreateCatalogBody.ts @@ -0,0 +1,50 @@ +import { NextResponse } from "next/server"; +import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; +import { z } from "zod"; + +const materializationSourceSchema = z.object({ + snapshot_id: z.string().uuid("snapshot_id must be a valid UUID"), +}); + +export const createCatalogBodySchema = z + .object({ + name: z.string().min(1, "name must not be empty").optional(), + from: materializationSourceSchema.optional(), + }) + .refine(data => data.name !== undefined || data.from !== undefined, { + message: "Provide at least one of name or from", + }); + +export type CreateCatalogBody = z.infer; + +/** + * Validates a create-catalog request body. + * + * Accepts `{ name?, from?: { snapshot_id } }`; at least one of `name` or + * `from` is required. The owning account is never taken from the body - it is + * resolved from the request credentials by the handler. + * + * @param body - The parsed request body to validate. + * @returns A NextResponse with a 400 error if validation fails, or the + * validated body if it passes. + */ +export function validateCreateCatalogBody(body: unknown): NextResponse | CreateCatalogBody { + const result = createCatalogBodySchema.safeParse(body); + + if (!result.success) { + const firstError = result.error.issues[0]; + return NextResponse.json( + { + status: "error", + missing_fields: firstError.path, + error: firstError.message, + }, + { + status: 400, + headers: getCorsHeaders(), + }, + ); + } + + return result.data; +} diff --git a/lib/supabase/account_catalogs/insertAccountCatalog.ts b/lib/supabase/account_catalogs/insertAccountCatalog.ts new file mode 100644 index 000000000..1facd5b63 --- /dev/null +++ b/lib/supabase/account_catalogs/insertAccountCatalog.ts @@ -0,0 +1,21 @@ +import supabase from "../serverClient"; + +/** + * Links a catalog to an account by inserting an `account_catalogs` row. + * + * @param params.account - Account id (owner) + * @param params.catalog - Catalog id to link + * @throws Error if the insert fails + */ +export async function insertAccountCatalog(params: { + account: string; + catalog: string; +}): Promise { + const { error } = await supabase + .from("account_catalogs") + .insert({ account: params.account, catalog: params.catalog }); + + if (error) { + throw new Error(`Failed to link account_catalogs: ${error.message}`); + } +} diff --git a/lib/supabase/catalogs/insertCatalog.ts b/lib/supabase/catalogs/insertCatalog.ts new file mode 100644 index 000000000..ecd0c667b --- /dev/null +++ b/lib/supabase/catalogs/insertCatalog.ts @@ -0,0 +1,19 @@ +import supabase from "../serverClient"; +import { Tables } from "@/types/database.types"; + +/** + * Inserts a new `catalogs` row and returns it. + * + * @param name - Display name for the catalog + * @returns The inserted catalog row + * @throws Error if the insert fails + */ +export async function insertCatalog(name: string): Promise> { + const { data, error } = await supabase.from("catalogs").insert({ name }).select().single(); + + if (error || !data) { + throw new Error(`Failed to insert catalog: ${error?.message ?? "no row returned"}`); + } + + return data; +} diff --git a/lib/supabase/catalogs/selectCatalogById.ts b/lib/supabase/catalogs/selectCatalogById.ts new file mode 100644 index 000000000..39f961e3d --- /dev/null +++ b/lib/supabase/catalogs/selectCatalogById.ts @@ -0,0 +1,19 @@ +import supabase from "../serverClient"; +import { Tables } from "@/types/database.types"; + +/** + * Selects a single `catalogs` row by id, or null if none exists. + * + * @param id - Catalog id + * @returns The catalog row, or null if not found + * @throws Error if the query fails + */ +export async function selectCatalogById(id: string): Promise | null> { + const { data, error } = await supabase.from("catalogs").select("*").eq("id", id).maybeSingle(); + + if (error) { + throw new Error(`Failed to fetch catalog: ${error.message}`); + } + + return data; +} diff --git a/lib/supabase/playcount_snapshots/updateSnapshotCatalog.ts b/lib/supabase/playcount_snapshots/updateSnapshotCatalog.ts new file mode 100644 index 000000000..aa05354f0 --- /dev/null +++ b/lib/supabase/playcount_snapshots/updateSnapshotCatalog.ts @@ -0,0 +1,24 @@ +import supabase from "../serverClient"; + +/** + * Sets the `catalog` foreign key on a `playcount_snapshots` row. This records + * which catalog a valuation run was materialized into, and is the idempotency + * key for re-claims (a snapshot with `catalog` set is already materialized). + * + * @param params.snapshotId - Snapshot id to update + * @param params.catalogId - Catalog id to record on the snapshot + * @throws Error if the update fails + */ +export async function updateSnapshotCatalog(params: { + snapshotId: string; + catalogId: string; +}): Promise { + const { error } = await supabase + .from("playcount_snapshots") + .update({ catalog: params.catalogId }) + .eq("id", params.snapshotId); + + if (error) { + throw new Error(`Failed to set snapshot catalog: ${error.message}`); + } +} From 7de73dd56aa88eef6c1f1431d4733c8dfebdd99b Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Thu, 18 Jun 2026 13:16:06 -0500 Subject: [PATCH 2/4] refactor: re-anchor POST /api/catalogs to merged contract + review fixes - Flatten request to the merged docs#243 contract: from:{snapshot_id} -> a root snapshot field (validator + handler + tests). Error copy follows. - DRY/SRP: drop the inline success() helper; use the shared successResponse(). - KISS rename: materializeSnapshotCatalog.ts -> createSnapshotCatalog.ts. - DRY: delete the redundant updateSnapshotCatalog helper; reuse the existing updatePlaycountSnapshot(id, fields). Validator change done red->green. lib/catalog: 24 tests pass; tsc + eslint clean. Addresses review on PR #677. --- .../__tests__/createCatalogHandler.test.ts | 22 ++++++------- .../validateCreateCatalogBody.test.ts | 18 +++++------ lib/catalog/createCatalogHandler.ts | 31 +++++++------------ ...hotCatalog.ts => createSnapshotCatalog.ts} | 14 ++++----- lib/catalog/validateCreateCatalogBody.ts | 17 +++++----- .../updateSnapshotCatalog.ts | 24 -------------- 6 files changed, 45 insertions(+), 81 deletions(-) rename lib/catalog/{materializeSnapshotCatalog.ts => createSnapshotCatalog.ts} (70%) delete mode 100644 lib/supabase/playcount_snapshots/updateSnapshotCatalog.ts diff --git a/lib/catalog/__tests__/createCatalogHandler.test.ts b/lib/catalog/__tests__/createCatalogHandler.test.ts index 669d1f1b2..5e2b1ebf0 100644 --- a/lib/catalog/__tests__/createCatalogHandler.test.ts +++ b/lib/catalog/__tests__/createCatalogHandler.test.ts @@ -3,7 +3,7 @@ import { NextRequest, NextResponse } from "next/server"; import { createCatalogHandler } from "../createCatalogHandler"; import { validateCreateCatalogBody } from "../validateCreateCatalogBody"; -import { materializeSnapshotCatalog } from "../materializeSnapshotCatalog"; +import { createSnapshotCatalog } from "../createSnapshotCatalog"; import { validateAuthContext } from "@/lib/auth/validateAuthContext"; import { selectPlaycountSnapshots } from "@/lib/supabase/playcount_snapshots/selectPlaycountSnapshots"; import { selectCatalogById } from "@/lib/supabase/catalogs/selectCatalogById"; @@ -14,7 +14,7 @@ vi.mock("@/lib/networking/getCorsHeaders", () => ({ getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), })); vi.mock("../validateCreateCatalogBody", () => ({ validateCreateCatalogBody: vi.fn() })); -vi.mock("../materializeSnapshotCatalog", () => ({ materializeSnapshotCatalog: vi.fn() })); +vi.mock("../createSnapshotCatalog", () => ({ createSnapshotCatalog: vi.fn() })); vi.mock("@/lib/auth/validateAuthContext", () => ({ validateAuthContext: vi.fn() })); vi.mock("@/lib/supabase/playcount_snapshots/selectPlaycountSnapshots", () => ({ selectPlaycountSnapshots: vi.fn(), @@ -84,19 +84,19 @@ describe("createCatalogHandler", () => { it("materializes a catalog from an owned, not-yet-claimed snapshot", async () => { vi.mocked(validateCreateCatalogBody).mockReturnValue({ name: "Bad Bunny — Catalog", - from: { snapshot_id: snapshotId }, + snapshot: snapshotId, }); okAuth(); vi.mocked(selectPlaycountSnapshots).mockResolvedValue([ { id: snapshotId, account: accountId, catalog: null, isrcs: ["A", "B"] } as never, ]); - vi.mocked(materializeSnapshotCatalog).mockResolvedValue({ catalog, songsAdded: 2 }); + vi.mocked(createSnapshotCatalog).mockResolvedValue({ catalog, songsAdded: 2 }); const res = await createCatalogHandler(makeRequest()); const body = await res.json(); expect(res.status).toBe(200); - expect(materializeSnapshotCatalog).toHaveBeenCalledWith({ + expect(createSnapshotCatalog).toHaveBeenCalledWith({ accountId, snapshot: expect.objectContaining({ id: snapshotId }), name: "Bad Bunny — Catalog", @@ -105,18 +105,18 @@ describe("createCatalogHandler", () => { }); it("returns 404 when the snapshot does not exist", async () => { - vi.mocked(validateCreateCatalogBody).mockReturnValue({ from: { snapshot_id: snapshotId } }); + vi.mocked(validateCreateCatalogBody).mockReturnValue({ snapshot: snapshotId }); okAuth(); vi.mocked(selectPlaycountSnapshots).mockResolvedValue([]); const res = await createCatalogHandler(makeRequest()); expect(res.status).toBe(404); - expect(materializeSnapshotCatalog).not.toHaveBeenCalled(); + expect(createSnapshotCatalog).not.toHaveBeenCalled(); }); it("returns 403 when the snapshot belongs to a different account", async () => { - vi.mocked(validateCreateCatalogBody).mockReturnValue({ from: { snapshot_id: snapshotId } }); + vi.mocked(validateCreateCatalogBody).mockReturnValue({ snapshot: snapshotId }); okAuth(); vi.mocked(selectPlaycountSnapshots).mockResolvedValue([ { id: snapshotId, account: otherAccountId, catalog: null, isrcs: ["A"] } as never, @@ -125,11 +125,11 @@ describe("createCatalogHandler", () => { const res = await createCatalogHandler(makeRequest()); expect(res.status).toBe(403); - expect(materializeSnapshotCatalog).not.toHaveBeenCalled(); + expect(createSnapshotCatalog).not.toHaveBeenCalled(); }); it("is idempotent: returns the existing catalog when the snapshot is already claimed", async () => { - vi.mocked(validateCreateCatalogBody).mockReturnValue({ from: { snapshot_id: snapshotId } }); + vi.mocked(validateCreateCatalogBody).mockReturnValue({ snapshot: snapshotId }); okAuth(); vi.mocked(selectPlaycountSnapshots).mockResolvedValue([ { id: snapshotId, account: accountId, catalog: catalogId, isrcs: ["A", "B"] } as never, @@ -141,7 +141,7 @@ describe("createCatalogHandler", () => { expect(res.status).toBe(200); expect(selectCatalogById).toHaveBeenCalledWith(catalogId); - expect(materializeSnapshotCatalog).not.toHaveBeenCalled(); + expect(createSnapshotCatalog).not.toHaveBeenCalled(); expect(body).toEqual({ status: "success", catalog, songs_added: 0 }); }); diff --git a/lib/catalog/__tests__/validateCreateCatalogBody.test.ts b/lib/catalog/__tests__/validateCreateCatalogBody.test.ts index ea02b1857..670895f6a 100644 --- a/lib/catalog/__tests__/validateCreateCatalogBody.test.ts +++ b/lib/catalog/__tests__/validateCreateCatalogBody.test.ts @@ -11,23 +11,23 @@ describe("validateCreateCatalogBody", () => { expect(result).toEqual({ name: "My Catalog" }); }); - it("accepts a from-only body and parses snapshot_id", () => { - const result = validateCreateCatalogBody({ from: { snapshot_id: snapshotId } }); - expect(result).toEqual({ from: { snapshot_id: snapshotId } }); + it("accepts a snapshot-only body", () => { + const result = validateCreateCatalogBody({ snapshot: snapshotId }); + expect(result).toEqual({ snapshot: snapshotId }); }); - it("accepts name and from together", () => { + it("accepts name and snapshot together", () => { const result = validateCreateCatalogBody({ name: "Bad Bunny — Catalog", - from: { snapshot_id: snapshotId }, + snapshot: snapshotId, }); expect(result).toEqual({ name: "Bad Bunny — Catalog", - from: { snapshot_id: snapshotId }, + snapshot: snapshotId, }); }); - it("rejects an empty body (neither name nor from) with 400", async () => { + it("rejects an empty body (neither name nor snapshot) with 400", async () => { const result = validateCreateCatalogBody({}); expect(result).toBeInstanceOf(NextResponse); const res = result as NextResponse; @@ -36,8 +36,8 @@ describe("validateCreateCatalogBody", () => { expect(body.status).toBe("error"); }); - it("rejects a non-uuid snapshot_id with 400", async () => { - const result = validateCreateCatalogBody({ from: { snapshot_id: "not-a-uuid" } }); + it("rejects a non-uuid snapshot with 400", async () => { + const result = validateCreateCatalogBody({ snapshot: "not-a-uuid" }); expect(result).toBeInstanceOf(NextResponse); expect((result as NextResponse).status).toBe(400); }); diff --git a/lib/catalog/createCatalogHandler.ts b/lib/catalog/createCatalogHandler.ts index ab59c9502..583746971 100644 --- a/lib/catalog/createCatalogHandler.ts +++ b/lib/catalog/createCatalogHandler.ts @@ -1,10 +1,9 @@ import { NextRequest, NextResponse } from "next/server"; -import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { errorResponse } from "@/lib/networking/errorResponse"; +import { successResponse } from "@/lib/networking/successResponse"; import { validateAuthContext } from "@/lib/auth/validateAuthContext"; -import { Tables } from "@/types/database.types"; import { validateCreateCatalogBody } from "./validateCreateCatalogBody"; -import { materializeSnapshotCatalog } from "./materializeSnapshotCatalog"; +import { createSnapshotCatalog } from "./createSnapshotCatalog"; import { selectPlaycountSnapshots } from "@/lib/supabase/playcount_snapshots/selectPlaycountSnapshots"; import { selectCatalogById } from "@/lib/supabase/catalogs/selectCatalogById"; import { insertCatalog } from "@/lib/supabase/catalogs/insertCatalog"; @@ -12,22 +11,15 @@ import { insertAccountCatalog } from "@/lib/supabase/account_catalogs/insertAcco const DEFAULT_CATALOG_NAME = "Valuation Catalog"; -function success(catalog: Tables<"catalogs">, songsAdded: number): NextResponse { - return NextResponse.json( - { status: "success", catalog, songs_added: songsAdded }, - { status: 200, headers: getCorsHeaders() }, - ); -} - /** * POST /api/catalogs * * Creates a catalog owned by the authenticated account. The owning account is * resolved from credentials (Privy bearer or x-api-key), never from the body. * - * With `from.snapshot_id`, materializes the catalog from a completed valuation - * snapshot: the snapshot must be owned by the caller, and re-claiming the same - * snapshot is idempotent (returns the catalog already created for that run). + * With `snapshot`, materializes the catalog from a completed valuation snapshot: + * the snapshot must be owned by the caller, and re-claiming the same snapshot is + * idempotent (returns the catalog already created for that run). * * @param request - The request object * @returns A NextResponse with `{ status, catalog, songs_added }` @@ -47,14 +39,13 @@ export async function createCatalogHandler(request: NextRequest): Promise; name?: string; @@ -35,7 +35,7 @@ export async function materializeSnapshotCatalog(params: { await insertCatalogSongs(isrcs.map(isrc => ({ catalog: catalog.id, song: isrc }))); } - await updateSnapshotCatalog({ snapshotId: snapshot.id, catalogId: catalog.id }); + await updatePlaycountSnapshot(snapshot.id, { catalog: catalog.id }); return { catalog, songsAdded: isrcs.length }; } diff --git a/lib/catalog/validateCreateCatalogBody.ts b/lib/catalog/validateCreateCatalogBody.ts index be6b72e59..b41c058c1 100644 --- a/lib/catalog/validateCreateCatalogBody.ts +++ b/lib/catalog/validateCreateCatalogBody.ts @@ -2,17 +2,13 @@ import { NextResponse } from "next/server"; import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; import { z } from "zod"; -const materializationSourceSchema = z.object({ - snapshot_id: z.string().uuid("snapshot_id must be a valid UUID"), -}); - export const createCatalogBodySchema = z .object({ name: z.string().min(1, "name must not be empty").optional(), - from: materializationSourceSchema.optional(), + snapshot: z.string().uuid("snapshot must be a valid UUID").optional(), }) - .refine(data => data.name !== undefined || data.from !== undefined, { - message: "Provide at least one of name or from", + .refine(data => data.name !== undefined || data.snapshot !== undefined, { + message: "Provide at least one of name or snapshot", }); export type CreateCatalogBody = z.infer; @@ -20,9 +16,10 @@ export type CreateCatalogBody = z.infer; /** * Validates a create-catalog request body. * - * Accepts `{ name?, from?: { snapshot_id } }`; at least one of `name` or - * `from` is required. The owning account is never taken from the body - it is - * resolved from the request credentials by the handler. + * Accepts `{ name?, snapshot? }`; at least one is required. `snapshot` is a + * completed playcount snapshot id (valuation run) to materialize from. The + * owning account is never taken from the body - it is resolved from the + * request credentials by the handler. * * @param body - The parsed request body to validate. * @returns A NextResponse with a 400 error if validation fails, or the diff --git a/lib/supabase/playcount_snapshots/updateSnapshotCatalog.ts b/lib/supabase/playcount_snapshots/updateSnapshotCatalog.ts deleted file mode 100644 index aa05354f0..000000000 --- a/lib/supabase/playcount_snapshots/updateSnapshotCatalog.ts +++ /dev/null @@ -1,24 +0,0 @@ -import supabase from "../serverClient"; - -/** - * Sets the `catalog` foreign key on a `playcount_snapshots` row. This records - * which catalog a valuation run was materialized into, and is the idempotency - * key for re-claims (a snapshot with `catalog` set is already materialized). - * - * @param params.snapshotId - Snapshot id to update - * @param params.catalogId - Catalog id to record on the snapshot - * @throws Error if the update fails - */ -export async function updateSnapshotCatalog(params: { - snapshotId: string; - catalogId: string; -}): Promise { - const { error } = await supabase - .from("playcount_snapshots") - .update({ catalog: params.catalogId }) - .eq("id", params.snapshotId); - - if (error) { - throw new Error(`Failed to set snapshot catalog: ${error.message}`); - } -} From 66c61b06eecbbdd09d6acb5a9035f215d6bd087b Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Thu, 18 Jun 2026 14:45:08 -0500 Subject: [PATCH 3/4] fix: materialize catalog songs from song_measurements, not snapshot.isrcs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Testing the full materialize path surfaced a real bug: a valuation snapshot is album_ids-scoped, so its own isrcs column is null — createSnapshotCatalog read snapshot.isrcs and would link an EMPTY catalog. The measured ISRCs live in song_measurements (snapshot lineage), so source them there. New selectSnapshotIsrcs(snapshotId) helper (distinct song_measurements.song for the snapshot). createSnapshotCatalog now uses it. TDD: new createSnapshotCatalog.test.ts (3 tests) red->green; lib/catalog 27 pass. Addresses PR #677 verification. --- .../__tests__/createSnapshotCatalog.test.ts | 71 +++++++++++++++++++ lib/catalog/createSnapshotCatalog.ts | 9 ++- .../song_measurements/selectSnapshotIsrcs.ts | 24 +++++++ 3 files changed, 101 insertions(+), 3 deletions(-) create mode 100644 lib/catalog/__tests__/createSnapshotCatalog.test.ts create mode 100644 lib/supabase/song_measurements/selectSnapshotIsrcs.ts diff --git a/lib/catalog/__tests__/createSnapshotCatalog.test.ts b/lib/catalog/__tests__/createSnapshotCatalog.test.ts new file mode 100644 index 000000000..7b2d39737 --- /dev/null +++ b/lib/catalog/__tests__/createSnapshotCatalog.test.ts @@ -0,0 +1,71 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +import { createSnapshotCatalog } from "../createSnapshotCatalog"; +import { insertCatalog } from "@/lib/supabase/catalogs/insertCatalog"; +import { insertAccountCatalog } from "@/lib/supabase/account_catalogs/insertAccountCatalog"; +import { insertCatalogSongs } from "@/lib/supabase/catalog_songs/insertCatalogSongs"; +import { updatePlaycountSnapshot } from "@/lib/supabase/playcount_snapshots/updatePlaycountSnapshot"; +import { selectSnapshotIsrcs } from "@/lib/supabase/song_measurements/selectSnapshotIsrcs"; + +vi.mock("@/lib/supabase/catalogs/insertCatalog", () => ({ insertCatalog: vi.fn() })); +vi.mock("@/lib/supabase/account_catalogs/insertAccountCatalog", () => ({ + insertAccountCatalog: vi.fn(), +})); +vi.mock("@/lib/supabase/catalog_songs/insertCatalogSongs", () => ({ insertCatalogSongs: vi.fn() })); +vi.mock("@/lib/supabase/playcount_snapshots/updatePlaycountSnapshot", () => ({ + updatePlaycountSnapshot: vi.fn(), +})); +vi.mock("@/lib/supabase/song_measurements/selectSnapshotIsrcs", () => ({ + selectSnapshotIsrcs: vi.fn(), +})); + +const accountId = "550e8400-e29b-41d4-a716-446655440000"; +const snapshotId = "11111111-2222-3333-4444-555555555555"; +const catalogId = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"; +const catalog = { id: catalogId, name: "Bad Bunny Catalog", created_at: "t", updated_at: "t" }; +// A real valuation snapshot is scoped by album_ids, so its own `isrcs` column is null — +// the measured ISRCs live in song_measurements (sourced via selectSnapshotIsrcs). +const snapshot = { id: snapshotId, account: accountId, catalog: null, isrcs: null } as never; + +describe("createSnapshotCatalog", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(insertCatalog).mockResolvedValue(catalog); + }); + + it("sources measured ISRCs from song_measurements and adds them as catalog songs", async () => { + vi.mocked(selectSnapshotIsrcs).mockResolvedValue(["ISRC_A", "ISRC_B", "ISRC_C"]); + + const result = await createSnapshotCatalog({ accountId, snapshot, name: "Bad Bunny Catalog" }); + + expect(insertCatalog).toHaveBeenCalledWith("Bad Bunny Catalog"); + expect(insertAccountCatalog).toHaveBeenCalledWith({ account: accountId, catalog: catalogId }); + // ISRCs come from measurements, NOT snapshot.isrcs (which is null here) + expect(selectSnapshotIsrcs).toHaveBeenCalledWith(snapshotId); + expect(insertCatalogSongs).toHaveBeenCalledWith([ + { catalog: catalogId, song: "ISRC_A" }, + { catalog: catalogId, song: "ISRC_B" }, + { catalog: catalogId, song: "ISRC_C" }, + ]); + expect(updatePlaycountSnapshot).toHaveBeenCalledWith(snapshotId, { catalog: catalogId }); + expect(result).toEqual({ catalog, songsAdded: 3 }); + }); + + it("adds no songs when the snapshot has no measurements", async () => { + vi.mocked(selectSnapshotIsrcs).mockResolvedValue([]); + + const result = await createSnapshotCatalog({ accountId, snapshot }); + + expect(insertCatalogSongs).not.toHaveBeenCalled(); + expect(updatePlaycountSnapshot).toHaveBeenCalledWith(snapshotId, { catalog: catalogId }); + expect(result).toEqual({ catalog, songsAdded: 0 }); + }); + + it("falls back to a default name when none is supplied", async () => { + vi.mocked(selectSnapshotIsrcs).mockResolvedValue([]); + + await createSnapshotCatalog({ accountId, snapshot }); + + expect(insertCatalog).toHaveBeenCalledWith("Valuation Catalog"); + }); +}); diff --git a/lib/catalog/createSnapshotCatalog.ts b/lib/catalog/createSnapshotCatalog.ts index 0d7326ecd..982775b74 100644 --- a/lib/catalog/createSnapshotCatalog.ts +++ b/lib/catalog/createSnapshotCatalog.ts @@ -3,14 +3,17 @@ import { insertCatalog } from "@/lib/supabase/catalogs/insertCatalog"; import { insertAccountCatalog } from "@/lib/supabase/account_catalogs/insertAccountCatalog"; import { insertCatalogSongs } from "@/lib/supabase/catalog_songs/insertCatalogSongs"; import { updatePlaycountSnapshot } from "@/lib/supabase/playcount_snapshots/updatePlaycountSnapshot"; +import { selectSnapshotIsrcs } from "@/lib/supabase/song_measurements/selectSnapshotIsrcs"; const DEFAULT_CATALOG_NAME = "Valuation Catalog"; /** * Creates an account-linked catalog from a valuation snapshot: creates the * `catalogs` row, links it to the account via `account_catalogs`, adds the - * snapshot's measured ISRCs as `catalog_songs`, and records the new catalog on - * the snapshot (the idempotency key for re-claims). + * snapshot's **measured** ISRCs (from `song_measurements`, not the snapshot's + * own `isrcs` column — that's null for album-scoped valuation runs) as + * `catalog_songs`, and records the new catalog on the snapshot (the idempotency + * key for re-claims). * * Callers must first confirm the snapshot is owned by `accountId` and not yet * claimed (`snapshot.catalog` is null). @@ -30,7 +33,7 @@ export async function createSnapshotCatalog(params: { const catalog = await insertCatalog(name ?? DEFAULT_CATALOG_NAME); await insertAccountCatalog({ account: accountId, catalog: catalog.id }); - const isrcs = snapshot.isrcs ?? []; + const isrcs = await selectSnapshotIsrcs(snapshot.id); if (isrcs.length > 0) { await insertCatalogSongs(isrcs.map(isrc => ({ catalog: catalog.id, song: isrc }))); } diff --git a/lib/supabase/song_measurements/selectSnapshotIsrcs.ts b/lib/supabase/song_measurements/selectSnapshotIsrcs.ts new file mode 100644 index 000000000..f0f50dc1e --- /dev/null +++ b/lib/supabase/song_measurements/selectSnapshotIsrcs.ts @@ -0,0 +1,24 @@ +import supabase from "../serverClient"; + +/** + * Returns the distinct ISRCs that were measured for a snapshot — the tracks + * that actually landed play counts in `song_measurements` for this run. This is + * the source of truth for a snapshot's tracks: the snapshot's own `isrcs` + * column is only set for ISRC-scoped runs, whereas valuations are album-scoped. + * + * @param snapshotId - The snapshot id (lineage on `song_measurements.snapshot`) + * @returns Distinct ISRC strings, or [] if none exist or on error + */ +export async function selectSnapshotIsrcs(snapshotId: string): Promise { + const { data, error } = await supabase + .from("song_measurements") + .select("song") + .eq("snapshot", snapshotId); + + if (error) { + console.error("Error fetching snapshot ISRCs:", error); + return []; + } + + return [...new Set((data ?? []).map(row => row.song))]; +} From b7e49ba9aacc0e9029db89891e8c3ade4ca8f32e Mon Sep 17 00:00:00 2001 From: Sweets Sweetman Date: Thu, 18 Jun 2026 14:53:01 -0500 Subject: [PATCH 4/4] refactor: reuse selectSongMeasurements (snapshot filter) instead of a new helper KISS/DRY per review: drop selectSnapshotIsrcs; add an optional snapshot filter to the existing selectSongMeasurements, and derive distinct ISRCs in createSnapshotCatalog. lib/catalog + song_measurements: 36 tests pass. Addresses review on PR #677. --- .../__tests__/createSnapshotCatalog.test.ts | 41 ++++++++++++++----- lib/catalog/createSnapshotCatalog.ts | 5 ++- .../song_measurements/selectSnapshotIsrcs.ts | 24 ----------- .../selectSongMeasurements.ts | 14 ++++--- 4 files changed, 43 insertions(+), 41 deletions(-) delete mode 100644 lib/supabase/song_measurements/selectSnapshotIsrcs.ts diff --git a/lib/catalog/__tests__/createSnapshotCatalog.test.ts b/lib/catalog/__tests__/createSnapshotCatalog.test.ts index 7b2d39737..9ee7a3ede 100644 --- a/lib/catalog/__tests__/createSnapshotCatalog.test.ts +++ b/lib/catalog/__tests__/createSnapshotCatalog.test.ts @@ -5,7 +5,7 @@ import { insertCatalog } from "@/lib/supabase/catalogs/insertCatalog"; import { insertAccountCatalog } from "@/lib/supabase/account_catalogs/insertAccountCatalog"; import { insertCatalogSongs } from "@/lib/supabase/catalog_songs/insertCatalogSongs"; import { updatePlaycountSnapshot } from "@/lib/supabase/playcount_snapshots/updatePlaycountSnapshot"; -import { selectSnapshotIsrcs } from "@/lib/supabase/song_measurements/selectSnapshotIsrcs"; +import { selectSongMeasurements } from "@/lib/supabase/song_measurements/selectSongMeasurements"; vi.mock("@/lib/supabase/catalogs/insertCatalog", () => ({ insertCatalog: vi.fn() })); vi.mock("@/lib/supabase/account_catalogs/insertAccountCatalog", () => ({ @@ -15,8 +15,8 @@ vi.mock("@/lib/supabase/catalog_songs/insertCatalogSongs", () => ({ insertCatalo vi.mock("@/lib/supabase/playcount_snapshots/updatePlaycountSnapshot", () => ({ updatePlaycountSnapshot: vi.fn(), })); -vi.mock("@/lib/supabase/song_measurements/selectSnapshotIsrcs", () => ({ - selectSnapshotIsrcs: vi.fn(), +vi.mock("@/lib/supabase/song_measurements/selectSongMeasurements", () => ({ + selectSongMeasurements: vi.fn(), })); const accountId = "550e8400-e29b-41d4-a716-446655440000"; @@ -24,8 +24,9 @@ const snapshotId = "11111111-2222-3333-4444-555555555555"; const catalogId = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"; const catalog = { id: catalogId, name: "Bad Bunny Catalog", created_at: "t", updated_at: "t" }; // A real valuation snapshot is scoped by album_ids, so its own `isrcs` column is null — -// the measured ISRCs live in song_measurements (sourced via selectSnapshotIsrcs). +// the measured ISRCs live in song_measurements (sourced via selectSongMeasurements). const snapshot = { id: snapshotId, account: accountId, catalog: null, isrcs: null } as never; +const measurement = (song: string) => ({ song }) as never; describe("createSnapshotCatalog", () => { beforeEach(() => { @@ -33,15 +34,19 @@ describe("createSnapshotCatalog", () => { vi.mocked(insertCatalog).mockResolvedValue(catalog); }); - it("sources measured ISRCs from song_measurements and adds them as catalog songs", async () => { - vi.mocked(selectSnapshotIsrcs).mockResolvedValue(["ISRC_A", "ISRC_B", "ISRC_C"]); + it("sources measured ISRCs from song_measurements (by snapshot) and adds them as catalog songs", async () => { + vi.mocked(selectSongMeasurements).mockResolvedValue([ + measurement("ISRC_A"), + measurement("ISRC_B"), + measurement("ISRC_C"), + ]); const result = await createSnapshotCatalog({ accountId, snapshot, name: "Bad Bunny Catalog" }); expect(insertCatalog).toHaveBeenCalledWith("Bad Bunny Catalog"); expect(insertAccountCatalog).toHaveBeenCalledWith({ account: accountId, catalog: catalogId }); - // ISRCs come from measurements, NOT snapshot.isrcs (which is null here) - expect(selectSnapshotIsrcs).toHaveBeenCalledWith(snapshotId); + // ISRCs come from measurements by snapshot, NOT snapshot.isrcs (null here) + expect(selectSongMeasurements).toHaveBeenCalledWith({ snapshot: snapshotId }); expect(insertCatalogSongs).toHaveBeenCalledWith([ { catalog: catalogId, song: "ISRC_A" }, { catalog: catalogId, song: "ISRC_B" }, @@ -51,8 +56,24 @@ describe("createSnapshotCatalog", () => { expect(result).toEqual({ catalog, songsAdded: 3 }); }); + it("dedupes ISRCs across multiple measurement rows per track", async () => { + vi.mocked(selectSongMeasurements).mockResolvedValue([ + measurement("ISRC_A"), + measurement("ISRC_A"), + measurement("ISRC_B"), + ]); + + const result = await createSnapshotCatalog({ accountId, snapshot }); + + expect(insertCatalogSongs).toHaveBeenCalledWith([ + { catalog: catalogId, song: "ISRC_A" }, + { catalog: catalogId, song: "ISRC_B" }, + ]); + expect(result).toEqual({ catalog, songsAdded: 2 }); + }); + it("adds no songs when the snapshot has no measurements", async () => { - vi.mocked(selectSnapshotIsrcs).mockResolvedValue([]); + vi.mocked(selectSongMeasurements).mockResolvedValue([]); const result = await createSnapshotCatalog({ accountId, snapshot }); @@ -62,7 +83,7 @@ describe("createSnapshotCatalog", () => { }); it("falls back to a default name when none is supplied", async () => { - vi.mocked(selectSnapshotIsrcs).mockResolvedValue([]); + vi.mocked(selectSongMeasurements).mockResolvedValue([]); await createSnapshotCatalog({ accountId, snapshot }); diff --git a/lib/catalog/createSnapshotCatalog.ts b/lib/catalog/createSnapshotCatalog.ts index 982775b74..ab1ba96c2 100644 --- a/lib/catalog/createSnapshotCatalog.ts +++ b/lib/catalog/createSnapshotCatalog.ts @@ -3,7 +3,7 @@ import { insertCatalog } from "@/lib/supabase/catalogs/insertCatalog"; import { insertAccountCatalog } from "@/lib/supabase/account_catalogs/insertAccountCatalog"; import { insertCatalogSongs } from "@/lib/supabase/catalog_songs/insertCatalogSongs"; import { updatePlaycountSnapshot } from "@/lib/supabase/playcount_snapshots/updatePlaycountSnapshot"; -import { selectSnapshotIsrcs } from "@/lib/supabase/song_measurements/selectSnapshotIsrcs"; +import { selectSongMeasurements } from "@/lib/supabase/song_measurements/selectSongMeasurements"; const DEFAULT_CATALOG_NAME = "Valuation Catalog"; @@ -33,7 +33,8 @@ export async function createSnapshotCatalog(params: { const catalog = await insertCatalog(name ?? DEFAULT_CATALOG_NAME); await insertAccountCatalog({ account: accountId, catalog: catalog.id }); - const isrcs = await selectSnapshotIsrcs(snapshot.id); + const measurements = await selectSongMeasurements({ snapshot: snapshot.id }); + const isrcs = [...new Set(measurements.map(m => m.song))]; if (isrcs.length > 0) { await insertCatalogSongs(isrcs.map(isrc => ({ catalog: catalog.id, song: isrc }))); } diff --git a/lib/supabase/song_measurements/selectSnapshotIsrcs.ts b/lib/supabase/song_measurements/selectSnapshotIsrcs.ts deleted file mode 100644 index f0f50dc1e..000000000 --- a/lib/supabase/song_measurements/selectSnapshotIsrcs.ts +++ /dev/null @@ -1,24 +0,0 @@ -import supabase from "../serverClient"; - -/** - * Returns the distinct ISRCs that were measured for a snapshot — the tracks - * that actually landed play counts in `song_measurements` for this run. This is - * the source of truth for a snapshot's tracks: the snapshot's own `isrcs` - * column is only set for ISRC-scoped runs, whereas valuations are album-scoped. - * - * @param snapshotId - The snapshot id (lineage on `song_measurements.snapshot`) - * @returns Distinct ISRC strings, or [] if none exist or on error - */ -export async function selectSnapshotIsrcs(snapshotId: string): Promise { - const { data, error } = await supabase - .from("song_measurements") - .select("song") - .eq("snapshot", snapshotId); - - if (error) { - console.error("Error fetching snapshot ISRCs:", error); - return []; - } - - return [...new Set((data ?? []).map(row => row.song))]; -} diff --git a/lib/supabase/song_measurements/selectSongMeasurements.ts b/lib/supabase/song_measurements/selectSongMeasurements.ts index 24d50bd44..1e118d14b 100644 --- a/lib/supabase/song_measurements/selectSongMeasurements.ts +++ b/lib/supabase/song_measurements/selectSongMeasurements.ts @@ -2,13 +2,14 @@ import supabase from "../serverClient"; import { Tables } from "@/types/database.types"; /** - * Select measurements newest-first. Filter by one song or a batch of songs - * (one is required), and optionally by platform/metric. Uses the - * (song, platform, metric, captured_at DESC) series index; pass `limit: 1` - * for the latest capture, omit for the full series. + * Select measurements newest-first. Filter by one song, a batch of songs, or a + * snapshot (one of the three is required), and optionally by platform/metric. + * Uses the (song, platform, metric, captured_at DESC) series index; pass + * `limit: 1` for the latest capture, omit for the full series. * * @param params.song - Single song ISRC filter * @param params.songs - Batch of song ISRCs (e.g. all tracks on an album) + * @param params.snapshot - Snapshot id filter (all measurements captured for a run) * @param params.platform - Optional platform filter (e.g. "spotify") * @param params.metric - Optional metric filter (e.g. "platform_displayed_play_count") * @param params.limit - Optional cap on returned rows @@ -17,17 +18,19 @@ import { Tables } from "@/types/database.types"; export async function selectSongMeasurements({ song, songs, + snapshot, platform, metric, limit, }: { song?: string; songs?: string[]; + snapshot?: string; platform?: string; metric?: string; limit?: number; }): Promise[]> { - if (!song && (!songs || songs.length === 0)) return []; + if (!song && (!songs || songs.length === 0) && !snapshot) return []; let query = supabase .from("song_measurements") @@ -36,6 +39,7 @@ export async function selectSongMeasurements({ if (song) query = query.eq("song", song); if (songs && songs.length > 0) query = query.in("song", songs); + if (snapshot) query = query.eq("snapshot", snapshot); if (platform) query = query.eq("platform", platform); if (metric) query = query.eq("metric", metric); if (limit) query = query.limit(limit);