diff --git a/lib/supabase/socials/selectSocials.ts b/lib/supabase/socials/selectSocials.ts index ee04075b0..9cf37bf9a 100644 --- a/lib/supabase/socials/selectSocials.ts +++ b/lib/supabase/socials/selectSocials.ts @@ -4,6 +4,13 @@ import supabase from "../serverClient"; type SelectSocialsParams = { id?: string; profile_url?: string; + /** + * Substring match (ilike) on profile_url. Use for Spotify-id lookups: + * profile_url is stored inconsistently — with and without a scheme, + * sometimes with a ?si= query — so the exact-match `profile_url` param + * misses real matches (chat#1889 row 8). + */ + profileUrlContains?: string; }; /** @@ -26,6 +33,10 @@ export async function selectSocials( query = query.eq("profile_url", params.profile_url); } + if (params.profileUrlContains) { + query = query.ilike("profile_url", `%${params.profileUrlContains}%`); + } + const { data, error } = await query; if (error) { diff --git a/lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts b/lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts new file mode 100644 index 000000000..e991e1f6c --- /dev/null +++ b/lib/valuation/__tests__/findCanonicalArtistBySpotifyId.test.ts @@ -0,0 +1,59 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { findCanonicalArtistBySpotifyId } from "@/lib/valuation/findCanonicalArtistBySpotifyId"; +import { selectSocials } from "@/lib/supabase/socials/selectSocials"; +import { selectAccountSocials } from "@/lib/supabase/account_socials/selectAccountSocials"; + +vi.mock("@/lib/supabase/socials/selectSocials", () => ({ + selectSocials: vi.fn(), +})); +vi.mock("@/lib/supabase/account_socials/selectAccountSocials", () => ({ + selectAccountSocials: vi.fn(), +})); + +const SPOTIFY_ID = "0xPoVNPnxIIUS1vrxAYV00"; + +describe("findCanonicalArtistBySpotifyId", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + // Artists are canonical and shared (chat#1866); account_artist_ids is the + // join that lets many accounts roster one artist. So the lookup is global, + // NOT scoped to the requesting account — otherwise every account mints its + // own copy of the same Spotify artist (chat#1889 row 8). + it("finds the artist globally, not scoped to any account", async () => { + vi.mocked(selectSocials).mockResolvedValue([{ id: "social-1" }] as never); + vi.mocked(selectAccountSocials).mockResolvedValue([ + { account_id: "canonical-artist-1" }, + ] as never); + + const found = await findCanonicalArtistBySpotifyId(SPOTIFY_ID); + + expect(found).toBe("canonical-artist-1"); + expect(selectSocials).toHaveBeenCalledWith({ profileUrlContains: SPOTIFY_ID }); + }); + + it("returns null when no social carries that Spotify id", async () => { + vi.mocked(selectSocials).mockResolvedValue([] as never); + + const found = await findCanonicalArtistBySpotifyId(SPOTIFY_ID); + + expect(found).toBeNull(); + expect(selectAccountSocials).not.toHaveBeenCalled(); + }); + + it("returns null when the social exists but no artist is linked to it", async () => { + vi.mocked(selectSocials).mockResolvedValue([{ id: "social-1" }] as never); + vi.mocked(selectAccountSocials).mockResolvedValue([] as never); + + expect(await findCanonicalArtistBySpotifyId(SPOTIFY_ID)).toBeNull(); + }); + + // Never fail a valuation over a dedup lookup: falling back to creating an + // artist is strictly better than a 500. + it("returns null when the lookup throws", async () => { + vi.mocked(selectSocials).mockRejectedValue(new Error("db down")); + + expect(await findCanonicalArtistBySpotifyId(SPOTIFY_ID)).toBeNull(); + }); +}); diff --git a/lib/valuation/__tests__/linkSearchedArtistToAccount.test.ts b/lib/valuation/__tests__/linkSearchedArtistToAccount.test.ts index 968edc3ac..5e262f5f8 100644 --- a/lib/valuation/__tests__/linkSearchedArtistToAccount.test.ts +++ b/lib/valuation/__tests__/linkSearchedArtistToAccount.test.ts @@ -3,9 +3,21 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { linkSearchedArtistToAccount } from "../linkSearchedArtistToAccount"; import { createArtistInDb } from "@/lib/artists/createArtistInDb"; import { updateArtistSocials } from "@/lib/artist/updateArtistSocials"; +import { findCanonicalArtistBySpotifyId } from "@/lib/valuation/findCanonicalArtistBySpotifyId"; +import { selectAccountArtistId } from "@/lib/supabase/account_artist_ids/selectAccountArtistId"; +import { insertAccountArtistId } from "@/lib/supabase/account_artist_ids/insertAccountArtistId"; vi.mock("@/lib/artists/createArtistInDb", () => ({ createArtistInDb: vi.fn() })); vi.mock("@/lib/artist/updateArtistSocials", () => ({ updateArtistSocials: 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(), +})); const accountId = "550e8400-e29b-41d4-a716-446655440000"; const spotifyArtistId = "4q3ewBCX7sLwd24euuV69X"; @@ -17,6 +29,9 @@ describe("linkSearchedArtistToAccount", () => { vi.clearAllMocks(); vi.mocked(createArtistInDb).mockResolvedValue({ account_id: createdArtistId } as never); vi.mocked(updateArtistSocials).mockResolvedValue([] as never); + vi.mocked(findCanonicalArtistBySpotifyId).mockResolvedValue(null); + vi.mocked(selectAccountArtistId).mockResolvedValue(null as never); + vi.mocked(insertAccountArtistId).mockResolvedValue(undefined as never); }); it("creates the searched artist, attaches its Spotify social, and returns the new id", async () => { @@ -57,4 +72,26 @@ describe("linkSearchedArtistToAccount", () => { linkSearchedArtistToAccount({ accountId, spotifyArtistId, artistName }), ).resolves.toBeNull(); }); + // Artists are canonical and shared (chat#1866). Creating a second one for a + // Spotify id that already exists is what duplicated every cold-start signup's + // roster (chat#1889 row 8). + it("rosters the existing canonical artist instead of creating a duplicate", async () => { + vi.mocked(findCanonicalArtistBySpotifyId).mockResolvedValue("canonical-1"); + + const result = await linkSearchedArtistToAccount({ accountId, spotifyArtistId, artistName }); + + expect(createArtistInDb).not.toHaveBeenCalled(); + expect(insertAccountArtistId).toHaveBeenCalledWith(accountId, "canonical-1"); + expect(result).toBe("canonical-1"); + }); + + it("does not re-link a canonical artist the account already rosters", async () => { + vi.mocked(findCanonicalArtistBySpotifyId).mockResolvedValue("canonical-1"); + vi.mocked(selectAccountArtistId).mockResolvedValue({ id: "link-1" } as never); + + const result = await linkSearchedArtistToAccount({ accountId, spotifyArtistId, artistName }); + + expect(insertAccountArtistId).not.toHaveBeenCalled(); + expect(result).toBe("canonical-1"); + }); }); diff --git a/lib/valuation/findCanonicalArtistBySpotifyId.ts b/lib/valuation/findCanonicalArtistBySpotifyId.ts new file mode 100644 index 000000000..6f5c31267 --- /dev/null +++ b/lib/valuation/findCanonicalArtistBySpotifyId.ts @@ -0,0 +1,38 @@ +import { selectSocials } from "@/lib/supabase/socials/selectSocials"; +import { selectAccountSocials } from "@/lib/supabase/account_socials/selectAccountSocials"; + +/** + * The canonical artist account for a Spotify artist id, if one already exists. + * + * Artists are canonical and shared — `account_artist_ids` is the join that lets + * many accounts roster the same artist (chat#1866). So this lookup is + * deliberately **global**, not scoped to the requesting account: scoping it + * per-account is what lets every signup mint its own copy of the same Spotify + * artist, which is the duplicate `linkSearchedArtistToAccount` was producing + * (chat#1889 row 8). + * + * Best-effort: never throws. Failing this lookup falls back to creating an + * artist, which is strictly better than failing the valuation. + * + * @param spotifyArtistId - The Spotify artist id to resolve. + * @returns The artist's account id, or null when none exists yet. + */ +export async function findCanonicalArtistBySpotifyId( + spotifyArtistId: string, +): Promise { + try { + const socials = (await selectSocials({ profileUrlContains: spotifyArtistId })) ?? []; + if (socials.length === 0) return null; + + for (const social of socials) { + const links = await selectAccountSocials({ socialId: social.id }); + const linked = links.find(link => link.account_id); + if (linked?.account_id) return linked.account_id; + } + + return null; + } catch (error) { + console.error("Error resolving canonical artist by spotify id:", error); + return null; + } +} diff --git a/lib/valuation/linkSearchedArtistToAccount.ts b/lib/valuation/linkSearchedArtistToAccount.ts index 5009d7707..236c7cbdb 100644 --- a/lib/valuation/linkSearchedArtistToAccount.ts +++ b/lib/valuation/linkSearchedArtistToAccount.ts @@ -1,5 +1,8 @@ import { createArtistInDb } from "@/lib/artists/createArtistInDb"; import { updateArtistSocials } from "@/lib/artist/updateArtistSocials"; +import { findCanonicalArtistBySpotifyId } from "@/lib/valuation/findCanonicalArtistBySpotifyId"; +import { selectAccountArtistId } from "@/lib/supabase/account_artist_ids/selectAccountArtistId"; +import { insertAccountArtistId } from "@/lib/supabase/account_artist_ids/insertAccountArtistId"; /** * Fallback roster attach for the valuation flow: when the ISRC → song_artists @@ -9,6 +12,12 @@ import { updateArtistSocials } from "@/lib/artist/updateArtistSocials"; * roster it can confirm. Server-side, account-scoped version of the marketing * funnel's old client-side linkArtistToAccount. * + * Reuses the canonical artist when one already carries this Spotify id, and + * only creates when none does. Artists are canonical and shared — + * `account_artist_ids` is the join that lets many accounts roster the same + * artist (chat#1866) — so creating unconditionally minted a duplicate for + * every account whose first artist was added this way (chat#1889 row 8). + * * Link-only: the caller resolves the Spotify profile once and enriches the * returned artist separately (runValuationHandler → enrichSearchedArtistProfile), * so enrichment covers both the canonical and fallback paths (chat#1881 2a). @@ -29,6 +38,15 @@ export async function linkSearchedArtistToAccount(params: { try { if (!artistName) return null; + const canonicalId = await findCanonicalArtistBySpotifyId(spotifyArtistId); + if (canonicalId) { + const alreadyRostered = await selectAccountArtistId(accountId, canonicalId); + if (!alreadyRostered) { + await insertAccountArtistId(accountId, canonicalId); + } + return canonicalId; + } + const created = await createArtistInDb(artistName, accountId); if (!created?.account_id) return null;