Merge pull request 'fix notifications' (#126) from claude/wimbledon-bracket-announcement-bug-tg1wzl into main

Reviewed-on: #126

app/models/__tests__/populate-bracket-from-draw.test.ts

@@ -0,0 +1,113 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+
+// populateBracketFromDraw upserts a full draw into playoff_matches and reports
+// which losers' matches *transitioned to complete on this run* — the signal used
+// to announce knockouts once, idempotently across re-syncs.
+
+const existingRows: Array<Record<string, unknown>> = [];
+
+const mockDb = {
+  query: {
+    playoffMatches: {
+      findMany: vi.fn(async () => existingRows),
+    },
+  },
+  update: vi.fn(() => ({
+    set: vi.fn(() => ({ where: vi.fn().mockResolvedValue(undefined) })),
+  })),
+  insert: vi.fn(() => ({
+    values: vi.fn(() => ({ returning: vi.fn().mockResolvedValue([]) })),
+  })),
+};
+
+vi.mock("~/database/context", () => ({ database: () => mockDb }));
+
+import { populateBracketFromDraw, type ResolvedDrawMatch } from "../playoff-match";
+
+const EVENT_ID = "ev-1";
+
+function match(overrides: Partial<ResolvedDrawMatch> = {}): ResolvedDrawMatch {
+  return {
+    externalMatchId: "m-1",
+    round: "Round of 64",
+    matchNumber: 1,
+    participant1Id: "winner",
+    participant2Id: "loser",
+    winnerId: "winner",
+    loserId: "loser",
+    isScoring: false,
+    ...overrides,
+  };
+}
+
+beforeEach(() => {
+  vi.clearAllMocks();
+  existingRows.length = 0;
+});
+
+describe("populateBracketFromDraw newlyDecidedLoserIds", () => {
+  it("reports the loser of a brand-new completed match", async () => {
+    const { newlyDecidedLoserIds } = await populateBracketFromDraw(EVENT_ID, [
+      match({ externalMatchId: "m-1", loserId: "mensik" }),
+    ]);
+
+    expect(newlyDecidedLoserIds).toEqual(["mensik"]);
+  });
+
+  it("reports the loser of an existing match that just reached completion", async () => {
+    existingRows.push({
+      id: "row-1",
+      externalMatchId: "m-1",
+      isComplete: false,
+      loserId: null,
+      winnerId: null,
+    });
+
+    const { newlyDecidedLoserIds } = await populateBracketFromDraw(EVENT_ID, [
+      match({ externalMatchId: "m-1", loserId: "mensik" }),
+    ]);
+
+    expect(newlyDecidedLoserIds).toEqual(["mensik"]);
+  });
+
+  it("reports nothing on an idempotent re-sync of an already-complete match", async () => {
+    existingRows.push({
+      id: "row-1",
+      externalMatchId: "m-1",
+      isComplete: true,
+      loserId: "mensik",
+      winnerId: "winner",
+    });
+
+    const { newlyDecidedLoserIds } = await populateBracketFromDraw(EVENT_ID, [
+      match({ externalMatchId: "m-1", loserId: "mensik" }),
+    ]);
+
+    expect(newlyDecidedLoserIds).toEqual([]);
+  });
+
+  it("ignores incomplete matches (no winner/loser yet)", async () => {
+    const { newlyDecidedLoserIds, completed } = await populateBracketFromDraw(EVENT_ID, [
+      match({ externalMatchId: "m-2", winnerId: null, loserId: null }),
+    ]);
+
+    expect(newlyDecidedLoserIds).toEqual([]);
+    expect(completed).toBe(0);
+  });
+
+  it("collects only the newly-decided losers in a mixed batch", async () => {
+    existingRows.push(
+      { id: "row-1", externalMatchId: "m-1", isComplete: true, loserId: "old", winnerId: "w1" },
+      { id: "row-2", externalMatchId: "m-2", isComplete: false, loserId: null, winnerId: null },
+    );
+
+    const { newlyDecidedLoserIds } = await populateBracketFromDraw(EVENT_ID, [
+      match({ externalMatchId: "m-1", loserId: "old" }), // already complete → skip
+      match({ externalMatchId: "m-2", loserId: "freshly-out" }), // transitioned → include
+      match({ externalMatchId: "m-3", loserId: "brand-new-out" }), // new complete → include
+      match({ externalMatchId: "m-4", winnerId: null, loserId: null }), // incomplete → skip
+    ]);
+
+    expect(newlyDecidedLoserIds.toSorted()).toEqual(["brand-new-out", "freshly-out"]);
+  });
+});

app/models/playoff-match.ts

@@ -164,12 +164,16 @@ export interface ResolvedDrawMatch {
  * (e.g. Wikipedia) that reports the full draw including completed rounds. A row
  * is marked complete when both its winner and loser are known.
  *
- * @returns counts of rows written (inserted or updated) and those carrying a result.
+ * @returns counts of rows written (inserted or updated), those carrying a result,
+ * and the participant ids of losers whose match *transitioned to complete on this
+ * run* (a row that was absent or not-yet-complete before and is complete now).
+ * That set is the newly-decided eliminations — used to announce knockouts once,
+ * idempotently across re-syncs, since playoff_matches persist between syncs.
  */
 export async function populateBracketFromDraw(
   eventId: string,
   matches: ResolvedDrawMatch[]
-): Promise<{ written: number; completed: number }> {
+): Promise<{ written: number; completed: number; newlyDecidedLoserIds: string[] }> {
   const db = database();
 
   const existing = await db.query.playoffMatches.findMany({
@@ -184,12 +188,20 @@ export async function populateBracketFromDraw(
   const toInsert: NewPlayoffMatch[] = [];
   let written = 0;
   let completed = 0;
+  const newlyDecidedLoserIds: string[] = [];
 
   for (const m of matches) {
     const isComplete = m.winnerId !== null && m.loserId !== null;
     if (isComplete) completed++;
 
     const existingRow = byExternalId.get(m.externalMatchId);
+    // Loser is "newly decided" when this match reaches completion for the first
+    // time: either a brand-new complete row, or an existing row that was not
+    // complete before. Re-syncing an already-complete match yields nothing.
+    if (isComplete && m.loserId && !existingRow?.isComplete) {
+      newlyDecidedLoserIds.push(m.loserId);
+    }
+
     if (existingRow) {
       await db
         .update(schema.playoffMatches)
@@ -229,7 +241,7 @@ export async function populateBracketFromDraw(
     written += toInsert.length;
   }
 
-  return { written, completed };
+  return { written, completed, newlyDecidedLoserIds };
 }
 
 /**

app/services/__tests__/discord.test.ts

@@ -819,6 +819,47 @@ describe("sendQualifyingPointsUpdateNotification", () => {
     expect(fetch).not.toHaveBeenCalled();
   });
 
+  it("shows Knocked Out section for eliminated entries, tagging the manager", async () => {
+    await sendQualifyingPointsUpdateNotification({
+      webhookUrl: WEBHOOK_URL,
+      seasonName: "Slam League 2025",
+      entries: BASE_ENTRIES,
+      eliminated: [{ participantName: "Jakob Mensik", ownerUsername: "chris" }],
+    });
+
+    const desc = getDescription();
+    expect(desc).toContain("**Knocked Out**");
+    expect(desc).toContain("• Jakob Mensik (chris)");
+  });
+
+  it("sends with only a Knocked Out section when there are no QP entries, omitting QP Standings", async () => {
+    await sendQualifyingPointsUpdateNotification({
+      webhookUrl: WEBHOOK_URL,
+      seasonName: "Slam League 2025",
+      entries: [],
+      eliminated: [{ participantName: "Jakob Mensik", ownerUsername: "chris" }],
+    });
+
+    expect(fetch).toHaveBeenCalledOnce();
+    const desc = getDescription();
+    expect(desc).toContain("**Knocked Out**");
+    expect(desc).not.toContain("**QP Standings**");
+    expect(desc).not.toContain("**Points Awarded**");
+  });
+
+  it("pings opted-in owners who appear only in the Knocked Out section", async () => {
+    await sendQualifyingPointsUpdateNotification({
+      webhookUrl: WEBHOOK_URL,
+      seasonName: "Slam League 2025",
+      entries: [],
+      eliminated: [{ participantName: "Jakob Mensik", ownerDiscordUserId: "777" }],
+    });
+
+    const payload = getPayload();
+    expect(payload.content).toContain("<@777>");
+    expect(payload.allowed_mentions.users).toContain("777");
+  });
+
   it("escapes markdown in participant names and usernames", async () => {
     await sendQualifyingPointsUpdateNotification({
       webhookUrl: WEBHOOK_URL,

app/services/__tests__/qualifying-points-discord.server.test.ts

@@ -291,4 +291,128 @@ describe("notifyQualifyingPointsUpdate", () => {
 
     expect(sendQualifyingPointsUpdateNotification).not.toHaveBeenCalled();
   });
+
+  // A knocked-out drafted player (e.g. a 2nd-round loser) earns no QP and so has
+  // no event_results row — surfaced only via the eliminatedParticipantIds arg.
+  const MENSIK_ID = "p-2";
+  function makeDbWithEliminated() {
+    return makeDb({
+      draftPicks: {
+        findMany: vi.fn().mockResolvedValue([
+          {
+            participantId: PARTICIPANT_ID,
+            seasonId: SEASON_ID,
+            team: { id: "t-1", name: "Alpha FC", ownerId: OWNER_ID },
+          },
+          {
+            participantId: MENSIK_ID,
+            seasonId: SEASON_ID,
+            team: { id: "t-1", name: "Alpha FC", ownerId: OWNER_ID },
+          },
+        ]),
+      },
+      seasonParticipants: {
+        findMany: vi.fn().mockResolvedValue([
+          { id: PARTICIPANT_ID, name: "Carlos Alcaraz" },
+          { id: MENSIK_ID, name: "Jakob Mensik" },
+        ]),
+      },
+    });
+  }
+
+  it("announces a knocked-out drafted player who earned no QP", async () => {
+    const db = makeDbWithEliminated();
+
+    await notifyQualifyingPointsUpdate(
+      SPORTS_SEASON_ID,
+      SCORING_EVENT_ID,
+      db as never,
+      new Set([PARTICIPANT_ID]),
+      new Set([MENSIK_ID])
+    );
+
+    expect(sendQualifyingPointsUpdateNotification).toHaveBeenCalledWith(
+      expect.objectContaining({
+        eliminated: [
+          expect.objectContaining({ participantName: "Jakob Mensik", ownerUsername: "chris" }),
+        ],
+      })
+    );
+  });
+
+  it("does not announce a knocked-out player who is not drafted in the league", async () => {
+    const db = makeDb();
+
+    await notifyQualifyingPointsUpdate(
+      SPORTS_SEASON_ID,
+      SCORING_EVENT_ID,
+      db as never,
+      new Set([PARTICIPANT_ID]),
+      new Set(["p-undrafted"])
+    );
+
+    const call = vi.mocked(sendQualifyingPointsUpdateNotification).mock.calls[0][0];
+    expect(call.eliminated).toEqual([]);
+  });
+
+  it("does not double-list a QP earner that is also passed as eliminated", async () => {
+    const db = makeDb();
+
+    // PARTICIPANT_ID earned 10 QP this sync AND is passed as eliminated
+    // (e.g. a Round-of-16 loss). It should stay in entries, not the eliminated list.
+    await notifyQualifyingPointsUpdate(
+      SPORTS_SEASON_ID,
+      SCORING_EVENT_ID,
+      db as never,
+      new Set([PARTICIPANT_ID]),
+      new Set([PARTICIPANT_ID])
+    );
+
+    const call = vi.mocked(sendQualifyingPointsUpdateNotification).mock.calls[0][0];
+    expect(call.entries).toHaveLength(1);
+    expect(call.eliminated).toEqual([]);
+  });
+
+  it("short-circuits before the QP-total/season lookups when nothing drafted is involved", async () => {
+    // A knockout occurred but the player isn't drafted in any league. The notifier
+    // is invoked (the sync fires it on any elimination) but must not run the rest
+    // of its query battery just to send nothing.
+    const db = makeDb({
+      draftPicks: { findMany: vi.fn().mockResolvedValue([]) },
+    });
+
+    await notifyQualifyingPointsUpdate(
+      SPORTS_SEASON_ID,
+      SCORING_EVENT_ID,
+      db as never,
+      new Set(["p-nonexistent"]),
+      new Set(["p-undrafted"])
+    );
+
+    expect(db.query.draftPicks.findMany).toHaveBeenCalledOnce();
+    expect(db.query.seasonParticipantQualifyingTotals.findMany).not.toHaveBeenCalled();
+    expect(db.query.seasons.findMany).not.toHaveBeenCalled();
+    expect(db.query.seasonParticipants.findMany).not.toHaveBeenCalled();
+    expect(sendQualifyingPointsUpdateNotification).not.toHaveBeenCalled();
+  });
+
+  it("fires when a drafted player is knocked out even though no QP changed", async () => {
+    const db = makeDbWithEliminated();
+
+    // participantIdFilter matches nobody → no QP entries, but a knockout exists.
+    await notifyQualifyingPointsUpdate(
+      SPORTS_SEASON_ID,
+      SCORING_EVENT_ID,
+      db as never,
+      new Set(["p-nonexistent"]),
+      new Set([MENSIK_ID])
+    );
+
+    expect(sendQualifyingPointsUpdateNotification).toHaveBeenCalledOnce();
+    const call = vi.mocked(sendQualifyingPointsUpdateNotification).mock.calls[0][0];
+    expect(call.entries).toEqual([]);
+    expect(call.eliminated).toEqual([
+      expect.objectContaining({ participantName: "Jakob Mensik" }),
+    ]);
+  });
 });

app/services/discord.ts

@@ -255,20 +255,29 @@ export interface QPEventEntry {
   ownerDiscordUserId?: string;
 }
 
+/** A drafted player knocked out this sync in a non-scoring round (0 QP). */
+export interface QPEliminatedEntry {
+  participantName: string;
+  ownerUsername?: string;
+  ownerDiscordUserId?: string;
+}
+
 export async function sendQualifyingPointsUpdateNotification({
   webhookUrl,
   seasonName,
   sportName,
   eventName,
   entries,
+  eliminated = [],
 }: {
   webhookUrl: string;
   seasonName: string;
   sportName?: string;
   eventName?: string;
   entries: QPEventEntry[];
+  eliminated?: QPEliminatedEntry[];
 }): Promise<void> {
-  if (entries.length === 0) return;
+  if (entries.length === 0 && eliminated.length === 0) return;
 
   const sections: string[] = [];
 
@@ -312,6 +321,23 @@ export async function sendQualifyingPointsUpdateNotification({
     }
   }
 
+  // Knocked-out section — drafted players eliminated this sync in a non-scoring
+  // round. They earn no QP, so they'd otherwise never be surfaced to their manager.
+  if (eliminated.length > 0) {
+    sections.push("\n**Knocked Out**");
+    for (const e of eliminated) {
+      const managerLabel = e.ownerDiscordUserId
+        ? `<@${e.ownerDiscordUserId}>`
+        : e.ownerUsername
+          ? escapeMarkdown(e.ownerUsername)
+          : undefined;
+      const label = managerLabel
+        ? `${escapeMarkdown(e.participantName)} (${managerLabel})`
+        : escapeMarkdown(e.participantName);
+      sections.push(`• ${label}`);
+    }
+  }
+
   const sorted = [...entries].toSorted((a, b) => b.qpTotal - a.qpTotal);
   // Compute ranks with tie detection
   const ranks: number[] = [];
@@ -324,6 +350,9 @@ export async function sendQualifyingPointsUpdateNotification({
   }
   const isTied = buildTiedRankChecker(ranks);
 
+  // The standings block reflects QP earners; skip it entirely when this sync only
+  // reported knockouts (no QP change) so we don't emit an empty header.
+  if (sorted.length > 0) {
     sections.push("\n**QP Standings**");
     for (let i = 0; i < sorted.length; i++) {
       const e = sorted[i];
@@ -337,6 +366,7 @@ export async function sendQualifyingPointsUpdateNotification({
       const managerLabel = ownerLabel ? ` (${ownerLabel})` : "";
       sections.push(`${rankPrefix}\\. ${escapeMarkdown(e.participantName)}${managerLabel} — ${Math.round(e.qpTotal)} QP`);
     }
+  }
 
   const MAX_DESCRIPTION = 4096;
   let description = sections.join("\n");
@@ -345,7 +375,7 @@ export async function sendQualifyingPointsUpdateNotification({
   }
 
   const pingUserIds = new Set<string>();
-  for (const e of [...awardedEntries, ...zeroEntries]) {
+  for (const e of [...awardedEntries, ...zeroEntries, ...eliminated]) {
     if (e.ownerDiscordUserId) pingUserIds.add(e.ownerDiscordUserId);
   }
   const pingIds = [...pingUserIds];

app/services/match-sync/index.ts

@@ -593,7 +593,10 @@ export async function syncTennisDraw(eventId: string): Promise<DrawSyncResult> {
     });
   }
 
-  const { written, completed } = await populateBracketFromDraw(eventId, resolvedMatches);
+  const { written, completed, newlyDecidedLoserIds } = await populateBracketFromDraw(
+    eventId,
+    resolvedMatches,
+  );
 
   // ---- Score (qualifying points) + fan out to siblings ----------------------
   // Re-derive QP from the bracket and learn which participants' QP changed so the
@@ -618,9 +621,20 @@ export async function syncTennisDraw(eventId: string): Promise<DrawSyncResult> {
   // processQualifyingBracketEvent, which does NOT notify — so do it here. Run
   // outside the rescore transaction so the webhook HTTP call neither holds the
   // transaction open nor rolls back the score if Discord fails.
-  if (changedParticipantIds.size > 0) {
+  //
+  // Also announce players knocked out this sync in a non-scoring round (rounds
+  // 1–3 of a Grand Slam), who earn no QP and so never appear via changedParticipantIds.
+  // Fire even when no QP changed, so an early-round elimination is still surfaced.
+  const newlyEliminatedIds = new Set(newlyDecidedLoserIds);
+  if (changedParticipantIds.size > 0 || newlyEliminatedIds.size > 0) {
     try {
-      await notifyQualifyingPointsUpdate(sportsSeasonId, eventId, db, changedParticipantIds);
+      await notifyQualifyingPointsUpdate(
+        sportsSeasonId,
+        eventId,
+        db,
+        changedParticipantIds,
+        newlyEliminatedIds,
+      );
     } catch (error) {
       logger.error(`[syncTennisDraw] QP Discord notification failed for event ${eventId}:`, error);
     }

app/services/qualifying-points-discord.server.ts

@@ -6,13 +6,21 @@ import { getUserDisplayName } from "~/models/user";
 import {
   sendQualifyingPointsUpdateNotification,
   type QPEventEntry,
+  type QPEliminatedEntry,
 } from "~/services/discord";
 
 export async function notifyQualifyingPointsUpdate(
   sportsSeasonId: string,
   scoringEventId: string,
   db: ReturnType<typeof database>,
-  participantIdFilter?: Set<string>
+  participantIdFilter?: Set<string>,
+  /**
+   * Participants knocked out this sync in a non-scoring round: they earn no QP,
+   * so they never appear via qualifyingPointsAwarded, but a manager who drafted
+   * them should still be told their player is out. Surfaced in a "Knocked Out"
+   * section, deduped against QP earners.
+   */
+  eliminatedParticipantIds?: Set<string>
 ): Promise<void> {
   const event = await db.query.scoringEvents.findFirst({
     where: eq(schema.scoringEvents.id, scoringEventId),
@@ -43,7 +51,41 @@ export async function notifyQualifyingPointsUpdate(
       .map((r) => [r.seasonParticipantId, parseFloat(r.qualifyingPointsAwarded as string)])
   );
 
-  if (qpEarnedById.size === 0) return;
+  // Knocked-out players with no QP change. Exclude anyone who also earned QP this
+  // sync (e.g. a Round-of-16 loser) so they aren't listed twice.
+  const eliminatedIds = new Set(
+    [...(eliminatedParticipantIds ?? [])].filter((id) => !qpEarnedById.has(id))
+  );
+
+  if (qpEarnedById.size === 0 && eliminatedIds.size === 0) return;
+
+  const seasonIds = seasonSports.map((s) => s.seasonId);
+
+  // Batch-fetch all draft picks for all leagues at once, grouped by season.
+  // Fetched before the remaining lookups so we can short-circuit when nothing
+  // that happened this sync was actually drafted: during a Grand Slam's early
+  // rounds most eliminated players are undrafted, and the notifier now fires on
+  // every sync that has any elimination — no point running the rest of the
+  // queries just to send nothing.
+  const allPicks = await db.query.draftPicks.findMany({
+    where: inArray(schema.draftPicks.seasonId, seasonIds),
+    with: { team: true },
+  });
+  const picksBySeasonId = new Map<string, (typeof allPicks)[number][]>();
+  const draftedParticipantIds = new Set<string>();
+  for (const pick of allPicks) {
+    draftedParticipantIds.add(pick.participantId);
+    const bucket = picksBySeasonId.get(pick.seasonId);
+    if (bucket) {
+      bucket.push(pick);
+    } else {
+      picksBySeasonId.set(pick.seasonId, [pick]);
+    }
+  }
+
+  const hasDraftedQP = [...qpEarnedById.keys()].some((id) => draftedParticipantIds.has(id));
+  const hasDraftedEliminated = [...eliminatedIds].some((id) => draftedParticipantIds.has(id));
+  if (!hasDraftedQP && !hasDraftedEliminated) return;
 
   // Current running QP totals for the season
   const qpTotals = await db.query.seasonParticipantQualifyingTotals.findMany({
@@ -53,8 +95,6 @@ export async function notifyQualifyingPointsUpdate(
     qpTotals.map((t) => [t.participantId, parseFloat(t.totalQualifyingPoints)])
   );
 
-  const seasonIds = seasonSports.map((s) => s.seasonId);
-
   // Batch-fetch all season + league metadata in one query
   const seasons = await db.query.seasons.findMany({
     where: inArray(schema.seasons.id, seasonIds),
@@ -62,23 +102,8 @@ export async function notifyQualifyingPointsUpdate(
   });
   const seasonMap = new Map(seasons.map((s) => [s.id, s]));
 
-  // Batch-fetch all draft picks for all leagues at once, grouped by season
-  const allPicks = await db.query.draftPicks.findMany({
-    where: inArray(schema.draftPicks.seasonId, seasonIds),
-    with: { team: true },
-  });
-  const picksBySeasonId = new Map<string, (typeof allPicks)[number][]>();
-  for (const pick of allPicks) {
-    const bucket = picksBySeasonId.get(pick.seasonId);
-    if (bucket) {
-      bucket.push(pick);
-    } else {
-      picksBySeasonId.set(pick.seasonId, [pick]);
-    }
-  }
-
   // Batch-fetch participant display names once (same participants across all leagues)
-  const allParticipantIds = [...qpEarnedById.keys()];
+  const allParticipantIds = [...new Set([...qpEarnedById.keys(), ...eliminatedIds])];
   const participants = await db.query.seasonParticipants.findMany({
     where: inArray(schema.seasonParticipants.id, allParticipantIds),
   });
@@ -125,7 +150,11 @@ export async function notifyQualifyingPointsUpdate(
     const relevantParticipantIds = [...qpEarnedById.keys()].filter((id) =>
       teamByParticipantId.has(id)
     );
-    if (relevantParticipantIds.length === 0) continue;
+    // Knocked-out players drafted in this league (0 QP, non-scoring-round exits)
+    const eliminatedForLeague = [...eliminatedIds].filter((id) =>
+      teamByParticipantId.has(id)
+    );
+    if (relevantParticipantIds.length === 0 && eliminatedForLeague.length === 0) continue;
 
     const entries: QPEventEntry[] = relevantParticipantIds.map((participantId) => {
       const ownerId = teamByParticipantId.get(participantId)?.ownerId ?? null;
@@ -138,12 +167,22 @@ export async function notifyQualifyingPointsUpdate(
       };
     });
 
+    const eliminated: QPEliminatedEntry[] = eliminatedForLeague.map((participantId) => {
+      const ownerId = teamByParticipantId.get(participantId)?.ownerId ?? null;
+      return {
+        participantName: participantNameById.get(participantId) ?? participantId,
+        ownerUsername: ownerId ? (usernameByUserId.get(ownerId) ?? undefined) : undefined,
+        ownerDiscordUserId: ownerId ? discordIdByUserId.get(ownerId) : undefined,
+      };
+    });
+
     await sendQualifyingPointsUpdateNotification({
       webhookUrl,
       seasonName,
       sportName,
       eventName,
       entries,
+      eliminated,
     });
   }
 }