From 21c6fd6229faa57ec5de73c7c9913bb0493daf21 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 1 Jul 2026 21:20:50 +0000 Subject: [PATCH] Extract shared QP change-detection into diffChangedQualifyingPoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The before/after QP diff (with its null = "no prior award" and null → value "first-time score" semantics) was duplicated in processQualifyingEvent and rescoreTennisBracketAndDetectChanges. The two entry points must stay separate (tennis snapshots before an authoritative delete-all; CS2 preserves rows), but the comparison itself should live in one place. Add diffChangedQualifyingPoints(before, after) in qualifying-points.ts and call it from both paths. Unit-tested directly; the two integration tests still cover the end-to-end behavior. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Hq3fG4Zn3unntbSQAGSqCq --- .../__tests__/qualifying-points.test.ts | 39 ++++++++++++++++++ app/models/qualifying-points.ts | 27 +++++++++++++ app/models/scoring-calculator.ts | 22 +++++----- .../__tests__/sync-tennis-rescore.test.ts | 40 +++++++++++-------- app/services/match-sync/index.ts | 28 +++++-------- 5 files changed, 109 insertions(+), 47 deletions(-) diff --git a/app/models/__tests__/qualifying-points.test.ts b/app/models/__tests__/qualifying-points.test.ts index e49ae78..d56ab37 100644 --- a/app/models/__tests__/qualifying-points.test.ts +++ b/app/models/__tests__/qualifying-points.test.ts @@ -2,9 +2,48 @@ import { describe, it, expect } from "vitest"; import { calculateSplitQualifyingPoints, DEFAULT_QP_VALUES, + diffChangedQualifyingPoints, hasProcessedQualifyingPlacement, } from "../qualifying-points"; +describe("diffChangedQualifyingPoints", () => { + it("flags a changed value, a first-time score (null → value), and ignores unchanged", () => { + const before = [ + { id: "A", qp: "10.00" }, + { id: "B", qp: "5.00" }, + { id: "C", qp: null }, + ]; + const after = [ + { id: "A", qp: "10.00" }, // unchanged + { id: "B", qp: "8.00" }, // changed + { id: "C", qp: "3.00" }, // first-time score + ]; + + expect([...diffChangedQualifyingPoints(before, after)].sort()).toEqual(["B", "C"]); + }); + + it("normalizes decimal formatting so 10 and 10.00 are equal", () => { + const changed = diffChangedQualifyingPoints([{ id: "A", qp: "10" }], [{ id: "A", qp: "10.00" }]); + expect(changed.size).toBe(0); + }); + + it("does not report participants absent from the after set", () => { + const changed = diffChangedQualifyingPoints( + [ + { id: "A", qp: "10.00" }, + { id: "D", qp: "2.00" }, + ], + [{ id: "A", qp: "10.00" }], + ); + expect(changed.size).toBe(0); + }); + + it("treats a value → zero drop as a change", () => { + const changed = diffChangedQualifyingPoints([{ id: "A", qp: "10.00" }], [{ id: "A", qp: "0.00" }]); + expect([...changed]).toEqual(["A"]); + }); +}); + describe("Qualifying Points Configuration", () => { describe("DEFAULT_QP_VALUES", () => { it("should have 16 placement values", () => { diff --git a/app/models/qualifying-points.ts b/app/models/qualifying-points.ts index cde2d03..5812f33 100644 --- a/app/models/qualifying-points.ts +++ b/app/models/qualifying-points.ts @@ -33,6 +33,33 @@ export function roundQualifyingPoints(points: number): number { return Math.round((points + Number.EPSILON) * 100) / 100; } +/** + * Given a participant's awarded QP before and after a re-score, return the ids + * whose QP changed. A row's `qp` is the raw decimal string (or null when no QP + * was awarded). A participant counts as changed when their new value differs from + * the old, including a first-time score (null → value). Participants absent from + * `after` are not reported (their removal is handled by the caller's recalc). + * + * Shared by the two QP re-score paths — processQualifyingEvent (sibling windows, + * CS2, manual admin) and rescoreTennisBracketAndDetectChanges (tennis primary) — + * so the "only announce real changes" semantics live in one place. + */ +export function diffChangedQualifyingPoints( + before: Iterable<{ id: string; qp: string | null }>, + after: Iterable<{ id: string; qp: string | null }> +): Set { + const beforeQP = new Map(); + for (const r of before) { + if (r.qp !== null) beforeQP.set(r.id, parseFloat(r.qp)); + } + const changed = new Set(); + for (const r of after) { + if (r.qp === null) continue; + if (beforeQP.get(r.id) !== parseFloat(r.qp)) changed.add(r.id); + } + return changed; +} + export function calculateSplitQualifyingPoints( placement: number, tieCount: number, diff --git a/app/models/scoring-calculator.ts b/app/models/scoring-calculator.ts index 3b16eb7..190f1eb 100644 --- a/app/models/scoring-calculator.ts +++ b/app/models/scoring-calculator.ts @@ -21,6 +21,7 @@ import { logger } from "~/lib/logger"; import { getEventResults } from "./event-result"; import { calculateSplitQualifyingPoints, + diffChangedQualifyingPoints, getQPConfig, hasProcessedQualifyingPlacement, recalculateParticipantQP, @@ -883,11 +884,10 @@ export async function processQualifyingEvent( // announce only the participants whose QP actually changed. Without this, a // sibling window re-scored on every fan-out sync (syncTournamentResults) would // re-ping the full QP standings each run even when nothing changed. - const beforeQP = new Map( - results - .filter((r) => r.qualifyingPointsAwarded !== null) - .map((r) => [r.seasonParticipantId, parseFloat(r.qualifyingPointsAwarded as string)]) - ); + const beforeQP = results.map((r) => ({ + id: r.seasonParticipantId, + qp: r.qualifyingPointsAwarded, + })); if (event.bracketTemplateId) { // Bracket-based qualifying event (e.g. CS2 Champions Stage): QP is owned by the @@ -976,14 +976,10 @@ export async function processQualifyingEvent( const afterRows = await db.query.eventResults.findMany({ where: eq(schema.eventResults.scoringEventId, eventId), }); - const changedParticipantIds = new Set(); - for (const r of afterRows) { - if (r.qualifyingPointsAwarded === null) continue; - const newQP = parseFloat(r.qualifyingPointsAwarded); - if (beforeQP.get(r.seasonParticipantId) !== newQP) { - changedParticipantIds.add(r.seasonParticipantId); - } - } + const changedParticipantIds = diffChangedQualifyingPoints( + beforeQP, + afterRows.map((r) => ({ id: r.seasonParticipantId, qp: r.qualifyingPointsAwarded })) + ); if (changedParticipantIds.size > 0) { try { diff --git a/app/services/match-sync/__tests__/sync-tennis-rescore.test.ts b/app/services/match-sync/__tests__/sync-tennis-rescore.test.ts index 8d03a88..6b73b7f 100644 --- a/app/services/match-sync/__tests__/sync-tennis-rescore.test.ts +++ b/app/services/match-sync/__tests__/sync-tennis-rescore.test.ts @@ -10,9 +10,15 @@ vi.mock("~/models/scoring-calculator", () => ({ recalculateAffectedLeagues: vi.fn(), })); -vi.mock("~/models/qualifying-points", () => ({ - recalculateParticipantQP: vi.fn().mockResolvedValue(undefined), -})); +// Keep the real diffChangedQualifyingPoints (the change-detection primitive under +// test here); only stub the dropped-participant total recalc. +vi.mock("~/models/qualifying-points", async (importActual) => { + const actual = await importActual(); + return { + ...actual, + recalculateParticipantQP: vi.fn().mockResolvedValue(undefined), + }; +}); import { rescoreTennisBracketAndDetectChanges } from "../index"; import { processQualifyingBracketEvent } from "~/models/scoring-calculator"; @@ -21,7 +27,7 @@ import { recalculateParticipantQP } from "~/models/qualifying-points"; const EVENT_ID = "ev-1"; const SPORTS_SEASON_ID = "ss-1"; -type Row = { pid: string; qp: string | null }; +type Row = { id: string; qp: string | null }; /** * Fake Drizzle db whose two `select().from().where()` calls resolve, in order, to @@ -51,13 +57,13 @@ describe("rescoreTennisBracketAndDetectChanges", () => { it("flags participants whose QP changed and newly-scored participants, ignoring unchanged ones", async () => { const db = makeDb( [ - { pid: "A", qp: "10" }, - { pid: "B", qp: "5" }, + { id: "A", qp: "10" }, + { id: "B", qp: "5" }, ], [ - { pid: "A", qp: "10" }, // unchanged - { pid: "B", qp: "8" }, // changed 5 -> 8 - { pid: "C", qp: "3" }, // newly scored + { id: "A", qp: "10" }, // unchanged + { id: "B", qp: "8" }, // changed 5 -> 8 + { id: "C", qp: "3" }, // newly scored ], ); @@ -70,7 +76,7 @@ describe("rescoreTennisBracketAndDetectChanges", () => { }); it("treats a null-before → value-after transition as a change", async () => { - const db = makeDb([{ pid: "A", qp: null }], [{ pid: "A", qp: "10" }]); + const db = makeDb([{ id: "A", qp: null }], [{ id: "A", qp: "10" }]); const changed = await rescoreTennisBracketAndDetectChanges(EVENT_ID, SPORTS_SEASON_ID, db as never); @@ -80,12 +86,12 @@ describe("rescoreTennisBracketAndDetectChanges", () => { it("announces nothing when a re-sync produces identical QP", async () => { const db = makeDb( [ - { pid: "A", qp: "10" }, - { pid: "B", qp: "5" }, + { id: "A", qp: "10" }, + { id: "B", qp: "5" }, ], [ - { pid: "A", qp: "10" }, - { pid: "B", qp: "5" }, + { id: "A", qp: "10" }, + { id: "B", qp: "5" }, ], ); @@ -98,10 +104,10 @@ describe("rescoreTennisBracketAndDetectChanges", () => { it("recalculates totals for participants dropped from the rewritten results", async () => { const db = makeDb( [ - { pid: "A", qp: "10" }, - { pid: "D", qp: "2" }, // present before, gone after (early-round loser) + { id: "A", qp: "10" }, + { id: "D", qp: "2" }, // present before, gone after (early-round loser) ], - [{ pid: "A", qp: "10" }], + [{ id: "A", qp: "10" }], ); const changed = await rescoreTennisBracketAndDetectChanges(EVENT_ID, SPORTS_SEASON_ID, db as never); diff --git a/app/services/match-sync/index.ts b/app/services/match-sync/index.ts index 824aa4b..41cff46 100644 --- a/app/services/match-sync/index.ts +++ b/app/services/match-sync/index.ts @@ -24,7 +24,10 @@ import { } from "~/models/scoring-calculator"; import { fanOutMajorIfPrimary } from "~/services/sync-tournament-results"; import { notifyQualifyingPointsUpdate } from "~/services/qualifying-points-discord.server"; -import { recalculateParticipantQP } from "~/models/qualifying-points"; +import { + recalculateParticipantQP, + diffChangedQualifyingPoints, +} from "~/models/qualifying-points"; import { getScoringEventById, updateScoringEvent, isReadOnlySibling } from "~/models/scoring-event"; import { findMatchingTeamName, normalizeTeamName } from "~/lib/normalize-team-name"; import { buildUnmatchedTeamResolutionView } from "~/lib/unmatched-team-reconciliation"; @@ -651,43 +654,34 @@ export async function rescoreTennisBracketAndDetectChanges( sportsSeasonId: string, db: ReturnType, ): Promise> { - const changed = new Set(); + let changed = new Set(); await db.transaction(async (tx) => { const beforeRows = await tx .select({ - pid: schema.eventResults.seasonParticipantId, + id: schema.eventResults.seasonParticipantId, qp: schema.eventResults.qualifyingPointsAwarded, }) .from(schema.eventResults) .where(eq(schema.eventResults.scoringEventId, eventId)); - const beforeQP = new Map( - beforeRows - .filter((r) => r.qp !== null) - .map((r) => [r.pid, parseFloat(r.qp as string)]), - ); await tx.delete(schema.eventResults).where(eq(schema.eventResults.scoringEventId, eventId)); await processQualifyingBracketEvent(eventId, tx); const afterRows = await tx .select({ - pid: schema.eventResults.seasonParticipantId, + id: schema.eventResults.seasonParticipantId, qp: schema.eventResults.qualifyingPointsAwarded, }) .from(schema.eventResults) .where(eq(schema.eventResults.scoringEventId, eventId)); - const afterIds = new Set(afterRows.map((r) => r.pid)); + const afterIds = new Set(afterRows.map((r) => r.id)); - for (const { pid } of beforeRows) { - if (!afterIds.has(pid)) await recalculateParticipantQP(pid, sportsSeasonId, tx); + for (const { id } of beforeRows) { + if (!afterIds.has(id)) await recalculateParticipantQP(id, sportsSeasonId, tx); } - for (const r of afterRows) { - if (r.qp === null) continue; - const newQP = parseFloat(r.qp as string); - if (beforeQP.get(r.pid) !== newQP) changed.add(r.pid); - } + changed = diffChangedQualifyingPoints(beforeRows, afterRows); }); return changed;