No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: timothy/ersatztv#242
Reference in New Issue
Block a user
Fixes the two blockers the adversarial reviewer returned CHANGES REQUIRED on in its post-merge verification of PR #229 (adversarial-reviewer#19, comment 7926): #212 reopened (bare-channel create unreachable on an empty lineup) and #230 promoted (dirty guard bypassed by popstate; shuffle-toggle leaves a stale item draft).
SPA + docs only — no dotnet changes.
Commits
d3d50f60— #212 (reopened) + #230 finding 1.ChannelsEmptyStateearly-returned before the action bar that owns "New blank channel", so a fresh install could never create its first channel; it now offers both create paths, reusing the exactcreateBlankChannelhandler (number = max+1 → 1 on an empty lineup, group"ErsatzTV", default ffmpeg profile). Same commit adds the App-levelpopstatedirty guard: browser Back/Forward can't be cancelled, so a vetoed pop re-pushes the pre-pop path via acurrentPathRefread synchronously at event time (StrictMode-safe).f09045e1—spa-conventions§8 rewritten to document popstate coverage of the dirty guard.11b43d5e— #230 finding 2. A properties save that flipsshuffleScheduleItemson the active schedule now reloads the items draft via GET so the server'sEnforcePropertiesre-normalizes it; the Edit button is blocked while the item draft is dirty (confirm-to-discard, same semantics asguardedSwitch).Behavior decision (Fix 3)
Chose block-edit-while-dirty over confirm-at-reload: a cancelled discard under confirm-at-reload would strand Fixed/Flood values in a now-shuffled schedule. Blocking the editor open while dirty guarantees the properties form only ever opens over a clean baseline draft, making the post-save reload lossless.
Pinning tests (per fix)
App.test.tsxbare-creates from a[]lineup, asserts the POST payload (number: "1",group: "ErsatzTV", default ffmpeg profile) and nav to/app/edit-channel/{id}.App.test.tsxpopstate cases — vetoed pop re-pushes the path; approved pop advances the route.SchedulesScreen.test.tsx— Fixed item on a non-shuffled schedule → edit properties to shuffle=true → a fresh items GET fires, the Fixed option is gone, and a subsequent Save's PUT carries no Fixed startType.Web gate green locally: 582 tests / 65 files, lint, build.
🤖 Generated with Claude Code
ChannelsScreen returned ChannelsEmptyState before the action bar that owns "New blank channel", so a fresh install could never create its first channel. The empty state now offers both create paths (bare-create + ChannelBuilder), reusing the exact createBlankChannel handler (number = max+1 → 1 on empty, group "ErsatzTV", default ffmpeg profile). Regression: App.test.tsx bare-creates from a [] lineup, asserts the POST payload (number "1") + navigation to /app/edit-channel/{id}. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>Adversarial review pass (Codex + scoped Claude fork) — findings applied
Both reviewers examined the diff independently. Applied as follow-up commit
6f6fcf6c:ChannelsEmptyStateswallowedmutationError(both reviewers, should-fix) — on the exact fresh-install path #212 targets, a bare-create can 4xx (e.g. no default ffmpeg profile) and the empty branch never rendered the error the non-empty screen shows. Now renders the samectv-channels-erroralert; regression test asserts the error surfaces with no navigation on an empty lineup.saving(Codex, flagged blocker) — during an items-save PUT the draft is still dirty, so a discard-to-open → shuffle-flip could let the in-flight PUT resolve after the shuffle reload and clobber the normalized draft. Edit is nowdisabled={saving}(consistent with the schedule Select); regression test with a deferred PUT proves Edit is disabled in-flight and re-enables once the save settles.Deferred as nits (both reviews rate low): a vetoed
navigateToPathleaves two stray history entries (cosmetic; only reachable from a dirty schedules screen, and fixing it means refactoring the central nav function — out of scope for a targeted blocker fix); rapid double-Back is best-effort (inherent popstate non-cancellability, acknowledged in the code comment).Web gate green: 584 tests / 65 files, lint, build.
Codex re-review of the prior fix commit found the Edit gate closed the exact repro but two paths remained. One is reachable: Delete was the only schedule-switch path that guardedSwitch's `saving` guard didn't cover — deleting mid-save runs applySwitch to the next schedule while the in-flight items PUT is still outstanding, and that PUT's completion handler then overwrites the next schedule's draft with the deleted schedule's response. Delete is now `disabled={saving}`, consistent with the Select, Edit, and guardedSwitch. Regression: the deferred-PUT test now also asserts Delete is disabled in-flight and re-enables after the save settles. Also widens the test mock's onRequest return type to `Response | Promise<Response> | null` (removes the `as unknown as Response` cast — a test-only type hole the re-review flagged). Deferred to #248: the other residual path (properties dialog not focus-trapped, so keyboard focus can escape to underlying Add/Save mid-save) is a pre-existing, cross-cutting overlay.tsx a11y gap affecting all dialogs — out of scope for this targeted blocker fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>timothy referenced this pull request2026-07-12 19:33:15 +02:00
timothy referenced this pull request2026-07-25 20:44:59 +02:00