fix: league settings sports swap and audit log detail improvements (#294)

- Fix sports seasons not updating when saving league settings. The form
  submits intent="update" but sports were only handled under the dead
  intent="update-sports" branch, which nothing called.

- Fix spurious audit log entries (scoring rules, draft settings) firing
  on every settings save. Change detection now compares submitted values
  against current DB values before adding to seasonUpdates.

- Add previousValues to scoring_rules_changed and draft_settings_changed
  audit log entries so the detail formatter can show old → new diffs
  (e.g. "Scoring updated: 1st: 100 → 105").

- Add sports_changed audit action (migration 0076) with sport names
  resolved at log time. Batch-fetch added sports in one query instead
  of N individual queries.

- Improve audit log display for all action types: field-level old→new
  diffs, readable labels, truncated sport lists (+N more).

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

app/lib/audit-log-display.ts

@@ -1,9 +1,11 @@
+import { format } from "date-fns";
 import type { AuditLogEntry } from "~/models/audit-log";
 
 export const AUDIT_ACTION_LABELS: Record<string, string> = {
   league_settings_changed: "League Settings Changed",
   draft_settings_changed: "Draft Settings Changed",
   scoring_rules_changed: "Scoring Rules Changed",
+  sports_changed: "Sports Changed",
   draft_order_set: "Draft Order Set",
   draft_order_randomized: "Draft Order Randomized",
   draft_started: "Draft Started",
@@ -17,6 +19,89 @@ export const AUDIT_ACTION_LABELS: Record<string, string> = {
   time_bank_edited: "Time Bank Edited",
 };
 
+const SCORING_FIELD_LABELS: Record<string, string> = {
+  pointsFor1st: "1st",
+  pointsFor2nd: "2nd",
+  pointsFor3rd: "3rd",
+  pointsFor4th: "4th",
+  pointsFor5th: "5th",
+  pointsFor6th: "6th",
+  pointsFor7th: "7th",
+  pointsFor8th: "8th",
+};
+
+const DRAFT_FIELD_LABELS: Record<string, string> = {
+  draftRounds: "Rounds",
+  draftDateTime: "Draft date",
+  draftInitialTime: "Initial time",
+  draftIncrementTime: "Increment",
+  draftTimerMode: "Timer mode",
+};
+
+const LEAGUE_FIELD_LABELS: Record<string, string> = {
+  name: "Name",
+  isPublicDraftBoard: "Public draft board",
+  discordWebhookUrl: "Discord webhook",
+};
+
+function formatDuration(seconds: number): string {
+  const m = Math.floor(seconds / 60);
+  const s = seconds % 60;
+  if (m === 0) return `${s}s`;
+  if (s === 0) return `${m}m`;
+  return `${m}m ${s}s`;
+}
+
+function formatDraftField(key: string, value: unknown): string {
+  if (value === null || value === undefined) return "none";
+  if (key === "draftInitialTime" || key === "draftIncrementTime") {
+    return formatDuration(Number(value));
+  }
+  if (key === "draftDateTime") {
+    return format(new Date(value as string), "MMM d, yyyy h:mm a");
+  }
+  if (key === "draftTimerMode") {
+    return value === "chess_clock" ? "Chess clock" : "Standard";
+  }
+  // draftRounds and any other numeric fields render as plain numbers
+  return String(value);
+}
+
+function formatLeagueField(key: string, value: unknown): string {
+  if (value === null || value === undefined || value === "") return "none";
+  if (key === "isPublicDraftBoard") return value ? "Yes" : "No";
+  if (key === "discordWebhookUrl") return "set";
+  return String(value);
+}
+
+function diffLines(
+  fields: string[],
+  prev: Record<string, unknown>,
+  next: Record<string, unknown>,
+  labels: Record<string, string>,
+  formatter: (key: string, value: unknown) => string
+): string {
+  if (fields.length === 0) return "";
+  return fields
+    .map((f) => {
+      const label = labels[f] ?? f;
+      const from = prev[f];
+      const to = next[f];
+      // `from` is undefined when the log entry predates the addition of previousValues —
+      // in that case just show the new value without an arrow.
+      if (from !== undefined) {
+        return `${label}: ${formatter(f, from)} → ${formatter(f, to)}`;
+      }
+      return `${label}: ${formatter(f, to)}`;
+    })
+    .join(", ");
+}
+
+function truncateList(names: string[], max = 3): string {
+  if (names.length <= max) return names.join(", ");
+  return `${names.slice(0, max).join(", ")} +${names.length - max} more`;
+}
+
 function formatSeconds(seconds: number): string {
   const m = Math.floor(Math.abs(seconds) / 60);
   const s = Math.abs(seconds) % 60;
@@ -70,23 +155,6 @@ export function formatAuditDetail(entry: AuditLogEntry): string {
         : "Draft reset";
     }
 
-    case "league_settings_changed": {
-      const fields = (d.changedFields as string[] | undefined) ?? [];
-      return fields.length > 0
-        ? `Changed: ${fields.join(", ")}`
-        : "League settings updated";
-    }
-
-    case "draft_settings_changed": {
-      const fields = (d.changedFields as string[] | undefined) ?? [];
-      return fields.length > 0
-        ? `Changed: ${fields.join(", ")}`
-        : "Draft settings updated";
-    }
-
-    case "scoring_rules_changed":
-      return "Scoring rules updated";
-
     case "draft_started":
       return "Draft started";
 
@@ -100,6 +168,39 @@ export function formatAuditDetail(entry: AuditLogEntry): string {
         ? `Draft resumed at pick #${d.pickNumber}`
         : "Draft resumed";
 
+    case "league_settings_changed": {
+      const fields = (d.changedFields as string[] | undefined) ?? [];
+      const prev = (d.previousValues as Record<string, unknown> | undefined) ?? {};
+      const next = (d.newValues as Record<string, unknown> | undefined) ?? {};
+      const detail = diffLines(fields, prev, next, LEAGUE_FIELD_LABELS, formatLeagueField);
+      return detail || "League settings updated";
+    }
+
+    case "draft_settings_changed": {
+      const fields = (d.changedFields as string[] | undefined) ?? [];
+      const prev = (d.previousValues as Record<string, unknown> | undefined) ?? {};
+      const next = (d.newValues as Record<string, unknown> | undefined) ?? {};
+      const detail = diffLines(fields, prev, next, DRAFT_FIELD_LABELS, formatDraftField);
+      return detail || "Draft settings updated";
+    }
+
+    case "scoring_rules_changed": {
+      const fields = (d.changedFields as string[] | undefined) ?? [];
+      const prev = (d.previousValues as Record<string, unknown> | undefined) ?? {};
+      const next = (d.newValues as Record<string, unknown> | undefined) ?? {};
+      const detail = diffLines(fields, prev, next, SCORING_FIELD_LABELS, (_, v) => String(v ?? ""));
+      return detail ? `Scoring updated: ${detail}` : "Scoring rules updated";
+    }
+
+    case "sports_changed": {
+      const added = (d.added as string[] | undefined) ?? [];
+      const removed = (d.removed as string[] | undefined) ?? [];
+      const parts: string[] = [];
+      if (added.length > 0) parts.push(`Added: ${truncateList(added)}`);
+      if (removed.length > 0) parts.push(`Removed: ${truncateList(removed)}`);
+      return parts.length > 0 ? parts.join(" | ") : "Sports updated";
+    }
+
     default:
       return (entry.action as string).replace(/_/g, " ");
   }

app/models/sports-season.ts

@@ -1,4 +1,4 @@
-import { eq, sql, lte, gte } from "drizzle-orm";
+import { eq, inArray, sql, lte, gte } from "drizzle-orm";
 import { database } from "~/database/context";
 import * as schema from "~/database/schema";
 import { getQPConfig, updateQPConfig } from "~/models/qualifying-points";
@@ -41,6 +41,15 @@ export async function findSportsSeasonById(id: string): Promise<SportsSeasonWith
   }) as SportsSeasonWithSport | undefined;
 }
 
+export async function findSportsSeasonsByIds(ids: string[]): Promise<SportsSeasonWithSport[]> {
+  if (ids.length === 0) return [];
+  const db = database();
+  return await db.query.sportsSeasons.findMany({
+    where: inArray(schema.sportsSeasons.id, ids),
+    with: { sport: true },
+  }) as SportsSeasonWithSport[];
+}
+
 export async function findSportsSeasonsBySportId(sportId: string): Promise<SportsSeason[]> {
   const db = database();
   return await db.query.sportsSeasons.findMany({

app/routes/leagues/$leagueId.audit-log.tsx

@@ -122,7 +122,8 @@ function getActionBadgeVariant(
   if (
     action === "league_settings_changed" ||
     action === "draft_settings_changed" ||
-    action === "scoring_rules_changed"
+    action === "scoring_rules_changed" ||
+    action === "sports_changed"
   ) {
     return "outline";
   }

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

@@ -18,7 +18,7 @@ import { findCurrentSeasonWithSports, updateSeason, type NewSeason } from "~/mod
 import { findTeamsBySeasonId, deleteTeam, removeTeamOwner, claimTeam } from "~/models/team";
 import { findAllUsers, findUserByClerkId, findUsersByClerkIds, getUserDisplayName, isUserAdminByClerkId } from "~/models/user";
 import { unlinkSportFromSeason, linkMultipleSportsToSeason } from "~/models/season-sport";
-import { findDraftableSportsSeasons } from "~/models/sports-season";
+import { findDraftableSportsSeasons, findSportsSeasonsByIds } from "~/models/sports-season";
 import { findDraftSlotsBySeasonId, setDraftOrder, randomizeDraftOrder } from "~/models/draft-slot";
 import { database } from "~/database/context";
 import * as schema from "~/database/schema";
@@ -534,19 +534,13 @@ export async function action(args: Route.ActionArgs) {
     const draftSpeed = formData.get("draftSpeed");
     const draftTimerMode = formData.get("draftTimerMode") as "chess_clock" | "standard" | null;
 
-    // Map draft speed to time values
-    const { draftInitialTime, draftIncrementTime } = parseDraftSpeed(
-      draftSpeed as string | null,
-      draftTimerMode ?? "chess_clock"
-    );
-
     // League-level fields (name, isPublicDraftBoard) are already saved above.
     // Now handle season-specific updates.
     try {
       // Update season settings
       const seasonUpdates: Partial<NewSeason> = {};
 
-      // Handle draft rounds
+      // Handle draft rounds (only if value actually changed)
       if (typeof draftRounds === "string") {
         const draftRoundsNum = parseInt(draftRounds, 10);
         if (!isNaN(draftRoundsNum)) {
@@ -557,28 +551,42 @@ export async function action(args: Route.ActionArgs) {
           if (draftRoundsNum < 1 || draftRoundsNum > 50) {
             return { error: "Draft rounds must be between 1 and 50" };
           }
-          seasonUpdates.draftRounds = draftRoundsNum;
+          if (draftRoundsNum !== season.draftRounds) {
+            seasonUpdates.draftRounds = draftRoundsNum;
+          }
         }
       }
 
-      // Handle draft date/time
+      // Handle draft date/time (only if value actually changed)
       if (typeof draftDateTime === "string") {
-        seasonUpdates.draftDateTime = draftDateTime ? new Date(draftDateTime) : null;
+        const newDateTime = draftDateTime ? new Date(draftDateTime) : null;
+        const currentDateTime = season.draftDateTime ? new Date(season.draftDateTime) : null;
+        const changed = newDateTime?.getTime() !== currentDateTime?.getTime();
+        if (changed) {
+          seasonUpdates.draftDateTime = newDateTime;
+        }
       }
 
-      // Set draft times from speed preset (only if draftSpeed was submitted;
-      // the select is disabled during an active draft so it won't be present)
+      // Set draft times from speed preset (only if draftSpeed was submitted and values changed)
       if (draftSpeed !== null) {
-        seasonUpdates.draftInitialTime = draftInitialTime;
-        seasonUpdates.draftIncrementTime = draftIncrementTime;
+        const { draftInitialTime, draftIncrementTime } = parseDraftSpeed(
+          draftSpeed as string | null,
+          draftTimerMode ?? "chess_clock"
+        );
+        if (draftInitialTime !== season.draftInitialTime) {
+          seasonUpdates.draftInitialTime = draftInitialTime;
+        }
+        if (draftIncrementTime !== season.draftIncrementTime) {
+          seasonUpdates.draftIncrementTime = draftIncrementTime;
+        }
       }
 
-      // Timer mode (only if submitted; disabled during active draft)
-      if (draftTimerMode !== null) {
+      // Timer mode (only if submitted and changed)
+      if (draftTimerMode !== null && draftTimerMode !== season.draftTimerMode) {
         seasonUpdates.draftTimerMode = draftTimerMode;
       }
 
-      // Handle scoring rules (only if in pre_draft status)
+      // Handle scoring rules (only if in pre_draft status and values changed)
       if (season.status === "pre_draft") {
         const scoringFields = [
           "pointsFor1st", "pointsFor2nd", "pointsFor3rd", "pointsFor4th",
@@ -593,7 +601,9 @@ export async function action(args: Route.ActionArgs) {
               if (points < 0 || points > 1000) {
                 return { error: `${field} must be between 0 and 1000 points` };
               }
-              (seasonUpdates as Record<string, unknown>)[field] = points;
+              if (points !== (season as unknown as Record<string, unknown>)[field]) {
+                (seasonUpdates as Record<string, unknown>)[field] = points;
+              }
             }
           }
         }
@@ -608,6 +618,9 @@ export async function action(args: Route.ActionArgs) {
         const draftFields = Object.keys(seasonUpdates).filter((k) => !scoringFields.includes(k));
         const scoringChangedFields = Object.keys(seasonUpdates).filter((k) => scoringFields.includes(k));
 
+        const seasonAsMap = season as unknown as Record<string, unknown>;
+        const updatesAsMap = seasonUpdates as Record<string, unknown>;
+
         if (draftFields.length > 0) {
           await logCommissionerAction({
             seasonId: season.id,
@@ -616,7 +629,8 @@ export async function action(args: Route.ActionArgs) {
             action: "draft_settings_changed",
             details: {
               changedFields: draftFields,
-              newValues: Object.fromEntries(draftFields.map((k) => [k, (seasonUpdates as Record<string, unknown>)[k]])),
+              previousValues: Object.fromEntries(draftFields.map((k) => [k, seasonAsMap[k]])),
+              newValues: Object.fromEntries(draftFields.map((k) => [k, updatesAsMap[k]])),
             },
           });
         }
@@ -629,7 +643,58 @@ export async function action(args: Route.ActionArgs) {
             action: "scoring_rules_changed",
             details: {
               changedFields: scoringChangedFields,
-              newValues: Object.fromEntries(scoringChangedFields.map((k) => [k, (seasonUpdates as Record<string, unknown>)[k]])),
+              previousValues: Object.fromEntries(scoringChangedFields.map((k) => [k, seasonAsMap[k]])),
+              newValues: Object.fromEntries(scoringChangedFields.map((k) => [k, updatesAsMap[k]])),
+            },
+          });
+        }
+      }
+
+      // Handle sports changes (only valid in pre_draft)
+      if (season.status === "pre_draft") {
+        const selectedSports = formData.getAll("sportsSeasons");
+        const currentSportIds = new Set(
+          season.seasonSports?.map((s) => s.sportsSeason.id) || []
+        );
+        const newSportIds = new Set(selectedSports as string[]);
+
+        for (const sportId of currentSportIds) {
+          if (!newSportIds.has(sportId)) {
+            await unlinkSportFromSeason(season.id, sportId);
+          }
+        }
+
+        const sportsToAdd = [];
+        for (const sportId of newSportIds) {
+          if (!currentSportIds.has(sportId)) {
+            sportsToAdd.push({
+              seasonId: season.id,
+              sportsSeasonId: sportId as string,
+            });
+          }
+        }
+
+        if (sportsToAdd.length > 0) {
+          await linkMultipleSportsToSeason(sportsToAdd);
+        }
+
+        const removedIds = [...currentSportIds].filter((id) => !newSportIds.has(id));
+        const addedIds = [...newSportIds].filter((id) => !currentSportIds.has(id));
+        if (removedIds.length > 0 || addedIds.length > 0) {
+          const nameMap = new Map(
+            season.seasonSports?.map((s) => [s.sportsSeason.id, `${s.sportsSeason.sport.name} ${s.sportsSeason.year}`]) ?? []
+          );
+          const addedSeasons = await findSportsSeasonsByIds(addedIds);
+          const addedNameMap = new Map(addedSeasons.map((ss) => [ss.id, `${ss.sport.name} ${ss.year}`]));
+          const addedNames = addedIds.map((id) => addedNameMap.get(id) ?? id);
+          await logCommissionerAction({
+            seasonId: season.id,
+            leagueId,
+            actorClerkId: userId,
+            action: "sports_changed",
+            details: {
+              added: addedNames,
+              removed: removedIds.map((id) => nameMap.get(id) ?? id),
             },
           });
         }

database/schema.ts

@@ -1253,6 +1253,7 @@ export const auditActionEnum = pgEnum("audit_action", [
   "league_settings_changed",
   "draft_settings_changed",
   "scoring_rules_changed",
+  "sports_changed",
   "draft_order_set",
   "draft_order_randomized",
   "draft_started",

drizzle/0076_heavy_zemo.sql

@@ -0,0 +1 @@
+ALTER TYPE "public"."audit_action" ADD VALUE 'sports_changed' BEFORE 'draft_order_set';

drizzle/meta/_journal.json

@@ -533,6 +533,13 @@
       "when": 1776101579459,
       "tag": "0075_chubby_stephen_strange",
       "breakpoints": true
+    },
+    {
+      "idx": 76,
+      "version": "7",
+      "when": 1776226761564,
+      "tag": "0076_heavy_zemo",
+      "breakpoints": true
     }
   ]
 }