From bcbdc9c97695f5ef92d39899fd5c200f6c2e3c3d Mon Sep 17 00:00:00 2001 From: Timothy Date: Sun, 26 Jul 2026 11:01:53 +0200 Subject: [PATCH 1/3] fix(634): page the rerun-collection picker to completeness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SchedulesScreen loaded the rerun-collection picker with getRerunCollections({ pageNum: 0, pageSize: 1000 }). The server (RerunCollectionController) clamps pageSize via Math.Clamp(pageSize, 1, MaxPageSize) with MaxPageSize=100, so the request was silently served only the first 100 rows regardless of what was asked for. With >100 rerun collections, the picker omitted the rest with no error and no truncation indicator — a schedule item couldn't be pointed at a rerun collection past the 100th. Fix: page the client to completeness against totalCount, mirroring CollectionsScreen.enterReorder (fetch page 0, keep requesting subsequent pages while accumulated < totalCount, break early if a page returns zero rows to guard against a non-terminating loop on a server-side anomaly). Per api.search-allitems-paging precedent, the client pages rather than raising the server's MaxPageSize cap. Audited the other loadPickerData fetches (getPlaylistGroups, getWatermarks, getGraphicsElements, getLanguages, getFillerPresetsByKind): their endpoints return a plain, unpaged array server-side with no pageNum/pageSize params and no clamp, so they aren't subject to the same silent-truncation defect and don't need the same treatment. Adds a vitest case pinning the exact expected option set (150 rerun collections across two pages) rather than a non-empty/truthy check. Co-Authored-By: Claude Opus 5 (1M context) --- web/src/screens/SchedulesScreen.test.tsx | 33 ++++++++++++++++++ web/src/screens/SchedulesScreen.tsx | 43 ++++++++++++++++++++---- 2 files changed, 69 insertions(+), 7 deletions(-) diff --git a/web/src/screens/SchedulesScreen.test.tsx b/web/src/screens/SchedulesScreen.test.tsx index bc527c8cf..f9f945dc6 100644 --- a/web/src/screens/SchedulesScreen.test.tsx +++ b/web/src/screens/SchedulesScreen.test.tsx @@ -186,6 +186,39 @@ describe('SchedulesScreen — load', () => { } }); + it('pages the rerun-collection picker to completeness when totalCount exceeds one page (#634)', async () => { + // Server clamps pageSize to 100 (MaxPageSize) regardless of what's requested, so a single + // request only ever returns 100 rows. With 150 rerun collections the picker must issue a + // second page (pageNum=1) and end up with all 150 — not silently stop at 100. + const page0 = Array.from({ length: 100 }, (_, i) => ({ id: i + 1, name: `Rerun ${i + 1}` })); + const page1 = Array.from({ length: 50 }, (_, i) => ({ id: i + 101, name: `Rerun ${i + 101}` })); + + const handle = await renderReady({ + items: [responseItem({ collectionType: 'RerunFirstRun', collectionId: null, rerunCollectionId: null })], + onRequest: (url) => { + if (!url.startsWith('/api/v1/rerun-collections')) { + return null; + } + const pageNum = new URL(url, 'http://localhost').searchParams.get('pageNum'); + if (pageNum === '0') { + return jsonResponse({ totalCount: 150, page: page0 }); + } + if (pageNum === '1') { + return jsonResponse({ totalCount: 150, page: page1 }); + } + return jsonResponse({ totalCount: 150, page: [] }); + } + }); + + const rerunRequests = handle.requests.filter((r) => r.url.startsWith('/api/v1/rerun-collections')); + expect(rerunRequests.length).toBe(2); + + const select = screen.getByLabelText('Rerun collection') as HTMLSelectElement; + // '(none)' + 150 rerun collections — the exact expected set, not a truthy/non-empty check. + const values = Array.from(select.options).map((o) => o.value); + expect(values).toEqual(['', ...Array.from({ length: 150 }, (_, i) => `${i + 1}`)]); + }, 15000); + 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 b3a660858..f9203df51 100644 --- a/web/src/screens/SchedulesScreen.tsx +++ b/web/src/screens/SchedulesScreen.tsx @@ -22,7 +22,8 @@ import { getWatermarks, messageFromScheduleError, type ProgramSchedule, - replaceScheduleItems + replaceScheduleItems, + type RerunCollection } from '../api'; import { registerNavigationGuard } from '../navigationGuard'; import { usePrimaryAction } from '../primaryAction'; @@ -41,18 +42,46 @@ import { ScheduleItemInspector, type SchedulePickerData } from '../schedules/Sch const DIRTY_PROMPT = 'You have unsaved schedule changes. Discard them?'; +// Server clamps pageSize to MaxPageSize=100 (RerunCollectionController) regardless of what's +// requested, so a single `pageSize: 1000` request only ever returns the first 100 rows. Page to +// completeness against totalCount instead of raising the server cap (api.search-allitems-paging +// precedent — the client pages, the server stays bounded). Mirrors CollectionsScreen.enterReorder. +const RERUN_PAGE_SIZE = 100; + +async function loadAllRerunCollections(): Promise { + const first = await getRerunCollections({ pageNum: 0, pageSize: RERUN_PAGE_SIZE }); + let all = first.page ?? []; + const totalCount = first.totalCount ?? all.length; + let pageNum = 1; + + while (all.length < totalCount) { + const next = await getRerunCollections({ pageNum, pageSize: RERUN_PAGE_SIZE }); + const nextPage = next.page ?? []; + + if (nextPage.length === 0) { + break; + } + + all = [...all, ...nextPage]; + pageNum += 1; + } + + return all; +} + type BootState = | { status: 'loading' } | { status: 'error'; error: string } | { status: 'ready'; schedules: ProgramSchedule[]; pickers: SchedulePickerData }; +// Other picker fetches below (getPlaylistGroups, getWatermarks, getGraphicsElements, getLanguages, +// getFillerPresetsByKind) hit endpoints that return a plain, unpaged array server-side — no +// pageNum/pageSize params, no clamp — so they aren't subject to the same silent-truncation defect +// as rerun collections and don't need the same treatment (ersatztv#634). async function loadPickerData(): Promise { - const [rerun, playlistGroups, watermarks, graphicsElements, languages, pre, mid, post, tail, fallback] = + const [rerunCollections, playlistGroups, watermarks, graphicsElements, languages, pre, mid, post, tail, fallback] = await Promise.all([ - // 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 }), + loadAllRerunCollections(), getPlaylistGroups(), getWatermarks(), getGraphicsElements(), @@ -65,7 +94,7 @@ async function loadPickerData(): Promise { ]); return { - rerunCollections: rerun.page ?? [], + rerunCollections, playlistGroups, watermarks, graphicsElements, -- 2.47.3 From f9164b71afdeec75eb16d77e0b536f837e568a2a Mon Sep 17 00:00:00 2001 From: Timothy Date: Sun, 26 Jul 2026 11:11:33 +0200 Subject: [PATCH 2/3] fix(634): keep the 0-based lesson at the call site MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #616 comment this call site carried recorded WHY it reads `pageNum: 0` — a previous version passed 1 and skipped the whole first page. Rewriting the call for #634 dropped it. Restore it next to the new paging loop, which also starts its follow-up requests at 1 and is only correct because the first page is 0. --- web/src/screens/SchedulesScreen.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/web/src/screens/SchedulesScreen.tsx b/web/src/screens/SchedulesScreen.tsx index f9203df51..b350b82e6 100644 --- a/web/src/screens/SchedulesScreen.tsx +++ b/web/src/screens/SchedulesScreen.tsx @@ -46,6 +46,11 @@ const DIRTY_PROMPT = 'You have unsaved schedule changes. Discard them?'; // requested, so a single `pageSize: 1000` request only ever returns the first 100 rows. Page to // completeness against totalCount instead of raising the server cap (api.search-allitems-paging // precedent — the client pages, the server stays bounded). Mirrors CollectionsScreen.enterReorder. +// +// `pageNum` is 0-BASED here and everywhere on /api/v1 (api.paging-zero-based): the first page is 0, +// so the loop below starts its follow-up requests at 1. This call previously passed `pageNum: 1`, +// which skipped the first page entirely and showed nothing at all for the normal case of <=100 +// rerun collections (ersatztv#616) — the same call site, one defect earlier. const RERUN_PAGE_SIZE = 100; async function loadAllRerunCollections(): Promise { -- 2.47.3 From f9380eb494db1ed27221ab31494eb62f4c7e2647 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sun, 26 Jul 2026 11:16:20 +0200 Subject: [PATCH 3/3] fix(634): stop the #616 guard from asserting an invariant the fix violates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cold review finding. The #616 regression test looped over EVERY rerun-collection request asserting pageNum === '0'. That was right when exactly one request was ever issued, but since this branch the loader legitimately walks pageNum 1, 2, … to page to completeness — so the assertion now describes something the correct code does not do. It passes today only because the shared fixture's totalCount fits in a single page. Raising that default would have failed the #616 test with a "picker requested page 1" signal for what is proper paging, sending the next reader after a defect that isn't there. Narrow it to the first request, which is the offset #616 actually cared about. Also corrects the new comment's history: the `pageNum: 1` it describes is pre-#616, not the previous commit. --- web/src/screens/SchedulesScreen.test.tsx | 11 +++++++---- web/src/screens/SchedulesScreen.tsx | 6 +++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/web/src/screens/SchedulesScreen.test.tsx b/web/src/screens/SchedulesScreen.test.tsx index f9f945dc6..db1a56101 100644 --- a/web/src/screens/SchedulesScreen.test.tsx +++ b/web/src/screens/SchedulesScreen.test.tsx @@ -180,10 +180,13 @@ describe('SchedulesScreen — load', () => { // 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'); - } + // + // Assert only the FIRST request. Since #634 the loader legitimately walks pageNum 1, 2, … to + // page to completeness, so "every request is page 0" is an invariant the CORRECT code violates. + // It holds today only because this fixture's totalCount fits in one page — raising that default + // would fail this test with a misleading "requested page 1" signal for what is proper paging. + const firstPageNum = new URL(rerunRequests[0].url, 'http://localhost').searchParams.get('pageNum'); + expect(firstPageNum).toBe('0'); }); it('pages the rerun-collection picker to completeness when totalCount exceeds one page (#634)', async () => { diff --git a/web/src/screens/SchedulesScreen.tsx b/web/src/screens/SchedulesScreen.tsx index b350b82e6..ab30f1bb8 100644 --- a/web/src/screens/SchedulesScreen.tsx +++ b/web/src/screens/SchedulesScreen.tsx @@ -48,9 +48,9 @@ const DIRTY_PROMPT = 'You have unsaved schedule changes. Discard them?'; // precedent — the client pages, the server stays bounded). Mirrors CollectionsScreen.enterReorder. // // `pageNum` is 0-BASED here and everywhere on /api/v1 (api.paging-zero-based): the first page is 0, -// so the loop below starts its follow-up requests at 1. This call previously passed `pageNum: 1`, -// which skipped the first page entirely and showed nothing at all for the normal case of <=100 -// rerun collections (ersatztv#616) — the same call site, one defect earlier. +// so the loop below starts its follow-up requests at 1. Before ersatztv#616 this call site passed +// `pageNum: 1`, which skipped the first page entirely and showed nothing at all for the ordinary +// case of <=100 rerun collections — the same call site, one defect earlier. const RERUN_PAGE_SIZE = 100; async function loadAllRerunCollections(): Promise { -- 2.47.3