From c036cf6e2869ccbf30bcf8bc27964c8449104397 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 23 Feb 2026 02:41:43 +0000 Subject: [PATCH] Code review fixes: type safety, security hardening, and dead code removal - Fix Socket.IO event types: draft-paused and draft-resumed were typed as () => void but are emitted with { seasonId, paused } data payloads - Fix draft.force-manual-pick: add missing season.status === "draft" guard so commissioners cannot force picks outside an active draft; add duplicate pick-number check so a slot cannot be assigned two picks (the previous code only checked participant uniqueness, not slot uniqueness) - Replace args: any with ActionFunctionArgs / Route.LoaderArgs across all API routes and league loaders; replace (auth as any).userId casts with proper const { userId } = await getAuth(args) destructuring - Remove unused isSnakeDraft = true dead variable from draft.make-pick - Replace autodraftSettings: any and draftSlots: any[] in draft-utils with properly typed InferSelectModel / DraftSlot types - Update force-manual-pick tests: sequence draftPicks.findFirst mock for the two-call flow; add new tests for status-check and slot-uniqueness https://claude.ai/code/session_01FKq2gPFYpgdfxr8cw4Z2AZ --- app/models/draft-utils.ts | 17 +++++++--- .../__tests__/draft.force-manual-pick.test.ts | 33 +++++++++++++++++-- app/routes/api/autodraft.update.ts | 6 ++-- app/routes/api/draft.adjust-time-bank.ts | 6 ++-- app/routes/api/draft.force-autopick.ts | 6 ++-- app/routes/api/draft.force-manual-pick.ts | 22 +++++++++++-- app/routes/api/draft.make-pick.ts | 11 +++---- app/routes/api/draft.pause.ts | 6 ++-- app/routes/api/draft.replace-pick.ts | 6 ++-- app/routes/api/draft.resume.ts | 6 ++-- app/routes/api/draft.rollback.ts | 6 ++-- app/routes/api/draft.start.ts | 6 ++-- app/routes/api/queue.add.ts | 6 ++-- app/routes/api/queue.clear.ts | 6 ++-- app/routes/api/queue.remove.ts | 6 ++-- app/routes/api/queue.reorder.ts | 6 ++-- .../$leagueId.draft-board.$seasonId.tsx | 6 ++-- .../leagues/$leagueId.draft.$seasonId.tsx | 6 ++-- app/routes/leagues/$leagueId.server.ts | 3 +- .../leagues/$leagueId.standings.$seasonId.tsx | 6 ++-- server/socket.ts | 4 +-- 21 files changed, 116 insertions(+), 64 deletions(-) diff --git a/app/models/draft-utils.ts b/app/models/draft-utils.ts index 618adda..a8af2dd 100644 --- a/app/models/draft-utils.ts +++ b/app/models/draft-utils.ts @@ -1,6 +1,7 @@ import { database } from "~/database/context"; import * as schema from "~/database/schema"; import { eq, and, notInArray, desc, inArray } from "drizzle-orm"; +import type { InferSelectModel } from "drizzle-orm"; import { getTeamQueue } from "./draft-queue"; import { isParticipantDrafted, getDraftPicksWithSports, getTeamDraftPicksWithSports } from "./draft-pick"; import { getParticipantsForSeasonWithSports } from "./participant"; @@ -12,11 +13,13 @@ import { getSocketIO } from "../../server/socket"; * Check if the next team has autodraft enabled and immediately execute their pick * This is called after a pick is made to chain autodraft picks */ +type DraftSlot = { teamId: string; draftOrder: number }; + export async function checkAndTriggerNextAutodraft(params: { seasonId: string; nextPickNumber: number; totalTeams: number; - draftSlots: any[]; + draftSlots: DraftSlot[]; db?: ReturnType; }): Promise { const { seasonId, totalTeams, draftSlots, db: providedDb } = params; @@ -367,20 +370,26 @@ export function getTeamForPick( * @param params.db - Database instance (optional, will use database() if not provided) * @returns Result object with success status and pick data */ +type AutodraftSettings = InferSelectModel; + export async function executeAutoPick(params: { seasonId: string; teamId: string; pickNumber: number; triggeredBy: "commissioner" | "timer"; commissionerUserId?: string; - autodraftSettings?: any; + autodraftSettings?: AutodraftSettings | null; db?: ReturnType; chainEnabled?: boolean; // Set to false when called from within the autodraft chain to prevent recursion }): Promise<{ success: boolean; error?: string; - pick?: any; - participant?: any; + pick?: InferSelectModel; + participant?: InferSelectModel & { + sportsSeason: InferSelectModel & { + sport: InferSelectModel; + }; + }; nextPickNumber?: number; isDraftComplete?: boolean; }> { diff --git a/app/routes/api/__tests__/draft.force-manual-pick.test.ts b/app/routes/api/__tests__/draft.force-manual-pick.test.ts index 1c30d93..7e363ad 100644 --- a/app/routes/api/__tests__/draft.force-manual-pick.test.ts +++ b/app/routes/api/__tests__/draft.force-manual-pick.test.ts @@ -220,14 +220,41 @@ describe("draft.force-manual-pick action", () => { expect(response.status).toBe(404); }); - it("returns 400 when the participant is already drafted", async () => { - mockDb.query.draftPicks.findFirst.mockResolvedValue({ + it("returns 400 when the draft is not active", async () => { + mockDb.query.seasons.findFirst.mockResolvedValue({ ...mockSeason, status: "pre_draft" }); + + const response = await action({ request: defaultPickRequest(), params: {}, context: {} }); + + expect(response.status).toBe(400); + const data = await response.json(); + expect(data.error).toMatch(/not currently active/i); + }); + + it("returns 400 when a pick already exists at the requested slot", async () => { + mockDb.query.draftPicks.findFirst.mockResolvedValueOnce({ id: "existing-pick", - participantId: PARTICIPANT_ID, + participantId: "other-participant", }); const response = await action({ request: defaultPickRequest(), params: {}, context: {} }); + expect(response.status).toBe(400); + const data = await response.json(); + expect(data.error).toMatch(/already exists at this slot/i); + }); + + it("returns 400 when the participant is already drafted", async () => { + // First call: slot check (no pick at this slot number) + // Second call: participant check (participant already drafted elsewhere) + mockDb.query.draftPicks.findFirst + .mockResolvedValueOnce(null) + .mockResolvedValueOnce({ + id: "existing-pick", + participantId: PARTICIPANT_ID, + }); + + const response = await action({ request: defaultPickRequest(), params: {}, context: {} }); + expect(response.status).toBe(400); const data = await response.json(); expect(data.error).toMatch(/already drafted/i); diff --git a/app/routes/api/autodraft.update.ts b/app/routes/api/autodraft.update.ts index 6ffb14b..5da956e 100644 --- a/app/routes/api/autodraft.update.ts +++ b/app/routes/api/autodraft.update.ts @@ -4,10 +4,10 @@ import * as schema from "~/database/schema"; import { eq, and } from "drizzle-orm"; import { getSocketIO } from "~/server/socket"; -export async function action(args: any) { +import type { ActionFunctionArgs } from "react-router"; +export async function action(args: ActionFunctionArgs) { const { request } = args; - const auth = await getAuth(args); - const userId = (auth as any).userId as string | null; + const { userId } = await getAuth(args); if (!userId) { return Response.json({ error: "Unauthorized" }, { status: 401 }); diff --git a/app/routes/api/draft.adjust-time-bank.ts b/app/routes/api/draft.adjust-time-bank.ts index 78246e9..09cc59e 100644 --- a/app/routes/api/draft.adjust-time-bank.ts +++ b/app/routes/api/draft.adjust-time-bank.ts @@ -4,10 +4,10 @@ import { database } from "~/database/context"; import * as schema from "~/database/schema"; import { getSocketIO } from "../../../server/socket"; -export async function action(args: any) { +import type { ActionFunctionArgs } from "react-router"; +export async function action(args: ActionFunctionArgs) { const { request } = args; - const auth = await getAuth(args); - const userId = (auth as any).userId as string | null; + const { userId } = await getAuth(args); if (!userId) { return Response.json({ error: "Unauthorized" }, { status: 401 }); diff --git a/app/routes/api/draft.force-autopick.ts b/app/routes/api/draft.force-autopick.ts index c5679d7..a2fb8d8 100644 --- a/app/routes/api/draft.force-autopick.ts +++ b/app/routes/api/draft.force-autopick.ts @@ -4,10 +4,10 @@ import * as schema from "~/database/schema"; import { eq, and } from "drizzle-orm"; import { executeAutoPick } from "~/models/draft-utils"; -export async function action(args: any) { +import type { ActionFunctionArgs } from "react-router"; +export async function action(args: ActionFunctionArgs) { const { request } = args; - const auth = await getAuth(args); - const userId = (auth as any).userId as string | null; + const { userId } = await getAuth(args); if (!userId) { return Response.json({ error: "Unauthorized" }, { status: 401 }); diff --git a/app/routes/api/draft.force-manual-pick.ts b/app/routes/api/draft.force-manual-pick.ts index 5fa8276..8df1d35 100644 --- a/app/routes/api/draft.force-manual-pick.ts +++ b/app/routes/api/draft.force-manual-pick.ts @@ -7,11 +7,11 @@ import { getDraftPicksWithSports, getTeamDraftPicksWithSports } from "~/models/d import { getParticipantsForSeasonWithSports } from "~/models/participant"; import { getSeasonSportsSimple } from "~/models/season-sport"; import { getSocketIO } from "../../../server/socket"; +import type { ActionFunctionArgs } from "react-router"; -export async function action(args: any) { +export async function action(args: ActionFunctionArgs) { const { request } = args; - const auth = await getAuth(args); - const userId = (auth as any).userId as string | null; + const { userId } = await getAuth(args); if (!userId) { return Response.json({ error: "Unauthorized" }, { status: 401 }); @@ -50,6 +50,22 @@ export async function action(args: any) { return Response.json({ error: "Only commissioners can force a manual pick" }, { status: 403 }); } + if (season.status !== "draft") { + return Response.json({ error: "Draft is not currently active" }, { status: 400 }); + } + + // Check if a pick already exists at this pick number slot (prevents duplicate picks at same slot) + const existingPickAtSlot = await db.query.draftPicks.findFirst({ + where: and( + eq(schema.draftPicks.seasonId, seasonId), + eq(schema.draftPicks.pickNumber, pickNumber) + ), + }); + + if (existingPickAtSlot) { + return Response.json({ error: "A pick already exists at this slot" }, { status: 400 }); + } + // Check if participant is already drafted const existingPick = await db.query.draftPicks.findFirst({ where: and( diff --git a/app/routes/api/draft.make-pick.ts b/app/routes/api/draft.make-pick.ts index 9d2bdf8..82c33c9 100644 --- a/app/routes/api/draft.make-pick.ts +++ b/app/routes/api/draft.make-pick.ts @@ -8,10 +8,10 @@ import { getParticipantsForSeasonWithSports } from "~/models/participant"; import { getSeasonSportsSimple } from "~/models/season-sport"; import { getSocketIO } from "../../../server/socket"; -export async function action(args: any) { +import type { ActionFunctionArgs } from "react-router"; +export async function action(args: ActionFunctionArgs) { const { request } = args; - const auth = await getAuth(args); - const userId = (auth as any).userId as string | null; + const { userId } = await getAuth(args); if (!userId) { return Response.json({ error: "Unauthorized" }, { status: 401 }); @@ -56,12 +56,11 @@ export async function action(args: any) { const totalTeams = draftSlots.length; const currentRound = Math.ceil(currentPickNumber / totalTeams); - const isSnakeDraft = true; // Assuming snake draft const isEvenRound = currentRound % 2 === 0; - // Calculate which team should pick + // Calculate which team should pick (snake draft) let pickInRound = ((currentPickNumber - 1) % totalTeams) + 1; - if (isSnakeDraft && isEvenRound) { + if (isEvenRound) { pickInRound = totalTeams - pickInRound + 1; } diff --git a/app/routes/api/draft.pause.ts b/app/routes/api/draft.pause.ts index d11c9bc..36a3c3d 100644 --- a/app/routes/api/draft.pause.ts +++ b/app/routes/api/draft.pause.ts @@ -4,10 +4,10 @@ import { database } from "~/database/context"; import * as schema from "~/database/schema"; import { getSocketIO } from "../../../server/socket"; -export async function action(args: any) { +import type { ActionFunctionArgs } from "react-router"; +export async function action(args: ActionFunctionArgs) { const { request } = args; - const auth = await getAuth(args); - const userId = (auth as any).userId as string | null; + const { userId } = await getAuth(args); if (!userId) { return Response.json({ error: "Unauthorized" }, { status: 401 }); diff --git a/app/routes/api/draft.replace-pick.ts b/app/routes/api/draft.replace-pick.ts index cf47755..2c6de37 100644 --- a/app/routes/api/draft.replace-pick.ts +++ b/app/routes/api/draft.replace-pick.ts @@ -8,10 +8,10 @@ import { getParticipantsForSeasonWithSports } from "~/models/participant"; import { getSeasonSportsSimple } from "~/models/season-sport"; import { getSocketIO } from "../../../server/socket"; -export async function action(args: any) { +import type { ActionFunctionArgs } from "react-router"; +export async function action(args: ActionFunctionArgs) { const { request } = args; - const auth = await getAuth(args); - const userId = (auth as any).userId as string | null; + const { userId } = await getAuth(args); if (!userId) { return Response.json({ error: "Unauthorized" }, { status: 401 }); diff --git a/app/routes/api/draft.resume.ts b/app/routes/api/draft.resume.ts index 9ecc499..20fd25e 100644 --- a/app/routes/api/draft.resume.ts +++ b/app/routes/api/draft.resume.ts @@ -4,10 +4,10 @@ import { database } from "~/database/context"; import * as schema from "~/database/schema"; import { getSocketIO } from "../../../server/socket"; -export async function action(args: any) { +import type { ActionFunctionArgs } from "react-router"; +export async function action(args: ActionFunctionArgs) { const { request } = args; - const auth = await getAuth(args); - const userId = (auth as any).userId as string | null; + const { userId } = await getAuth(args); if (!userId) { return Response.json({ error: "Unauthorized" }, { status: 401 }); diff --git a/app/routes/api/draft.rollback.ts b/app/routes/api/draft.rollback.ts index 9c18dc2..839ff9e 100644 --- a/app/routes/api/draft.rollback.ts +++ b/app/routes/api/draft.rollback.ts @@ -4,10 +4,10 @@ import * as schema from "~/database/schema"; import { eq, and, gte } from "drizzle-orm"; import { getSocketIO } from "../../../server/socket"; -export async function action(args: any) { +import type { ActionFunctionArgs } from "react-router"; +export async function action(args: ActionFunctionArgs) { const { request } = args; - const auth = await getAuth(args); - const userId = (auth as any).userId as string | null; + const { userId } = await getAuth(args); if (!userId) { return Response.json({ error: "Unauthorized" }, { status: 401 }); diff --git a/app/routes/api/draft.start.ts b/app/routes/api/draft.start.ts index b53fdee..7f76321 100644 --- a/app/routes/api/draft.start.ts +++ b/app/routes/api/draft.start.ts @@ -4,10 +4,10 @@ import * as schema from "~/database/schema"; import { eq, and } from "drizzle-orm"; import { getSocketIO } from "../../../server/socket"; -export async function action(args: any) { +import type { ActionFunctionArgs } from "react-router"; +export async function action(args: ActionFunctionArgs) { const { request } = args; - const auth = await getAuth(args); - const userId = (auth as any).userId as string | null; + const { userId } = await getAuth(args); if (!userId) { return Response.json({ error: "Unauthorized" }, { status: 401 }); diff --git a/app/routes/api/queue.add.ts b/app/routes/api/queue.add.ts index 1a4738b..beb70b4 100644 --- a/app/routes/api/queue.add.ts +++ b/app/routes/api/queue.add.ts @@ -4,10 +4,10 @@ import * as schema from "~/database/schema"; import { eq } from "drizzle-orm"; import { addToQueue, getTeamQueue, isParticipantInQueue } from "~/models/draft-queue"; -export async function action(args: any) { +import type { ActionFunctionArgs } from "react-router"; +export async function action(args: ActionFunctionArgs) { const { request } = args; - const auth = await getAuth(args); - const userId = (auth as any).userId as string | null; + const { userId } = await getAuth(args); if (!userId) { return Response.json({ error: "Unauthorized" }, { status: 401 }); diff --git a/app/routes/api/queue.clear.ts b/app/routes/api/queue.clear.ts index cc3bf4b..b8bc6a5 100644 --- a/app/routes/api/queue.clear.ts +++ b/app/routes/api/queue.clear.ts @@ -3,11 +3,11 @@ import { database } from "~/database/context"; import * as schema from "~/database/schema"; import { eq } from "drizzle-orm"; import { clearTeamQueue } from "~/models/draft-queue"; +import type { ActionFunctionArgs } from "react-router"; -export async function action(args: any) { +export async function action(args: ActionFunctionArgs) { const { request } = args; - const auth = await getAuth(args); - const userId = (auth as any).userId as string | null; + const { userId } = await getAuth(args); if (!userId) { return Response.json({ error: "Unauthorized" }, { status: 401 }); diff --git a/app/routes/api/queue.remove.ts b/app/routes/api/queue.remove.ts index f5aceb3..bd23b2d 100644 --- a/app/routes/api/queue.remove.ts +++ b/app/routes/api/queue.remove.ts @@ -4,10 +4,10 @@ import * as schema from "~/database/schema"; import { eq } from "drizzle-orm"; import { removeFromQueue, getTeamQueue } from "~/models/draft-queue"; -export async function action(args: any) { +import type { ActionFunctionArgs } from "react-router"; +export async function action(args: ActionFunctionArgs) { const { request } = args; - const auth = await getAuth(args); - const userId = (auth as any).userId as string | null; + const { userId } = await getAuth(args); if (!userId) { return Response.json({ error: "Unauthorized" }, { status: 401 }); diff --git a/app/routes/api/queue.reorder.ts b/app/routes/api/queue.reorder.ts index a7accf3..bb09981 100644 --- a/app/routes/api/queue.reorder.ts +++ b/app/routes/api/queue.reorder.ts @@ -4,10 +4,10 @@ import * as schema from "~/database/schema"; import { eq } from "drizzle-orm"; import { reorderQueueWithSeason, getTeamQueue } from "~/models/draft-queue"; -export async function action(args: any) { +import type { ActionFunctionArgs } from "react-router"; +export async function action(args: ActionFunctionArgs) { const { request } = args; - const auth = await getAuth(args); - const userId = (auth as any).userId as string | null; + const { userId } = await getAuth(args); if (!userId) { return Response.json({ error: "Unauthorized" }, { status: 401 }); diff --git a/app/routes/leagues/$leagueId.draft-board.$seasonId.tsx b/app/routes/leagues/$leagueId.draft-board.$seasonId.tsx index 2fa61e2..d36ec48 100644 --- a/app/routes/leagues/$leagueId.draft-board.$seasonId.tsx +++ b/app/routes/leagues/$leagueId.draft-board.$seasonId.tsx @@ -7,8 +7,9 @@ import { DraftGrid } from "~/components/DraftGrid"; import { useDraftSocket } from "~/hooks/useDraftSocket"; import { useState, useEffect } from "react"; import { buildOwnerMap } from "~/lib/owner-map"; +import type { Route } from "./+types/$leagueId.draft-board.$seasonId"; -export async function loader(args: any) { +export async function loader(args: Route.LoaderArgs) { const { params } = args; const { leagueId, seasonId } = params; @@ -38,8 +39,7 @@ export async function loader(args: any) { // Check access: public boards are accessible to everyone without auth if (!season.league.isPublicDraftBoard) { // Not public - check if the user is a league member or commissioner - const auth = await getAuth(args); - const userId = (auth as any).userId as string | null; + const { userId } = await getAuth(args); if (!userId) { throw new Response("This draft board is not public", { status: 403 }); diff --git a/app/routes/leagues/$leagueId.draft.$seasonId.tsx b/app/routes/leagues/$leagueId.draft.$seasonId.tsx index db207e2..53e8a78 100644 --- a/app/routes/leagues/$leagueId.draft.$seasonId.tsx +++ b/app/routes/leagues/$leagueId.draft.$seasonId.tsx @@ -25,12 +25,12 @@ import { useDraftNotifications } from "~/hooks/useDraftNotifications"; import { NotificationSettings } from "~/components/NotificationSettings"; import { toast } from "sonner"; import { formatClockTime, getTimerColorClass } from "~/lib/draft-timer"; +import type { Route } from "./+types/$leagueId.draft.$seasonId"; -export async function loader(args: any) { +export async function loader(args: Route.LoaderArgs) { const { params } = args; const { seasonId, leagueId } = params; - const auth = await getAuth(args); - const userId = (auth as any).userId as string | null; + const { userId } = await getAuth(args); if (!seasonId) { throw new Response("Season ID is required", { status: 400 }); diff --git a/app/routes/leagues/$leagueId.server.ts b/app/routes/leagues/$leagueId.server.ts index 8596ee5..ec662c4 100644 --- a/app/routes/leagues/$leagueId.server.ts +++ b/app/routes/leagues/$leagueId.server.ts @@ -10,8 +10,9 @@ import { findDraftSlotsBySeasonId, } from "~/models"; import { findCurrentSeasonWithSports } from "~/models/season"; +import type { Route } from "./+types/$leagueId"; -export async function loader(args: any) { +export async function loader(args: Route.LoaderArgs) { const { userId } = await getAuth(args); const { params } = args; const { leagueId } = params; diff --git a/app/routes/leagues/$leagueId.standings.$seasonId.tsx b/app/routes/leagues/$leagueId.standings.$seasonId.tsx index fa7893f..7f30705 100644 --- a/app/routes/leagues/$leagueId.standings.$seasonId.tsx +++ b/app/routes/leagues/$leagueId.standings.$seasonId.tsx @@ -9,11 +9,11 @@ import { StandingsTable } from "~/components/standings/StandingsTable"; import { getSevenDayStandingsChange, getSeasonPointProgression } from "~/models/standings"; import { isSeasonComplete, getSeasonCompletionPercentage } from "~/lib/season-helpers.server"; import { PointProgressionChart } from "~/components/standings/PointProgressionChart"; +import type { Route } from "./+types/$leagueId.standings.$seasonId"; -export async function loader(args: any) { +export async function loader(args: Route.LoaderArgs) { try { - const auth = await getAuth(args); - const userId = (auth as any).userId as string | null; + const { userId } = await getAuth(args); const { params } = args; const { leagueId, seasonId } = params; diff --git a/server/socket.ts b/server/socket.ts index 837f179..7c8b66d 100644 --- a/server/socket.ts +++ b/server/socket.ts @@ -26,8 +26,8 @@ interface ServerToClientEvents { "team-connected": (data: { teamId: string }) => void; "team-disconnected": (data: { teamId: string }) => void; "connected-teams-list": (data: { teamIds: string[] }) => void; - "draft-paused": () => void; - "draft-resumed": () => void; + "draft-paused": (data: { seasonId: string; paused: boolean }) => void; + "draft-resumed": (data: { seasonId: string; paused: boolean }) => void; "participant-removed-from-queues": (data: { participantId: string }) => void; }