From 4a9a70ca7c37a23357ef4e3a99288692f8010f46 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 29 Jun 2026 22:19:30 +0000 Subject: [PATCH 1/2] Show winning team's owner in scoring notifications when loser is eliminated When a match is displayed because the loser was eliminated (0-pt), the winner's manager was previously omitted if the winner only advanced without earning points. Now the winner's owner is always credited on any match that passes the notability gate, while the gate itself (winnerScoreChanged || loserNotifiable) still prevents purely-advancing matches from appearing. Adds isMatchNotable() pure helper alongside isLoserNotifiable() to make the display rule testable in isolation. Co-Authored-By: Claude --- .../__tests__/process-match-result.test.ts | 20 ++++++- app/models/scoring-calculator.ts | 52 +++++++++++++------ app/services/__tests__/discord.test.ts | 23 ++++++++ 3 files changed, 79 insertions(+), 16 deletions(-) diff --git a/app/models/__tests__/process-match-result.test.ts b/app/models/__tests__/process-match-result.test.ts index fdb5d6d..4484a18 100644 --- a/app/models/__tests__/process-match-result.test.ts +++ b/app/models/__tests__/process-match-result.test.ts @@ -59,7 +59,7 @@ function makeDb(existingResult?: { id: string; isPartialScore: boolean }) { }; } -import { processMatchResult, isLoserNotifiable, processPlayoffEvent } from "../scoring-calculator"; +import { processMatchResult, isLoserNotifiable, isMatchNotable, processPlayoffEvent } from "../scoring-calculator"; import { doesLoserAdvance } from "../playoff-match"; import { updateProbabilitiesAfterResult } from "~/services/probability-updater"; @@ -399,6 +399,24 @@ describe("isLoserNotifiable", () => { }); }); +describe("isMatchNotable", () => { + it("returns true when winner scored (loser not notifiable)", () => { + expect(isMatchNotable(true, false)).toBe(true); + }); + + it("returns true when loser is notifiable (winner did not score)", () => { + expect(isMatchNotable(false, true)).toBe(true); + }); + + it("returns true when both winner scored and loser is notifiable", () => { + expect(isMatchNotable(true, true)).toBe(true); + }); + + it("returns false when winner only advanced (no points) and loser is not notifiable", () => { + expect(isMatchNotable(false, false)).toBe(false); + }); +}); + describe("processPlayoffEvent - NBA Play-In Round 1 loserAdvances fix", () => { function makePlayInDb(matches: object[]) { const insertedRows: Array<{ participantId: string; finalPosition: number; isPartialScore: boolean }> = []; diff --git a/app/models/scoring-calculator.ts b/app/models/scoring-calculator.ts index 4df2bbd..9a40961 100644 --- a/app/models/scoring-calculator.ts +++ b/app/models/scoring-calculator.ts @@ -623,6 +623,25 @@ export function isLoserNotifiable( return scoreChanged || finalizedLoserIds.has(loserId); } +/** + * Returns true if a scored match should be displayed in a Discord standings + * notification at all. + * + * A match is shown only when the winner earned points this round or the loser is + * notifiable (eliminated / scored — see {@link isLoserNotifiable}). A drafted + * winner that merely advanced without earning points is NOT enough on its own. + * + * The winner's owner is still credited on any match that passes this gate (even + * a points-less advance) — this function only decides whether the match line + * appears, not which managers are labelled within it. + */ +export function isMatchNotable( + winnerScoreChanged: boolean, + loserNotifiable: boolean +): boolean { + return winnerScoreChanged || loserNotifiable; +} + export function getGuaranteedMinimumPosition( round: string, bracketTemplateId: string | null | undefined, @@ -1826,12 +1845,13 @@ export async function recalculateAffectedLeagues( ); // Build scored matches for the notification. - // Winners only appear if their team's score changed (they earned points this round). - // Losers appear if their team's score changed OR they were definitively eliminated - // (finalPosition set, non-partial) — the latter catches 0-pt eliminations that - // produce no score delta. Losers who advance to another match (loserAdvances=true, - // e.g. NBA 7v8 → PIR2) have no finalized result and no score change, so they're - // correctly suppressed. + // A match is included only when it is "notable": either the 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 (see isMatchNotable). + // On any match that IS included, the winning team's owner is always credited — even + // when the winner only advanced. 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. let scoredMatches: ScoredMatch[] | undefined; if (allCompletedMatches.length > 0) { const relevant = allCompletedMatches.filter( @@ -1857,15 +1877,17 @@ export async function recalculateAffectedLeagues( const winnerOwnerId = lookupOwner(m.winnerId); const loserOwnerId = lookupOwner(m.loserId); const showLoser = isLoserNotifiable(m.loserId, loserTeamId, changedTeamIds, finalizedLoserIds); - return { - winnerName: m.winnerName ?? "", - loserName: m.loserName ?? "", - winnerUsername: winnerScoreChanged && winnerOwnerId ? usernameByUserId.get(winnerOwnerId) : undefined, - loserUsername: showLoser && loserOwnerId ? usernameByUserId.get(loserOwnerId) : undefined, - winnerDiscordUserId: winnerScoreChanged && winnerOwnerId ? discordIdByUserId.get(winnerOwnerId) : undefined, - loserDiscordUserId: showLoser && loserOwnerId ? discordIdByUserId.get(loserOwnerId) : undefined, - }; - }); + return { m, winnerScoreChanged, showLoser, winnerOwnerId, loserOwnerId }; + }) + .filter((x) => isMatchNotable(x.winnerScoreChanged, x.showLoser)) + .map((x) => ({ + winnerName: x.m.winnerName ?? "", + loserName: x.m.loserName ?? "", + winnerUsername: x.winnerOwnerId ? usernameByUserId.get(x.winnerOwnerId) : undefined, + loserUsername: x.showLoser && x.loserOwnerId ? usernameByUserId.get(x.loserOwnerId) : undefined, + winnerDiscordUserId: 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 59f0be5..db897a7 100644 --- a/app/services/__tests__/discord.test.ts +++ b/app/services/__tests__/discord.test.ts @@ -186,6 +186,29 @@ 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. The calculator now sets winnerUsername unconditionally + // on any match that passes the notability gate. + await sendStandingsUpdateNotification({ + webhookUrl: WEBHOOK_URL, + seasonName: "Rumble League 2026", + standings: [{ teamId: "a", teamName: "Alpha", totalPoints: 100, rank: 1 }], + previousStandings: new Map([["a", 100]]), + scoredMatches: [ + { + winnerName: "Brazil", + loserName: "Japan", + winnerUsername: "someManager", + loserUsername: "ikyn", + }, + ], + }); + + const desc = getDescription(); + expect(desc).toContain("• **Brazil (someManager)** def. Japan (ikyn)"); + }); + it("omits scored matches where neither side has a username", async () => { await sendStandingsUpdateNotification({ webhookUrl: WEBHOOK_URL, -- 2.45.3 From 9a39267bdb32929618d463bb3e0ee437d5cae1d6 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 29 Jun 2026 23:40:19 +0000 Subject: [PATCH 2/2] Address code-review findings: ping guard, dual filter, inline helper - Restore winnerScoreChanged guard on winnerDiscordUserId so advancing-only winners are shown in the embed but not Discord-pinged - Add display-name filter at the end of the scoredMatches chain so scoredMatches always reflects exactly what appears in the embed, eliminating the dual-gate split between scoring-calculator.ts and discord.ts - Remove isMatchNotable export and inline x.winnerScoreChanged || x.showLoser at its sole call site; move the reasoning into the existing comment block Co-Authored-By: Claude --- .../__tests__/process-match-result.test.ts | 19 +------- app/models/scoring-calculator.ts | 43 +++++++------------ app/services/__tests__/discord.test.ts | 4 +- 3 files changed, 18 insertions(+), 48 deletions(-) diff --git a/app/models/__tests__/process-match-result.test.ts b/app/models/__tests__/process-match-result.test.ts index 4484a18..df825ee 100644 --- a/app/models/__tests__/process-match-result.test.ts +++ b/app/models/__tests__/process-match-result.test.ts @@ -59,7 +59,7 @@ function makeDb(existingResult?: { id: string; isPartialScore: boolean }) { }; } -import { processMatchResult, isLoserNotifiable, isMatchNotable, processPlayoffEvent } from "../scoring-calculator"; +import { processMatchResult, isLoserNotifiable, processPlayoffEvent } from "../scoring-calculator"; import { doesLoserAdvance } from "../playoff-match"; import { updateProbabilitiesAfterResult } from "~/services/probability-updater"; @@ -399,23 +399,6 @@ describe("isLoserNotifiable", () => { }); }); -describe("isMatchNotable", () => { - it("returns true when winner scored (loser not notifiable)", () => { - expect(isMatchNotable(true, false)).toBe(true); - }); - - it("returns true when loser is notifiable (winner did not score)", () => { - expect(isMatchNotable(false, true)).toBe(true); - }); - - it("returns true when both winner scored and loser is notifiable", () => { - expect(isMatchNotable(true, true)).toBe(true); - }); - - it("returns false when winner only advanced (no points) and loser is not notifiable", () => { - expect(isMatchNotable(false, false)).toBe(false); - }); -}); describe("processPlayoffEvent - NBA Play-In Round 1 loserAdvances fix", () => { function makePlayInDb(matches: object[]) { diff --git a/app/models/scoring-calculator.ts b/app/models/scoring-calculator.ts index 9a40961..f5cb685 100644 --- a/app/models/scoring-calculator.ts +++ b/app/models/scoring-calculator.ts @@ -623,24 +623,6 @@ export function isLoserNotifiable( return scoreChanged || finalizedLoserIds.has(loserId); } -/** - * Returns true if a scored match should be displayed in a Discord standings - * notification at all. - * - * A match is shown only when the winner earned points this round or the loser is - * notifiable (eliminated / scored — see {@link isLoserNotifiable}). A drafted - * winner that merely advanced without earning points is NOT enough on its own. - * - * The winner's owner is still credited on any match that passes this gate (even - * a points-less advance) — this function only decides whether the match line - * appears, not which managers are labelled within it. - */ -export function isMatchNotable( - winnerScoreChanged: boolean, - loserNotifiable: boolean -): boolean { - return winnerScoreChanged || loserNotifiable; -} export function getGuaranteedMinimumPosition( round: string, @@ -1845,13 +1827,17 @@ export async function recalculateAffectedLeagues( ); // Build scored matches for the notification. - // A match is included only when it is "notable": either the 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 (see isMatchNotable). - // On any match that IS included, the winning team's owner is always credited — even - // when the winner only advanced. 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. + // 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. let scoredMatches: ScoredMatch[] | undefined; if (allCompletedMatches.length > 0) { const relevant = allCompletedMatches.filter( @@ -1879,15 +1865,16 @@ export async function recalculateAffectedLeagues( const showLoser = isLoserNotifiable(m.loserId, loserTeamId, changedTeamIds, finalizedLoserIds); return { m, winnerScoreChanged, showLoser, winnerOwnerId, loserOwnerId }; }) - .filter((x) => isMatchNotable(x.winnerScoreChanged, x.showLoser)) + .filter((x) => x.winnerScoreChanged || x.showLoser) .map((x) => ({ winnerName: x.m.winnerName ?? "", loserName: x.m.loserName ?? "", winnerUsername: x.winnerOwnerId ? usernameByUserId.get(x.winnerOwnerId) : undefined, loserUsername: x.showLoser && x.loserOwnerId ? usernameByUserId.get(x.loserOwnerId) : undefined, - winnerDiscordUserId: x.winnerOwnerId ? discordIdByUserId.get(x.winnerOwnerId) : undefined, + winnerDiscordUserId: x.winnerScoreChanged && x.winnerOwnerId ? discordIdByUserId.get(x.winnerOwnerId) : undefined, loserDiscordUserId: x.showLoser && x.loserOwnerId ? discordIdByUserId.get(x.loserOwnerId) : undefined, - })); + })) + .filter((m) => m.winnerUsername !== undefined || m.loserUsername !== undefined); } } diff --git a/app/services/__tests__/discord.test.ts b/app/services/__tests__/discord.test.ts index db897a7..45d97a3 100644 --- a/app/services/__tests__/discord.test.ts +++ b/app/services/__tests__/discord.test.ts @@ -188,8 +188,8 @@ describe("sendStandingsUpdateNotification", () => { 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. The calculator now sets winnerUsername unconditionally - // on any match that passes the notability gate. + // Japan was eliminated. winnerUsername is shown in the embed; winnerDiscordUserId + // is left unset (no ping) because the winner did not score this round. await sendStandingsUpdateNotification({ webhookUrl: WEBHOOK_URL, seasonName: "Rumble League 2026", -- 2.45.3