feat: add artist detail read endpoint#416
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdded a new Next.js API endpoint Changes
Sequence DiagramsequenceDiagram
participant Client
participant RouteHandler as Route Handler<br/>/api/artists/[id]
participant Validator as validateGetArtistRequest
participant BusinessLogic as getArtistHandler
participant DB as selectAccountWithArtistDetails
participant Formatter as getFormattedArtist
Client->>RouteHandler: GET /api/artists/[id]
RouteHandler->>BusinessLogic: getArtistHandler(request, params)
BusinessLogic->>Validator: validateGetArtistRequest(request, id)
Validator->>Validator: validateAccountParams(id)
Validator->>Validator: validateAuthContext()
Validator->>Validator: checkAccountArtistAccess()
Validator-->>BusinessLogic: GetArtistRequest | NextResponse
alt Validation Failed
BusinessLogic-->>RouteHandler: Error NextResponse
RouteHandler-->>Client: 403/400 Error JSON
else Validation Passed
BusinessLogic->>DB: selectAccountWithArtistDetails(artistId)
DB-->>BusinessLogic: AccountWithArtistDetails | null
alt Artist Not Found
BusinessLogic-->>RouteHandler: 404 Error NextResponse
RouteHandler-->>Client: 404 Error JSON
else Artist Found
BusinessLogic->>Formatter: getFormattedArtist(account)
Formatter-->>BusinessLogic: Formatted artist data
BusinessLogic-->>RouteHandler: 200 Success NextResponse
RouteHandler-->>Client: 200 { artist }
end
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
2 issues found across 7 files
Confidence score: 3/5
- Access check in
lib/artists/getArtistHandler.tsruns after the 404 path, which can leak artist ID existence to unauthorized users (403 vs 404 behavior). lib/artists/getArtistById.tsspreadsformattedArtist, which can leakpinnedandisWorkspacefields into the API response at runtime despite the type hiding them.- Score 3 because there are concrete, user-facing response/authorization risks even though fixes look straightforward.
- Pay close attention to
lib/artists/getArtistHandler.tsandlib/artists/getArtistById.ts- authorization ordering and response field leakage.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/artists/getArtistById.ts">
<violation number="1" location="lib/artists/getArtistById.ts:32">
P2: Spreading `formattedArtist` leaks undeclared `pinned` and `isWorkspace` fields into the response at runtime. The `ArtistDetail` type hides them at compile time but they'll appear in the serialized JSON. Destructure only the needed fields to keep the response shape consistent with the declared interface.</violation>
</file>
<file name="lib/artists/getArtistHandler.ts">
<violation number="1" location="lib/artists/getArtistHandler.ts:26">
P2: Access check runs after the 404 response, allowing unauthorized users to enumerate valid artist IDs (404 vs 403). Move `validateAccountIdOverride` before the `getArtistById` call so denied users always receive 403 regardless of whether the artist exists.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client
participant API as API Route Handler
participant Auth as Auth & Access Service
participant DB as Supabase DB
Note over Client,DB: NEW: GET /api/artists/{id} Flow
Client->>API: GET /api/artists/{id}
rect rgb(240, 240, 240)
Note right of API: Request Validation Phase
API->>API: NEW: validateAccountParams(id)
API->>Auth: NEW: validateAuthContext(request)
Auth-->>API: requesterAccountId
alt Invalid UUID or Unauthenticated
API-->>Client: 400 Bad Request / 401 Unauthorized
end
end
API->>DB: NEW: selectAccountWithArtistDetails(id)
DB-->>API: Account + Info + Socials
alt Artist Not Found
API-->>Client: 404 Not Found (with CORS headers)
end
rect rgb(240, 240, 240)
Note right of API: Authorization Phase
API->>Auth: NEW: validateAccountIdOverride(requester, target)
Note over Auth: Checks if requester has permission<br/>to view this specific artist
alt Forbidden Access
Auth-->>API: 403 Forbidden
API-->>Client: 403 Forbidden
end
end
API->>API: NEW: getFormattedArtist(data)
Note right of API: Merges DB relations into<br/>canonical artist payload
API-->>Client: 200 OK (Artist JSON Payload)
Note over Client,API: Includes getCorsHeaders() in all responses
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
lib/supabase/accounts/selectAccountWithArtistDetails.ts (1)
1-1: Use the canonical Supabase client import path for consistency.Line 1 should import from
@/lib/supabase/serverClientto match the repository boundary rule for Supabase modules.Suggested fix
-import supabase from "../serverClient"; +import supabase from "@/lib/supabase/serverClient";As per coding guidelines, Supabase modules should import
@/lib/supabase/serverClientwithinlib/supabase/**.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/supabase/accounts/selectAccountWithArtistDetails.ts` at line 1, Replace the current import of the Supabase client to use the canonical repository path; update the import that brings in the symbol "supabase" in selectAccountWithArtistDetails.ts to import from "@/lib/supabase/serverClient" instead of "../serverClient" so it follows the lib/supabase module boundary convention.lib/artists/validateGetArtistRequest.ts (1)
19-50: Split schema validation from auth/access checks to match validator conventions.
validateGetArtistRequestcurrently combines param parsing, auth validation, and authorization policy. Consider keeping this file focused on schema-based input validation (+ inferred type), and moving access policy to a separate helper for better reuse/testability.As per coding guidelines, validation files in
lib/**/validate*.tsshould export schema/inferred types and follow focused responsibilities (SRP).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/artists/validateGetArtistRequest.ts` around lines 19 - 50, Split validateGetArtistRequest so it only handles schema/param validation and returns the inferred GetArtistRequest type (or NextResponse on validation failure) by extracting auth and authorization logic into a separate helper; remove calls to validateAuthContext and checkAccountArtistAccess from this file, export the validation schema and its inferred type from validateGetArtistRequest, and create a new function (e.g., ensureArtistAccess or authorizeArtistRequest) that takes the request and validatedParams and performs auth (validateAuthContext) plus access checks (checkAccountArtistAccess) returning either an enriched context with requesterAccountId or a NextResponse for denial.lib/artists/getArtistById.ts (1)
28-41: Prefer a DRY mapper to avoid field drift.The manual property-by-property remap duplicates
getFormattedArtistoutput and increases maintenance risk when fields evolve.Suggested fix
const formattedArtist = getFormattedArtist(account); - const { account_id, name, image, instruction, knowledges, label, account_socials } = - formattedArtist; + const { pinned: _pinned, ...artistFields } = formattedArtist; return { id: artistId, - account_id, - name, - image, - instruction, - knowledges, - label, - account_socials, + ...artistFields, };As per coding guidelines, extract shared logic and avoid duplication (DRY) to keep functions focused.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/artists/getArtistById.ts` around lines 28 - 41, The function duplicates getFormattedArtist output by manually remapping fields; instead, use the formattedArtist directly to avoid drift: after calling getFormattedArtist(account) (variable formattedArtist) return an object that injects id: artistId and spreads or selectively copies formattedArtist (e.g., { id: artistId, ...formattedArtist }) rather than listing account_id, name, image, instruction, knowledges, label, account_socials individually; this keeps getArtistById focused and DRY and ensures future field changes in getFormattedArtist propagate automatically.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/artists/getArtistHandler.ts`:
- Around line 49-55: In the catch block of getArtistHandler replace returning
raw error.message to clients: keep the existing console.error("[ERROR]
getArtistHandler:", error) for logs, but change the NextResponse.json payload to
use a generic message like "Internal server error" (or "An unexpected error
occurred") instead of error instanceof Error ? error.message : ... so no
internal details are leaked; ensure NextResponse.json still sets status: "error"
and optionally include a non-sensitive errorCode if your API uses one.
In `@lib/supabase/accounts/selectAccountWithArtistDetails.ts`:
- Around line 22-30: The current code in selectAccountWithArtistDetails
collapses both query errors and "not found" into null; change the logic so that
if (error) you surface or throw the Supabase error (e.g., throw error or return
Promise.reject(error)) while only returning null when !data (not found). Update
the block around the supabase.from(...).single() call to check error first
(handle/propagate it using the existing error-handling convention) and only
return null when error is falsy and data is missing.
---
Nitpick comments:
In `@lib/artists/getArtistById.ts`:
- Around line 28-41: The function duplicates getFormattedArtist output by
manually remapping fields; instead, use the formattedArtist directly to avoid
drift: after calling getFormattedArtist(account) (variable formattedArtist)
return an object that injects id: artistId and spreads or selectively copies
formattedArtist (e.g., { id: artistId, ...formattedArtist }) rather than listing
account_id, name, image, instruction, knowledges, label, account_socials
individually; this keeps getArtistById focused and DRY and ensures future field
changes in getFormattedArtist propagate automatically.
In `@lib/artists/validateGetArtistRequest.ts`:
- Around line 19-50: Split validateGetArtistRequest so it only handles
schema/param validation and returns the inferred GetArtistRequest type (or
NextResponse on validation failure) by extracting auth and authorization logic
into a separate helper; remove calls to validateAuthContext and
checkAccountArtistAccess from this file, export the validation schema and its
inferred type from validateGetArtistRequest, and create a new function (e.g.,
ensureArtistAccess or authorizeArtistRequest) that takes the request and
validatedParams and performs auth (validateAuthContext) plus access checks
(checkAccountArtistAccess) returning either an enriched context with
requesterAccountId or a NextResponse for denial.
In `@lib/supabase/accounts/selectAccountWithArtistDetails.ts`:
- Line 1: Replace the current import of the Supabase client to use the canonical
repository path; update the import that brings in the symbol "supabase" in
selectAccountWithArtistDetails.ts to import from "@/lib/supabase/serverClient"
instead of "../serverClient" so it follows the lib/supabase module boundary
convention.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 02e9fa62-7a54-4812-b80b-ffe094049800
⛔ Files ignored due to path filters (2)
lib/artists/__tests__/getArtistHandler.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/artists/__tests__/validateGetArtistRequest.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (5)
app/api/artists/[id]/route.tslib/artists/getArtistById.tslib/artists/getArtistHandler.tslib/artists/validateGetArtistRequest.tslib/supabase/accounts/selectAccountWithArtistDetails.ts
| } catch (error) { | ||
| console.error("[ERROR] getArtistHandler:", error); | ||
| return NextResponse.json( | ||
| { | ||
| status: "error", | ||
| error: error instanceof Error ? error.message : "Internal server error", | ||
| }, |
There was a problem hiding this comment.
Avoid returning raw exception messages to clients.
Line 54 exposes error.message in the API response, which can leak internals. Return a generic error message and keep details in logs only.
Suggested fix
} catch (error) {
console.error("[ERROR] getArtistHandler:", error);
return NextResponse.json(
{
status: "error",
- error: error instanceof Error ? error.message : "Internal server error",
+ error: "Internal server error",
},
{
status: 500,
headers: getCorsHeaders(),
},
);
}As per coding guidelines, errors should be handled gracefully without exposing sensitive internal details.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (error) { | |
| console.error("[ERROR] getArtistHandler:", error); | |
| return NextResponse.json( | |
| { | |
| status: "error", | |
| error: error instanceof Error ? error.message : "Internal server error", | |
| }, | |
| } catch (error) { | |
| console.error("[ERROR] getArtistHandler:", error); | |
| return NextResponse.json( | |
| { | |
| status: "error", | |
| error: "Internal server error", | |
| }, | |
| { | |
| status: 500, | |
| headers: getCorsHeaders(), | |
| }, | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/artists/getArtistHandler.ts` around lines 49 - 55, In the catch block of
getArtistHandler replace returning raw error.message to clients: keep the
existing console.error("[ERROR] getArtistHandler:", error) for logs, but change
the NextResponse.json payload to use a generic message like "Internal server
error" (or "An unexpected error occurred") instead of error instanceof Error ?
error.message : ... so no internal details are leaked; ensure NextResponse.json
still sets status: "error" and optionally include a non-sensitive errorCode if
your API uses one.
| const { data, error } = await supabase | ||
| .from("accounts") | ||
| .select("*, account_info(*), account_socials(*, social:socials(*))") | ||
| .eq("id", accountId) | ||
| .single(); | ||
|
|
||
| if (error || !data) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Do not collapse Supabase errors into null (causes false 404s).
Line 28 currently treats query errors the same as “artist not found.” That makes upstream handlers return 404 during DB failures instead of 500.
Suggested fix
export async function selectAccountWithArtistDetails(
accountId: string,
): Promise<AccountWithArtistDetails | null> {
const { data, error } = await supabase
.from("accounts")
.select("*, account_info(*), account_socials(*, social:socials(*))")
.eq("id", accountId)
- .single();
+ .maybeSingle();
- if (error || !data) {
- return null;
+ if (error) {
+ throw new Error("Failed to fetch account artist details");
}
- return data as AccountWithArtistDetails;
+ return (data as AccountWithArtistDetails) ?? null;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/supabase/accounts/selectAccountWithArtistDetails.ts` around lines 22 -
30, The current code in selectAccountWithArtistDetails collapses both query
errors and "not found" into null; change the logic so that if (error) you
surface or throw the Supabase error (e.g., throw error or return
Promise.reject(error)) while only returning null when !data (not found). Update
the block around the supabase.from(...).single() call to check error first
(handle/propagate it using the existing error-handling convention) and only
return null when error is falsy and data is missing.
|
Preview smoke test on https://recoup-api-git-codex-artist-read-mig-98a286-recoupable-ad724970.vercel.app Verified Results:
Returned payload fields on the |
Testing Results ✅Tested Requestcurl -s -X GET \
-H "x-api-key: $API_KEY" \
"https://recoup-api-git-codex-artist-read-mig-98a286-recoupable-ad724970.vercel.app/api/artists/1873859c-dd37-4e9a-9bac-80d3558527a9"Result
🤖 Generated with Claude Code |
This reverts commit 7705373.
Summary
Verification
Summary by cubic
Adds
GET /api/artists/{id}to fetch a single artist’s details with consistent JSON errors and CORS headers. Access now mirrors the list endpoint, with checks enforced during request validation.New Features
GET /api/artists/{id}and CORSOPTIONS.selectAccountWithArtistDetails+getFormattedArtist; 404 “Artist not found”.Bug Fixes
Written for commit fb3a315. Summary will update on new commits.
Summary by CodeRabbit