-
Notifications
You must be signed in to change notification settings - Fork 9
feat(api): migrate GET /api/artists/pro #477
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
754ccbb
33b2c3a
2ead2d3
a65d688
0503921
7336ef9
22a6025
235797a
339808d
299d2f1
3e68114
98e5922
5947a3d
6b5057f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,29 @@ | ||||||||||||||||||||||||||||||||||||||||||||
| import { NextRequest, NextResponse } from "next/server"; | ||||||||||||||||||||||||||||||||||||||||||||
| import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; | ||||||||||||||||||||||||||||||||||||||||||||
| import { getArtistsProHandler } from "@/lib/artists/getArtistsProHandler"; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||
| * OPTIONS handler for CORS preflight requests. | ||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||
| * @returns A NextResponse with CORS headers. | ||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||
| export async function OPTIONS() { | ||||||||||||||||||||||||||||||||||||||||||||
| return new NextResponse(null, { | ||||||||||||||||||||||||||||||||||||||||||||
| status: 200, | ||||||||||||||||||||||||||||||||||||||||||||
| headers: getCorsHeaders(), | ||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||
| * GET /api/admins/artists/pro | ||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||
| * Returns a deduplicated list of artist IDs owned by "pro" accounts | ||||||||||||||||||||||||||||||||||||||||||||
| * (enterprise-domain emails or active Stripe subscriptions). Admin-scoped — | ||||||||||||||||||||||||||||||||||||||||||||
| * the response is the paying-customer list. | ||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||
| * @param request - The incoming request. | ||||||||||||||||||||||||||||||||||||||||||||
| * @returns A NextResponse with `{ status, artists, [error] }`. | ||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+17
to
+26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Stale JSDoc: "active Stripe subscriptions" no longer reflects the implementation. Per the PR's commit history, later commits replaced the Stripe API calls with reads from a local Supabase subscriptions mirror. Keeping the Stripe reference in this doc comment will mislead future readers hunting down the source of truth for "active subscription" semantics — a small clean-code nit, but docs that drift from reality erode trust faster than bad code does. 📝 Suggested wording tweak /**
* GET /api/artists/pro
*
* Returns a deduplicated list of artist IDs owned by "pro" accounts
- * (enterprise-domain emails or active Stripe subscriptions). Admin-scoped —
- * the response is the paying-customer list.
+ * (enterprise-domain emails or accounts with an active subscription in the
+ * local subscriptions mirror). Admin-scoped — the response is the
+ * paying-customer list.
*
* `@param` request - The incoming request.
* `@returns` A NextResponse with `{ status, artists, [error] }`.
*/📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||
| export async function GET(request: NextRequest) { | ||||||||||||||||||||||||||||||||||||||||||||
| return getArtistsProHandler(request); | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| import { describe, it, expect, vi, beforeEach } from "vitest"; | ||
| import { NextRequest, NextResponse } from "next/server"; | ||
| import { getArtistsProHandler } from "../getArtistsProHandler"; | ||
| import { validateGetArtistsProRequest } from "../validateGetArtistsProRequest"; | ||
| import { getProArtists } from "@/lib/artists/getProArtists"; | ||
|
|
||
| vi.mock("@/lib/networking/getCorsHeaders", () => ({ | ||
| getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), | ||
| })); | ||
|
|
||
| vi.mock("../validateGetArtistsProRequest", () => ({ | ||
| validateGetArtistsProRequest: vi.fn(), | ||
| })); | ||
|
|
||
| vi.mock("@/lib/artists/getProArtists", () => ({ | ||
| getProArtists: vi.fn(), | ||
| })); | ||
|
|
||
| describe("getArtistsProHandler", () => { | ||
| const request = new NextRequest("http://localhost/api/admins/artists/pro"); | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| vi.mocked(validateGetArtistsProRequest).mockResolvedValue({}); | ||
| }); | ||
|
|
||
| it.each([ | ||
| ["enterprise-only", ["a1", "a2"]], | ||
| ["subscription-only", ["b1"]], | ||
| ["both enterprise and subscription, deduped", ["a1", "b1", "c1"]], | ||
| ["neither — empty", []], | ||
| ])("returns %s artists on 200", async (_label, artists) => { | ||
| vi.mocked(getProArtists).mockResolvedValue(artists); | ||
|
|
||
| const response = await getArtistsProHandler(request); | ||
| const body = await response.json(); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| expect(body).toEqual({ status: "success", artists }); | ||
| }); | ||
|
|
||
| it("propagates the 401 response when validator fails auth", async () => { | ||
| const err = NextResponse.json({ status: "error", error: "no auth" }, { status: 401 }); | ||
| vi.mocked(validateGetArtistsProRequest).mockResolvedValue(err); | ||
|
|
||
| const response = await getArtistsProHandler(request); | ||
|
|
||
| expect(response).toBe(err); | ||
| expect(getProArtists).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("propagates the 403 response for non-admin callers", async () => { | ||
| const err = NextResponse.json({ status: "error", message: "Forbidden" }, { status: 403 }); | ||
| vi.mocked(validateGetArtistsProRequest).mockResolvedValue(err); | ||
|
|
||
| const response = await getArtistsProHandler(request); | ||
|
|
||
| expect(response).toBe(err); | ||
| expect(getProArtists).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("returns a generic 500 without leaking error details", async () => { | ||
| vi.mocked(getProArtists).mockRejectedValue( | ||
| new Error("db.internal.host=10.0.0.5 connection refused"), | ||
| ); | ||
|
|
||
| const response = await getArtistsProHandler(request); | ||
| const bodyText = JSON.stringify(await response.json()); | ||
|
|
||
| expect(response.status).toBe(500); | ||
| expect(bodyText).not.toContain("db.internal"); | ||
| expect(bodyText).not.toContain("10.0.0.5"); | ||
| expect(bodyText).not.toContain("connection refused"); | ||
| expect(JSON.parse(bodyText)).toEqual({ | ||
| status: "error", | ||
| artists: [], | ||
| error: "Internal server error", | ||
| }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| import { describe, it, expect, vi, beforeEach } from "vitest"; | ||
| import { NextRequest, NextResponse } from "next/server"; | ||
| import { validateGetArtistsProRequest } from "../validateGetArtistsProRequest"; | ||
| import { validateAdminAuth } from "@/lib/admins/validateAdminAuth"; | ||
|
|
||
| vi.mock("@/lib/networking/getCorsHeaders", () => ({ | ||
| getCorsHeaders: vi.fn(() => ({ "Access-Control-Allow-Origin": "*" })), | ||
| })); | ||
|
|
||
| vi.mock("@/lib/admins/validateAdminAuth", () => ({ | ||
| validateAdminAuth: vi.fn(), | ||
| })); | ||
|
|
||
| describe("validateGetArtistsProRequest", () => { | ||
| const request = new NextRequest("http://localhost/api/admins/artists/pro"); | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it("returns an empty payload when admin auth passes", async () => { | ||
| vi.mocked(validateAdminAuth).mockResolvedValue({ | ||
| accountId: "acc-1", | ||
| orgId: null, | ||
| authToken: "t", | ||
| }); | ||
|
|
||
| const result = await validateGetArtistsProRequest(request); | ||
|
|
||
| expect(result).toEqual({}); | ||
| expect(validateAdminAuth).toHaveBeenCalledWith(request); | ||
| }); | ||
|
|
||
| it("propagates the 401 NextResponse when auth is missing", async () => { | ||
| const err = NextResponse.json({ status: "error", error: "no auth" }, { status: 401 }); | ||
| vi.mocked(validateAdminAuth).mockResolvedValue(err); | ||
|
|
||
| const result = await validateGetArtistsProRequest(request); | ||
|
|
||
| expect(result).toBe(err); | ||
| }); | ||
|
|
||
| it("propagates the 403 NextResponse when caller is not admin", async () => { | ||
| const err = NextResponse.json({ status: "error", message: "Forbidden" }, { status: 403 }); | ||
| vi.mocked(validateAdminAuth).mockResolvedValue(err); | ||
|
|
||
| const result = await validateGetArtistsProRequest(request); | ||
|
|
||
| expect(result).toBe(err); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| import { NextRequest, NextResponse } from "next/server"; | ||
| import { getCorsHeaders } from "@/lib/networking/getCorsHeaders"; | ||
| import { getProArtists } from "@/lib/artists/getProArtists"; | ||
| import { validateGetArtistsProRequest } from "@/lib/artists/validateGetArtistsProRequest"; | ||
|
|
||
| export async function getArtistsProHandler(request: NextRequest): Promise<NextResponse> { | ||
| try { | ||
| const validated = await validateGetArtistsProRequest(request); | ||
| if (validated instanceof NextResponse) { | ||
| return validated; | ||
| } | ||
|
|
||
| const artists = await getProArtists(); | ||
|
|
||
| return NextResponse.json( | ||
| { status: "success", artists }, | ||
| { status: 200, headers: getCorsHeaders() }, | ||
| ); | ||
| } catch (error) { | ||
| // Never leak error.message — it can surface DB hosts / stack hints. | ||
| console.error("[ERROR] getArtistsProHandler:", error); | ||
| return NextResponse.json( | ||
| { status: "error", artists: [], error: "Internal server error" }, | ||
| { status: 500, headers: getCorsHeaders() }, | ||
| ); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| import { getEnterpriseAccountIds } from "@/lib/enterprise/getEnterpriseAccountIds"; | ||
| import selectSubscriptions from "@/lib/supabase/subscriptions/selectSubscriptions"; | ||
| import { selectAccountArtistIds } from "@/lib/supabase/account_artist_ids/selectAccountArtistIds"; | ||
|
|
||
| /** | ||
| * Deduped artist_ids owned by "pro" accounts — enterprise email domain OR | ||
| * active Stripe subscription. Dedupes at both layers (account_id then | ||
| * artist_id) so accounts present in both sources and artists shared across | ||
| * accounts do not produce cartesian-product rows. Short-circuits before the | ||
| * account_artist_ids query when no pro accounts are found. | ||
| */ | ||
| export async function getProArtists(): Promise<string[]> { | ||
| const [enterpriseIds, subscriptionRows] = await Promise.all([ | ||
| getEnterpriseAccountIds(), | ||
| selectSubscriptions({ active: true }), | ||
| ]); | ||
|
|
||
| const subscriberIds = subscriptionRows | ||
| .map(row => row.account_id) | ||
| .filter((id): id is string => Boolean(id)); | ||
|
|
||
| const accountIds = Array.from(new Set([...enterpriseIds, ...subscriberIds])); | ||
| if (accountIds.length === 0) return []; | ||
|
|
||
| const artistIdRows = await selectAccountArtistIds(accountIds); | ||
|
|
||
| return Array.from( | ||
| new Set(artistIdRows.map(row => row.artist_id).filter((id): id is string => Boolean(id))), | ||
| ); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| import { NextRequest, NextResponse } from "next/server"; | ||
| import { validateAdminAuth } from "@/lib/admins/validateAdminAuth"; | ||
|
|
||
| /** | ||
| * Scope, not access: the response is the list of all pro-tier artists — the | ||
| * paying-customer set. There is no per-row id to bind an access check to, so | ||
| * admin-scope is enforced at the request boundary. Reuses `validateAdminAuth` | ||
| * (validateAuthContext + Recoup-org membership check) rather than forking. | ||
| */ | ||
| export async function validateGetArtistsProRequest( | ||
| request: NextRequest, | ||
| ): Promise<NextResponse | Record<string, never>> { | ||
| const auth = await validateAdminAuth(request); | ||
| if (auth instanceof NextResponse) { | ||
| return auth; | ||
| } | ||
| return {}; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| /** | ||
| * Email domains that unconditionally mark an account as "pro" regardless of | ||
| * Stripe subscription state. Ported from Recoup-Agent-APIs/lib/consts.ts. | ||
| */ | ||
| export const ENTERPRISE_DOMAINS: ReadonlySet<string> = new Set([ | ||
| "recoupable.com", | ||
| "rostrum.com", | ||
| "spaceheatermusic.io", | ||
| "fatbeats.com", | ||
| "cantorarecords.net", | ||
| "rostrumrecords.com", | ||
| ]); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| import { ENTERPRISE_DOMAINS } from "@/lib/enterprise/consts"; | ||
| import selectAccountEmails from "@/lib/supabase/account_emails/selectAccountEmails"; | ||
|
|
||
| /** | ||
| * Resolve account_ids whose email belongs to an enterprise domain. Fans out | ||
| * one ilike query per domain in parallel and projects down to account_ids — | ||
| * the email rows themselves aren't consumed downstream. | ||
| */ | ||
| export async function getEnterpriseAccountIds(): Promise<string[]> { | ||
| const rowsPerDomain = await Promise.all( | ||
| Array.from(ENTERPRISE_DOMAINS).map(domain => selectAccountEmails({ domain })), | ||
| ); | ||
|
|
||
| return rowsPerDomain | ||
| .flat() | ||
| .map(row => row.account_id) | ||
| .filter((id): id is string => Boolean(id)); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| import supabase from "@/lib/supabase/serverClient"; | ||
|
|
||
| /** | ||
| * Project `artist_id` values for the given account_ids. Returns `[]` when | ||
| * `account_ids` is empty to avoid the undefined `.in(..., [])` behavior. | ||
| */ | ||
| export async function selectAccountArtistIds(accountIds: string[]) { | ||
| if (accountIds.length === 0) return []; | ||
|
|
||
| const { data, error } = await supabase | ||
| .from("account_artist_ids") | ||
| .select("artist_id") | ||
| .in("account_id", accountIds); | ||
|
|
||
| if (error) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Database errors are being swallowed and converted to an empty result, which can silently produce incorrect responses instead of triggering proper error handling. Prompt for AI agents |
||
| console.error("Error fetching account_artist_ids:", error); | ||
| return []; | ||
| } | ||
|
|
||
| return data ?? []; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| import supabase from "@/lib/supabase/serverClient"; | ||
| import type { Tables } from "@/types/database.types"; | ||
|
|
||
| /** | ||
| * Select subscription rows, optionally filtered by account IDs and/or active | ||
| * state. The `subscriptions` table is the local mirror of cross-provider | ||
| * billing state (stripe, lemon-squeezy, paddle). | ||
| */ | ||
| export default async function selectSubscriptions({ | ||
| accountIds, | ||
| active, | ||
| }: { | ||
| accountIds?: string | string[]; | ||
| active?: boolean; | ||
| } = {}): Promise<Tables<"subscriptions">[]> { | ||
| let query = supabase.from("subscriptions").select("*"); | ||
|
|
||
| if (accountIds !== undefined) { | ||
| const ids = Array.isArray(accountIds) ? accountIds : [accountIds]; | ||
| query = query.in("account_id", ids); | ||
| } | ||
|
|
||
| if (active !== undefined) { | ||
| query = query.eq("active", active); | ||
| } | ||
|
|
||
| const { data, error } = await query; | ||
|
|
||
| if (error) { | ||
| console.error("Error fetching subscriptions:", error); | ||
| return []; | ||
| } | ||
|
|
||
| return data || []; | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Custom agent: Module should export a single primary function whose name matches the filename
Module exports multiple top-level functions instead of a single primary export, violating the filename-to-export convention.
Prompt for AI agents