From f4031d2a3861d028f1f99bc51cd427bd75e63771 Mon Sep 17 00:00:00 2001 From: Chris Parsons <438676+chrisparsons83@users.noreply.github.com> Date: Mon, 23 Mar 2026 21:22:53 -0700 Subject: [PATCH] Fix standings upsert no-op: use EXCLUDED pseudo-table (#215) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix upsert standings no-op: use EXCLUDED pseudo-table for conflict update Drizzle's onConflictDoUpdate set block was referencing the existing table columns instead of the incoming values, causing every sync to silently overwrite records with their own current data (no-op). Fixes #211 Co-Authored-By: Claude Sonnet 4.6 * Add unit tests for regular-season-standings model Covers upsertRegularSeasonStandings (empty-array short-circuit, value mapping, syncedAt behaviour), upsertManualStanding, getRegularSeasonStandings sorting, getLastSyncedAt, deleteRegularSeasonStandings, and getParticipantStanding — 19 tests total. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- .../regular-season-standings.test.ts | 333 ++++++++++++++++++ app/models/regular-season-standings.ts | 40 +-- 2 files changed, 353 insertions(+), 20 deletions(-) create mode 100644 app/models/__tests__/regular-season-standings.test.ts diff --git a/app/models/__tests__/regular-season-standings.test.ts b/app/models/__tests__/regular-season-standings.test.ts new file mode 100644 index 0000000..c27eb6e --- /dev/null +++ b/app/models/__tests__/regular-season-standings.test.ts @@ -0,0 +1,333 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("~/database/context", () => ({ + database: vi.fn(), +})); + +import { + upsertRegularSeasonStandings, + upsertManualStanding, + getRegularSeasonStandings, + getLastSyncedAt, + deleteRegularSeasonStandings, + getParticipantStanding, +} from "../regular-season-standings"; +import { database } from "~/database/context"; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function makeUpsertDb(returnedRows: object[] = []) { + const returning = vi.fn().mockResolvedValue(returnedRows); + const onConflictDoUpdate = vi.fn().mockReturnValue({ returning }); + const values = vi.fn().mockReturnValue({ onConflictDoUpdate }); + const insert = vi.fn().mockReturnValue({ values }); + const db: Record = { + insert, + transaction: vi.fn().mockImplementation(async (fn: (tx: unknown) => unknown) => fn(db)), + }; + return { db, insert, values, onConflictDoUpdate, returning }; +} + +function makeRecord(overrides: Record = {}) { + return { + participantId: "p-1", + sportsSeasonId: "ss-1", + wins: 10, + losses: 5, + gamesPlayed: 15, + ...overrides, + }; +} + +beforeEach(() => { + vi.clearAllMocks(); +}); + +// --------------------------------------------------------------------------- +// upsertRegularSeasonStandings +// --------------------------------------------------------------------------- + +describe("upsertRegularSeasonStandings", () => { + it("returns empty array immediately when given no records", async () => { + const { db } = makeUpsertDb(); + vi.mocked(database).mockReturnValue(db as never); + + const result = await upsertRegularSeasonStandings([]); + expect(result).toEqual([]); + expect(db.insert).not.toHaveBeenCalled(); + }); + + it("maps winPct and gamesBack to strings", async () => { + const { db, values } = makeUpsertDb([]); + vi.mocked(database).mockReturnValue(db as never); + + await upsertRegularSeasonStandings([ + makeRecord({ winPct: 0.667, gamesBack: 2.5 }), + ]); + + const inserted = values.mock.calls[0][0]; + expect(inserted[0].winPct).toBe("0.667"); + expect(inserted[0].gamesBack).toBe("2.5"); + }); + + it("maps null winPct and gamesBack to null", async () => { + const { db, values } = makeUpsertDb([]); + vi.mocked(database).mockReturnValue(db as never); + + await upsertRegularSeasonStandings([ + makeRecord({ winPct: null, gamesBack: null }), + ]); + + const inserted = values.mock.calls[0][0]; + expect(inserted[0].winPct).toBeNull(); + expect(inserted[0].gamesBack).toBeNull(); + }); + + it("uses the provided db instead of calling database()", async () => { + const { db } = makeUpsertDb([]); + + await upsertRegularSeasonStandings([makeRecord()], db as never); + + expect(database).not.toHaveBeenCalled(); + expect(db.transaction).toHaveBeenCalled(); + }); + + it("returns the rows from the DB", async () => { + const row = { id: "row-1", wins: 10 }; + const { db } = makeUpsertDb([row]); + vi.mocked(database).mockReturnValue(db as never); + + const result = await upsertRegularSeasonStandings([makeRecord()]); + expect(result).toEqual([row]); + }); + + it("inserts multiple records in a single call", async () => { + const { db, values } = makeUpsertDb([]); + vi.mocked(database).mockReturnValue(db as never); + + await upsertRegularSeasonStandings([ + makeRecord({ participantId: "p-1" }), + makeRecord({ participantId: "p-2" }), + ]); + + expect(values.mock.calls[0][0]).toHaveLength(2); + }); + + it("sets syncedAt to now when not provided", async () => { + const before = new Date(); + const { db, values } = makeUpsertDb([]); + vi.mocked(database).mockReturnValue(db as never); + + await upsertRegularSeasonStandings([makeRecord()]); + + const inserted = values.mock.calls[0][0]; + expect(inserted[0].syncedAt).toBeInstanceOf(Date); + expect(inserted[0].syncedAt.getTime()).toBeGreaterThanOrEqual(before.getTime()); + }); +}); + +// --------------------------------------------------------------------------- +// upsertManualStanding +// --------------------------------------------------------------------------- + +describe("upsertManualStanding", () => { + it("sets syncedAt to null", async () => { + const { db, values } = makeUpsertDb([{ id: "row-1" }]); + vi.mocked(database).mockReturnValue(db as never); + + await upsertManualStanding("p-1", "ss-1", { + wins: 10, + losses: 5, + gamesPlayed: 15, + }); + + const inserted = values.mock.calls[0][0]; + expect(inserted[0].syncedAt).toBeNull(); + }); + + it("returns the first row", async () => { + const row = { id: "row-1" }; + const { db } = makeUpsertDb([row]); + vi.mocked(database).mockReturnValue(db as never); + + const result = await upsertManualStanding("p-1", "ss-1", { + wins: 10, + losses: 5, + gamesPlayed: 15, + }); + + expect(result).toEqual(row); + }); +}); + +// --------------------------------------------------------------------------- +// getRegularSeasonStandings — sorting logic +// --------------------------------------------------------------------------- + +function makeRow(overrides: Record) { + return { + conference: null, + division: null, + divisionRank: null, + leagueRank: null, + participant: {}, + ...overrides, + }; +} + +function makeQueryDb(rows: object[]) { + return { + query: { + regularSeasonStandings: { + findMany: vi.fn().mockResolvedValue(rows), + }, + }, + }; +} + +describe("getRegularSeasonStandings", () => { + it("sorts by conference alphabetically", async () => { + const rows = [ + makeRow({ conference: "Western", leagueRank: 1 }), + makeRow({ conference: "Eastern", leagueRank: 2 }), + ]; + vi.mocked(database).mockReturnValue(makeQueryDb(rows) as never); + + const result = await getRegularSeasonStandings("ss-1"); + expect(result[0].conference).toBe("Eastern"); + expect(result[1].conference).toBe("Western"); + }); + + it("sorts by division within conference", async () => { + const rows = [ + makeRow({ conference: "East", division: "Southeast", leagueRank: 1 }), + makeRow({ conference: "East", division: "Atlantic", leagueRank: 2 }), + ]; + vi.mocked(database).mockReturnValue(makeQueryDb(rows) as never); + + const result = await getRegularSeasonStandings("ss-1"); + expect(result[0].division).toBe("Atlantic"); + expect(result[1].division).toBe("Southeast"); + }); + + it("sorts by divisionRank within division", async () => { + const rows = [ + makeRow({ conference: "East", division: "Atlantic", divisionRank: 3 }), + makeRow({ conference: "East", division: "Atlantic", divisionRank: 1 }), + makeRow({ conference: "East", division: "Atlantic", divisionRank: 2 }), + ]; + vi.mocked(database).mockReturnValue(makeQueryDb(rows) as never); + + const result = await getRegularSeasonStandings("ss-1"); + expect(result.map((r) => r.divisionRank)).toEqual([1, 2, 3]); + }); + + it("falls back to leagueRank when divisionRank is null", async () => { + const rows = [ + makeRow({ leagueRank: 3 }), + makeRow({ leagueRank: 1 }), + makeRow({ leagueRank: 2 }), + ]; + vi.mocked(database).mockReturnValue(makeQueryDb(rows) as never); + + const result = await getRegularSeasonStandings("ss-1"); + expect(result.map((r) => r.leagueRank)).toEqual([1, 2, 3]); + }); + + it("places null conference/division rows last", async () => { + const rows = [ + makeRow({ conference: null, leagueRank: 1 }), + makeRow({ conference: "Eastern", leagueRank: 2 }), + ]; + vi.mocked(database).mockReturnValue(makeQueryDb(rows) as never); + + const result = await getRegularSeasonStandings("ss-1"); + expect(result[0].conference).toBe("Eastern"); + expect(result[1].conference).toBeNull(); + }); +}); + +// --------------------------------------------------------------------------- +// getLastSyncedAt +// --------------------------------------------------------------------------- + +describe("getLastSyncedAt", () => { + function makeSelectDb(lastSync: Date | null) { + return { + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + where: vi.fn().mockResolvedValue([{ lastSync }]), + }), + }), + }; + } + + it("returns the max syncedAt date", async () => { + const date = new Date("2025-03-01T00:00:00Z"); + vi.mocked(database).mockReturnValue(makeSelectDb(date) as never); + + const result = await getLastSyncedAt("ss-1"); + expect(result).toEqual(date); + }); + + it("returns null when no synced records exist", async () => { + vi.mocked(database).mockReturnValue(makeSelectDb(null) as never); + + const result = await getLastSyncedAt("ss-1"); + expect(result).toBeNull(); + }); +}); + +// --------------------------------------------------------------------------- +// deleteRegularSeasonStandings +// --------------------------------------------------------------------------- + +describe("deleteRegularSeasonStandings", () => { + it("calls delete on the DB", async () => { + const where = vi.fn().mockResolvedValue(undefined); + const del = vi.fn().mockReturnValue({ where }); + const db = { delete: del }; + vi.mocked(database).mockReturnValue(db as never); + + await deleteRegularSeasonStandings("ss-1"); + expect(del).toHaveBeenCalled(); + expect(where).toHaveBeenCalled(); + }); +}); + +// --------------------------------------------------------------------------- +// getParticipantStanding +// --------------------------------------------------------------------------- + +describe("getParticipantStanding", () => { + it("returns the standing row when found", async () => { + const row = { id: "row-1", participantId: "p-1" }; + const db = { + query: { + regularSeasonStandings: { + findFirst: vi.fn().mockResolvedValue(row), + }, + }, + }; + vi.mocked(database).mockReturnValue(db as never); + + const result = await getParticipantStanding("p-1", "ss-1"); + expect(result).toEqual(row); + }); + + it("returns undefined when not found", async () => { + const db = { + query: { + regularSeasonStandings: { + findFirst: vi.fn().mockResolvedValue(undefined), + }, + }, + }; + vi.mocked(database).mockReturnValue(db as never); + + const result = await getParticipantStanding("p-99", "ss-1"); + expect(result).toBeUndefined(); + }); +}); diff --git a/app/models/regular-season-standings.ts b/app/models/regular-season-standings.ts index b332455..3b867ee 100644 --- a/app/models/regular-season-standings.ts +++ b/app/models/regular-season-standings.ts @@ -1,6 +1,6 @@ import { database } from "~/database/context"; import * as schema from "~/database/schema"; -import { eq, and, max } from "drizzle-orm"; +import { eq, and, max, sql } from "drizzle-orm"; export interface UpsertRegularSeasonStandingData { participantId: string; @@ -73,25 +73,25 @@ export async function upsertRegularSeasonStandings( schema.regularSeasonStandings.sportsSeasonId, ], set: { - wins: schema.regularSeasonStandings.wins, - losses: schema.regularSeasonStandings.losses, - otLosses: schema.regularSeasonStandings.otLosses, - ties: schema.regularSeasonStandings.ties, - winPct: schema.regularSeasonStandings.winPct, - gamesPlayed: schema.regularSeasonStandings.gamesPlayed, - gamesBack: schema.regularSeasonStandings.gamesBack, - conference: schema.regularSeasonStandings.conference, - division: schema.regularSeasonStandings.division, - conferenceRank: schema.regularSeasonStandings.conferenceRank, - divisionRank: schema.regularSeasonStandings.divisionRank, - leagueRank: schema.regularSeasonStandings.leagueRank, - streak: schema.regularSeasonStandings.streak, - lastTen: schema.regularSeasonStandings.lastTen, - homeRecord: schema.regularSeasonStandings.homeRecord, - awayRecord: schema.regularSeasonStandings.awayRecord, - externalTeamId: schema.regularSeasonStandings.externalTeamId, - syncedAt: schema.regularSeasonStandings.syncedAt, - updatedAt: schema.regularSeasonStandings.updatedAt, + wins: sql`excluded.wins`, + losses: sql`excluded.losses`, + otLosses: sql`excluded.ot_losses`, + ties: sql`excluded.ties`, + winPct: sql`excluded.win_pct`, + gamesPlayed: sql`excluded.games_played`, + gamesBack: sql`excluded.games_back`, + conference: sql`excluded.conference`, + division: sql`excluded.division`, + conferenceRank: sql`excluded.conference_rank`, + divisionRank: sql`excluded.division_rank`, + leagueRank: sql`excluded.league_rank`, + streak: sql`excluded.streak`, + lastTen: sql`excluded.last_ten`, + homeRecord: sql`excluded.home_record`, + awayRecord: sql`excluded.away_record`, + externalTeamId: sql`excluded.external_team_id`, + syncedAt: sql`excluded.synced_at`, + updatedAt: sql`excluded.updated_at`, }, }) .returning();