From 1b5016ab2aff34e0b0e87136eae41b6c71f3090b Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 18 Mar 2026 23:30:31 +0000 Subject: [PATCH] Fix N+1 user queries in league loader and settings loader Add findUsersByClerkIds() batch function to the user model and replace two separate Promise.all+findUserByClerkId loops (one for owners, one for commissioners) with a single inArray query in both $leagueId.server.ts and $leagueId.settings.tsx. The merged query covers both owner and commissioner IDs in one round-trip. https://claude.ai/code/session_01VAkeDDVZMYS1DweQnUrRnH --- app/models/user.ts | 11 +++++- app/routes/leagues/$leagueId.server.ts | 46 ++++++++--------------- app/routes/leagues/$leagueId.settings.tsx | 44 +++++++++++----------- 3 files changed, 47 insertions(+), 54 deletions(-) diff --git a/app/models/user.ts b/app/models/user.ts index 58ca681..0cc3c45 100644 --- a/app/models/user.ts +++ b/app/models/user.ts @@ -1,4 +1,4 @@ -import { eq } from "drizzle-orm"; +import { eq, inArray } from "drizzle-orm"; import { database } from "~/database/context"; import * as schema from "~/database/schema"; @@ -38,6 +38,15 @@ export async function findUserByClerkId( }); } +export async function findUsersByClerkIds(clerkIds: string[]): Promise { + if (clerkIds.length === 0) return []; + const db = database(); + return await db + .select() + .from(schema.users) + .where(inArray(schema.users.clerkId, clerkIds)); +} + export async function updateUser( id: string, data: Partial diff --git a/app/routes/leagues/$leagueId.server.ts b/app/routes/leagues/$leagueId.server.ts index b52c56d..a28e1b2 100644 --- a/app/routes/leagues/$leagueId.server.ts +++ b/app/routes/leagues/$leagueId.server.ts @@ -7,7 +7,7 @@ import { findCommissionersByLeagueId, isUserLeagueMember, isCommissioner, - findUserByClerkId, + findUsersByClerkIds, getUserDisplayName, findDraftSlotsBySeasonId, } from "~/models"; @@ -75,39 +75,25 @@ export async function loader(args: Route.LoaderArgs) { ? await getSeasonStandings(season.id) : []; - // Fetch user data for team owners - const ownerIds = teams - .map((t) => t.ownerId) - .filter((id): id is string => id !== null); - const uniqueOwnerIds = [...new Set(ownerIds)]; - const owners = await Promise.all( - uniqueOwnerIds.map(async (ownerId) => { - const user = await findUserByClerkId(ownerId); - return user - ? { clerkId: ownerId, name: getUserDisplayName(user) } - : null; - }) - ); + // Batch-fetch all users needed for owner and commissioner maps in one query + const ownerIds = [...new Set(teams.map((t) => t.ownerId).filter((id): id is string => id !== null))]; + const commissionerIds = commissioners.map((c) => c.userId); + const allUserIds = [...new Set([...ownerIds, ...commissionerIds])]; + const userRows = await findUsersByClerkIds(allUserIds); + const userByClerkId = new Map(userRows.map((u) => [u.clerkId, u])); + const ownerMap = new Map( - owners - .filter((o): o is NonNullable => o !== null) - .map((o) => [o.clerkId, o.name]) + ownerIds + .map((id) => [id, userByClerkId.get(id)] as const) + .filter((entry): entry is [string, NonNullable] => entry[1] != null) + .map(([id, user]) => [id, getUserDisplayName(user)]) ); - // Fetch user data for commissioners - const commissionerIds = commissioners.map((c) => c.userId); - const commissionerUsers = await Promise.all( - commissionerIds.map(async (commissionerId) => { - const user = await findUserByClerkId(commissionerId); - return user - ? { clerkId: commissionerId, name: getUserDisplayName(user) } - : null; - }) - ); const commissionerMap = new Map( - commissionerUsers - .filter((c): c is NonNullable => c !== null) - .map((c) => [c.clerkId, c.name]) + commissionerIds + .map((id) => [id, userByClerkId.get(id)] as const) + .filter((entry): entry is [string, NonNullable] => entry[1] != null) + .map(([id, user]) => [id, getUserDisplayName(user)]) ); // Count available teams diff --git a/app/routes/leagues/$leagueId.settings.tsx b/app/routes/leagues/$leagueId.settings.tsx index 15eea52..d7fccd3 100644 --- a/app/routes/leagues/$leagueId.settings.tsx +++ b/app/routes/leagues/$leagueId.settings.tsx @@ -15,7 +15,7 @@ import { } from "~/models/commissioner"; import { findCurrentSeasonWithSports, updateSeason } from "~/models/season"; import { findTeamsBySeasonId, deleteTeam, removeTeamOwner, claimTeam } from "~/models/team"; -import { findAllUsers, findUserByClerkId, getUserDisplayName, isUserAdminByClerkId } from "~/models/user"; +import { findAllUsers, findUserByClerkId, findUsersByClerkIds, getUserDisplayName, isUserAdminByClerkId } from "~/models/user"; import { unlinkSportFromSeason, linkMultipleSportsToSeason } from "~/models/season-sport"; import { findAllSportsSeasons } from "~/models/sports-season"; import { findDraftSlotsBySeasonId, setDraftOrder, randomizeDraftOrder } from "~/models/draft-slot"; @@ -112,30 +112,28 @@ export async function loader(args: Route.LoaderArgs) { // Get commissioners for this league with user info const commissioners = await findCommissionersByLeagueId(leagueId); - const commissionerUserData = await Promise.all( - commissioners.map(async (c) => { - const user = await findUserByClerkId(c.userId); - return { - ...c, - userName: user ? (getUserDisplayName(user) ?? "Unknown User") : "Unknown User", - }; - }) - ); - // Get owner details for teams - const ownerIds = teams - .map((t) => t.ownerId) - .filter((id): id is string => id !== null); - const uniqueOwnerIds = [...new Set(ownerIds)]; - const owners = await Promise.all( - uniqueOwnerIds.map(async (ownerId) => { - const user = await findUserByClerkId(ownerId); - return user - ? { clerkId: ownerId, name: getUserDisplayName(user), id: user.id } - : null; + // Batch-fetch all users needed for commissioner and owner lookups in one query + const uniqueOwnerIds = [...new Set(teams.map((t) => t.ownerId).filter((id): id is string => id !== null))]; + const commissionerUserIds = commissioners.map((c) => c.userId); + const allUserIds = [...new Set([...commissionerUserIds, ...uniqueOwnerIds])]; + const userRows = await findUsersByClerkIds(allUserIds); + const userByClerkId = new Map(userRows.map((u) => [u.clerkId, u])); + + const commissionerUserData = commissioners.map((c) => { + const user = userByClerkId.get(c.userId); + return { + ...c, + userName: user ? (getUserDisplayName(user) ?? "Unknown User") : "Unknown User", + }; + }); + + const validOwners = uniqueOwnerIds + .map((ownerId) => { + const user = userByClerkId.get(ownerId); + return user ? { clerkId: ownerId, name: getUserDisplayName(user), id: user.id } : null; }) - ); - const validOwners = owners.filter((o): o is NonNullable => o !== null); + .filter((o): o is NonNullable => o !== null); const ownerMap = new Map(validOwners.map((o) => [o.clerkId, o.name])); // League members (team owners) - available to all commissioners for adding co-commissioners