fix(634): stop the #616 guard from asserting an invariant the fix violates
Review verdict / Set review-verdict status (pull_request) Successful in 3s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 14s
PR Gates / Docs update reminder (pull_request) Successful in 14s
PR Gates / decisions lifecycle (pull_request) Successful in 30s
review-verdict/h10 Review-verdict: MERGEABLE @ f9380eb
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 6m41s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 10s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 7s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 18m48s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 24m49s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Review verdict / Set review-verdict status (pull_request) Successful in 3s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 14s
PR Gates / Docs update reminder (pull_request) Successful in 14s
PR Gates / decisions lifecycle (pull_request) Successful in 30s
review-verdict/h10 Review-verdict: MERGEABLE @ f9380eb
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 6m41s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 10s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 7s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 18m48s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 24m49s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
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.
This commit is contained in:
@@ -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 () => {
|
||||
|
||||
@@ -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<RerunCollection[]> {
|
||||
|
||||
Reference in New Issue
Block a user