Address code review feedback on Discord pick announcements

- Fix "on the clock" timing: read currentPickNumber fresh from DB post-autodraft-chain (matches sendOnTheClockEmail guarantee)
- Remove outer try/catch from notifyPickMadeOnDiscord so callers' .catch() is not dead code
- Add missing draft-discord.server.test.ts with 12 tests covering all early-exit and happy paths
- Fix silent empty-string fallback for missing pickedSlot: warn and skip instead
- Eliminate sequential season→league DB queries by accepting leagueId as a direct param
- Show "save webhook URL to configure options" hint when URL is typed but not yet saved
- Remove block-scope braces at both call sites (plain const declarations)
- Remove redundant "Round N, Pick M" description line (title already carries this info)
- Inline pickInRoundFor helper to avoid circular import with draft-utils

https://claude.ai/code/session_01Tvwsv3LfL9JUqxoLct8dTn

app/components/league/settings/NotificationsSection.tsx

@@ -43,7 +43,7 @@ export function NotificationsSection({
             Create a webhook in Discord under Server Settings, Integrations, Webhooks.
           </p>
         </div>
-        {savedDiscordWebhookUrl && (
+        {savedDiscordWebhookUrl ? (
           <div className="flex items-center gap-3">
             <Switch
               id="discordPicksAnnouncementEnabled"
@@ -60,7 +60,11 @@ export function NotificationsSection({
               </p>
             </div>
           </div>
-        )}
+        ) : discordWebhookUrl ? (
+          <p className="text-sm text-muted-foreground">
+            Save the webhook URL above to configure additional notification options.
+          </p>
+        ) : null}
       </div>
     </SettingsSection>
   );

app/models/draft-utils.ts

@@ -842,23 +842,22 @@ export async function executeAutoPick(params: {
         .catch((err) => logger.error("[AutoPick] On-the-clock email failed:", err));
     }
 
-    {
-      const pickedSlot = draftSlots.find((s) => s.teamId === teamId);
-      let nextDraftSlotTeamId: string | null = null;
-      if (!isDraftComplete) {
-        const { pickInRound: nextPickInRound } = calculatePickInfo(nextPickNumber, totalTeams);
-        nextDraftSlotTeamId = draftSlots.find((s) => s.draftOrder === nextPickInRound)?.teamId ?? null;
-      }
+    const pickedSlot = draftSlots.find((s) => s.teamId === teamId);
+    if (!pickedSlot) {
+      logger.warn(`[AutoPick] Skipping Discord pick announcement: no draft slot found for team ${teamId}`);
+    } else {
       notifyPickMadeOnDiscord({
         seasonId,
-        pickedTeamName: pickedSlot?.team.name ?? "",
+        leagueId: season.leagueId,
+        pickedTeamName: pickedSlot.team.name,
         participantName: participantToPick.name,
         sportName: participantToPick.sportsSeason.sport.name,
         pickNumber,
         round: currentRound,
         pickInRound,
         isDraftComplete,
-        nextDraftSlotTeamId,
+        totalTeams,
+        draftSlots: draftSlots.map((s) => ({ teamId: s.teamId, draftOrder: s.draftOrder })),
         db,
       }).catch((err) => logger.error("[AutoPick] Discord pick announcement failed:", err));
     }

app/routes/api/draft.make-pick.ts

@@ -331,25 +331,20 @@ export async function action(args: ActionFunctionArgs) {
       .catch((err) => logger.error("On-the-clock email failed:", err));
   }
 
-  {
-    let nextDraftSlotTeamId: string | null = null;
-    if (!isDraftComplete) {
-      const { pickInRound: nextPickInRound } = calculatePickInfo(nextPickNumber, totalTeams);
-      nextDraftSlotTeamId = draftSlots.find((s) => s.draftOrder === nextPickInRound)?.teamId ?? null;
-    }
-    notifyPickMadeOnDiscord({
-      seasonId,
-      pickedTeamName: currentDraftSlot.team.name,
-      participantName: participant.name,
-      sportName: participant.sportsSeason.sport.name,
-      pickNumber: currentPickNumber,
-      round: currentRound,
-      pickInRound,
-      isDraftComplete,
-      nextDraftSlotTeamId,
-      db,
-    }).catch((err) => logger.error("Discord pick announcement failed:", err));
-  }
+  notifyPickMadeOnDiscord({
+    seasonId,
+    leagueId: season.leagueId,
+    pickedTeamName: currentDraftSlot.team.name,
+    participantName: participant.name,
+    sportName: participant.sportsSeason.sport.name,
+    pickNumber: currentPickNumber,
+    round: currentRound,
+    pickInRound,
+    isDraftComplete,
+    totalTeams,
+    draftSlots: draftSlots.map((s) => ({ teamId: s.teamId, draftOrder: s.draftOrder })),
+    db,
+  }).catch((err) => logger.error("Discord pick announcement failed:", err));
 
   return Response.json({
     success: true,

app/services/__tests__/draft-discord.server.test.ts

@@ -0,0 +1,234 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
+
+vi.mock("~/database/context", () => ({
+  database: vi.fn(),
+}));
+
+vi.mock("~/services/discord", () => ({
+  sendPickAnnouncementNotification: vi.fn().mockResolvedValue(undefined),
+}));
+
+vi.mock("~/models/account", () => ({
+  findDiscordIdsByUserIds: vi.fn().mockResolvedValue(new Map()),
+}));
+
+import { notifyPickMadeOnDiscord } from "../draft-discord.server";
+import { sendPickAnnouncementNotification } from "~/services/discord";
+import { findDiscordIdsByUserIds } from "~/models/account";
+
+// ---------------------------------------------------------------------------
+// Fixtures
+// ---------------------------------------------------------------------------
+
+const SEASON_ID = "season-1";
+const LEAGUE_ID = "league-1";
+const TEAM_ID = "team-1";
+const NEXT_TEAM_ID = "team-2";
+const OWNER_ID = "owner-1";
+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,
+  pickedTeamName: "Alpha FC",
+  participantName: "Erling Haaland",
+  sportName: "Soccer",
+  pickNumber: 1,
+  round: 1,
+  pickInRound: 1,
+  isDraftComplete: false,
+  totalTeams: 2,
+  draftSlots: DRAFT_SLOTS,
+};
+
+function makeLeague(overrides: object = {}) {
+  return {
+    id: LEAGUE_ID,
+    discordWebhookUrl: WEBHOOK_URL,
+    discordPicksAnnouncementEnabled: true,
+    ...overrides,
+  };
+}
+
+function makeSeason(currentPickNumber = 2) {
+  return { id: SEASON_ID, leagueId: LEAGUE_ID, currentPickNumber };
+}
+
+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;
+  season?: object | null;
+  nextTeam?: object | null;
+  owner?: object | null;
+} = {}) {
+  const league = "league" in overrides ? overrides.league : makeLeague();
+  const season = "season" in overrides ? overrides.season : makeSeason();
+  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) },
+      seasons: { findFirst: vi.fn().mockResolvedValue(season) },
+      teams: { findFirst: vi.fn().mockResolvedValue(nextTeam) },
+      users: { findFirst: vi.fn().mockResolvedValue(owner) },
+    },
+  };
+}
+
+// ---------------------------------------------------------------------------
+// Tests
+// ---------------------------------------------------------------------------
+
+beforeEach(() => {
+  vi.clearAllMocks();
+  process.env.APP_URL = "https://test.brackt.com";
+});
+
+afterEach(() => {
+  delete process.env.APP_URL;
+});
+
+describe("notifyPickMadeOnDiscord", () => {
+  it("returns early when league has no webhook URL", async () => {
+    const db = makeMockDb({ league: makeLeague({ discordWebhookUrl: null }) });
+
+    await notifyPickMadeOnDiscord({ ...BASE_PARAMS, db: db as never });
+
+    expect(sendPickAnnouncementNotification).not.toHaveBeenCalled();
+  });
+
+  it("returns early when picks announcement toggle is disabled", async () => {
+    const db = makeMockDb({ league: makeLeague({ discordPicksAnnouncementEnabled: false }) });
+
+    await notifyPickMadeOnDiscord({ ...BASE_PARAMS, db: db as never });
+
+    expect(sendPickAnnouncementNotification).not.toHaveBeenCalled();
+  });
+
+  it("returns early when league is not found", async () => {
+    const db = makeMockDb({ league: null });
+
+    await notifyPickMadeOnDiscord({ ...BASE_PARAMS, db: db as never });
+
+    expect(sendPickAnnouncementNotification).not.toHaveBeenCalled();
+  });
+
+  it("sends announcement with correct pick details", async () => {
+    const db = makeMockDb();
+
+    await notifyPickMadeOnDiscord({ ...BASE_PARAMS, db: db as never });
+
+    expect(sendPickAnnouncementNotification).toHaveBeenCalledWith(
+      expect.objectContaining({
+        webhookUrl: WEBHOOK_URL,
+        pickedTeamName: "Alpha FC",
+        participantName: "Erling Haaland",
+        sportName: "Soccer",
+        pickNumber: 1,
+        round: 1,
+        pickInRound: 1,
+        isDraftComplete: false,
+      })
+    );
+  });
+
+  it("builds the draft URL from APP_URL env var", async () => {
+    const db = makeMockDb();
+
+    await notifyPickMadeOnDiscord({ ...BASE_PARAMS, db: db as never });
+
+    expect(sendPickAnnouncementNotification).toHaveBeenCalledWith(
+      expect.objectContaining({
+        draftUrl: `https://test.brackt.com/leagues/${LEAGUE_ID}/draft/${SEASON_ID}`,
+      })
+    );
+  });
+
+  it("passes nextTeamName when fresh season resolves the next slot", async () => {
+    const db = makeMockDb({ season: makeSeason(2) }); // currentPickNumber=2 → pickInRound=2 → NEXT_TEAM_ID
+
+    await notifyPickMadeOnDiscord({ ...BASE_PARAMS, db: db as never });
+
+    expect(sendPickAnnouncementNotification).toHaveBeenCalledWith(
+      expect.objectContaining({ nextTeamName: "Beta United" })
+    );
+  });
+
+  it("reads next team from fresh DB pick number, not caller-supplied snapshot", async () => {
+    // Simulate autodraft chain advancing 2 picks: currentPickNumber=3 on a 2-team snake
+    // draft means pickInRound=2 again (round 2 reversed) → NEXT_TEAM_ID
+    const db = makeMockDb({ season: makeSeason(3) });
+
+    await notifyPickMadeOnDiscord({ ...BASE_PARAMS, db: db as never });
+
+    expect(db.query.seasons.findFirst).toHaveBeenCalledTimes(1);
+  });
+
+  it("passes nextOwnerDiscordId when owner has discordPingEnabled", async () => {
+    vi.mocked(findDiscordIdsByUserIds).mockResolvedValue(new Map([[NEXT_OWNER_ID, "discord-999"]]));
+    const db = makeMockDb();
+
+    await notifyPickMadeOnDiscord({ ...BASE_PARAMS, db: db as never });
+
+    expect(sendPickAnnouncementNotification).toHaveBeenCalledWith(
+      expect.objectContaining({ nextOwnerDiscordId: "discord-999" })
+    );
+  });
+
+  it("omits nextOwnerDiscordId when owner has discordPingEnabled: false", async () => {
+    const db = makeMockDb({ owner: makeOwner(NEXT_OWNER_ID, false) });
+
+    await notifyPickMadeOnDiscord({ ...BASE_PARAMS, db: db as never });
+
+    expect(findDiscordIdsByUserIds).not.toHaveBeenCalled();
+    expect(sendPickAnnouncementNotification).toHaveBeenCalledWith(
+      expect.objectContaining({ nextOwnerDiscordId: undefined })
+    );
+  });
+
+  it("omits nextTeamName when next team has no owner", async () => {
+    const db = makeMockDb({ nextTeam: makeTeam(NEXT_TEAM_ID, null) });
+
+    await notifyPickMadeOnDiscord({ ...BASE_PARAMS, db: db as never });
+
+    expect(sendPickAnnouncementNotification).toHaveBeenCalledWith(
+      expect.objectContaining({ nextTeamName: "Beta United", nextOwnerDiscordId: undefined })
+    );
+  });
+
+  it("skips next-team 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.seasons.findFirst).not.toHaveBeenCalled();
+    expect(sendPickAnnouncementNotification).toHaveBeenCalledWith(
+      expect.objectContaining({ isDraftComplete: true, nextTeamName: undefined })
+    );
+  });
+
+  it("suppresses errors thrown by sendPickAnnouncementNotification — caller's .catch handles them", async () => {
+    vi.mocked(sendPickAnnouncementNotification).mockRejectedValue(new Error("webhook down"));
+    const db = makeMockDb();
+
+    // The function itself does NOT swallow errors; the caller chains .catch().
+    await expect(
+      notifyPickMadeOnDiscord({ ...BASE_PARAMS, db: db as never })
+    ).rejects.toThrow("webhook down");
+  });
+});

app/services/discord.ts

@@ -217,7 +217,6 @@ export async function sendPickAnnouncementNotification({
 }): Promise<void> {
   const lines: string[] = [
     `**${escapeMarkdown(pickedTeamName)}** selected **${escapeMarkdown(participantName)}** (${escapeMarkdown(sportName)})`,
-    `Round ${round}, Pick ${pickInRound}`,
   ];
 
   if (isDraftComplete) {

app/services/draft-discord.server.ts

@@ -5,8 +5,21 @@ import { findDiscordIdsByUserIds } from "~/models/account";
 import { sendPickAnnouncementNotification } from "~/services/discord";
 import { logger } from "~/lib/logger";
 
+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;
   pickedTeamName: string;
   participantName: string;
   sportName: string;
@@ -14,11 +27,13 @@ export async function notifyPickMadeOnDiscord(params: {
   round: number;
   pickInRound: number;
   isDraftComplete: boolean;
-  nextDraftSlotTeamId: string | null;
+  totalTeams: number;
+  draftSlots: DraftSlot[];
   db: ReturnType<typeof database>;
 }): Promise<void> {
   const {
     seasonId,
+    leagueId,
     pickedTeamName,
     participantName,
     sportName,
@@ -26,59 +41,62 @@ export async function notifyPickMadeOnDiscord(params: {
     round,
     pickInRound,
     isDraftComplete,
-    nextDraftSlotTeamId,
+    totalTeams,
+    draftSlots,
     db,
   } = params;
 
-  try {
-    const season = await db.query.seasons.findFirst({
+  const league = await db.query.leagues.findFirst({
+    where: eq(schema.leagues.id, leagueId),
+  });
+  if (!league?.discordWebhookUrl || !league.discordPicksAnnouncementEnabled) return;
+
+  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) {
+    // Read currentPickNumber fresh from DB so we see the post-autodraft-chain state,
+    // matching the same guarantee sendOnTheClockEmail provides.
+    const freshSeason = await db.query.seasons.findFirst({
       where: eq(schema.seasons.id, seasonId),
     });
-    if (!season) return;
-
-    const league = await db.query.leagues.findFirst({
-      where: eq(schema.leagues.id, season.leagueId),
-    });
-    if (!league?.discordWebhookUrl || !league.discordPicksAnnouncementEnabled) return;
-
-    const appUrl = process.env.APP_URL ?? "https://brackt.com";
-    const draftUrl = `${appUrl}/leagues/${season.leagueId}/draft/${seasonId}`;
-
-    let nextTeamName: string | undefined;
-    let nextOwnerDiscordId: string | undefined;
-
-    if (!isDraftComplete && nextDraftSlotTeamId) {
-      const nextTeam = await db.query.teams.findFirst({
-        where: eq(schema.teams.id, nextDraftSlotTeamId),
-      });
-      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 (freshSeason) {
+      const nextPickInRound = pickInRoundFor(freshSeason.currentPickNumber ?? 1, 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);
+            }
           }
         }
       }
     }
-
-    await sendPickAnnouncementNotification({
-      webhookUrl: league.discordWebhookUrl,
-      draftUrl,
-      pickNumber,
-      round,
-      pickInRound,
-      pickedTeamName,
-      participantName,
-      sportName,
-      nextTeamName,
-      nextOwnerDiscordId,
-      isDraftComplete,
-    });
-  } catch (err) {
-    logger.error("[DiscordPick] Failed to send pick announcement:", err);
   }
+
+  await sendPickAnnouncementNotification({
+    webhookUrl: league.discordWebhookUrl,
+    draftUrl,
+    pickNumber,
+    round,
+    pickInRound,
+    pickedTeamName,
+    participantName,
+    sportName,
+    nextTeamName,
+    nextOwnerDiscordId,
+    isDraftComplete,
+  });
 }