From 7129d9c5b545bbb858550c87df76ec452d6b28ef Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 10 Jul 2026 23:26:08 +0200 Subject: [PATCH 01/15] fix: episode cards navigate to season detail + anchor (API SeasonId on browse items) (fixes #220) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Episode cards in the SPA search and media-browse screens were inert (mediaDetailPath had no Episode case, and LibraryBrowseItemResponseModel carried no parent-season id to route with). - API: add nullable SeasonId to LibraryBrowseItemResponseModel; populate it in LibraryBrowseItemMapper.GetEpisodes (the single shared hydration site used by both the library-browse search/browse handler and the season episode drill-in), leave it null for every other kind. Regenerated v1.json + v1.d.ts per docs/api-conventions.md §5. - SPA: mediaDetailPath now routes Episode items with a seasonId to /app/media/seasons/{seasonId}#episode-{id} (matching Blazor's Search.razor:241 link), null otherwise. MediaPosterCard accepts an id/highlighted pair; SeasonDetailScreen's episode grid gives each card a stable `episode-{id}` anchor and scrolls/highlights it on mount and on hashchange (deep-link support). - Tests: GetLibraryBrowseItemsHandlerTests asserts SeasonId is populated for episode drill-in results and null for other kinds; web tests cover mediaDetailPath's episode cases and the anchor/scroll/highlight behavior (jsdom scrollIntoView stub). - Docs: blazor-route-parity.md's episode-browse row and the Search cluster verdict updated — the standalone SPA episode browse exists and episode cards now navigate, closing the adversarial-reviewer#18 finding. Co-Authored-By: Claude Fable 5 --- .../LibraryBrowse/LibraryBrowseItemMapper.cs | 3 +- .../LibraryBrowseItemResponseModel.cs | 3 +- .../GetLibraryBrowseItemsHandlerTests.cs | 30 ++++++++++++++ ErsatzTV/wwwroot/openapi/v1.json | 7 ++++ docs/blazor-route-parity.md | 4 +- web/src/api/generated/v1.d.ts | 1 + web/src/media/MediaPosterCard.tsx | 11 +++++- web/src/media/mediaKinds.test.ts | 35 +++++++++++++++++ web/src/media/mediaKinds.ts | 2 + web/src/screens/MediaDetailScreen.test.tsx | 39 +++++++++++++++++++ web/src/screens/MediaDetailScreen.tsx | 21 ++++++++++ web/src/shell.css | 16 ++++++++ 12 files changed, 166 insertions(+), 6 deletions(-) create mode 100644 web/src/media/mediaKinds.test.ts diff --git a/ErsatzTV.Application/LibraryBrowse/LibraryBrowseItemMapper.cs b/ErsatzTV.Application/LibraryBrowse/LibraryBrowseItemMapper.cs index 245101502..874f1c9a2 100644 --- a/ErsatzTV.Application/LibraryBrowse/LibraryBrowseItemMapper.cs +++ b/ErsatzTV.Application/LibraryBrowse/LibraryBrowseItemMapper.cs @@ -274,7 +274,8 @@ internal static class LibraryBrowseItemMapper null, em.EpisodeId, null, - EpisodeSubtitle(em))).ToList()); + EpisodeSubtitle(em), + em.Episode.SeasonId)).ToList()); } public static async Task> GetMusicVideos( diff --git a/ErsatzTV.Core/Api/LibraryBrowse/LibraryBrowseItemResponseModel.cs b/ErsatzTV.Core/Api/LibraryBrowse/LibraryBrowseItemResponseModel.cs index 67f6cffd2..c7a84a5f1 100644 --- a/ErsatzTV.Core/Api/LibraryBrowse/LibraryBrowseItemResponseModel.cs +++ b/ErsatzTV.Core/Api/LibraryBrowse/LibraryBrowseItemResponseModel.cs @@ -20,4 +20,5 @@ public record LibraryBrowseItemResponseModel( int? RerunCollectionId, int? MediaItemId, int? PlaylistId, - string? Subtitle = null); + string? Subtitle = null, + int? SeasonId = null); diff --git a/ErsatzTV.Tests/Application/LibraryBrowse/GetLibraryBrowseItemsHandlerTests.cs b/ErsatzTV.Tests/Application/LibraryBrowse/GetLibraryBrowseItemsHandlerTests.cs index b04b5c3b8..5afc03187 100644 --- a/ErsatzTV.Tests/Application/LibraryBrowse/GetLibraryBrowseItemsHandlerTests.cs +++ b/ErsatzTV.Tests/Application/LibraryBrowse/GetLibraryBrowseItemsHandlerTests.cs @@ -288,6 +288,10 @@ public class GetLibraryBrowseItemsHandlerTests result.Page[1].MediaItemId.ShouldBe(711); result.Page[2].MediaItemId.ShouldBe(713); + // #220: every episode carries its parent season id so the SPA can route to the season + // detail page and anchor to the episode (`/app/media/seasons/{seasonId}#episode-{id}`). + result.Page.ShouldAllBe(p => p.SeasonId == 701); + await _searchIndex.DidNotReceive().Search( Arg.Any(), Arg.Any(), @@ -297,6 +301,32 @@ public class GetLibraryBrowseItemsHandlerTests Arg.Any()); } + [Test] + public async Task Handle_Should_Leave_SeasonId_Null_For_Non_Episode_Kinds() + { + await SeedLibraryGraph(); + _searchIndex.Search( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any>(), + Arg.Any()) + .Returns(new SearchResult( + [ + new SearchItem(LuceneSearchIndex.ShowType, 20), + new SearchItem(LuceneSearchIndex.MovieType, 10) + ], + 2)); + var handler = new GetLibraryBrowseItemsHandler(_searchIndex, _db.Factory); + + PagedLibraryBrowseItemsResponseModel result = await handler.Handle( + new GetLibraryBrowseItems("", null, null, 0, 10), + CancellationToken.None); + + result.Page.ShouldAllBe(p => p.SeasonId == null); + } + [Test] public async Task Handle_Should_Browse_Music_Videos_For_A_Specific_Artist_By_ParentId() { diff --git a/ErsatzTV/wwwroot/openapi/v1.json b/ErsatzTV/wwwroot/openapi/v1.json index 847e90c5a..1b6da0c9e 100644 --- a/ErsatzTV/wwwroot/openapi/v1.json +++ b/ErsatzTV/wwwroot/openapi/v1.json @@ -18013,6 +18013,13 @@ "null", "string" ] + }, + "seasonId": { + "type": [ + "null", + "integer" + ], + "format": "int32" } } }, diff --git a/docs/blazor-route-parity.md b/docs/blazor-route-parity.md index bb617695a..eb1e7b49d 100644 --- a/docs/blazor-route-parity.md +++ b/docs/blazor-route-parity.md @@ -64,7 +64,7 @@ redirect). > | Collections | PARITY-OK (custom-order endpoint + reorder UI, all-10-kind add picker, 2026-07-09) | — | > | Channel editor | GAPS (external logo URL, bare create, pickers) | #212 | > | Channels-numbers / Logs | PARITY-OK / minors | #213 | -> | Search | PARITY-OK (card nav, per-card/multi-select add-to, add-all, save-as-smart-collection, 2026-07-10) | — | +> | Search | PARITY-OK (card nav — episode cards now navigate too, closing the adversarial-reviewer#18 finding tracked as #220, 2026-07-10; per-card/multi-select add-to, add-all, save-as-smart-collection, 2026-07-10) | — | > | Media browse/detail (read paths + image browser) | PARITY-OK | — | > | Media browse/detail (mutations, per-show scan, episode info/troubleshoot) | PARITY-OK (shared Add-to layer + scan/info/troubleshoot wired, 2026-07-10) | — | > | Schedules editors | **disproven — moved to Section 3** | **#207** | @@ -115,7 +115,7 @@ been added to the redirect map yet. | `/media/tv/shows/{ShowId:int}` | `TelevisionSeasonList.razor` | `/app/media/shows/{id}` | show + season list (`ShowDetailScreen`); PR #183 / #141 | | `/media/tv/seasons`(`/page/{n}`) | `TelevisionSeasonSearchResults.razor` | `/app/media?kind=seasons` | seasons browsable as a top-level kind (`MediaBrowseScreen`; also reachable via show drill-in); #209 review fix | | `/media/tv/seasons/{SeasonId:int}` | `TelevisionEpisodeList.razor` | `/app/media/seasons/{id}` | season + episode list (`SeasonDetailScreen`); PR #183 / #141 | -| `/media/tv/episodes`(`/page/{n}`) | `EpisodeList.razor` | `/app/media/seasons/{id}` | no standalone SPA episode browse; covered via season detail drill-in; PR #183 / #141 | +| `/media/tv/episodes`(`/page/{n}`) | `EpisodeList.razor` | `/app/media?kind=episodes` | standalone SPA episode browse EXISTS (`MediaBrowseScreen`, generic grid, top-level `episodes` kind); episode cards there and on the Search screen now navigate to the season detail page and anchor/highlight the episode (`/app/media/seasons/{seasonId}#episode-{id}`), matching `Search.razor:241`'s `media/tv/seasons/{SeasonId}#episode-{EpisodeId}` link (`LibraryBrowseItemResponseModel.SeasonId`, `mediaDetailPath`); #220 | | `/media/music/artists`(`/page/{n}`) | `ArtistList.razor` | `/app/media?kind=artists` | generic browse; PR #183 / #141 | | `/media/music/artists/{ArtistId:int}` | `Artist.razor` | `/app/media/artists/{id}` | detail page (`ArtistDetailScreen`); PR #183 / #141 | | `/media/music/videos`(`/page/{n}`) | `MusicVideoList.razor` | `/app/media?kind=music-videos` | generic browse; PR #183 / #141 | diff --git a/web/src/api/generated/v1.d.ts b/web/src/api/generated/v1.d.ts index 9c9024447..005a6ff25 100644 --- a/web/src/api/generated/v1.d.ts +++ b/web/src/api/generated/v1.d.ts @@ -761,6 +761,7 @@ export interface components { "mediaItemId": null | number; "playlistId": null | number; "subtitle"?: null | string; + "seasonId"?: null | number; }; "LibraryBrowseMediaType": "Movie" | "TelevisionShow" | "TelevisionSeason" | "Artist" | "Collection" | "SmartCollection" | "MultiCollection" | "RerunCollection" | "Playlist" | "Episode" | "MusicVideo" | "Song" | "OtherVideo" | "Image" | "RemoteStream"; "LibraryMediaKind": "Movies" | "Shows" | "MusicVideos" | "OtherVideos" | "Songs" | "Images" | "RemoteStreams"; diff --git a/web/src/media/MediaPosterCard.tsx b/web/src/media/MediaPosterCard.tsx index d7cbecaf8..e00fa271b 100644 --- a/web/src/media/MediaPosterCard.tsx +++ b/web/src/media/MediaPosterCard.tsx @@ -13,7 +13,9 @@ export function MediaPosterCard({ onToggleSelect, onOpen, height = 150, - actions + actions, + id, + highlighted }: { item: LibraryBrowseItem; selected?: boolean; @@ -21,6 +23,10 @@ export function MediaPosterCard({ onOpen?: (item: LibraryBrowseItem) => void; height?: number; actions?: ReactNode; + // Stable DOM id (e.g. `episode-{id}`) so callers can deep-link/scroll to this card. + id?: string; + // Applies a temporary visual highlight, e.g. when this card is the deep-link target. + highlighted?: boolean; }) { const hue = hueOf(item.title); const Icon = TYPE_ICON[item.mediaType] ?? Film; @@ -45,7 +51,8 @@ export function MediaPosterCard({ return (
): LibraryBrowseItem { + return { + artwork: '', + id: 91, + mediaType: 'Episode', + title: 'Pilot', + ...overrides + } as unknown as LibraryBrowseItem; +} + +describe('mediaDetailPath', () => { + it('routes an episode with a seasonId to the season detail page, anchored to the episode (#220)', () => { + const item = episodeItem({ seasonId: 8 } as Partial); + expect(mediaDetailPath(item)).toBe('/app/media/seasons/8#episode-91'); + }); + + it('returns null for an episode with no seasonId', () => { + const item = episodeItem({}); + expect(mediaDetailPath(item)).toBeNull(); + }); + + it('still routes a movie to its detail page', () => { + const item = { + artwork: '', + id: 5, + mediaType: 'Movie', + title: 'Blade Runner' + } as unknown as LibraryBrowseItem; + expect(mediaDetailPath(item)).toBe('/app/media/movies/5'); + }); +}); diff --git a/web/src/media/mediaKinds.ts b/web/src/media/mediaKinds.ts index 763dfeb05..96fb8f939 100644 --- a/web/src/media/mediaKinds.ts +++ b/web/src/media/mediaKinds.ts @@ -61,6 +61,8 @@ export function mediaDetailPath(item: LibraryBrowseItem): string | null { return `/app/media/seasons/${item.id}`; case 'Artist': return `/app/media/artists/${item.id}`; + case 'Episode': + return item.seasonId != null ? `/app/media/seasons/${item.seasonId}#episode-${item.id}` : null; default: return null; } diff --git a/web/src/screens/MediaDetailScreen.test.tsx b/web/src/screens/MediaDetailScreen.test.tsx index a36d80dbd..e3f92bdb4 100644 --- a/web/src/screens/MediaDetailScreen.test.tsx +++ b/web/src/screens/MediaDetailScreen.test.tsx @@ -178,4 +178,43 @@ describe('media detail screens', () => { expect(screen.getByRole('button', { name: 'Media Info' })).toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Troubleshoot Playback' })).toBeInTheDocument(); }); + + it('anchors and highlights the episode targeted by an #episode-{id} hash (#220)', async () => { + vi.spyOn(window, 'fetch').mockImplementation((input: RequestInfo | URL) => { + const url = input.toString(); + if (url.startsWith('/api/library/browse')) { + return Promise.resolve( + jsonResponse({ + page: [ + { artwork: '', collectionType: 'Episode', id: 90, mediaType: 'Episode', title: 'Pilot' }, + { artwork: '', collectionType: 'Episode', id: 91, mediaType: 'Episode', title: 'Second' } + ], + totalCount: 2 + }) + ); + } + if (url === '/api/collections' || url === '/api/playlists/groups' || url === '/api/schedules') { + return Promise.resolve(jsonResponse([])); + } + return Promise.resolve(jsonResponse(season)); + }); + + const scrollIntoView = vi.fn(); + vi.stubGlobal('HTMLElement', HTMLElement); + Element.prototype.scrollIntoView = scrollIntoView; + window.location.hash = '#episode-91'; + + render(); + await waitFor(() => expect(screen.getByText('Second')).toBeInTheDocument()); + + const target = document.getElementById('episode-91'); + expect(target).not.toBeNull(); + await waitFor(() => expect(scrollIntoView).toHaveBeenCalled()); + expect(target?.className).toContain('ctv-media-card-highlighted'); + + const other = document.getElementById('episode-90'); + expect(other?.className).not.toContain('ctv-media-card-highlighted'); + + window.location.hash = ''; + }); }); diff --git a/web/src/screens/MediaDetailScreen.tsx b/web/src/screens/MediaDetailScreen.tsx index 14e8c8aaa..f80ce857c 100644 --- a/web/src/screens/MediaDetailScreen.tsx +++ b/web/src/screens/MediaDetailScreen.tsx @@ -211,6 +211,7 @@ function ChildGrid({ const [pageNum, setPageNum] = useState(0); const [status, setStatus] = useState<'loading' | 'error' | 'success'>('loading'); const [error, setError] = useState(null); + const [anchorId, setAnchorId] = useState(() => window.location.hash.slice(1) || null); const activeRef = useRef(true); const seqRef = useRef(0); @@ -221,6 +222,23 @@ function ChildGrid({ }; }, []); + // Deep-link support (#220): a season detail's episode grid can be opened with an + // `#episode-{id}` hash (from search/browse cards, or Blazor-parity links). Track hash changes + // so re-navigating to the same season with a different anchor still scrolls/highlights. + useEffect(() => { + const onHashChange = () => setAnchorId(window.location.hash.slice(1) || null); + window.addEventListener('hashchange', onHashChange); + return () => window.removeEventListener('hashchange', onHashChange); + }, []); + + useEffect(() => { + if (status !== 'success' || mediaType !== 'Episode' || !anchorId) { + return; + } + const target = document.getElementById(anchorId); + target?.scrollIntoView({ behavior: 'smooth', block: 'center' }); + }, [status, mediaType, anchorId, items]); + const load = useCallback(() => { const id = ++seqRef.current; getLibraryBrowseItems({ mediaType, pageNum, pageSize: CHILD_PAGE_SIZE, parentId }) @@ -278,9 +296,12 @@ function ChildGrid({
{items.map((item) => { const detailPath = mediaDetailPath(item); + const cardId = item.mediaType === 'Episode' ? `episode-${item.id}` : undefined; return ( navigateToPath(detailPath) : undefined} diff --git a/web/src/shell.css b/web/src/shell.css index f51cc1482..7a48e3ae2 100644 --- a/web/src/shell.css +++ b/web/src/shell.css @@ -2529,6 +2529,22 @@ body { box-shadow: 0 0 0 1px var(--action-primary); } +/* Deep-link target highlight (#220), e.g. `#episode-{id}` from search/browse episode cards. */ +.ctv-media-card-highlighted { + border-color: var(--action-primary); + box-shadow: 0 0 0 2px var(--action-primary); + animation: ctv-media-card-highlight-fade 2400ms ease-out 1; +} + +@keyframes ctv-media-card-highlight-fade { + 0% { + box-shadow: 0 0 0 4px var(--action-primary); + } + 100% { + box-shadow: 0 0 0 2px var(--action-primary); + } +} + .ctv-media-card-poster { position: relative; overflow: hidden; From 81ddd4e5ca1f241a1133ec545538e475aaf8fc79 Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 10 Jul 2026 23:25:42 +0200 Subject: [PATCH 02/15] fix(spa): gate mutation controls + add-all on stale result sets during refetch (fixes #221) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Search and Media browse keep the previous successful result set rendered during a refetch (query on Search; kind/query/page on Media browse) with no gating, so per-card Add-to, Select/select-mode, the selection action bar, Add all, and Save-as-smart-collection stayed live over stale, about-to-be-replaced items. Worst path: SearchScreen.addAll only checked activeRef, so a late GET /api/search/all-items could open a bulk-add dialog scoped to the previous query's entire result set. Key the success state to the request params that produced it and derive a `refreshing` flag; while refreshing, keep cards visible but disable every mutation surface, show a "Refreshing…" cue, and dim the grid. Card navigation stays live. Bind addAll's completion to its query via lastQueryRef so a stale all-items result is discarded. Same pattern applied to both screens. Docs: spa-conventions §3a (refreshing/gating pattern) + §8 (temporal-semantics review checklist); blazor-route-parity search/media-browse verdicts. Co-Authored-By: Claude Fable 5 --- docs/blazor-route-parity.md | 12 ++- docs/spa-conventions.md | 42 +++++++++ web/src/screens/MediaBrowseScreen.test.tsx | 47 ++++++++++ web/src/screens/MediaBrowseScreen.tsx | 41 +++++++-- web/src/screens/SearchScreen.test.tsx | 101 +++++++++++++++++++++ web/src/screens/SearchScreen.tsx | 46 ++++++++-- web/src/shell.css | 8 ++ 7 files changed, 280 insertions(+), 17 deletions(-) diff --git a/docs/blazor-route-parity.md b/docs/blazor-route-parity.md index bb617695a..e45d238ef 100644 --- a/docs/blazor-route-parity.md +++ b/docs/blazor-route-parity.md @@ -64,9 +64,9 @@ redirect). > | Collections | PARITY-OK (custom-order endpoint + reorder UI, all-10-kind add picker, 2026-07-09) | — | > | Channel editor | GAPS (external logo URL, bare create, pickers) | #212 | > | Channels-numbers / Logs | PARITY-OK / minors | #213 | -> | Search | PARITY-OK (card nav, per-card/multi-select add-to, add-all, save-as-smart-collection, 2026-07-10) | — | +> | Search | PARITY-OK (card nav, per-card/multi-select add-to, add-all, save-as-smart-collection, 2026-07-10; mutation controls + Add-all now gated during refetch — #221) | — | > | Media browse/detail (read paths + image browser) | PARITY-OK | — | -> | Media browse/detail (mutations, per-show scan, episode info/troubleshoot) | PARITY-OK (shared Add-to layer + scan/info/troubleshoot wired, 2026-07-10) | — | +> | Media browse/detail (mutations, per-show scan, episode info/troubleshoot) | PARITY-OK (shared Add-to layer + scan/info/troubleshoot wired, 2026-07-10; mutation controls gated on kind/query/page refetch — #221) | — | > | Schedules editors | **disproven — moved to Section 3** | **#207** | > | Media sources | **disproven — moved to Section 3** | **#202** | > @@ -192,6 +192,14 @@ query-wide Add All via `GET /api/search/all-items`, Save As Smart Collection) an pages + child grids, per-show Quick/Deep scan gated to Plex/Jellyfin/Emby, per-episode Media Info + Troubleshoot entries; `POST /api/playlists/{id}/items` added). Known accepted deviations (select-mode toggle, per-card target superset) recorded in `docs/decisions.md`. +CLOSED 2026-07-10: **#221** (adversarial-reviewer#18 follow-up to #208/#209) — those PRs added +mutation actions to two screens whose fetch model keeps the previous result set rendered during a +refetch. On Search and Media browse the per-card Add-to menu, Select/select-mode, selection action +bar, Add-all, and Save-as-smart-collection are now **gated while a refetch is in flight** (query on +Search; kind/query/page on Media browse), with a visible "Refreshing…" cue and dimmed grid; card +navigation stays live. `SearchScreen.addAll` also binds its completion to the requesting query so a +late `GET /api/search/all-items` can no longer open a bulk-add dialog scoped to the previous query. +See `docs/spa-conventions.md` §3a for the pattern. ## Section 4 — Blazor home / escape hatch diff --git a/docs/spa-conventions.md b/docs/spa-conventions.md index 3e4b29422..65036d80f 100644 --- a/docs/spa-conventions.md +++ b/docs/spa-conventions.md @@ -60,6 +60,35 @@ fetches from the API: `eslint-plugin-react-hooks` in `web/eslint.config.js` — a synchronous `setState` in an effect body will fail `npm run lint`. +## 3a. "Keep results visible during refetch" ⇒ gate mutations + show a refreshing cue + +Some grid screens deliberately keep the **previous** successful result set rendered while a refetch +is in flight (no full-screen loading state on a query/kind/page change), so the grid doesn't flash +empty. `SearchScreen.tsx` and `MediaBrowseScreen.tsx` do this. If such a screen also carries +**mutation surfaces** (per-card Add-to menu, Select/select-mode, a selection action bar, "Add all", +"Save as smart collection"), those surfaces would otherwise stay live over a **stale** result set — +an add/select action then targets the about-to-be-replaced items, or (worse) a query-wide "Add all" +bulk request resolves against the previous query. This was issue #221 (adversarial-reviewer#18). + +Convention — when a screen keeps stale results visible during a refetch: + +- **Key the success state to the request params that produced it.** Store the identifying params on + the `status: 'success'` variant (`SearchScreen`: the `query`; `MediaBrowseScreen`: a + `kind|query|page` `key`), set in the seq-guarded `.then`. Derive + `const refreshing = state.status === 'success' && state. !== ;` in render. + Prefer this over a synchronously-set `refreshing` flag: setting state synchronously from the load + path trips the `react-hooks` "no set-state-in-effect" rule (§3), and a param-keyed derivation is + self-correcting (it can never get stuck true/false). +- **While `refreshing`:** show a visible cue (a `role="status"` "Refreshing…" row with `` + plus the `.ctv-media-grid-dim` opacity class on the grid) and **disable every mutation surface** — + per-card Add-to menu (withhold the `actions` node), select toggle + in-grid selection + (`const canSelect = selectMode && !refreshing;` gates `onToggleSelect`), the selection action bar, + "Add all", "Save as smart collection". Card navigation (`onOpen`) **may** stay live. +- **Bind async bulk completions to their request params, not just mount.** A whole-query/whole-set + request (e.g. `getSearchAllItems`) must, on resolve, check that its snapshotted params are still + current (compare against a ref that always holds the committed value — `SearchScreen` reuses + `lastQueryRef`) and **discard** otherwise. Checking only `activeRef` (mounted) is insufficient. + ## 4. API client modules One file per domain in `web/src/api/`, e.g. `logs.ts`, `blocks.ts`, `playouts.ts`. Pattern (see @@ -145,3 +174,16 @@ npm run build # tsc -b && vite build Also run `npm run check:api` if you touched anything OpenAPI-relevant (see `api-conventions.md` §5) — it regenerates `src/api/generated/v1.d.ts` and fails the build if it's out of sync with what's committed. + +## 8. Review checklist — temporal semantics + +- **For every effect / timer / async completion, ask: *when* does it fire (mount, dependency change, + unmount, StrictMode double-invoke) and *which* render/request does it still own?** A debounce timer + fires on mount too (§3, the `lastQueryRef` no-change guard exists precisely for that); a `.then` + can resolve after the params it was launched for have moved on (§3a, the `refreshing` gate and the + Add-all query binding exist for that). A guard that only checks "still mounted" (`activeRef`) does + not answer "still current". +- **For any "make X consistent with Y" change, re-validate the exemplar Y's temporal behavior before + copying it.** #221 came from copying a fetch model that keeps stale results visible onto screens + that had gained mutation surfaces — the exemplar was safe read-only, the copy was not. Copying a + pattern copies its *assumptions*; confirm they still hold in the new context. diff --git a/web/src/screens/MediaBrowseScreen.test.tsx b/web/src/screens/MediaBrowseScreen.test.tsx index 50812c246..bec84464c 100644 --- a/web/src/screens/MediaBrowseScreen.test.tsx +++ b/web/src/screens/MediaBrowseScreen.test.tsx @@ -130,6 +130,53 @@ describe('MediaBrowseScreen', () => { }); }); + it('gates old-kind cards and shows a refreshing cue during a kind-change refetch (issue #221)', async () => { + function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((r) => { + resolve = r; + }); + return { promise, resolve }; + } + + const browseDeferreds: Array<{ resolve: (body: unknown) => void }> = []; + vi.spyOn(window, 'fetch').mockImplementation((input: RequestInfo | URL) => { + const url = input.toString(); + if (url.startsWith('/api/library/browse')) { + const d = deferred(); + browseDeferreds.push({ resolve: (body) => d.resolve(jsonResponse(body)) }); + return d.promise; + } + if (url === '/api/collections') { + return Promise.resolve(jsonResponse([{ collectionType: 'Collection', id: 7, name: 'Favorites', useCustomPlaybackOrder: false }])); + } + if (url === '/api/playlists/groups') { + return Promise.resolve(jsonResponse([])); + } + return Promise.resolve(new Response(null, { status: 204 })); + }); + + render(); + browseDeferreds[0].resolve({ page: items, totalCount: items.length }); + await waitFor(() => expect(screen.getByText('Blade Runner')).toBeInTheDocument()); + expect(screen.getAllByRole('button', { name: 'Add to…' })).toHaveLength(items.length); + + // Change kind: the refetch for the new kind is held pending while old-kind cards stay rendered. + fireEvent.change(screen.getByRole('combobox'), { target: { value: 'shows' } }); + await waitFor(() => expect(browseDeferreds.length).toBe(2)); + + // Refreshing window: cue visible, mutation surfaces gated, old-kind cards still visible but inert. + await waitFor(() => expect(screen.getByText('Refreshing…')).toBeInTheDocument()); + expect(screen.getByText('Blade Runner')).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Add to…' })).toBeNull(); + expect(screen.getByRole('button', { name: 'Select' })).toBeDisabled(); + + // New-kind results resolve: controls re-enable, cue gone. + browseDeferreds[1].resolve({ page: items, totalCount: items.length }); + await waitFor(() => expect(screen.queryByText('Refreshing…')).not.toBeInTheDocument()); + expect(screen.getAllByRole('button', { name: 'Add to…' })).toHaveLength(items.length); + }); + it('selects every loaded item with Select all on page', async () => { mockFetch(); render(); diff --git a/web/src/screens/MediaBrowseScreen.tsx b/web/src/screens/MediaBrowseScreen.tsx index 876b9b7cf..70046957d 100644 --- a/web/src/screens/MediaBrowseScreen.tsx +++ b/web/src/screens/MediaBrowseScreen.tsx @@ -55,10 +55,17 @@ function kindFromSlug(slug: string | null): MediaKind { } type BrowseState = - | { items: LibraryBrowseItem[]; error: null; status: 'success'; totalCount: number } + // `key` records the request params (kind + query + page) that produced this result set. When it + // no longer matches the current params, the visible items are stale (a refetch is in flight) — + // see `refreshing` below. + | { items: LibraryBrowseItem[]; error: null; status: 'success'; totalCount: number; key: string } | { items: []; error: string; status: 'error'; totalCount: 0 } | { items: []; error: null; status: 'loading'; totalCount: 0 }; +function browseKeyOf(mediaType: LibraryBrowseMediaType, query: string, pageNum: number): string { + return `${mediaType}\u0000${query}\u0000${pageNum}`; +} + type Notice = { tone: 'ok' | 'error'; message: string }; export function MediaBrowseScreen() { @@ -108,10 +115,11 @@ export function MediaBrowseScreen() { const load = useCallback(() => { const id = ++seqRef.current; + const key = browseKeyOf(kind.mediaType, query, pageNum); getLibraryBrowseItems({ mediaType: kind.mediaType, query: query || undefined, pageNum, pageSize: PAGE_SIZE }) .then((paged) => { if (activeRef.current && id === seqRef.current) { - setState({ items: paged.page ?? [], error: null, status: 'success', totalCount: paged.totalCount ?? 0 }); + setState({ items: paged.page ?? [], error: null, status: 'success', totalCount: paged.totalCount ?? 0, key }); } }) .catch((error: unknown) => { @@ -185,6 +193,13 @@ export function MediaBrowseScreen() { const selectedItems = Array.from(selected.values()); const totalPages = state.status === 'success' ? Math.max(1, Math.ceil(state.totalCount / PAGE_SIZE)) : 1; + // A refetch is in flight when the currently-rendered items were produced by different params than + // the current kind/query/page (items stay visible during refetch — see `load`). While refreshing we + // keep items visible but gate every mutation surface and show a refreshing cue, so no add/select + // action is scoped to the stale, about-to-be-replaced result set (issue #221). + const refreshing = state.status === 'success' && state.key !== browseKeyOf(kind.mediaType, query, pageNum); + const canSelect = selectMode && !refreshing; + return (
@@ -213,6 +228,7 @@ export function MediaBrowseScreen() { )}
)} + {refreshing && ( +
+ + Refreshing… +
+ )} + {!hasQuery && (
Type a query to search across every media kind.
@@ -344,21 +372,23 @@ export function SearchScreen() { )}
-
+
{data.items.map((item) => { const key = itemKey(item); const detailPath = mediaDetailPath(item); return ( setNotice({ tone: 'ok', message })} /> ) } item={item} key={key} - onOpen={!selectMode && detailPath ? () => navigateToPath(detailPath) : undefined} - onToggleSelect={selectMode ? toggleSelect : undefined} + onOpen={!canSelect && detailPath ? () => navigateToPath(detailPath) : undefined} + onToggleSelect={canSelect ? toggleSelect : undefined} selected={selectMode ? selected.has(key) : undefined} /> ); diff --git a/web/src/shell.css b/web/src/shell.css index f51cc1482..4a1fee71d 100644 --- a/web/src/shell.css +++ b/web/src/shell.css @@ -2509,6 +2509,14 @@ body { gap: var(--space-6, 12px); } +/* Dim the stale result set while a refetch is in flight (search/media browse — issue #221). The + per-card mutation menu is withheld in the same state; navigation stays live so no pointer-events + change here. */ +.ctv-media-grid-dim { + opacity: 0.5; + transition: opacity var(--dur-fast, 120ms) var(--ease-standard, ease); +} + .ctv-media-card { position: relative; border-radius: var(--radius-sm); From fc2c054b445bef28114df70f2e48115c294cc3f4 Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 10 Jul 2026 23:27:57 +0200 Subject: [PATCH 03/15] fix(api): validate + include RemoteStream in playlist add-items (fixes #217) AddItemsToPlaylistHandler only validated existence for movies/shows/ seasons/episodes, leaving artist/music-video/other-video/song/image/ remote-stream ids unchecked (silently accepted, or in RemoteStream's case silently dropped entirely - the apply dictionary never included CollectionType.RemoteStream). Mirror AddItemsToCollectionHandler's established pattern: add RemoteStream to the apply dictionary, and add an aggregate existence check (ValidateMediaItems/GetRequestedMediaItemIds) across all ten kinds against dbContext.MediaItems. Add ErsatzTV.Tests/Application/MediaCollections/PlaylistHandlerTests.cs covering: a bogus id of each of the ten kinds fails validation; a valid RemoteStream id is actually persisted to the playlist (regression test for the drop bug). Co-Authored-By: Claude Fable 5 --- .../Commands/AddItemsToPlaylistHandler.cs | 34 ++++- .../MediaCollections/PlaylistHandlerTests.cs | 139 ++++++++++++++++++ 2 files changed, 170 insertions(+), 3 deletions(-) create mode 100644 ErsatzTV.Tests/Application/MediaCollections/PlaylistHandlerTests.cs diff --git a/ErsatzTV.Application/MediaCollections/Commands/AddItemsToPlaylistHandler.cs b/ErsatzTV.Application/MediaCollections/Commands/AddItemsToPlaylistHandler.cs index 98251b91a..fd8586c21 100644 --- a/ErsatzTV.Application/MediaCollections/Commands/AddItemsToPlaylistHandler.cs +++ b/ErsatzTV.Application/MediaCollections/Commands/AddItemsToPlaylistHandler.cs @@ -46,7 +46,8 @@ public class AddItemsToPlaylistHandler : IRequestHandler 0 ? playlist.Items.Max(i => i.Index) + 1 : 0; @@ -81,8 +82,9 @@ public class AddItemsToPlaylistHandler : IRequestHandler collection); + await ValidateEpisodes(request), + await ValidateMediaItems(dbContext, request, cancellationToken)) + .Apply((collection, _, _, _, _, _) => collection); private static async Task> PlaylistMustExist( TvContext dbContext, @@ -126,4 +128,30 @@ public class AddItemsToPlaylistHandler : IRequestHandler v == true) .MapT(_ => Unit.Default) .Map(v => v.ToValidation("Episode does not exist")); + + private static async Task> ValidateMediaItems( + TvContext dbContext, + AddItemsToPlaylist request, + CancellationToken cancellationToken) + { + List ids = GetRequestedMediaItemIds(request).Distinct().ToList(); + int existingCount = await dbContext.MediaItems + .CountAsync(mi => ids.Contains(mi.Id), cancellationToken); + + return existingCount == ids.Count + ? Unit.Default + : BaseError.New("Media item does not exist"); + } + + private static IEnumerable GetRequestedMediaItemIds(AddItemsToPlaylist request) => + request.MovieIds + .Append(request.ShowIds) + .Append(request.SeasonIds) + .Append(request.EpisodeIds) + .Append(request.ArtistIds) + .Append(request.MusicVideoIds) + .Append(request.OtherVideoIds) + .Append(request.SongIds) + .Append(request.ImageIds) + .Append(request.RemoteStreamIds); } diff --git a/ErsatzTV.Tests/Application/MediaCollections/PlaylistHandlerTests.cs b/ErsatzTV.Tests/Application/MediaCollections/PlaylistHandlerTests.cs new file mode 100644 index 000000000..b0082fcbd --- /dev/null +++ b/ErsatzTV.Tests/Application/MediaCollections/PlaylistHandlerTests.cs @@ -0,0 +1,139 @@ +using ErsatzTV.Application.MediaCollections; +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Errors; +using ErsatzTV.Core.Interfaces.Repositories; +using ErsatzTV.Infrastructure.Data; +using ErsatzTV.Tests.Support; +using LanguageExt; +using NSubstitute; +using NUnit.Framework; +using Shouldly; +using Unit = LanguageExt.Unit; + +namespace ErsatzTV.Tests.Application.MediaCollections; + +[TestFixture] +public class PlaylistHandlerTests : MediaCollectionHandlerTestBase +{ + [TestCase(CollectionType.Artist)] + [TestCase(CollectionType.MusicVideo)] + [TestCase(CollectionType.OtherVideo)] + [TestCase(CollectionType.Song)] + [TestCase(CollectionType.Image)] + [TestCase(CollectionType.RemoteStream)] + public async Task AddItems_Should_Return_ValidationError_When_Previously_Unvalidated_Kind_Missing( + CollectionType collectionType) + { + await SeedPlaylist(1); + var handler = MakeHandler(); + + Either result = + await handler.Handle(MakeAddItems(1, collectionType, [999]), CancellationToken.None); + + BaseError error = LeftOf(result); + error.ShouldNotBeOfType(); + error.Value.ShouldContain("Media item does not exist"); + } + + [TestCase(CollectionType.Movie)] + [TestCase(CollectionType.TelevisionShow)] + [TestCase(CollectionType.TelevisionSeason)] + [TestCase(CollectionType.Episode)] + public async Task AddItems_Should_Return_ValidationError_When_Existing_Validated_Kind_Missing( + CollectionType collectionType) + { + await SeedPlaylist(1); + var handler = MakeHandler(); + + Either result = + await handler.Handle(MakeAddItems(1, collectionType, [999]), CancellationToken.None); + + BaseError error = LeftOf(result); + error.ShouldNotBeOfType(); + } + + [Test] + public async Task AddItems_Should_Add_Valid_RemoteStream_Item_To_Playlist() + { + await SeedPlaylist(1); + await SeedRemoteStream(50); + var handler = MakeHandler(); + + Either result = + await handler.Handle(MakeAddItems(1, CollectionType.RemoteStream, [50]), CancellationToken.None); + + RightOf(result); + + await using TvContext context = Db.CreateContext(); + Playlist playlist = await context.Playlists.FindAsync(1); + List items = context.Entry(playlist!).Collection(p => p.Items).Query().ToList(); + + items.ShouldHaveSingleItem(); + items[0].CollectionType.ShouldBe(CollectionType.RemoteStream); + items[0].MediaItemId.ShouldBe(50); + } + + private AddItemsToPlaylistHandler MakeHandler() + { + IMovieRepository movieRepository = Substitute.For(); + movieRepository.AllMoviesExist(Arg.Any>()).Returns(true); + ITelevisionRepository televisionRepository = Substitute.For(); + televisionRepository.AllShowsExist(Arg.Any>()).Returns(true); + televisionRepository.AllSeasonsExist(Arg.Any>()).Returns(true); + televisionRepository.AllEpisodesExist(Arg.Any>()).Returns(true); + + return new AddItemsToPlaylistHandler(Db.Factory, movieRepository, televisionRepository); + } + + private async Task SeedPlaylist(int id, string name = "Playlist") + { + await using TvContext context = Db.CreateContext(); + context.Playlists.Add(new Playlist { Id = id, Name = name, Items = [] }); + await context.SaveChangesAsync(); + } + + private async Task SeedRemoteStream(int id) + { + await using TvContext context = Db.CreateContext(); + context.RemoteStreams.Add(new RemoteStream + { + Id = id, + RemoteStreamMetadata = [] + }); + await context.SaveChangesAsync(); + } + + private static AddItemsToPlaylist MakeAddItems(int playlistId, CollectionType collectionType, List ids) + { + List movieIds = collectionType == CollectionType.Movie ? ids : []; + List showIds = collectionType == CollectionType.TelevisionShow ? ids : []; + List seasonIds = collectionType == CollectionType.TelevisionSeason ? ids : []; + List episodeIds = collectionType == CollectionType.Episode ? ids : []; + List artistIds = collectionType == CollectionType.Artist ? ids : []; + List musicVideoIds = collectionType == CollectionType.MusicVideo ? ids : []; + List otherVideoIds = collectionType == CollectionType.OtherVideo ? ids : []; + List songIds = collectionType == CollectionType.Song ? ids : []; + List imageIds = collectionType == CollectionType.Image ? ids : []; + List remoteStreamIds = collectionType == CollectionType.RemoteStream ? ids : []; + + return new AddItemsToPlaylist( + playlistId, + movieIds, + showIds, + seasonIds, + episodeIds, + artistIds, + musicVideoIds, + otherVideoIds, + songIds, + imageIds, + remoteStreamIds); + } + + private static BaseError LeftOf(Either either) => + either.Match(Left: e => e, Right: _ => throw new AssertionException("Expected a Left result")); + + private static TR RightOf(Either either) => + either.Match(Left: e => throw new AssertionException($"Expected a Right result, got: {e.Value}"), Right: v => v); +} From 8ddf0ae16932cdf3754719a90e3e6c223e5ebb56 Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 10 Jul 2026 23:28:09 +0200 Subject: [PATCH 04/15] fix(api): resolve per-show scan by exact show id, not substring title (fixes #219) POST /api/libraries/{id}/scan-show resolved the target show via GetShowIdByTitle, an EF.Functions.Like "%title%" substring match with no OrderBy - non-deterministic under duplicate/overlapping titles and capable of scanning the wrong show. The Blazor UI never had this bug (it always passed the exact show id); this endpoint shipped days ago in PR #216 with no external consumers, so the contract break is safe. BREAKING CHANGE: ScanShowRequest now takes `showId: int` instead of `showTitle: string`. Replaced ITelevisionRepository.GetShowIdByTitle with GetShowTitle(libraryId, showId), which also enforces the show belongs to the given library. LibrariesController.ScanShow now returns a genuine 404 ProblemDetails (via ApiResults.NotFoundProblem, the established pre-check pattern from TemplateController.DeleteGroup) when the show id doesn't exist in that library, then queues QueueShowScanByLibraryId with the DB-resolved title. SPA: libraries.ts ScanShowParams.showId replaces showTitle; MediaDetailScreen.tsx passes show.id. Extended ApiErrorResponseMetadataTests and OpenApiErrorResponseContractTests with the new 404 contract for ScanShow. Regenerated v1.json / v1.d.ts via scripts/update-openapi.sh + npm run generate:api. Co-Authored-By: Claude Fable 5 --- .../Fakes/FakeTelevisionRepository.cs | 2 +- .../Repositories/ITelevisionRepository.cs | 2 +- .../Data/Repositories/TelevisionRepository.cs | 8 +-- .../ApiErrorResponseMetadataTests.cs | 1 + .../Controllers/LibrariesControllerTests.cs | 45 ++++++++++++++++- .../OpenApiErrorResponseContractTests.cs | 1 + .../Controllers/Api/LibrariesController.cs | 21 ++++---- ErsatzTV/wwwroot/openapi/v1.json | 50 ++++++++++++++++--- web/src/api/generated/v1.d.ts | 2 +- web/src/api/libraries.test.ts | 10 ++-- web/src/api/libraries.ts | 10 ++-- web/src/screens/MediaDetailScreen.test.tsx | 2 +- web/src/screens/MediaDetailScreen.tsx | 2 +- 13 files changed, 118 insertions(+), 38 deletions(-) diff --git a/ErsatzTV.Core.Tests/Fakes/FakeTelevisionRepository.cs b/ErsatzTV.Core.Tests/Fakes/FakeTelevisionRepository.cs index e69ac7432..eb4087d8a 100644 --- a/ErsatzTV.Core.Tests/Fakes/FakeTelevisionRepository.cs +++ b/ErsatzTV.Core.Tests/Fakes/FakeTelevisionRepository.cs @@ -14,7 +14,7 @@ public class FakeTelevisionRepository : ITelevisionRepository public Task> GetAllShows() => throw new NotSupportedException(); public Task> GetShow(int showId, CancellationToken cancellationToken) => throw new NotSupportedException(); - public Task> GetShowIdByTitle(int libraryId, string title) => throw new NotSupportedException(); + public Task> GetShowTitle(int libraryId, int showId) => throw new NotSupportedException(); public Task> GetShowItems(int showId) => throw new NotSupportedException(); public Task> GetEpisodeIdsForShow(int showId) => throw new NotSupportedException(); diff --git a/ErsatzTV.Core/Interfaces/Repositories/ITelevisionRepository.cs b/ErsatzTV.Core/Interfaces/Repositories/ITelevisionRepository.cs index 947802782..3518743c2 100644 --- a/ErsatzTV.Core/Interfaces/Repositories/ITelevisionRepository.cs +++ b/ErsatzTV.Core/Interfaces/Repositories/ITelevisionRepository.cs @@ -10,7 +10,7 @@ public interface ITelevisionRepository Task AllEpisodesExist(List episodeIds); Task> GetAllShows(); Task> GetShow(int showId, CancellationToken cancellationToken); - Task> GetShowIdByTitle(int libraryId, string title); + Task> GetShowTitle(int libraryId, int showId); Task> GetShowItems(int showId); Task> GetEpisodeIdsForShow(int showId); Task> GetAllSeasons(); diff --git a/ErsatzTV.Infrastructure/Data/Repositories/TelevisionRepository.cs b/ErsatzTV.Infrastructure/Data/Repositories/TelevisionRepository.cs index b788851bc..4909612a7 100644 --- a/ErsatzTV.Infrastructure/Data/Repositories/TelevisionRepository.cs +++ b/ErsatzTV.Infrastructure/Data/Repositories/TelevisionRepository.cs @@ -80,16 +80,16 @@ public class TelevisionRepository : ITelevisionRepository .SelectOneAsync(s => s.Id, s => s.Id == showId, cancellationToken); } - public async Task> GetShowIdByTitle(int libraryId, string title) + public async Task> GetShowTitle(int libraryId, int showId) { await using TvContext dbContext = await _dbContextFactory.CreateDbContextAsync(); return await dbContext.ShowMetadata .AsNoTracking() + .Where(sm => sm.ShowId == showId) .Where(sm => sm.Show.LibraryPath.LibraryId == libraryId) - .Where(sm => EF.Functions.Like(sm.Title, $"%{title}%")) - .Map(sm => sm.ShowId) + .Map(sm => sm.Title) .FirstOrDefaultAsync() - .Map(showId => showId > 0 ? Option.Some(showId) : Option.None); + .Map(Optional); } public async Task> GetEpisodeIdsForShow(int showId) diff --git a/ErsatzTV.Tests/Controllers/ApiErrorResponseMetadataTests.cs b/ErsatzTV.Tests/Controllers/ApiErrorResponseMetadataTests.cs index 7ab30a490..9ed6f5474 100644 --- a/ErsatzTV.Tests/Controllers/ApiErrorResponseMetadataTests.cs +++ b/ErsatzTV.Tests/Controllers/ApiErrorResponseMetadataTests.cs @@ -139,6 +139,7 @@ public class ApiErrorResponseMetadataTests [TestCase(typeof(FFmpegProfileController), nameof(FFmpegProfileController.DeleteProfileAsync), StatusCodes.Status404NotFound)] [TestCase(typeof(FFmpegProfileController), nameof(FFmpegProfileController.DeleteProfileAsync), StatusCodes.Status401Unauthorized)] [TestCase(typeof(FFmpegProfileController), nameof(FFmpegProfileController.DeleteProfileAsync), StatusCodes.Status422UnprocessableEntity)] + [TestCase(typeof(LibrariesController), nameof(LibrariesController.ScanShow), StatusCodes.Status404NotFound)] public void Api_Error_Response_Metadata_Should_Document_ProblemDetails( Type controllerType, string actionName, diff --git a/ErsatzTV.Tests/Controllers/LibrariesControllerTests.cs b/ErsatzTV.Tests/Controllers/LibrariesControllerTests.cs index 95d67c9a0..0971c747c 100644 --- a/ErsatzTV.Tests/Controllers/LibrariesControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/LibrariesControllerTests.cs @@ -3,7 +3,10 @@ using ErsatzTV.Application.Libraries; using ErsatzTV.Controllers.Api; using ErsatzTV.Core.Api.Libraries; using ErsatzTV.Core.Interfaces.Repositories; +using LanguageExt; using MediatR; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Routing; using NSubstitute; using NUnit.Framework; @@ -16,12 +19,14 @@ public class LibrariesControllerTests { private LibrariesController _controller = null!; private IMediator _mediator = null!; + private ITelevisionRepository _televisionRepository = null!; [SetUp] public void SetUp() { _mediator = Substitute.For(); - _controller = new LibrariesController(Substitute.For(), _mediator); + _televisionRepository = Substitute.For(); + _controller = new LibrariesController(_televisionRepository, _mediator); } [Test] @@ -52,4 +57,42 @@ public class LibrariesControllerTests result.ShouldBe(expected); } + + [Test] + public async Task ScanShow_Should_Return_NotFoundProblem_When_Show_Not_In_Library() + { + _televisionRepository.GetShowTitle(3, 999).Returns(Option.None); + + IActionResult result = await _controller.ScanShow(3, new ScanShowRequest(999)); + + var notFound = result.ShouldBeOfType(); + notFound.Value.ShouldBeOfType().Status.ShouldBe(StatusCodes.Status404NotFound); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task ScanShow_Should_Queue_Scan_By_Show_Id_When_Show_Belongs_To_Library() + { + _televisionRepository.GetShowTitle(3, 42).Returns(Option.Some("The Office")); + _mediator.Send(Arg.Any(), Arg.Any()).Returns(true); + + IActionResult result = await _controller.ScanShow(3, new ScanShowRequest(42, DeepScan: true)); + + result.ShouldBeOfType(); + await _mediator.Received(1).Send( + Arg.Is(r => + r.LibraryId == 3 && r.ShowId == 42 && r.ShowTitle == "The Office" && r.DeepScan), + Arg.Any()); + } + + [Test] + public async Task ScanShow_Should_Return_BadRequest_When_Mediator_Fails_To_Queue() + { + _televisionRepository.GetShowTitle(3, 42).Returns(Option.Some("The Office")); + _mediator.Send(Arg.Any(), Arg.Any()).Returns(false); + + IActionResult result = await _controller.ScanShow(3, new ScanShowRequest(42)); + + result.ShouldBeOfType(); + } } diff --git a/ErsatzTV.Tests/Controllers/OpenApiErrorResponseContractTests.cs b/ErsatzTV.Tests/Controllers/OpenApiErrorResponseContractTests.cs index a24049ea0..aeb42758c 100644 --- a/ErsatzTV.Tests/Controllers/OpenApiErrorResponseContractTests.cs +++ b/ErsatzTV.Tests/Controllers/OpenApiErrorResponseContractTests.cs @@ -260,6 +260,7 @@ public class OpenApiErrorResponseContractTests [TestCase("/api/trakt/lists/{id}", "put", "404")] [TestCase("/api/trakt/lists/{id}", "put", "422")] [TestCase("/api/troubleshoot/playback/subtitles/{mediaItemId}", "get", "404")] + [TestCase("/api/libraries/{id}/scan-show", "post", "404")] public void Static_OpenApi_Should_Document_ProblemDetails_For_Api_Error_Responses( string path, string method, diff --git a/ErsatzTV/Controllers/Api/LibrariesController.cs b/ErsatzTV/Controllers/Api/LibrariesController.cs index 15878394b..844a73859 100644 --- a/ErsatzTV/Controllers/Api/LibrariesController.cs +++ b/ErsatzTV/Controllers/Api/LibrariesController.cs @@ -1,6 +1,7 @@ using ErsatzTV.Application.Libraries; using ErsatzTV.Core.Api.Libraries; using ErsatzTV.Core.Interfaces.Repositories; +using ErsatzTV.Extensions; using MediatR; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; @@ -29,27 +30,23 @@ public class LibrariesController(ITelevisionRepository televisionRepository, IMe [HttpPost("/api/libraries/{id:int}/scan-show")] [Tags("Libraries")] [EndpointSummary("Scan show")] + [ProducesResponseType(StatusCodes.Status200OK)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(StatusCodes.Status400BadRequest)] public async Task ScanShow(int id, [FromBody] ScanShowRequest request) { - if (string.IsNullOrWhiteSpace(request.ShowTitle)) + Option maybeTitle = await televisionRepository.GetShowTitle(id, request.ShowId); + foreach (string title in maybeTitle) { - return new BadRequestObjectResult(new { error = "ShowTitle is required" }); - } - - string trimmedTitle = request.ShowTitle.Trim(); - Option maybeShowId = await televisionRepository.GetShowIdByTitle(id, trimmedTitle); - foreach (int showId in maybeShowId) - { - bool result = await mediator.Send(new QueueShowScanByLibraryId(id, showId, trimmedTitle, request.DeepScan)); + bool result = await mediator.Send(new QueueShowScanByLibraryId(id, request.ShowId, title, request.DeepScan)); return result ? new OkResult() : new BadRequestObjectResult(new { error = "Unable to queue show scan. Library may not exist, may not support single show scanning, or may already be scanning." }); } - return new BadRequestObjectResult( - new { error = $"Unable to locate show with title {request.ShowTitle} in library {id}" }); + return ApiResults.NotFoundProblem($"Show {request.ShowId} does not exist in library {id}."); } } -public record ScanShowRequest(string ShowTitle, bool DeepScan = false); +public record ScanShowRequest(int ShowId, bool DeepScan = false); diff --git a/ErsatzTV/wwwroot/openapi/v1.json b/ErsatzTV/wwwroot/openapi/v1.json index 847e90c5a..7d1af9f06 100644 --- a/ErsatzTV/wwwroot/openapi/v1.json +++ b/ErsatzTV/wwwroot/openapi/v1.json @@ -5317,6 +5317,46 @@ "responses": { "200": { "description": "OK" + }, + "404": { + "description": "Not Found", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, + "400": { + "description": "Bad Request", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } } } } @@ -20790,15 +20830,13 @@ }, "ScanShowRequest": { "required": [ - "showTitle" + "showId" ], "type": "object", "properties": { - "showTitle": { - "type": [ - "null", - "string" - ] + "showId": { + "type": "integer", + "format": "int32" }, "deepScan": { "type": "boolean", diff --git a/web/src/api/generated/v1.d.ts b/web/src/api/generated/v1.d.ts index 9c9024447..7bd68ccd1 100644 --- a/web/src/api/generated/v1.d.ts +++ b/web/src/api/generated/v1.d.ts @@ -1280,7 +1280,7 @@ export interface components { "libraryRefreshInterval": number; }; "ScanShowRequest": { - "showTitle": null | string; + "showId": number; "deepScan"?: boolean; }; "ScheduleItemRequest": { diff --git a/web/src/api/libraries.test.ts b/web/src/api/libraries.test.ts index f50fa5436..349f26bb0 100644 --- a/web/src/api/libraries.test.ts +++ b/web/src/api/libraries.test.ts @@ -22,19 +22,19 @@ describe('libraries api client', () => { expect(fetchMock).toHaveBeenCalledWith('/api/libraries/4/scan', expect.objectContaining({ method: 'POST' })); }); - it('scanShow POSTs the show title and deepScan flag', async () => { + it('scanShow POSTs the show id and deepScan flag', async () => { const fetchMock = vi.spyOn(window, 'fetch').mockResolvedValue(noContent()); - await scanShow(4, { deepScan: true, showTitle: 'The Office' }); + await scanShow(4, { deepScan: true, showId: 42 }); const { init, url } = lastCall(fetchMock); expect(url).toBe('/api/libraries/4/scan-show'); expect(init?.method).toBe('POST'); - expect(JSON.parse(String(init?.body))).toEqual({ deepScan: true, showTitle: 'The Office' }); + expect(JSON.parse(String(init?.body))).toEqual({ deepScan: true, showId: 42 }); }); it('scanShow defaults deepScan to false when omitted', async () => { const fetchMock = vi.spyOn(window, 'fetch').mockResolvedValue(noContent()); - await scanShow(9, { showTitle: 'Firefly' }); + await scanShow(9, { showId: 17 }); const { init } = lastCall(fetchMock); - expect(JSON.parse(String(init?.body))).toEqual({ deepScan: false, showTitle: 'Firefly' }); + expect(JSON.parse(String(init?.body))).toEqual({ deepScan: false, showId: 17 }); }); }); diff --git a/web/src/api/libraries.ts b/web/src/api/libraries.ts index 0def1f765..b2b1e0d01 100644 --- a/web/src/api/libraries.ts +++ b/web/src/api/libraries.ts @@ -52,16 +52,16 @@ export function scanLibrary(libraryId: number): Promise { } export interface ScanShowParams { - showTitle: string; + showId: number; deepScan?: boolean; } -// Queues a scan of a single show (by title) within a library. Returns 200 on success, 400 when -// the title can't be resolved / the library doesn't support single-show scanning. Body keys are -// `showTitle` and `deepScan` (see LibrariesController.ScanShowRequest). +// Queues a scan of a single show (by id) within a library. Returns 200 on success, 404 when the +// show id doesn't exist in the library, 400 when the library doesn't support single-show +// scanning. Body keys are `showId` and `deepScan` (see LibrariesController.ScanShowRequest). export function scanShow(libraryId: number, params: ScanShowParams): Promise { return request(`/api/libraries/${libraryId}/scan-show`, { - body: { deepScan: params.deepScan ?? false, showTitle: params.showTitle }, + body: { deepScan: params.deepScan ?? false, showId: params.showId }, method: 'POST' }); } diff --git a/web/src/screens/MediaDetailScreen.test.tsx b/web/src/screens/MediaDetailScreen.test.tsx index a36d80dbd..c0b97538c 100644 --- a/web/src/screens/MediaDetailScreen.test.tsx +++ b/web/src/screens/MediaDetailScreen.test.tsx @@ -154,7 +154,7 @@ describe('media detail screens', () => { const scanCall = fetchSpy.mock.calls.find(([url]) => String(url) === '/api/libraries/3/scan-show'); expect(scanCall).toBeTruthy(); const body = JSON.parse(String((scanCall![1] as RequestInit).body)); - expect(body).toMatchObject({ deepScan: true, showTitle: 'The Show' }); + expect(body).toMatchObject({ deepScan: true, showId: 42 }); }); await waitFor(() => expect(screen.getByText('Scan queued')).toBeInTheDocument()); }); diff --git a/web/src/screens/MediaDetailScreen.tsx b/web/src/screens/MediaDetailScreen.tsx index 14e8c8aaa..e6490ff4b 100644 --- a/web/src/screens/MediaDetailScreen.tsx +++ b/web/src/screens/MediaDetailScreen.tsx @@ -584,7 +584,7 @@ function ShowScanControls({ show }: { show: ShowDetail }) { const runScan = (deepScan: boolean) => { setScanning(deepScan ? 'deep' : 'quick'); setMessage(null); - scanShow(show.libraryId, { deepScan, showTitle: show.title }) + scanShow(show.libraryId, { deepScan, showId: show.id }) .then(() => { if (activeRef.current) { setScanning(false); From 28910ff557501d62e279c1f05b0dd701a23bc367 Mon Sep 17 00:00:00 2001 From: Timothy Date: Fri, 10 Jul 2026 23:34:23 +0200 Subject: [PATCH 05/15] fix(api): gate playout mutations on EntityLocker build lock (409) + mirror lock state in SPA (fixes #215) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Blazor disabled per-playout Reset/Erase/Delete/Edit while a BuildPlayout was in flight (EntityLocker.IsPlayoutLocked); the REST API had no equivalent, so a client could race an in-flight build with a destructive ExecuteDelete and leave a half-built playout. After Blazor removal this safety invariant would vanish entirely (adversarial-reviewer#18 removal gate). Server: - Add public ApiResults.ConflictProblem(title, detail) (409, mirrors NotFoundProblem). - Inject IEntityLocker into PlayoutController; guard every id-keyed mutation (PUT {id}, PUT .../deco, PUT .../alternate-schedules, PUT .../templates, POST .../erase-items, POST .../erase-items-and-history, DELETE {id}) → 409 when IsPlayoutLocked(id); add [ProducesResponseType(...409)] to each. - Guard ChannelController.ResetPlayout the same way after resolving the id. - reset-all stays 202 (ResetAllPlayoutsHandler already skips locked playouts). - Stamp IsLocked onto PlayoutListItemResponseModel from IsPlayoutLocked. SPA: - Disable Reset/Erase/Erase-and-history/Delete for a locked row + show a "Building…" Badge; on a 409 surface the error and refresh the list. Tests: controller-level 409 guard tests (delete/erase/PUT/deco/channel-reset) + IsLocked projection test; new OpenAPI contract + metadata 409 rows. Docs: api-conventions §3a, blazor-route-parity playouts verdict, decisions.md. Regenerated v1.json + web types. Co-Authored-By: Claude Fable 5 --- .../Playouts/PlayoutListItemResponseModel.cs | 3 +- .../ApiErrorResponseMetadataTests.cs | 12 ++ .../Controllers/ChannelControllerTests.cs | 19 +- .../OpenApiErrorResponseContractTests.cs | 10 ++ .../Controllers/PlayoutControllerTests.cs | 81 ++++++++- ErsatzTV/Controllers/Api/ChannelController.cs | 15 +- ErsatzTV/Controllers/Api/PlayoutController.cs | 66 ++++++- ErsatzTV/Extensions/ApiResults.cs | 7 + ErsatzTV/wwwroot/openapi/v1.json | 166 +++++++++++++++++- docs/api-conventions.md | 21 +++ docs/blazor-route-parity.md | 8 +- docs/decisions.md | 29 +++ web/src/App.test.tsx | 20 +++ web/src/App.tsx | 23 ++- web/src/api/generated/v1.d.ts | 1 + 15 files changed, 466 insertions(+), 15 deletions(-) diff --git a/ErsatzTV.Core/Api/Playouts/PlayoutListItemResponseModel.cs b/ErsatzTV.Core/Api/Playouts/PlayoutListItemResponseModel.cs index 33aa15d5b..a5da52036 100644 --- a/ErsatzTV.Core/Api/Playouts/PlayoutListItemResponseModel.cs +++ b/ErsatzTV.Core/Api/Playouts/PlayoutListItemResponseModel.cs @@ -11,4 +11,5 @@ public record PlayoutListItemResponseModel( string ScheduleName, TimeSpan? DailyRebuildTime, PlayoutBuildStatusResponseModel? BuildStatus, - ChannelPlayoutMode PlayoutMode); + ChannelPlayoutMode PlayoutMode, + bool IsLocked); diff --git a/ErsatzTV.Tests/Controllers/ApiErrorResponseMetadataTests.cs b/ErsatzTV.Tests/Controllers/ApiErrorResponseMetadataTests.cs index 7ab30a490..f4833f19a 100644 --- a/ErsatzTV.Tests/Controllers/ApiErrorResponseMetadataTests.cs +++ b/ErsatzTV.Tests/Controllers/ApiErrorResponseMetadataTests.cs @@ -26,6 +26,7 @@ public class ApiErrorResponseMetadataTests [TestCase(typeof(ChannelController), nameof(ChannelController.BulkDelete), StatusCodes.Status404NotFound)] [TestCase(typeof(ChannelController), nameof(ChannelController.BulkDelete), StatusCodes.Status422UnprocessableEntity)] [TestCase(typeof(ChannelController), nameof(ChannelController.ResetPlayout), StatusCodes.Status404NotFound)] + [TestCase(typeof(ChannelController), nameof(ChannelController.ResetPlayout), StatusCodes.Status409Conflict)] [TestCase(typeof(ChannelTemplateController), nameof(ChannelTemplateController.GetDefault), StatusCodes.Status404NotFound)] [TestCase(typeof(ChannelTemplateController), nameof(ChannelTemplateController.SetDefault), StatusCodes.Status404NotFound)] [TestCase(typeof(ChannelTemplateController), nameof(ChannelTemplateController.SetDefault), StatusCodes.Status422UnprocessableEntity)] @@ -113,22 +114,33 @@ public class ApiErrorResponseMetadataTests [TestCase(typeof(DecoTemplateController), nameof(DecoTemplateController.Replace), StatusCodes.Status404NotFound)] [TestCase(typeof(DecoTemplateController), nameof(DecoTemplateController.Replace), StatusCodes.Status422UnprocessableEntity)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.UpdateDefaultDeco), StatusCodes.Status404NotFound)] + [TestCase(typeof(PlayoutController), nameof(PlayoutController.UpdateDefaultDeco), StatusCodes.Status409Conflict)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.UpdateDefaultDeco), StatusCodes.Status422UnprocessableEntity)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.GetById), StatusCodes.Status404NotFound)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.Create), StatusCodes.Status404NotFound)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.Create), StatusCodes.Status422UnprocessableEntity)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.Update), StatusCodes.Status404NotFound)] + [TestCase(typeof(PlayoutController), nameof(PlayoutController.Update), StatusCodes.Status409Conflict)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.Update), StatusCodes.Status422UnprocessableEntity)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.Delete), StatusCodes.Status404NotFound)] + [TestCase(typeof(PlayoutController), nameof(PlayoutController.Delete), StatusCodes.Status409Conflict)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.Delete), StatusCodes.Status422UnprocessableEntity)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.GetAlternateSchedules), StatusCodes.Status404NotFound)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.GetAlternateSchedules), StatusCodes.Status422UnprocessableEntity)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.ReplaceAlternateSchedules), StatusCodes.Status404NotFound)] + [TestCase(typeof(PlayoutController), nameof(PlayoutController.ReplaceAlternateSchedules), StatusCodes.Status409Conflict)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.ReplaceAlternateSchedules), StatusCodes.Status422UnprocessableEntity)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.GetTemplates), StatusCodes.Status404NotFound)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.GetTemplates), StatusCodes.Status422UnprocessableEntity)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.ReplaceTemplates), StatusCodes.Status404NotFound)] + [TestCase(typeof(PlayoutController), nameof(PlayoutController.ReplaceTemplates), StatusCodes.Status409Conflict)] [TestCase(typeof(PlayoutController), nameof(PlayoutController.ReplaceTemplates), StatusCodes.Status422UnprocessableEntity)] + [TestCase(typeof(PlayoutController), nameof(PlayoutController.EraseItems), StatusCodes.Status404NotFound)] + [TestCase(typeof(PlayoutController), nameof(PlayoutController.EraseItems), StatusCodes.Status409Conflict)] + [TestCase(typeof(PlayoutController), nameof(PlayoutController.EraseItems), StatusCodes.Status422UnprocessableEntity)] + [TestCase(typeof(PlayoutController), nameof(PlayoutController.EraseItemsAndHistory), StatusCodes.Status404NotFound)] + [TestCase(typeof(PlayoutController), nameof(PlayoutController.EraseItemsAndHistory), StatusCodes.Status409Conflict)] + [TestCase(typeof(PlayoutController), nameof(PlayoutController.EraseItemsAndHistory), StatusCodes.Status422UnprocessableEntity)] [TestCase(typeof(FFmpegProfileController), nameof(FFmpegProfileController.GetById), StatusCodes.Status404NotFound)] [TestCase(typeof(FFmpegProfileController), nameof(FFmpegProfileController.AddOne), StatusCodes.Status404NotFound)] [TestCase(typeof(FFmpegProfileController), nameof(FFmpegProfileController.AddOne), StatusCodes.Status401Unauthorized)] diff --git a/ErsatzTV.Tests/Controllers/ChannelControllerTests.cs b/ErsatzTV.Tests/Controllers/ChannelControllerTests.cs index 9377d2f7c..cd25e7760 100644 --- a/ErsatzTV.Tests/Controllers/ChannelControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/ChannelControllerTests.cs @@ -10,6 +10,7 @@ using ErsatzTV.Core.Api.Channels; using ErsatzTV.Core.Api.LibraryBrowse; using ErsatzTV.Core.Domain; using ErsatzTV.Core.Errors; +using ErsatzTV.Core.Interfaces.Locking; using ErsatzTV.Core.Scheduling; using LanguageExt; using static LanguageExt.Prelude; @@ -27,6 +28,7 @@ public class ChannelControllerTests { private IMediator _mediator = null!; private Channel _workerChannel = null!; + private IEntityLocker _entityLocker = null!; private ChannelController _controller = null!; [SetUp] @@ -34,7 +36,8 @@ public class ChannelControllerTests { _mediator = Substitute.For(); _workerChannel = System.Threading.Channels.Channel.CreateUnbounded(); - _controller = new ChannelController(_workerChannel.Writer, _mediator); + _entityLocker = Substitute.For(); + _controller = new ChannelController(_workerChannel.Writer, _mediator, _entityLocker); } [Test] @@ -374,6 +377,20 @@ public class ChannelControllerTests buildPlayout.Mode.ShouldBe(expectedMode); } + [Test] + public async Task ResetPlayout_Should_Return_409_When_Playout_Locked() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(9)); + _entityLocker.IsPlayoutLocked(9).Returns(true); + + IActionResult result = await _controller.ResetPlayout("5", mode: null, CancellationToken.None); + + var conflict = result.ShouldBeOfType(); + conflict.Value.ShouldBeOfType().Status.ShouldBe(409); + _workerChannel.Reader.TryRead(out _).ShouldBeFalse(); + } + [Test] public async Task ResetPlayout_Should_Honor_Explicit_Mode() { diff --git a/ErsatzTV.Tests/Controllers/OpenApiErrorResponseContractTests.cs b/ErsatzTV.Tests/Controllers/OpenApiErrorResponseContractTests.cs index a24049ea0..48eee50c5 100644 --- a/ErsatzTV.Tests/Controllers/OpenApiErrorResponseContractTests.cs +++ b/ErsatzTV.Tests/Controllers/OpenApiErrorResponseContractTests.cs @@ -115,6 +115,7 @@ public class OpenApiErrorResponseContractTests [TestCase("/api/channels/bulk/delete", "post", "404")] [TestCase("/api/channels/bulk/delete", "post", "422")] [TestCase("/api/channels/{channelNumber}/playout/reset", "post", "404")] + [TestCase("/api/channels/{channelNumber}/playout/reset", "post", "409")] [TestCase("/api/channel-templates/default", "get", "404")] [TestCase("/api/channel-templates/default/{id}", "put", "404")] [TestCase("/api/channel-templates/default/{id}", "put", "422")] @@ -188,12 +189,21 @@ public class OpenApiErrorResponseContractTests [TestCase("/api/playouts/{id}", "get", "404")] [TestCase("/api/playouts", "post", "404")] [TestCase("/api/playouts", "post", "422")] + [TestCase("/api/playouts/{id}", "put", "404")] + [TestCase("/api/playouts/{id}", "put", "409")] + [TestCase("/api/playouts/{id}", "put", "422")] [TestCase("/api/playouts/{id}", "delete", "404")] + [TestCase("/api/playouts/{id}", "delete", "409")] [TestCase("/api/playouts/{id}", "delete", "422")] + [TestCase("/api/playouts/{id}/deco", "put", "409")] + [TestCase("/api/playouts/{id}/alternate-schedules", "put", "409")] + [TestCase("/api/playouts/{id}/templates", "put", "409")] [TestCase("/api/playouts/{id}/items", "get", "404")] [TestCase("/api/playouts/{id}/erase-items", "post", "404")] + [TestCase("/api/playouts/{id}/erase-items", "post", "409")] [TestCase("/api/playouts/{id}/erase-items", "post", "422")] [TestCase("/api/playouts/{id}/erase-items-and-history", "post", "404")] + [TestCase("/api/playouts/{id}/erase-items-and-history", "post", "409")] [TestCase("/api/playouts/{id}/erase-items-and-history", "post", "422")] [TestCase("/api/playouts/items/{id}/scheduling-context", "get", "404")] [TestCase("/api/collections/{id}/custom-order", "put", "404")] diff --git a/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs b/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs index cd285aad6..2f8a85930 100644 --- a/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs @@ -13,6 +13,7 @@ using ErsatzTV.Core.Domain; using ErsatzTV.Core.Domain.Filler; using ErsatzTV.Core.Domain.Scheduling; using ErsatzTV.Core.Errors; +using ErsatzTV.Core.Interfaces.Locking; using ErsatzTV.Core.Scheduling; using LanguageExt; using MediatR; @@ -31,12 +32,14 @@ public class PlayoutControllerTests { private PlayoutController _controller = null!; private IMediator _mediator = null!; + private IEntityLocker _entityLocker = null!; [SetUp] public void SetUp() { _mediator = Substitute.For(); - _controller = new PlayoutController(_mediator); + _entityLocker = Substitute.For(); + _controller = new PlayoutController(_mediator, _entityLocker); } [Test] @@ -68,6 +71,82 @@ public class PlayoutControllerTests "/api/playouts/items/{id:int}/scheduling-context"); } + // ----- Build-lock guard (#215): id-keyed mutations return 409 while the build lock is held ----- + + [Test] + public async Task Delete_Should_Return_409_When_Playout_Locked() + { + _entityLocker.IsPlayoutLocked(9).Returns(true); + + IActionResult result = await _controller.Delete(9, CancellationToken.None); + + var conflict = result.ShouldBeOfType(); + conflict.Value.ShouldBeOfType().Status.ShouldBe(409); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task EraseItems_Should_Return_409_When_Playout_Locked() + { + _entityLocker.IsPlayoutLocked(9).Returns(true); + + IActionResult result = await _controller.EraseItems(9, CancellationToken.None); + + result.ShouldBeOfType().Value.ShouldBeOfType().Status.ShouldBe(409); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task EraseItemsAndHistory_Should_Return_409_When_Playout_Locked() + { + _entityLocker.IsPlayoutLocked(9).Returns(true); + + IActionResult result = await _controller.EraseItemsAndHistory(9, CancellationToken.None); + + result.ShouldBeOfType().Value.ShouldBeOfType().Status.ShouldBe(409); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task Update_Should_Return_409_When_Playout_Locked() + { + _entityLocker.IsPlayoutLocked(9).Returns(true); + + IActionResult result = await _controller.Update( + 9, + new UpdatePlayoutDetailsRequest(TimeSpan.FromHours(4), null), + CancellationToken.None); + + result.ShouldBeOfType().Value.ShouldBeOfType().Status.ShouldBe(409); + await _mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } + + [Test] + public async Task UpdateDefaultDeco_Should_Return_409_When_Playout_Locked() + { + _entityLocker.IsPlayoutLocked(9).Returns(true); + + IActionResult result = await _controller.UpdateDefaultDeco( + 9, + new UpdateDefaultDecoRequest(null), + CancellationToken.None); + + result.ShouldBeOfType().Value.ShouldBeOfType().Status.ShouldBe(409); + } + + [Test] + public async Task GetAll_Should_Stamp_IsLocked_From_Locker() + { + _entityLocker.IsPlayoutLocked(9).Returns(true); + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(new PagedPlayoutsViewModel(1, [MakePlayout(9)])); + + PagedPlayoutsResponseModel result = await _controller.GetAll("", 0, 100, CancellationToken.None); + + result.Page.Single().IsLocked.ShouldBeTrue(); + } + // ----- Erase items / history ----- [Test] diff --git a/ErsatzTV/Controllers/Api/ChannelController.cs b/ErsatzTV/Controllers/Api/ChannelController.cs index 9082d7f3b..e675e1ec2 100644 --- a/ErsatzTV/Controllers/Api/ChannelController.cs +++ b/ErsatzTV/Controllers/Api/ChannelController.cs @@ -7,6 +7,7 @@ using ErsatzTV.Controllers.Api.Requests; using ErsatzTV.Core; using ErsatzTV.Core.Api.Channels; using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Interfaces.Locking; using ErsatzTV.Core.Scheduling; using ErsatzTV.Extensions; using MediatR; @@ -16,7 +17,10 @@ using Microsoft.AspNetCore.Mvc; namespace ErsatzTV.Controllers.Api; [ApiController] -public class ChannelController(ChannelWriter workerChannel, IMediator mediator) +public class ChannelController( + ChannelWriter workerChannel, + IMediator mediator, + IEntityLocker entityLocker) { [HttpGet("/api/channels")] [EndpointGroupName("general")] @@ -192,6 +196,7 @@ public class ChannelController(ChannelWriter workerCh [EndpointGroupName("general")] [ProducesResponseType(StatusCodes.Status200OK)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] public async Task ResetPlayout( string channelNumber, [FromQuery] PlayoutBuildMode? mode, @@ -201,6 +206,14 @@ public class ChannelController(ChannelWriter workerCh await mediator.Send(new GetPlayoutIdByChannelNumber(channelNumber), cancellationToken); foreach (int playoutId in maybePlayoutId) { + // Mirror Blazor's EntityLocker gating: don't enqueue a rebuild while one is already in flight. + if (entityLocker.IsPlayoutLocked(playoutId)) + { + return ApiResults.ConflictProblem( + "Playout build in progress", + "A build for this playout is currently in progress; try again once it completes."); + } + PlayoutBuildMode buildMode = mode ?? await DefaultResetMode(playoutId, cancellationToken); await workerChannel.WriteAsync(new BuildPlayout(playoutId, buildMode), cancellationToken); return new OkResult(); diff --git a/ErsatzTV/Controllers/Api/PlayoutController.cs b/ErsatzTV/Controllers/Api/PlayoutController.cs index d7da83203..a0bff515f 100644 --- a/ErsatzTV/Controllers/Api/PlayoutController.cs +++ b/ErsatzTV/Controllers/Api/PlayoutController.cs @@ -10,6 +10,7 @@ using ErsatzTV.Core.Api.Playouts; using ErsatzTV.Core.Api.Scheduling; using ErsatzTV.Core.Domain; using ErsatzTV.Core.Domain.Filler; +using ErsatzTV.Core.Interfaces.Locking; using ErsatzTV.Extensions; using MediatR; using Microsoft.AspNetCore.Http; @@ -18,10 +19,21 @@ using Microsoft.AspNetCore.Mvc; namespace ErsatzTV.Controllers.Api; [ApiController] -public class PlayoutController(IMediator mediator) : ControllerBase +public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : ControllerBase { private const int MaxPageSize = 100; + // Blazor disables per-playout Reset/Erase/Delete/Edit while a build is in flight + // (EntityLocker.IsPlayoutLocked); the API mirrors that invariant by rejecting any + // id-keyed mutation with 409 while the build lock is held. See docs/decisions.md 2026-07-10. + private const string BuildInProgressTitle = "Playout build in progress"; + + private const string BuildInProgressDetail = + "A build for this playout is currently in progress; try again once it completes."; + + private static IActionResult PlayoutLockedProblem() => + ApiResults.ConflictProblem(BuildInProgressTitle, BuildInProgressDetail); + [HttpGet("/api/playouts", Name = "GetPlayouts")] [Tags("Playouts")] [EndpointSummary("List playouts")] @@ -37,7 +49,7 @@ public class PlayoutController(IMediator mediator) : ControllerBase await mediator.Send(new GetPagedPlayouts(query, pageNum, pageSize), cancellationToken); return new PagedPlayoutsResponseModel( result.TotalCount, - result.Page.Map(ToListItemResponse).ToList()); + result.Page.Map(vm => ToListItemResponse(vm, entityLocker.IsPlayoutLocked(vm.PlayoutId))).ToList()); } [HttpGet("/api/playouts/warnings/count", Name = "GetPlayoutWarningsCount")] @@ -131,12 +143,18 @@ public class PlayoutController(IMediator mediator) : ControllerBase [EndpointGroupName("general")] [ProducesResponseType(typeof(PlayoutResponseModel), StatusCodes.Status200OK)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task Update( int id, [Required] [FromBody] UpdatePlayoutDetailsRequest request, CancellationToken cancellationToken) { + if (entityLocker.IsPlayoutLocked(id)) + { + return PlayoutLockedProblem(); + } + Option maybePlayout = await mediator.Send(new GetPlayoutById(id), cancellationToken); if (maybePlayout.IsNone) { @@ -204,12 +222,18 @@ public class PlayoutController(IMediator mediator) : ControllerBase [EndpointGroupName("general")] [ProducesResponseType(typeof(PlayoutResponseModel), StatusCodes.Status200OK)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task UpdateDefaultDeco( int id, [Required] [FromBody] UpdateDefaultDecoRequest request, CancellationToken cancellationToken) { + if (entityLocker.IsPlayoutLocked(id)) + { + return PlayoutLockedProblem(); + } + Option maybePlayout = await mediator.Send(new GetPlayoutById(id), cancellationToken); if (maybePlayout.IsNone) { @@ -284,12 +308,18 @@ public class PlayoutController(IMediator mediator) : ControllerBase [EndpointGroupName("general")] [ProducesResponseType(typeof(List), StatusCodes.Status200OK)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task ReplaceAlternateSchedules( int id, [Required] [FromBody] ReplacePlayoutAlternateSchedulesRequest request, CancellationToken cancellationToken) { + if (entityLocker.IsPlayoutLocked(id)) + { + return PlayoutLockedProblem(); + } + Option maybePlayout = await mediator.Send(new GetPlayoutById(id), cancellationToken); if (maybePlayout.IsNone) { @@ -376,12 +406,18 @@ public class PlayoutController(IMediator mediator) : ControllerBase [EndpointGroupName("general")] [ProducesResponseType(typeof(List), StatusCodes.Status200OK)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task ReplaceTemplates( int id, [Required] [FromBody] ReplacePlayoutTemplatesRequest request, CancellationToken cancellationToken) { + if (entityLocker.IsPlayoutLocked(id)) + { + return PlayoutLockedProblem(); + } + Option maybePlayout = await mediator.Send(new GetPlayoutById(id), cancellationToken); if (maybePlayout.IsNone) { @@ -515,6 +551,9 @@ public class PlayoutController(IMediator mediator) : ControllerBase [EndpointSummary("Reset all playouts")] [EndpointGroupName("general")] [ProducesResponseType(StatusCodes.Status202Accepted)] + // No 409 lock guard here (unlike the id-keyed mutations): ResetAllPlayoutsHandler already + // skips any playout whose build lock is held, matching Blazor. It is a fire-and-forget + // bulk enqueue, so it always accepts. See docs/decisions.md 2026-07-10. public async Task ResetAll(CancellationToken cancellationToken) { await mediator.Send(new ResetAllPlayouts(), cancellationToken); @@ -531,9 +570,15 @@ public class PlayoutController(IMediator mediator) : ControllerBase [EndpointGroupName("general")] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task EraseItems(int id, CancellationToken cancellationToken) { + if (entityLocker.IsPlayoutLocked(id)) + { + return PlayoutLockedProblem(); + } + Option maybePlayout = await mediator.Send(new GetPlayoutById(id), cancellationToken); if (maybePlayout.IsNone) { @@ -563,9 +608,15 @@ public class PlayoutController(IMediator mediator) : ControllerBase [EndpointGroupName("general")] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task EraseItemsAndHistory(int id, CancellationToken cancellationToken) { + if (entityLocker.IsPlayoutLocked(id)) + { + return PlayoutLockedProblem(); + } + Option maybePlayout = await mediator.Send(new GetPlayoutById(id), cancellationToken); if (maybePlayout.IsNone) { @@ -608,9 +659,15 @@ public class PlayoutController(IMediator mediator) : ControllerBase [EndpointGroupName("general")] [ProducesResponseType(StatusCodes.Status204NoContent)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)] + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status422UnprocessableEntity)] public async Task Delete(int id, CancellationToken cancellationToken) { + if (entityLocker.IsPlayoutLocked(id)) + { + return PlayoutLockedProblem(); + } + Either result = await mediator.Send(new DeletePlayout(id), cancellationToken); return result.ToDeletedResult(); } @@ -722,7 +779,7 @@ public class PlayoutController(IMediator mediator) : ControllerBase vm.EndDay, vm.EndYear); - private static PlayoutListItemResponseModel ToListItemResponse(PlayoutNameViewModel vm) => + private static PlayoutListItemResponseModel ToListItemResponse(PlayoutNameViewModel vm, bool isLocked) => new( vm.PlayoutId, vm.ChannelNumber, @@ -731,7 +788,8 @@ public class PlayoutController(IMediator mediator) : ControllerBase vm.ScheduleName, vm.DbDailyRebuildTime, ToBuildStatus(vm.BuildStatus), - vm.PlayoutMode); + vm.PlayoutMode, + isLocked); private static PlayoutBuildStatusResponseModel ToBuildStatus(PlayoutBuildStatus buildStatus) => buildStatus is null diff --git a/ErsatzTV/Extensions/ApiResults.cs b/ErsatzTV/Extensions/ApiResults.cs index 8ec9a331f..ced802a41 100644 --- a/ErsatzTV/Extensions/ApiResults.cs +++ b/ErsatzTV/Extensions/ApiResults.cs @@ -50,6 +50,13 @@ public static class ApiResults public static IActionResult NotFoundProblem(string detail = "Resource not found") => new NotFoundObjectResult(CreateProblemDetails(404, "Resource not found", detail)); + /// + /// 409 directly — for a mutation that races a background operation + /// holding a lock (e.g. a playout build in flight). Mirrors . + /// + public static IActionResult ConflictProblem(string title, string detail) => + new ConflictObjectResult(CreateProblemDetails(409, title, detail)); + private static ProblemDetails CreateProblemDetails(int status, string title, string detail) => new() { diff --git a/ErsatzTV/wwwroot/openapi/v1.json b/ErsatzTV/wwwroot/openapi/v1.json index 847e90c5a..48d6b54aa 100644 --- a/ErsatzTV/wwwroot/openapi/v1.json +++ b/ErsatzTV/wwwroot/openapi/v1.json @@ -1764,6 +1764,26 @@ } } } + }, + "409": { + "description": "Conflict", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } } } } @@ -7359,6 +7379,26 @@ } } }, + "409": { + "description": "Conflict", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Unprocessable Entity", "content": { @@ -7421,6 +7461,26 @@ } } }, + "409": { + "description": "Conflict", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Unprocessable Entity", "content": { @@ -7616,6 +7676,26 @@ } } }, + "409": { + "description": "Conflict", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Unprocessable Entity", "content": { @@ -7822,6 +7902,26 @@ } } }, + "409": { + "description": "Conflict", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Unprocessable Entity", "content": { @@ -8028,6 +8128,26 @@ } } }, + "409": { + "description": "Conflict", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Unprocessable Entity", "content": { @@ -8353,6 +8473,26 @@ } } }, + "409": { + "description": "Conflict", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Unprocessable Entity", "content": { @@ -8419,6 +8559,26 @@ } } }, + "409": { + "description": "Conflict", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, "422": { "description": "Unprocessable Entity", "content": { @@ -19667,7 +19827,8 @@ "scheduleName", "dailyRebuildTime", "buildStatus", - "playoutMode" + "playoutMode", + "isLocked" ], "type": "object", "properties": { @@ -19706,6 +19867,9 @@ }, "playoutMode": { "$ref": "#/components/schemas/ChannelPlayoutMode" + }, + "isLocked": { + "type": "boolean" } } }, diff --git a/docs/api-conventions.md b/docs/api-conventions.md index 2a89fe5b0..f83857f15 100644 --- a/docs/api-conventions.md +++ b/docs/api-conventions.md @@ -67,6 +67,27 @@ hand-rolling `IActionResult` status codes: | `ToDeletedResult()` | `Either` | `Left` → `ToErrorResult()`; `Right` → 204 | | `ToGetResult()` | `Option` | `Some` → 200 + body; `None` → 404 | | `ApiResults.NotFoundProblem(detail?)` | — | 404 `ProblemDetails` directly (e.g. when a controller has to pre-check existence itself, see `TemplateController.DeleteGroup`) | +| `ApiResults.ConflictProblem(title, detail)` | — | 409 `ProblemDetails` directly — for a mutation that races a background operation holding a lock (see §3a) | + +### 3a. 409 when a mutation races a background lock + +When an endpoint mutates an entity that a background operation may be actively rebuilding under an +`IEntityLocker` lock, guard the mutation and return **409 Conflict** (`ApiResults.ConflictProblem`) +while the lock is held, rather than letting the write race the build. This mirrors the Blazor UI, +which disables the same actions while the lock event is live. + +Established by issue #215 (`PlayoutController` + `ChannelController.ResetPlayout`): inject +`IEntityLocker`, and at the top of every id-keyed mutation (`PUT`/`POST`/`DELETE`) check +`IsPlayoutLocked(id)` → `ConflictProblem("Playout build in progress", ...)`; add +`[ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)]` to each guarded +action. Precedent for the 409 shape: `TraktController` (its private `ConflictProblem()`). Two nuances: +- **Fire-and-forget bulk operations don't 409** — `POST /api/playouts/reset-all` stays 202; its + handler (`ResetAllPlayoutsHandler`) already *skips* locked playouts, matching Blazor + the handler + semantics. Only per-id mutations 409. +- **Surface the lock state to clients** so they can pre-disable the buttons: stamp an `IsLocked` + boolean onto the list DTO (`PlayoutListItemResponseModel`, set from `IsPlayoutLocked` in the + controller's list projection) rather than adding a push channel. The SPA reads it and, on a 409, + refreshes the list to pick up the flag. `NotFoundError : BaseError` lives in `ErsatzTV.Core/Errors/NotFoundError.cs` — return it from a handler's validation when a lookup fails, so the controller-side mapping falls out for free. diff --git a/docs/blazor-route-parity.md b/docs/blazor-route-parity.md index bb617695a..da8405fb3 100644 --- a/docs/blazor-route-parity.md +++ b/docs/blazor-route-parity.md @@ -59,7 +59,7 @@ redirect). > | Filler presets / Trakt / FFmpeg profiles | PARITY-OK | — | > | Watermarks | PARITY-OK (copy via `/add?from=` prefill, 2026-07-09) | — | > | Playout creation + alternate-schedules | PARITY-OK | — | -> | Playout kind-editors + list actions | PARITY-OK (delete/reset/erase/scheduling-context wired + templates preview calendar, 2026-07-09) | — | +> | Playout kind-editors + list actions | PARITY-OK (delete/reset/erase/scheduling-context wired + templates preview calendar, 2026-07-09; EntityLocker build-lock gating enforced server-side via 409 + mirrored in SPA `IsLocked`, 2026-07-10 #215) | — | > | Multi/rerun collections, playlists, trash | PARITY-OK (trash select-all/clear added; 100/kind cap → decisions.md) | — | > | Collections | PARITY-OK (custom-order endpoint + reorder UI, all-10-kind add picker, 2026-07-09) | — | > | Channel editor | GAPS (external logo URL, bare create, pickers) | #212 | @@ -192,6 +192,12 @@ query-wide Add All via `GET /api/search/all-items`, Save As Smart Collection) an pages + child grids, per-show Quick/Deep scan gated to Plex/Jellyfin/Emby, per-episode Media Info + Troubleshoot entries; `POST /api/playlists/{id}/items` added). Known accepted deviations (select-mode toggle, per-card target superset) recorded in `docs/decisions.md`. +CLOSED 2026-07-10: **#215** (adversarial-reviewer#18 removal gate) — Blazor's `EntityLocker` +build-lock gating of per-playout Reset/Erase/Delete/Edit is now enforced server-side: every +id-keyed `PlayoutController` mutation + `ChannelController.ResetPlayout` returns **409** while +`IsPlayoutLocked(id)`, and the SPA mirrors the lock via an `IsLocked` flag on the playout list +DTO (disables the buttons + shows a "Building…" cue). The safety invariant no longer depends on +Blazor, so its removal can't silently drop it. See `docs/api-conventions.md` §3a + `decisions.md`. ## Section 4 — Blazor home / escape hatch diff --git a/docs/decisions.md b/docs/decisions.md index 7dc4f6dec..d1180e032 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -225,3 +225,32 @@ TelevisionShow/TelevisionSeason/Artist per-media-item CollectionTypes and 422s t "Add All" (query-wide) mirrors Blazor's two-step: materialize ids via `GET /api/search/all-items`, then reuse the id-list add endpoints — no query-based add command exists server-side. Issues #208/#209. + +## 2026-07-10 — Playout API mutations return 409 while the build lock is held (#215) + +Blazor disabled per-playout Reset/Erase/Delete/Edit while a `BuildPlayout` was in flight +(`EntityLocker.IsPlayoutLocked`, `Playouts.razor` + per-kind editors); the REST API had no +equivalent, so a client could race an in-flight build with a destructive `ExecuteDelete` and leave +a half-built playout. Adversarial-reviewer#18 promoted this to a #91-phase-(b) removal gate: after +Blazor is deleted the invariant would vanish entirely. + +Decision: enforce the invariant **server-side** on the API rather than re-implementing a live push +channel. `PlayoutController` and `ChannelController` inject `IEntityLocker`; every id-keyed mutation +— `PUT /api/playouts/{id}`, `.../deco`, `.../alternate-schedules`, `.../templates`, +`POST .../erase-items`, `.../erase-items-and-history`, `DELETE /api/playouts/{id}`, and +`POST /api/channels/{channelNumber}/playout/reset` — checks `IsPlayoutLocked(id)` first and returns +**409 Conflict** (`ApiResults.ConflictProblem`, new shared helper mirroring `NotFoundProblem`) while +the build lock is held. The PUTs are gated too (not just the destructive ops): the invariant is "no +mutation during a build", matching Blazor's edit-disable. + +- **`reset-all` is deliberately NOT gated** — it stays 202. `ResetAllPlayoutsHandler` already + *silently skips* locked playouts, which matches Blazor and the handler semantics; a fire-and-forget + bulk enqueue always accepts. +- **SPA mirrors the lock via data, not a push channel** — `PlayoutListItemResponseModel` gains an + `IsLocked` bool (set from `IsPlayoutLocked` in the controller's list projection). The playouts + screen disables Reset/Erase/Erase-and-history/Delete for a locked row and shows a "Building…" + Badge; on a 409 from any mutation it surfaces the error and calls `query.refresh()` so the row + picks up the flag. No new polling was added (the existing 30s channel-state poll is unchanged). + +Precedent for the 409 shape: `TraktController` (left as-is with its own private `ConflictProblem()` +to keep the diff small). Convention recorded in `api-conventions.md` §3a. diff --git a/web/src/App.test.tsx b/web/src/App.test.tsx index 9d6bb692e..fd6beb59d 100644 --- a/web/src/App.test.tsx +++ b/web/src/App.test.tsx @@ -2005,6 +2005,25 @@ describe('ChicoryTV SPA scaffold', () => { expect(screen.queryByRole('button', { name: 'Erase items' })).not.toBeInTheDocument(); }); + it('disables mutation buttons and shows a Building cue for a locked (building) playout', async () => { + mockDashboardApi({ + playoutItems: [playoutItem()], + playoutDetails: playout({ id: 20, scheduleKind: 'Block' }), + playouts: { page: [listPlayout({ id: 20, isLocked: true, scheduleKind: 'Block' })], totalCount: 1 } + }); + + render(); + + fireEvent.click(screen.getByRole('link', { name: /Playouts/ })); + expect(await screen.findByRole('heading', { name: 'Retro Cartoons' })).toBeInTheDocument(); + + expect(screen.getByText('Building…')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Reset' })).toBeDisabled(); + expect(screen.getByRole('button', { name: 'Erase items' })).toBeDisabled(); + expect(screen.getByRole('button', { name: 'Erase items and history' })).toBeDisabled(); + expect(screen.getByRole('button', { name: 'Delete' })).toBeDisabled(); + }); + it('disables Alternate schedules for an on-demand Classic playout', async () => { mockDashboardApi({ playoutItems: [playoutItem()], @@ -3531,6 +3550,7 @@ function listPlayout(overrides: Record = {}): Record { setMutationError(messageFromError(error)); + // A 409 means a build lock is now held for this playout; refresh so the row + // picks up its IsLocked state and the mutation buttons disable themselves. + if (error instanceof ApiError && error.status === 409) { + query.refresh(); + } }) .finally(() => { setMutatingState(false); @@ -3194,6 +3200,9 @@ function PlayoutsScreen() { } const selectedState = channelStates.find((state) => state.channelNumber === selectedSummary.channelNumber); + // The playout's build lock is held (a build is in flight). Server rejects destructive + // mutations with 409 while locked; mirror that by disabling the buttons here. + const selectedLocked = selectedSummary.isLocked; const nowPlaying = selectedState?.nowPlaying ?? null; const nowItem = itemMatchingNow(items, nowPlaying?.title) ?? items[0] ?? null; const nextItem = nextPlayoutItem(items, nowItem); @@ -3258,7 +3267,7 @@ function PlayoutsScreen() {
- {selectedSummary.channelNumber} {selectedState?.onAir && On air} + {selectedSummary.channelNumber} {selectedState?.onAir && On air} {selectedLocked && Building…}

{selectedSummary.channelName}

@@ -3346,20 +3355,22 @@ function PlayoutsScreen() { )}
{selectedSummary.scheduleKind === 'Block' && ( )} )}
+ {error && (