Address code review feedback on Play-In loserAdvances fix
- Use outer `round` variable instead of match.round in processPlayoffEvent (they're identical, but consistent with surrounding code) - Add comment at recalculate-floors call site explaining skipDiscord intent - Combine two redundant test cases into one covering all four assertions - Add West conference matches (M3/M4) to test fixture — East-only was insufficient given doesLoserAdvance checks matchNumber 1 & 3 - Add guard test: when bracketTemplateId is null all losers are eliminated, catching any future refactor that drops the field from the DB query https://claude.ai/code/session_01QmvezscLYY38gN4GbvXZbA
This commit is contained in:
parent
23daeb1dde
commit
ee316fef21
3 changed files with 36 additions and 4 deletions
|
|
@ -452,6 +452,7 @@ describe("processPlayoffEvent - NBA Play-In Round 1 loserAdvances fix", () => {
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// All four Play-In Round 1 matches: East 7v8 (M1), East 9v10 (M2), West 7v8 (M3), West 9v10 (M4)
|
||||||
const PLAY_IN_MATCHES = [
|
const PLAY_IN_MATCHES = [
|
||||||
{
|
{
|
||||||
id: "m1", round: "Play-In Round 1", matchNumber: 1, isScoring: false,
|
id: "m1", round: "Play-In Round 1", matchNumber: 1, isScoring: false,
|
||||||
|
|
@ -465,9 +466,21 @@ describe("processPlayoffEvent - NBA Play-In Round 1 loserAdvances fix", () => {
|
||||||
participant1Id: "winner-910e", participant2Id: "loser-910e",
|
participant1Id: "winner-910e", participant2Id: "loser-910e",
|
||||||
winnerId: "winner-910e", loserId: "loser-910e",
|
winnerId: "winner-910e", loserId: "loser-910e",
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
id: "m3", round: "Play-In Round 1", matchNumber: 3, isScoring: false,
|
||||||
|
isComplete: true, scoringEventId: "event-1",
|
||||||
|
participant1Id: "winner-78w", participant2Id: "loser-78w",
|
||||||
|
winnerId: "winner-78w", loserId: "loser-78w",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
id: "m4", round: "Play-In Round 1", matchNumber: 4, isScoring: false,
|
||||||
|
isComplete: true, scoringEventId: "event-1",
|
||||||
|
participant1Id: "winner-910w", participant2Id: "loser-910w",
|
||||||
|
winnerId: "winner-910w", loserId: "loser-910w",
|
||||||
|
},
|
||||||
];
|
];
|
||||||
|
|
||||||
it("does NOT eliminate the 7v8 loser (they advance to Play-In Round 2)", async () => {
|
it("eliminates 9v10 losers (East & West) but NOT 7v8 losers", async () => {
|
||||||
const { db, insertedRows } = makePlayInDb(PLAY_IN_MATCHES);
|
const { db, insertedRows } = makePlayInDb(PLAY_IN_MATCHES);
|
||||||
|
|
||||||
await processPlayoffEvent("event-1", db, { skipRecalculate: true });
|
await processPlayoffEvent("event-1", db, { skipRecalculate: true });
|
||||||
|
|
@ -476,11 +489,28 @@ describe("processPlayoffEvent - NBA Play-In Round 1 loserAdvances fix", () => {
|
||||||
.filter((r) => r.finalPosition === 0 && !r.isPartialScore)
|
.filter((r) => r.finalPosition === 0 && !r.isPartialScore)
|
||||||
.map((r) => r.participantId);
|
.map((r) => r.participantId);
|
||||||
|
|
||||||
|
// 7v8 losers advance to Play-In Round 2 — must not be eliminated yet
|
||||||
expect(eliminatedIds).not.toContain("loser-78e");
|
expect(eliminatedIds).not.toContain("loser-78e");
|
||||||
|
expect(eliminatedIds).not.toContain("loser-78w");
|
||||||
|
// 9v10 losers are out immediately
|
||||||
|
expect(eliminatedIds).toContain("loser-910e");
|
||||||
|
expect(eliminatedIds).toContain("loser-910w");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("DOES eliminate the 9v10 loser", async () => {
|
it("eliminates ALL losers when bracketTemplateId is missing (falls back to safe default)", async () => {
|
||||||
|
// Guards against a future refactor that drops bracketTemplateId from the DB query.
|
||||||
|
// doesLoserAdvance receives "" → returns false for all matches → all losers eliminated.
|
||||||
const { db, insertedRows } = makePlayInDb(PLAY_IN_MATCHES);
|
const { db, insertedRows } = makePlayInDb(PLAY_IN_MATCHES);
|
||||||
|
(db.query.scoringEvents.findFirst as ReturnType<typeof vi.fn>).mockResolvedValueOnce({
|
||||||
|
id: "event-1",
|
||||||
|
playoffRound: "Play-In Round 1",
|
||||||
|
bracketTemplateId: null, // simulates missing templateId
|
||||||
|
sportsSeasonId: "ss-1",
|
||||||
|
name: "NBA Playoffs",
|
||||||
|
sportsSeason: {
|
||||||
|
seasonSports: [{ seasonId: "season-1", season: {} }],
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
await processPlayoffEvent("event-1", db, { skipRecalculate: true });
|
await processPlayoffEvent("event-1", db, { skipRecalculate: true });
|
||||||
|
|
||||||
|
|
@ -488,6 +518,7 @@ describe("processPlayoffEvent - NBA Play-In Round 1 loserAdvances fix", () => {
|
||||||
.filter((r) => r.finalPosition === 0 && !r.isPartialScore)
|
.filter((r) => r.finalPosition === 0 && !r.isPartialScore)
|
||||||
.map((r) => r.participantId);
|
.map((r) => r.participantId);
|
||||||
|
|
||||||
expect(eliminatedIds).toContain("loser-910e");
|
expect(eliminatedIds).toContain("loser-78e");
|
||||||
|
expect(eliminatedIds).toContain("loser-78w");
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -252,7 +252,7 @@ export async function processPlayoffEvent(
|
||||||
// receive floor points — R64 and R32 winners are not yet guaranteed top-8.
|
// receive floor points — R64 and R32 winners are not yet guaranteed top-8.
|
||||||
const awardFloor = doesNonScoringRoundFeedIntoScoringRound(round, event.bracketTemplateId);
|
const awardFloor = doesNonScoringRoundFeedIntoScoringRound(round, event.bracketTemplateId);
|
||||||
for (const match of matches) {
|
for (const match of matches) {
|
||||||
const loserAdvances = doesLoserAdvance(match.round, match.matchNumber, event.bracketTemplateId ?? "");
|
const loserAdvances = doesLoserAdvance(round, match.matchNumber, event.bracketTemplateId ?? "");
|
||||||
if (match.loserId && !loserAdvances) {
|
if (match.loserId && !loserAdvances) {
|
||||||
await upsertParticipantResult(match.loserId, event.sportsSeasonId, 0, db);
|
await upsertParticipantResult(match.loserId, event.sportsSeasonId, 0, db);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -619,6 +619,7 @@ export async function action({ request, params }: Route.ActionArgs) {
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// skipDiscord: recalculate-floors is a data-correction tool, not a result announcement.
|
||||||
await recalculateAffectedLeagues(event.sportsSeasonId, undefined, { skipDiscord: true });
|
await recalculateAffectedLeagues(event.sportsSeasonId, undefined, { skipDiscord: true });
|
||||||
|
|
||||||
return { success: `Recalculated floors from ${sortedMatches.length} completed match(es)` };
|
return { success: `Recalculated floors from ${sortedMatches.length} completed match(es)` };
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue