Optimize user data fetching with batch queries and centralize display name logic (#176)
* Fall back to displayName when username is null for Discord webhook Users who sign up via OAuth (Google, GitHub, etc.) without setting a Clerk username have a null `username` field but always have a `displayName` (computed from firstName+lastName or email). Previously, `usernameByClerkId` was filtered to only include users with a non-null username, causing those owners to appear without any identifier in Discord standings messages (e.g. "Liverpool def. Galatasaray" instead of "Liverpool def. Galatasaray (Madmike)"). https://claude.ai/code/session_01VAkeDDVZMYS1DweQnUrRnH * Extract getUserDisplayName helper and use consistently throughout Add a single getUserDisplayName(user) function to app/models/user.ts that encapsulates the username → displayName fallback logic. Replace 9 scattered inline expressions across the codebase (owner-map, scoring-calculator, league routes, settings, invite flow, draft API, Clerk webhook) with calls to the shared helper. No behaviour change — all existing logic preserved, just centralised. https://claude.ai/code/session_01VAkeDDVZMYS1DweQnUrRnH * 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 * Fix N+1 user queries in buildOwnerMap Replace the Promise.all+findUserByClerkId loop with a single findUsersByClerkIds batch query, consistent with the league loader and settings loader fixes. https://claude.ai/code/session_01VAkeDDVZMYS1DweQnUrRnH --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
parent
143698cb96
commit
09b9b9bdad
9 changed files with 82 additions and 83 deletions
|
|
@ -1,4 +1,4 @@
|
||||||
import { findUserByClerkId } from "~/models/user";
|
import { findUsersByClerkIds, getUserDisplayName } from "~/models/user";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Builds a map of teamId -> manager username (or displayName fallback).
|
* Builds a map of teamId -> manager username (or displayName fallback).
|
||||||
|
|
@ -7,30 +7,20 @@ import { findUserByClerkId } from "~/models/user";
|
||||||
export async function buildOwnerMap(
|
export async function buildOwnerMap(
|
||||||
slots: Array<{ team: { id: string; ownerId: string | null } }>
|
slots: Array<{ team: { id: string; ownerId: string | null } }>
|
||||||
): Promise<Record<string, string>> {
|
): Promise<Record<string, string>> {
|
||||||
const ownerIds = slots
|
const uniqueOwnerIds = [...new Set(
|
||||||
.map((s) => s.team.ownerId)
|
slots.map((s) => s.team.ownerId).filter((id): id is string => id !== null)
|
||||||
.filter((id): id is string => id !== null);
|
)];
|
||||||
const uniqueOwnerIds = [...new Set(ownerIds)];
|
|
||||||
|
|
||||||
const ownerEntries = await Promise.all(
|
const userRows = await findUsersByClerkIds(uniqueOwnerIds);
|
||||||
uniqueOwnerIds.map(async (ownerId) => {
|
const userByClerkId = new Map(
|
||||||
const user = await findUserByClerkId(ownerId);
|
userRows
|
||||||
const name = user?.username || user?.displayName || null;
|
.map((u) => [u.clerkId, getUserDisplayName(u)] as const)
|
||||||
return name ? { ownerId, name } : null;
|
.filter((entry): entry is [string, string] => entry[1] != null)
|
||||||
})
|
|
||||||
);
|
|
||||||
|
|
||||||
const ownerByClerkId = new Map(
|
|
||||||
ownerEntries
|
|
||||||
.filter((e): e is NonNullable<typeof e> => e !== null)
|
|
||||||
.map((e) => [e.ownerId, e.name])
|
|
||||||
);
|
);
|
||||||
|
|
||||||
const ownerMap: Record<string, string> = {};
|
const ownerMap: Record<string, string> = {};
|
||||||
for (const slot of slots) {
|
for (const slot of slots) {
|
||||||
const name = slot.team.ownerId
|
const name = slot.team.ownerId ? userByClerkId.get(slot.team.ownerId) : undefined;
|
||||||
? ownerByClerkId.get(slot.team.ownerId)
|
|
||||||
: undefined;
|
|
||||||
if (name) {
|
if (name) {
|
||||||
ownerMap[slot.team.id] = name;
|
ownerMap[slot.team.id] = name;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -6,6 +6,7 @@ import { getSeasonResults } from "./participant-season-result";
|
||||||
import { updateProbabilitiesAfterResult } from "~/services/probability-updater";
|
import { updateProbabilitiesAfterResult } from "~/services/probability-updater";
|
||||||
import { sendStandingsUpdateNotification, type ScoredMatch } from "~/services/discord";
|
import { sendStandingsUpdateNotification, type ScoredMatch } from "~/services/discord";
|
||||||
import { BRACKET_TEMPLATES } from "~/lib/bracket-templates";
|
import { BRACKET_TEMPLATES } from "~/lib/bracket-templates";
|
||||||
|
import { getUserDisplayName } from "~/models/user";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Core scoring calculation engine
|
* Core scoring calculation engine
|
||||||
|
|
@ -1313,8 +1314,8 @@ export async function recalculateAffectedLeagues(
|
||||||
: [];
|
: [];
|
||||||
const usernameByClerkId = new Map(
|
const usernameByClerkId = new Map(
|
||||||
teamOwnerUsers
|
teamOwnerUsers
|
||||||
.filter((u) => u.username != null)
|
.map((u) => [u.clerkId, getUserDisplayName(u)])
|
||||||
.map((u) => [u.clerkId, u.username!])
|
.filter((entry): entry is [string, string] => entry[1] != null)
|
||||||
);
|
);
|
||||||
|
|
||||||
// Build teamId → ownerId map from standings data
|
// Build teamId → ownerId map from standings data
|
||||||
|
|
|
||||||
|
|
@ -1,10 +1,21 @@
|
||||||
import { eq } from "drizzle-orm";
|
import { eq, inArray } from "drizzle-orm";
|
||||||
import { database } from "~/database/context";
|
import { database } from "~/database/context";
|
||||||
import * as schema from "~/database/schema";
|
import * as schema from "~/database/schema";
|
||||||
|
|
||||||
export type User = typeof schema.users.$inferSelect;
|
export type User = typeof schema.users.$inferSelect;
|
||||||
export type NewUser = typeof schema.users.$inferInsert;
|
export type NewUser = typeof schema.users.$inferInsert;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Returns the best available display identifier for a user.
|
||||||
|
* Prefers Clerk username; falls back to displayName (always set from
|
||||||
|
* firstName+lastName or email prefix). Returns null only if both are absent.
|
||||||
|
*/
|
||||||
|
export function getUserDisplayName(
|
||||||
|
user: Pick<User, "username" | "displayName">
|
||||||
|
): string | null {
|
||||||
|
return user.username ?? user.displayName ?? null;
|
||||||
|
}
|
||||||
|
|
||||||
export async function createUser(data: NewUser): Promise<User> {
|
export async function createUser(data: NewUser): Promise<User> {
|
||||||
const db = database();
|
const db = database();
|
||||||
const [user] = await db.insert(schema.users).values(data).returning();
|
const [user] = await db.insert(schema.users).values(data).returning();
|
||||||
|
|
@ -27,6 +38,15 @@ export async function findUserByClerkId(
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export async function findUsersByClerkIds(clerkIds: string[]): Promise<User[]> {
|
||||||
|
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(
|
export async function updateUser(
|
||||||
id: string,
|
id: string,
|
||||||
data: Partial<NewUser>
|
data: Partial<NewUser>
|
||||||
|
|
|
||||||
|
|
@ -2,6 +2,7 @@ import { database } from "~/database/context";
|
||||||
import * as schema from "~/database/schema";
|
import * as schema from "~/database/schema";
|
||||||
import { eq, inArray } from "drizzle-orm";
|
import { eq, inArray } from "drizzle-orm";
|
||||||
import { calculatePickInfo } from "~/models/draft-utils";
|
import { calculatePickInfo } from "~/models/draft-utils";
|
||||||
|
import { getUserDisplayName } from "~/models/user";
|
||||||
|
|
||||||
import type { LoaderFunctionArgs } from "react-router";
|
import type { LoaderFunctionArgs } from "react-router";
|
||||||
|
|
||||||
|
|
@ -77,7 +78,7 @@ export async function loader({ params }: LoaderFunctionArgs) {
|
||||||
.from(schema.users)
|
.from(schema.users)
|
||||||
.where(inArray(schema.users.clerkId, allOwnerIds));
|
.where(inArray(schema.users.clerkId, allOwnerIds));
|
||||||
for (const owner of owners) {
|
for (const owner of owners) {
|
||||||
usernameByClerkId.set(owner.clerkId, owner.username ?? owner.displayName ?? null);
|
usernameByClerkId.set(owner.clerkId, getUserDisplayName(owner));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,6 @@
|
||||||
import { Webhook } from "svix";
|
import { Webhook } from "svix";
|
||||||
import type { Route } from "./+types/clerk";
|
import type { Route } from "./+types/clerk";
|
||||||
import { findOrCreateUser } from "~/models/user";
|
import { findOrCreateUser, getUserDisplayName } from "~/models/user";
|
||||||
|
|
||||||
export async function action({ request }: Route.ActionArgs) {
|
export async function action({ request }: Route.ActionArgs) {
|
||||||
// Get the webhook secret from environment
|
// Get the webhook secret from environment
|
||||||
|
|
@ -62,7 +62,7 @@ export async function action({ request }: Route.ActionArgs) {
|
||||||
imageUrl: image_url,
|
imageUrl: image_url,
|
||||||
});
|
});
|
||||||
console.log(
|
console.log(
|
||||||
`User created in database: ${user.id} (${user.username || user.displayName})`
|
`User created in database: ${user.id} (${getUserDisplayName(user)})`
|
||||||
);
|
);
|
||||||
} else {
|
} else {
|
||||||
// Update existing user (or create if doesn't exist)
|
// Update existing user (or create if doesn't exist)
|
||||||
|
|
@ -78,7 +78,7 @@ export async function action({ request }: Route.ActionArgs) {
|
||||||
imageUrl: image_url,
|
imageUrl: image_url,
|
||||||
});
|
});
|
||||||
console.log(
|
console.log(
|
||||||
`User updated in database: ${user.id} (${user.username || user.displayName})`
|
`User updated in database: ${user.id} (${getUserDisplayName(user)})`
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
|
|
|
||||||
|
|
@ -11,6 +11,7 @@ import {
|
||||||
claimTeam,
|
claimTeam,
|
||||||
isUserLeagueMember,
|
isUserLeagueMember,
|
||||||
findUserByClerkId,
|
findUserByClerkId,
|
||||||
|
getUserDisplayName,
|
||||||
} from "~/models";
|
} from "~/models";
|
||||||
import { Button } from "~/components/ui/button";
|
import { Button } from "~/components/ui/button";
|
||||||
import {
|
import {
|
||||||
|
|
@ -103,7 +104,7 @@ export async function action(args: Route.ActionArgs) {
|
||||||
|
|
||||||
// Claim the first available team, setting owner and name atomically
|
// Claim the first available team, setting owner and name atomically
|
||||||
const team = availableTeams[0];
|
const team = availableTeams[0];
|
||||||
const teamName = `Team ${user.username || user.displayName || "Member"}`;
|
const teamName = `Team ${getUserDisplayName(user) ?? "Member"}`;
|
||||||
await claimTeam(team.id, userId, teamName);
|
await claimTeam(team.id, userId, teamName);
|
||||||
|
|
||||||
// Redirect to league page
|
// Redirect to league page
|
||||||
|
|
|
||||||
|
|
@ -7,7 +7,8 @@ import {
|
||||||
findCommissionersByLeagueId,
|
findCommissionersByLeagueId,
|
||||||
isUserLeagueMember,
|
isUserLeagueMember,
|
||||||
isCommissioner,
|
isCommissioner,
|
||||||
findUserByClerkId,
|
findUsersByClerkIds,
|
||||||
|
getUserDisplayName,
|
||||||
findDraftSlotsBySeasonId,
|
findDraftSlotsBySeasonId,
|
||||||
} from "~/models";
|
} from "~/models";
|
||||||
import { findCurrentSeasonWithSports } from "~/models/season";
|
import { findCurrentSeasonWithSports } from "~/models/season";
|
||||||
|
|
@ -74,39 +75,25 @@ export async function loader(args: Route.LoaderArgs) {
|
||||||
? await getSeasonStandings(season.id)
|
? await getSeasonStandings(season.id)
|
||||||
: [];
|
: [];
|
||||||
|
|
||||||
// Fetch user data for team owners
|
// Batch-fetch all users needed for owner and commissioner maps in one query
|
||||||
const ownerIds = teams
|
const ownerIds = [...new Set(teams.map((t) => t.ownerId).filter((id): id is string => id !== null))];
|
||||||
.map((t) => t.ownerId)
|
const commissionerIds = commissioners.map((c) => c.userId);
|
||||||
.filter((id): id is string => id !== null);
|
const allUserIds = [...new Set([...ownerIds, ...commissionerIds])];
|
||||||
const uniqueOwnerIds = [...new Set(ownerIds)];
|
const userRows = await findUsersByClerkIds(allUserIds);
|
||||||
const owners = await Promise.all(
|
const userByClerkId = new Map(userRows.map((u) => [u.clerkId, u]));
|
||||||
uniqueOwnerIds.map(async (ownerId) => {
|
|
||||||
const user = await findUserByClerkId(ownerId);
|
|
||||||
return user
|
|
||||||
? { clerkId: ownerId, name: user.username || user.displayName }
|
|
||||||
: null;
|
|
||||||
})
|
|
||||||
);
|
|
||||||
const ownerMap = new Map(
|
const ownerMap = new Map(
|
||||||
owners
|
ownerIds
|
||||||
.filter((o): o is NonNullable<typeof o> => o !== null)
|
.map((id) => [id, userByClerkId.get(id)] as const)
|
||||||
.map((o) => [o.clerkId, o.name])
|
.filter((entry): entry is [string, NonNullable<typeof entry[1]>] => 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: user.username || user.displayName }
|
|
||||||
: null;
|
|
||||||
})
|
|
||||||
);
|
|
||||||
const commissionerMap = new Map(
|
const commissionerMap = new Map(
|
||||||
commissionerUsers
|
commissionerIds
|
||||||
.filter((c): c is NonNullable<typeof c> => c !== null)
|
.map((id) => [id, userByClerkId.get(id)] as const)
|
||||||
.map((c) => [c.clerkId, c.name])
|
.filter((entry): entry is [string, NonNullable<typeof entry[1]>] => entry[1] != null)
|
||||||
|
.map(([id, user]) => [id, getUserDisplayName(user)])
|
||||||
);
|
);
|
||||||
|
|
||||||
// Count available teams
|
// Count available teams
|
||||||
|
|
|
||||||
|
|
@ -15,7 +15,7 @@ import {
|
||||||
} from "~/models/commissioner";
|
} from "~/models/commissioner";
|
||||||
import { findCurrentSeasonWithSports, updateSeason } from "~/models/season";
|
import { findCurrentSeasonWithSports, updateSeason } from "~/models/season";
|
||||||
import { findTeamsBySeasonId, deleteTeam, removeTeamOwner, claimTeam } from "~/models/team";
|
import { findTeamsBySeasonId, deleteTeam, removeTeamOwner, claimTeam } from "~/models/team";
|
||||||
import { findAllUsers, findUserByClerkId, isUserAdminByClerkId } from "~/models/user";
|
import { findAllUsers, findUserByClerkId, findUsersByClerkIds, getUserDisplayName, isUserAdminByClerkId } from "~/models/user";
|
||||||
import { unlinkSportFromSeason, linkMultipleSportsToSeason } from "~/models/season-sport";
|
import { unlinkSportFromSeason, linkMultipleSportsToSeason } from "~/models/season-sport";
|
||||||
import { findAllSportsSeasons } from "~/models/sports-season";
|
import { findAllSportsSeasons } from "~/models/sports-season";
|
||||||
import { findDraftSlotsBySeasonId, setDraftOrder, randomizeDraftOrder } from "~/models/draft-slot";
|
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
|
// Get commissioners for this league with user info
|
||||||
const commissioners = await findCommissionersByLeagueId(leagueId);
|
const commissioners = await findCommissionersByLeagueId(leagueId);
|
||||||
const commissionerUserData = await Promise.all(
|
|
||||||
commissioners.map(async (c) => {
|
|
||||||
const user = await findUserByClerkId(c.userId);
|
|
||||||
return {
|
|
||||||
...c,
|
|
||||||
userName: user?.username || user?.displayName || "Unknown User",
|
|
||||||
};
|
|
||||||
})
|
|
||||||
);
|
|
||||||
|
|
||||||
// Get owner details for teams
|
// Batch-fetch all users needed for commissioner and owner lookups in one query
|
||||||
const ownerIds = teams
|
const uniqueOwnerIds = [...new Set(teams.map((t) => t.ownerId).filter((id): id is string => id !== null))];
|
||||||
.map((t) => t.ownerId)
|
const commissionerUserIds = commissioners.map((c) => c.userId);
|
||||||
.filter((id): id is string => id !== null);
|
const allUserIds = [...new Set([...commissionerUserIds, ...uniqueOwnerIds])];
|
||||||
const uniqueOwnerIds = [...new Set(ownerIds)];
|
const userRows = await findUsersByClerkIds(allUserIds);
|
||||||
const owners = await Promise.all(
|
const userByClerkId = new Map(userRows.map((u) => [u.clerkId, u]));
|
||||||
uniqueOwnerIds.map(async (ownerId) => {
|
|
||||||
const user = await findUserByClerkId(ownerId);
|
const commissionerUserData = commissioners.map((c) => {
|
||||||
return user
|
const user = userByClerkId.get(c.userId);
|
||||||
? { clerkId: ownerId, name: user.username || user.displayName, id: user.id }
|
return {
|
||||||
: null;
|
...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;
|
||||||
})
|
})
|
||||||
);
|
.filter((o): o is NonNullable<typeof o> => o !== null);
|
||||||
const validOwners = owners.filter((o): o is NonNullable<typeof o> => o !== null);
|
|
||||||
const ownerMap = new Map(validOwners.map((o) => [o.clerkId, o.name]));
|
const ownerMap = new Map(validOwners.map((o) => [o.clerkId, o.name]));
|
||||||
|
|
||||||
// League members (team owners) - available to all commissioners for adding co-commissioners
|
// League members (team owners) - available to all commissioners for adding co-commissioners
|
||||||
|
|
@ -382,7 +380,7 @@ export async function action(args: Route.ActionArgs) {
|
||||||
}
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const teamName = `Team ${assignedUser.username || assignedUser.displayName || "Member"}`;
|
const teamName = `Team ${getUserDisplayName(assignedUser) ?? "Member"}`;
|
||||||
await claimTeam(teamId, userClerkId, teamName);
|
await claimTeam(teamId, userClerkId, teamName);
|
||||||
|
|
||||||
return { success: true, message: "Owner assigned successfully" };
|
return { success: true, message: "Owner assigned successfully" };
|
||||||
|
|
|
||||||
|
|
@ -5,6 +5,7 @@ import {
|
||||||
isUserLeagueMember,
|
isUserLeagueMember,
|
||||||
isCommissioner,
|
isCommissioner,
|
||||||
findTeamsBySeasonId,
|
findTeamsBySeasonId,
|
||||||
|
getUserDisplayName,
|
||||||
} from "~/models";
|
} from "~/models";
|
||||||
import { getDraftPicks } from "~/models/draft-pick";
|
import { getDraftPicks } from "~/models/draft-pick";
|
||||||
import { getSeasonResults } from "~/models/participant-season-result";
|
import { getSeasonResults } from "~/models/participant-season-result";
|
||||||
|
|
@ -98,7 +99,7 @@ export async function loader(args: Route.LoaderArgs) {
|
||||||
})
|
})
|
||||||
: [];
|
: [];
|
||||||
const ownerMap = new Map(
|
const ownerMap = new Map(
|
||||||
ownerUserRows.map((u) => [u.clerkId, u.username || u.displayName || "Unknown"])
|
ownerUserRows.map((u) => [u.clerkId, getUserDisplayName(u) ?? "Unknown"])
|
||||||
);
|
);
|
||||||
|
|
||||||
// Map draft picks to ownership
|
// Map draft picks to ownership
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue