From 64e5cb23ef2d6c24ba149012f7523c997d665857 Mon Sep 17 00:00:00 2001 From: chrisp Date: Tue, 30 Jun 2026 23:48:11 +0000 Subject: [PATCH] claude/discord-ping-notification-filter-6m1ymt (#120) Co-authored-by: Claude Reviewed-on: https://forge.brackt.com/chrisp/brackt/pulls/120 --- app/models/scoring-calculator.ts | 22 ++++++++++------------ app/services/__tests__/discord.test.ts | 12 ++++++------ 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/app/models/scoring-calculator.ts b/app/models/scoring-calculator.ts index f5cb685..6e56f20 100644 --- a/app/models/scoring-calculator.ts +++ b/app/models/scoring-calculator.ts @@ -1827,17 +1827,15 @@ export async function recalculateAffectedLeagues( ); // Build scored matches for the notification. - // A match is included only when it is "notable": winner earned points this round OR - // the loser is notifiable (eliminated / scored). A drafted winner that merely advanced - // without earning points is NOT enough on its own. - // On any included match, the winning team's owner is shown in the embed (winnerUsername), - // but Discord-pinged only when they actually scored (winnerDiscordUserId gated on - // winnerScoreChanged) — advancing-only wins don't warrant a ping. - // Losers appear if their score changed or they were definitively eliminated (finalPosition - // set, non-partial); losers who advance to another match (loserAdvances=true, e.g. NBA - // 7v8 → PIR2) are correctly suppressed. - // Only entries with at least one displayable username are kept, so scoredMatches always - // reflects exactly what will be shown in the embed. + // A match is worth announcing when: + // • the winner is owned by a manager AND earned points this round, OR + // • the loser is owned by a manager (eliminated or scored). + // A winning team that merely advanced through a non-scoring round (e.g. R32 → R16 + // in the World Cup) does not qualify on its own — their owner hasn't earned anything yet. + // When a match qualifies because the loser is owned, the winner's manager tag is still + // shown for context (who beat them), but the winner is not Discord-pinged. + // Losers who advance to another match (loserAdvances=true, e.g. NBA 7v8 → PIR2) have + // showLoser=false and are correctly suppressed. let scoredMatches: ScoredMatch[] | undefined; if (allCompletedMatches.length > 0) { const relevant = allCompletedMatches.filter( @@ -1865,7 +1863,7 @@ export async function recalculateAffectedLeagues( const showLoser = isLoserNotifiable(m.loserId, loserTeamId, changedTeamIds, finalizedLoserIds); return { m, winnerScoreChanged, showLoser, winnerOwnerId, loserOwnerId }; }) - .filter((x) => x.winnerScoreChanged || x.showLoser) + .filter((x) => (x.winnerScoreChanged && !!x.winnerOwnerId) || (x.showLoser && !!x.loserOwnerId)) .map((x) => ({ winnerName: x.m.winnerName ?? "", loserName: x.m.loserName ?? "", diff --git a/app/services/__tests__/discord.test.ts b/app/services/__tests__/discord.test.ts index 45d97a3..f88a9d9 100644 --- a/app/services/__tests__/discord.test.ts +++ b/app/services/__tests__/discord.test.ts @@ -186,10 +186,10 @@ describe("sendStandingsUpdateNotification", () => { expect(desc).toContain("• **Sporting (christhrowsrocks)** def. Bodø/Glimt (apatel)"); }); - it("shows winner's owner even when winner only advanced (loser was eliminated)", async () => { - // Mirrors the Brazil/Japan scenario: Brazil advanced without earning points, - // Japan was eliminated. winnerUsername is shown in the embed; winnerDiscordUserId - // is left unset (no ping) because the winner did not score this round. + it("shows winner's manager for context when the match fires due to an owned loser", async () => { + // Brazil beats Japan (R32, non-scoring). Japan's manager is the reason for the + // notification; Brazil's manager is shown for context even though they didn't score. + // scoring-calculator.ts controls whether to include the match; discord.ts just renders. await sendStandingsUpdateNotification({ webhookUrl: WEBHOOK_URL, seasonName: "Rumble League 2026", @@ -199,14 +199,14 @@ describe("sendStandingsUpdateNotification", () => { { winnerName: "Brazil", loserName: "Japan", - winnerUsername: "someManager", + winnerUsername: "aliceManager", loserUsername: "ikyn", }, ], }); const desc = getDescription(); - expect(desc).toContain("• **Brazil (someManager)** def. Japan (ikyn)"); + expect(desc).toContain("• **Brazil (aliceManager)** def. Japan (ikyn)"); }); it("omits scored matches where neither side has a username", async () => {