From 7f7ea4e29da96ca12278880890d021a23da77af3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 2 Jul 2026 05:17:37 +0000 Subject: [PATCH 1/2] Improve shared-tournament/event management UX Deleting an event from a season only affects that season's scoring event; the shared canonical tournament and the other linked seasons survive. The old flow never communicated this and offered no way to manage the relationship, so it felt like deleting an event might wipe the whole shared tournament. - deleteScoringEvent is now shared-tournament-aware: it auto-heals the primary window (promotes the earliest remaining window when the primary is removed) and optionally deletes the canonical tournament when the last linked window is removed. Returns a result describing what happened. - Add deleteTournament model helper. - Events page: delete confirmation now explains exactly what a delete does (removed from this season only vs. last window), with an opt-in checkbox to also remove the orphaned shared tournament. - Tournament page: add a per-window "Remove" control on the Linked Sports Seasons card to unlink a season, reusing the auto-heal path. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01CBzjCLVkVaQMj1MF3K54t2 --- .../__tests__/scoring-event-delete.test.ts | 97 +++++++++++++++++++ app/models/__tests__/tournament.test.ts | 24 +++++ app/models/scoring-event.ts | 71 +++++++++++++- app/models/tournament.ts | 14 +++ .../admin.sports-seasons.$id.events.server.ts | 46 ++++++++- .../admin.sports-seasons.$id.events.tsx | 71 +++++++++++--- app/routes/admin.tournaments.$id.tsx | 78 +++++++++++++++ 7 files changed, 380 insertions(+), 21 deletions(-) diff --git a/app/models/__tests__/scoring-event-delete.test.ts b/app/models/__tests__/scoring-event-delete.test.ts index c73741c..6e29e84 100644 --- a/app/models/__tests__/scoring-event-delete.test.ts +++ b/app/models/__tests__/scoring-event-delete.test.ts @@ -63,4 +63,101 @@ describe("deleteScoringEvent", () => { ]) ); }); + + describe("shared tournament handling", () => { + // Builds a mock db for a non-qualifying schedule_event linked to a tournament, + // so the delete skips the QP/league branches and exercises only the + // shared-tournament bookkeeping. + function makeSharedDb({ + deletedEvent, + remaining, + siblingTournamentId = "tournament-1", + }: { + deletedEvent: { tournamentId: string | null; isPrimary: boolean }; + remaining: Array<{ id: string; isPrimary: boolean }>; + siblingTournamentId?: string; + }) { + const findFirst = vi + .fn() + // 1st call: the event being deleted + .mockResolvedValueOnce({ + id: "event-1", + sportsSeasonId: "sports-season-1", + eventType: "schedule_event", + isQualifyingEvent: false, + ...deletedEvent, + }) + // subsequent calls: setPrimaryEvent looking up the promoted event + .mockResolvedValue({ id: "promoted", tournamentId: siblingTournamentId }); + + const deleteWhere = vi.fn().mockResolvedValue(undefined); + const updateWhere = vi.fn().mockResolvedValue(undefined); + const db = { + query: { + scoringEvents: { + findFirst, + findMany: vi.fn().mockResolvedValue(remaining), + }, + }, + transaction: vi.fn(async (callback) => callback(db)), + delete: vi.fn(() => ({ where: deleteWhere })), + update: vi.fn(() => ({ set: vi.fn(() => ({ where: updateWhere })) })), + } as any; + return { db, deleteWhere }; + } + + it("keeps the tournament and promotes nothing when a non-primary window is deleted", async () => { + const { db } = makeSharedDb({ + deletedEvent: { tournamentId: "tournament-1", isPrimary: false }, + remaining: [{ id: "event-2", isPrimary: true }], + }); + + const result = await deleteScoringEvent("event-1", db); + + expect(result.remainingWindows).toBe(1); + expect(result.promotedPrimaryId).toBeNull(); + expect(result.deletedTournament).toBe(false); + }); + + it("promotes the earliest remaining window when the primary window is deleted", async () => { + const { db } = makeSharedDb({ + deletedEvent: { tournamentId: "tournament-1", isPrimary: true }, + // findMany is ordered asc(createdAt); first is the earliest. + remaining: [ + { id: "event-2", isPrimary: false }, + { id: "event-3", isPrimary: false }, + ], + }); + + const result = await deleteScoringEvent("event-1", db); + + expect(result.promotedPrimaryId).toBe("event-2"); + expect(result.remainingWindows).toBe(2); + }); + + it("leaves the orphaned tournament intact when the last window is deleted without the flag", async () => { + const { db } = makeSharedDb({ + deletedEvent: { tournamentId: "tournament-1", isPrimary: true }, + remaining: [], + }); + + const result = await deleteScoringEvent("event-1", db); + + expect(result.remainingWindows).toBe(0); + expect(result.deletedTournament).toBe(false); + }); + + it("deletes the orphaned tournament when the last window is deleted with the flag", async () => { + const { db } = makeSharedDb({ + deletedEvent: { tournamentId: "tournament-1", isPrimary: true }, + remaining: [], + }); + + const result = await deleteScoringEvent("event-1", db, { + deleteOrphanTournament: true, + }); + + expect(result.deletedTournament).toBe(true); + }); + }); }); diff --git a/app/models/__tests__/tournament.test.ts b/app/models/__tests__/tournament.test.ts index e7f0406..6e5e4f9 100644 --- a/app/models/__tests__/tournament.test.ts +++ b/app/models/__tests__/tournament.test.ts @@ -11,6 +11,7 @@ import { findTournamentBySportNameYear, upsertTournament, updateTournamentStatus, + deleteTournament, } from "../tournament"; import { database } from "~/database/context"; @@ -172,3 +173,26 @@ describe("updateTournamentStatus", () => { expect(result.status).toBe("in_progress"); }); }); + +describe("deleteTournament", () => { + it("deletes the tournament row by id", async () => { + const where = vi.fn().mockResolvedValue(undefined); + const db = { delete: vi.fn().mockReturnValue({ where }) }; + vi.mocked(database).mockReturnValue(db as never); + + await deleteTournament(TOURNAMENT_ID); + + expect(db.delete).toHaveBeenCalledTimes(1); + expect(where).toHaveBeenCalledTimes(1); + }); + + it("uses a provided db when passed (transaction)", async () => { + const where = vi.fn().mockResolvedValue(undefined); + const providedDb = { delete: vi.fn().mockReturnValue({ where }) }; + + await deleteTournament(TOURNAMENT_ID, providedDb as never); + + expect(providedDb.delete).toHaveBeenCalledTimes(1); + expect(vi.mocked(database)).not.toHaveBeenCalled(); + }); +}); diff --git a/app/models/scoring-event.ts b/app/models/scoring-event.ts index dac8156..5ba0811 100644 --- a/app/models/scoring-event.ts +++ b/app/models/scoring-event.ts @@ -5,6 +5,7 @@ import type { BracketRegion } from "~/lib/bracket-templates"; import { recalculateAffectedLeagues } from "./scoring-calculator"; import { hasProcessedQualifyingPlacement, recalculateParticipantQP } from "./qualifying-points"; import { findParticipantNamesByIds } from "./season-participant"; +import { deleteTournament } from "./tournament"; import type { EventType } from "./scoring-event-types"; export { type EventType, getEventTypeLabel } from "./scoring-event-types"; @@ -201,17 +202,46 @@ export async function completeScoringEvent( * sports season (placements are stored there with no FK back to the event) and * recalculates league standings. */ +export interface DeleteScoringEventOptions { + /** + * When this was the last window linked to its shared tournament, also delete + * the now-orphaned canonical tournament (and its results). No-op when other + * windows remain. Off by default so deleting one season's event never touches + * the shared tournament unless explicitly requested. + */ + deleteOrphanTournament?: boolean; +} + +export interface DeleteScoringEventResult { + /** The shared tournament this event was linked to, if any. */ + tournamentId: string | null; + /** How many other windows still link to that tournament after the delete. */ + remainingWindows: number; + /** Event promoted to primary because the deleted event was the primary window. */ + promotedPrimaryId: string | null; + /** Whether the orphaned canonical tournament was deleted. */ + deletedTournament: boolean; +} + export async function deleteScoringEvent( eventId: string, - providedDb?: ReturnType -) { + providedDb?: ReturnType, + opts?: DeleteScoringEventOptions +): Promise { const db = providedDb || database(); const event = await db.query.scoringEvents.findFirst({ where: eq(schema.scoringEvents.id, eventId), }); - if (!event) return; + if (!event) { + return { + tournamentId: null, + remainingWindows: 0, + promotedPrimaryId: null, + deletedTournament: false, + }; + } // For qualifying events: capture affected participant IDs before the cascade deletes // eventResults (which is how we know who had QP awarded from this event). @@ -260,6 +290,41 @@ export async function deleteScoringEvent( await recalculateAffectedLeagues(event.sportsSeasonId, db); } + + // Shared-tournament bookkeeping: keep the primary window valid and optionally + // clean up a canonical tournament left with no windows. + let remainingWindows = 0; + let promotedPrimaryId: string | null = null; + let deletedTournament = false; + + if (event.tournamentId) { + const remaining = await db.query.scoringEvents.findMany({ + where: eq(schema.scoringEvents.tournamentId, event.tournamentId), + orderBy: asc(schema.scoringEvents.createdAt), + }); + remainingWindows = remaining.length; + + if (remaining.length > 0) { + // Auto-heal: if the deleted event was the primary window (or no sibling is + // flagged primary anymore), promote the earliest remaining window so the + // shared major stays scorable and fan-out stays deterministic. + const hasPrimary = remaining.some((e) => e.isPrimary); + if (event.isPrimary || !hasPrimary) { + await setPrimaryEvent(remaining[0].id, db); + promotedPrimaryId = remaining[0].id; + } + } else if (opts?.deleteOrphanTournament) { + await deleteTournament(event.tournamentId, db); + deletedTournament = true; + } + } + + return { + tournamentId: event.tournamentId, + remainingWindows, + promotedPrimaryId, + deletedTournament, + }; } /** diff --git a/app/models/tournament.ts b/app/models/tournament.ts index 2bf7c51..fadd706 100644 --- a/app/models/tournament.ts +++ b/app/models/tournament.ts @@ -136,3 +136,17 @@ export async function updateTournamentStatus( .returning(); return tournament; } + +/** + * Delete a canonical tournament. The DB cascades remove its tournament_results; + * any surviving scoring_events have their tournamentId set to NULL (onDelete: + * "set null"). Callers should only use this once the last linked window has been + * removed, so no scoring events are left pointing at it. + */ +export async function deleteTournament( + id: string, + providedDb?: ReturnType +): Promise { + const db = providedDb || database(); + await db.delete(schema.tournaments).where(eq(schema.tournaments.id, id)); +} diff --git a/app/routes/admin.sports-seasons.$id.events.server.ts b/app/routes/admin.sports-seasons.$id.events.server.ts index d31b256..b284421 100644 --- a/app/routes/admin.sports-seasons.$id.events.server.ts +++ b/app/routes/admin.sports-seasons.$id.events.server.ts @@ -8,6 +8,7 @@ import { deleteScoringEvent, bulkCreateScoringEvents, ensurePrimaryEvent, + getSportsSeasonsByTournament, type CreateScoringEventData, } from "~/models/scoring-event"; import { isBracketMajor } from "~/lib/event-utils"; @@ -59,11 +60,39 @@ export async function loader({ params }: Route.LoaderArgs) { ); const availableTournaments = allTournamentsForSport.filter((t) => !linkedTournamentIds.has(t.id)); + // Shared-tournament context for the delete confirmation dialog: for each event + // linked to a canonical tournament, how many OTHER seasons ("windows") also use + // it, and the tournament's name. Lets the UI explain exactly what a delete does. + const sharedInfo = new Map(); + await Promise.all( + [...linkedTournamentIds].map(async (tid) => { + const [tournament, windows] = await Promise.all([ + allTournamentsForSport.find((t) => t.id === tid) + ? Promise.resolve(allTournamentsForSport.find((t) => t.id === tid)!) + : getTournamentById(tid), + getSportsSeasonsByTournament(tid), + ]); + sharedInfo.set(tid, { + name: tournament?.name ?? "shared tournament", + windowCount: windows.length, + }); + }) + ); + + const eventsWithSharing = events.map((e) => { + const info = e.tournamentId ? sharedInfo.get(e.tournamentId) : null; + return { + ...e, + tournamentName: info?.name ?? null, + otherWindowCount: info ? Math.max(0, info.windowCount - 1) : 0, + }; + }); + return { sportsSeason: sportsSeason as typeof sportsSeason & { sport: { id: string; name: string; type: string; slug: string }; }, - events, + events: eventsWithSharing, qpStandings, scoringRules, availableTournaments, @@ -215,8 +244,19 @@ export async function action({ request, params }: Route.ActionArgs) { } try { - await deleteScoringEvent(eventId); - return { success: "Event deleted successfully" }; + const result = await deleteScoringEvent(eventId, undefined, { + deleteOrphanTournament: formData.get("deleteTournament") === "1", + }); + let success = "Event deleted"; + if (result.deletedTournament) { + success = "Event deleted and its shared tournament removed"; + } else if (result.tournamentId && result.remainingWindows > 0) { + success = `Event removed from this season (shared tournament kept — still used by ${result.remainingWindows} season${result.remainingWindows !== 1 ? "s" : ""})`; + if (result.promotedPrimaryId) { + success += "; another season was promoted to primary"; + } + } + return { success }; } catch (error) { logger.error("Error deleting event:", error); return { error: "Failed to delete event" }; diff --git a/app/routes/admin.sports-seasons.$id.events.tsx b/app/routes/admin.sports-seasons.$id.events.tsx index fd13261..f18b0e6 100644 --- a/app/routes/admin.sports-seasons.$id.events.tsx +++ b/app/routes/admin.sports-seasons.$id.events.tsx @@ -305,7 +305,11 @@ export default function SportsSeasonEvents({

) : (
- {events.map((event: { id: string; name: string; eventType: string; eventDate?: string | null; eventStartsAt?: Date | string | null; isComplete: boolean }) => ( + {events.map((event: { id: string; name: string; eventType: string; eventDate?: string | null; eventStartsAt?: Date | string | null; isComplete: boolean; tournamentId: string | null; isPrimary: boolean; tournamentName: string | null; otherWindowCount: number }) => { + const isLinked = !!event.tournamentId; + const hasOtherWindows = event.otherWindowCount > 0; + const isLastWindow = isLinked && !hasOtherWindows; + return (
@@ -352,28 +356,65 @@ export default function SportsSeasonEvents({ - - Delete "{event.name}"? - - This will also delete all results for this event. This action cannot be undone. - - - - Cancel -
- - + + + + + Delete "{event.name}"? + + {!isLinked ? ( + + This will also delete all results for this event. This action cannot be undone. + + ) : hasOtherWindows ? ( + + This event is part of the shared tournament{" "} + "{event.tournamentName}", also used by{" "} + + {event.otherWindowCount} other season{event.otherWindowCount !== 1 ? "s" : ""} + + . Deleting removes it from this season only — the + tournament and the other seasons keep their data. + {event.isPrimary && ( + <> This is the primary scoring window, so another season will be promoted to primary automatically. + )} + + ) : ( + + This is the only season linked to the shared tournament{" "} + "{event.tournamentName}". Deleting removes this + event and its results. This action cannot be undone. + + )} + + + {isLastWindow && ( + + )} + + Cancel Delete - -
+ +
- ))} + ); + })}
)} diff --git a/app/routes/admin.tournaments.$id.tsx b/app/routes/admin.tournaments.$id.tsx index a7fadf9..5b1c8b2 100644 --- a/app/routes/admin.tournaments.$id.tsx +++ b/app/routes/admin.tournaments.$id.tsx @@ -21,6 +21,7 @@ import { getSportsSeasonsByTournament, getPrimaryEventForTournament, setPrimaryEvent, + deleteScoringEvent, } from "~/models/scoring-event"; import { isBracketMajor } from "~/lib/event-utils"; import { @@ -30,6 +31,17 @@ import { CardHeader, CardTitle, } from "~/components/ui/card"; +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, + AlertDialogTrigger, +} from "~/components/ui/alert-dialog"; import { Badge } from "~/components/ui/badge"; import { Button } from "~/components/ui/button"; import { @@ -214,6 +226,26 @@ export async function action(args: Route.ActionArgs) { } } + if (intent === "unlink-window") { + const eventId = formData.get("eventId"); + if (typeof eventId !== "string" || !eventId) { + return { success: false as const, error: "Event ID is required", syncReport: null }; + } + try { + // Remove that season's scoring event. The tournament stays (we're on its + // page); if this was the primary window, another is auto-promoted. + await deleteScoringEvent(eventId); + return { success: true as const, error: null, syncReport: null }; + } catch (error) { + logger.error("unlink-window failed:", error); + return { + success: false as const, + error: error instanceof Error ? error.message : "Failed to remove season", + syncReport: null, + }; + } + } + return { success: false as const, error: "Invalid intent", @@ -236,6 +268,7 @@ export default function AdminTournamentDetail({ } = loaderData; const retryFetcher = useFetcher(); const primaryFetcher = useFetcher(); + const unlinkFetcher = useFetcher(); // For bracket majors, scoring lives on the primary window's bracket/stage UI. const primaryScoringHref = @@ -419,6 +452,51 @@ export default function AdminTournamentDetail({ > {link.sportsSeason.status} + + + + + + + + Remove {link.sportsSeason.sport.name} - {link.sportsSeason.name}? + + + + This deletes that season's scoring event (and its results) but keeps{" "} + {tournament.name} and the other linked seasons. + {link.isPrimary && linkedSportsSeasons.length > 1 && ( + <> Because this is the primary scoring window, another season will be promoted to primary automatically. + )} + {linkedSportsSeasons.length === 1 && ( + <> This is the only linked season — the tournament will remain but have no windows until you link one again. + )} + + + + + Cancel + + + + + Remove + + + + + ))} -- 2.45.3 From ec89e3eb8918960eb6e38573b75b2034b16b8939 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 2 Jul 2026 05:27:14 +0000 Subject: [PATCH 2/2] Address review: don't auto-promote primary for golf; lighten window count - Auto-heal now only promotes a new primary when the deleted event was itself the primary window. Golf-style shared majors intentionally have no primary (scored on the canonical tournament page), so deleting one of their windows no longer flips a sibling into a primary and changes its scoring/guard behavior. Adds a regression test for that case. - Replace the relation-heavy getSportsSeasonsByTournament + double Array.find in the events loader with a new countWindowsByTournament helper (count(distinct sports_season_id)) and a single cached lookup. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01CBzjCLVkVaQMj1MF3K54t2 --- .../__tests__/scoring-event-delete.test.ts | 18 +++++++++++ app/models/scoring-event.ts | 30 +++++++++++++++---- .../admin.sports-seasons.$id.events.server.ts | 13 ++++---- 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/app/models/__tests__/scoring-event-delete.test.ts b/app/models/__tests__/scoring-event-delete.test.ts index 6e29e84..81e98f7 100644 --- a/app/models/__tests__/scoring-event-delete.test.ts +++ b/app/models/__tests__/scoring-event-delete.test.ts @@ -119,6 +119,24 @@ describe("deleteScoringEvent", () => { expect(result.deletedTournament).toBe(false); }); + it("does not promote a primary when a golf-style tournament (no primary) loses a window", async () => { + // Golf-style shared majors intentionally have no primary window — every + // linked event is isPrimary=false. Deleting one must not flip a sibling + // into a primary, which would change its scoring/guard behavior. + const { db } = makeSharedDb({ + deletedEvent: { tournamentId: "tournament-1", isPrimary: false }, + remaining: [ + { id: "event-2", isPrimary: false }, + { id: "event-3", isPrimary: false }, + ], + }); + + const result = await deleteScoringEvent("event-1", db); + + expect(result.promotedPrimaryId).toBeNull(); + expect(result.remainingWindows).toBe(2); + }); + it("promotes the earliest remaining window when the primary window is deleted", async () => { const { db } = makeSharedDb({ deletedEvent: { tournamentId: "tournament-1", isPrimary: true }, diff --git a/app/models/scoring-event.ts b/app/models/scoring-event.ts index 5ba0811..fd6de2d 100644 --- a/app/models/scoring-event.ts +++ b/app/models/scoring-event.ts @@ -305,11 +305,13 @@ export async function deleteScoringEvent( remainingWindows = remaining.length; if (remaining.length > 0) { - // Auto-heal: if the deleted event was the primary window (or no sibling is - // flagged primary anymore), promote the earliest remaining window so the - // shared major stays scorable and fan-out stays deterministic. - const hasPrimary = remaining.some((e) => e.isPrimary); - if (event.isPrimary || !hasPrimary) { + // Auto-heal: if the deleted event was the primary window, promote the + // earliest remaining window so the shared major stays scorable and + // fan-out stays deterministic. Only heal when we actually removed the + // primary — golf-style majors intentionally have no primary (scored on the + // canonical tournament page), and event.isPrimary is false for them, so + // they are left untouched. + if (event.isPrimary) { await setPrimaryEvent(remaining[0].id, db); promotedPrimaryId = remaining[0].id; } @@ -1126,6 +1128,24 @@ export async function getSportsSeasonsByTournament(tournamentId: string) { }); } +/** + * Count the distinct sports-season windows linked to a tournament. Cheaper than + * getSportsSeasonsByTournament when only the count is needed (no relations). + */ +export async function countWindowsByTournament( + tournamentId: string, + providedDb?: ReturnType +): Promise { + const db = providedDb || database(); + const [row] = await db + .select({ + count: sql`count(distinct ${schema.scoringEvents.sportsSeasonId})::int`, + }) + .from(schema.scoringEvents) + .where(eq(schema.scoringEvents.tournamentId, tournamentId)); + return row?.count ?? 0; +} + /** * The primary scoring event for a tournament — the single window where the admin * builds the bracket/stages and scoring happens; its results fan out to siblings. diff --git a/app/routes/admin.sports-seasons.$id.events.server.ts b/app/routes/admin.sports-seasons.$id.events.server.ts index b284421..90b4968 100644 --- a/app/routes/admin.sports-seasons.$id.events.server.ts +++ b/app/routes/admin.sports-seasons.$id.events.server.ts @@ -8,7 +8,7 @@ import { deleteScoringEvent, bulkCreateScoringEvents, ensurePrimaryEvent, - getSportsSeasonsByTournament, + countWindowsByTournament, type CreateScoringEventData, } from "~/models/scoring-event"; import { isBracketMajor } from "~/lib/event-utils"; @@ -66,15 +66,14 @@ export async function loader({ params }: Route.LoaderArgs) { const sharedInfo = new Map(); await Promise.all( [...linkedTournamentIds].map(async (tid) => { - const [tournament, windows] = await Promise.all([ - allTournamentsForSport.find((t) => t.id === tid) - ? Promise.resolve(allTournamentsForSport.find((t) => t.id === tid)!) - : getTournamentById(tid), - getSportsSeasonsByTournament(tid), + const cached = allTournamentsForSport.find((t) => t.id === tid); + const [tournament, windowCount] = await Promise.all([ + cached ?? getTournamentById(tid), + countWindowsByTournament(tid), ]); sharedInfo.set(tid, { name: tournament?.name ?? "shared tournament", - windowCount: windows.length, + windowCount, }); }) ); -- 2.45.3