feat: cut segment list to dedicated GET /api/artists/{id}/segments#1680
feat: cut segment list to dedicated GET /api/artists/{id}/segments#1680arpitgupta1214 wants to merge 2 commits intotestfrom
Conversation
Migrate the segments list UI off the local `/api/segments?artistId=...`
route and onto the dedicated, nested `GET /api/artists/{id}/segments`
endpoint (api PR #443).
- `hooks/useArtistSegments.ts` now calls the dedicated endpoint with a
Bearer token via `getClientApiBaseUrl()`, maps the paginated
`{ status, segments, pagination }` envelope into the existing
`Segment[]` consumer shape, and preserves the `s.size > 0` filter.
- Extracted the `Segment` consumer type into `types/Segment.ts` so
components can depend on a type-only module after the supabase-backed
helpers go away. `components/Segments/Segments.tsx` and
`components/Segments/SegmentButton.tsx` now import from there. No
other consumer changes needed (SegmentsWrapper, FanGroupNavItem,
MiniMenu already depended on the hook's return shape).
- Updated the MCP `get_artist_segments` tool to call the nested
dedicated URL as well, since api PR #443 removes the legacy flat
`GET /api/artist/segments` in the same release.
- Removed the local `app/api/segments/route.ts` and the supabase
helpers that only backed it: `lib/supabase/getArtistSegments.ts`,
`getArtistSegmentNames.ts`, `getSegmentCounts.ts`, and
`fan_segments/selectFanSegments.ts`.
Follows SEGMENTS_SURFACE_MIGRATION_PLAN.md step "List -> chat PR".
Co-Authored-By: Claude Opus 4.6 <[email protected]>
📝 WalkthroughWalkthroughThis PR migrates segment data fetching from internal Supabase queries to a client API approach. The Changes
Sequence Diagram(s)sequenceDiagram
participant App as Client App
participant Hook as useArtistSegments
participant Privy as Privy Auth
participant ClientAPI as Client API<br/>(segments endpoint)
App->>Hook: useArtistSegments(artistId)
activate Hook
Note over Hook: Query enabled when<br/>artistId && authenticated
Hook->>Privy: usePrivy().getAccessToken()
activate Privy
Privy-->>Hook: accessToken
deactivate Privy
Hook->>ClientAPI: fetch with Bearer token<br/>/api/artists/{artistId}/segments
activate ClientAPI
ClientAPI-->>Hook: JSON response
deactivate ClientAPI
Note over Hook: Transform & filter<br/>(size > 0)
Hook-->>App: Segment[]
deactivate Hook
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Following project pattern (fetchArtists, deleteArtist) — network call lives in lib/artists/*, the hook just wires Privy auth + react-query.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11911ef130
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| throw new Error(data.error || "Failed to fetch segments"); | ||
| } | ||
|
|
||
| return (data.segments || []) |
There was a problem hiding this comment.
Fetch all segment pages before returning UI data
This helper returns only data.segments from a single response even though the API contract is explicitly paginated (pagination.total_pages is present), so any artist whose segments span multiple pages will silently lose segments in the sidebar and segments screen after this cutover. The previous local Supabase route returned the full set; this migration should either request a sufficiently large limit or iterate through all pages before mapping/filtering.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
No issues found across 11 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: Refactor and API cutover involving authentication (Privy) and external data source changes require human verification of the integration and dependencies.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/tools/getArtistSegments.ts (2)
15-24:⚠️ Potential issue | 🟠 MajorAlign response typing and status handling with the new endpoint contract.
Segmentis still typed as the legacy shape, and the return path markssuccess: truewithout checkingdata.status. That can surface false-positive successes.Suggested fix
interface Segment { id: string; - artist_account_id: string; - segment_id: string; - updated_at: string; - segment_name: string; - artist_name: string; + name: string; + size: number; + icon?: string; } // ... const data = (await response.json()) as SegmentResponse; -if (!response.ok) { - throw new Error(`API request failed with status ${response.status}`); +if (!response.ok || data.status === "error") { + throw new Error( + `API request failed with status ${response.status}`, + ); }As per coding guidelines, "For utility functions, ensure: Proper error handling."
Also applies to: 60-65
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/tools/getArtistSegments.ts` around lines 15 - 24, The response typing and success handling in getArtistSegments.ts must match the new endpoint contract: update the Segment type used by the SegmentResponse interface to the new shape expected by the endpoint, and in the getArtistSegments function (or whichever function processes the fetch) check data.status === "success" before returning a success result; if data.status !== "success" or the payload is missing/invalid, return/throw an error path (or a failure result) instead of always marking success: true. Also adjust the function's return type to reflect the revised SegmentResponse shape and ensure proper error handling/logging when status is "error".
39-54:⚠️ Potential issue | 🔴 CriticalAdd Bearer auth to this request before shipping.
This tool now targets the protected nested endpoint, but the request is unauthenticated. It will fail once the legacy route is gone.
Suggested fix
const schema = z.object({ artist_account_id: z.string().min(1, "Artist account ID is required"), + access_token: z.string().min(1, "Access token is required"), page: z.number().min(1).optional().default(1), limit: z.number().min(1).max(100).optional().default(20), }); const getArtistSegments = tool({ inputSchema: schema, - execute: async ({ artist_account_id, page, limit }) => { + execute: async ({ artist_account_id, access_token, page, limit }) => { // ... const response = await fetch(url.toString(), { method: "GET", headers: { "Content-Type": "application/json", + Authorization: `Bearer ${access_token}`, }, });As per coding guidelines, "Implement built-in security practices for authentication and data handling."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/tools/getArtistSegments.ts` around lines 39 - 54, The request to the protected nested endpoint currently omits authentication; before calling fetch in getArtistSegments (where getClientApiBaseUrl(), artist_account_id, page, limit and the fetch(...) call are used) obtain a valid access token from the app's auth helper (e.g., getAuthToken() or equivalent) and include it as an Authorization: Bearer <token> header in the fetch headers; if no helper exists, call the central auth provider to retrieve the token and fail-fast with a clear error when token retrieval fails. Ensure the Authorization header is added alongside Content-Type in the headers object passed to fetch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/useArtistSegments.ts`:
- Around line 9-13: Remove the non-null assertions in the queryFn and add
explicit checks: inside the queryFn of useArtistSegments.ts, call
getAccessToken() and verify the returned accessToken is a non-empty string (and
that artistId is defined) before calling getArtistSegments; if either is missing
throw or return a clear Error with a descriptive message (e.g., "Missing access
token" or "Missing artistId") so you don't rely on the ! assertions and the
enabled guard alone.
In `@lib/artists/getArtistSegments.ts`:
- Line 35: The URL is built by directly interpolating artistId into the path in
getArtistSegments (the
`${getClientApiBaseUrl()}/api/artists/${artistId}/segments` expression);
URL-encode artistId with encodeURIComponent before composing the URL (either
inline `${encodeURIComponent(artistId)}` or assign `const encodedArtistId =
encodeURIComponent(artistId)` and use that) so the request path is safe for any
characters in artistId.
---
Outside diff comments:
In `@lib/tools/getArtistSegments.ts`:
- Around line 15-24: The response typing and success handling in
getArtistSegments.ts must match the new endpoint contract: update the Segment
type used by the SegmentResponse interface to the new shape expected by the
endpoint, and in the getArtistSegments function (or whichever function processes
the fetch) check data.status === "success" before returning a success result; if
data.status !== "success" or the payload is missing/invalid, return/throw an
error path (or a failure result) instead of always marking success: true. Also
adjust the function's return type to reflect the revised SegmentResponse shape
and ensure proper error handling/logging when status is "error".
- Around line 39-54: The request to the protected nested endpoint currently
omits authentication; before calling fetch in getArtistSegments (where
getClientApiBaseUrl(), artist_account_id, page, limit and the fetch(...) call
are used) obtain a valid access token from the app's auth helper (e.g.,
getAuthToken() or equivalent) and include it as an Authorization: Bearer <token>
header in the fetch headers; if no helper exists, call the central auth provider
to retrieve the token and fail-fast with a clear error when token retrieval
fails. Ensure the Authorization header is added alongside Content-Type in the
headers object passed to fetch.
🪄 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: fe4e4142-9363-44fe-b798-dd6e45459097
⛔ Files ignored due to path filters (1)
types/Segment.tsis excluded by none and included by none
📒 Files selected for processing (10)
app/api/segments/route.tscomponents/Segments/SegmentButton.tsxcomponents/Segments/Segments.tsxhooks/useArtistSegments.tslib/artists/getArtistSegments.tslib/supabase/fan_segments/selectFanSegments.tslib/supabase/getArtistSegmentNames.tslib/supabase/getArtistSegments.tslib/supabase/getSegmentCounts.tslib/tools/getArtistSegments.ts
💤 Files with no reviewable changes (5)
- lib/supabase/getSegmentCounts.ts
- lib/supabase/getArtistSegmentNames.ts
- app/api/segments/route.ts
- lib/supabase/fan_segments/selectFanSegments.ts
- lib/supabase/getArtistSegments.ts
| queryFn: async () => { | ||
| const accessToken = await getAccessToken(); | ||
| return getArtistSegments(accessToken!, artistId!); | ||
| }, | ||
| enabled: !!artistId && authenticated, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In @privy-io/react-auth, can usePrivy().getAccessToken() return null/undefined even when authenticated is true? Please cite the official docs/type signature.
💡 Result:
No, usePrivy.getAccessToken does not return null/undefined when authenticated is true. The official Privy documentation states that when a user is authenticated, Privy issues an app access token, and getAccessToken retrieves this token as a string, automatically refreshing it if expired. The docs show it used without checking authentication first and describe it returning a string. The authenticated boolean from usePrivy indicates the user has a valid session with an access token. If not authenticated, calling getAccessToken would fail (likely throw), but when authenticated is true, it succeeds and returns the token string. No type signature explicitly showing | null was found in docs or npm pages, and examples treat it as Promise<string>.
Citations:
- 1: https://docs.privy.io/authentication/user-authentication/access-tokens
- 2: https://docs.privy.io/guide/react/authorization
- 3: https://docs.privy.io/guide/react/authentication/status
- 4: https://docs.privy.io/authentication/user-authentication/jwt-based-auth/usage
Remove non-null assertions and add explicit error handling for better clarity.
The enabled guard and Privy's contract ensure getAccessToken() returns a string when authenticated is true, making the assertions on accessToken redundant. However, for production auth code, explicit checks with clear error messages are better practice than relying on assertions.
Suggested fix
queryFn: async () => {
+ if (!artistId) {
+ throw new Error("Artist ID is required for fetching segments");
+ }
const accessToken = await getAccessToken();
- return getArtistSegments(accessToken!, artistId!);
+ return getArtistSegments(accessToken, artistId);
},Per Privy documentation, getAccessToken() returns Promise<string> when authenticated is true, and throws on error rather than returning null. The enabled guard prevents query execution when either condition is false, so the assertions are safe but unnecessary. Removing them improves readability and aligns with the guideline to handle edge cases explicitly in auth code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/useArtistSegments.ts` around lines 9 - 13, Remove the non-null
assertions in the queryFn and add explicit checks: inside the queryFn of
useArtistSegments.ts, call getAccessToken() and verify the returned accessToken
is a non-empty string (and that artistId is defined) before calling
getArtistSegments; if either is missing throw or return a clear Error with a
descriptive message (e.g., "Missing access token" or "Missing artistId") so you
don't rely on the ! assertions and the enabled guard alone.
| artistId: string, | ||
| ): Promise<Segment[]> { | ||
| const response = await fetch( | ||
| `${getClientApiBaseUrl()}/api/artists/${artistId}/segments`, |
There was a problem hiding this comment.
URL-encode artistId before path interpolation.
Direct interpolation can produce malformed paths for unexpected IDs. Encode it before composing the URL.
Suggested fix
- `${getClientApiBaseUrl()}/api/artists/${artistId}/segments`,
+ `${getClientApiBaseUrl()}/api/artists/${encodeURIComponent(artistId)}/segments`,As per coding guidelines, "Implement built-in security practices for authentication and data handling."
📝 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.
| `${getClientApiBaseUrl()}/api/artists/${artistId}/segments`, | |
| `${getClientApiBaseUrl()}/api/artists/${encodeURIComponent(artistId)}/segments`, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/artists/getArtistSegments.ts` at line 35, The URL is built by directly
interpolating artistId into the path in getArtistSegments (the
`${getClientApiBaseUrl()}/api/artists/${artistId}/segments` expression);
URL-encode artistId with encodeURIComponent before composing the URL (either
inline `${encodeURIComponent(artistId)}` or assign `const encodedArtistId =
encodeURIComponent(artistId)` and use that) so the request path is safe for any
characters in artistId.
Summary
Cuts the segment list UI over from the local
/api/segments?artistId=...route to the nested dedicatedGET /api/artists/{id}/segmentsendpoint.hooks/useArtistSegments.tsnow calls${API_BASE}/api/artists/{artistId}/segmentswith a Privy Bearer token (mirroringuseAccountOrganizations), then maps the paginated{ status, segments, pagination }envelope into the existingSegment[]consumer shape. Thes.size > 0filter is preserved at the hook layer.SegmentsWrapper,FanGroupNavItem,MiniMenu,Segments,SegmentButton) do not change behaviorally. TheSegmentconsumer type moved from the deleted supabase helper into a dedicatedtypes/Segment.ts;components/Segments/Segments.tsxandcomponents/Segments/SegmentButton.tsxupdated their imports accordingly.get_artist_segmentstool (lib/tools/getArtistSegments.ts) to the nested URL too, since the matching api PR removes the legacy flatGET /api/artist/segmentsin the same release. Without this, AI tool calls would 404 once the api PR merges.app/api/segments/route.tslib/supabase/getArtistSegments.tslib/supabase/getArtistSegmentNames.tslib/supabase/getSegmentCounts.tslib/supabase/fan_segments/selectFanSegments.tsMapping
The dedicated payload does not include a
fansroster.Segment.fansstays optional in the consumer type;SegmentButtonalready falls back to an empty avatar list whenfansis missing (const fansWithAvatars = segment.fans?.filter(...) || []), so the UI degrades to the users-icon placeholder without code changes.Dependencies and ordering
GET /api/artists/{id}/segmentsand removesGET /api/artist/segmentsin the same PR). This PR stays in draft until that lands on apitest.POST /api/artists/{id}/segments) should merge before this List cutover.Coordination with sibling chat PR
A parallel chat PR on
feat/chat-cutover-post-segmentshandles the Create cutover and deletesapp/api/segments/create/route.ts. This PR deletes the siblingapp/api/segments/route.ts(GET) and leavesapp/api/segments/create/untouched.Grep confirmation
GET /api/segments?artistId=...orGET /api/artist/segmentsin the chat repo./api/segments/createremains (owned by the sibling Create PR).Test plan
test.SegmentsWrapperrenders the same segments list it did before, with thesize > 0filter preserved.FanGroupNavItemin the sidebar conditionally renders only when the hook returns >= 1 segment.MiniMenu(collapsed sidebar) shows the segments affordance under the same condition.get_artist_segmentstool returns the same paginated envelope against the new URL.npx tsc --noEmit— no new errors on touched files.npx eslint hooks/useArtistSegments.ts components/Segments/ components/Sidebar/ lib/tools/getArtistSegments.ts types/Segment.ts— clean.Summary by cubic
Switch the segments list to the dedicated
GET /api/artists/{id}/segmentsendpoint and remove the local route and Supabase helpers. UI behavior is unchanged; this aligns chat with the API cutover.Refactors
hooks/useArtistSegmentsnow usesusePrivyand callslib/artists/getArtistSegments(Bearer auth), enables the query only whenauthenticated, maps{ status, segments, pagination }toSegment[], and keeps thesize > 0filter.Segmenttype totypes/Segment.ts; updated imports inSegmentsandSegmentButtononly.lib/tools/getArtistSegments.tsto the nested URL.app/api/segments/route.tsand Supabase helpers:lib/supabase/getArtistSegments.ts,getArtistSegmentNames.ts,getSegmentCounts.ts,fan_segments/selectFanSegments.ts.Migration
GET /api/artists/{id}/segments, removesGET /api/artist/segments).Written for commit 11911ef. Summary will update on new commits.
Summary by CodeRabbit
Release Notes