refactor(schedule): add phase-input adapter - #516
Merged
Merged
Conversation
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QBenrJ8YAv3Zk81uWHvSLi
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QBenrJ8YAv3Zk81uWHvSLi
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The focused refactor preserves behavior, centralizes defaults, and includes adequate unit coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Centralizes edition-row phase defaults in a shared adapter while preserving existing override behavior.
Changes:
- Adds and tests
phaseInputFromEdition. - Migrates all phase consumers to the adapter.
- Preserves automatic-phase preview behavior in admin settings.
| File | Description |
|---|---|
src/lib/festivalPhase.ts |
Adds the shared adapter and input type. |
src/lib/festivalPhase.test.ts |
Tests passthrough and defaults. |
src/lib/nowView.ts |
Uses the adapter for phase evaluation. |
src/hooks/useFestivalPhase.ts |
Uses the adapter in the phase hook. |
src/routes/festivals/$festivalSlug/editions/$editionSlug.tsx |
Uses the adapter for default-tab redirects. |
src/routes/admin/festivals/$festivalSlug/editions/$editionSlug/settings.tsx |
Uses derived adapter input for the automatic label. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Playwright test results
Details
|
chiptus
commented
Oct 1, 2026
…tion 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QBenrJ8YAv3Zk81uWHvSLi
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
UPL-17: adds
phaseInputFromEdition, one adapter owning the edition-row → festival-phase-input defaulting policy that was independently reconstructed at each call site (useFestivalPhase,nowView, the edition route redirect, the admin settings page).The admin settings call site intentionally keeps calling the non-override-aware
getFestivalPhase(now fed by the adapter'sderivedInput) for its "Automatic (...)" label, since that label previews what automatic derivation would produce even while an override is active — the override itself is already shown separately as the control's selected value. The brief described this as a bug ("silently ignores a manual phase override"), but readingPhaseOverrideControl.tsxshows this is the label's intended purpose, not a bug; switching it to the override-aware phase would make the "Automatic" option echo the override instead of previewing automatic derivation.Verification
pnpm run typecheck,pnpm run lint,pnpm exec vitest run,pnpm run buildall pass.phaseInputFromEdition's defaulting (missing reveal level → draft, missing dates → null, missing override → null, full passthrough)./festivals/:slug/editions/:slugstill redirects to the override's tab (unchanged behavior, now adapter-backed).<Select>'s "Automatic (...)" label still shows the derived phase, not the override, when an override is active.🤖 Generated with Claude Code
https://claude.ai/code/session_01QBenrJ8YAv3Zk81uWHvSLi
Generated by Claude Code