Fix code review findings for QP Discord notifications
Some checks failed
🚀 Deploy / 🧪 Test (pull_request) Successful in 2m57s
🚀 Deploy / ʦ🔍 Typecheck & Lint (pull_request) Failing after 47s
🚀 Deploy / 🐳 Build (pull_request) Has been skipped
🚀 Deploy / 🚀 Deploy (pull_request) Has been skipped

- Round qpEarned and qpTotal with Math.round() so fractional tie-split
  values display as integers ("+2 QP" not "+1.5 QP")
- Swap embed colors: standings → gold (0xffd700), QP → blurple (0x5865f2)
- Add logger.error in cs2-major-stage.ts to match scoring-calculator.ts
  error handling pattern (replaces silent .catch(() => {}))
- Fix cumulative CS2 re-announcement: snapshot existing QP before writing
  and only notify participants whose QP is new or changed in this call;
  add participantIdFilter parameter to notifyQualifyingPointsUpdate
- Batch all pre-loop queries in notifyQualifyingPointsUpdate (seasons,
  draftPicks, participantNames, users, Discord IDs) to eliminate N+1
  queries across league iterations
- Add unit tests for notifyQualifyingPointsUpdate covering early-return
  paths, per-league filtering, Discord ID resolution, and participantIdFilter

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NnJP1NTYXHxL2kbE4fZaD6
This commit is contained in:
Claude 2026-07-01 16:28:08 +00:00
parent df15386d2d
commit aea71a5ad6
No known key found for this signature in database
5 changed files with 374 additions and 49 deletions

View file

@ -18,6 +18,7 @@ import { eq, and, sql } from "drizzle-orm";
import { getQPConfig, calculateSplitQualifyingPoints, writeEventResultsQP, recalculateParticipantQP } from "~/models/qualifying-points"; import { getQPConfig, calculateSplitQualifyingPoints, writeEventResultsQP, recalculateParticipantQP } from "~/models/qualifying-points";
import { deleteEventResults } from "~/models/event-result"; import { deleteEventResults } from "~/models/event-result";
import { notifyQualifyingPointsUpdate } from "~/services/qualifying-points-discord.server"; import { notifyQualifyingPointsUpdate } from "~/services/qualifying-points-discord.server";
import { logger } from "~/lib/logger";
export interface Cs2StageResult { export interface Cs2StageResult {
id: string; id: string;
@ -439,7 +440,30 @@ export async function assignCs2EliminationQP(
} }
} }
// Snapshot existing QP before writing so the notification only covers new/changed participants,
// preventing earlier stage exits from being re-announced on each subsequent stage call.
const existingRows = await db.query.eventResults.findMany({
where: eq(eventResults.scoringEventId, scoringEventId),
});
const existingQPBySPId = new Map<string, number>(
existingRows
.filter((r) => r.qualifyingPointsAwarded !== null)
.map((r) => [r.seasonParticipantId, parseFloat(r.qualifyingPointsAwarded!)])
);
await writeEventResultsQP(scoringEventId, sportsSeasonId, resultsByParticipant, db); await writeEventResultsQP(scoringEventId, sportsSeasonId, resultsByParticipant, db);
await notifyQualifyingPointsUpdate(sportsSeasonId, scoringEventId, db).catch(() => {}); const changedIds = new Set<string>(
[...resultsByParticipant.entries()]
.filter(([id, { qp }]) => existingQPBySPId.get(id) !== qp)
.map(([id]) => id)
);
if (changedIds.size > 0) {
try {
await notifyQualifyingPointsUpdate(sportsSeasonId, scoringEventId, db, changedIds);
} catch (error) {
logger.error(`[CS2MajorStage] QP Discord notification failed for event ${scoringEventId}:`, error);
}
}
} }

View file

@ -695,7 +695,7 @@ describe("sendQualifyingPointsUpdateNotification", () => {
const payload = getPayload(); const payload = getPayload();
expect(payload.embeds[0].title).toBe("🏅 Qualifying Points Update — Slam League 2025"); expect(payload.embeds[0].title).toBe("🏅 Qualifying Points Update — Slam League 2025");
expect(payload.embeds[0].color).toBe(0xffd700); expect(payload.embeds[0].color).toBe(0x5865f2);
expect(payload.embeds[0].footer).toEqual({ text: "brackt.com" }); expect(payload.embeds[0].footer).toEqual({ text: "brackt.com" });
}); });

View file

@ -0,0 +1,294 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
vi.mock("~/database/context", () => ({
database: vi.fn(),
}));
vi.mock("~/services/discord", () => ({
sendQualifyingPointsUpdateNotification: vi.fn().mockResolvedValue(undefined),
}));
vi.mock("~/models/account", () => ({
findDiscordIdsByUserIds: vi.fn().mockResolvedValue(new Map()),
}));
vi.mock("~/models/user", () => ({
getUserDisplayName: vi.fn((u: { username?: string }) => u.username ?? null),
}));
import { notifyQualifyingPointsUpdate } from "../qualifying-points-discord.server";
import { sendQualifyingPointsUpdateNotification } from "~/services/discord";
import { findDiscordIdsByUserIds } from "~/models/account";
// ---------------------------------------------------------------------------
// Fixtures
// ---------------------------------------------------------------------------
const SPORTS_SEASON_ID = "ss-1";
const SCORING_EVENT_ID = "ev-1";
const SEASON_ID = "season-1";
const PARTICIPANT_ID = "p-1";
const OWNER_ID = "u-1";
const WEBHOOK_URL = "https://discord.com/api/webhooks/123/abc";
function makeEvent(overrides = {}) {
return {
id: SCORING_EVENT_ID,
name: "Roland Garros 2025",
sportsSeason: { sport: { name: "Tennis" } },
...overrides,
};
}
function makeDb(overrides: Record<string, unknown> = {}) {
return {
query: {
scoringEvents: {
findFirst: vi.fn().mockResolvedValue(makeEvent()),
},
seasonSports: {
findMany: vi.fn().mockResolvedValue([{ seasonId: SEASON_ID }]),
},
eventResults: {
findMany: vi.fn().mockResolvedValue([
{
seasonParticipantId: PARTICIPANT_ID,
qualifyingPointsAwarded: "10",
scoringEventId: SCORING_EVENT_ID,
},
]),
},
seasonParticipantQualifyingTotals: {
findMany: vi.fn().mockResolvedValue([
{ participantId: PARTICIPANT_ID, totalQualifyingPoints: "45", sportsSeasonId: SPORTS_SEASON_ID },
]),
},
seasons: {
findMany: vi.fn().mockResolvedValue([
{
id: SEASON_ID,
year: 2025,
league: { name: "Slam League", discordWebhookUrl: WEBHOOK_URL },
},
]),
},
draftPicks: {
findMany: vi.fn().mockResolvedValue([
{
participantId: PARTICIPANT_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" }]),
},
users: {
findMany: vi.fn().mockResolvedValue([
{ id: OWNER_ID, username: "chris", discordPingEnabled: false },
]),
},
...overrides,
},
};
}
// ---------------------------------------------------------------------------
// Tests
// ---------------------------------------------------------------------------
beforeEach(() => {
vi.clearAllMocks();
vi.mocked(findDiscordIdsByUserIds).mockResolvedValue(new Map());
});
describe("notifyQualifyingPointsUpdate", () => {
it("returns early when the scoring event is not found", async () => {
const db = makeDb({
scoringEvents: { findFirst: vi.fn().mockResolvedValue(null) },
});
await notifyQualifyingPointsUpdate(SPORTS_SEASON_ID, SCORING_EVENT_ID, db as never);
expect(sendQualifyingPointsUpdateNotification).not.toHaveBeenCalled();
});
it("returns early when no season sports exist for the sports season", async () => {
const db = makeDb({
seasonSports: { findMany: vi.fn().mockResolvedValue([]) },
});
await notifyQualifyingPointsUpdate(SPORTS_SEASON_ID, SCORING_EVENT_ID, db as never);
expect(sendQualifyingPointsUpdateNotification).not.toHaveBeenCalled();
});
it("returns early when no qualifying points were awarded in the event", async () => {
const db = makeDb({
eventResults: {
findMany: vi.fn().mockResolvedValue([
{ seasonParticipantId: PARTICIPANT_ID, qualifyingPointsAwarded: null },
]),
},
});
await notifyQualifyingPointsUpdate(SPORTS_SEASON_ID, SCORING_EVENT_ID, db as never);
expect(sendQualifyingPointsUpdateNotification).not.toHaveBeenCalled();
});
it("skips leagues without a Discord webhook URL", async () => {
const db = makeDb({
seasons: {
findMany: vi.fn().mockResolvedValue([
{ id: SEASON_ID, year: 2025, league: { name: "Slam League", discordWebhookUrl: null } },
]),
},
});
await notifyQualifyingPointsUpdate(SPORTS_SEASON_ID, SCORING_EVENT_ID, db as never);
expect(sendQualifyingPointsUpdateNotification).not.toHaveBeenCalled();
});
it("skips leagues where no QP participants were drafted", async () => {
const db = makeDb({
draftPicks: {
findMany: vi.fn().mockResolvedValue([]),
},
});
await notifyQualifyingPointsUpdate(SPORTS_SEASON_ID, SCORING_EVENT_ID, db as never);
expect(sendQualifyingPointsUpdateNotification).not.toHaveBeenCalled();
});
it("calls sendQualifyingPointsUpdateNotification with correct participant data", async () => {
const db = makeDb();
await notifyQualifyingPointsUpdate(SPORTS_SEASON_ID, SCORING_EVENT_ID, db as never);
expect(sendQualifyingPointsUpdateNotification).toHaveBeenCalledOnce();
expect(sendQualifyingPointsUpdateNotification).toHaveBeenCalledWith(
expect.objectContaining({
webhookUrl: WEBHOOK_URL,
seasonName: "Slam League 2025",
eventName: "Roland Garros 2025",
sportName: "Tennis",
entries: [
expect.objectContaining({
participantName: "Carlos Alcaraz",
qpEarned: 10,
qpTotal: 45,
ownerUsername: "chris",
ownerDiscordUserId: undefined,
}),
],
})
);
});
it("sends once per league, not once per participant", async () => {
const db = makeDb({
seasonSports: {
findMany: vi.fn().mockResolvedValue([{ seasonId: SEASON_ID }, { seasonId: "season-2" }]),
},
seasons: {
findMany: vi.fn().mockResolvedValue([
{ id: SEASON_ID, year: 2025, league: { name: "League A", discordWebhookUrl: WEBHOOK_URL } },
{ id: "season-2", year: 2025, league: { name: "League B", discordWebhookUrl: "https://discord.com/api/webhooks/456/def" } },
]),
},
draftPicks: {
findMany: vi.fn().mockResolvedValue([
{ participantId: PARTICIPANT_ID, seasonId: SEASON_ID, team: { name: "Alpha FC", ownerId: null } },
{ participantId: PARTICIPANT_ID, seasonId: "season-2", team: { name: "Beta FC", ownerId: null } },
]),
},
});
await notifyQualifyingPointsUpdate(SPORTS_SEASON_ID, SCORING_EVENT_ID, db as never);
expect(sendQualifyingPointsUpdateNotification).toHaveBeenCalledTimes(2);
});
it("resolves owner Discord ID and passes it when user has discordPingEnabled", async () => {
vi.mocked(findDiscordIdsByUserIds).mockResolvedValue(new Map([[OWNER_ID, "discord-999"]]));
const db = makeDb({
users: {
findMany: vi.fn().mockResolvedValue([
{ id: OWNER_ID, username: "chris", discordPingEnabled: true },
]),
},
});
await notifyQualifyingPointsUpdate(SPORTS_SEASON_ID, SCORING_EVENT_ID, db as never);
expect(sendQualifyingPointsUpdateNotification).toHaveBeenCalledWith(
expect.objectContaining({
entries: [expect.objectContaining({ ownerDiscordUserId: "discord-999" })],
})
);
});
it("omits ownerDiscordUserId when user has discordPingEnabled false", async () => {
const db = makeDb();
await notifyQualifyingPointsUpdate(SPORTS_SEASON_ID, SCORING_EVENT_ID, db as never);
expect(findDiscordIdsByUserIds).toHaveBeenCalledWith([]);
expect(sendQualifyingPointsUpdateNotification).toHaveBeenCalledWith(
expect.objectContaining({
entries: [expect.objectContaining({ ownerDiscordUserId: undefined })],
})
);
});
it("participantIdFilter limits notification to only specified participants", async () => {
const db = makeDb({
eventResults: {
findMany: vi.fn().mockResolvedValue([
{ seasonParticipantId: PARTICIPANT_ID, qualifyingPointsAwarded: "10" },
{ seasonParticipantId: "p-2", qualifyingPointsAwarded: "5" },
]),
},
draftPicks: {
findMany: vi.fn().mockResolvedValue([
{ participantId: PARTICIPANT_ID, seasonId: SEASON_ID, team: { name: "Alpha FC", ownerId: null } },
{ participantId: "p-2", seasonId: SEASON_ID, team: { name: "Beta FC", ownerId: null } },
]),
},
seasonParticipants: {
findMany: vi.fn().mockResolvedValue([
{ id: PARTICIPANT_ID, name: "Carlos Alcaraz" },
{ id: "p-2", name: "Rafael Nadal" },
]),
},
});
await notifyQualifyingPointsUpdate(
SPORTS_SEASON_ID,
SCORING_EVENT_ID,
db as never,
new Set([PARTICIPANT_ID])
);
const call = vi.mocked(sendQualifyingPointsUpdateNotification).mock.calls[0][0];
expect(call.entries).toHaveLength(1);
expect(call.entries[0].participantName).toBe("Carlos Alcaraz");
});
it("does not call sendQualifyingPointsUpdateNotification when all participants are filtered out", async () => {
const db = makeDb();
await notifyQualifyingPointsUpdate(
SPORTS_SEASON_ID,
SCORING_EVENT_ID,
db as never,
new Set(["p-nonexistent"])
);
expect(sendQualifyingPointsUpdateNotification).not.toHaveBeenCalled();
});
});

View file

@ -231,7 +231,7 @@ export async function sendStandingsUpdateNotification({
{ {
title: `📊 Standings Update — ${seasonName}`, title: `📊 Standings Update — ${seasonName}`,
description, description,
color: 0x5865f2, // Discord blurple color: 0xffd700, // Gold
footer: { text: "brackt.com" }, footer: { text: "brackt.com" },
}, },
], ],
@ -292,7 +292,7 @@ export async function sendQualifyingPointsUpdateNotification({
const label = managerLabel const label = managerLabel
? `${escapeMarkdown(e.participantName)} (${managerLabel})` ? `${escapeMarkdown(e.participantName)} (${managerLabel})`
: escapeMarkdown(e.participantName); : escapeMarkdown(e.participantName);
sections.push(`• **${label}** — +${e.qpEarned} QP`); sections.push(`• **${label}** — +${Math.round(e.qpEarned)} QP`);
} }
} }
@ -335,7 +335,7 @@ export async function sendQualifyingPointsUpdateNotification({
? escapeMarkdown(e.ownerUsername) ? escapeMarkdown(e.ownerUsername)
: undefined; : undefined;
const managerLabel = ownerLabel ? ` (${ownerLabel})` : ""; const managerLabel = ownerLabel ? ` (${ownerLabel})` : "";
sections.push(`${rankPrefix}\\. ${escapeMarkdown(e.participantName)}${managerLabel}${e.qpTotal} QP`); sections.push(`${rankPrefix}\\. ${escapeMarkdown(e.participantName)}${managerLabel}${Math.round(e.qpTotal)} QP`);
} }
const MAX_DESCRIPTION = 4096; const MAX_DESCRIPTION = 4096;
@ -355,7 +355,7 @@ export async function sendQualifyingPointsUpdateNotification({
{ {
title: `🏅 Qualifying Points Update — ${seasonName}`, title: `🏅 Qualifying Points Update — ${seasonName}`,
description, description,
color: 0xffd700, // Gold — distinguishable from standings blurple color: 0x5865f2, // Discord blurple
footer: { text: "brackt.com" }, footer: { text: "brackt.com" },
}, },
], ],

View file

@ -11,7 +11,8 @@ import {
export async function notifyQualifyingPointsUpdate( export async function notifyQualifyingPointsUpdate(
sportsSeasonId: string, sportsSeasonId: string,
scoringEventId: string, scoringEventId: string,
db: ReturnType<typeof database> db: ReturnType<typeof database>,
participantIdFilter?: Set<string>
): Promise<void> { ): Promise<void> {
const event = await db.query.scoringEvents.findFirst({ const event = await db.query.scoringEvents.findFirst({
where: eq(schema.scoringEvents.id, scoringEventId), where: eq(schema.scoringEvents.id, scoringEventId),
@ -31,13 +32,14 @@ export async function notifyQualifyingPointsUpdate(
}); });
if (seasonSports.length === 0) return; if (seasonSports.length === 0) return;
// QP earned in this specific event // QP earned in this specific event (optionally scoped to participants new/changed in this call)
const eventResultRows = await db.query.eventResults.findMany({ const eventResultRows = await db.query.eventResults.findMany({
where: eq(schema.eventResults.scoringEventId, scoringEventId), where: eq(schema.eventResults.scoringEventId, scoringEventId),
}); });
const qpEarnedById = new Map<string, number>( const qpEarnedById = new Map<string, number>(
eventResultRows eventResultRows
.filter((r) => r.qualifyingPointsAwarded !== null) .filter((r) => r.qualifyingPointsAwarded !== null)
.filter((r) => !participantIdFilter || participantIdFilter.has(r.seasonParticipantId))
.map((r) => [r.seasonParticipantId, parseFloat(r.qualifyingPointsAwarded!)]) .map((r) => [r.seasonParticipantId, parseFloat(r.qualifyingPointsAwarded!)])
); );
@ -51,23 +53,58 @@ export async function notifyQualifyingPointsUpdate(
qpTotals.map((t) => [t.participantId, parseFloat(t.totalQualifyingPoints)]) qpTotals.map((t) => [t.participantId, parseFloat(t.totalQualifyingPoints)])
); );
for (const { seasonId } of seasonSports) { const seasonIds = seasonSports.map((s) => s.seasonId);
const season = await db.query.seasons.findFirst({
where: eq(schema.seasons.id, seasonId), // Batch-fetch all season + league metadata in one query
const seasons = await db.query.seasons.findMany({
where: inArray(schema.seasons.id, seasonIds),
with: { league: true }, with: { league: true },
}); });
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) {
if (!picksBySeasonId.has(pick.seasonId)) picksBySeasonId.set(pick.seasonId, []);
picksBySeasonId.get(pick.seasonId)!.push(pick);
}
// Batch-fetch participant display names once (same participants across all leagues)
const allParticipantIds = [...qpEarnedById.keys()];
const participants = await db.query.seasonParticipants.findMany({
where: inArray(schema.seasonParticipants.id, allParticipantIds),
});
const participantNameById = new Map(participants.map((p) => [p.id, p.name]));
// Batch-fetch all team owners and their Discord IDs in two queries (not N per league)
const allOwnerIds = new Set<string>();
for (const pick of allPicks) {
if (pick.team.ownerId) allOwnerIds.add(pick.team.ownerId);
}
const allUsers =
allOwnerIds.size > 0
? await db.query.users.findMany({ where: inArray(schema.users.id, [...allOwnerIds]) })
: [];
const usernameByUserId = new Map(
allUsers
.map((u) => [u.id, getUserDisplayName(u)] as [string, string | null])
.filter((entry): entry is [string, string] => entry[1] !== null)
);
const optedInUserIds = allUsers.filter((u) => u.discordPingEnabled).map((u) => u.id);
const discordIdByUserId = await findDiscordIdsByUserIds(optedInUserIds);
for (const { seasonId } of seasonSports) {
const season = seasonMap.get(seasonId);
const webhookUrl = season?.league?.discordWebhookUrl; const webhookUrl = season?.league?.discordWebhookUrl;
if (!webhookUrl) continue; if (!webhookUrl) continue;
const seasonName = `${season!.league!.name} ${season!.year}`; const seasonName = `${season!.league!.name} ${season!.year}`;
// Map drafted participants → teams in this season const picks = picksBySeasonId.get(seasonId) ?? [];
const picks = await db.query.draftPicks.findMany({
where: eq(schema.draftPicks.seasonId, seasonId),
with: { team: true },
});
const teamByParticipantId = new Map< const teamByParticipantId = new Map<
string, string,
{ teamName: string; ownerId: string | null } { teamName: string; ownerId: string | null }
@ -85,36 +122,6 @@ export async function notifyQualifyingPointsUpdate(
); );
if (relevantParticipantIds.length === 0) continue; if (relevantParticipantIds.length === 0) continue;
// Participant display names
const participants = await db.query.seasonParticipants.findMany({
where: inArray(schema.seasonParticipants.id, relevantParticipantIds),
});
const participantNameById = new Map(participants.map((p) => [p.id, p.name]));
// Owner usernames + Discord IDs
const ownerIds = [
...new Set(
relevantParticipantIds
.map((id) => teamByParticipantId.get(id)?.ownerId)
.filter((id): id is string => id !== null && id !== undefined)
),
];
const users = ownerIds.length
? await db.query.users.findMany({
where: inArray(schema.users.id, ownerIds),
})
: [];
const usernameByUserId = new Map(
users
.map((u) => [u.id, getUserDisplayName(u)] as [string, string | null])
.filter((entry): entry is [string, string] => entry[1] !== null)
);
const optedInUserIds = users.filter((u) => u.discordPingEnabled).map((u) => u.id);
const discordIdByUserId = await findDiscordIdsByUserIds(optedInUserIds);
const entries: QPEventEntry[] = relevantParticipantIds.map((participantId) => { const entries: QPEventEntry[] = relevantParticipantIds.map((participantId) => {
const teamInfo = teamByParticipantId.get(participantId)!; const teamInfo = teamByParticipantId.get(participantId)!;
const ownerId = teamInfo.ownerId; const ownerId = teamInfo.ownerId;