diff --git a/components/Onboarding/AddArtistForm.tsx b/components/Onboarding/AddArtistForm.tsx index 3c32b393c..67529459f 100644 --- a/components/Onboarding/AddArtistForm.tsx +++ b/components/Onboarding/AddArtistForm.tsx @@ -1,24 +1,31 @@ "use client"; import { useState } from "react"; -import { Loader, Plus } from "lucide-react"; +import { Plus } from "lucide-react"; import { Button } from "@/components/ui/button"; -import { Input } from "@/components/ui/input"; +import SpotifyArtistSearch from "@/components/Artists/SpotifyArtistSearch"; import { useAddRosterArtist } from "@/hooks/onboarding/useAddRosterArtist"; +import type { SpotifyArtistSearchResult } from "@/types/spotify"; -/** Inline "add another artist" form for multi-artist managers. */ +/** + * "Add an artist" for the onboarding roster step, by Spotify typeahead. + * + * It used to take a free-text name, which produced an artist with no Spotify id + * — so no catalog and therefore no valuation could follow, and the payoff was + * structurally unreachable for anyone who arrived without a funnel valuation + * (chat#1889). Picking a real Spotify artist resolves the id, profile URL, and + * avatar in one step, reusing the same typeahead as verify-socials (chat#1882). + */ const AddArtistForm = () => { const { addArtist, isAdding } = useAddRosterArtist(); const [isOpen, setIsOpen] = useState(false); - const [name, setName] = useState(""); - const handleSubmit = async (e: React.FormEvent) => { - e.preventDefault(); - const added = await addArtist(name); - if (added) { - setName(""); - setIsOpen(false); - } + const handleSelect = async (artist: SpotifyArtistSearchResult) => { + const added = await addArtist(artist.name, { + profileUrl: artist.external_urls.spotify, + image: artist.images?.[0]?.url, + }); + if (added) setIsOpen(false); }; if (!isOpen) { @@ -36,27 +43,23 @@ const AddArtistForm = () => { } return ( -
- + setName(e.target.value)} - placeholder="Artist name" - disabled={isAdding} - aria-label="Artist name" + isBusy={isAdding} + onSelect={(artist) => void handleSelect(artist)} /> - - + ); }; diff --git a/components/Onboarding/ConfirmRosterStep.tsx b/components/Onboarding/ConfirmRosterStep.tsx index 67fdd9e3f..5e4df5998 100644 --- a/components/Onboarding/ConfirmRosterStep.tsx +++ b/components/Onboarding/ConfirmRosterStep.tsx @@ -3,6 +3,8 @@ import { Button } from "@/components/ui/button"; import { Skeleton } from "@/components/ui/skeleton"; import { useArtistProvider } from "@/providers/ArtistProvider"; +import useCatalogs from "@/hooks/useCatalogs"; +import { getConfirmRosterCopy } from "@/lib/onboarding/getConfirmRosterCopy"; import AddArtistForm from "./AddArtistForm"; import RosterArtistRow from "./RosterArtistRow"; @@ -13,7 +15,9 @@ import RosterArtistRow from "./RosterArtistRow"; */ const ConfirmRosterStep = ({ onConfirmed }: { onConfirmed: () => void }) => { const { sorted, isLoading } = useArtistProvider(); + const { data: catalogsData } = useCatalogs(); const artists = sorted.filter((artist) => !artist.isWorkspace); + const hasValuation = (catalogsData?.catalogs?.length ?? 0) > 0; return (
@@ -22,9 +26,10 @@ const ConfirmRosterStep = ({ onConfirmed }: { onConfirmed: () => void }) => { Confirm your roster

- We set {artists.length === 1 ? "this artist" : "these artists"} up - from your valuation. Add anyone else you manage — you can verify their - socials next. + {getConfirmRosterCopy({ + artistCount: artists.length, + hasValuation, + })}

@@ -36,7 +41,8 @@ const ConfirmRosterStep = ({ onConfirmed }: { onConfirmed: () => void }) => { ) : artists.length === 0 ? (

- No artists on your roster yet. Add your first artist below. + No artists on your roster yet. Add your first artist below to get + your catalog valued.

) : ( artists.map((artist) => ( @@ -52,7 +58,7 @@ const ConfirmRosterStep = ({ onConfirmed }: { onConfirmed: () => void }) => { disabled={isLoading || artists.length === 0} onClick={onConfirmed} > - {artists.length > 1 ? "These are my artists" : "This is my artist"} — + {artists.length > 1 ? "These are my artists" : "This is my artist"}, continue
diff --git a/components/Onboarding/VerifySocialsStep.tsx b/components/Onboarding/VerifySocialsStep.tsx index bef078593..ab550abc8 100644 --- a/components/Onboarding/VerifySocialsStep.tsx +++ b/components/Onboarding/VerifySocialsStep.tsx @@ -24,7 +24,7 @@ const VerifySocialsStep = ({ onConfirmed }: { onConfirmed: () => void }) => {

These are the profiles we matched. Fix any that point at the wrong - account — reports and tasks pull from them. + account. Reports and tasks pull from them.

@@ -40,7 +40,7 @@ const VerifySocialsStep = ({ onConfirmed }: { onConfirmed: () => void }) => { ); diff --git a/components/Onboarding/__tests__/AddArtistForm.test.tsx b/components/Onboarding/__tests__/AddArtistForm.test.tsx new file mode 100644 index 000000000..7e0b569c5 --- /dev/null +++ b/components/Onboarding/__tests__/AddArtistForm.test.tsx @@ -0,0 +1,58 @@ +// @vitest-environment jsdom +import React from "react"; +import { fireEvent, render, screen } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import AddArtistForm from "@/components/Onboarding/AddArtistForm"; + +const addArtist = vi.fn(); + +const SPOTIFY_RESULT = { + id: "3TVXtAsR1Inumwj472S9r4", + name: "Drake", + external_urls: { spotify: "https://open.spotify.com/artist/3TVXtAsR1Inumwj472S9r4" }, + images: [{ url: "https://i.scdn.co/image/drake.jpg" }], + followers: { total: 92000000 }, +}; + +vi.mock("@/hooks/onboarding/useAddRosterArtist", () => ({ + useAddRosterArtist: () => ({ addArtist, isAdding: false }), +})); + +vi.mock("@/hooks/useSpotifyArtistSearch", () => ({ + useSpotifyArtistSearch: () => ({ + results: [SPOTIFY_RESULT], + isSearching: false, + }), +})); + +describe("AddArtistForm", () => { + beforeEach(() => { + addArtist.mockClear(); + addArtist.mockResolvedValue(true); + }); + + const openForm = () => { + render(); + fireEvent.click(screen.getByRole("button", { name: /add another artist/i })); + }; + + it("adds by Spotify typeahead, not a free-text name field", () => { + openForm(); + + // The free-text input produced artists with no Spotify id, so no catalog and + // no valuation could follow — the whole payoff was unreachable for anyone + // who arrived without a funnel valuation (chat#1889). + expect(screen.queryByLabelText(/^artist name$/i)).toBeNull(); + expect(screen.getByLabelText(/search spotify for an artist/i)).toBeDefined(); + }); + + it("resolves the picked artist to its Spotify profile URL and avatar", () => { + openForm(); + fireEvent.click(screen.getByRole("option", { name: /drake/i })); + + expect(addArtist).toHaveBeenCalledWith("Drake", { + profileUrl: SPOTIFY_RESULT.external_urls.spotify, + image: SPOTIFY_RESULT.images[0].url, + }); + }); +}); diff --git a/hooks/onboarding/__tests__/useAddRosterArtist.test.tsx b/hooks/onboarding/__tests__/useAddRosterArtist.test.tsx new file mode 100644 index 000000000..f2be00ceb --- /dev/null +++ b/hooks/onboarding/__tests__/useAddRosterArtist.test.tsx @@ -0,0 +1,93 @@ +// @vitest-environment jsdom +import { act, renderHook } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { useAddRosterArtist } from "@/hooks/onboarding/useAddRosterArtist"; + +const createRosterArtist = vi.fn(); +const saveArtist = vi.fn(); +const getArtists = vi.fn(); +const toastError = vi.fn(); + +vi.mock("@privy-io/react-auth", () => ({ + usePrivy: () => ({ getAccessToken: async () => "token" }), +})); + +vi.mock("@/lib/artists/createRosterArtist", () => ({ + createRosterArtist: (...args: unknown[]) => createRosterArtist(...args), +})); + +vi.mock("@/lib/saveArtist", () => ({ + default: (...args: unknown[]) => saveArtist(...args), +})); + +vi.mock("@/providers/ArtistProvider", () => ({ + useArtistProvider: () => ({ getArtists }), +})); + +vi.mock("@/providers/OrganizationProvider", () => ({ + useOrganization: () => ({ selectedOrgId: "org-1" }), +})); + +vi.mock("sonner", () => ({ + toast: { error: (...args: unknown[]) => toastError(...args) }, +})); + +const SPOTIFY = { + profileUrl: "https://open.spotify.com/artist/43qxAkuKFB6fMNSeS5dO7Z", + image: "https://i.scdn.co/image/ana.jpg", +}; + +describe("useAddRosterArtist", () => { + beforeEach(() => { + vi.clearAllMocks(); + createRosterArtist.mockResolvedValue({ account_id: "artist-1" }); + saveArtist.mockResolvedValue(undefined); + }); + + it("enriches a created artist with its Spotify profile and avatar", async () => { + const { result } = renderHook(() => useAddRosterArtist()); + + let added: boolean | undefined; + await act(async () => { + added = await result.current.addArtist("Ana Bárbara", SPOTIFY); + }); + + expect(added).toBe(true); + expect(saveArtist).toHaveBeenCalledWith("token", "artist-1", { + profileUrls: { SPOTIFY: SPOTIFY.profileUrl }, + image: SPOTIFY.image, + }); + expect(getArtists).toHaveBeenCalled(); + }); + + // POST /api/artists has already succeeded by the time enrichment runs, so a + // failing PATCH must not report failure: the form would stay open over an + // artist that exists, and picking again creates a duplicate (chat#1889). + it("treats a failed enrichment as non-fatal", async () => { + saveArtist.mockRejectedValue(new Error("Forbidden")); + const { result } = renderHook(() => useAddRosterArtist()); + + let added: boolean | undefined; + await act(async () => { + added = await result.current.addArtist("Ana Bárbara", SPOTIFY); + }); + + expect(added).toBe(true); + expect(getArtists).toHaveBeenCalled(); + expect(toastError).not.toHaveBeenCalled(); + }); + + it("still reports failure when the artist itself cannot be created", async () => { + createRosterArtist.mockRejectedValue(new Error("nope")); + const { result } = renderHook(() => useAddRosterArtist()); + + let added: boolean | undefined; + await act(async () => { + added = await result.current.addArtist("Ana Bárbara", SPOTIFY); + }); + + expect(added).toBe(false); + expect(toastError).toHaveBeenCalled(); + expect(getArtists).not.toHaveBeenCalled(); + }); +}); diff --git a/hooks/onboarding/useAddRosterArtist.ts b/hooks/onboarding/useAddRosterArtist.ts index 965ec3d6f..2c4a40ee9 100644 --- a/hooks/onboarding/useAddRosterArtist.ts +++ b/hooks/onboarding/useAddRosterArtist.ts @@ -4,13 +4,28 @@ import { useState } from "react"; import { usePrivy } from "@privy-io/react-auth"; import { toast } from "sonner"; import { createRosterArtist } from "@/lib/artists/createRosterArtist"; +import saveArtist from "@/lib/saveArtist"; +import { buildSocialFixPayload } from "@/lib/onboarding/buildSocialFixPayload"; import { useArtistProvider } from "@/providers/ArtistProvider"; import { useOrganization } from "@/providers/OrganizationProvider"; +export interface AddRosterArtistOptions { + /** Spotify profile URL from the typeahead, linked as the artist's social. */ + profileUrl?: string; + /** Spotify avatar, so the roster row isn't a blank initial. */ + image?: string; +} + /** - * "Add another artist" for the onboarding roster step. Creates the - * artist through the existing `POST /api/artists` endpoint and refreshes - * the shared roster (ArtistProvider) so the new artist appears in-flow. + * "Add an artist" for the onboarding roster step. Creates the artist through + * `POST /api/artists`, then — when the caller resolved a real Spotify artist — + * attaches the profile URL and avatar via the existing + * `PATCH /api/artists/{id}` path, and refreshes the shared roster. + * + * The second call is what makes the added artist usable: `POST /api/artists` + * accepts only a name, and an artist with no Spotify id can never get a catalog + * or a valuation (chat#1889). Enrichment failing is non-fatal — the artist is + * already on the roster and its socials can still be fixed in the next step. */ export function useAddRosterArtist() { const { getAccessToken } = usePrivy(); @@ -18,7 +33,10 @@ export function useAddRosterArtist() { const { selectedOrgId } = useOrganization(); const [isAdding, setIsAdding] = useState(false); - const addArtist = async (name: string): Promise => { + const addArtist = async ( + name: string, + options: AddRosterArtistOptions = {}, + ): Promise => { const trimmed = name.trim(); if (!trimmed) return false; setIsAdding(true); @@ -27,7 +45,29 @@ export function useAddRosterArtist() { if (!accessToken) { throw new Error("Please sign in to add an artist"); } - await createRosterArtist(accessToken, trimmed, selectedOrgId); + const artist = await createRosterArtist( + accessToken, + trimmed, + selectedOrgId, + ); + + const payload = options.profileUrl + ? buildSocialFixPayload(options.profileUrl) + : null; + if (artist?.account_id && (payload || options.image)) { + try { + await saveArtist(accessToken, artist.account_id, { + ...(payload ? { profileUrls: payload.profileUrls } : {}), + ...(options.image ? { image: options.image } : {}), + }); + } catch { + // Swallowed on purpose. POST /api/artists already succeeded, so + // surfacing this would leave the form open over an artist that + // exists — and picking again would create a duplicate. The socials + // are still fixable in the next step. + } + } + await getArtists(); return true; } catch (error) { diff --git a/lib/onboarding/__tests__/getConfirmRosterCopy.test.ts b/lib/onboarding/__tests__/getConfirmRosterCopy.test.ts new file mode 100644 index 000000000..ee5add6c5 --- /dev/null +++ b/lib/onboarding/__tests__/getConfirmRosterCopy.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from "vitest"; +import { getConfirmRosterCopy } from "@/lib/onboarding/getConfirmRosterCopy"; + +describe("getConfirmRosterCopy", () => { + it("prompts an empty roster to search Spotify", () => { + const copy = getConfirmRosterCopy({ artistCount: 0, hasValuation: false }); + + expect(copy).toMatch(/search spotify/i); + }); + + it("credits the valuation only when one actually ran", () => { + expect( + getConfirmRosterCopy({ artistCount: 1, hasValuation: true }), + ).toMatch(/this artist up from your valuation/i); + expect( + getConfirmRosterCopy({ artistCount: 2, hasValuation: true }), + ).toMatch(/these artists up from your valuation/i); + }); + + // Em dashes read as machine-written and cost trust, so product copy uses + // periods or commas instead. + it("uses no em or en dashes in any branch", () => { + for (const hasValuation of [true, false]) { + for (const artistCount of [0, 1, 2, 5]) { + expect(getConfirmRosterCopy({ artistCount, hasValuation })).not.toMatch( + /[—–]/, + ); + } + } + }); + + // Two-thirds of welcome-email signups never ran a valuation. Crediting one + // is false for them the moment they add their first artist by typeahead, + // which is the only path this cohort has (chat#1889). + it("never claims a valuation the account never ran", () => { + for (const artistCount of [1, 2, 5]) { + const copy = getConfirmRosterCopy({ artistCount, hasValuation: false }); + + expect(copy).not.toMatch(/valuation/i); + expect(copy).toMatch(/verify their socials next/i); + } + }); +}); diff --git a/lib/onboarding/getConfirmRosterCopy.ts b/lib/onboarding/getConfirmRosterCopy.ts new file mode 100644 index 000000000..ef23f7752 --- /dev/null +++ b/lib/onboarding/getConfirmRosterCopy.ts @@ -0,0 +1,32 @@ +export interface ConfirmRosterCopyInput { + /** Artists on the roster, excluding the workspace pseudo-artist. */ + artistCount: number; + /** Whether a valuation actually ran for this account (it has a catalog). */ + hasValuation: boolean; +} + +/** + * Sub-heading for the confirm-roster onboarding step. + * + * The provenance line ("we set these up from your valuation") is only true for + * accounts that arrived through the valuation funnel. Two-thirds of + * welcome-email signups did not, and they reach a populated roster by adding an + * artist through the Spotify typeahead — so the claim is keyed on the valuation + * having happened, not on the roster being non-empty (chat#1889). + */ +export function getConfirmRosterCopy({ + artistCount, + hasValuation, +}: ConfirmRosterCopyInput): string { + if (artistCount === 0) { + return "Search Spotify for the artists you manage. Recoup uses them to measure your catalog, so pick the real profile."; + } + + const tail = + "Add anyone else you manage, you can verify their socials next."; + + if (!hasValuation) return tail; + + const subject = artistCount === 1 ? "this artist" : "these artists"; + return `We set ${subject} up from your valuation. ${tail}`; +}