From 77f0bbd50683d9622c4706ab838b11821a4d5ed4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 05:12:08 +0000 Subject: [PATCH 1/3] refactor(schedule): add phaseInputFromEdition adapter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consolidates the edition-row -> phase-input defaulting policy that was independently reconstructed at each festival-phase call site into one adapter, per UPL-17. The admin settings call site keeps calling getFestivalPhase (not the override-aware wrapper) for its "Automatic (...)" label, since that label must reflect what automatic derivation would produce even when an override is active — the override is already shown separately as the control's selected value. Current code reads this correctly, so the "silently ignores override" bug described in the ticket's brief does not reproduce on this call site; only the reconstruction itself was duplicated, which the adapter now owns. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01QBenrJ8YAv3Zk81uWHvSLi --- src/hooks/useFestivalPhase.ts | 14 ++--- src/lib/festivalPhase.test.ts | 56 +++++++++++++++++++ src/lib/festivalPhase.ts | 30 ++++++++++ src/lib/nowView.ts | 14 ++--- .../editions/$editionSlug/settings.tsx | 19 ++++--- .../$festivalSlug/editions/$editionSlug.tsx | 18 +++--- 6 files changed, 112 insertions(+), 39 deletions(-) diff --git a/src/hooks/useFestivalPhase.ts b/src/hooks/useFestivalPhase.ts index ea7158bb5..ae4e19c54 100644 --- a/src/hooks/useFestivalPhase.ts +++ b/src/hooks/useFestivalPhase.ts @@ -2,6 +2,7 @@ import { useRouteContext } from "@tanstack/react-router"; import { type FestivalPhase, getEffectiveFestivalPhase, + phaseInputFromEdition, } from "@/lib/festivalPhase"; export function useFestivalPhase(): { phase: FestivalPhase } { @@ -9,16 +10,9 @@ export function useFestivalPhase(): { phase: FestivalPhase } { from: "/festivals/$festivalSlug/editions/$editionSlug", }); - const phase = getEffectiveFestivalPhase({ - override: edition.phase_override, - derivedInput: { - revealLevel: edition.schedule_reveal_level ?? "draft", - startDate: edition.start_date, - endDate: edition.end_date, - timezone: festival.timezone, - now: new Date(), - }, - }); + const phase = getEffectiveFestivalPhase( + phaseInputFromEdition(edition, festival.timezone, new Date()), + ); return { phase }; } diff --git a/src/lib/festivalPhase.test.ts b/src/lib/festivalPhase.test.ts index cea885f38..cb3b0d593 100644 --- a/src/lib/festivalPhase.test.ts +++ b/src/lib/festivalPhase.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { getEffectiveFestivalPhase, getFestivalPhase, + phaseInputFromEdition, type FestivalPhase, type FestivalPhaseInput, } from "./festivalPhase"; @@ -161,3 +162,58 @@ describe("getEffectiveFestivalPhase", () => { }); } }); + +describe("phaseInputFromEdition", () => { + const NOW = new Date(LIVE_START); + + it("passes through a fully-populated edition", () => { + expect( + phaseInputFromEdition( + { + schedule_reveal_level: "full", + start_date: "2025-08-01", + end_date: "2025-08-03", + phase_override: "live", + }, + TZ, + NOW, + ), + ).toEqual({ + override: "live", + derivedInput: { + revealLevel: "full", + startDate: "2025-08-01", + endDate: "2025-08-03", + timezone: TZ, + now: NOW, + }, + }); + }); + + it("defaults a missing reveal level to draft", () => { + expect( + phaseInputFromEdition( + { start_date: null, end_date: null, phase_override: null }, + TZ, + NOW, + ).derivedInput.revealLevel, + ).toBe("draft"); + }); + + it("defaults missing dates to null", () => { + const { derivedInput } = phaseInputFromEdition( + { schedule_reveal_level: "full", phase_override: null }, + TZ, + NOW, + ); + expect(derivedInput.startDate).toBeNull(); + expect(derivedInput.endDate).toBeNull(); + }); + + it("defaults a missing override to null", () => { + expect( + phaseInputFromEdition({ schedule_reveal_level: "full" }, TZ, NOW) + .override, + ).toBeNull(); + }); +}); diff --git a/src/lib/festivalPhase.ts b/src/lib/festivalPhase.ts index 9b640d7f9..9d73cee84 100644 --- a/src/lib/festivalPhase.ts +++ b/src/lib/festivalPhase.ts @@ -32,6 +32,36 @@ export function getEffectiveFestivalPhase({ return override ?? getFestivalPhase(derivedInput); } +// Minimal edition-like shape phaseInputFromEdition needs — not a full +// FestivalEdition row — so callers with a partially-loaded or differently +// shaped edition can still build phase input. +export type PhaseInputEdition = { + schedule_reveal_level?: RevealLevel | null; + start_date?: string | null; + end_date?: string | null; + phase_override?: FestivalPhase | null; +}; + +// The one place that owns the edition-row -> phase-input defaulting policy +// (missing reveal level -> "draft", missing dates -> null), so callers never +// re-decide it independently. +export function phaseInputFromEdition( + edition: PhaseInputEdition, + timezone: string, + now: Date, +): GetEffectiveFestivalPhaseInput { + return { + override: edition.phase_override ?? null, + derivedInput: { + revealLevel: edition.schedule_reveal_level ?? "draft", + startDate: edition.start_date ?? null, + endDate: edition.end_date ?? null, + timezone, + now, + }, + }; +} + export function getFestivalPhase({ revealLevel, startDate, diff --git a/src/lib/nowView.ts b/src/lib/nowView.ts index a4d554f4c..fb3e0c81a 100644 --- a/src/lib/nowView.ts +++ b/src/lib/nowView.ts @@ -1,6 +1,7 @@ import { type FestivalPhase, getEffectiveFestivalPhase, + phaseInputFromEdition, } from "@/lib/festivalPhase"; import { canShowTime, type RevealLevel } from "@/lib/scheduleReveal"; @@ -23,15 +24,8 @@ export function canShowNowView( timezone: string, now: Date, ): boolean { - const phase = getEffectiveFestivalPhase({ - override: edition.phase_override, - derivedInput: { - revealLevel: edition.schedule_reveal_level, - startDate: edition.start_date, - endDate: edition.end_date, - timezone, - now, - }, - }); + const phase = getEffectiveFestivalPhase( + phaseInputFromEdition(edition, timezone, now), + ); return phase === "live" && canShowTime(edition.schedule_reveal_level); } diff --git a/src/routes/admin/festivals/$festivalSlug/editions/$editionSlug/settings.tsx b/src/routes/admin/festivals/$festivalSlug/editions/$editionSlug/settings.tsx index c9eb78302..26bd01abb 100644 --- a/src/routes/admin/festivals/$festivalSlug/editions/$editionSlug/settings.tsx +++ b/src/routes/admin/festivals/$festivalSlug/editions/$editionSlug/settings.tsx @@ -3,7 +3,7 @@ import { useSuspenseQuery } from "@tanstack/react-query"; import { Card, CardContent } from "@/components/ui/card"; import { editionBySlugQuery } from "@/api/editions/useFestivalEditionBySlug"; import { festivalBySlugQuery } from "@/api/festivals/useFestivalBySlug"; -import { getFestivalPhase } from "@/lib/festivalPhase"; +import { getFestivalPhase, phaseInputFromEdition } from "@/lib/festivalPhase"; import { ScheduleRevealControl } from "@/pages/admin/festivals/ScheduleRevealControl"; import { PhaseOverrideControl } from "@/pages/admin/festivals/PhaseOverrideControl"; import { pageMeta } from "@/lib/pageHead"; @@ -29,13 +29,16 @@ function FestivalEditionSettings() { editionBySlugQuery({ festivalId: festival.id, editionSlug }), ); - const derivedPhase = getFestivalPhase({ - revealLevel: currentEdition.schedule_reveal_level ?? "draft", - startDate: currentEdition.start_date, - endDate: currentEdition.end_date, - timezone: festival.timezone, - now: new Date(), - }); + // Deliberately the derived phase, not the override-aware one: this feeds + // the "Automatic (...)" option label below, which must show what automatic + // derivation would currently produce even when an override is active — the + // override itself is read separately as the control's selected value. + const { derivedInput } = phaseInputFromEdition( + currentEdition, + festival.timezone, + new Date(), + ); + const derivedPhase = getFestivalPhase(derivedInput); return ( diff --git a/src/routes/festivals/$festivalSlug/editions/$editionSlug.tsx b/src/routes/festivals/$festivalSlug/editions/$editionSlug.tsx index 471379229..ef22dd11f 100644 --- a/src/routes/festivals/$festivalSlug/editions/$editionSlug.tsx +++ b/src/routes/festivals/$festivalSlug/editions/$editionSlug.tsx @@ -11,7 +11,10 @@ import ErrorBoundary from "@/components/ErrorBoundary"; import { editionBySlugQuery } from "@/api/editions/useFestivalEditionBySlug"; import { stagesByEditionQuery } from "@/api/stages/useStagesByEdition"; import { stagesKeys } from "@/api/stages/types"; -import { getEffectiveFestivalPhase } from "@/lib/festivalPhase"; +import { + getEffectiveFestivalPhase, + phaseInputFromEdition, +} from "@/lib/festivalPhase"; import { getDefaultTab } from "@/pages/EditionView/TabNavigation/defaultTab"; import { tabRoutes } from "@/pages/EditionView/TabNavigation/tabRoutes"; import { pageMeta } from "@/lib/pageHead"; @@ -42,16 +45,9 @@ export const Route = createFileRoute( location.pathname === basePath || location.pathname === `${basePath}/` ) { - const phase = getEffectiveFestivalPhase({ - override: edition.phase_override, - derivedInput: { - revealLevel: edition.schedule_reveal_level, - startDate: edition.start_date, - endDate: edition.end_date, - timezone: context.festival.timezone, - now: new Date(), - }, - }); + const phase = getEffectiveFestivalPhase( + phaseInputFromEdition(edition, context.festival.timezone, new Date()), + ); throw redirect({ to: tabRoutes[getDefaultTab(phase)], From cba560558c0ab309a3ba193ff692d61f6ede9579 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 05:14:57 +0000 Subject: [PATCH 2/3] refactor(schedule): address self-review on phase adapter Use JSDoc blocks for the new adapter's comments (CLAUDE.md comment policy) and move it below getFestivalPhase to match the repo's implementation-first file ordering. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01QBenrJ8YAv3Zk81uWHvSLi --- src/lib/festivalPhase.ts | 60 +++++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/src/lib/festivalPhase.ts b/src/lib/festivalPhase.ts index 9d73cee84..fc9230beb 100644 --- a/src/lib/festivalPhase.ts +++ b/src/lib/festivalPhase.ts @@ -32,9 +32,33 @@ export function getEffectiveFestivalPhase({ return override ?? getFestivalPhase(derivedInput); } -// Minimal edition-like shape phaseInputFromEdition needs — not a full -// FestivalEdition row — so callers with a partially-loaded or differently -// shaped edition can still build phase input. +export function getFestivalPhase({ + revealLevel, + startDate, + endDate, + timezone, + now, +}: FestivalPhaseInput): FestivalPhase { + if (revealLevel === "draft") return "pre-schedule"; + + const liveStart = startDate + ? zonedInstant(shiftDayKey(startDate, -1), "00:00:00", timezone) + : null; + if (!liveStart || now.getTime() < liveStart.getTime()) return "planning"; + + const liveEnd = endDate + ? zonedInstant(shiftDayKey(endDate, 1), "06:00:00", timezone) + : null; + if (!liveEnd || now.getTime() <= liveEnd.getTime()) return "live"; + + return "post-festival"; +} + +/** + * Minimal edition-like shape {@link phaseInputFromEdition} needs — not a full + * FestivalEdition row — so callers with a partially-loaded or differently + * shaped edition can still build phase input. + */ export type PhaseInputEdition = { schedule_reveal_level?: RevealLevel | null; start_date?: string | null; @@ -42,9 +66,11 @@ export type PhaseInputEdition = { phase_override?: FestivalPhase | null; }; -// The one place that owns the edition-row -> phase-input defaulting policy -// (missing reveal level -> "draft", missing dates -> null), so callers never -// re-decide it independently. +/** + * The one place that owns the edition-row -> phase-input defaulting policy + * (missing reveal level -> "draft", missing dates -> null), so callers never + * re-decide it independently. + */ export function phaseInputFromEdition( edition: PhaseInputEdition, timezone: string, @@ -62,28 +88,6 @@ export function phaseInputFromEdition( }; } -export function getFestivalPhase({ - revealLevel, - startDate, - endDate, - timezone, - now, -}: FestivalPhaseInput): FestivalPhase { - if (revealLevel === "draft") return "pre-schedule"; - - const liveStart = startDate - ? zonedInstant(shiftDayKey(startDate, -1), "00:00:00", timezone) - : null; - if (!liveStart || now.getTime() < liveStart.getTime()) return "planning"; - - const liveEnd = endDate - ? zonedInstant(shiftDayKey(endDate, 1), "06:00:00", timezone) - : null; - if (!liveEnd || now.getTime() <= liveEnd.getTime()) return "live"; - - return "post-festival"; -} - // Shift a yyyy-MM-dd calendar day by whole days, staying a yyyy-MM-dd string. // Returns null for an unparseable date so callers degrade instead of throwing. function shiftDayKey(dateKey: string, delta: number): string | null { From ead17c50182b54b8a1d9a0ab13743aab438c45d6 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 1 Oct 2026 15:10:06 +0000 Subject: [PATCH 3/3] refactor(schedule): take a single options object in phaseInputFromEdition Addresses review feedback on PR #516: phaseInputFromEdition now takes { edition, timezone, now } instead of three positional args, and the settings.tsx comment explaining the deliberate non-override-aware derivedPhase is compacted to one line. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01QBenrJ8YAv3Zk81uWHvSLi --- src/hooks/useFestivalPhase.ts | 6 ++- src/lib/festivalPhase.test.ts | 37 ++++++++++--------- src/lib/festivalPhase.ts | 14 ++++--- src/lib/nowView.ts | 2 +- .../editions/$editionSlug/settings.tsx | 15 +++----- .../$festivalSlug/editions/$editionSlug.tsx | 6 ++- 6 files changed, 46 insertions(+), 34 deletions(-) diff --git a/src/hooks/useFestivalPhase.ts b/src/hooks/useFestivalPhase.ts index ae4e19c54..989e535b0 100644 --- a/src/hooks/useFestivalPhase.ts +++ b/src/hooks/useFestivalPhase.ts @@ -11,7 +11,11 @@ export function useFestivalPhase(): { phase: FestivalPhase } { }); const phase = getEffectiveFestivalPhase( - phaseInputFromEdition(edition, festival.timezone, new Date()), + phaseInputFromEdition({ + edition, + timezone: festival.timezone, + now: new Date(), + }), ); return { phase }; diff --git a/src/lib/festivalPhase.test.ts b/src/lib/festivalPhase.test.ts index cb3b0d593..b3ed11e77 100644 --- a/src/lib/festivalPhase.test.ts +++ b/src/lib/festivalPhase.test.ts @@ -168,16 +168,16 @@ describe("phaseInputFromEdition", () => { it("passes through a fully-populated edition", () => { expect( - phaseInputFromEdition( - { + phaseInputFromEdition({ + edition: { schedule_reveal_level: "full", start_date: "2025-08-01", end_date: "2025-08-03", phase_override: "live", }, - TZ, - NOW, - ), + timezone: TZ, + now: NOW, + }), ).toEqual({ override: "live", derivedInput: { @@ -192,28 +192,31 @@ describe("phaseInputFromEdition", () => { it("defaults a missing reveal level to draft", () => { expect( - phaseInputFromEdition( - { start_date: null, end_date: null, phase_override: null }, - TZ, - NOW, - ).derivedInput.revealLevel, + phaseInputFromEdition({ + edition: { start_date: null, end_date: null, phase_override: null }, + timezone: TZ, + now: NOW, + }).derivedInput.revealLevel, ).toBe("draft"); }); it("defaults missing dates to null", () => { - const { derivedInput } = phaseInputFromEdition( - { schedule_reveal_level: "full", phase_override: null }, - TZ, - NOW, - ); + const { derivedInput } = phaseInputFromEdition({ + edition: { schedule_reveal_level: "full", phase_override: null }, + timezone: TZ, + now: NOW, + }); expect(derivedInput.startDate).toBeNull(); expect(derivedInput.endDate).toBeNull(); }); it("defaults a missing override to null", () => { expect( - phaseInputFromEdition({ schedule_reveal_level: "full" }, TZ, NOW) - .override, + phaseInputFromEdition({ + edition: { schedule_reveal_level: "full" }, + timezone: TZ, + now: NOW, + }).override, ).toBeNull(); }); }); diff --git a/src/lib/festivalPhase.ts b/src/lib/festivalPhase.ts index fc9230beb..d4dec3bef 100644 --- a/src/lib/festivalPhase.ts +++ b/src/lib/festivalPhase.ts @@ -71,11 +71,15 @@ export type PhaseInputEdition = { * (missing reveal level -> "draft", missing dates -> null), so callers never * re-decide it independently. */ -export function phaseInputFromEdition( - edition: PhaseInputEdition, - timezone: string, - now: Date, -): GetEffectiveFestivalPhaseInput { +export function phaseInputFromEdition({ + edition, + timezone, + now, +}: { + edition: PhaseInputEdition; + timezone: string; + now: Date; +}): GetEffectiveFestivalPhaseInput { return { override: edition.phase_override ?? null, derivedInput: { diff --git a/src/lib/nowView.ts b/src/lib/nowView.ts index fb3e0c81a..02bcf86a9 100644 --- a/src/lib/nowView.ts +++ b/src/lib/nowView.ts @@ -25,7 +25,7 @@ export function canShowNowView( now: Date, ): boolean { const phase = getEffectiveFestivalPhase( - phaseInputFromEdition(edition, timezone, now), + phaseInputFromEdition({ edition, timezone, now }), ); return phase === "live" && canShowTime(edition.schedule_reveal_level); } diff --git a/src/routes/admin/festivals/$festivalSlug/editions/$editionSlug/settings.tsx b/src/routes/admin/festivals/$festivalSlug/editions/$editionSlug/settings.tsx index 26bd01abb..b1db9f977 100644 --- a/src/routes/admin/festivals/$festivalSlug/editions/$editionSlug/settings.tsx +++ b/src/routes/admin/festivals/$festivalSlug/editions/$editionSlug/settings.tsx @@ -29,15 +29,12 @@ function FestivalEditionSettings() { editionBySlugQuery({ festivalId: festival.id, editionSlug }), ); - // Deliberately the derived phase, not the override-aware one: this feeds - // the "Automatic (...)" option label below, which must show what automatic - // derivation would currently produce even when an override is active — the - // override itself is read separately as the control's selected value. - const { derivedInput } = phaseInputFromEdition( - currentEdition, - festival.timezone, - new Date(), - ); + // Deliberately derived, not override-aware: feeds the "Automatic (...)" label below, which must preview automatic derivation even when an override is active. + const { derivedInput } = phaseInputFromEdition({ + edition: currentEdition, + timezone: festival.timezone, + now: new Date(), + }); const derivedPhase = getFestivalPhase(derivedInput); return ( diff --git a/src/routes/festivals/$festivalSlug/editions/$editionSlug.tsx b/src/routes/festivals/$festivalSlug/editions/$editionSlug.tsx index ef22dd11f..2352bf2b3 100644 --- a/src/routes/festivals/$festivalSlug/editions/$editionSlug.tsx +++ b/src/routes/festivals/$festivalSlug/editions/$editionSlug.tsx @@ -46,7 +46,11 @@ export const Route = createFileRoute( location.pathname === `${basePath}/` ) { const phase = getEffectiveFestivalPhase( - phaseInputFromEdition(edition, context.festival.timezone, new Date()), + phaseInputFromEdition({ + edition, + timezone: context.festival.timezone, + now: new Date(), + }), ); throw redirect({