From 613106cb9e742eb0cf177bddc271153afc4e19bf Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 30 Jun 2026 23:40:41 +0000 Subject: [PATCH] Fix notification filter: only include matches where an owner earned points or lost MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous filter let matches through if showLoser was true (loser got a finalized result) regardless of whether the loser or winner was actually owned by a manager. This caused spurious notifications when a managed team advanced through a non-scoring round (e.g. R32 → R16 in World Cup) while beating an unowned opponent who happened to get a 0-pt finalized result. New rule: a match is worth announcing only when the winner is owned AND earned points, OR the loser is owned. When a match fires because the loser is owned, the winner's manager tag is still shown for context but they are not Discord-pinged (winnerDiscordUserId remains gated on winnerScoreChanged). Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_01UXKpR3MZJn5Zof1AKeETFM --- app/models/scoring-calculator.ts | 23 +++++++++++------------ app/services/__tests__/discord.test.ts | 13 ++++++------- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/app/models/scoring-calculator.ts b/app/models/scoring-calculator.ts index 3b4cebe..6e56f20 100644 --- a/app/models/scoring-calculator.ts +++ b/app/models/scoring-calculator.ts @@ -1827,16 +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. - // winnerUsername (and winnerDiscordUserId) are only set when the winner actually scored — - // advancing through a non-scoring round does not warrant mentioning or pinging the owner. - // 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( @@ -1864,11 +1863,11 @@ 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 ?? "", - winnerUsername: x.winnerScoreChanged && x.winnerOwnerId ? usernameByUserId.get(x.winnerOwnerId) : undefined, + winnerUsername: x.winnerOwnerId ? usernameByUserId.get(x.winnerOwnerId) : undefined, loserUsername: x.showLoser && x.loserOwnerId ? usernameByUserId.get(x.loserOwnerId) : undefined, winnerDiscordUserId: x.winnerScoreChanged && x.winnerOwnerId ? discordIdByUserId.get(x.winnerOwnerId) : undefined, loserDiscordUserId: x.showLoser && x.loserOwnerId ? discordIdByUserId.get(x.loserOwnerId) : undefined, diff --git a/app/services/__tests__/discord.test.ts b/app/services/__tests__/discord.test.ts index af5585e..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("omits winner's manager when they only advanced without earning points", async () => { - // Brazil advances through a non-scoring round (R32 → R16 in World Cup). - // scoring-calculator.ts does not set winnerUsername for advancing-only winners, - // so only Japan's manager appears in the notification. + 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,15 +199,14 @@ describe("sendStandingsUpdateNotification", () => { { winnerName: "Brazil", loserName: "Japan", - // winnerUsername intentionally absent — Brazil advanced without scoring + winnerUsername: "aliceManager", loserUsername: "ikyn", }, ], }); const desc = getDescription(); - expect(desc).toContain("• **Brazil** def. Japan (ikyn)"); - expect(desc).not.toContain("Brazil ("); + expect(desc).toContain("• **Brazil (aliceManager)** def. Japan (ikyn)"); }); it("omits scored matches where neither side has a username", async () => {