Fix draft order initialization for teams added mid-season (#449)
* Fix draft order showing only new team when league size increased before order was set When a commissioner increased the team count on a league that had teams but no draft order set yet, the server created draft slots only for the newly-added teams. This left the DB with N+K teams but only K slots, causing the drag-and-drop list to display only the K new teams. Two fixes: 1. Server: only append new draft slots when an order was already set (existingSlots.length > 0). If no order exists yet, skip slot creation so the page correctly treats the order as unset for all teams. 2. Frontend: buildDraftOrderTeams() appends any unslotted teams after the slotted ones, so a partially-corrupt DB state still shows all teams. https://claude.ai/code/session_01M3H55gnMxRztJK9KMXqqZo * Address code review feedback on draft order bug fix - Move buildDraftOrderTeams to app/lib/draft-order.ts so it is importable and testable; remove the local copy from the settings component - Add unit tests for buildDraftOrderTeams covering the empty, full, and partial-slot cases - Tighten the draft slot guard from existingSlots.length > 0 to existingSlots.length === currentTeamCount so partial legacy states are treated the same as "order not set" - Condense the 3-line server comment to a single line per project style - Rename getNumTeamsInSeason → getNumDraftSlotsBySeasonId to reflect what the function actually counts, and update its one call site - Add tests for the server-side slot-creation guard logic https://claude.ai/code/session_01M3H55gnMxRztJK9KMXqqZo * Fix lint: move shouldAppendDraftSlots to outer scope oxlint (consistent-function-scoping) requires functions that don't close over any variables to be defined at the outer scope rather than inside a describe block. https://claude.ai/code/session_01M3H55gnMxRztJK9KMXqqZo --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
parent
06d415d95c
commit
0ba2b391f8
7 changed files with 94 additions and 19 deletions
|
|
@ -1,5 +1,29 @@
|
||||||
import { describe, it, expect } from "vitest";
|
import { describe, it, expect } from "vitest";
|
||||||
import { getTeamForPick, getProjectedPicks } from "~/lib/draft-order";
|
import { buildDraftOrderTeams, getTeamForPick, getProjectedPicks } from "~/lib/draft-order";
|
||||||
|
|
||||||
|
describe("buildDraftOrderTeams", () => {
|
||||||
|
const teams = [
|
||||||
|
{ id: "t1" }, { id: "t2" }, { id: "t3" },
|
||||||
|
];
|
||||||
|
|
||||||
|
it("returns all teams in natural order when no slots exist", () => {
|
||||||
|
expect(buildDraftOrderTeams([], teams)).toEqual(["t1", "t2", "t3"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns slot order when all teams are slotted", () => {
|
||||||
|
const slots = [{ teamId: "t3" }, { teamId: "t1" }, { teamId: "t2" }];
|
||||||
|
expect(buildDraftOrderTeams(slots, teams)).toEqual(["t3", "t1", "t2"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("appends unslotted teams after slotted ones in partial state", () => {
|
||||||
|
const slots = [{ teamId: "t2" }];
|
||||||
|
expect(buildDraftOrderTeams(slots, teams)).toEqual(["t2", "t1", "t3"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns empty array when both slots and teams are empty", () => {
|
||||||
|
expect(buildDraftOrderTeams([], [])).toEqual([]);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
function makeSlots(teamCount: number) {
|
function makeSlots(teamCount: number) {
|
||||||
return Array.from({ length: teamCount }, (_, i) => ({
|
return Array.from({ length: teamCount }, (_, i) => ({
|
||||||
|
|
|
||||||
|
|
@ -1,3 +1,18 @@
|
||||||
|
/**
|
||||||
|
* Builds the ordered team-ID list for the draft order UI.
|
||||||
|
* Slotted teams come first (in their configured order); any teams without a
|
||||||
|
* slot are appended at the end so all teams are always visible.
|
||||||
|
*/
|
||||||
|
export function buildDraftOrderTeams(
|
||||||
|
slots: { teamId: string }[],
|
||||||
|
allTeams: { id: string }[]
|
||||||
|
): string[] {
|
||||||
|
if (slots.length === 0) return allTeams.map((t) => t.id);
|
||||||
|
const slottedIds = new Set(slots.map((s) => s.teamId));
|
||||||
|
const unslotted = allTeams.filter((t) => !slottedIds.has(t.id)).map((t) => t.id);
|
||||||
|
return [...slots.map((s) => s.teamId), ...unslotted];
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Returns the draft slot for a given pick number in a snake draft.
|
* Returns the draft slot for a given pick number in a snake draft.
|
||||||
*/
|
*/
|
||||||
|
|
|
||||||
|
|
@ -133,10 +133,7 @@ export async function setDraftOrder(
|
||||||
return await createManyDraftSlots(slots);
|
return await createManyDraftSlots(slots);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
export async function getNumDraftSlotsBySeasonId(seasonId: string): Promise<number> {
|
||||||
* Get the number of teams (draft slots) in a season
|
|
||||||
*/
|
|
||||||
export async function getNumTeamsInSeason(seasonId: string): Promise<number> {
|
|
||||||
const db = database();
|
const db = database();
|
||||||
const [result] = await db
|
const [result] = await db
|
||||||
.select({ count: count() })
|
.select({ count: count() })
|
||||||
|
|
|
||||||
|
|
@ -751,13 +751,16 @@ export async function action(args: Route.ActionArgs) {
|
||||||
.insert(schema.teams)
|
.insert(schema.teams)
|
||||||
.values(teamsToAdd)
|
.values(teamsToAdd)
|
||||||
.returning();
|
.returning();
|
||||||
await tx.insert(schema.draftSlots).values(
|
// Only append slots when all existing teams already have one (order fully set).
|
||||||
newTeams.map((team, i) => ({
|
if (existingSlots.length === currentTeamCount) {
|
||||||
seasonId: season.id,
|
await tx.insert(schema.draftSlots).values(
|
||||||
teamId: team.id,
|
newTeams.map((team, i) => ({
|
||||||
draftOrder: maxOrder + i + 1,
|
seasonId: season.id,
|
||||||
}))
|
teamId: team.id,
|
||||||
);
|
draftOrder: maxOrder + i + 1,
|
||||||
|
}))
|
||||||
|
);
|
||||||
|
}
|
||||||
});
|
});
|
||||||
await syncPrivateBracktParticipants(season.id);
|
await syncPrivateBracktParticipants(season.id);
|
||||||
} else if (newTeamCount < currentTeamCount) {
|
} else if (newTeamCount < currentTeamCount) {
|
||||||
|
|
|
||||||
|
|
@ -13,6 +13,7 @@ import {
|
||||||
} from "lucide-react";
|
} from "lucide-react";
|
||||||
import { type ScoringRules, DEFAULT_SCORING_RULES } from "~/lib/scoring-types";
|
import { type ScoringRules, DEFAULT_SCORING_RULES } from "~/lib/scoring-types";
|
||||||
import { getInitialDraftSpeed } from "~/lib/draft-timer";
|
import { getInitialDraftSpeed } from "~/lib/draft-timer";
|
||||||
|
import { buildDraftOrderTeams } from "~/lib/draft-order";
|
||||||
import type { Route } from "./+types/$leagueId.settings";
|
import type { Route } from "./+types/$leagueId.settings";
|
||||||
export { action, loader } from "./$leagueId.settings.server";
|
export { action, loader } from "./$leagueId.settings.server";
|
||||||
import { OMNIFANTASY_SCORING } from "~/components/league/ScoringPresetPicker";
|
import { OMNIFANTASY_SCORING } from "~/components/league/ScoringPresetPicker";
|
||||||
|
|
@ -165,17 +166,14 @@ export default function LeagueSettings({ loaderData, actionData }: Route.Compone
|
||||||
|
|
||||||
// Draft order state (separate form, separate dirty flag)
|
// Draft order state (separate form, separate dirty flag)
|
||||||
const [draftOrderTeams, setDraftOrderTeams] = useState<string[]>(
|
const [draftOrderTeams, setDraftOrderTeams] = useState<string[]>(
|
||||||
draftSlots.length > 0 ? draftSlots.map((slot) => slot.teamId) : teams.map((team) => team.id)
|
() => buildDraftOrderTeams(draftSlots, teams)
|
||||||
);
|
);
|
||||||
|
|
||||||
const [copied, setCopied] = useState(false);
|
const [copied, setCopied] = useState(false);
|
||||||
|
|
||||||
// Sync draft order from loader after randomization or other server updates
|
// Sync draft order from loader after randomization or other server updates
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
const savedOrder = draftSlots.length > 0
|
setDraftOrderTeams(buildDraftOrderTeams(draftSlots, teams));
|
||||||
? draftSlots.map((slot) => slot.teamId)
|
|
||||||
: teams.map((team) => team.id);
|
|
||||||
setDraftOrderTeams(savedOrder);
|
|
||||||
setHasDraftOrderChanges(false);
|
setHasDraftOrderChanges(false);
|
||||||
}, [draftSlots, teams]);
|
}, [draftSlots, teams]);
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -3,7 +3,7 @@ import type { Route } from "./+types/$leagueId.standings.$seasonId.teams.$teamId
|
||||||
|
|
||||||
import { TeamScoreBreakdown } from "~/components/standings/TeamScoreBreakdown";
|
import { TeamScoreBreakdown } from "~/components/standings/TeamScoreBreakdown";
|
||||||
import { getTeamScoreBreakdown, getTeamStanding } from "~/models/standings";
|
import { getTeamScoreBreakdown, getTeamStanding } from "~/models/standings";
|
||||||
import { getNumTeamsInSeason } from "~/models/draft-slot";
|
import { getNumDraftSlotsBySeasonId } from "~/models/draft-slot";
|
||||||
|
|
||||||
export function meta({ data }: Route.MetaArgs): Route.MetaDescriptors {
|
export function meta({ data }: Route.MetaArgs): Route.MetaDescriptors {
|
||||||
return [{ title: `${data?.breakdown?.team?.name ?? "Team"} - Brackt` }];
|
return [{ title: `${data?.breakdown?.team?.name ?? "Team"} - Brackt` }];
|
||||||
|
|
@ -21,7 +21,7 @@ export async function loader({ params }: Route.LoaderArgs) {
|
||||||
const [breakdown, standing, numTeams] = await Promise.all([
|
const [breakdown, standing, numTeams] = await Promise.all([
|
||||||
getTeamScoreBreakdown(teamId, seasonId),
|
getTeamScoreBreakdown(teamId, seasonId),
|
||||||
getTeamStanding(teamId, seasonId),
|
getTeamStanding(teamId, seasonId),
|
||||||
getNumTeamsInSeason(seasonId),
|
getNumDraftSlotsBySeasonId(seasonId),
|
||||||
]);
|
]);
|
||||||
|
|
||||||
if (!breakdown) {
|
if (!breakdown) {
|
||||||
|
|
|
||||||
|
|
@ -458,6 +458,44 @@ describe('Settings Update - Team Count Changes', () => {
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// Draft slot guard when increasing team count
|
||||||
|
// Mirrors the server action condition: existingSlots.length === currentTeamCount
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
function shouldAppendDraftSlots(existingSlotsCount: number, currentTeamCount: number): boolean {
|
||||||
|
return existingSlotsCount === currentTeamCount;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('Settings Update - Draft Slot Creation Guard', () => {
|
||||||
|
it('does not create slots when no draft order was ever set', () => {
|
||||||
|
expect(shouldAppendDraftSlots(0, 12)).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('creates slots when draft order is fully configured', () => {
|
||||||
|
expect(shouldAppendDraftSlots(12, 12)).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not create slots when existing slots are partial (legacy edge case)', () => {
|
||||||
|
expect(shouldAppendDraftSlots(8, 12)).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('appends new slots at consecutive positions after the current max', () => {
|
||||||
|
const existingSlots = [{ draftOrder: 1 }, { draftOrder: 2 }, { draftOrder: 3 }];
|
||||||
|
const maxOrder = existingSlots.reduce((max, s) => Math.max(max, s.draftOrder), 0);
|
||||||
|
const newTeams = [{ id: 'team-4' }, { id: 'team-5' }];
|
||||||
|
const newSlots = newTeams.map((team, i) => ({
|
||||||
|
seasonId: 'season-1',
|
||||||
|
teamId: team.id,
|
||||||
|
draftOrder: maxOrder + i + 1,
|
||||||
|
}));
|
||||||
|
expect(newSlots).toEqual([
|
||||||
|
{ seasonId: 'season-1', teamId: 'team-4', draftOrder: 4 },
|
||||||
|
{ seasonId: 'season-1', teamId: 'team-5', draftOrder: 5 },
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
// Draft order action responses
|
// Draft order action responses
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue