Skip to content

Conversation

@marcodejongh
Copy link
Owner

Adds a button to the playlist view that allows users to add all climbs
from a playlist to their queue at once. The feature:

  • Fetches all pages of playlist climbs (handling pagination)
  • Filters out cross-layout climbs that can't be displayed
  • Shows loading state while fetching
  • Displays success/error messages via toast notifications
  • Tracks the action via Vercel Analytics

@vercel
Copy link

vercel bot commented Jan 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
boardsesh Building Building Preview, Comment Jan 2, 2026 9:47pm

@claude
Copy link

claude bot commented Jan 2, 2026

Claude Review

Ready to merge - Minor issues noted below, but nothing blocking.

Issues

  1. Type mismatch (playlist-view-content.tsx:127): response.playlistClimbs.climbs is cast to Climb[] but the GraphQL response type (PlaylistClimbsResult.climbs) doesn't include the mirrored, userAscents, or userAttempts fields that Climb has. This works at runtime but is a loose type assertion.

  2. Potential race condition (playlist-view-content.tsx:96): The early return if (!token || isAddingToQueue) return relies on isAddingToQueue state, but multiple rapid clicks could potentially trigger multiple fetches before the state update propagates. Consider using a ref to track in-flight requests.

  3. No abort handling (playlist-view-content.tsx:103-136): The pagination loop has no abort mechanism. If the user navigates away while fetching multiple pages, the fetch continues and may update unmounted state. Consider adding AbortController support or checking component mount status between page fetches.

@claude
Copy link

claude bot commented Jan 2, 2026

Claude Review

Ready to merge - Minor issues noted below, but nothing blocking.

Issues

  1. Unsafe type cast - playlist-view-content.tsx:180: The cast climb as Climb is unsafe because PlaylistClimbsResult has different types for some fields (quality_average is string in both but litUpHoldsMap is Record<string, unknown> vs the expected LitUpHoldsMap). This could cause runtime issues if components access those fields. Consider properly mapping the types.

  2. Missing abort signal in fetch - playlist-view-content.tsx:121-144: The abort signal is only checked before/after executeGraphQL calls but not passed to the function itself. If the GraphQL client supports abort signals, passing it would allow canceling in-flight requests.

  3. No rate limiting on pagination - playlist-view-content.tsx:113-153: The while (hasMore) loop will fetch all pages as fast as possible without any delay. For very large playlists, this could overwhelm the server. Consider adding a small delay or batch limit.

Adds a button to the playlist view that allows users to add all climbs
from a playlist to their queue at once. The feature:
- Fetches all pages of playlist climbs (handling pagination)
- Filters out cross-layout climbs that can't be displayed
- Shows loading state while fetching
- Displays success/error messages via toast notifications
- Tracks the action via Vercel Analytics
@marcodejongh marcodejongh force-pushed the claude/add-queue-all-climbs-H3dxf branch from 009cb2f to 3f08a8b Compare January 2, 2026 21:47
@marcodejongh marcodejongh merged commit 6e9cd53 into main Jan 2, 2026
3 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants