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 <noreply@anthropic.com>
This commit is contained in:
parent
4a9a70ca7c
commit
9a39267bdb
3 changed files with 18 additions and 48 deletions
|
|
@ -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 { doesLoserAdvance } from "../playoff-match";
|
||||||
import { updateProbabilitiesAfterResult } from "~/services/probability-updater";
|
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", () => {
|
describe("processPlayoffEvent - NBA Play-In Round 1 loserAdvances fix", () => {
|
||||||
function makePlayInDb(matches: object[]) {
|
function makePlayInDb(matches: object[]) {
|
||||||
|
|
|
||||||
|
|
@ -623,24 +623,6 @@ export function isLoserNotifiable(
|
||||||
return scoreChanged || finalizedLoserIds.has(loserId);
|
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(
|
export function getGuaranteedMinimumPosition(
|
||||||
round: string,
|
round: string,
|
||||||
|
|
@ -1845,13 +1827,17 @@ export async function recalculateAffectedLeagues(
|
||||||
);
|
);
|
||||||
|
|
||||||
// Build scored matches for the notification.
|
// Build scored matches for the notification.
|
||||||
// A match is included only when it is "notable": either the winner earned points this
|
// A match is included only when it is "notable": winner earned points this round OR
|
||||||
// round OR the loser is notifiable (eliminated / scored). A drafted winner that merely
|
// the loser is notifiable (eliminated / scored). A drafted winner that merely advanced
|
||||||
// advanced without earning points is NOT enough on its own (see isMatchNotable).
|
// without earning points is NOT enough on its own.
|
||||||
// On any match that IS included, the winning team's owner is always credited — even
|
// On any included match, the winning team's owner is shown in the embed (winnerUsername),
|
||||||
// when the winner only advanced. Losers appear if their score changed or they were
|
// but Discord-pinged only when they actually scored (winnerDiscordUserId gated on
|
||||||
// definitively eliminated (finalPosition set, non-partial); losers who advance to
|
// winnerScoreChanged) — advancing-only wins don't warrant a ping.
|
||||||
// another match (loserAdvances=true, e.g. NBA 7v8 → PIR2) are correctly suppressed.
|
// 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;
|
let scoredMatches: ScoredMatch[] | undefined;
|
||||||
if (allCompletedMatches.length > 0) {
|
if (allCompletedMatches.length > 0) {
|
||||||
const relevant = allCompletedMatches.filter(
|
const relevant = allCompletedMatches.filter(
|
||||||
|
|
@ -1879,15 +1865,16 @@ export async function recalculateAffectedLeagues(
|
||||||
const showLoser = isLoserNotifiable(m.loserId, loserTeamId, changedTeamIds, finalizedLoserIds);
|
const showLoser = isLoserNotifiable(m.loserId, loserTeamId, changedTeamIds, finalizedLoserIds);
|
||||||
return { m, winnerScoreChanged, showLoser, winnerOwnerId, loserOwnerId };
|
return { m, winnerScoreChanged, showLoser, winnerOwnerId, loserOwnerId };
|
||||||
})
|
})
|
||||||
.filter((x) => isMatchNotable(x.winnerScoreChanged, x.showLoser))
|
.filter((x) => x.winnerScoreChanged || x.showLoser)
|
||||||
.map((x) => ({
|
.map((x) => ({
|
||||||
winnerName: x.m.winnerName ?? "",
|
winnerName: x.m.winnerName ?? "",
|
||||||
loserName: x.m.loserName ?? "",
|
loserName: x.m.loserName ?? "",
|
||||||
winnerUsername: 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,
|
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,
|
loserDiscordUserId: x.showLoser && x.loserOwnerId ? discordIdByUserId.get(x.loserOwnerId) : undefined,
|
||||||
}));
|
}))
|
||||||
|
.filter((m) => m.winnerUsername !== undefined || m.loserUsername !== undefined);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -188,8 +188,8 @@ describe("sendStandingsUpdateNotification", () => {
|
||||||
|
|
||||||
it("shows winner's owner even when winner only advanced (loser was eliminated)", async () => {
|
it("shows winner's owner even when winner only advanced (loser was eliminated)", async () => {
|
||||||
// Mirrors the Brazil/Japan scenario: Brazil advanced without earning points,
|
// Mirrors the Brazil/Japan scenario: Brazil advanced without earning points,
|
||||||
// Japan was eliminated. The calculator now sets winnerUsername unconditionally
|
// Japan was eliminated. winnerUsername is shown in the embed; winnerDiscordUserId
|
||||||
// on any match that passes the notability gate.
|
// is left unset (no ping) because the winner did not score this round.
|
||||||
await sendStandingsUpdateNotification({
|
await sendStandingsUpdateNotification({
|
||||||
webhookUrl: WEBHOOK_URL,
|
webhookUrl: WEBHOOK_URL,
|
||||||
seasonName: "Rumble League 2026",
|
seasonName: "Rumble League 2026",
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue