From ee316fef21c165a0119ad4c38b1dd4f4658de343 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 15 Apr 2026 16:31:44 +0000 Subject: [PATCH] Address code review feedback on Play-In loserAdvances fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- .../__tests__/process-match-result.test.ts | 37 +++++++++++++++++-- app/models/scoring-calculator.ts | 2 +- ...sons.$id.events.$eventId.bracket.server.ts | 1 + 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/app/models/__tests__/process-match-result.test.ts b/app/models/__tests__/process-match-result.test.ts index ddf0827..1112225 100644 --- a/app/models/__tests__/process-match-result.test.ts +++ b/app/models/__tests__/process-match-result.test.ts @@ -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 = [ { 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", 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); 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) .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-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); + (db.query.scoringEvents.findFirst as ReturnType).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 }); @@ -488,6 +518,7 @@ describe("processPlayoffEvent - NBA Play-In Round 1 loserAdvances fix", () => { .filter((r) => r.finalPosition === 0 && !r.isPartialScore) .map((r) => r.participantId); - expect(eliminatedIds).toContain("loser-910e"); + expect(eliminatedIds).toContain("loser-78e"); + expect(eliminatedIds).toContain("loser-78w"); }); }); diff --git a/app/models/scoring-calculator.ts b/app/models/scoring-calculator.ts index 3a94b9a..70c5b7d 100644 --- a/app/models/scoring-calculator.ts +++ b/app/models/scoring-calculator.ts @@ -252,7 +252,7 @@ export async function processPlayoffEvent( // receive floor points — R64 and R32 winners are not yet guaranteed top-8. const awardFloor = doesNonScoringRoundFeedIntoScoringRound(round, event.bracketTemplateId); 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) { await upsertParticipantResult(match.loserId, event.sportsSeasonId, 0, db); } diff --git a/app/routes/admin.sports-seasons.$id.events.$eventId.bracket.server.ts b/app/routes/admin.sports-seasons.$id.events.$eventId.bracket.server.ts index 90f697d..763d614 100644 --- a/app/routes/admin.sports-seasons.$id.events.$eventId.bracket.server.ts +++ b/app/routes/admin.sports-seasons.$id.events.$eventId.bracket.server.ts @@ -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 }); return { success: `Recalculated floors from ${sortedMatches.length} completed match(es)` };