diff --git a/CHANGELOG.md b/CHANGELOG.md index 64e3906..ec89c80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,8 @@ This file records what changes **in the product** – process and session state - Database roles `app_owner` (migrations) and `app_rw` (runtime, no RLS bypass); schema `app`. - Verify commands `pnpm verify:changed`, `pnpm verify`, `pnpm verify:full`; CI runs integration tests against real PostgreSQL + SeaweedFS. +- Request list pages by 50 (keyset on creation time and id, stable while new requests arrive): + "Ältere Anfragen" / "Zurück zum Anfang" keep the filters; an invalid page parameter shows page 1. ### Changed - AI service logs use the web/worker format: `time` (ISO 8601, `Z`) instead of `ts`, lower-case pino level labels (`warn`, not `WARNING`), `logger` only on library records. diff --git a/src/app/page.tsx b/src/app/page.tsx index 87e7134..b0ac0f4 100644 --- a/src/app/page.tsx +++ b/src/app/page.tsx @@ -1,7 +1,7 @@ import Link from "next/link"; import { Brand } from "@/app/_components/app-header"; import { getRuntime, requestActor } from "@/app/_server/runtime"; -import { listRequests } from "@/features/requests"; +import { countRequestsByStatus } from "@/features/requests"; export const dynamic = "force-dynamic"; @@ -25,10 +25,10 @@ export default async function HomePage() { ); } - // Pilot volumes (20–50 requests a day): counting the tenant's list in memory is enough. - const requests = await getRuntime().tenancy.withTenant(actor.companyId, (tx) => listRequests(tx, {})); - const count = (status: string) => requests.filter((request) => request.status === status).length; - const stats = requests.length ? `${count("REVIEW")} zur Prüfung · ${count("ERROR")} mit Fehler · ${requests.length} insgesamt` : "Noch keine Anfragen"; + // Counted in the database per status – the request list itself is paged (#48). + const counts = await getRuntime().tenancy.withTenant(actor.companyId, (tx) => countRequestsByStatus(tx)); + const total = Object.values(counts).reduce((sum, n) => sum + n, 0); + const stats = total ? `${counts.REVIEW ?? 0} zur Prüfung · ${counts.ERROR ?? 0} mit Fehler · ${total} insgesamt` : "Noch keine Anfragen"; return (

Start

diff --git a/src/app/requests/page.tsx b/src/app/requests/page.tsx index b1c540a..9430370 100644 --- a/src/app/requests/page.tsx +++ b/src/app/requests/page.tsx @@ -2,7 +2,7 @@ import Link from "next/link"; import { redirect } from "next/navigation"; import { getRuntime, requestActor } from "@/app/_server/runtime"; import { listExportRecords } from "@/features/export"; -import { listRequests, type RequestFilter } from "@/features/requests"; +import { listRequests, parseCursor, type RequestFilter } from "@/features/requests"; import { reprocessAction } from "./actions"; import { requestRowView } from "./row-view"; import { REQUEST_STATUS_LABEL } from "./status-labels"; @@ -26,16 +26,29 @@ function filterOf(query: Record): RequestFilter { return { status, possibleDuplicate }; } +/** Link to a page of the list with the current filters (paging never drops them, #48). */ +function pageHref(filter: RequestFilter, after?: string): string { + const params = new URLSearchParams(); + if (filter.status) params.set("status", filter.status); + if (filter.possibleDuplicate) params.set("duplicate", "1"); + if (after) params.set("after", after); + const query = params.toString(); + return query ? `/requests?${query}` : "/requests"; +} + // Request list (#26): status, attempts, last error with its stage, next retry; reprocess for ERROR. export default async function RequestsPage({ searchParams }: { searchParams: Promise> }) { const actor = await requestActor(); if (!actor) redirect("/login"); const query = await searchParams; const filter = filterOf(query); - const { requests, exports } = await getRuntime().tenancy.withTenant(actor.companyId, async (tx) => { - const rows = await listRequests(tx, filter); - return { requests: rows, exports: await listExportRecords(tx, rows.map((row) => row.id)) }; + const after = parseCursor(query.after) ?? undefined; + // One page (#48); export records only for the rows of this page. + const { requests, nextCursor, firstPage, exports } = await getRuntime().tenancy.withTenant(actor.companyId, async (tx) => { + const { rows, nextCursor, firstPage } = await listRequests(tx, filter, { after }); + return { requests: rows, nextCursor, firstPage, exports: await listExportRecords(tx, rows.map((row) => row.id)) }; }); + const paged = !firstPage || nextCursor !== null; const done = query.done === "reprocessed" ? pick("reprocessed") : undefined; const error = query.error === "refused" ? pick("refused") : undefined; @@ -65,7 +78,10 @@ export default async function RequestsPage({ searchParams }: { searchParams: Pro nur mögliche Duplikate - {requests.length === 1 ? "1 Anfrage" : `${requests.length} Anfragen`} + + {requests.length === 1 ? "1 Anfrage" : `${requests.length} Anfragen`} + {paged && " auf dieser Seite"} + {requests.length === 0 ? (

@@ -140,6 +156,12 @@ export default async function RequestsPage({ searchParams }: { searchParams: Pro )} + {paged && ( +

+ )}
); diff --git a/src/features/requests/cursor.test.ts b/src/features/requests/cursor.test.ts new file mode 100644 index 0000000..9715373 --- /dev/null +++ b/src/features/requests/cursor.test.ts @@ -0,0 +1,20 @@ +import { describe, expect, it } from "vitest"; +import { parseCursor, REQUEST_PAGE_SIZE } from "./cursor"; + +describe("request list cursor (#48)", () => { + it("accepts the id of the last row of a page (UUID) and normalises it to lower case", () => { + expect(parseCursor("0b9f5c2e-6a1d-4c3e-9f7a-2d4b8e1c0a55")).toBe("0b9f5c2e-6a1d-4c3e-9f7a-2d4b8e1c0a55"); + expect(parseCursor("0B9F5C2E-6A1D-4C3E-9F7A-2D4B8E1C0A55")).toBe("0b9f5c2e-6a1d-4c3e-9f7a-2d4b8e1c0a55"); + }); + + it.each([undefined, "", "nonsense", "1", "0b9f5c2e-6a1d-4c3e-9f7a-2d4b8e1c0a5", " 0b9f5c2e-6a1d-4c3e-9f7a-2d4b8e1c0a55", "0b9f5c2e-6a1d-4c3e-9f7a-2d4b8e1c0a55' or 1=1", ["0b9f5c2e-6a1d-4c3e-9f7a-2d4b8e1c0a55"]])( + "rejects %j – the list falls back to the first page", + (value) => { + expect(parseCursor(value)).toBeNull(); + }, + ); + + it("uses a fixed page size of 50", () => { + expect(REQUEST_PAGE_SIZE).toBe(50); + }); +}); diff --git a/src/features/requests/cursor.ts b/src/features/requests/cursor.ts new file mode 100644 index 0000000..88ae4fe --- /dev/null +++ b/src/features/requests/cursor.ts @@ -0,0 +1,13 @@ +// Keyset paging of the request list (#48). The cursor is the id of the last row of the previous page; +// the repository resolves its (created_at, id) position in the database, so the full microsecond +// precision of created_at is kept and a foreign or unknown id simply yields the first page. + +/** Fixed page size of the request list. */ +export const REQUEST_PAGE_SIZE = 50; + +const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +/** A cursor from the query string, or null (first page) for anything that is not a single UUID. */ +export function parseCursor(value: unknown): string | null { + return typeof value === "string" && UUID.test(value) ? value.toLowerCase() : null; +} diff --git a/src/features/requests/index.ts b/src/features/requests/index.ts index 14123b9..6c738ae 100644 --- a/src/features/requests/index.ts +++ b/src/features/requests/index.ts @@ -1,10 +1,12 @@ // Public API of the `requests` module: request aggregate and status machine. export { + countRequestsByStatus, createRequest, findDuplicate, getRequest, listRequests, type RequestFilter, + type RequestPage, lockDuplicateDetection, lockRequest, recordDuplicateDecision, @@ -14,4 +16,5 @@ export { type NewRequest, type RequestRow, } from "./repository"; +export { parseCursor, REQUEST_PAGE_SIZE } from "./cursor"; export { canTransition, InvalidTransition, nextStatus, type ErrorStage, type RequestEvent } from "./status"; diff --git a/src/features/requests/repository.ts b/src/features/requests/repository.ts index 3fc6f00..f6dd470 100644 --- a/src/features/requests/repository.ts +++ b/src/features/requests/repository.ts @@ -1,6 +1,7 @@ import { and, asc, desc, eq, or, sql, type SQL } from "drizzle-orm"; import { requests, type RequestStatus } from "@/db/schema"; import { tenantOf, type TenantTx } from "@/features/tenancy"; +import { parseCursor, REQUEST_PAGE_SIZE } from "./cursor"; import { nextStatus, type RequestEvent } from "./status"; export type RequestRow = typeof requests.$inferSelect; @@ -23,14 +24,51 @@ export interface RequestFilter { possibleDuplicate?: boolean; } -/** The company's requests, newest first, optionally filtered (#26). */ -export async function listRequests(tx: TenantTx, filter: RequestFilter = {}): Promise { +export interface RequestPage { + rows: RequestRow[]; + /** Cursor of the next (older) page, or null on the last page. */ + nextCursor: string | null; + /** false when a valid cursor positioned this page; a missing, unknown or foreign cursor gives the first page. */ + firstPage: boolean; +} + +/** + * One page of the company's requests, newest first, optionally filtered (#26, #48). Keyset paging on + * (created_at desc, id desc): `after` is the id of the last row of the previous page; its position is + * read inside this tenant transaction, so a malformed, unknown or foreign id yields the first page. + */ +export async function listRequests(tx: TenantTx, filter: RequestFilter = {}, page: { after?: string | null } = {}): Promise { tenantOf(tx); + const cursor = parseCursor(page.after); + // The anchor's created_at is read as text at full (microsecond) precision in the same query as its + // id, so a JS Date never truncates it and there is no second lookup that could miss (#48 review). + const anchor = cursor + ? (await tx.select({ id: requests.id, createdAt: sql`${requests.createdAt}::text` }).from(requests).where(eq(requests.id, cursor)))[0] + : undefined; const conditions = [ filter.status ? eq(requests.status, filter.status) : undefined, filter.possibleDuplicate === undefined ? undefined : eq(requests.possibleDuplicate, filter.possibleDuplicate), + anchor ? sql`(${requests.createdAt}, ${requests.id}) < (${anchor.createdAt}::timestamptz, ${anchor.id}::uuid)` : undefined, ].filter((condition) => condition !== undefined); - return tx.select().from(requests).where(and(...conditions)).orderBy(desc(requests.createdAt)); + const rows = await tx + .select() + .from(requests) + .where(and(...conditions)) + .orderBy(desc(requests.createdAt), desc(requests.id)) + .limit(REQUEST_PAGE_SIZE + 1); + const hasMore = rows.length > REQUEST_PAGE_SIZE; + const visible = hasMore ? rows.slice(0, REQUEST_PAGE_SIZE) : rows; + return { rows: visible, nextCursor: hasMore ? visible.at(-1)!.id : null, firstPage: anchor === undefined }; +} + +/** Number of the company's requests per status (start page); statuses without requests are absent. */ +export async function countRequestsByStatus(tx: TenantTx): Promise>> { + tenantOf(tx); + const rows = await tx + .select({ status: requests.status, count: sql`count(*)::int` }) + .from(requests) + .groupBy(requests.status); + return Object.fromEntries(rows.map((row) => [row.status, row.count])); } export async function getRequest(tx: TenantTx, id: string): Promise { diff --git a/tests/integration/request-list.test.ts b/tests/integration/request-list.test.ts index 0161314..3bea942 100644 --- a/tests/integration/request-list.test.ts +++ b/tests/integration/request-list.test.ts @@ -10,7 +10,7 @@ import { syntheticExtractResponse } from "@/features/extraction/fixtures"; import { approveRequest, currentFieldValues, currentLineItemValues } from "@/features/review"; import { getActor, type Actor } from "@/features/identity"; import { QUEUES } from "@/features/jobs"; -import { createRequest, getRequest, listRequests, lockRequest, transitionRequest } from "@/features/requests"; +import { countRequestsByStatus, createRequest, getRequest, listRequests, lockRequest, REQUEST_PAGE_SIZE, transitionRequest, type RequestFilter } from "@/features/requests"; import { requestRowView } from "@/app/requests/row-view"; import { createTenancy, type Tenancy } from "@/features/tenancy"; import { companyWithAdmin, createStack, invitedUser, type Stack } from "./helpers/stack"; @@ -89,7 +89,7 @@ describe("request list: errors, retries, filters and reprocessing (#26)", () => const failed = await seeded(clerk, "ERROR_EXPORT"); const foreign = await seeded(other, "ERROR_PROCESSING"); - const rows = await tenancy.withTenant(clerk.companyId, (tx) => listRequests(tx)); + const { rows } = await tenancy.withTenant(clerk.companyId, (tx) => listRequests(tx)); const byId = new Map(rows.map((row) => [row.id, row])); expect(byId.get(retrying)).toMatchObject({ status: "PROCESSING", attempts: 2, errorMessage: "Der KI-Dienst ist nicht erreichbar.", nextRetryAt: expect.any(Date) }); @@ -132,8 +132,8 @@ describe("request list: errors, retries, filters and reprocessing (#26)", () => const duplicate = await seeded(clerk, "DUPLICATE"); const failed = await seeded(clerk, "ERROR_PROCESSING"); - const errors = await tenancy.withTenant(clerk.companyId, (tx) => listRequests(tx, { status: "ERROR" })); - const duplicates = await tenancy.withTenant(clerk.companyId, (tx) => listRequests(tx, { possibleDuplicate: true })); + const { rows: errors } = await tenancy.withTenant(clerk.companyId, (tx) => listRequests(tx, { status: "ERROR" })); + const { rows: duplicates } = await tenancy.withTenant(clerk.companyId, (tx) => listRequests(tx, { possibleDuplicate: true })); expect(errors.every((row) => row.status === "ERROR")).toBe(true); expect(errors.map((row) => row.id)).toContain(failed); @@ -178,3 +178,128 @@ describe("request list: errors, retries, filters and reprocessing (#26)", () => expect((await tenancy.withTenant(other.companyId, (tx) => getRequest(tx, foreign)))?.status).toBe("ERROR"); }); }); + +describe("request list: keyset paging (#48)", () => { + let stack: Stack; + let tenancy: Tenancy; + + const actorOf = async () => (await getActor(stack.auth, stack.database.db, new Headers({ cookie: (await companyWithAdmin(stack)).cookie })))!; + /** Creates `count` requests in ONE transaction – they share created_at (now() is the transaction start), so ties are real. */ + async function seedBatch(actor: Actor, count: number, input: { possibleDuplicate?: boolean } = {}): Promise { + return tenancy.withTenant(actor.companyId, async (tx) => { + const ids: string[] = []; + for (let i = 0; i < count; i++) ids.push((await createRequest(tx, { createdBy: actor.userId, subject: `Seite ${i}`, ...input })).id); + return ids; + }); + } + const page = (actor: Actor, filter: RequestFilter = {}, after?: string) => tenancy.withTenant(actor.companyId, (tx) => listRequests(tx, filter, { after })); + const newestFirst = (a: { createdAt: Date; id: string }, b: { createdAt: Date; id: string }) => + b.createdAt.getTime() - a.createdAt.getTime() || (a.id < b.id ? 1 : a.id > b.id ? -1 : 0); + + beforeAll(() => { + stack = createStack(); + tenancy = createTenancy(stack.database.db); + }); + afterAll(async () => { + await stack.close(); + }); + + it("page 1 holds the 50 newest, the cursor yields the rest – no overlap, no gap, ties broken by id", async () => { + const actor = await actorOf(); + const older = await seedBatch(actor, 30); + const newer = await seedBatch(actor, 30); + + const first = await page(actor); + expect(first.rows).toHaveLength(REQUEST_PAGE_SIZE); + expect(first.nextCursor).toBe(first.rows.at(-1)!.id); + // All 30 of the newer batch come first, then 20 of the older batch (same created_at, id desc). + expect(first.rows.slice(0, 30).map((row) => row.id).sort()).toEqual([...newer].sort()); + expect(first.rows[29]!.createdAt).toEqual(first.rows[0]!.createdAt); + expect(first.rows[30]!.createdAt.getTime()).toBeLessThan(first.rows[29]!.createdAt.getTime()); + + const second = await page(actor, {}, first.nextCursor!); + expect(second.rows).toHaveLength(10); + expect(second.nextCursor).toBeNull(); + expect([first.firstPage, second.firstPage]).toEqual([true, false]); + + const all = [...first.rows, ...second.rows]; + expect(new Set(all.map((row) => row.id)).size).toBe(60); + expect(all.map((row) => row.id).sort()).toEqual([...older, ...newer].sort()); + expect(all).toEqual([...all].sort(newestFirst)); + }); + + it("stays stable when new requests arrive between two page views", async () => { + const actor = await actorOf(); + const seeded = [...(await seedBatch(actor, 26)), ...(await seedBatch(actor, 26))]; + const first = await page(actor); + + const arrived = await seedBatch(actor, 3); + const second = await page(actor, {}, first.nextCursor!); + + expect(second.rows).toHaveLength(2); + expect([...first.rows, ...second.rows].map((row) => row.id).sort()).toEqual([...seeded].sort()); + expect(second.rows.some((row) => arrived.includes(row.id))).toBe(false); + }); + + it("keeps the filter across pages", async () => { + const actor = await actorOf(); + const plainOld = await seedBatch(actor, 5); + const duplicates = [...(await seedBatch(actor, 30, { possibleDuplicate: true })), ...(await seedBatch(actor, 22, { possibleDuplicate: true }))]; + const plainNew = await seedBatch(actor, 5); + + const first = await page(actor, { possibleDuplicate: true }); + const second = await page(actor, { possibleDuplicate: true }, first.nextCursor!); + + expect(first.rows).toHaveLength(50); + expect(second.rows).toHaveLength(2); + expect(second.nextCursor).toBeNull(); + const ids = [...first.rows, ...second.rows].map((row) => row.id); + expect(ids.sort()).toEqual([...duplicates].sort()); + expect(ids.some((id) => plainOld.includes(id) || plainNew.includes(id))).toBe(false); + }); + + it("never shows another company's requests – not even with that company's id as cursor", async () => { + const actor = await actorOf(); + const other = await actorOf(); + const own = await seedBatch(actor, 55); + const foreign = await seedBatch(other, 55); + + const first = await page(actor); + const second = await page(actor, {}, first.nextCursor!); + const ids = [...first.rows, ...second.rows].map((row) => row.id); + expect(ids.sort()).toEqual([...own].sort()); + expect(ids.some((id) => foreign.includes(id))).toBe(false); + + // A foreign row as cursor is unknown inside this tenant → first page, nothing foreign leaks. + const probed = await page(actor, {}, foreign[0]); + expect(probed.rows.map((row) => row.id)).toEqual(first.rows.map((row) => row.id)); + }); + + it("counts the company's requests per status beyond one page (start page) – own company only", async () => { + const actor = await actorOf(); + const other = await actorOf(); + await seedBatch(actor, 53); + await seedBatch(other, 4); + await tenancy.withTenant(actor.companyId, async (tx) => { + const id = (await createRequest(tx, { createdBy: actor.userId })).id; + await transitionRequest(tx, (await lockRequest(tx, id))!, "processing.started"); + }); + + const counts = await tenancy.withTenant(actor.companyId, (tx) => countRequestsByStatus(tx)); + + expect(counts).toEqual({ NEW: 53, PROCESSING: 1 }); + }); + + it("falls back to the first page for a malformed or unknown cursor", async () => { + const actor = await actorOf(); + await seedBatch(actor, 3); + const first = await page(actor); + + for (const cursor of ["nonsense", "", "' or 1=1 --", randomUUID()]) { + const result = await page(actor, {}, cursor); + expect(result.rows.map((row) => row.id)).toEqual(first.rows.map((row) => row.id)); + expect(result.nextCursor).toBeNull(); + expect(result.firstPage).toBe(true); + } + }); +}); diff --git a/tests/integration/tenancy.test.ts b/tests/integration/tenancy.test.ts index 2b06f5c..79126fc 100644 --- a/tests/integration/tenancy.test.ts +++ b/tests/integration/tenancy.test.ts @@ -29,8 +29,8 @@ describe("tenancy: withTenant and forced RLS", () => { }); it("returns only the own company's requests through the repository", async () => { - const rowsA = await tenancy.withTenant(companyA, (tx) => listRequests(tx)); - const rowsB = await tenancy.withTenant(companyB, (tx) => listRequests(tx)); + const { rows: rowsA } = await tenancy.withTenant(companyA, (tx) => listRequests(tx)); + const { rows: rowsB } = await tenancy.withTenant(companyB, (tx) => listRequests(tx)); expect(rowsA.length).toBeGreaterThan(0); expect(rowsA.every((row) => row.companyId === companyA)).toBe(true);