diff --git a/ErsatzTV.Core/Api/Playouts/PlayoutResponseModel.cs b/ErsatzTV.Core/Api/Playouts/PlayoutResponseModel.cs index 00d023216..15cfebba5 100644 --- a/ErsatzTV.Core/Api/Playouts/PlayoutResponseModel.cs +++ b/ErsatzTV.Core/Api/Playouts/PlayoutResponseModel.cs @@ -8,6 +8,7 @@ public record PlayoutResponseModel( PlayoutScheduleKind ScheduleKind, string ChannelName, string ChannelNumber, + int ChannelId, ChannelPlayoutMode PlayoutMode, string ScheduleName, string? ScheduleFile, @@ -23,6 +24,7 @@ public record PlayoutResponseModel( PlayoutScheduleKind scheduleKind, string channelName, string channelNumber, + int channelId, ChannelPlayoutMode playoutMode, string scheduleName, string? scheduleFile, @@ -37,6 +39,7 @@ public record PlayoutResponseModel( scheduleKind, channelName, channelNumber, + channelId, playoutMode, scheduleName, scheduleFile, diff --git a/ErsatzTV.Mcp.Tests/ToolCatalogTests.cs b/ErsatzTV.Mcp.Tests/ToolCatalogTests.cs index e7fb2d7c4..d220511f3 100644 --- a/ErsatzTV.Mcp.Tests/ToolCatalogTests.cs +++ b/ErsatzTV.Mcp.Tests/ToolCatalogTests.cs @@ -202,6 +202,51 @@ public class ToolCatalogTests (tool.QueryParameters ?? new HashSet()).ShouldNotContain("ifMatch"); } + // #616: the catalog described pageNum as "1-based" while every paged REST controller defaults it + // to 0 and skips pageNum * pageSize. A caller that trusted the description started at page 1 and + // silently lost the first page — no error, just a short set that reads like missing data. The + // description is the whole contract an MCP client has, so it is pinned here for EVERY paged tool. + [Test] + public void Paged_Tools_Should_Document_PageNum_As_Zero_Based() + { + ToolDefinition[] paged = ToolCatalog.All + .Where(t => t.InputSchema.RootElement.GetProperty("properties").TryGetProperty("pageNum", out _)) + .ToArray(); + + // Guard the guard twice over. An emptiness check alone is not enough: this test filters on + // tools that ALREADY declare pageNum, so a tool wrapping a paged endpoint while declaring no + // paging args escapes the filter entirely and the test still passes. That is not + // hypothetical — ersatztv_list_playouts and ersatztv_get_playout_items did exactly that, and + // because ToolArgumentValidator rejects undeclared arguments, an MCP caller was hard-capped + // at the first 100 rows with no way to ask for more. So the expected set is named here: a + // new tool over a paged endpoint must be added to it, and dropping paging from any of these + // fails the test rather than silently shrinking its scope. + string[] mustDeclarePaging = + [ + "ersatztv_get_collection_items", + "ersatztv_get_playout_items", + "ersatztv_list_playouts", + "ersatztv_search", + "ersatztv_search_all_items" + ]; + + paged.Select(t => t.Name).OrderBy(n => n, StringComparer.Ordinal) + .ShouldBe(mustDeclarePaging.OrderBy(n => n, StringComparer.Ordinal)); + + foreach (ToolDefinition tool in paged) + { + string description = tool.InputSchema.RootElement + .GetProperty("properties") + .GetProperty("pageNum") + .GetProperty("description") + .GetString() + .ShouldNotBeNull(); + + description.ShouldContain("0-based"); + description.ShouldNotContain("1-based"); + } + } + [Test] public void Scan_Library_Tool_Should_Register_Deep_As_A_Query_Parameter() { diff --git a/ErsatzTV.Mcp/ToolCatalog.cs b/ErsatzTV.Mcp/ToolCatalog.cs index 8c3e675f6..f18c93032 100644 --- a/ErsatzTV.Mcp/ToolCatalog.cs +++ b/ErsatzTV.Mcp/ToolCatalog.cs @@ -29,9 +29,14 @@ public static class ToolCatalog Get("ersatztv_list_schedules", "List schedules.", "/api/v1/schedules"), Get("ersatztv_get_schedule", "Get a schedule by id.", "/api/v1/schedules/{id}", IdPath("Schedule id.")), Get("ersatztv_get_schedule_items", "Get a schedule's items. Emits the schedule ETag.", "/api/v1/schedules/{id}/items", IdPath("Schedule id.")), - Get("ersatztv_list_playouts", "List playouts.", "/api/v1/playouts"), + Get("ersatztv_list_playouts", "List playouts (paged).", "/api/v1/playouts", [], Page()), Get("ersatztv_get_playout", "Get a playout by id.", "/api/v1/playouts/{id}", IdPath("Playout id.")), - Get("ersatztv_get_playout_items", "Get upcoming items (and unscheduled gaps) for a playout.", "/api/v1/playouts/{id}/items", IdPath("Playout id.")), + Get( + "ersatztv_get_playout_items", + "Get upcoming items (and unscheduled gaps) for a playout (paged).", + "/api/v1/playouts/{id}/items", + [IdPath("Playout id.")], + Page()), Get("ersatztv_list_ffmpeg_profiles", "List FFmpeg profiles.", "/api/v1/ffmpeg/profiles"), Get("ersatztv_get_ffmpeg_profile", "Get an FFmpeg profile by id.", "/api/v1/ffmpeg/profiles/{id}", IdPath("FFmpeg profile id.")), Get( @@ -161,7 +166,12 @@ public static class ToolCatalog "ersatztv_reset_channel_playout", "Queue a rebuild of a channel's playout (202 Accepted; 409 if a build is already running).", "/api/v1/channels/{id}/playout/reset", - [IdPath("Channel id.")], + [ + IdPath( + "CHANNEL id — not the playout id. The two id spaces overlap numerically, so " + + "passing a playout id here silently resets a different channel; take this " + + "value from a playout row's channelId field (issue #616).") + ], [Str("mode", "Optional playout build mode; omit for the default. GET a playout to see valid values.", arg: In.Query)]), Delete("ersatztv_delete_channel", "Delete a channel.", "/api/v1/channels/{id}", IdPath("Channel id.")), Put( @@ -230,10 +240,21 @@ public static class ToolCatalog private static Arg ObjArray(string name, string description, bool required = false) => new(name, "array", description, required, In.Body, ItemType: "object"); + // Paging mirrors the REST API it wraps, which is 0-BASED everywhere (issue #616): every paged + // controller defaults pageNum to 0 and skips `pageNum * pageSize`. The description said "1-based", + // so a caller that trusted it silently skipped the first page and read the result as data loss. + // Documented rather than translated: a 1-based MCP over a 0-based API would make the same + // parameter name mean two different things depending on which surface you were reading. + // pageSize is clamped server-side and the cap is PER-ENDPOINT (100 for most reads, 200 for + // auto-tune members, 1000 for search/all-items). The invariant is not a single number: the + // offset always derives from the EFFECTIVE page size, never the requested one. private static Arg[] Page() => [ - Int("pageNum", "1-based page number (optional).", arg: In.Query), - Int("pageSize", "Page size (optional).", arg: In.Query) + Int("pageNum", "0-based page number; the first page is 0 (optional, default 0).", arg: In.Query), + Int( + "pageSize", + "Page size (optional). Clamped server-side, so pages may be narrower than requested.", + arg: In.Query) ]; // The channel create/update body (CreateChannelRequest / UpdateChannelRequest — the id comes from diff --git a/ErsatzTV.Tests/Application/MediaCollections/GetCollectionItemsHandlerTests.cs b/ErsatzTV.Tests/Application/MediaCollections/GetCollectionItemsHandlerTests.cs index 99ecf15bf..8f7e4255f 100644 --- a/ErsatzTV.Tests/Application/MediaCollections/GetCollectionItemsHandlerTests.cs +++ b/ErsatzTV.Tests/Application/MediaCollections/GetCollectionItemsHandlerTests.cs @@ -61,6 +61,93 @@ public class GetCollectionItemsHandlerTests page.Page.Select(i => i.Title).ShouldBe(["Zeta", "Alpha", "Beta"]); } + // Paging semantics, pinned because #616 reported them as two bugs that measurement did not + // support. pageNum is 0-BASED (the trap: the MCP catalog documented it as 1-based, so a caller + // starting at 1 silently skipped the first page and read a short set as data loss). + [Test] + public async Task Handle_Should_Treat_PageNum_As_Zero_Based() + { + await SeedNumberedCollection(150); + var handler = new GetCollectionItemsHandler(_db.Factory); + + Either first = + await handler.Handle(new GetCollectionItems(10, 0, 10), CancellationToken.None); + Either second = + await handler.Handle(new GetCollectionItems(10, 1, 10), CancellationToken.None); + + // Page 0 is the FIRST page, not a skipped one; page 1 is the second. + first.RightToSeq().Single().Page.Select(i => i.Title).First().ShouldBe("Item 001"); + second.RightToSeq().Single().Page.Select(i => i.Title).First().ShouldBe("Item 011"); + } + + // The second half of #616's claim was that an over-large pageSize caps the returned page but + // leaves the OFFSET computed from the requested value, so page 1 at pageSize=500 would land past + // item 500. It does not: the size is clamped first and the offset derives from the clamped value. + [Test] + public async Task Handle_Should_Derive_Offset_From_The_Clamped_PageSize() + { + await SeedNumberedCollection(150); + var handler = new GetCollectionItemsHandler(_db.Factory); + + // pageSize 500 clamps to 100, so page 1 starts at item 101 and runs to the end (50 items). + // If the offset honored the requested 500, this page would start past the collection and be + // empty — which is exactly what the mutation of this fix produces. + Either result = + await handler.Handle(new GetCollectionItems(10, 1, 500), CancellationToken.None); + + PagedLibraryBrowseItemsResponseModel page = result.RightToSeq().Single(); + page.TotalCount.ShouldBe(150); + page.Page.Count.ShouldBe(50); + page.Page.Select(i => i.Title).First().ShouldBe("Item 101"); + page.Page.Select(i => i.Title).Last().ShouldBe("Item 150"); + } + + private async Task SeedNumberedCollection(int count) + { + await using TvContext context = _db.CreateContext(); + + var library = new LocalLibrary + { + Id = 1, + Name = "Library", + MediaKind = LibraryMediaKind.Movies, + Paths = [] + }; + var path = new LibraryPath + { + Id = 1, + Path = "/media", + Library = library, + LibraryFolders = [], + MediaItems = [] + }; + library.Paths.Add(path); + + var collection = new Collection + { + Id = 10, + Name = "Manual", + UseCustomPlaybackOrder = false, + MediaItems = [], + CollectionItems = [], + MultiCollections = [], + MultiCollectionItems = [] + }; + + var movies = new List(); + for (var i = 1; i <= count; i++) + { + // Zero-padded so the handler's title ordering matches numeric order. + movies.Add(MakeMovie(1000 + i, path, $"Item {i:D3}")); + collection.CollectionItems.Add(new CollectionItem { MediaItemId = 1000 + i }); + } + + context.LocalLibraries.Add(library); + context.Movies.AddRange(movies); + context.Collections.Add(collection); + await context.SaveChangesAsync(); + } + private async Task SeedCollectionGraph(bool useCustomPlaybackOrder) { await using TvContext context = _db.CreateContext(); diff --git a/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs b/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs index 9a23ce239..ab59c4893 100644 --- a/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs @@ -166,6 +166,25 @@ public class PlayoutControllerTests .Seed.ShouldBe(4242); } + // #616: the playout DETAIL response carried channelName/channelNumber but no channelId, while + // reset_channel_playout takes a CHANNEL id. The two id spaces overlap numerically, so a caller + // that reached for the row's `id` reset a different channel and got a plausible 202 back. The + // list rows gained channelId in #297; this pins the same field on the detail response, and the + // distinct ids below prove it is the channel's, not the playout's. + [Test] + public async Task GetById_Should_Surface_ChannelId_Distinct_From_PlayoutId() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakePlayout(9) with { ChannelId = 400 })); + + IActionResult result = await _controller.GetById(9, CancellationToken.None); + + PlayoutResponseModel body = result.ShouldBeOfType() + .Value.ShouldBeOfType(); + body.Id.ShouldBe(9); + body.ChannelId.ShouldBe(400); + } + [Test] public async Task GetAll_Should_Surface_Seed() { @@ -1432,6 +1451,7 @@ public class PlayoutControllerTests vm.ScheduleKind, vm.ChannelName, vm.ChannelNumber, + vm.ChannelId, vm.PlayoutMode, vm.ScheduleName, vm.ScheduleFile, diff --git a/ErsatzTV/Controllers/Api/PlayoutController.cs b/ErsatzTV/Controllers/Api/PlayoutController.cs index a52f8d85a..4fa94da83 100644 --- a/ErsatzTV/Controllers/Api/PlayoutController.cs +++ b/ErsatzTV/Controllers/Api/PlayoutController.cs @@ -818,6 +818,7 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : vm.ScheduleKind, vm.ChannelName, vm.ChannelNumber, + vm.ChannelId, vm.PlayoutMode, vm.ScheduleName, vm.ScheduleFile, diff --git a/ErsatzTV/wwwroot/openapi/v1.json b/ErsatzTV/wwwroot/openapi/v1.json index 02dec0207..5d384d488 100644 --- a/ErsatzTV/wwwroot/openapi/v1.json +++ b/ErsatzTV/wwwroot/openapi/v1.json @@ -28954,6 +28954,7 @@ "scheduleKind", "channelName", "channelNumber", + "channelId", "playoutMode", "scheduleName", "scheduleFile", @@ -28979,6 +28980,10 @@ "channelNumber": { "type": "string" }, + "channelId": { + "type": "integer", + "format": "int32" + }, "playoutMode": { "$ref": "#/components/schemas/ChannelPlayoutMode" }, diff --git a/docs/api-conventions.md b/docs/api-conventions.md index 636ae60f9..db3546bd5 100644 --- a/docs/api-conventions.md +++ b/docs/api-conventions.md @@ -41,7 +41,12 @@ Exemplars: - **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. + input for page math. **`pageNum` is 0-based** across the whole surface (the first page is `0`) and + the offset is always derived from the *clamped* `pageSize`, so an over-large `pageSize` yields + narrower pages — it never widens the offset. Say "0-based" in the description of any paging + parameter you expose, including on wrapper surfaces like the MCP tool catalog: describing it as + 1-based makes a caller skip the first page silently, which reads as data loss rather than as an + off-by-one (ersatztv#616). See `api.paging-zero-based`. - **Sortable GET with allow-listed sort params**: same file — `sortField`/`sortDirection` are normalized against a fixed allow-list (`AllowedSortFields`) rather than trusted or rejected with a 422: an unrecognized `sortField` silently falls back to the default field, an unrecognized diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 1ac29b186..9d77bfabb 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -19,6 +19,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `api.logs-sort-params` | `GET /api/logs` takes allow-listed `sortField` (`timestamp`\|`level`) and `sortDirection` (`asc`\|`desc`) query params, normalized (not rejected) on an unrecognized value. | 2026-07-11 | [link](records/api/logs-sort-params.md) | | `api.mediatr-passthrough` | The REST API is thin controllers over existing MediatR handlers, with no new service/business-logic layer. | 2026-06 | [link](records/api/mediatr-passthrough.md) | | `api.openapi-mirrors-runtime` | The generated OpenAPI spec is made to match the runtime Newtonsoft wire contract (via `NewtonsoftSchemaNamingTransformer`), not the reverse. | 2026-07-09 | [link](records/api/openapi-mirrors-runtime.md) | +| `api.paging-zero-based` | `pageNum` is 0-based across the entire `/api/v1` surface and every wrapper of it (MCP tool catalog, SPA hooks, docs); the page offset is always derived from the EFFECTIVE (bounded) `pageSize`, never the requested one, so a `pageSize` above an endpoint's cap narrows the page without widening the offset. The cap itself is per-endpoint (100 typical, 200 auto-tune members, 1000 search/all-items) and must not be documented as one number. A paging parameter description that omits or contradicts "0-based" is a defect. | 2026-07-25 | [link](records/api/paging-zero-based.md) | | `api.parentid-drillin` | Media drill-in (season/episode/artist/music-video) is served by an optional `parentId` query param on library-browse, not dedicated per-kind child-listing endpoints. | 2026-07-07 | [link](records/api/parentid-drillin.md) | | `api.playout-build-lock-409` | Every id-keyed playout/channel mutation endpoint checks `IEntityLocker.IsPlayoutLocked(id)` and returns 409 Conflict while a build is in-flight, mirroring Blazor's disabled-buttons behavior; reset-all stays 202 and silently skips locked playouts. | 2026-07-10 | [link](records/api/playout-build-lock-409.md) | | `api.postcommit-cancellation-none` | Once a mutation commits, its entire compensating side effect (enqueues, publishes, reindexes, cache refresh, and any post-commit lookup gating one of those) runs on `CancellationToken.None` so a late client disconnect can't half-abort an already-committed change. | 2026-07-11 | [link](records/api/postcommit-cancellation-none.md) | diff --git a/docs/decisions/records/api/paging-zero-based.md b/docs/decisions/records/api/paging-zero-based.md new file mode 100644 index 000000000..4b5d8f930 --- /dev/null +++ b/docs/decisions/records/api/paging-zero-based.md @@ -0,0 +1,58 @@ +--- +key: api.paging-zero-based +title: 2026-07-25 — Paging is 0-based everywhere; every wrapper must say so (OpenAPI still doesn't) (#616) +status: active +since: '2026-07-25' +supersedes: none +superseded-by: none +rule: '`pageNum` is 0-based across the entire `/api/v1` surface and every wrapper of it (MCP tool catalog, SPA hooks, docs); the page offset is always derived from the EFFECTIVE (bounded) `pageSize`, never the requested one, so a `pageSize` above an endpoint''s cap narrows the page without widening the offset. The cap itself is per-endpoint (100 typical, 200 auto-tune members, 1000 search/all-items) and must not be documented as one number. A paging parameter description that omits or contradicts "0-based" is a defect.' +signals: '`pageNum`, `pageSize`, `Math.Clamp(pageSize`, `Skip(pageNum * pageSize)`, "1-based", off-by-one paging, short result set, MCP `Page()` · paths: `ErsatzTV.Mcp/ToolCatalog.cs`, `ErsatzTV/Controllers/Api/*Controller.cs` · issues: #616, #487, #58' +mechanics: '`Math.Max(0, pageNum)` + a per-endpoint upper bound on `pageSize` (`Math.Clamp(pageSize, 1, MaxPageSize)` in most controllers) then `Skip(PageNum * PageSize)` in the handler' +--- + +Every paged controller on `/api/v1` defaults `pageNum` to `0`, floors it at 0 (`Math.Max(0, pageNum)` +— `search/all-items` uses `Math.Clamp(pageNum, 0, 2_000_000)` because it additionally needs an upper +bound to keep `pageNum * pageSize` inside `int`), bounds `pageSize` above by a per-endpoint maximum, +and passes **both bounded values** to a handler that skips `PageNum * PageSize`. That makes paging uniformly 0-based, and makes the offset a function +of the effective size rather than the requested one. + +The *maximum* is deliberately not uniform and must not be documented as if it were: most reads clamp +`Math.Clamp(pageSize, 1, MaxPageSize)` with `MaxPageSize = 100`, `GetAutoTuneChannelMembers` uses +`pageSize <= 0 ? 100 : Math.Min(pageSize, 200)`, and `GET /api/v1/search/all-items` defaults to 500 +and caps at 1000. So `pageSize=500` is narrowed to 100 on a collection listing and honored verbatim +on all-items. The invariant that holds everywhere is the *derivation* (offset from the effective +size), not any single cap. + +The convention was correct in code and unwritten everywhere else, which is how it produced a bug +report. The MCP tool catalog described `pageNum` as "1-based page number", so a caller that started +at `pageNum=1` skipped the first page: a 15-item collection returned 0 items and a 204-item +collection returned 104. Nothing errored — the caller just got a short set, which reads as *data +loss*, not as an off-by-one, and cost a verification pass being chased as one (#487). + +The same report's second claim — that `pageSize` is capped for the returned page while the offset +still honours the requested value — was **not** reproducible and is not true of any endpoint. The +observation behind it (`pageSize=500&pageNum=2` on a 204-item collection returning 4 items) is +exactly correct 0-based behaviour at the clamped width of 100: page 2 is items 201–204. Both +behaviours are now pinned by mutation-verified tests in `GetCollectionItemsHandlerTests`, so the next +reader does not have to re-derive which half was real. + +**Direction of the fix.** The alternative was to make the MCP layer 1-based and translate. Rejected: +`/api/v1` is additive-only post-freeze (`api.versioning-v1`), 0-based is already load-bearing in a +dozen controllers and the SPA, and a 1-based wrapper over a 0-based API would make the *same +parameter name* mean different things on two surfaces a reader routinely reads together — trading a +documented off-by-one for an undocumented one. Accuracy in the description is the cheaper contract. + +**Where "0-based" is stated, and where it still isn't.** The MCP tool catalog and these docs say it +explicitly. The generated OpenAPI `pageNum` parameters carry **no description at all** (12 of them), +so a REST consumer reading only `v1.json` still has to infer the base from the default — a real +remaining gap, tracked separately rather than fixed here. Treat "every wrapper says 0-based" as the +target this record sets, not a property already true of the OpenAPI surface. + +**Corollary — ids in paged rows.** A row that names a related entity should expose that entity's id, +not only its display fields, wherever a caller is expected to act on that entity. This is a rule about +actionable ids, not an audit result: `PlayoutListItemResponseModel.ScheduleName` still ships without +a schedule id, which is fine while nothing asks a caller to address a schedule from that row. `reset_channel_playout` takes a *channel* id while playout rows exposed only +the playout `id` plus channel name/number; the id spaces overlap numerically, so passing the row's id +silently reset a different channel and returned a plausible 202. List rows gained `channelId` in #297; +#616 added it to `PlayoutResponseModel` (the detail response) and named the trap in the MCP argument +description. diff --git a/docs/mcp.md b/docs/mcp.md index 17cd40f0d..251570a7f 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -84,6 +84,33 @@ When a response carries an `ETag` header (versioned aggregates emit it on GET an replace PUT), the tool result appends a `\n[etag: "N"]` marker so an agent can round-trip it as `ifMatch` on a subsequent write. +### Paging (`api.paging-zero-based`) + +Tools whose endpoint is paged declare `pageNum`/`pageSize` and pass them through unchanged — the MCP +layer does **not** translate them, so they mean exactly what they mean on `/api/v1`. (A tool that +wraps a paged endpoint *without* declaring these is a bug, not a "no paging needed" signal: +`ToolArgumentValidator` rejects undeclared arguments, so the caller is hard-capped at the first page +with no way to ask for more. `ersatztv_list_playouts` and `ersatztv_get_playout_items` were capped +that way until #616. `ToolCatalogTests` pins the expected set.) + +- **`pageNum` is 0-based.** The first page is `0`. Starting at `1` silently skips a page and returns a + short set with no error, which is easy to misread as missing data (ersatztv#616 — the catalog used + to describe it as 1-based, and #487 lost a verification pass to it). +- **`pageSize` is clamped server-side, and the cap is per-endpoint** — 100 for most reads + (collections, library browse, logs, playouts), 200 for auto-tune channel members, 1000 for + `ersatztv_search_all_items` (whose *default* is 500). The invariant is not a single number: it is + that the offset is derived from the **effective** page size, never the requested one. So + `pageSize=500` on a 100-capped tool gives 100-wide pages (page 1 = items 101–200), while the same + value on `search_all_items` is under its cap and is honored (page 1 = items 501–1000). Don't assume + your requested size held — page to completeness against `totalCount`. + +Where a row is meant to be acted on, it carries the *id* of the entity you act on and not only its +display name — take the channel id for `ersatztv_reset_channel_playout` from a playout row's +`channelId`, never from its `id` (that is the *playout* id, and the two id spaces overlap +numerically, so the wrong one silently succeeds against a different channel). This is a rule for +actionable references, not a claim that every row is fully id-annotated: `scheduleName` on a playout +row still ships without a schedule id, because nothing asks you to address a schedule from there. + ### Optimistic concurrency (`api-conventions.md` §7a) Only the **replace-all aggregate PUTs** honor `If-Match` — here that is diff --git a/web/src/api/generated/v1.d.ts b/web/src/api/generated/v1.d.ts index 5e4ad129b..f1edb164a 100644 --- a/web/src/api/generated/v1.d.ts +++ b/web/src/api/generated/v1.d.ts @@ -1168,6 +1168,7 @@ export interface components { "scheduleKind": components["schemas"]["PlayoutScheduleKind"]; "channelName": string; "channelNumber": string; + "channelId": number; "playoutMode": components["schemas"]["ChannelPlayoutMode"]; "scheduleName": string; "scheduleFile": null | string; diff --git a/web/src/screens/SchedulesScreen.test.tsx b/web/src/screens/SchedulesScreen.test.tsx index 72ddbfe75..bc527c8cf 100644 --- a/web/src/screens/SchedulesScreen.test.tsx +++ b/web/src/screens/SchedulesScreen.test.tsx @@ -170,6 +170,22 @@ describe('SchedulesScreen — load', () => { expect(screen.getByLabelText('Schedule lineup')).toBeTruthy(); }); + it('requests the rerun-collection picker from page 0, not page 1 (#616)', async () => { + const handle = await renderReady(); + + const rerunRequests = handle.requests.filter((r) => r.url.startsWith('/api/v1/rerun-collections')); + expect(rerunRequests.length).toBeGreaterThan(0); + + // pageNum is 0-based (api.paging-zero-based). This asked for page 1, and because pageSize is + // clamped to 100 server-side that skipped the first 100 rows — so the picker showed nothing at + // all for the ordinary case of <=100 rerun collections. Assert the offset, not just the URL + // shape: `pageNum=1` here is a silent empty picker, never an error. + for (const request of rerunRequests) { + const pageNum = new URL(request.url, 'http://localhost').searchParams.get('pageNum'); + expect(pageNum).toBe('0'); + } + }); + it('renders the "Active schedule" selector non-full-width so it cannot overflow the header (#463)', async () => { await renderReady(); const select = screen.getByLabelText('Active schedule'); diff --git a/web/src/screens/SchedulesScreen.tsx b/web/src/screens/SchedulesScreen.tsx index a00b7b820..b3a660858 100644 --- a/web/src/screens/SchedulesScreen.tsx +++ b/web/src/screens/SchedulesScreen.tsx @@ -49,7 +49,10 @@ type BootState = async function loadPickerData(): Promise { const [rerun, playlistGroups, watermarks, graphicsElements, languages, pre, mid, post, tail, fallback] = await Promise.all([ - getRerunCollections({ pageNum: 1, pageSize: 1000 }), + // pageNum is 0-BASED (api.paging-zero-based). This asked for page 1, which skipped the first + // page entirely: pageSize is clamped to 100 server-side, so the picker was served rows 101+ + // and showed nothing at all for the normal case of <=100 rerun collections (ersatztv#616). + getRerunCollections({ pageNum: 0, pageSize: 1000 }), getPlaylistGroups(), getWatermarks(), getGraphicsElements(),