From a9cf2a2940cda92f15be5e96ed3ed750a6bfa9e2 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 13 Feb 2026 14:13:26 +0000 Subject: [PATCH] Fix N+1 query in playlist memberships and improve hook reliability Replace N separate GET_PLAYLISTS_FOR_CLIMB GraphQL requests with a single batched playlistMembershipsForClimbs query. This adds: - New GraphQL query, input type, and return type in shared schema - Backend resolver using a single SQL query with inArray for batch lookup - Validation schema for the batch input (max 200 UUIDs) - Client-side operation and TypeScript types Also fixes stale closure risk in addToPlaylist/removeFromPlaylist callbacks by creating new Sets instead of mutating existing ones in-place, and refactors the playlists fetch from useEffect+useState to useQuery for consistency with the rest of the hook and automatic caching/deduplication. https://claude.ai/code/session_013FeKjWGrgDj3N2oq9kvU6R --- .../graphql/resolvers/playlists/queries.ts | 64 ++++++++ packages/backend/src/validation/schemas.ts | 6 + packages/shared-schema/src/schema.ts | 28 ++++ .../web/app/hooks/use-climb-actions-data.tsx | 139 +++++++++--------- .../app/lib/graphql/operations/playlists.ts | 29 ++++ 5 files changed, 195 insertions(+), 71 deletions(-) diff --git a/packages/backend/src/graphql/resolvers/playlists/queries.ts b/packages/backend/src/graphql/resolvers/playlists/queries.ts index c1a9da9f..4bdca48f 100644 --- a/packages/backend/src/graphql/resolvers/playlists/queries.ts +++ b/packages/backend/src/graphql/resolvers/playlists/queries.ts @@ -8,6 +8,7 @@ import { GetUserPlaylistsInputSchema, GetAllUserPlaylistsInputSchema, GetPlaylistsForClimbInputSchema, + GetPlaylistMembershipsForClimbsInputSchema, GetPlaylistClimbsInputSchema, DiscoverPlaylistsInputSchema, GetPlaylistCreatorsInputSchema, @@ -309,6 +310,69 @@ export const playlistQueries = { return results.map(r => r.playlistUuid); }, + /** + * Batch-fetch playlist memberships for multiple climbs in a single query. + * Replaces N separate playlistsForClimb calls with one batched query. + */ + playlistMembershipsForClimbs: async ( + _: unknown, + { input }: { input: { boardType: string; layoutId: number; climbUuids: string[] } }, + ctx: ConnectionContext + ): Promise> => { + requireAuthenticated(ctx); + validateInput(GetPlaylistMembershipsForClimbsInputSchema, input, 'input'); + + const userId = ctx.userId!; + + if (input.climbUuids.length === 0) { + return []; + } + + // Single query to get all playlist memberships for all requested climbs + const results = await db + .select({ + climbUuid: dbSchema.playlistClimbs.climbUuid, + playlistUuid: dbSchema.playlists.uuid, + }) + .from(dbSchema.playlistClimbs) + .innerJoin( + dbSchema.playlists, + eq(dbSchema.playlists.id, dbSchema.playlistClimbs.playlistId) + ) + .innerJoin( + dbSchema.playlistOwnership, + eq(dbSchema.playlistOwnership.playlistId, dbSchema.playlists.id) + ) + .where( + and( + inArray(dbSchema.playlistClimbs.climbUuid, input.climbUuids), + eq(dbSchema.playlists.boardType, input.boardType), + or( + eq(dbSchema.playlists.layoutId, input.layoutId), + isNull(dbSchema.playlists.layoutId) + ), + eq(dbSchema.playlistOwnership.userId, userId) + ) + ); + + // Group results by climbUuid + const membershipsMap = new Map(); + for (const row of results) { + const existing = membershipsMap.get(row.climbUuid); + if (existing) { + existing.push(row.playlistUuid); + } else { + membershipsMap.set(row.climbUuid, [row.playlistUuid]); + } + } + + // Return entries for all requested climbUuids (empty array for those with no memberships) + return input.climbUuids.map(uuid => ({ + climbUuid: uuid, + playlistIds: membershipsMap.get(uuid) ?? [], + })); + }, + /** * Get climbs in a playlist with full climb data */ diff --git a/packages/backend/src/validation/schemas.ts b/packages/backend/src/validation/schemas.ts index c31ecf17..513041d0 100644 --- a/packages/backend/src/validation/schemas.ts +++ b/packages/backend/src/validation/schemas.ts @@ -438,6 +438,12 @@ export const GetPlaylistsForClimbInputSchema = z.object({ climbUuid: ExternalUUIDSchema, }); +export const GetPlaylistMembershipsForClimbsInputSchema = z.object({ + boardType: BoardNameSchema, + layoutId: z.number().int().positive(), + climbUuids: z.array(ExternalUUIDSchema).min(1).max(200), +}); + export const GetPlaylistClimbsInputSchema = z.object({ playlistId: z.string().min(1), boardName: BoardNameSchema, diff --git a/packages/shared-schema/src/schema.ts b/packages/shared-schema/src/schema.ts index eaa84a33..e919fb67 100644 --- a/packages/shared-schema/src/schema.ts +++ b/packages/shared-schema/src/schema.ts @@ -931,6 +931,28 @@ export const typeDefs = /* GraphQL */ ` climbUuid: String! } + """ + Input for batch-fetching playlist memberships for multiple climbs. + """ + input GetPlaylistMembershipsForClimbsInput { + "Board type" + boardType: String! + "Layout ID" + layoutId: Int! + "Climb UUIDs to check memberships for" + climbUuids: [String!]! + } + + """ + A single climb's playlist membership entry. + """ + type PlaylistMembershipEntry { + "The climb UUID" + climbUuid: String! + "Playlist UUIDs containing this climb" + playlistIds: [ID!]! + } + """ Input for getting climbs in a playlist with full data. """ @@ -2625,6 +2647,12 @@ export const typeDefs = /* GraphQL */ ` """ playlistsForClimb(input: GetPlaylistsForClimbInput!): [ID!]! + """ + Batch-fetch playlist memberships for multiple climbs in a single query. + Returns which playlists each climb belongs to. + """ + playlistMembershipsForClimbs(input: GetPlaylistMembershipsForClimbsInput!): [PlaylistMembershipEntry!]! + """ Get climbs in a playlist with full climb data. """ diff --git a/packages/web/app/hooks/use-climb-actions-data.tsx b/packages/web/app/hooks/use-climb-actions-data.tsx index 08189c0b..a28d1bce 100644 --- a/packages/web/app/hooks/use-climb-actions-data.tsx +++ b/packages/web/app/hooks/use-climb-actions-data.tsx @@ -1,6 +1,6 @@ 'use client'; -import { useCallback, useState, useEffect, useMemo } from 'react'; +import { useCallback, useMemo } from 'react'; import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; import { useWsAuthToken } from '@/app/hooks/use-ws-auth-token'; import { useSnackbar } from '@/app/components/providers/snackbar-provider'; @@ -13,12 +13,12 @@ import { } from '@/app/lib/graphql/operations/favorites'; import { GET_USER_PLAYLISTS, - GET_PLAYLISTS_FOR_CLIMB, + GET_PLAYLIST_MEMBERSHIPS_FOR_CLIMBS, ADD_CLIMB_TO_PLAYLIST, REMOVE_CLIMB_FROM_PLAYLIST, CREATE_PLAYLIST, type GetUserPlaylistsQueryResponse, - type GetPlaylistsForClimbQueryResponse, + type GetPlaylistMembershipsForClimbsQueryResponse, type AddClimbToPlaylistMutationResponse, type RemoveClimbFromPlaylistMutationResponse, type CreatePlaylistMutationResponse, @@ -131,67 +131,70 @@ export function useClimbActionsData({ // === Playlists === - const [playlists, setPlaylists] = useState([]); - const [playlistMemberships, setPlaylistMemberships] = useState>>( - new Map(), - ); - const [playlistsLoading, setPlaylistsLoading] = useState(false); + const playlistsEnabled = + isAuthenticated && !isAuthLoading && boardName !== 'moonboard'; - // Fetch user's playlists - useEffect(() => { - if (!token || !isAuthenticated || boardName === 'moonboard') return; + // Fetch user's playlists using useQuery for consistency and automatic caching + const playlistsQueryKey = useMemo( + () => ['userPlaylists', boardName, layoutId] as const, + [boardName, layoutId], + ); - const fetchPlaylists = async () => { + const { data: playlistsData, isLoading: playlistsLoading } = useQuery({ + queryKey: playlistsQueryKey, + queryFn: async (): Promise => { + const client = createGraphQLHttpClient(token); try { - setPlaylistsLoading(true); - const client = createGraphQLHttpClient(token); const response = await client.request(GET_USER_PLAYLISTS, { input: { boardType: boardName, layoutId }, }); - setPlaylists(response.userPlaylists); + return response.userPlaylists; } catch (error) { console.error('Failed to fetch playlists:', error); - setPlaylists([]); - } finally { - setPlaylistsLoading(false); + return []; } - }; + }, + enabled: playlistsEnabled, + staleTime: 5 * 60 * 1000, + refetchOnWindowFocus: false, + }); - fetchPlaylists(); - }, [token, isAuthenticated, boardName, layoutId]); + const playlists = playlistsData ?? []; - // Fetch playlist memberships for visible climbs + // Fetch playlist memberships for visible climbs using a single batched query const climbUuidsKey = useMemo(() => sortedClimbUuids.join(','), [sortedClimbUuids]); + const membershipsQueryKey = useMemo( + () => ['playlistMemberships', boardName, layoutId, climbUuidsKey] as const, + [boardName, layoutId, climbUuidsKey], + ); + const { data: membershipsData } = useQuery({ - queryKey: ['playlistMemberships', boardName, layoutId, climbUuidsKey], + queryKey: membershipsQueryKey, queryFn: async (): Promise>> => { if (sortedClimbUuids.length === 0) return new Map(); const client = createGraphQLHttpClient(token); - const memberships = new Map>(); - - await Promise.all( - sortedClimbUuids.map(async (uuid) => { - const response = await client.request( - GET_PLAYLISTS_FOR_CLIMB, - { input: { boardType: boardName, layoutId, climbUuid: uuid } }, - ); - memberships.set(uuid, new Set(response.playlistsForClimb)); - }), - ); - - return memberships; + try { + const response = await client.request( + GET_PLAYLIST_MEMBERSHIPS_FOR_CLIMBS, + { input: { boardType: boardName, layoutId, climbUuids: sortedClimbUuids } }, + ); + const memberships = new Map>(); + for (const entry of response.playlistMembershipsForClimbs) { + memberships.set(entry.climbUuid, new Set(entry.playlistIds)); + } + return memberships; + } catch (error) { + console.error('Failed to fetch playlist memberships:', error); + throw error; + } }, - enabled: - isAuthenticated && !isAuthLoading && sortedClimbUuids.length > 0 && boardName !== 'moonboard', + enabled: playlistsEnabled && sortedClimbUuids.length > 0, staleTime: 5 * 60 * 1000, refetchOnWindowFocus: false, }); - // Merge query data with local optimistic state - const effectiveMemberships = membershipsData - ? new Map([...membershipsData, ...playlistMemberships]) - : playlistMemberships; + const effectiveMemberships = membershipsData ?? new Map>(); const addToPlaylist = useCallback( async (playlistId: string, climbUuid: string, climbAngle: number) => { @@ -200,18 +203,19 @@ export function useClimbActionsData({ await client.request(ADD_CLIMB_TO_PLAYLIST, { input: { playlistId, climbUuid, angle: climbAngle }, }); - setPlaylistMemberships((prev) => { + // Optimistically update memberships cache with new Set to avoid stale closure mutation + queryClient.setQueryData>>(membershipsQueryKey, (prev) => { const updated = new Map(prev); - const current = updated.get(climbUuid) || new Set(); - current.add(playlistId); - updated.set(climbUuid, current); + const current = updated.get(climbUuid) ?? new Set(); + updated.set(climbUuid, new Set([...current, playlistId])); return updated; }); - setPlaylists((prev) => - prev.map((p) => (p.uuid === playlistId ? { ...p, climbCount: p.climbCount + 1 } : p)), + // Update playlist climb count + queryClient.setQueryData(playlistsQueryKey, (prev) => + prev?.map((p) => (p.uuid === playlistId ? { ...p, climbCount: p.climbCount + 1 } : p)), ); }, - [token], + [token, membershipsQueryKey, playlistsQueryKey, queryClient], ); const removeFromPlaylist = useCallback( @@ -221,22 +225,25 @@ export function useClimbActionsData({ await client.request(REMOVE_CLIMB_FROM_PLAYLIST, { input: { playlistId, climbUuid }, }); - setPlaylistMemberships((prev) => { + // Optimistically update memberships cache with new Set to avoid stale closure mutation + queryClient.setQueryData>>(membershipsQueryKey, (prev) => { const updated = new Map(prev); const current = updated.get(climbUuid); if (current) { - current.delete(playlistId); - updated.set(climbUuid, current); + const next = new Set(current); + next.delete(playlistId); + updated.set(climbUuid, next); } return updated; }); - setPlaylists((prev) => - prev.map((p) => + // Update playlist climb count + queryClient.setQueryData(playlistsQueryKey, (prev) => + prev?.map((p) => p.uuid === playlistId ? { ...p, climbCount: Math.max(0, p.climbCount - 1) } : p, ), ); }, - [token], + [token, membershipsQueryKey, playlistsQueryKey, queryClient], ); const createPlaylist = useCallback( @@ -251,27 +258,17 @@ export function useClimbActionsData({ const response = await client.request(CREATE_PLAYLIST, { input: { boardType: boardName, layoutId, name, description, color, icon }, }); - setPlaylists((prev) => [response.createPlaylist, ...prev]); + queryClient.setQueryData(playlistsQueryKey, (prev) => + [response.createPlaylist, ...(prev ?? [])], + ); return response.createPlaylist; }, - [token, boardName, layoutId], + [token, boardName, layoutId, playlistsQueryKey, queryClient], ); const refreshPlaylists = useCallback(async () => { - if (!token) return; - try { - setPlaylistsLoading(true); - const client = createGraphQLHttpClient(token); - const response = await client.request(GET_USER_PLAYLISTS, { - input: { boardType: boardName, layoutId }, - }); - setPlaylists(response.userPlaylists); - } catch (error) { - console.error('Failed to refresh playlists:', error); - } finally { - setPlaylistsLoading(false); - } - }, [token, boardName, layoutId]); + await queryClient.invalidateQueries({ queryKey: playlistsQueryKey }); + }, [queryClient, playlistsQueryKey]); return { favoritesProviderProps: { diff --git a/packages/web/app/lib/graphql/operations/playlists.ts b/packages/web/app/lib/graphql/operations/playlists.ts index 241f7924..d6957a6a 100644 --- a/packages/web/app/lib/graphql/operations/playlists.ts +++ b/packages/web/app/lib/graphql/operations/playlists.ts @@ -57,6 +57,16 @@ export const GET_PLAYLISTS_FOR_CLIMB = gql` } `; +// Batch-fetch playlist memberships for multiple climbs in a single query +export const GET_PLAYLIST_MEMBERSHIPS_FOR_CLIMBS = gql` + query GetPlaylistMembershipsForClimbs($input: GetPlaylistMembershipsForClimbsInput!) { + playlistMembershipsForClimbs(input: $input) { + climbUuid + playlistIds + } + } +`; + // Create new playlist export const CREATE_PLAYLIST = gql` ${PLAYLIST_FIELDS} @@ -197,6 +207,25 @@ export interface GetPlaylistsForClimbQueryResponse { playlistsForClimb: string[]; } +export interface GetPlaylistMembershipsForClimbsInput { + boardType: string; + layoutId: number; + climbUuids: string[]; +} + +export interface GetPlaylistMembershipsForClimbsQueryVariables { + input: GetPlaylistMembershipsForClimbsInput; +} + +export interface PlaylistMembershipEntry { + climbUuid: string; + playlistIds: string[]; +} + +export interface GetPlaylistMembershipsForClimbsQueryResponse { + playlistMembershipsForClimbs: PlaylistMembershipEntry[]; +} + export interface CreatePlaylistInput { boardType: string; layoutId: number;