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

app/lib/__tests__/draft-order.test.ts

@@ -1,5 +1,29 @@
 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) {
   return Array.from({ length: teamCount }, (_, i) => ({

app/lib/draft-order.ts

@@ -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.
  */

app/models/draft-slot.ts

@@ -133,10 +133,7 @@ export async function setDraftOrder(
   return await createManyDraftSlots(slots);
 }
 
-/**
- * Get the number of teams (draft slots) in a season
- */
-export async function getNumTeamsInSeason(seasonId: string): Promise<number> {
+export async function getNumDraftSlotsBySeasonId(seasonId: string): Promise<number> {
   const db = database();
   const [result] = await db
     .select({ count: count() })

app/routes/leagues/$leagueId.settings.server.ts

@@ -751,10 +751,8 @@ export async function action(args: Route.ActionArgs) {
                 .insert(schema.teams)
                 .values(teamsToAdd)
                 .returning();
-              // Only append draft slots if a draft order was already set for existing teams.
-              // When existingSlots is empty, draft order was never configured — don't create
-              // partial slots for only the new teams (that would break the order UI).
-              if (existingSlots.length > 0) {
+              // Only append slots when all existing teams already have one (order fully set).
+              if (existingSlots.length === currentTeamCount) {
                 await tx.insert(schema.draftSlots).values(
                   newTeams.map((team, i) => ({
                     seasonId: season.id,

app/routes/leagues/$leagueId.settings.tsx

@@ -13,6 +13,7 @@ import {
 } from "lucide-react";
 import { type ScoringRules, DEFAULT_SCORING_RULES } from "~/lib/scoring-types";
 import { getInitialDraftSpeed } from "~/lib/draft-timer";
+import { buildDraftOrderTeams } from "~/lib/draft-order";
 import type { Route } from "./+types/$leagueId.settings";
 export { action, loader } from "./$leagueId.settings.server";
 import { OMNIFANTASY_SCORING } from "~/components/league/ScoringPresetPicker";
@@ -112,16 +113,6 @@ const UPDATE_SECTIONS: SettingsSectionId[] = [
   "notifications",
 ];
 
-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];
-}
-
 export default function LeagueSettings({ loaderData, actionData }: Route.ComponentProps) {
   const {
     league, season, teams, teamCount, teamsWithOwners, allSportsSeasons,

app/routes/leagues/$leagueId.standings.$seasonId.teams.$teamId.tsx

@@ -3,7 +3,7 @@ import type { Route } from "./+types/$leagueId.standings.$seasonId.teams.$teamId
 
 import { TeamScoreBreakdown } from "~/components/standings/TeamScoreBreakdown";
 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 {
   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([
     getTeamScoreBreakdown(teamId, seasonId),
     getTeamStanding(teamId, seasonId),
-    getNumTeamsInSeason(seasonId),
+    getNumDraftSlotsBySeasonId(seasonId),
   ]);
 
   if (!breakdown) {

app/routes/leagues/__tests__/settings-update.test.ts

@@ -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
+// ---------------------------------------------------------------------------
+
+describe('Settings Update - Draft Slot Creation Guard', () => {
+  function shouldAppendDraftSlots(existingSlotsCount: number, currentTeamCount: number): boolean {
+    return existingSlotsCount === currentTeamCount;
+  }
+
+  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
 // ---------------------------------------------------------------------------