diff --git a/docs/contracts/ip-assignment-pagination.md b/docs/contracts/ip-assignment-pagination.md new file mode 100644 index 00000000..1e108a64 --- /dev/null +++ b/docs/contracts/ip-assignment-pagination.md @@ -0,0 +1,33 @@ +# IP assignment pagination (#208) + +The administrative audit uses `from_id` as a row anchor in the server's +`(from_date, id)` order, in either direction. This requires +[vpsAdmin PR44](https://github.com/vpsfreecz/vpsadmin/pull/44), tested resource +proposal `320af0e152ed223bf0365e0f1cf4b38cf00d7b1d`. Do not promote this UI +against the old API's ID-only predicate: taking the minimum or maximum ID +cannot recover rows omitted by a differently ordered server. + +Each request carries the same exact IP, owner, VPS, active-state and order +filters and asks for page size plus one. Only the visible rows are rendered. +Next uses the final displayed row, not the hidden lookahead; no lookahead means +no next page, even for a full terminal page. A refreshed previous page rebuilds +its forward edge. Cancellation prevents obsolete requests from continuing. + +A failed request offers retry and, for cursor pages, a return to the first page +without clearing filters. Empty pages retain backward navigation. The shared +committed-URL restoration fix from UI PR496 is required for browser history; +this PR is stacked on that branch. The member route guard denies this admin +page before the audit query runs. This does not change API permissions. + +Fixture Playwright covers both directions, three pages, timestamp ties, +nonmonotonic IDs, exact end, history/reload, stale forward edges, error recovery, +empty cursor pages, member denial and the existing exact-IP/legacy-link flow. +The cs/en pagination and recovery cases run on desktop and mobile. These mocks +implement the proposed server contract; they do not certify real API behavior. + +Remaining gate: run the exact UI/API pair on the existing isolated VM with +synthetic records, including real owner scoping and invalid cursor responses. +The resource specs and fixture tests are separate evidence. Pages are separate +requests, not a snapshot across concurrent changes. UserNetwork's bounded +active-assignment lookup and IncidentReportNew's active lookup are not paginated +history views and are not changed here. diff --git a/e2e/specs/admin/ip_address_assignment_exact_filter.spec.ts b/e2e/specs/admin/ip_address_assignment_exact_filter.spec.ts index b4d07552..3492774d 100644 --- a/e2e/specs/admin/ip_address_assignment_exact_filter.spec.ts +++ b/e2e/specs/admin/ip_address_assignment_exact_filter.spec.ts @@ -117,7 +117,7 @@ test('@pr-smoke legacy assignment q links are canonical before the first list re ipAddr: exactAddress, q: null, fromId: null, - limit: '25', + limit: '26', }]); await page.goBack(); diff --git a/e2e/specs/admin/ip_assignment_cursor.spec.ts b/e2e/specs/admin/ip_assignment_cursor.spec.ts new file mode 100644 index 00000000..b68d720c --- /dev/null +++ b/e2e/specs/admin/ip_assignment_cursor.spec.ts @@ -0,0 +1,152 @@ +import { expect, test } from '../../fixtures/vpsadmin-window'; +import { bootstrapVpsAdminWindow } from '../../fixtures/bootstrap'; +import { failEnvelope, installHaveApiMock, jsonFulfill } from '../../fixtures/haveapi'; +import { setUiSettingsLocalStorage } from '../../fixtures/uiSettings'; + +const prefix = 'admin.ip_assignments'; +const base = '/admin/networking/ip-address-assignments'; +// API44 tuple ordering: timestamps have ties, while IDs are not monotonic. +const assignments = Array.from({ length: 75 }, (_, i) => ({ + id: ((i * 29) % 75) + 1, + from_date: new Date(Date.UTC(2026, 0, 1, 0, Math.floor(i / 3))).toISOString(), + ip_addr: '192.0.2.10', ip_prefix: 32, + user: { id: 7, login: 'fixture-member' }, + vps: { id: 42, hostname: 'fixture-vps' }, +})); + +for (const language of ['cs', 'en'] as const) { + for (const order of ['oldest', 'newest'] as const) { + test(`@pr-smoke @pr-smoke-mobile IP audit traverses tied dates and unrelated IDs (${language}, ${order})`, async ({ page }) => { + await setUiSettingsLocalStorage(page, { language }); + const rows = [...assignments].sort((a, b) => + (a.from_date.localeCompare(b.from_date) || a.id - b.id) * (order === 'oldest' ? 1 : -1)); + const cursors: number[] = []; + const writes: string[] = []; + page.on('request', request => { + if (['POST', 'PUT', 'PATCH', 'DELETE'].includes(request.method()) + && !request.url().endsWith('/webui_user_settings')) writes.push(request.url()); + }); + await installHaveApiMock(page, { + user: { id: 1, login: 'admin', level: 99 }, + handlers: { + 'GET ip_address_assignments': ({ searchParams }) => { + const param = (key: string) => searchParams.get(`ip_address_assignment[${key}]`); + expect(param('order')).toBe(order); + expect(param('user')).toBe('7'); + expect(param('vps')).toBe('42'); + expect(param('ip_addr')).toBe('192.0.2.10'); + expect(param('active')).toBe('true'); + expect(param('limit')).toBe('26'); + const cursor = Number(param('from_id') ?? 0); + cursors.push(cursor); + const start = cursor ? rows.findIndex(row => row.id === cursor) + 1 : 0; + return { ip_address_assignments: rows.slice(start, start + 26) }; + }, + }, + }); + await bootstrapVpsAdminWindow(page); + await page.goto(`${base}?limit=25&order=${order}&user=7&vps=42&ip_addr=192.0.2.10&active=true`); + const visible = () => page.locator(`[data-testid^="${prefix}.row."]:not([data-testid$=".dot"])`); + for (let p = 0; p < 3; p++) { + const expected = rows.slice(p * 25, (p + 1) * 25).map(row => `${prefix}.row.${row.id}`); + await expect(visible()).toHaveCount(25); + await expect.poll(() => visible().evaluateAll(elements => elements.map(el => el.getAttribute('data-testid')))).toEqual(expected); + if (p < 2) await page.getByTestId(`${prefix}.pagination.next`).click(); + } + await expect(page.getByTestId(`${prefix}.pagination.next`)).toBeDisabled(); + expect(cursors).toEqual([0, rows[24].id, rows[49].id]); + await page.reload(); + await expect(page.getByTestId(`${prefix}.row.${rows[50].id}`)).toBeVisible(); + await expect(page.getByTestId(`${prefix}.pagination.next`)).toBeDisabled(); + await page.getByTestId(`${prefix}.pagination.prev`).click(); + await expect(page.getByTestId(`${prefix}.row.${rows[25].id}`)).toBeVisible(); + await page.goBack(); + await expect(page.getByTestId(`${prefix}.row.${rows[50].id}`)).toBeVisible(); + expect(writes).toEqual([]); + }); + } + + test(`@pr-smoke @pr-smoke-mobile IP audit recovers cursor errors and keeps filters (${language})`, async ({ page }) => { + await setUiSettingsLocalStorage(page, { language }); + let fail = true; + const cursors: number[] = []; + await installHaveApiMock(page, { + user: { id: 1, login: 'admin', level: 99 }, + handlers: { + 'GET ip_address_assignments': ({ searchParams }) => { + const param = (key: string) => searchParams.get(`ip_address_assignment[${key}]`); + expect(param('user')).toBe('7'); + expect(param('active')).toBe('false'); + expect(param('order')).toBe('oldest'); + const cursor = Number(param('from_id') ?? 0); + cursors.push(cursor); + if (cursor && fail) return jsonFulfill(failEnvelope('Invalid cursor', { from_id: ['invalid'] }), 400); + return { ip_address_assignments: cursor ? [] : [assignments[0]] }; + }, + }, + }); + await bootstrapVpsAdminWindow(page); + await page.goto(`${base}?limit=25&from_id=999&page=2&user=7&active=false&order=oldest`); + await expect(page.getByTestId(`${prefix}.error`)).toBeVisible(); + await expect(page.getByTestId(`${prefix}.pagination.next`)).toBeDisabled(); + fail = false; + await page.getByTestId(`${prefix}.error.primary`).click(); + // Empty cursor results retain a way back; the whole pager must not vanish. + await expect(page.getByTestId(`${prefix}.error`)).toHaveCount(0); + await expect(page.getByTestId(`${prefix}.pagination.prev`)).toBeEnabled(); + await page.getByTestId(`${prefix}.pagination.prev`).click(); + await expect(page.getByTestId(`${prefix}.row.${assignments[0].id}`)).toBeVisible(); + expect(cursors.at(-1)).toBe(0); + fail = true; + await page.goto(`${base}?limit=25&from_id=999&page=2&user=7&active=false&order=oldest`); + await page.getByTestId(`${prefix}.error.secondary`).click(); + await expect(page.getByTestId(`${prefix}.row.${assignments[0].id}`)).toBeVisible(); + await expect(page).not.toHaveURL(/from_id=/); + await expect(page).toHaveURL(/user=7/); + await expect(page).toHaveURL(/active=false/); + }); +} + +test('@pr-smoke @pr-smoke-mobile IP audit rebuilds a visited forward cursor after a changed first page', async ({ page }) => { + let rows = [...assignments].sort((a, b) => a.from_date.localeCompare(b.from_date) || a.id - b.id); + const cursors: number[] = []; + await installHaveApiMock(page, { + user: { id: 1, login: 'admin', level: 99 }, + handlers: { + 'GET ip_address_assignments': ({ searchParams }) => { + const cursor = Number(searchParams.get('ip_address_assignment[from_id]') ?? 0); + cursors.push(cursor); + const start = cursor ? rows.findIndex(row => row.id === cursor) + 1 : 0; + return { ip_address_assignments: rows.slice(start, start + 26) }; + }, + }, + }); + await bootstrapVpsAdminWindow(page); + await page.goto(`${base}?limit=25&order=oldest`); + await page.getByTestId(`${prefix}.pagination.next`).click(); + await expect(page.getByTestId(`${prefix}.row.${rows[25].id}`)).toBeVisible(); + await page.getByTestId(`${prefix}.pagination.prev`).click(); + await expect(page.getByTestId(`${prefix}.row.${rows[0].id}`)).toBeVisible(); + rows = rows.slice(1); + await page.reload(); + await expect(page.getByTestId(`${prefix}.row.${rows[24].id}`)).toBeVisible(); + await page.getByTestId(`${prefix}.pagination.next`).click(); + await expect(page.getByTestId(`${prefix}.row.${rows[25].id}`)).toBeVisible(); + expect(cursors.at(-1)).toBe(rows[24].id); +}); + +test('@pr-smoke @pr-smoke-mobile member cannot fetch the administrative IP audit', async ({ page }) => { + await setUiSettingsLocalStorage(page, { language: 'en' }); + let reads = 0; + await installHaveApiMock(page, { + user: { id: 7, login: 'member', level: 1 }, + handlers: { + 'GET ip_address_assignments': () => { reads++; return { ip_address_assignments: [] }; }, + }, + }); + await bootstrapVpsAdminWindow(page); + await page.goto(base); + await expect(page.getByTestId(`${prefix}.page`)).toHaveCount(0); + await expect(page.getByText('Admin access required', { exact: true })).toBeVisible(); + expect(reads).toBe(0); +}); diff --git a/src/i18n/locales/cs/admin/ip_assignments.ts b/src/i18n/locales/cs/admin/ip_assignments.ts index b829703b..a7fe4d4b 100644 --- a/src/i18n/locales/cs/admin/ip_assignments.ts +++ b/src/i18n/locales/cs/admin/ip_assignments.ts @@ -18,6 +18,7 @@ export const csAdmin_ip_assignments = { "admin.ip_assignments.filter.user.placeholder": "Přezdívka nebo ID uživatele…", "admin.ip_assignments.filter.vps.placeholder": "Název hostitele nebo ID VPS…", "admin.ip_assignments.load_error": "Přiřazení IP se nepodařilo načíst", + "admin.ip_assignments.restart": "Zpět na první stránku", "admin.ip_assignments.subtitle": "Audituj, který uživatel/VPS držel adresu a kdy.", "admin.ip_assignments.title": "Audit přiřazení IP", } as const; diff --git a/src/i18n/locales/en/admin/ip_assignments.ts b/src/i18n/locales/en/admin/ip_assignments.ts index a6e91c23..49f25beb 100644 --- a/src/i18n/locales/en/admin/ip_assignments.ts +++ b/src/i18n/locales/en/admin/ip_assignments.ts @@ -18,6 +18,7 @@ export const enAdmin_ip_assignments = { "admin.ip_assignments.filter.user.placeholder": "User login or ID…", "admin.ip_assignments.filter.vps.placeholder": "VPS hostname or ID…", "admin.ip_assignments.load_error": "Failed to load IP assignments", + "admin.ip_assignments.restart": "Back to first page", "admin.ip_assignments.subtitle": "Audit which user/VPS held an address and when.", "admin.ip_assignments.title": "IP assignment audit", } as const; diff --git a/src/lib/api/networking.ts b/src/lib/api/networking.ts index 5f87cd88..60bb8308 100644 --- a/src/lib/api/networking.ts +++ b/src/lib/api/networking.ts @@ -156,6 +156,7 @@ export async function fetchIpAddressAssignments(opts?: { network?: number; order?: 'newest' | 'oldest'; includes?: string; + signal?: AbortSignal; }) { const params: Record = {}; if (opts?.limit !== undefined) params['limit'] = opts.limit; @@ -172,6 +173,7 @@ export async function fetchIpAddressAssignments(opts?: { method: 'GET', path: '/ip_address_assignments', namespace: 'ip_address_assignment', + signal: opts?.signal, params, meta: { includes: opts?.includes ?? 'user,vps,assigned_by_chain,unassigned_by_chain,ip_address', diff --git a/src/pages/app/admin/networking/IpAssignmentsPage.tsx b/src/pages/app/admin/networking/IpAssignmentsPage.tsx index 17add364..8f1c1330 100644 --- a/src/pages/app/admin/networking/IpAssignmentsPage.tsx +++ b/src/pages/app/admin/networking/IpAssignmentsPage.tsx @@ -6,7 +6,6 @@ import { useI18n } from '../../../../app/i18n'; import { fetchIpAddressAssignments } from '../../../../lib/api/networking'; import { formatDateTime } from '../../../../lib/format'; import { useKeysetPagination } from '../../../../lib/hooks/useKeysetPagination'; -import { cursorFromDescendingPage } from '../../../../lib/lockIndex'; import { parseBoolParam, parsePositiveInt } from '../../../../lib/parse'; import { ListShell } from '../../../../components/layout/ListShell'; import { PageHeader } from '../../../../components/layout/PageHeader'; @@ -61,18 +60,31 @@ function IpAssignmentsPageContent() { setSearchParams: setSp, defaultLimit: limit, allowedLimits: [25, 50, 100], + restoreUrlCursorOnSignatureChange: true, }); const listQ = useQuery({ queryKey: ['ip_address_assignments', 'list', { ipAddr, userId, vpsId, active, order, limit: paging.limit, fromId: paging.cursor ?? null }], - queryFn: async () => - (await fetchIpAddressAssignments({ ipAddr: ipAddr || undefined, user: userId, vps: vpsId, active, order, limit: paging.limit, fromId: paging.cursor ?? undefined })).data, - placeholderData: (prev) => prev, + queryFn: async ({ signal }) => + (await fetchIpAddressAssignments({ ipAddr: ipAddr || undefined, user: userId, vps: vpsId, active, order, limit: paging.limit + 1, fromId: paging.cursor ?? undefined, signal })).data, }); - const rows = listQ.data ?? []; - const nextCursor = cursorFromDescendingPage(rows, (r) => Number((r as any).id)); - const canNext = Boolean(nextCursor); + const rows = (listQ.data ?? []).slice(0, paging.limit); + // API44 orders by (from_date, id), not ID alone. The lookahead row is + // intentionally excluded from the anchor so the next page still displays it. + const nextCursor = rows.at(-1)?.id; + const canNext = !listQ.isFetching && !listQ.isError + && (listQ.data?.length ?? 0) > paging.limit + && Number.isSafeInteger(nextCursor) && Number(nextCursor) > 0; + const restart = () => paging.goToPageWithStack(1, [null]); + const goNext = () => { + if (!canNext || nextCursor == null) return; + // Rebuild the forward edge after a refetch instead of reusing a stale + // visited cursor (e.g. when assignments changed on a previous page). + paging.goToPageWithStack(paging.page + 1, [ + ...paging.stack.slice(0, paging.index + 1), nextCursor, + ]); + }; const setParam = (key: string, value?: string) => { const next = new URLSearchParams(sp); @@ -109,8 +121,18 @@ function IpAssignmentsPageContent() { /> } > - {listQ.isLoading ? : listQ.isError ? : rows.length === 0 ? : ( - paging.goNext(nextCursor ?? null)} onGoToPage={paging.goToPage} limit={paging.limit} onLimitChange={paging.setLimit} />}> + {listQ.isLoading ? : listQ.isError ? listQ.refetch() }, + secondary: paging.cursor != null + ? { label: t('admin.ip_assignments.restart'), onClick: restart } + : undefined, + }} + /> : rows.length === 0 ? : ( + @@ -151,6 +173,14 @@ function IpAssignmentsPageContent() { )} + ); }