diff --git a/CLAUDE.md b/CLAUDE.md index be4dfc036..792aa0cfb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -5,7 +5,7 @@ Custom IPTV channel server for Jellyfin. Forked from [ErsatzTV/ErsatzTV](https:/ ## Architecture - **Language**: C# / .NET 10 -- **UI**: ChicoryTV React SPA (`web/`, Vite, served at `/app`) over the REST API — the default UI; root `/` and migrated legacy routes 302 there (`ErsatzTV/LegacyUiRedirects.cs`). The legacy Blazor Server UI (MudBlazor) still serves un-migrated admin screens (media detail pages + image browser, block-playout/playback troubleshooting + YAML validator, multi/rerun collections + playlists; Blazor home = `/system/health`, reachable via the Settings → System "Classic UI" link); its removal is #91 phase (b), gated on #141/#145 leftovers and API gaps #151–#153/#155/#158/#161 (scheduling parity #144/#162 DONE 2026-07-07: blocks/templates/decos/deco-templates/playout editors all in the SPA) +- **UI**: ChicoryTV React SPA (`web/`, Vite, served at `/app`) over the REST API — the default UI; root `/` and migrated legacy routes 302 there (`ErsatzTV/LegacyUiRedirects.cs`). The legacy Blazor Server UI (MudBlazor) still serves the remaining un-migrated admin screens — playback troubleshooting, multi/rerun collections, and playlist editing depth; Blazor home = `/system/health`, reachable via the Settings → System "Classic UI" link. Media detail pages + image folder browser landed in the SPA via #141 (PR #183); its removal is #91 phase (b), gated on #145 (playback troubleshooting) and API gaps #151/#152/#153/#155 (scheduling parity #144/#162 DONE 2026-07-07: blocks/templates/decos/deco-templates/playout editors all in the SPA; #141/#158/#161/#180 also DONE) - **Pattern**: CQRS via MediatR — queries/commands in `ErsatzTV.Application/` - **Database**: EF Core (SQLite default, MySQL optional) — context in `ErsatzTV.Infrastructure/Data/TvContext.cs` - **Media**: FFmpeg via CliWrap, SkiaSharp for logo generation @@ -56,6 +56,7 @@ docker build -f docker/Dockerfile -t ersatztv:dev . ## Conventions - **Read [`docs/contributing.md`](docs/contributing.md)** before non-trivial changes — it documents the established patterns (layering, CQRS handlers, LanguageExt, Blazor/MudBlazor, EF Core + dual-provider migrations, the FFmpeg pipeline, analyzers, testing) and the **deviation policy**: match the established style; diverge only with a concrete, stated reason. +- **Convention docs replace re-recon**: before API/SPA/E2E/parity work, read `docs/README.md` (index) → `docs/api-conventions.md`, `docs/spa-conventions.md`, `docs/e2e-local.md`, `docs/domain-model.md`, `docs/blazor-route-parity.md`, `docs/decisions.md`. Any PR that changes a convention, migrates a route, or reverses a decision MUST update the relevant doc in the same PR. - Follow existing MediatR CQRS pattern for new features - Domain logic in `ErsatzTV.Core`, infrastructure in `ErsatzTV.Infrastructure` - Keep UI thin: the SPA talks to `/api/*` only; legacy Blazor pages delegate to MediatR handlers. New screens go in the SPA (`web/`), never in Blazor diff --git a/docs/README.md b/docs/README.md new file mode 100644 index 000000000..8487cd05c --- /dev/null +++ b/docs/README.md @@ -0,0 +1,37 @@ +# docs/ — reading order + +Purpose: index of `docs/` so a fresh contributor/agent knows what to read and in what order. +**Update this doc in the same PR that adds, removes, or retitles a doc below.** + +Read in this order at session start: + +1. **`CLAUDE.md`** (repo root) — project intro: architecture, layout, dev commands, conventions. +2. **`docs/contributing.md`** — established code patterns (CQRS/MediatR, LanguageExt, Blazor/ + MudBlazor, EF Core dual-provider migrations, FFmpeg pipeline, analyzers, testing). Read before + any non-trivial change. +3. **`docs/domain-model.md`** — what the app IS: entity glossary, channel→playout→schedule/block + concept map, where each concept is edited in the SPA. +4. **`docs/api-conventions.md`** — checklist for adding/changing a `/api/*` endpoint (controllers, + DTOs, error mapping, auth, OpenAPI regen, tests). +5. **`docs/spa-conventions.md`** — playbook for adding a screen to the ChicoryTV React SPA. +6. **`docs/e2e-local.md`** (+ `scripts/e2e-local.sh`) — how to run a live local instance for manual + or Playwright-MCP verification. +7. **`docs/blazor-route-parity.md`** — the #91 phase (b) tracker: which Blazor routes are + redirected, SPA-ready-but-not-redirected, or still Blazor-only (and which issue blocks each). +8. **`docs/decisions.md`** — append-only "why" log. Check here before challenging an existing + convention. +9. **`docs/ci-cd.md`** — build/test/release pipeline, versioning, dependency management. + +Also present in `docs/`: + +- **`docs/rest-api.md`** — REST API design doc for ersatztv#2 (goals, conventions, per-slice plan). + Largely superseded day-to-day by `docs/api-conventions.md`; read this for the original rationale. +- **`docs/channels.md`** — Channel entity field reference. +- **`docs/m3u-xmltv.md`** — M3U/XMLTV generation overview (`ChannelPlaylist`, `GetChannelGuideHandler`). +- **`docs/fork-strategy.md`** — divergence policy vs upstream ErsatzTV. +- **`docs/design-sync.md`** — Claude Design ↔ repo screen workflow (#92). +- **`docs/handoffs/chicorytv-issue-queue.md`** — living session-to-session handoff: current queue + state, what's next. Check this for what's actively in flight before starting new work. +- **`docs/handoffs/rest-api.md`** — original handoff prompt for kicking off the REST API work (#2). + +Still to come (tracked under #185): a testing map and a generated-endpoint index. diff --git a/docs/api-conventions.md b/docs/api-conventions.md new file mode 100644 index 000000000..0add7c508 --- /dev/null +++ b/docs/api-conventions.md @@ -0,0 +1,165 @@ +# API conventions — "Add a REST endpoint" checklist + +Purpose: a precise, file-path-and-exemplar checklist for adding or changing a `/api/*` endpoint on +the ErsatzTV fork, so an agent with no prior context in this repo can do it correctly on the first +pass. **Update this doc in the same PR that changes any convention below.** + +This is a companion to `docs/contributing.md` (general CQRS/LanguageExt/testing conventions) and +`docs/ci-cd.md` (build/release pipeline) — read those first for anything not API-specific. + +## 1. Controller shape + +Controllers live in `ErsatzTV/Controllers/Api/*.cs`, one per domain (e.g. `TemplateController.cs`, +`BlockController.cs`, `LogsController.cs`). Every action needs this attribute set: + +- `[ApiController]` on the class. +- `[HttpGet/Post/Put/Delete("/api/...")]` on the action, with `Name = "..."` on at least the + primary GET (used by the SPA's OpenAPI-generated client and by route-assertion tests). +- `[Tags("Domain")]` — groups the endpoint in Swagger UI / the SPA's generated client namespace. +- `[EndpointSummary("...")]` — one-line description; optionally `[EndpointDescription("...")]` for + more detail. +- `[EndpointGroupName("general")]` — **REQUIRED**. Endpoints without it are excluded from the + generated OpenAPI document entirely (verified: every action in `TemplateController.cs` and + `LogsController.cs` has it; there is no other endpoint group in use). +- `[ProducesResponseType(typeof(X), StatusCodes.StatusYyy)]` for every status code the action can + actually return (200/201/204 success, plus 404/422 as applicable) — this drives the generated + TypeScript response types on the SPA side. + +Exemplars: +- **Full CRUD controller**: `ErsatzTV/Controllers/Api/TemplateController.cs` — group CRUD + + item CRUD + copy, all four verbs, `ToCreatedResult`/`ToErrorResult` usage. +- **Paged GET with clamped params**: `ErsatzTV/Controllers/Api/LogsController.cs` — + `pageNum` clamped via `Math.Max(0, pageNum)`, `pageSize` via `Math.Clamp(pageSize, 1, MaxPageSize)` + (`MaxPageSize = 100`). Any new paged endpoint should clamp the same way — don't trust client + input for page math. + +## 2. DTOs: where they live and their nullable context + +- **Response DTOs**: `ErsatzTV.Core/Api//*ResponseModel.cs`. They are `record`s that mirror + the shape of the Application-layer ViewModel for that domain — **never expose a VM type directly** + from a controller. Convert framework types the SPA can't consume as-is (e.g. `CultureInfo` → + its name string). + - Most response-model files start with `#nullable enable` (verified: 60 of 72 files under + `ErsatzTV.Core/Api/`, e.g. `ErsatzTV.Core/Api/Scheduling/BlockResponseModel.cs`, + `ErsatzTV.Core/Api/Settings/*ResponseModel.cs`). A minority of the simplest ones (e.g. + `FFmpegProfileResponseModel.cs`) have no pragma and rely on the project default. Check + `ErsatzTV.Core/ErsatzTV.Core.csproj` — the project sets `disable` — so + **add `#nullable enable` at the top of any new response-model file** that has an optional + (nullable) member; don't rely on the project default. +- **Request DTOs**: `ErsatzTV/Controllers/Api/Requests/*Request.cs`. No `#nullable enable` pragma — + match the existing files (e.g. `CreateFFmpegProfileRequest.cs`, `ReplaceScheduleItemsRequest.cs`). + A request record typically carries a `ToCommand()` (or `ToCommand(int parentId)`, + `ToReplaceCommand(int index)`) method that maps it to the Application-layer command type. +- **`ErsatzTV.Application` has no nullable context** (no `` = C# default `disable` for + that TFM in this repo — confirms CS8632 would otherwise fire) — do **not** add `?` nullable + annotations to types living there; that's a Core/Api-layer-only convention. + +## 3. Error mapping + +Central helper: `ErsatzTV/Extensions/ApiResults.cs`. Use these extension methods instead of +hand-rolling `IActionResult` status codes: + +| Method | Input | Output | +|---|---|---| +| `ToErrorResult()` | `BaseError` | 404 if `NotFoundError`, else 422 (`ProblemDetails`) | +| `ToCreatedResult(location, body)` | `Either` | `Left` → `ToErrorResult()`; `Right` → 201 + `Location` header | +| `ToUpdatedResult()` | `Either` | `Left` → `ToErrorResult()`; `Right` → 200 + body | +| `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`) | + +`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. + +**Handler-hardening checklist** when you touch (or add) a handler behind a new/changed endpoint: +- Missing FK existence checks that would otherwise 500 → convert to a `NotFoundError` (404) or a + validation `BaseError` (422). +- Dictionary-indexer lookups (`dict[key]`) that can throw `KeyNotFoundException` → guard or use + `TryGetValue`. +- Silent item filtering (e.g. `.Where(x => x.IsValid)` dropping bad rows without telling the + caller) → surface as 422 instead of silently returning a shorter list. +- Unbounded `int`/`TimeSpan` inputs from the request → clamp or validate, per the Logs pagination + pattern above. +- **Known, deliberate exception**: deep FK ids nested inside item-list request bodies (e.g. a + schedule item's `CollectionId`) are **not** existence-checked at that depth — this is established + precedent from the schedules endpoints (see issue #172) and intentional to avoid N+1 validation + queries; don't "fix" this without discussing it first. + +## 4. Artwork contract + +API response DTOs return **rooted, directly-usable artwork URLs** — e.g. `/artwork/posters/...`, +`/artwork/thumbnails/...`, plus passthrough for `http://`/`https://` absolute URLs and for +Jellyfin/Emby proxy variants. This was established by PR #181 in +`ErsatzTV.Application/LibraryBrowse/Queries/GetLibraryBrowseItemsHandler.cs` (private `Artwork(...)` +helper, ~line 1330) — copy that helper's logic (or call a shared version of it) for any new +API surface that returns artwork paths. Comment in that file: *"Returns a rooted, directly-usable +artwork URL for the SPA's ``. Blazor pages rely on GetPosterUrl to prefix +`artwork/posters/`... but the SPA [needs it pre-rooted]."* + +Do **not** reuse the Application-layer Mappers used by Blazor (e.g. `MediaCards`/`Television` +mappers) for new API DTOs — those still return the old Blazor-convention relative paths. Map from +the domain/VM directly and root the path yourself, following the PR #181 pattern. + +## 5. OpenAPI regeneration — commit both generated artifacts + +After any controller/DTO change: +1. `dotnet build ErsatzTV.sln` (normal build first). +2. `./scripts/update-openapi.sh` — runs `dotnet build -t:GenerateOpenApiDocuments` from `ErsatzTV/`, + regenerating `ErsatzTV/wwwroot/openapi/v1.json`. +3. `cd web && npm run generate:api` — runs `scripts/generate-openapi-types.mjs`, regenerating + `web/src/api/generated/v1.d.ts`. + +**Commit both** `ErsatzTV/wwwroot/openapi/v1.json` and `web/src/api/generated/v1.d.ts` — the `.d.ts` +file is what the SPA actually imports (`web/src/api/*.ts` files do +`import type { components } from './generated/v1'`), and CI enforces it stays in sync +(`npm run check:api` = regenerate + `git diff --exit-code`). + +**Known type-generation wart**: `DayOfWeek` serializes as an integer in the OpenAPI schema but the +runtime JSON payload is actually the enum's **name string** ("Sunday".."Saturday") — the generator +gets this wrong. The SPA works around it with a manual override type; see `web/src/api/playouts.ts` +(`type WithDayNames = Omit & { daysOfWeek: DayOfWeek[] }`, applied to +`PlayoutAlternateSchedule`, `PlayoutTemplate`, and their request types). Copy this pattern for any +new DTO with a `DayOfWeek` (or `DayOfWeek[]`) member — don't trust the generated numeric type. + +## 6. Tests + +- **Controller tests**: `ErsatzTV.Tests/Controllers/ControllerTests.cs`. NUnit + Shouldly + + NSubstitute (mock `IMediator`). Exemplar: `ErsatzTV.Tests/Controllers/TemplateControllerTests.cs` + — asserts every route via a `ShouldHaveActionRoute(actionName, verb, path)` helper (route-table + regression net), then per-action tests asserting the DTO shape returned and the exact + `mediator.Received(1).Send(Arg.Is(...))` call. `PlayoutControllerTests.cs` is the same + pattern for a larger, mixed-verb controller — use it as the template for a new controller with + many actions. +- **`ApiControllerSecurityTests.cs`** (`ErsatzTV.Tests/Controllers/`): has a hardcoded + `Type[] apiControllers` registry. **Add every new controller to that array** — the test walks + every mutating (POST/PUT/PATCH/DELETE) action on each listed controller and asserts it's covered + by the global `ApiKeyAuthorizationFilter` (or has an explicit `[SkipApiKeyAuthorizationAttribute]` + exemption; only `ScannerController` is exempt today). **Note**: as of this writing the array does + not include every controller under `ErsatzTV/Controllers/Api/` (e.g. `ArtworkUploadController`, + `ChannelTemplateController`, `GraphicsElementController`, `LibraryBrowseController`, + `SearchController`, `VersionController`, `HealthController`, `PlaylistController`, + `MediaSourcesController` are missing) — treat that as a pre-existing gap to close opportunistically, + not a precedent to extend. +- **Handler tests** (business logic behind the controller) use the shared in-memory SQLite fixture: + `ErsatzTV.Tests/Support/InMemoryTvContext.cs`. Pattern: `SqliteConnection("Data Source=:memory:;Foreign + Keys=False")` kept open for the fixture's lifetime, `EnsureCreatedAsync()` (not full migration + replay), then `PRAGMA foreign_keys=OFF` so partial object graphs can be seeded without satisfying + every FK. Exemplar: `ErsatzTV.Tests/Application/LibraryBrowse/GetLibraryBrowseItemsHandlerTests.cs` + (`_db = await InMemoryTvContext.CreateAsync();` in `[SetUp]`, `_db.CreateContext()` per test body, + `_db.Factory` where an `IDbContextFactory` is needed by the handler under test). + +## 7. PUT-replace list endpoints + +For "replace the whole list" endpoints (PUT semantics over a collection, e.g. schedule/template +items), index items from **array order** in the request body rather than trusting a client-supplied +index/order field. Exemplar: `ReplaceScheduleItemsRequest.ToCommand(scheduleId)` — +`Items.Select((item, index) => item.ToReplaceCommand(index))`. + +## 8. Known API warts (don't "fix" without discussion — they're deliberate synthesized rows) + +`GET /api/blocks` and `GET /api/templates` (via `GetAllBlocksHandler` / `GetAllTemplatesHandler` in +`ErsatzTV.Application/Scheduling/Queries/`) synthesize a fake **negative-id "(none)" group row** +for items that have no group, so the SPA can render an "ungrouped" bucket +(`Id = unusedGroup.Id * -1`, `Name = "(none)"`). See issue #172. If you add a similar "ungrouped" +concept elsewhere, this is the established pattern to follow — but be aware it means `Id` is not a +reliable real-entity id for those synthetic rows. diff --git a/docs/blazor-route-parity.md b/docs/blazor-route-parity.md new file mode 100644 index 000000000..361bf5431 --- /dev/null +++ b/docs/blazor-route-parity.md @@ -0,0 +1,130 @@ +# Blazor → SPA route parity tracker + +Purpose: the living route-by-route tracker for ersatztv#91 phase (b) — retiring the legacy Blazor +Server UI once every route it serves has a ChicoryTV SPA equivalent and a redirect. **Update this +doc in the same PR that migrates, redirects, or removes any route below** — this table is the +single source of truth for "what's left." + +Sources of truth checked when compiling this table: `ErsatzTV/LegacyUiRedirects.cs` (redirect map), +`web/src/App.tsx` (SPA route table), `ErsatzTV/Pages/**/*.razor` (Blazor pages still present). + +## How to read this + +- **REDIRECTED**: in `LegacyUiRedirects.cs`'s `Map` — the Blazor route 302s to the SPA route. +- **SPA-READY (not yet redirected)**: the SPA screen exists and covers the functionality, but the + Blazor route is still reachable directly (no redirect entry yet) — adding the redirect is a small, + low-risk PR once the SPA screen has been spot-checked against the Blazor page it replaces. +- **BLAZOR-ONLY**: no SPA equivalent yet; removing/redirecting this route is blocked on the listed + issue(s). + +## Section 1 — REDIRECTED + +(`ErsatzTV/LegacyUiRedirects.cs`'s `Map`, verified current as of this doc — 13 entries, unchanged +since ersatztv#91 phase (a)/PR #148): + +| Blazor route | SPA route | +|---|---| +| `/` | `/app` | +| `/channels` | `/app/channels` | +| `/channels/add` | `/app/new-channel` | +| `/schedules` | `/app/schedules` | +| `/playouts` | `/app/playouts` | +| `/media/libraries` | `/app/libraries` | +| `/settings/ffmpeg` | `/app/settings/streaming` | +| `/settings/hdhr` | `/app/settings/system` | +| `/settings/logging` | `/app/settings/logging` | +| `/settings/playout` | `/app/settings/playout` | +| `/settings/scanner` | `/app/settings/scanner` | +| `/settings/ui` | `/app/settings/general` | +| `/settings/xmltv` | `/app/settings/xmltv` | + +Note: **the settings sub-routes above are already redirected**, not Blazor-only as an earlier draft +of this table implied — `LegacyUiRedirects.cs` covers all seven `/settings/*` pages that have SPA +equivalents. There is no Blazor-only settings sub-route left except the ones with no SPA screen at +all (there are none currently — every Blazor `Settings/*.razor` page has both an SPA screen and a +redirect). + +## Section 2 — SPA-READY, not yet redirected + +Confirmed as of this doc: the SPA screen exists (verified against `web/src/App.tsx`'s route table +and `web/src/screens/`) but `LegacyUiRedirects.cs` has **no entry** for the Blazor route yet. +Scheduling-parity work (#144/#162, DONE 2026-07-07, PRs #170–#175/#179) built the SPA screens for +blocks/templates/decos/deco-templates/playout editors, #145 (PR #182, merged to main) built the +troubleshooting/YAML-validator screens, and #141 (PR #183, merged to main) built the media detail +pages + image folder browser (`MediaDetailScreen.tsx`'s `MovieDetailScreen`/`ShowDetailScreen`/ +`SeasonDetailScreen`/`ArtistDetailScreen`, `ImageBrowserScreen.tsx`, both dispatched via `App.tsx`'s +`MediaRouteScreen` sub-route wrapper, same pattern as `PlayoutsRouteScreen`) — none of these have +been added to the redirect map yet. + +| Blazor route | Blazor file | SPA route | Notes | +|---|---|---|---| +| `/channels/{Id:int?}` | `ChannelEditor.razor` | `/app/edit-channel/{id}` | allowSubPaths | +| `/channels/numbers` | `ChannelNumbers.razor` | `/app/channels` | merged into channels table | +| `/search` | `Search.razor` | `/app/search` | | +| `/system/logs` | `Logs.razor` | `/app/logs` | | +| `/system/troubleshooting` | `Troubleshooting/Troubleshooting.razor` | `/app/troubleshooting` | | +| `/system/troubleshooting/block-playout` | `Troubleshooting/BlockPlayoutTroubleshooting.razor` (+`BlockPlayoutHistory.razor`) | `/app/troubleshooting/blocks` | **covered by PR #182 / #145** | +| `/system/troubleshooting/sequential-schedule` | `Troubleshooting/YamlValidator.razor` | `/app/troubleshooting/yaml` | **covered by PR #182 / #145** | +| `/blocks`, `/blocks/{Id:int}` | `Blocks.razor`, `BlockEditor.razor` | `/app/blocks`(`/{id}`) | allowSubPaths; #144 S1 | +| `/templates`, `/templates/{Id:int}` | `Templates.razor`, `TemplateEditor.razor` | `/app/templates`(`/{id}`) | allowSubPaths; #144 S2 | +| `/decos`, `/decos/{Id:int}` | `Decos.razor`, `DecoEditor.razor` | `/app/decos`(`/{id}`) | allowSubPaths; #144 S3 | +| `/deco-templates`, `/deco-templates/{Id:int}` | `DecoTemplates.razor`, `DecoTemplateEditor.razor` | `/app/deco-templates`(`/{id}`) | allowSubPaths; #144 S4 | +| `/playouts/add`(`/{kind}`) | `PlayoutEditor.razor` variants | `/app/playouts` | merged into playouts screen creation flow; #144 S5 | +| `/playouts/classic/{Id}`, `/playouts/block/{Id}`, `/playouts/scripted/{Id}`, `/playouts/sequential/{Id}` | `ClassicPlayoutEditor.razor`, `BlockPlayoutEditor.razor`, `ScriptedPlayoutEditor.razor`, `SequentialPlayoutEditor.razor` | `/app/playouts` | merged; #144 S5/S6 | +| `/playouts/{Id:int}/alternate-schedules` | `PlayoutAlternateSchedulesEditor.razor` | `/app/playouts/{id}/alternate-schedules` | allowSubPaths (`PlayoutsRouteScreen`); #144 S6/#162 | +| `/playouts/{Id:int}/templates` | `PlayoutTemplatesEditor.razor` | `/app/playouts/{id}/templates` | allowSubPaths (`PlayoutsRouteScreen`); #144 S6/#162 | +| `/schedules/{Id:int}`, `/schedules/add`, `/schedules/{Id:int}/items` | `ScheduleEditor.razor`, `ScheduleItemsEditor.razor` | `/app/schedules` | merged into schedules screen | +| `/media/filler/presets`(`/add`, `/{Id}/edit`) | `FillerPresets.razor`, `FillerPresetEditor.razor` | `/app/filler-presets` | allowSubPaths | +| `/media/collections`(`/add`, `/{Id}/edit`, `/{Id}`) | `ManualCollections.razor`, `CollectionEditor.razor`, `CollectionItems.razor` + `SmartCollections.razor`/`SmartCollectionEditor.razor` | `/app/collections` | allowSubPaths | +| `/media/trash` | `Trash.razor` | `/app/trash` | | +| `/media/playlists`(`/{Id}`) | `Playlists.razor`, `PlaylistEditor.razor` | `/app/collections` | merged into collections screen | +| `/media/trakt/lists`(`/{Id}`) | `TraktLists.razor`, `TraktListEditor.razor` | `/app/trakt-lists`(`/{id}`) | allowSubPaths | +| `/ffmpeg`(`/add`, `/{Id}`) | `FFmpeg.razor`, `FFmpegEditor.razor` | `/app/ffmpeg-profiles` | allowSubPaths | +| `/watermarks`(`/add`, `/{Id}`) | `Watermarks.razor`, `WatermarkEditor.razor` | `/app/watermarks` | allowSubPaths | +| `/media/sources/{local,plex,jellyfin,emby}/...` | `LocalLibraries.razor`, `PlexMediaSources.razor`, `JellyfinMediaSources.razor`, `EmbyMediaSources.razor` + editors | `/app/libraries` | merged into libraries screen | +| `/media/movies`(`/page/{n}`) | `MovieList.razor` | `/app/media?kind=movies` | generic browse (`MediaBrowseScreen`); PR #183 / #141 | +| `/media/movies/{MovieId:int}` | `Movie.razor` | `/app/media/movies/{id}` | detail page (`MovieDetailScreen`); PR #183 / #141 | +| `/media/tv/shows`(`/page/{n}`) | `TelevisionShowList.razor` | `/app/media?kind=shows` | generic browse; PR #183 / #141 | +| `/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=shows` | no dedicated season-list SPA screen; covered via show drill-in; PR #183 / #141 | +| `/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/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 | +| `/media/music/songs`(`/page/{n}`) | `SongList.razor` | `/app/media?kind=songs` | no dedicated SPA song browse beyond generic grid; PR #183 / #141 | +| `/media/other/videos`(`/page/{n}`) | `OtherVideoList.razor` | `/app/media?kind=other-videos` | generic browse; PR #183 / #141 | +| `/media/remote/streams`(`/page/{n}`) | `RemoteStreamList.razor` | `/app/media?kind=remote-streams` | generic browse; PR #183 / #141 | +| `/media/images`(`/page/{n}`) | `ImageList.razor` | `/app/media?kind=images` | generic browse; PR #183 / #141 | +| `/media/browser/images` | `ImageBrowser.razor` | `/app/media/images/browser` | interactive image grid picker used by channel editors etc. (`ImageBrowserScreen`); PR #183 / #141 | + +## Section 3 — BLAZOR-ONLY (blocking issues) + +### Multi/rerun collections & playlist variants — API gaps #151/#152/#153/#155 + +| Blazor route | File | Blocking issue | +|---|---|---| +| `/media/multi-collections`(`/add`, `/{Id}/edit`) | `MultiCollections.razor`, `MultiCollectionEditor.razor` | #151 (multi-collection management API) | +| `/media/rerun-collections`(`/add`, `/{Id}/edit`) | `RerunCollections.razor`, `RerunCollectionEditor.razor` | #152 (rerun-collection management API) | +| `/media/playlists`(`/{Id}`) editing depth beyond what `/app/collections` covers | `Playlists.razor`, `PlaylistEditor.razor` | #153/#155 (playlist variant management API + collection-items editing depth) | + +### Playback troubleshooting — #145 + +| Blazor route | File | Blocking issue | +|---|---|---| +| `/system/troubleshooting/playback` | `Troubleshooting/PlaybackTroubleshooting.razor` | #145 (playback troubleshooting diagnostics API) — distinct from block-playout troubleshooting, which is already covered (Section 2) | + +## Section 4 — Blazor home / escape hatch + +Not gated on an issue — kept separate from Section 3 because it isn't blocked on anything, just the +intentional exit ramp until phase (b) removes Blazor entirely. + +| Blazor route | File | Notes | +|---|---|---| +| `/system/health` | `Index.razor` | Blazor's own home page; reachable via Settings → System "Classic UI" link. | + +## Cross-reference: `docs/handoffs/chicorytv-issue-queue.md` + +Keep this table and that handoff doc in sync at a high level — this table is the detailed route +inventory; the handoff doc tracks issue sequencing/session planning. If they disagree on an issue's +status, the more recently updated one wins; fix the stale one in the same PR you notice it. diff --git a/docs/decisions.md b/docs/decisions.md new file mode 100644 index 000000000..4cea408a1 --- /dev/null +++ b/docs/decisions.md @@ -0,0 +1,98 @@ +# Decisions — append-only log + +Purpose: why the codebase does what it does, so agents don't "fix" an established convention or +relitigate a settled call. Append new entries at the bottom in date order; never edit or delete +past entries except to fix a factual error. **Update this doc in the same PR that changes any fact +below (or that establishes a new convention worth recording).** + +## 2026-06 — REST API wraps existing MediatR handlers 1:1, no service layer + +The REST API (#2, `docs/rest-api.md`) is thin controllers over the existing MediatR +Create/Update/Delete handlers — no new service/business-logic layer was introduced, since nearly +every handler already returns `Either`, which maps cleanly to HTTP status codes. +Latent handler bugs (missing existence checks, `KeyNotFoundException` risk, etc.) are fixed **at +the handler**, converting what would have 500'd into a proper 404/422 — not papered over in the +controller. Established across the #2a–#2e gap-issue PRs. Deep FK ids nested inside item-list +request bodies (e.g. a schedule item's `CollectionId`) are deliberately **not** existence-checked at +that depth, to avoid N+1 validation queries — precedent set by the schedules endpoints (#172); see +`docs/api-conventions.md` §3 for the up-to-date statement of this rule. + +## 2026-06 — UI rebuild is a React SPA (ChicoryTV) on the REST API, not a Blazor reskin + +#59 committed to a full SPA rebuild rather than reskinning Blazor Server pages. Blazor removal is +split into two phases under #91: **(a)** root-flip (SPA becomes `/`) + legacy-route redirects — +DONE, merged via PR #148 (`ErsatzTV/LegacyUiRedirects.cs`, `feat/91-cutover` → main). **(b)** full +Blazor removal — gated on every route having an SPA equivalent; tracked route-by-route in +`docs/blazor-route-parity.md`. + +## 2026-07 — Response DTOs live in `ErsatzTV.Core/Api`, file-scoped `#nullable enable` + +New REST response DTOs go in `ErsatzTV.Core/Api//*ResponseModel.cs` and mirror the shape of +the corresponding Application-layer ViewModel — controllers never expose VM types directly. Because +`ErsatzTV.Core.csproj` sets `disable` project-wide, any response-model file +with an optional member needs its own `#nullable enable` pragma at the top (most already have one). +`ErsatzTV.Application` has no nullable context at all — do not add `?` annotations to types living +there; that's a Core/Api-layer-only convention. Full detail: `docs/api-conventions.md` §2. + +## 2026-07 — PUT-replace list endpoints derive `Index` from array order; alternate-schedules last row = catch-all default + +For "replace the whole list" endpoints (PUT over a collection — schedule items, template items, +etc.), the item's `Index` is derived from its position in the request array, not from a +client-supplied index/order field — established by `ReplaceScheduleItemsRequest.ToCommand` +(`Items.Select((item, index) => item.ToReplaceCommand(index))`). Separately, `ProgramScheduleAlternate` +and `PlayoutTemplate` rows (both `IAlternateScheduleItem`) are evaluated in `Index` order, +first-match-wins; the convention is to place the least-conditional (or unconditional) row **last** +so it acts as the catch-all default. Established by the alternate-schedules work (PR #179, +`AlternateScheduleSelector.cs`). + +## 2026-07 — Templates editor in the SPA is a table, not Blazor's drag-calendar + +The legacy Blazor `TemplateEditor.razor` used a drag-and-drop day-grid calendar UI. The SPA +equivalent (`/app/templates/{id}`, PR #173) renders the same day/block assignment as a table +instead. This is an accepted, deliberate parity deviation — don't "fix" it to match Blazor's +interaction model without discussing it first. + +## 2026-07-07 — API artwork contract: rooted URLs produced server-side + +API response DTOs return artwork as rooted, directly-usable URLs (`/artwork/posters/...`, +`/artwork/thumbnails/...`, `/artwork/fanart/...`), plus passthrough for absolute `http(s)://` URLs +and Jellyfin/Emby proxy variants. Established by PR #181 +(`ErsatzTV.Application/LibraryBrowse/Queries/GetLibraryBrowseItemsHandler.cs`, private `Artwork(...)` +helper — comment: *"Returns a rooted, directly-usable artwork URL for the SPA's ``... the +SPA [needs it pre-rooted]"*), then generalized into the reusable `ApiArtwork` helper +(`ErsatzTV.Core/Api/ApiArtwork.cs`, PR #183). Root cause: the SPA has no ``, unlike +Blazor, so relative artwork paths that worked for Blazor pages 404 in the SPA. Do **not** reuse the +Application-layer Mappers used by Blazor (e.g. `MediaCards`/`Television` mappers) for new API +DTOs — those still return old Blazor-convention relative paths; map from the domain/VM directly and +root the path via `ApiArtwork`. + +## 2026-07-07 — Decode-style endpoints take a row id and look up server-side + +Endpoints that decode/expand opaque stored state accept a database row id and resolve server-side, +rather than accepting client-supplied serialized state to decode. Established by +`GET /api/playouts/history/{id}` (`PlayoutController.GetHistoryDetails`, PR #182) — the row's raw +JSON (`Key`/`Details`) is decoded server-side into `PlayoutHistoryDetailsResponseModel`, the client +never round-trips the raw payload itself. + +## 2026-07-07 — Season/episode/music-video drill-in via `parentId`, not new child-listing endpoints + +Rather than adding dedicated child-listing endpoints per media kind (e.g. "list episodes of a +season"), the library-browse endpoint takes an optional `parentId` query param and the SPA drills +in by re-querying with it. Established across PRs #181/#183 (library-picker season drill-in, then +media-detail's season/episode/artist/music-video browsing). Avoids a combinatorial explosion of +per-kind child endpoints. + +## 2026-07-07 — Convention docs read at session start, updated in-PR + +`docs/api-conventions.md`, `docs/spa-conventions.md`, `docs/e2e-local.md`, +`docs/blazor-route-parity.md`, `docs/domain-model.md`, `docs/decisions.md`, and `docs/README.md` +are the standing reference set every ChicoryTV session should read before starting work, and each +one carries an explicit "update this doc in the same PR" rule rather than deferring doc updates to +a follow-up. These docs **replace per-session recon** — an agent reads the index +(`docs/README.md`) and the relevant convention doc instead of re-deriving conventions from the code +each time it starts API/SPA/E2E/parity work. A testing map and a generated-endpoint index are +tracked as still-to-come under #185. Drafting this doc set also surfaced a drift in +`ApiControllerSecurityTests.cs`'s hardcoded controller registry (several controllers under +`ErsatzTV/Controllers/Api/` are missing from it — see `docs/api-conventions.md` §6) — tracked as a +follow-up under #184 rather than fixed inline, since it's a pre-existing gap, not something this +doc-drafting pass caused. diff --git a/docs/domain-model.md b/docs/domain-model.md new file mode 100644 index 000000000..93e824114 --- /dev/null +++ b/docs/domain-model.md @@ -0,0 +1,112 @@ +# Domain model — glossary + concept map + +Purpose: what the app IS — the entity chain and vocabulary an agent needs before touching +scheduling, playout, or IPTV code. **Update this doc in the same PR that changes any fact below.** + +## The pipeline in one paragraph + +Media libraries (Local/Plex/Jellyfin/Emby) are scanned into media items, which are grouped into +collections/playlists/etc. Those groupings are arranged onto a **playout** by one of several +scheduling engines (classic schedule, block/template calendar, sequential YAML, scripted, +external-JSON) — a playout belongs to exactly one **channel**. Channels are exposed to IPTV clients +(Jellyfin, Dispatcharr) as an M3U playlist + XMLTV guide, and streamed on demand via FFmpeg, all +under the `/iptv/*` routes (`ErsatzTV/Controllers/IptvController.cs`). + +## Entity chain sketch + +``` +Channel (1) ──< Playout (0..N per channel; ChannelPlayoutSource distinguishes Generated vs Mirror) + │ + ├─ ScheduleKind: None/Classic/Block/Sequential/Scripted/ExternalJson + │ (PlayoutScheduleKind, ErsatzTV.Core/Domain/PlayoutScheduleKind.cs) + │ + ├─ Classic: ProgramSchedule ──< ProgramScheduleItem (TPT: One/Multiple/Flood/Duration) + │ └─< ProgramScheduleAlternate (day/date-conditional alt schedule) + │ + ├─ Block: PlayoutTemplate (day/date-conditional, like ProgramScheduleAlternate) + │ ──> Template ──< TemplateItem (time-of-day) ──> Block + │ Block ──< BlockItem (ordered collection items) + │ PlayoutTemplate also optionally points at a DecoTemplate + │ Playout can also point directly at a default Deco (DecoId) + │ + ├─ Sequential: Playout.ScheduleFile (YAML, validated via the SPA's Schedule + │ Validator / TroubleshootController) + │ + ├─ Scripted: ScriptedScheduleController-backed (not yet detailed here) + │ + └─ ExternalJson: Playout.ScheduleFile (JSON) + Playout.Items = List (the built, materialized schedule) + Playout.PlayoutHistory = rotation/rerun state (per Block for block playouts) +``` + +`Deco`/`DecoTemplate` are orthogonal to the schedule kind (mostly used with Block playouts but +`Playout.DecoId` and `Playout.Templates` (`PlayoutTemplate.DecoTemplateId`) are independent FKs on +`Playout`, not nested inside `ProgramSchedule`). + +## Glossary + +| Term | Meaning | Key entity | Edited at (SPA) | +|---|---|---|---| +| **Classic playout** | Schedule = ordered `ProgramScheduleItem` rows on a `ProgramSchedule`, played in sequence/loop. Alternate schedules let day-of-week/day-of-month/month/date-range conditions pick a different `ProgramScheduleAlternate` by `Index` order, **first match wins**; the row with the broadest/no conditions, placed **last**, acts as the catch-all default. | `ProgramSchedule`, `ProgramScheduleItem`, `ProgramScheduleAlternate` | `/app/schedules` | +| **Block playout** | A calendar of `Template`s assigned to times of day (`TemplateItem.StartTime`) and to a playout via `PlayoutTemplate` (day/date-conditional, same first-match/catch-all-last pattern as alternate schedules). Each `Template` is a day-grid of `Block`s; each `Block` is an ordered list of `BlockItem`s (collection/media/search references) with a `Minutes` duration and a `BlockStopScheduling` rule (`AfterDurationEnd` vs `BeforeDurationEnd`). | `Template`, `TemplateItem`, `Block`, `BlockItem`, `PlayoutTemplate` | `/app/templates`, `/app/blocks`, `/app/playouts/{id}/templates` | +| **Sequential playout** | Driven by a YAML file (`Playout.ScheduleFile`); validated via the Schedule Validator screen. | `Playout.ScheduleFile` | `/app/troubleshooting/yaml` (validate only; file itself is server-side) | +| **Scripted playout** | `PlayoutScheduleKind.Scripted`; backed by `ScriptedScheduleController`. | — | — | +| **External-JSON playout** | `PlayoutScheduleKind.ExternalJson = 20`; JSON-driven, same shape idea as Sequential but JSON instead of YAML. | `Playout.ScheduleFile` | — | +| **Deco** | Per-playout "decoration": one of 4 independently-modal sections — watermark, graphics elements, default filler, dead-air fallback — plus break content. Each mode section is `Inherit`/`Disable`/`Override`/`Merge` (`DecoMode`). Can attach directly to a Block playout via `Playout.DecoId`. | `Deco`, `DecoGroup`, `DecoBreakContent` | `/app/decos` | +| **DecoTemplate** | Time-of-day (`DecoTemplateItem.StartTime`/`EndTime`) calendar of `Deco`s, assigned to a playout via `PlayoutTemplate.DecoTemplateId` (same row as the Block-template assignment — one `PlayoutTemplate` entry carries both a `Template` and an optional `DecoTemplate`). | `DecoTemplate`, `DecoTemplateItem`, `DecoTemplateGroup` | `/app/deco-templates` | +| **Default deco vs deco templates** | `Playout.DecoId` = one static deco for the whole playout; `PlayoutTemplate.DecoTemplateId` = a time-varying deco schedule. Both are optional and independent. | `Playout`, `PlayoutTemplate` | `/app/playouts/{id}/templates` | +| **FillerPreset** | A reusable filler definition: `FillerKind` (PreRoll/MidRoll/PostRoll/Tail/Fallback; also `GuideMode=99`, `DecoDefault=100`) × `FillerMode` (None/Duration/Count/Pad/RandomCount) over a collection/media-item/multi-collection/smart-collection/playlist source, with an optional `Expression` (NCalc). Referenced from `ProgramScheduleItem` (Pre/Mid/Post/Tail/FallbackFillerId) and `Channel.FallbackFillerId`. | `FillerPreset`, `FillerKind`, `FillerMode` | `/app/filler-presets` | +| **Watermark** | `ChannelWatermark` image overlay; attached at channel, schedule-item, block-item, deco, or playout-item level with position/size/opacity. | `ChannelWatermark`, `DecoWatermark`, `BlockItemWatermark`, `ProgramScheduleItemWatermark` | `/app/watermarks` | +| **Collection** | Manual list of media items (`CollectionItem`). | `Collection` | `/app/collections` | +| **SmartCollection** | Saved search — a `Query` string, no static item list. | `SmartCollection` | `/app/collections` | +| **MultiCollection** | Combines multiple `Collection`s and/or `SmartCollection`s (with grouping via `MultiCollectionItem`/`MultiCollectionSmartItem`). | `MultiCollection` | `/app/collections` | +| **RerunCollection** | One source (collection/media item/multi/smart) with separate `FirstRunPlaybackOrder` vs `RerunPlaybackOrder`. | `RerunCollection` | `/app/collections` (blocked on API #152 per parity tracker) | +| **Playlist** / **PlaylistGroup** | Ordered `PlaylistItem`s; `IsSystem` flag marks built-in/non-deletable playlists and groups. | `Playlist`, `PlaylistGroup`, `PlaylistItem` | `/app/collections` | +| **Media kinds** | `CollectionType` enum distinguishes container kinds (Collection/TelevisionShow/TelevisionSeason/Artist/MultiCollection/SmartCollection/Playlist/RerunFirstRun/RerunRerun/SearchQuery) from leaf media kinds (Movie/Episode/MusicVideo/OtherVideo/Song/Image/RemoteStream) plus synthetic `FakeCollection`/`FakePlaylistItem`. Concrete media entities: `Movie`, `Show`/`Season`/`Episode`, `Artist`/`MusicVideo`/`Song`, `OtherVideo`, `Image`, `RemoteStream` (`ErsatzTV.Core/Domain/MediaItem/`). | `MediaItem` subclasses | `/app/media?kind=...` | +| **Library / LibraryPath / LibraryFolder** | `Library` (abstract; Local/Plex/Jellyfin/Emby subclasses) owns one or more `LibraryPath`s (scan roots); each path has a `LibraryFolder` tree used for browsing and image-folder duration metadata. | `Library`, `LibraryPath`, `LibraryFolder` | `/app/libraries` | +| **Media source kind** | `MediaSourceKind`: Local/Plex/Jellyfin/Emby — the origin server type for a `Library`. | `MediaSourceKind` | `/app/libraries` | +| **MediaItemState** | Health flag on a media item: Normal/FileNotFound/Unavailable/RemoteOnly. Drives the Trash screen. | `MediaItemState` | `/app/trash` | +| **PlayoutItem** | One materialized, built entry in a playout's timeline (the actual thing that will play at a given time). | `PlayoutItem` | (generated, not directly edited) | +| **PlayoutHistory** | Rotation/rerun bookkeeping per block (`BlockId`) + collection `Key`/`ChildKey`, used by block-playout schedulers to avoid repeats; inspectable via Troubleshooting. | `PlayoutHistory` | `/app/troubleshooting/blocks` | +| **Channel concepts** | `Number` (validated by `Channel.NumberValidator` regex), `Group`, `PlayoutSource` (Generated/Mirror; Mirror channels relay another channel via `MirrorSourceChannelId`+`PlayoutOffset`), `PlayoutMode` (Continuous/OnDemand), `TranscodeMode` (OnDemand only, today), `IdleBehavior` (StopOnDisconnect/KeepRunning), `StreamingMode` (TransportStream/HttpLiveStreamingDirect/HttpLiveStreamingSegmenter/TransportStreamHybrid). | `Channel` | `/app/channels`, `/app/edit-channel/{id}`, `/app/new-channel` | +| **Guide / EPG (XMLTV)** | Per-channel programme guide generated from playout items; channels with `ShowInEpg=false` are excluded. | `GetChannelGuideHandler` | `/app/guide` (viewer); settings at `/app/settings/xmltv` | +| **M3U** | The channel lineup playlist Jellyfin/Dispatcharr consume. | `ChannelPlaylist.ToM3U()` | — | + +## Where things are edited (SPA routes) + +Primary nav: `/app` (dashboard), `/app/channels`, `/app/new-channel`, `/app/guide`, +`/app/schedules`, `/app/blocks`, `/app/templates`, `/app/decos`, `/app/deco-templates`, +`/app/playouts` (+ sub-paths `/app/playouts/{id}/alternate-schedules`, +`/app/playouts/{id}/templates`). + +Media nav: `/app/media` (generic kind-filtered browse, `?kind=movies|shows|artists|music-videos| +other-videos|remote-streams|images`), `/app/search`, `/app/trash`, `/app/collections` (manual + +smart + multi + playlist; rerun-collection editing still gated on API #152), +`/app/filler-presets`, `/app/libraries`, `/app/trakt-lists`. + +System nav: `/app/settings` (sub-tabs: streaming/system/logging/playout/scanner/general/xmltv — +all mapped 1:1 from legacy `/settings/*` Blazor routes), `/app/logs`, `/app/troubleshooting` (+ +`/app/troubleshooting/blocks` block-playout history, `/app/troubleshooting/yaml` sequential-schedule +validator; playback troubleshooting still gated on API #145), `/app/ffmpeg-profiles`, +`/app/watermarks`. + +`/app/edit-channel/{id}` (edit) is reached from the channels table, not the primary sidebar nav. +Per-item media detail pages and the image-folder browser (`MediaDetailScreen`'s +`MovieDetailScreen`/`ShowDetailScreen`/`SeasonDetailScreen`/`ArtistDetailScreen`, +`ImageBrowserScreen`) landed via #141 (PR #183) at `/app/media/{movies|shows|seasons|artists}/{id}` +and `/app/media/images/browser`. Not yet in the SPA: multi/rerun-collection + playlist-variant +management (API gaps #151/#152/#153/#155). See `docs/blazor-route-parity.md` for the full +route-by-route tracker. + +## Key handler / file locations + +- **Classic scheduling engine**: `ErsatzTV.Core/Scheduling/PlayoutBuilder.cs`, + `PlayoutModeSchedulerBase.cs` (+ `One`/`Multiple`/`Flood`/`Duration` variants), + `AlternateScheduleSelector.cs` (first-match-wins alternate/template selection). +- **Block scheduling engine**: `ErsatzTV.Core/Scheduling/PlayoutModeBlock.cs`. +- **Build entry point**: `ErsatzTV.Application/Playouts/Commands/BuildPlayoutHandler.cs`. +- **M3U generation**: `ErsatzTV.Core/Iptv/ChannelPlaylist.cs` → `ToM3U()`. +- **XMLTV generation**: `ErsatzTV.Application/Channels/Queries/GetChannelGuideHandler.cs`. +- **Streaming / IPTV routes**: `ErsatzTV/Controllers/IptvController.cs` (`/iptv/channels.m3u`, + `/iptv/xmltv.xml`, `/iptv/channel/{number}.ts`, `/iptv/session/{number}/hls.m3u8`, HDHR routes, + logos). diff --git a/docs/e2e-local.md b/docs/e2e-local.md new file mode 100644 index 000000000..77b281333 --- /dev/null +++ b/docs/e2e-local.md @@ -0,0 +1,97 @@ +# Local live-E2E recipe + +Purpose: how to stand up a real, running instance of this fork locally (dotnet host + built SPA) +for manual or Playwright-MCP-driven end-to-end verification — no live Docker/prod dependency. +**Update this doc (and `scripts/e2e-local.sh`) in the same PR that changes any convention below.** + +This is for interactive/agent-driven verification, not CI (`docs/ci-cd.md` covers the CI pipeline, +which never runs the app itself). + +## Why the steps are in this order + +- **`ErsatzTV/Startup.cs`'s `/app` SPA middleware resolves its static-file root once, at startup** + (`SpaStaticFileRoot()`, checked with `Directory.Exists` and swapped to a `NullFileProvider` if + missing — see `Startup.cs` around the `app.MapWhen(... "/app" ...)` block). If `wwwroot/app` + doesn't exist yet when the process starts, **the SPA will 404 forever until you restart the + process** — copying the built files in after the fact does nothing for an already-running + instance. +- The dotnet host and the SPA share **one port** (default 8409, `ETV_UI_PORT` / `SystemEnvironment. + UiPort` in `ErsatzTV.Core/SystemEnvironment.cs`) — there's no separate dev server/proxy in this + workflow; you're testing the actual production static-hosting path. +- A fresh config folder per run avoids state bleed (leftover channels/schedules/DB) between test + sessions corrupting your assertions. + +## Steps + +1. **Build the prerequisites** (once, or after any source change): + ```bash + dotnet build ErsatzTV.sln + cd web && npm run build && cd .. # → ErsatzTV/wwwroot/app (vite.config.ts outDir) + ``` + +2. **Copy `wwwroot` into the build output** (the `dotnet build` output directory does not + automatically pick up `web/`'s build artifacts placed directly into the source `wwwroot`): + ```bash + cp -R ErsatzTV/wwwroot ErsatzTV/bin/Debug/net10.0/wwwroot + ``` + If you rebuild the SPA (`npm run build`) while the dotnet process from step 4 is already + running, **re-copy and then restart the process** — see the "why" note above; it will not pick + up new files live. + +3. **Use a fresh scratch config folder** — never reuse one across test runs: + ```bash + CONFIG_DIR=$(mktemp -d) + ``` + +4. **Run the app**, pointed at the scratch folder: + ```bash + cd ErsatzTV/bin/Debug/net10.0 + ETV_CONFIG_FOLDER="$CONFIG_DIR" dotnet ErsatzTV.dll + ``` + Wait for the log line **`Done migrating search index`** (emitted by + `RebuildSearchIndexHandler` in `ErsatzTV.Application/Search/Commands/`, with a + `... in {Duration}` suffix) — that's the last long-running startup step; before that, requests + may 404/error. The UI and the entire `/api/*` surface are served on the **same port**, 8409 by + default (override with `ETV_UI_PORT`). + +5. **Seed data** as needed via the API — no API key is required for local mutating requests by + default (`ApiKeyAuthorizationFilter` only enforces the `X-Api-Key` header when + `Api:WriteKey` is configured; it's empty/unset in a fresh local config, so writes are open). + Examples: + - Create a channel: `POST /api/channels` — check the current + `ErsatzTV/wwwroot/openapi/v1.json` (or `CreateChannelRequest.cs`) for the exact required field + list before assuming these are complete, but as of this writing it requires (among plain + fields) these enums: `PlayoutSource: "Generated"`, `PlayoutMode: "Continuous"`, + `SongVideoMode: "Default"`, `TranscodeMode: "OnDemand"`, `IdleBehavior: "StopOnDisconnect"`. + - Create a playout for an existing channel: `POST /api/playouts` with + `{"channelId": , "scheduleKind": "Block"}` (or `"Classic"`/`"Scripted"`/`"Sequential"` per + `ChannelPlayoutSource`/schedule-kind enums — check `v1.json` for the current set). + +6. **Tear down**: kill the `dotnet ErsatzTV.dll` process and confirm the port is freed + (`lsof -i :8409` should return nothing) before starting another run — a stray process holding + the port will make the next run's health check hang or fail confusingly. + +## Playwright MCP screenshots + +If you're driving the browser via the Playwright MCP server for visual verification, screenshots +land in **the MCP server process's own cwd** (this repo's root, not wherever you ran the dotnet +process from) — expect stray `*.png` files at the repo root after a session; this is tolerated, +not a bug to fix, but don't check them in. + +## Script: `scripts/e2e-local.sh` + +A copy of this script is included in this doc's directory; it is intended to land at +`scripts/e2e-local.sh` in the repo. It automates steps 2–4 above (build is assumed already done — +run `dotnet build` / `npm run build` yourself first, since rebuilding on every invocation is slow +and this script is meant to be re-run often during a debugging session). + +Usage: +```bash +scripts/e2e-local.sh [CONFIG_DIR] +``` +- `CONFIG_DIR` defaults to a fresh `mktemp -d` if omitted. +- Copies `ErsatzTV/wwwroot` → `ErsatzTV/bin/Debug/net10.0/wwwroot`. +- Launches `dotnet ErsatzTV.dll` in the background with `ETV_CONFIG_FOLDER` set. +- Waits (up to 120s) for the `Done migrating search index` log line. +- Prints the PID and port, then **exits leaving the server running** — the caller is responsible + for killing the PID when done (`kill `). diff --git a/docs/handoffs/chicorytv-issue-queue.md b/docs/handoffs/chicorytv-issue-queue.md index 7c599bee1..7a56c14c5 100644 --- a/docs/handoffs/chicorytv-issue-queue.md +++ b/docs/handoffs/chicorytv-issue-queue.md @@ -4,163 +4,136 @@ Paste the prompt below into a fresh session to work the next item. Each session UPDATING THIS FILE in place (rewrite the state section and the queue for the next item) so it always holds the current handoff. History: created 2026-07-02 after the plan audit (#59 epic); all backend gap issues (#100–#111), all SPA screens (#84–#89, #93, #109), the rebrand (#90), -the cutover root-flip (#91 phase a), parity pass 2 (#140/#142/#143/#146/#147, PRs #165–#169), -and the FULL scheduling parity #144/#162 (PRs #170/#171/#173/#174/#175/#179) are MERGED. -v26.5.0 tagged + DEPLOYED to prod 2026-07-07. Blazor removal (#91 phase b) is now gated ONLY -on: #145 leftovers, #141 leftovers, and gap issues #151–#153/#155 (collections/playlists -CRUD APIs) + #158 items 4–5 + #161 items 3+5. +the cutover root-flip (#91 phase a), parity pass 2 (#140/#142/#143/#146/#147), the FULL +scheduling parity #144/#162 (PRs #170–#175/#179), and the media/troubleshooting parity +#141/#161 + #158 + #180 (PRs #181/#182/#183) are MERGED. v26.5.0 deployed to prod +2026-07-07. Blazor removal (#91 phase b) is now gated ONLY on: #151/#152 (multi/rerun +collection APIs), #153/#155 (playlists + collection-items depth), and #145's last item +(playback troubleshooting). -**Session state (2026-07-07 evening, #144 finale)**: main = c480fbe8, all pre+post-merge CI -green through PR #179. This session merged SIX PRs — the entire #162 slice plan — closing -#144 and #162: -- S5 → PR #170: POST /api/playouts all 5 kinds (scheduleKind discriminator) + PUT - /api/playouts/{id} (dailyRebuildTime null-clears, scheduleFile file-kinds-only); - AddPlayoutDialog + EditPlayoutDetailsDialog (App.tsx inline). Add Playout button fixed. -- S1 → PR #171: BlockController (groups/blocks CRUD, GET items, PUT full-replace with - index-from-array-order, POST preview non-persisting) + GET /api/search/{collections, - television-shows,television-seasons,smart-collections} pickers + /app/blocks list+editor. -- S2 → PR #173: TemplateController (+?templateGroupId= filter) + POST .../copy for BOTH - templates and blocks + /app/templates (table editor, not Blazor's drag calendar — - accepted deviation; client-side overlap+midnight warnings mirror the server). -- S3 → PR #174: DecoController (6-section 22-field PUT; Merge-mode request validation) + - PUT /api/playouts/{id}/deco + GET-only PlaylistController (/api/playlists/groups, - /api/playlists?playlistGroupId=) + GET /api/search/{artists,multi-collections} + - /app/decos. Legacy Artist/MultiCollection break-content renders read-only, round-trips. -- S4 → PR #175: DecoTemplateController + /app/deco-templates (start + duration editor, - endTime 00:00:00 = end-of-day, displays 24:00). No copy exists upstream — not a gap. -- S6 → PR #179: GET/PUT /api/playouts/{id}/alternate-schedules (Classic-only) + - /api/playouts/{id}/templates (Block-only); index from array order, LAST alt-schedule row - = catch-all default (writes playout.ProgramScheduleId; SPA badges it + disables its - recurrence controls); PlayoutResponseModel gained decoId/decoName; default-deco select - on Block playout cards; playouts route now allowSubPaths via PlayoutsRouteScreen. -- Upstream Application-handler bugs fixed at the root along the way (all surfaced by the - new API, all 422-now-not-500): missing group-existence checks in Create{Block,Template, - Deco,DecoTemplate}Handler; ReplaceTemplateItems KeyNotFoundException on unknown blockId; - midnight-crossing template items accepted-then-silently-dropped; ReplaceDecoTemplateItems - silent-drop/no-overlap-check; UpdateDecoHandler dropped MediaItemId on break-content - writes (silent corruption); alternate-schedules empty-list Max() throw; out-of-day-range - TimeSpans/dates persisted raw then crashed the selector/builder. -- Issues: #144 CLOSED, #162 CLOSED. Filed: #172 (API hardening nits consolidated), #176 - (PseudoTV-style channel-first creation rethink — user vision, post-phase-(b)), #177 - (Jellyfin music-video Album/Track metadata gap), #178 (far-future Jellyfin plugin idea). -- Tests: ErsatzTV.Tests 621→786, Core.Tests 493 (stable), web 218→301. All slices: - adversarial fork review + live browser E2E + should-fixes applied before merge. -- Worktrees all removed except .worktrees/docs-wrapup (this doc commit; remove after). +**Session state (2026-07-07 night, parity endgame session)**: main = dbda2d89 + this docs +commit; all pre+post-merge CI green through PR #183. This session merged THREE PRs: +- PR #181 → #180 CLOSED: library-picker artwork fixed at the ROOT (API now returns rooted + /artwork/... URLs — Blazor-relative values never worked in the SPA), season tiles + replaced by a show-tile "Seasons" drill-in (browse `parentId` param), collections + add-items dialog got media-kind filter chips. +- PR #182 → #158 CLOSED (#145 now playback-only): block-playout history endpoints + (GET /api/playouts/{id}/blocks, .../blocks/{blockId}/history paged, /api/playouts/ + history/{id} decode-by-row-id) + POST /api/troubleshoot/validate-schedule (YAML in + body); SPA screens /app/troubleshooting/blocks + /app/troubleshooting/yaml. +- PR #183 → #141 + #161 CLOSED: detail endpoints GET /api/{movies,shows,seasons,artists}/ + {id} + /api/media-items/{id}/info; image folders GET/PUT; browse parentId extended to + Episode + MusicVideo; SPA detail pages /app/media/{kind}/{id} via MediaRouteScreen + (sub-path ownership) + /app/media/images/browser; shared ApiArtwork.Root() helper. +- **ONBOARDING DOCS LANDED (part 1)**: docs/README.md (index), api-conventions.md, + spa-conventions.md, e2e-local.md + scripts/e2e-local.sh, blazor-route-parity.md (the + #91 tracker), domain-model.md, decisions.md. RULE (also in CLAUDE.md + memory): read + these at session start INSTEAD of re-recon; update them in the same PR that changes a + convention/route/decision. Part 2 (testing map + generated endpoint index) = **#185, + HIGH PRIORITY next session**. +- New issues: #184 (ApiControllerSecurityTests registry drifted — 9 controllers unlisted, + 2 with mutating verbs; fix = assembly scanning, S), #185 (docs part 2, S+S). +- #91 phase (b) readiness plan POSTED on #91 (comment 2026-07-07): gates = #145 playback + + #151 + #152 + #153/#155; then Step 1 redirect sweep [S, independent — can ship any + session], Step 2 Blazor deletion [M-L, own session], Step 3 verification+docs. ~105 + routes are SPA-ready awaiting redirects; only 4 functional areas remain Blazor-only. +- Tests: ErsatzTV.Tests 828, Core.Tests 493 (+1 skip), Architecture 5, web 330. All PRs: + fork adversarial review + live browser E2E + CI before merge; all three reviews came + back with zero must-fixes. +- Worktrees: fix-180 / feat-145 / feat-141 / wrapup can all be removed (work merged). -**Lessons for all remaining prompts** (accumulated; pruned): -- The main checkout (/Users/timothy/ersatztv) sits on the STALE docs/59-ui-redesign-brief - branch. NEVER recon/edit there — point subagents at a worktree pinned to origin/main. -- IMPLEMENTER AGENTS MUST RUN `npm run lint` before finishing — CI lint failed once (S5) - on react-hooks set-state-in-effect. Rule of thumb: NO synchronous setState inside - useEffect; derive state or set only in promise callbacks (useChannelsQuery precedent). -- SPA sub-path screens MUST own their pathname state + popstate listener (BlocksScreen / - PlayoutsRouteScreen pattern). App-level routeFromLocation returns the SAME route object - for /app/x and /app/x/{id} (allowSubPaths prefix match), so setActiveRoute bails via - Object.is and nothing re-renders — navigateToPath's synthetic popstate is NOT enough. -- Live E2E: copy ErsatzTV/wwwroot INCLUDING app/ into bin/Debug/net10.0/wwwroot AND - RESTART (static middleware won't see wwwroot appearing post-startup). UI port 8410, API - 8409. Scratch ETV_CONFIG_FOLDER; curl localhost; wait "Done migrating search index"; - mutating verbs open without API key. Playwright MCP can only write screenshots into the - ROOT checkout (.playwright-mcp) — tolerated exception. Creating a Classic playout via - API needs channel enums: ChannelPlayoutSource=Generated, ChannelPlayoutMode=Continuous, - ChannelSongVideoMode=Default, ChannelTranscodeMode=OnDemand, - ChannelIdleBehavior=StopOnDisconnect. -- WRAPPING CQRS HANDLERS IN REST: check for these latent bug classes and fix at the - handler (established convention, ~8 fixes merged): missing FK existence checks (FK - violation → unhandled 500), dictionary-indexer lookups (KeyNotFoundException → 500), - silent item filtering at persist/read time (reject with 422 instead), unbounded - TimeSpan/month/day ints that crash consumers later. Deep FK ids (collection/media refs - in items) are deliberately NOT existence-checked (matches schedules precedent) — #172. -- DayOfWeek serializes as day-name STRINGS (StringEnumConverter) though OpenAPI says - integer — TS client overrides both directions (playouts.ts WithDayNames precedent). -- PUT-replace list endpoints: index from array order (ReplaceScheduleItemsRequest - precedent). Alternate schedules: LAST row = catch-all default (selector is - OrderBy(Index) first-match-wins; handler writes Max(Index) row's scheduleId to the - playout). GET /api/blocks and /api/templates synthesize negative-id "(none)" rows per - empty group — SPA pickers/lists must filter id > 0 (#172 wants them gone at the API). -- PARALLEL/STACKED PARITY BRANCHES CONFLICT on web/src/App.tsx, v1.json/v1.d.ts, and the - security/contract test lists. Routine: stack the next slice on the previous branch, and - merge origin/main into the branch before PR → union resolve → REGENERATE v1.json + - v1.d.ts (authoritative) → full test pass. Stacked merges were conflict-free all session. -- Gitea 1.24 has no rerun-run API; retrigger CI with an empty commit. Poll CI by commit: +**Lessons for all remaining prompts** (pruned — conventions moved to docs/): +- READ docs/README.md → the convention docs FIRST; point recon/implementer agents at + specific doc sections instead of re-explaining. Only recon what the docs don't cover. +- The main checkout (/Users/timothy/ersatztv) sits on a STALE branch. NEVER recon/edit + there — worktrees off origin/main only; `cd web && npm ci` (or copy node_modules from a + sibling worktree) in NEW worktrees. +- PR routine (unchanged, works): worktree → implement (opus agent; give it doc pointers + + exact facts) → merge origin/main into branch before PR (v1.json/v1.d.ts conflicts → + take either side, REGENERATE, full test pass; sonnet reconcile agent) → push, PR → + fork adversarial review + sonnet live-E2E in parallel → CI green → merge (consent: ask + in-conversation; the 2026-07-07 "pre-approved when issue complete and tests green" + grant was SESSION-SCOPED — re-ask each new session) → structured close comments per + CLAUDE.md protocol. +- Live E2E: scripts/e2e-local.sh automates launch (wwwroot+app copy, fresh scratch + ETV_CONFIG_FOLDER, waits for "Done migrating search index"); single port 8409 for + UI+API; mutating verbs need no API key locally; seed a Block playout via POST + /api/playouts {"channelId":N,"scheduleKind":"Block"}. Playwright MCP screenshots land + in the repo-root cwd — tolerated. +- Gitea 1.24: no rerun-run API — retrigger CI with an empty commit; poll by commit: /api/v1/repos/timothy/ersatztv/commits/{sha}/status. -- ./scripts/update-openapi.sh needs a prior normal dotnet build; web typegen = npm run - generate:api. Request DTOs (Controllers/Api/Requests) have no #nullable; response DTOs - in ErsatzTV.Core/Api MUST get file-scoped #nullable enable. ErsatzTV.Application has NO - nullable context (CS8632). NSubstitute+ConfigElementKey: Arg.Any + generic. LanguageExt: - MatchUnsafe for Option→nullable; foreach(x in either.LeftToSeq()) for error extraction. -- Known dead UI nit (multiple screens): the header primaryAction "New Group" button isn't - wired on Blocks/Decos-style screens (screen-local buttons work) — fold into a sweep. -- Backlog nits live in #172 (hardening) + #163/#164 (goldens, health UX). Trakt - matched-items link → /app/search swap and GET /api/search parallelization still open. +- A subagent died mid-run once (connection error): resume via SendMessage ("check git + status/log, finish verification, commit") — its context survives, work isn't lost. +- Backlog nits: #172 (API hardening incl. negative-id "(none)" rows), #163/#164, #176 + (PseudoTV-style creation, post-phase-b), #177/#178 (Jellyfin), #66. Quick wins still + open: Trakt matched-items link → /app/search; parallelize GET /api/search + (Task.WhenAll); dead header "New Group" primaryAction sweep. --- -# PROMPT — Parity endgame: #145 leftovers (block-playout history, YAML validator), #141 leftovers (media detail pages, image browser) → #91 phase (b) readiness +# PROMPT — Docs part 2 + quick wins, then collections/playlists API gaps (#155 → #151/#152 → #153) toward #91 phase (b) You are Fable, the ORCHESTRATOR in the main Claude Code session (Claude Code only). Fable is EXPENSIVE: delegate (recon → Explore/haiku; mechanical/reconciles → sonnet; judgment-heavy -code → opus; fable forks for review). Read CLAUDE.md + the Lessons above first. +code → opus; fable forks for review). FIRST read CLAUDE.md, docs/README.md and the convention +docs it indexes, and the Lessons above. HARD CONSTRAINTS: - Work in worktrees off origin/main; NEVER touch /Users/timothy/ersatztv (stale branch). `cd web && npm ci` in any NEW worktree (or copy node_modules from a sibling worktree). - Max 2–3 concurrent builds; ONE dotnet build at a time. NEVER set ETV_UPDATE_GOLDENS. -- Merge consent in-conversation per PR (user may pre-approve the session). CI reruns = - empty commit. -- Live-E2E new screens (Lessons above: wwwroot+app copy + restart, ports, enums). +- Merge consent in-conversation per PR (user may pre-approve the session; last session's + pre-approval does NOT carry over). CI reruns = empty commit. +- Live-E2E new screens via scripts/e2e-local.sh (see docs/e2e-local.md). - Adversarial review fork over each PR diff before merge; apply should-fixes. -- Implementers must run `npm run lint`; sub-path screens own pathname state (Lessons). +- Convention docs: read at start; any PR that changes a convention, migrates a route, or + reverses a decision updates the relevant doc IN THE SAME PR. -## Known facts (2026-07-07 — re-verify cheaply) -- main = c480fbe8 (post-#179): ALL scheduling editors are in the SPA. Remaining Blazor-only - surface (phase (b) gates): #145 leftovers + #141 leftovers + #151–#153/#155. -- #145 leftovers (issue open): block-playout history screens (#158 item 4 endpoints: - GetAllBlockPlayouts/GetAllBlocksForPlayout/GetBlockPlayoutHistory/DecodePlayoutHistory) - [M]; sequential-schedule YAML validator (#158 item 5: POST body, not server path) [S-M]; - playback-troubleshooting screen (endpoints live + in spec; needs HLS player + session-log - retrieval design) [L — own session, LAST]. -- #141 leftovers (issue open): detail endpoints+pages for movie/show/season/artist + - GetMediaItemInfo (#161 item 3) [M-L]; image folder browser (#161 item 5) [S-M]. -- #151–#153/#155 (collections/playlists CRUD APIs) are NOT in this prompt's scope unless - everything above lands with capacity to spare — but S3 already shipped playlist LIST - endpoints (PlaylistController) + artist/multi-collection search, which those issues can - build on. -- Quick wins interleavable: Trakt matched-items link → /app/search; parallelize - GET /api/search (Task.WhenAll); dead header "New Group" primaryAction sweep. +## Known facts (2026-07-07 night — re-verify cheaply) +- main = post-#183 + docs commit: #141/#158/#161/#162/#144/#180 all CLOSED. #145 open + (playback troubleshooting ONLY — L, own session, do LAST). Phase (b) gates + removal + plan live on #91. +- #185 (HIGH, S+S): docs/testing.md consolidation + scripts/generate-endpoint-index + hooked into update-openapi.sh (docs/endpoint-index.md, generated not hand-written). +- #184 (S): ApiControllerSecurityTests → assembly scanning (9 controllers unlisted; + ArtworkUploadController POST + ChannelTemplateController POST/PUT/DELETE unchecked). +- #155 (S): GET /api/collections/{id}/items missing (+ POST items 500 fix noted in the + issue) — smallest gap, unblocks collections depth; coordinate DTO with #141's paged + shapes. +- #151/#152 (M, mirror each other): multi-collections + rerun-collections REST CRUD + + SPA editors (Blazor refs: MultiCollections/MultiCollectionEditor.razor, + RerunCollections/RerunCollectionEditor.razor). One session together. +- #153 (M): playlists/playlist-groups CRUD (PlaylistController is GET-only today) + SPA + playlist editing to Blazor PlaylistEditor.razor depth. +- Quick wins interleavable: Trakt link swap, GET /api/search parallelization, dead "New + Group" primaryAction sweep. ## Task -1. Verify main green (post-#179 run 596 was green at handoff time). -1b. FIRST: #180 (Channel Builder season-tile flooding + broken artwork) — small, - user-visible, diagnosis already in the issue. Same PR routine. -2. #145 leftovers: block-playout history [M] first (worktree, recon the 4 CQRS queries + - Blazor history pages, thin wrappers + SPA screen), then YAML validator [S-M] (same or - second PR). Tests + live E2E, merge main in, PR, fork review, CI, consent, merge. - Comment on #145/#158 per landing; close #158 when items 4–5 done (1–3 landed in #165). -3. #141 leftovers: media detail endpoints+pages [M-L], then image folder browser [S-M]. - Same routine; close #141/#161 when done. -4. If #145's playback-troubleshooting is all that remains, leave it for its own session - (update #145 accordingly) — do NOT start it late in a session. -5. READINESS CHECK once #141 closes and #145 is troubleshooting-only: enumerate remaining - Blazor-only routes (grep @page in ErsatzTV/Pages minus migrated), reconcile against - #151–#153/#155 + troubleshooting, and file the phase (b) removal plan on #91 (or file - what's missing). -6. Update THIS handoff (state, lessons, next prompt), commit to main, print next prompt in - a fenced block. +1. Verify main green (post-#183 merge + docs commit). +2. #185 docs part 2 [S+S] — small PR, no browser E2E needed (docs+script only; the + endpoint-index generation must run in the PR to prove it works). +3. Quick-wins PR [S]: #184 registry scan + Trakt link swap + search parallelization + + "New Group" sweep (one branch, one review pass; E2E only the UI-visible bits). +4. #155 [S] then #151/#152 [M] (one combined PR is fine if clean) — full PR routine each. + Comment/close issues per the CLAUDE.md protocol; update docs/blazor-route-parity.md + rows in the same PRs. +5. #153 [M] if capacity remains — else queue it top for next session. +6. Do NOT start #145 playback troubleshooting late in the session — own session. +7. Update THIS handoff (state, lessons, next prompt), commit to main, print next prompt + in a fenced block. --- ## Issue queue (work top-down) -1. #180 Channel Builder library picker: season tiles flood the grid + ALL artwork broken - [S-M, user-visible bug on the flagship flow — diagnosis + fix options in the issue; - check CollectionsScreen add-items dialog too; verify prod 26.5.0 vs :latest] -2. #145 block-playout history screens [M] ← PROMPT above -2. #145 sequential-YAML validator [S-M] -3. #141 media detail endpoints+pages [M-L] -4. #141 image folder browser [S-M] -5. #91 phase (b) readiness check + removal plan (gated also on #151–#153/#155) -6. #145 playback troubleshooting [L, own session] -7. #151–#153/#155 collections/playlists CRUD APIs (S3's PlaylistController is the seed) +1. #185 onboarding docs part 2: testing map + generated endpoint index [S+S, HIGH] +2. Quick wins: #184 security-registry scan + Trakt link → /app/search + parallelize + GET /api/search + dead "New Group" sweep [S combined] +3. #155 GET /api/collections/{id}/items (+POST fix) [S] +4. #151 + #152 multi/rerun collections API + SPA editors [M, mirror pair] +5. #153 playlists CRUD + SPA editor depth [M] +6. #145 playback troubleshooting screen [L, OWN SESSION] +7. #91 phase (b): Step 1 redirect sweep [S — independent, can ship any session]; + Steps 2–3 Blazor deletion + verification [M-L, own session, after gates close] Backlog (non-blocking): #172 API hardening, #163 playout goldens, #164 health UX, #176 -PseudoTV-style creation rethink (post-phase-(b)), #177 Jellyfin music-video Album/Track, -#178 Jellyfin plugin idea, #66 artwork sniffing, search parallelization, Trakt link swap. +PseudoTV-style creation rethink (post-phase-b), #177 Jellyfin music-video metadata, #178 +Jellyfin plugin idea, #66 artwork sniffing. diff --git a/docs/spa-conventions.md b/docs/spa-conventions.md new file mode 100644 index 000000000..f42ea8a55 --- /dev/null +++ b/docs/spa-conventions.md @@ -0,0 +1,112 @@ +# SPA conventions — "Add a screen" playbook + +Purpose: a precise playbook for adding a new screen (or sub-path editor) to the ChicoryTV React SPA +(`web/`), for an agent with no prior context in this repo. **Update this doc in the same PR that +changes any convention below.** + +Companion to `api-conventions.md` (the API surface the SPA talks to) and `docs/contributing.md` +(general repo conventions). + +## 1. Stack & layout + +Vite + React + TypeScript, builds to `ErsatzTV/wwwroot/app` (see `web/vite.config.ts`: +`base: '/app/'`, `build.outDir: '../ErsatzTV/wwwroot/app'`), served by the ASP.NET host at `/app`. + +- **Routes + nav**: `web/src/App.tsx` — one big route table of `ScreenRoute` objects (`path`, + `label`, `title`, `kicker`, `icon`, etc.) plus an `allowSubPaths?: boolean` flag. +- **Screens**: `web/src/screens/*.tsx`, one file per top-level screen, generally with a colocated + `*.test.tsx`. +- **API clients**: `web/src/api/.ts` (see §4). +- **Styling**: `web/src/shell.css` (+ `web/src/components/components.css`) — utility classes with a + `ctv-` prefix (~690 occurrences across those two files). Reuse an existing `ctv-*` class before + inventing a new one. + +## 2. CRITICAL: sub-path screens must own their own pathname state + +If a route sets `allowSubPaths: true` (e.g. so `/app/blocks/{id}` works under the `/app/blocks` nav +entry), **the screen component itself must track `window.location.pathname` and listen for +`popstate`** — do not rely on `App.tsx` re-rendering `ScreenContent` when the sub-path changes. + +**Why**: `App.tsx`'s `routeFromLocation()` matches an `allowSubPaths` route by prefix +(`pathname.startsWith(\`${route.path}/\`)`) and returns the **same `ScreenRoute` object reference** +for the base path and every sub-path under it. `App`'s state update is +`setActiveRoute(routeFromLocation())`; React's `useState` setter bails via `Object.is` when the new +value is reference-equal to the old one — so navigating from `/app/blocks` to `/app/blocks/42` (or +between `/app/blocks/42` and `/app/blocks/17`) **never re-invokes `ScreenContent`** at the `App` +level. See the comment block directly above `PlayoutsRouteScreen` in `App.tsx` (~line 3540) for the +canonical explanation, and its implementation (`useState(() => window.location.pathname)` + +`useEffect` with a `popstate` listener local to the wrapper component) for the fix. + +Exemplars of screens that already do this correctly: `BlocksScreen.tsx`, `TemplatesScreen.tsx`, +`DecosScreen.tsx`, `DecoTemplatesScreen.tsx`, and the `PlayoutsRouteScreen` wrapper in `App.tsx` +(which owns two sibling sub-paths, `/playouts/{id}/alternate-schedules` and +`/playouts/{id}/templates`, dispatching internally via `parsePlayoutSubRoute`). + +## 3. Data loading pattern + +Reference implementation: `web/src/screens/LogsScreen.tsx`. Structure to copy for any screen that +fetches from the API: + +- A **discriminated-union state type** covering loading/success/error, e.g. + `type LogsState = { status: 'loading'; ... } | { status: 'success'; ... } | { status: 'error'; ... }`. +- A `seqRef` (monotonically incremented request counter) + `activeRef` (mount-tracking boolean, + flipped in a mount/unmount `useEffect`) pair — guards against a stale, slower request overwriting + a newer one's result, and against setting state after unmount. +- The actual fetch lives in a `useCallback` (`load`), called from a **separate** `useEffect(() => { + load(); }, [load])`. +- **Lint rule — `react-hooks` "no set-state-in-effect"**: never call `setState` **synchronously in + the body** of a `useEffect`. State transitions happen only inside event handlers or promise + `.then()`/`.catch()` callbacks (as in `LogsScreen`'s `load`). This is enforced by + `eslint-plugin-react-hooks` in `web/eslint.config.js` — a synchronous `setState` in an effect body + will fail `npm run lint`. + +## 4. API client modules + +One file per domain in `web/src/api/`, e.g. `logs.ts`, `blocks.ts`, `playouts.ts`. Pattern (see +`web/src/api/logs.ts`): + +- Re-export the generated response/DTO types from `./generated/v1`: + `export type LogEntry = components['schemas']['LogEntryResponseModel'];` +- A typed params interface for the endpoint's query string (e.g. `GetLogsParams`). +- The fetch function builds a `URLSearchParams` from only the params that are set, then calls the + shared `request(url)` helper from `./client`. +- An error-message helper (e.g. `messageFromLogsError`) that narrows `unknown` → `ApiError` (from + `./client`) → a human string, with a fallback message — screens use this instead of stringifying + errors themselves. +- `web/src/api/index.ts` re-exports everything so screens import from `'../api'`, not from the + individual domain file directly. + +## 5. Artwork rendering + +Render `item.artwork` / `item.poster` (or whatever the DTO field is named) **directly as an ``** — since PR #181, API responses already return rooted, directly-usable URLs (see +`api-conventions.md` §4). **Do not** client-side-prefix artwork paths (no `/artwork/posters/` string +building in SPA code) — if you see that pattern, it's stale/wrong. + +## 6. Tests + +- **vitest**, colocated `*.test.ts` / `*.test.tsx` next to the source file. +- Every screen with meaningful logic gets a screen test; every API client module gets a + param-mapping / URL-building test (e.g. `logs.test.ts` next to `logs.ts`). +- `web/src/App.test.tsx` covers navigation + the route table, including regressions like the + sub-path bug in §2 (see the tests around `PlayoutsRouteScreen`, ~line 1682+, that click into + `/app/playouts/{id}/...` sub-paths and assert the correct sub-screen rendered). +- **Nav-label test-selector care**: `getByRole('link'/'button', { name: /Regex/ })` matches by + substring by default — a loose regex can match more than one nav item. Verified example: the + System nav button is matched with an **anchored** regex (`name: /^System/`) rather than a bare + `/System/`, specifically to avoid ambiguous matches against other labels that start with or + contain "System". Anchor (`^`/`$`) or use exact strings in `getByRole` name matchers whenever a + new label could be a substring of (or share a substring with) an existing one — check + `App.tsx`'s nav `label:` list for collisions before picking a new label. + +## 7. Verification gate — run before every commit touching `web/` + +From `web/`: +```bash +npm test # vitest +npm run lint # eslint . +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. diff --git a/scripts/e2e-local.sh b/scripts/e2e-local.sh new file mode 100755 index 000000000..d6fe591d0 --- /dev/null +++ b/scripts/e2e-local.sh @@ -0,0 +1,81 @@ +#!/usr/bin/env bash +# scripts/e2e-local.sh — launch a local ErsatzTV instance for live E2E verification. +# +# See docs/e2e-local.md for the full recipe and the "why" behind each step. This script assumes +# `dotnet build ErsatzTV.sln` and (if the SPA changed) `cd web && npm run build` have ALREADY been +# run — it only does the wwwroot copy + launch + ready-wait, since rebuilding on every invocation +# is slow and this is meant to be re-run often during a debugging session. +# +# Usage: +# scripts/e2e-local.sh [CONFIG_DIR] +# +# CONFIG_DIR defaults to a fresh `mktemp -d` if omitted. NEVER reuse a config dir across runs. +# +# On success, prints: +# PID= +# PORT= +# CONFIG_DIR= +# LOG= +# and exits 0, leaving the server RUNNING in the background. The caller is responsible for +# stopping it (`kill `) and for freeing the port before starting another run. + +set -euo pipefail + +REPO_ROOT="$(git rev-parse --show-toplevel)" +BUILD_DIR="$REPO_ROOT/ErsatzTV/bin/Debug/net10.0" +PORT="${ETV_UI_PORT:-8409}" +READY_LINE="Done migrating search index" +TIMEOUT_SECS=120 + +CONFIG_DIR="${1:-$(mktemp -d)}" +mkdir -p "$CONFIG_DIR" + +if [ ! -d "$BUILD_DIR" ]; then + echo "error: $BUILD_DIR does not exist — run 'dotnet build ErsatzTV.sln' first" >&2 + exit 1 +fi + +if [ ! -d "$REPO_ROOT/ErsatzTV/wwwroot/app" ]; then + echo "warning: $REPO_ROOT/ErsatzTV/wwwroot/app does not exist — the SPA hasn't been built" \ + "('cd web && npm run build'). The /app UI will 404 until it exists AND the server is" \ + "(re)started after copying it in." >&2 +fi + +echo "Copying wwwroot into build output (static middleware resolves its file root at startup;" \ + "a running process will never see files added later)..." +cp -R "$REPO_ROOT/ErsatzTV/wwwroot" "$BUILD_DIR/wwwroot" + +LOG_FILE="$(mktemp)" +echo "Launching dotnet ErsatzTV.dll (log: $LOG_FILE, config: $CONFIG_DIR, port: $PORT)..." + +( + cd "$BUILD_DIR" + ETV_CONFIG_FOLDER="$CONFIG_DIR" ETV_UI_PORT="$PORT" dotnet ErsatzTV.dll +) >"$LOG_FILE" 2>&1 & +PID=$! + +echo "Waiting up to ${TIMEOUT_SECS}s for '$READY_LINE'..." +elapsed=0 +until grep -q "$READY_LINE" "$LOG_FILE" 2>/dev/null; do + if ! kill -0 "$PID" 2>/dev/null; then + echo "error: process $PID exited before becoming ready. Log tail:" >&2 + tail -n 40 "$LOG_FILE" >&2 + exit 1 + fi + + if [ "$elapsed" -ge "$TIMEOUT_SECS" ]; then + echo "error: timed out after ${TIMEOUT_SECS}s waiting for readiness. Log tail:" >&2 + tail -n 40 "$LOG_FILE" >&2 + kill "$PID" 2>/dev/null || true + exit 1 + fi + + sleep 1 + elapsed=$((elapsed + 1)) +done + +echo "Server ready." +echo "PID=$PID" +echo "PORT=$PORT" +echo "CONFIG_DIR=$CONFIG_DIR" +echo "LOG=$LOG_FILE"