From ad10bd3f7d299e5a411ed9356a727726d7c037a5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 29 May 2026 15:38:09 +0000 Subject: [PATCH 1/2] Simplify Discord pick notifications and fix force-manual-pick gap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace 30-line Map+Set queue with a 4-line per-league promise chain in discord.ts — picks still arrive in order, errors don't break the chain - Remove the internal DB re-queries from notifyPickMadeOnDiscord; callers now pre-resolve nextTeamName and nextTeamOwnerId from the draftSlots they already hold, eliminating the redundant team/owner lookups and the pickInRoundFor circular-import workaround - Add the missing Discord notification to draft.force-manual-pick.ts (was the only pick path that never notified Discord) - Update tests to match the simplified function signature https://claude.ai/code/session_01GCkguG2muQwnh3WTvENrwJ --- app/models/draft-utils.ts | 9 ++- app/routes/api/draft.force-manual-pick.ts | 24 +++++- app/routes/api/draft.make-pick.ts | 9 ++- .../__tests__/draft-discord.server.test.ts | 73 +++++-------------- app/services/discord.ts | 37 +--------- app/services/draft-discord.server.ts | 56 ++++---------- 6 files changed, 70 insertions(+), 138 deletions(-) diff --git a/app/models/draft-utils.ts b/app/models/draft-utils.ts index 415be11..053f0ed 100644 --- a/app/models/draft-utils.ts +++ b/app/models/draft-utils.ts @@ -834,6 +834,8 @@ export async function executeAutoPick(params: { if (!pickedSlot) { logger.warn(`[AutoPick] Skipping Discord pick announcement: no draft slot found for team ${teamId}`); } else { + const { pickInRound: nextPickInRound } = calculatePickInfo(nextPickNumber, totalTeams); + const nextSlot = !isDraftComplete ? draftSlots.find((s) => s.draftOrder === nextPickInRound) : undefined; enqueuePickNotification(season.leagueId, () => notifyPickMadeOnDiscord({ seasonId, @@ -842,12 +844,11 @@ export async function executeAutoPick(params: { participantName: participantToPick.name, sportName: participantToPick.sportsSeason.sport.name, pickNumber, - nextPickNumber, round: currentRound, - rawPickInRound, + pickInRound: rawPickInRound, isDraftComplete, - totalTeams, - draftSlots: draftSlots.map((s) => ({ teamId: s.teamId, draftOrder: s.draftOrder })), + nextTeamName: nextSlot?.team.name, + nextTeamOwnerId: nextSlot?.team.ownerId, db, }).catch((err) => logger.error("[AutoPick] Discord pick announcement failed:", err)) ); diff --git a/app/routes/api/draft.force-manual-pick.ts b/app/routes/api/draft.force-manual-pick.ts index 6350d8d..3ac2fd1 100644 --- a/app/routes/api/draft.force-manual-pick.ts +++ b/app/routes/api/draft.force-manual-pick.ts @@ -8,6 +8,8 @@ import { getDraftPicksWithSports, getTeamDraftPicksWithSports } from "~/models/d import { getParticipantsForSeasonWithSports } from "~/models/season-participant"; import { getSeasonSportsSimple } from "~/models/season-sport"; import { calculatePickInfo, checkAndTriggerNextAutodraft } from "~/models/draft-utils"; +import { enqueuePickNotification } from "~/services/discord"; +import { notifyPickMadeOnDiscord } from "~/services/draft-discord.server"; import { logCommissionerAction } from "~/models/audit-log"; import { getSocketIO, scheduleDraftRoomClosure } from "../../../server/socket"; import { logger } from "~/lib/logger"; @@ -113,7 +115,7 @@ export async function action(args: ActionFunctionArgs) { }); const totalTeams = draftSlots.length; - const { round: currentRound, pickInRound } = calculatePickInfo(pickNumber, totalTeams); + const { round: currentRound, pickInRound, rawPickInRound } = calculatePickInfo(pickNumber, totalTeams); // Validate that the submitted teamId is actually the team whose turn it is at pickNumber const expectedDraftSlot = draftSlots.find((slot) => slot.draftOrder === pickInRound); @@ -264,6 +266,26 @@ export async function action(args: ActionFunctionArgs) { const pickedTeam = draftSlots.find((slot) => slot.team.id === teamId)?.team; + // Announce pick on Discord + const { pickInRound: nextPickInRound } = calculatePickInfo(nextPickNumber, totalTeams); + const nextSlot = !isDraftComplete ? draftSlots.find((s) => s.draftOrder === nextPickInRound) : undefined; + enqueuePickNotification(season.leagueId, () => + notifyPickMadeOnDiscord({ + seasonId, + leagueId: season.leagueId, + pickedTeamName: pickedTeam?.name ?? teamId, + participantName: participant.name, + sportName: participant.sportsSeason.sport.name, + pickNumber, + round: currentRound, + pickInRound: rawPickInRound, + isDraftComplete, + nextTeamName: nextSlot?.team.name, + nextTeamOwnerId: nextSlot?.team.ownerId, + db, + }).catch((err) => logger.error("Discord pick announcement failed:", err)) + ); + await logCommissionerAction({ seasonId, leagueId: season.leagueId, diff --git a/app/routes/api/draft.make-pick.ts b/app/routes/api/draft.make-pick.ts index 168ff5e..1ecb368 100644 --- a/app/routes/api/draft.make-pick.ts +++ b/app/routes/api/draft.make-pick.ts @@ -306,6 +306,8 @@ export async function action(args: ActionFunctionArgs) { // Announce before triggering the chain so Discord messages arrive in pick-number // order. If the chain ran first, chained picks would announce before this one. + const { pickInRound: nextPickInRound } = calculatePickInfo(nextPickNumber, totalTeams); + const nextSlot = !isDraftComplete ? draftSlots.find((s) => s.draftOrder === nextPickInRound) : undefined; enqueuePickNotification(season.leagueId, () => notifyPickMadeOnDiscord({ seasonId, @@ -314,12 +316,11 @@ export async function action(args: ActionFunctionArgs) { participantName: participant.name, sportName: participant.sportsSeason.sport.name, pickNumber: currentPickNumber, - nextPickNumber, round: currentRound, - rawPickInRound, + pickInRound: rawPickInRound, isDraftComplete, - totalTeams, - draftSlots: draftSlots.map((s) => ({ teamId: s.teamId, draftOrder: s.draftOrder })), + nextTeamName: nextSlot?.team.name, + nextTeamOwnerId: nextSlot?.team.ownerId, db, }).catch((err) => logger.error("Discord pick announcement failed:", err)) ); diff --git a/app/services/__tests__/draft-discord.server.test.ts b/app/services/__tests__/draft-discord.server.test.ts index ac5c9fb..5bcbe5f 100644 --- a/app/services/__tests__/draft-discord.server.test.ts +++ b/app/services/__tests__/draft-discord.server.test.ts @@ -22,17 +22,9 @@ import { findDiscordIdsByUserIds } from "~/models/account"; const SEASON_ID = "season-1"; const LEAGUE_ID = "league-1"; -const TEAM_ID = "team-1"; -const NEXT_TEAM_ID = "team-2"; const NEXT_OWNER_ID = "owner-2"; const WEBHOOK_URL = "https://discord.com/api/webhooks/123/abc"; -/** 2-team draft; snake: pick 1→team1, pick 2→team2, pick 3→team2, pick 4→team1 */ -const DRAFT_SLOTS = [ - { teamId: TEAM_ID, draftOrder: 1 }, - { teamId: NEXT_TEAM_ID, draftOrder: 2 }, -]; - const BASE_PARAMS = { seasonId: SEASON_ID, leagueId: LEAGUE_ID, @@ -40,12 +32,11 @@ const BASE_PARAMS = { participantName: "Erling Haaland", sportName: "Soccer", pickNumber: 1, - nextPickNumber: 2, round: 1, - rawPickInRound: 1, + pickInRound: 1, isDraftComplete: false, - totalTeams: 2, - draftSlots: DRAFT_SLOTS, + nextTeamName: "Beta United", + nextTeamOwnerId: NEXT_OWNER_ID, }; function makeLeague(overrides: object = {}) { @@ -57,27 +48,20 @@ function makeLeague(overrides: object = {}) { }; } -function makeTeam(id: string, ownerId: string | null = null, name = "Beta United") { - return { id, name, ownerId }; -} - function makeOwner(id: string, discordPingEnabled = true) { return { id, discordPingEnabled }; } function makeMockDb(overrides: { league?: object | null; - nextTeam?: object | null; owner?: object | null; } = {}) { const league = "league" in overrides ? overrides.league : makeLeague(); - const nextTeam = "nextTeam" in overrides ? overrides.nextTeam : makeTeam(NEXT_TEAM_ID, NEXT_OWNER_ID); const owner = "owner" in overrides ? overrides.owner : makeOwner(NEXT_OWNER_ID); return { query: { leagues: { findFirst: vi.fn().mockResolvedValue(league) }, - teams: { findFirst: vi.fn().mockResolvedValue(nextTeam) }, users: { findFirst: vi.fn().mockResolvedValue(owner) }, }, }; @@ -153,48 +137,29 @@ describe("notifyPickMadeOnDiscord", () => { ); }); - it("passes nextTeamName based on caller-supplied nextPickNumber", async () => { - // pick 3 in a 2-team snake: round 2 (reversed) → draftOrder 2 → NEXT_TEAM_ID → "Beta United" + it("forwards caller-supplied nextTeamName to the notification", async () => { const db = makeMockDb(); - await notifyPickMadeOnDiscord({ ...BASE_PARAMS, nextPickNumber: 3, db: db as never }); + await notifyPickMadeOnDiscord({ ...BASE_PARAMS, nextTeamName: "Beta United", db: db as never }); expect(sendPickAnnouncementNotification).toHaveBeenCalledWith( expect.objectContaining({ nextTeamName: "Beta United" }) ); }); - it("uses caller-supplied nextPickNumber instead of reading from DB", async () => { + it("does not query the teams table (team resolution is the caller's responsibility)", async () => { const db = makeMockDb(); await notifyPickMadeOnDiscord({ ...BASE_PARAMS, db: db as never }); - expect((db.query as Record).seasons).toBeUndefined(); + expect((db.query as Record).teams).toBeUndefined(); }); - it("shows the immediate next drafter, not the post-autodraft-chain drafter", async () => { - // Regression: manual pick #1 by alpha; beta immediately autodrafts (#2); DB advances to pick #3. - // Discord for pick #1 must show beta ("On the clock: beta"), not gamma. - // Uses a 3-team snake: pick 1→alpha, pick 2→beta, pick 3→gamma. - const ALPHA_ID = "team-alpha"; - const BETA_ID = "team-beta"; - const GAMMA_ID = "team-gamma"; - const threeTeamSlots = [ - { teamId: ALPHA_ID, draftOrder: 1 }, - { teamId: BETA_ID, draftOrder: 2 }, - { teamId: GAMMA_ID, draftOrder: 3 }, - ]; - const betaTeam = { id: BETA_ID, name: "Beta Squad", ownerId: null }; - const db = makeMockDb({ nextTeam: betaTeam }); + it("forwards caller-supplied nextTeamName regardless of pick number", async () => { + // Callers must pass the correct nextTeamName; this service just forwards it. + const db = makeMockDb(); - await notifyPickMadeOnDiscord({ - ...BASE_PARAMS, - pickNumber: 1, - nextPickNumber: 2, // immediately after alpha's pick — before beta's autodraft - totalTeams: 3, - draftSlots: threeTeamSlots, - db: db as never, - }); + await notifyPickMadeOnDiscord({ ...BASE_PARAMS, nextTeamName: "Beta Squad", db: db as never }); expect(sendPickAnnouncementNotification).toHaveBeenCalledWith( expect.objectContaining({ nextTeamName: "Beta Squad" }) @@ -223,24 +188,25 @@ describe("notifyPickMadeOnDiscord", () => { ); }); - it("omits nextTeamName when next team has no owner", async () => { - const db = makeMockDb({ nextTeam: makeTeam(NEXT_TEAM_ID, null) }); + it("omits nextOwnerDiscordId when nextTeamOwnerId is null", async () => { + const db = makeMockDb(); - await notifyPickMadeOnDiscord({ ...BASE_PARAMS, db: db as never }); + await notifyPickMadeOnDiscord({ ...BASE_PARAMS, nextTeamOwnerId: null, db: db as never }); + expect(findDiscordIdsByUserIds).not.toHaveBeenCalled(); expect(sendPickAnnouncementNotification).toHaveBeenCalledWith( expect.objectContaining({ nextTeamName: "Beta United", nextOwnerDiscordId: undefined }) ); }); - it("skips next-team lookup and passes isDraftComplete: true when draft is done", async () => { + it("skips owner lookup and passes isDraftComplete: true when draft is done", async () => { const db = makeMockDb(); await notifyPickMadeOnDiscord({ ...BASE_PARAMS, isDraftComplete: true, db: db as never }); - expect(db.query.teams.findFirst).not.toHaveBeenCalled(); + expect(db.query.users.findFirst).not.toHaveBeenCalled(); expect(sendPickAnnouncementNotification).toHaveBeenCalledWith( - expect.objectContaining({ isDraftComplete: true, nextTeamName: undefined }) + expect.objectContaining({ isDraftComplete: true }) ); }); @@ -263,8 +229,7 @@ describe("notifyPickMadeOnDiscord", () => { ...BASE_PARAMS, pickNumber: 22, round: 2, - rawPickInRound: 9, - totalTeams: 13, + pickInRound: 9, db: db as never, }); diff --git a/app/services/discord.ts b/app/services/discord.ts index ff8ed9c..ef8e1a0 100644 --- a/app/services/discord.ts +++ b/app/services/discord.ts @@ -14,42 +14,13 @@ interface DiscordWebhookPayload { allowed_mentions?: { parse: string[]; users: string[] }; } -// Per-league serial queue: serializes pick notifications within a league to +// Per-league serial promise chain: serializes pick notifications within a league to // respect Discord's per-webhook rate limit (~5 req/2s) during autodraft chains. -// Keyed by leagueId so concurrent drafts in different leagues don't block each other. -const leagueQueues = new Map Promise>>(); -const drainingLeagues = new Set(); - -async function drainLeagueQueue(leagueId: string) { - if (drainingLeagues.has(leagueId)) return; - drainingLeagues.add(leagueId); - try { - for (;;) { - const queue = leagueQueues.get(leagueId); - if (!queue || queue.length === 0) break; - const task = queue.shift(); - if (!task) break; - try { - await task(); - } catch { - // task already .catch()es its own errors; this is a safety net so one - // bad task doesn't prevent the rest of the queue from draining - } - } - } finally { - drainingLeagues.delete(leagueId); - leagueQueues.delete(leagueId); - } -} +const leagueChains = new Map>(); export function enqueuePickNotification(leagueId: string, fn: () => Promise): void { - let queue = leagueQueues.get(leagueId); - if (!queue) { - queue = []; - leagueQueues.set(leagueId, queue); - } - queue.push(fn); - drainLeagueQueue(leagueId).catch(() => {}); + const prev = leagueChains.get(leagueId) ?? Promise.resolve(); + leagueChains.set(leagueId, prev.then(() => fn().catch(() => {}))); } export async function sendDiscordWebhook( diff --git a/app/services/draft-discord.server.ts b/app/services/draft-discord.server.ts index 5884e89..a3f892f 100644 --- a/app/services/draft-discord.server.ts +++ b/app/services/draft-discord.server.ts @@ -4,18 +4,6 @@ import type { database } from "~/database/context"; import { findDiscordIdsByUserIds } from "~/models/account"; import { sendPickAnnouncementNotification } from "~/services/discord"; -type DraftSlot = { teamId: string; draftOrder: number }; - -// Inline snake-draft pick calculation — mirrors calculatePickInfo in draft-utils.ts -// without creating a circular import (draft-utils imports this module). -function pickInRoundFor(pickNumber: number, teamCount: number): number { - const round = Math.ceil(pickNumber / teamCount); - const rawPickInRound = ((pickNumber - 1) % teamCount) + 1; - const isOddRound = round % 2 === 1; - const teamIndex = isOddRound ? rawPickInRound - 1 : teamCount - rawPickInRound; - return teamIndex + 1; -} - export async function notifyPickMadeOnDiscord(params: { seasonId: string; leagueId: string; @@ -23,12 +11,11 @@ export async function notifyPickMadeOnDiscord(params: { participantName: string; sportName: string; pickNumber: number; - nextPickNumber: number; round: number; - rawPickInRound: number; + pickInRound: number; isDraftComplete: boolean; - totalTeams: number; - draftSlots: DraftSlot[]; + nextTeamName?: string; + nextTeamOwnerId?: string | null; db: ReturnType; }): Promise { const { @@ -38,12 +25,11 @@ export async function notifyPickMadeOnDiscord(params: { participantName, sportName, pickNumber, - nextPickNumber, round, - rawPickInRound, + pickInRound, isDraftComplete, - totalTeams, - draftSlots, + nextTeamName, + nextTeamOwnerId, db, } = params; @@ -55,28 +41,14 @@ export async function notifyPickMadeOnDiscord(params: { const appUrl = process.env.APP_URL ?? "https://brackt.com"; const draftUrl = `${appUrl}/leagues/${leagueId}/draft/${seasonId}`; - let nextTeamName: string | undefined; let nextOwnerDiscordId: string | undefined; - - if (!isDraftComplete) { - const nextPickInRound = pickInRoundFor(nextPickNumber, totalTeams); - const nextSlot = draftSlots.find((s) => s.draftOrder === nextPickInRound); - if (nextSlot) { - const nextTeam = await db.query.teams.findFirst({ - where: eq(schema.teams.id, nextSlot.teamId), - }); - if (nextTeam) { - nextTeamName = nextTeam.name; - if (nextTeam.ownerId) { - const owner = await db.query.users.findFirst({ - where: eq(schema.users.id, nextTeam.ownerId), - }); - if (owner?.discordPingEnabled) { - const discordIds = await findDiscordIdsByUserIds([owner.id]); - nextOwnerDiscordId = discordIds.get(owner.id); - } - } - } + if (!isDraftComplete && nextTeamOwnerId) { + const owner = await db.query.users.findFirst({ + where: eq(schema.users.id, nextTeamOwnerId), + }); + if (owner?.discordPingEnabled) { + const discordIds = await findDiscordIdsByUserIds([owner.id]); + nextOwnerDiscordId = discordIds.get(owner.id); } } @@ -85,7 +57,7 @@ export async function notifyPickMadeOnDiscord(params: { draftUrl, pickNumber, round, - pickInRound: rawPickInRound, + pickInRound, pickedTeamName, participantName, sportName, -- 2.45.3 From 5506af65fdf2aa64d098a4f925d4dbe05ea5975e Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 29 May 2026 16:15:31 +0000 Subject: [PATCH 2/2] Fix three code-review findings from Discord notification refactor - Add reference-equality cleanup guard to enqueuePickNotification so leagueChains entries are deleted once a league's chain settles (matching the old queue's finally-block cleanup), while correctly skipping deletion if a new pick was enqueued before the current one resolved - Use getTeamForPick from ~/lib/draft-order in draft.make-pick.ts and draft.force-manual-pick.ts instead of the inline calculatePickInfo + draftSlots.find pattern (draft-utils.ts has a same-named local function with a different signature so it keeps the two-liner) - Replace pickedTeam?.name ?? teamId UUID fallback in force-manual-pick with expectedDraftSlot.team.name, which is already validated non-null by the route's own guard; also removes two duplicate draftSlots.find calls that re-searched for a slot already in hand https://claude.ai/code/session_01GCkguG2muQwnh3WTvENrwJ --- app/routes/api/draft.force-manual-pick.ts | 12 +++++------- app/routes/api/draft.make-pick.ts | 4 ++-- app/services/discord.ts | 6 +++++- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/app/routes/api/draft.force-manual-pick.ts b/app/routes/api/draft.force-manual-pick.ts index 3ac2fd1..5752f51 100644 --- a/app/routes/api/draft.force-manual-pick.ts +++ b/app/routes/api/draft.force-manual-pick.ts @@ -8,6 +8,7 @@ import { getDraftPicksWithSports, getTeamDraftPicksWithSports } from "~/models/d import { getParticipantsForSeasonWithSports } from "~/models/season-participant"; import { getSeasonSportsSimple } from "~/models/season-sport"; import { calculatePickInfo, checkAndTriggerNextAutodraft } from "~/models/draft-utils"; +import { getTeamForPick } from "~/lib/draft-order"; import { enqueuePickNotification } from "~/services/discord"; import { notifyPickMadeOnDiscord } from "~/services/draft-discord.server"; import { logCommissionerAction } from "~/models/audit-log"; @@ -240,7 +241,7 @@ export async function action(args: ActionFunctionArgs) { // Emit socket event try { const io = getSocketIO(); - const team = draftSlots.find((slot) => slot.team.id === teamId)?.team; + const team = expectedDraftSlot.team; io.to(`draft-${seasonId}`).emit("pick-made", { pick: { @@ -264,16 +265,13 @@ export async function action(args: ActionFunctionArgs) { logger.error("Socket.IO error:", error); } - const pickedTeam = draftSlots.find((slot) => slot.team.id === teamId)?.team; - // Announce pick on Discord - const { pickInRound: nextPickInRound } = calculatePickInfo(nextPickNumber, totalTeams); - const nextSlot = !isDraftComplete ? draftSlots.find((s) => s.draftOrder === nextPickInRound) : undefined; + const nextSlot = !isDraftComplete ? getTeamForPick(nextPickNumber, draftSlots) : undefined; enqueuePickNotification(season.leagueId, () => notifyPickMadeOnDiscord({ seasonId, leagueId: season.leagueId, - pickedTeamName: pickedTeam?.name ?? teamId, + pickedTeamName: expectedDraftSlot.team.name, participantName: participant.name, sportName: participant.sportsSeason.sport.name, pickNumber, @@ -295,7 +293,7 @@ export async function action(args: ActionFunctionArgs) { details: { pickNumber, teamId, - teamName: pickedTeam?.name ?? teamId, + teamName: expectedDraftSlot.team.name, participantId, participantName: participant.name, }, diff --git a/app/routes/api/draft.make-pick.ts b/app/routes/api/draft.make-pick.ts index 1ecb368..032a43e 100644 --- a/app/routes/api/draft.make-pick.ts +++ b/app/routes/api/draft.make-pick.ts @@ -1,4 +1,5 @@ import { auth } from "~/lib/auth.server"; +import { getTeamForPick } from "~/lib/draft-order"; import { database } from "~/database/context"; import * as schema from "~/database/schema"; import { eq, and, sql } from "drizzle-orm"; @@ -306,8 +307,7 @@ export async function action(args: ActionFunctionArgs) { // Announce before triggering the chain so Discord messages arrive in pick-number // order. If the chain ran first, chained picks would announce before this one. - const { pickInRound: nextPickInRound } = calculatePickInfo(nextPickNumber, totalTeams); - const nextSlot = !isDraftComplete ? draftSlots.find((s) => s.draftOrder === nextPickInRound) : undefined; + const nextSlot = !isDraftComplete ? getTeamForPick(nextPickNumber, draftSlots) : undefined; enqueuePickNotification(season.leagueId, () => notifyPickMadeOnDiscord({ seasonId, diff --git a/app/services/discord.ts b/app/services/discord.ts index ef8e1a0..016c064 100644 --- a/app/services/discord.ts +++ b/app/services/discord.ts @@ -20,7 +20,11 @@ const leagueChains = new Map>(); export function enqueuePickNotification(leagueId: string, fn: () => Promise): void { const prev = leagueChains.get(leagueId) ?? Promise.resolve(); - leagueChains.set(leagueId, prev.then(() => fn().catch(() => {}))); + const next = prev.then(() => fn().catch(() => {})); + leagueChains.set(leagueId, next); + // Clean up the map entry once the chain settles, but only if no newer pick + // was enqueued after this one (reference equality guards against that race). + next.then(() => { if (leagueChains.get(leagueId) === next) leagueChains.delete(leagueId); }); } export async function sendDiscordWebhook( -- 2.45.3