From 84ef9a7cbe32158203147a4d327c2f16a3753e6c Mon Sep 17 00:00:00 2001 From: "Gabriel A. Devenyi" Date: Mon, 20 Jul 2026 15:11:21 -0400 Subject: [PATCH 1/5] perf(datahub): match export rows against a set of listed subject ids The export filter built its list of subject ids by iterating each row's visible cells: .rows.flatMap((row) => row.getVisibleCells().map((cell) => removeSubjectIdScope(cell.row.original.id))) The callback ignores `cell` and reads `cell.row.original.id`, so the row's id is emitted once per rendered column. Measured against the real table, which has three data columns plus the row-actions column libui adds: 5,000 rows produce a 20,000 entry array, a 4x duplication, with `removeSubjectIdScope` called four times per row. That array was then the right-hand side of an `includes` inside a `filter` over the export, so matching was a linear scan per exported row against a list four times longer than necessary. Read one id per row into a Set instead. Measured filtering 200,000 export rows against 5,000 listed subjects, identical results (167,000 matched either way): before 3,070 ms after 17 ms A 25,000-record group with 30 measures per record produces around 750,000 export rows, so the figure above is conservative. Still client-side: the export is fetched for the whole group and narrowed in the browser, so every filter the user has applied is honoured only after the payload has crossed the network. Pushing the subject ids to the export endpoint is the real fix and composes with the streaming rework in #1407. Refs #1412 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Bfen9VQemZanJLXinJ6MMG --- .../_app/datahub/__tests__/index.test.tsx | 85 +++++++++++++++++++ apps/web/src/routes/_app/datahub/index.tsx | 18 +++- 2 files changed, 99 insertions(+), 4 deletions(-) create mode 100644 apps/web/src/routes/_app/datahub/__tests__/index.test.tsx diff --git a/apps/web/src/routes/_app/datahub/__tests__/index.test.tsx b/apps/web/src/routes/_app/datahub/__tests__/index.test.tsx new file mode 100644 index 000000000..a74a5c0ce --- /dev/null +++ b/apps/web/src/routes/_app/datahub/__tests__/index.test.tsx @@ -0,0 +1,85 @@ +import React from 'react'; + +import { DataTable } from '@douglasneuroinformatics/libui/components'; +import type { TanstackTable } from '@douglasneuroinformatics/libui/components'; +import { render } from '@testing-library/react'; +import { beforeAll, describe, expect, it } from 'vitest'; + +// Initialises the shared libui translator, which the table's controls read on render. +import '@/services/i18n'; + +const noop = () => undefined; + +type Row = { id: string }; + +/** + * Renders a table shaped like the datahub master table (several columns plus row actions) and hands + * back its tanstack instance, so the id extraction can be exercised against a real row model rather + * than a hand-built stand-in. + */ +const renderMasterTableLike = (data: Row[]) => { + let table: TanstackTable.Table | undefined; + const Capture = (props: { table: TanstackTable.Table }) => { + table = props.table; + return null; + }; + render( + + columns={[ + { accessorFn: (row) => row.id, header: 'Subject', id: 'subjectId' }, + { accessorFn: () => null, header: 'DOB', id: 'dateOfBirth' }, + { accessorFn: () => null, header: 'Sex', id: 'sex' } + ]} + data={data} + rowActions={[{ label: 'View', onSelect: noop }]} + togglesComponent={Capture} + /> + ); + return table!; +}; + +describe('datahub export subject filter', () => { + beforeAll(() => { + // libui measures the table container; happy-dom has no layout engine. + globalThis.ResizeObserver ??= class { + disconnect = noop; + observe = noop; + unobserve = noop; + } as never; + }); + + it('yields one id per row, not one per rendered cell', () => { + const data = [{ id: 'subject-a' }, { id: 'subject-b' }, { id: 'subject-c' }]; + const table = renderMasterTableLike(data); + const rows = table.getPrePaginationRowModel().rows; + + // The shape the previous implementation produced: `getVisibleCells()` repeats the row's id once + // per column, so the ids arrive duplicated. This asserts the duplication is real rather than + // assumed, and that the current extraction does not reproduce it. + const perCell = rows.flatMap((row) => row.getVisibleCells().map((cell) => cell.row.original.id)); + expect(perCell.length).toBeGreaterThan(rows.length); + + const perRow = new Set(rows.map((row) => row.original.id)); + expect(perRow.size).toBe(data.length); + expect([...perRow]).toStrictEqual(['subject-a', 'subject-b', 'subject-c']); + }); + + it('membership is a set lookup, so an export row is matched without scanning the list', () => { + const table = renderMasterTableLike([{ id: 'subject-a' }, { id: 'subject-b' }]); + const listed = new Set(table.getPrePaginationRowModel().rows.map((row) => row.original.id)); + + const exportRows = [{ subjectId: 'subject-a' }, { subjectId: 'subject-z' }, { subjectId: 'subject-b' }]; + expect(exportRows.filter((row) => listed.has(row.subjectId)).map((row) => row.subjectId)).toStrictEqual([ + 'subject-a', + 'subject-b' + ]); + }); + + it('excludes every export row when the table lists no subjects', () => { + const table = renderMasterTableLike([]); + const listed = new Set(table.getPrePaginationRowModel().rows.map((row) => row.original.id)); + + expect(listed.size).toBe(0); + expect([{ subjectId: 'subject-a' }].filter((row) => listed.has(row.subjectId))).toStrictEqual([]); + }); +}); diff --git a/apps/web/src/routes/_app/datahub/index.tsx b/apps/web/src/routes/_app/datahub/index.tsx index 22317a4a1..5dfc9ee48 100644 --- a/apps/web/src/routes/_app/datahub/index.tsx +++ b/apps/web/src/routes/_app/datahub/index.tsx @@ -38,6 +38,18 @@ type HasSearchStringFilter = { searchString: string; }; +/** + * The subject ids currently listed by the table, for filtering an export down to them. + * + * Read one per row: iterating `getVisibleCells()` yields the row's id once per rendered column + * (including the row-actions column), so the ids arrive duplicated as many times as there are + * columns. A set also keeps the membership test that consumes this constant time rather than a + * linear scan per exported row. + */ +const getListedSubjectIds = (table: TanstackTable.Table): Set => { + return new Set(table.getPrePaginationRowModel().rows.map((row) => removeSubjectIdScope(row.original.id))); +}; + const Filters: React.FC<{ hasRecords: boolean; setHasRecords: (v: boolean) => void; @@ -239,11 +251,9 @@ const Toggles: React.FC<{ getExportRecords() .then((data): any => { - const listedSubjects = table - .getPrePaginationRowModel() - .rows.flatMap((row) => row.getVisibleCells().map((cell) => removeSubjectIdScope(cell.row.original.id))); + const listedSubjects = getListedSubjectIds(table); - const filteredData = data.filter((dataEntry) => listedSubjects.includes(dataEntry.subjectId)); + const filteredData = data.filter((dataEntry) => listedSubjects.has(dataEntry.subjectId)); if (filteredData.length < 1) { throw Error( From c77e33ce7900006092f4c626dba12b49ee85b1f2 Mon Sep 17 00:00:00 2001 From: "Gabriel A. Devenyi" Date: Mon, 27 Jul 2026 14:57:45 -0400 Subject: [PATCH 2/5] test(web): assert the export subject filter against the real helper The test lived under src/routes, which main now bans by eslint because the TanStack route generator scans that directory and reads a dot as a path separator. It also only exercised tanstack's row model and `Set.has`, so it would still have passed had the per-cell extraction been reintroduced. Move `getListedSubjectIds` to src/utils/table.ts and assert it directly, and throw when the table capture fails rather than asserting on `table!`. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014ZEJLwa7jwD8sKzcE4PfSc --- .../_app/datahub/__tests__/index.test.tsx | 85 ------------------- apps/web/src/routes/_app/datahub/index.tsx | 13 +-- apps/web/src/utils/__tests__/table.test.tsx | 76 +++++++++++++++++ apps/web/src/utils/table.ts | 15 ++++ 4 files changed, 92 insertions(+), 97 deletions(-) delete mode 100644 apps/web/src/routes/_app/datahub/__tests__/index.test.tsx create mode 100644 apps/web/src/utils/__tests__/table.test.tsx create mode 100644 apps/web/src/utils/table.ts diff --git a/apps/web/src/routes/_app/datahub/__tests__/index.test.tsx b/apps/web/src/routes/_app/datahub/__tests__/index.test.tsx deleted file mode 100644 index a74a5c0ce..000000000 --- a/apps/web/src/routes/_app/datahub/__tests__/index.test.tsx +++ /dev/null @@ -1,85 +0,0 @@ -import React from 'react'; - -import { DataTable } from '@douglasneuroinformatics/libui/components'; -import type { TanstackTable } from '@douglasneuroinformatics/libui/components'; -import { render } from '@testing-library/react'; -import { beforeAll, describe, expect, it } from 'vitest'; - -// Initialises the shared libui translator, which the table's controls read on render. -import '@/services/i18n'; - -const noop = () => undefined; - -type Row = { id: string }; - -/** - * Renders a table shaped like the datahub master table (several columns plus row actions) and hands - * back its tanstack instance, so the id extraction can be exercised against a real row model rather - * than a hand-built stand-in. - */ -const renderMasterTableLike = (data: Row[]) => { - let table: TanstackTable.Table | undefined; - const Capture = (props: { table: TanstackTable.Table }) => { - table = props.table; - return null; - }; - render( - - columns={[ - { accessorFn: (row) => row.id, header: 'Subject', id: 'subjectId' }, - { accessorFn: () => null, header: 'DOB', id: 'dateOfBirth' }, - { accessorFn: () => null, header: 'Sex', id: 'sex' } - ]} - data={data} - rowActions={[{ label: 'View', onSelect: noop }]} - togglesComponent={Capture} - /> - ); - return table!; -}; - -describe('datahub export subject filter', () => { - beforeAll(() => { - // libui measures the table container; happy-dom has no layout engine. - globalThis.ResizeObserver ??= class { - disconnect = noop; - observe = noop; - unobserve = noop; - } as never; - }); - - it('yields one id per row, not one per rendered cell', () => { - const data = [{ id: 'subject-a' }, { id: 'subject-b' }, { id: 'subject-c' }]; - const table = renderMasterTableLike(data); - const rows = table.getPrePaginationRowModel().rows; - - // The shape the previous implementation produced: `getVisibleCells()` repeats the row's id once - // per column, so the ids arrive duplicated. This asserts the duplication is real rather than - // assumed, and that the current extraction does not reproduce it. - const perCell = rows.flatMap((row) => row.getVisibleCells().map((cell) => cell.row.original.id)); - expect(perCell.length).toBeGreaterThan(rows.length); - - const perRow = new Set(rows.map((row) => row.original.id)); - expect(perRow.size).toBe(data.length); - expect([...perRow]).toStrictEqual(['subject-a', 'subject-b', 'subject-c']); - }); - - it('membership is a set lookup, so an export row is matched without scanning the list', () => { - const table = renderMasterTableLike([{ id: 'subject-a' }, { id: 'subject-b' }]); - const listed = new Set(table.getPrePaginationRowModel().rows.map((row) => row.original.id)); - - const exportRows = [{ subjectId: 'subject-a' }, { subjectId: 'subject-z' }, { subjectId: 'subject-b' }]; - expect(exportRows.filter((row) => listed.has(row.subjectId)).map((row) => row.subjectId)).toStrictEqual([ - 'subject-a', - 'subject-b' - ]); - }); - - it('excludes every export row when the table lists no subjects', () => { - const table = renderMasterTableLike([]); - const listed = new Set(table.getPrePaginationRowModel().rows.map((row) => row.original.id)); - - expect(listed.size).toBe(0); - expect([{ subjectId: 'subject-a' }].filter((row) => listed.has(row.subjectId))).toStrictEqual([]); - }); -}); diff --git a/apps/web/src/routes/_app/datahub/index.tsx b/apps/web/src/routes/_app/datahub/index.tsx index 5dfc9ee48..f8c8c6058 100644 --- a/apps/web/src/routes/_app/datahub/index.tsx +++ b/apps/web/src/routes/_app/datahub/index.tsx @@ -25,6 +25,7 @@ import { PageHeader } from '@/components/PageHeader'; import { subjectsQueryOptions, useSubjectsQuery } from '@/hooks/useSubjectsQuery'; import { useAppStore } from '@/store'; import { downloadExcel } from '@/utils/excel'; +import { getListedSubjectIds } from '@/utils/table'; type DateFilter = { allowNull: boolean; @@ -38,18 +39,6 @@ type HasSearchStringFilter = { searchString: string; }; -/** - * The subject ids currently listed by the table, for filtering an export down to them. - * - * Read one per row: iterating `getVisibleCells()` yields the row's id once per rendered column - * (including the row-actions column), so the ids arrive duplicated as many times as there are - * columns. A set also keeps the membership test that consumes this constant time rather than a - * linear scan per exported row. - */ -const getListedSubjectIds = (table: TanstackTable.Table): Set => { - return new Set(table.getPrePaginationRowModel().rows.map((row) => removeSubjectIdScope(row.original.id))); -}; - const Filters: React.FC<{ hasRecords: boolean; setHasRecords: (v: boolean) => void; diff --git a/apps/web/src/utils/__tests__/table.test.tsx b/apps/web/src/utils/__tests__/table.test.tsx new file mode 100644 index 000000000..e78cb6fee --- /dev/null +++ b/apps/web/src/utils/__tests__/table.test.tsx @@ -0,0 +1,76 @@ +import React from 'react'; + +import { DataTable } from '@douglasneuroinformatics/libui/components'; +import type { TanstackTable } from '@douglasneuroinformatics/libui/components'; +import type { Subject } from '@opendatacapture/schemas/subject'; +import { render } from '@testing-library/react'; +import { beforeAll, describe, expect, it } from 'vitest'; + +import { getListedSubjectIds } from '@/utils/table'; + +// Initialises the shared libui translator, which the table's controls read on render. +import '@/services/i18n'; + +const noop = () => undefined; + +/** + * Renders a table shaped like the datahub master table (several columns plus row actions) and hands + * back its tanstack instance, so the id extraction is exercised against a real row model rather than + * a hand-built stand-in. + */ +const renderMasterTableLike = (ids: string[]) => { + let table: TanstackTable.Table | undefined; + const Capture = (props: { table: TanstackTable.Table }) => { + table = props.table; + return null; + }; + render( + + columns={[ + { accessorFn: (subject) => subject.id, header: 'Subject', id: 'subjectId' }, + { accessorFn: () => null, header: 'DOB', id: 'dateOfBirth' }, + { accessorFn: () => null, header: 'Sex', id: 'sex' } + ]} + data={ids.map((id) => ({ id }) as Subject)} + rowActions={[{ label: 'View', onSelect: noop }]} + togglesComponent={Capture} + /> + ); + if (!table) { + throw new Error('DataTable did not invoke togglesComponent, so no tanstack table was captured'); + } + return table; +}; + +describe('getListedSubjectIds', () => { + beforeAll(() => { + // libui measures the table container; happy-dom has no layout engine. + globalThis.ResizeObserver ??= class { + disconnect = noop; + observe = noop; + unobserve = noop; + } as never; + }); + + it('should yield one id per row, not the one-per-rendered-cell duplication the row model offers', () => { + const table = renderMasterTableLike(['subject-a', 'subject-b', 'subject-c']); + const rows = table.getPrePaginationRowModel().rows; + + // Asserts the duplication the previous implementation hit is real rather than assumed: + // `getVisibleCells()` repeats the row's id once per column, including the row-actions column. + const perCell = rows.flatMap((row) => row.getVisibleCells().map((cell) => cell.row.original.id)); + expect(perCell.length).toBeGreaterThan(rows.length); + + expect([...getListedSubjectIds(table)]).toStrictEqual(['subject-a', 'subject-b', 'subject-c']); + }); + + it('should strip the group scope, so the ids match the unscoped subject ids an export carries', () => { + const table = renderMasterTableLike(['Group_A$subject-a', 'Group_A$subject-b']); + + expect([...getListedSubjectIds(table)]).toStrictEqual(['subject-a', 'subject-b']); + }); + + it('should return an empty set when the table lists no subjects, so nothing is exported', () => { + expect(getListedSubjectIds(renderMasterTableLike([])).size).toBe(0); + }); +}); diff --git a/apps/web/src/utils/table.ts b/apps/web/src/utils/table.ts new file mode 100644 index 000000000..2065fdcbd --- /dev/null +++ b/apps/web/src/utils/table.ts @@ -0,0 +1,15 @@ +import type { TanstackTable } from '@douglasneuroinformatics/libui/components'; +import type { Subject } from '@opendatacapture/schemas/subject'; +import { removeSubjectIdScope } from '@opendatacapture/subject-utils'; + +/** + * The subject ids currently listed by the table, for filtering an export down to them. + * + * Read one per row: iterating `getVisibleCells()` yields the row's id once per rendered column + * (including the row-actions column), so the ids arrive duplicated as many times as there are + * columns. A set also keeps the membership test that consumes this constant time rather than a + * linear scan per exported row. + */ +export function getListedSubjectIds(table: TanstackTable.Table): Set { + return new Set(table.getPrePaginationRowModel().rows.map((row) => removeSubjectIdScope(row.original.id))); +} From f3a117b56be3205e2cc506b4edcea2a468e4bbdc Mon Sep 17 00:00:00 2001 From: "Gabriel A. Devenyi" Date: Tue, 28 Jul 2026 13:32:10 -0400 Subject: [PATCH 3/5] test(e2e): pin the datahub export to the subjects the table lists Which records reach an exported file is decided client-side, from the rows the master table is currently listing. Nothing covered that: `datahub.spec.ts` asserted only that the page header renders, so getting the scoping wrong would have handed the user another subject's data with no test failing. Seeds two subjects with records, filters the table down to one, exports JSON and asserts the payload names only that subject. Verified by mutation -- reading the core row model instead of the pre-pagination one, which ignores the filter, fails it. The test needs to know exactly what its group holds, and the `roleAccount` group is shared by every spec in a worker, so this adds an `isolatedGroupManager` fixture that authenticates into a group of its own, plus `uploadRecords` and `findInstrumentIdByName` on the api client for seeding a subject that has a record at all. Also from review: the fixture builds a complete `Subject` rather than casting `{ id }` into one. Note: #1426 introduces the same `isolatedGroupManager` fixture, so whichever of the two merges second should drop its copy. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014ZEJLwa7jwD8sKzcE4PfSc --- apps/web/src/utils/__tests__/table.test.tsx | 15 +++++- testing/src/pages/_app/datahub/index.page.ts | 22 ++++++++- testing/src/specs/datahub.spec.ts | 50 ++++++++++++++++++++ testing/src/support/api-client.ts | 31 ++++++++++++ testing/src/support/fixtures.ts | 25 ++++++++++ 5 files changed, 141 insertions(+), 2 deletions(-) diff --git a/apps/web/src/utils/__tests__/table.test.tsx b/apps/web/src/utils/__tests__/table.test.tsx index e78cb6fee..c3b0bd48a 100644 --- a/apps/web/src/utils/__tests__/table.test.tsx +++ b/apps/web/src/utils/__tests__/table.test.tsx @@ -13,6 +13,19 @@ import '@/services/i18n'; const noop = () => undefined; +/** A complete `Subject`, so the fixture satisfies the type rather than being cast into it. */ +const subject = (id: string): Subject => ({ + createdAt: new Date(0), + dateOfBirth: null, + firstName: null, + groupIds: [], + id, + lastName: null, + sessionIds: [], + sex: null, + updatedAt: new Date(0) +}); + /** * Renders a table shaped like the datahub master table (several columns plus row actions) and hands * back its tanstack instance, so the id extraction is exercised against a real row model rather than @@ -31,7 +44,7 @@ const renderMasterTableLike = (ids: string[]) => { { accessorFn: () => null, header: 'DOB', id: 'dateOfBirth' }, { accessorFn: () => null, header: 'Sex', id: 'sex' } ]} - data={ids.map((id) => ({ id }) as Subject)} + data={ids.map(subject)} rowActions={[{ label: 'View', onSelect: noop }]} togglesComponent={Capture} /> diff --git a/testing/src/pages/_app/datahub/index.page.ts b/testing/src/pages/_app/datahub/index.page.ts index 6f3bc1461..678eb2b73 100644 --- a/testing/src/pages/_app/datahub/index.page.ts +++ b/testing/src/pages/_app/datahub/index.page.ts @@ -1,13 +1,33 @@ -import type { Locator, Page } from '@playwright/test'; +import type { Download, Locator, Page } from '@playwright/test'; import { AppPage } from '../route.page'; export class DatahubPage extends AppPage { + readonly exportDropdown: Locator; readonly pageHeader: Locator; readonly rowActionsTrigger: Locator; + readonly rows: Locator; + /** libui's `SearchBar` carries no testid; it renders an `input[type=search]`. */ + readonly searchInput: Locator; constructor(page: Page) { super(page); + this.exportDropdown = page.getByTestId('datahub-export-dropdown'); this.pageHeader = page.getByTestId('page-header'); this.rowActionsTrigger = page.getByTestId('row-actions-trigger').first(); + this.rows = page.getByTestId('data-table-body').getByTestId('data-table-row'); + this.searchInput = page.getByRole('searchbox').first(); + } + + /** Picks a format from the export menu and returns the file it produced. */ + async exportAs(format: 'CSV' | 'Excel' | 'JSON'): Promise { + const started = this.$ref.waitForEvent('download'); + await this.exportDropdown.click(); + await this.$ref.getByRole('menuitem', { exact: true, name: format }).click(); + return started; + } + + /** Filters the master table by subject id, which is what the export is scoped to. */ + async searchSubjects(value: string) { + await this.searchInput.fill(value); } } diff --git a/testing/src/specs/datahub.spec.ts b/testing/src/specs/datahub.spec.ts index f529a3b39..57e262975 100644 --- a/testing/src/specs/datahub.spec.ts +++ b/testing/src/specs/datahub.spec.ts @@ -1,9 +1,59 @@ +import { DatahubPage } from '../pages/_app/datahub/index.page'; import { expect, test } from '../support/fixtures'; +/** A minimal payload satisfying the seeded happiness questionnaire's validation schema. */ +const HAPPINESS_RECORD = { + isSatisfiedOverall: true, + personalLifeSatisfaction: 8, + professionalLifeSatisfaction: 7 +}; + test.describe('data hub', () => { test('should display the data hub header', async ({ getPageModel }) => { const datahubPage = await getPageModel('/datahub'); await expect(datahubPage.pageHeader).toBeVisible(); await expect(datahubPage.pageHeader).toContainText('Data Hub'); }); + + // The export endpoint returns every record in the group; which of them reach the file is decided + // client-side from the rows the table is currently listing. Nothing else covers that scoping, and + // getting it wrong hands the user another subject's data. + test('should export only the subjects the table is listing', async ({ + api, + isolatedGroupManager, + page, + uniqueId + }) => { + const group = await isolatedGroupManager(); + const instrumentId = await api.findInstrumentIdByName('DNP_HAPPINESS_QUESTIONNAIRE'); + const listed = `export-${uniqueId}-listed`; + const filteredOut = `export-${uniqueId}-filtered-out`; + await api.uploadRecords( + group.id, + instrumentId, + [listed, filteredOut].map((subjectId) => ({ data: HAPPINESS_RECORD, date: new Date(), subjectId })) + ); + + const datahubPage = new DatahubPage(page); + await datahubPage.goto('/datahub'); + await expect(datahubPage.rows).toHaveCount(2); + + await datahubPage.searchSubjects(listed); + await expect(datahubPage.rows).toHaveCount(1); + + const download = await datahubPage.exportAs('JSON'); + const payload = JSON.parse(await readAll(download)) as { subjectId: string }[]; + + expect(payload.length).toBeGreaterThan(0); + expect([...new Set(payload.map((row) => row.subjectId))]).toStrictEqual([listed]); + }); }); + +async function readAll(download: Awaited>): Promise { + const stream = await download.createReadStream(); + const chunks: Buffer[] = []; + for await (const chunk of stream) { + chunks.push(chunk as Buffer); + } + return Buffer.concat(chunks).toString('utf8'); +} diff --git a/testing/src/support/api-client.ts b/testing/src/support/api-client.ts index 6b96e4761..cb164308a 100644 --- a/testing/src/support/api-client.ts +++ b/testing/src/support/api-client.ts @@ -1,5 +1,6 @@ import type { $LoginCredentials } from '@opendatacapture/schemas/auth'; import type { CreateGroupData, Group } from '@opendatacapture/schemas/group'; +import type { UploadInstrumentRecordsData } from '@opendatacapture/schemas/instrument-records'; import type { CreateUserData, User } from '@opendatacapture/schemas/user'; import type { APIRequestContext } from '@playwright/test'; @@ -8,6 +9,8 @@ import { randomId } from './unique'; const API = '/api/v1'; +type UploadRecord = UploadInstrumentRecordsData['records'][number]; + /** Typed helper for seeding preconditions (groups, users) and authenticating over the API. */ export class ApiClient { private readonly request: APIRequestContext; @@ -68,6 +71,34 @@ export class ApiClient { return { credentials: { password, username }, user }; } + /** The id of a seeded instrument, looked up by the internal name its source file declares. */ + async findInstrumentIdByName(name: string): Promise { + const instruments = await this.expectJson<{ id: string; internal?: { name: string } }[]>( + this.request.get(`${API}/instruments/info`, { headers: this.authHeaders }), + 200, + 'list instruments' + ); + const instrument = instruments.find((candidate) => candidate.internal?.name === name); + if (!instrument) { + throw new Error(`No instrument named '${name}' among ${instruments.length} returned`); + } + return instrument.id; + } + + /** + * Bulk-creates one record per entry, and with them the subjects and sessions they name. This is the + * cheapest way to give a subject an instrument record: the export only carries subjects that have + * one. + */ + async uploadRecords(groupId: string, instrumentId: string, records: UploadRecord[]): Promise { + const data: UploadInstrumentRecordsData = { groupId, instrumentId, records }; + await this.expectJson( + this.request.post(`${API}/instrument-records/upload`, { data, headers: this.authHeaders }), + 201, + 'upload instrument records' + ); + } + private async expectJson( pending: ReturnType, status: number, diff --git a/testing/src/support/fixtures.ts b/testing/src/support/fixtures.ts index c9a0fd032..5b8e57fa3 100644 --- a/testing/src/support/fixtures.ts +++ b/testing/src/support/fixtures.ts @@ -1,5 +1,6 @@ /* eslint-disable no-empty-pattern */ +import type { Group } from '@opendatacapture/schemas/group'; import { request as apiRequestFactory, test as base, expect } from '@playwright/test'; import type { APIRequestContext } from '@playwright/test'; @@ -60,6 +61,15 @@ type TestFixtures = { authenticateAs: (role: Role) => Promise; /** Navigates to a route as `actingRole` and returns its page object. */ getPageModel: GetPageModel; + /** + * Authenticates as a group manager of a group created for this test alone, and returns that group. + * + * The `roleAccount` group is cached per worker and shared by every spec running in it, so a test + * that asserts on exactly what a group contains cannot use it. A group manager's `read Subject` + * rule is scoped to their own groups, so a fresh group bounds what this test can see to what it + * seeded. + */ + isolatedGroupManager: () => Promise; /** Short run-unique suffix for naming seeded data in this test. */ uniqueId: string; }; @@ -111,6 +121,21 @@ export const test = base.extend({ } ); }, + isolatedGroupManager: async ({ api, apiRequestContext, appState, page }, use) => { + await use(async () => { + const group = await api.createGroup(); + const { credentials } = await api.createUser({ basePermissionLevel: 'GROUP_MANAGER', groupIds: [group.id] }); + const accessToken = await ApiClient.login(apiRequestContext, credentials); + await page.addInitScript( + (injected) => { + window.__PLAYWRIGHT_ACCESS_TOKEN__ = injected.accessToken; + localStorage.setItem('app', JSON.stringify({ state: injected.state, version: 1 })); + }, + { accessToken, state: appState } + ); + return group; + }); + }, roleAccount: [ async ({ adminToken, api, apiRequestContext }, use) => { const cache = new Map([ From 91c59fb8df1687edbb5f81fb917be774a3236340 Mon Sep 17 00:00:00 2001 From: "Gabriel A. Devenyi" Date: Tue, 28 Jul 2026 14:19:52 -0400 Subject: [PATCH 4/5] test(e2e): target the search bar by its testid and document the new fixture The searchbox locator claimed libui's SearchBar carries no testid; it does (data-table-search-bar, on the form wrapping the input), so the role-only lookup and its .first() disambiguation were standing in for a selector the conventions already prefer. The isolatedGroupManager fixture also lands in the AGENTS.md fixture table, and the auth-injection init script it repeated from authenticateAs is shared instead of restated. Co-Authored-By: Claude Fable 5 --- testing/AGENTS.md | 5 ++++ testing/src/pages/_app/datahub/index.page.ts | 3 +-- testing/src/support/fixtures.ts | 28 +++++++++----------- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/testing/AGENTS.md b/testing/AGENTS.md index 6d353efb3..1cb49ce71 100644 --- a/testing/AGENTS.md +++ b/testing/AGENTS.md @@ -65,11 +65,16 @@ A page object is only reachable from a spec once it is registered in the `pageMo | `appState` | test option | localStorage first-run gating; both flags default to accepted/complete | | `uniqueId` | test | Short random suffix for seeded data | | `api` | worker | `ApiClient` as admin — `createGroup()` / `createUser()` for preconditions | +| `isolatedGroupManager` | test | Authenticates into a group created for this test alone; returns the group | | `roleAccount(role)` | worker | Seeds a group + user per role once, then caches its token and username | Set up preconditions over the API with the `api` fixture rather than by clicking through the UI; only drive the UI for the behaviour actually under test. +`roleAccount`'s group is cached per worker and shared by every spec running in it, so a test that +asserts on **how much** a group contains must use `isolatedGroupManager` instead. A group manager +reads only their own groups, so a fresh group bounds what the test can see to what it seeded. + Auth is injected as `window.__PLAYWRIGHT_ACCESS_TOKEN__`, which `apps/web`'s `src/store/slices/auth.slice.ts` reads on boot. It is memory-only and never persisted. diff --git a/testing/src/pages/_app/datahub/index.page.ts b/testing/src/pages/_app/datahub/index.page.ts index 678eb2b73..ba9f6d0e9 100644 --- a/testing/src/pages/_app/datahub/index.page.ts +++ b/testing/src/pages/_app/datahub/index.page.ts @@ -7,7 +7,6 @@ export class DatahubPage extends AppPage { readonly pageHeader: Locator; readonly rowActionsTrigger: Locator; readonly rows: Locator; - /** libui's `SearchBar` carries no testid; it renders an `input[type=search]`. */ readonly searchInput: Locator; constructor(page: Page) { super(page); @@ -15,7 +14,7 @@ export class DatahubPage extends AppPage { this.pageHeader = page.getByTestId('page-header'); this.rowActionsTrigger = page.getByTestId('row-actions-trigger').first(); this.rows = page.getByTestId('data-table-body').getByTestId('data-table-row'); - this.searchInput = page.getByRole('searchbox').first(); + this.searchInput = page.getByTestId('data-table-search-bar').getByRole('searchbox'); } /** Picks a format from the export menu and returns the file it produced. */ diff --git a/testing/src/support/fixtures.ts b/testing/src/support/fixtures.ts index 5b8e57fa3..edc30fbf8 100644 --- a/testing/src/support/fixtures.ts +++ b/testing/src/support/fixtures.ts @@ -2,7 +2,7 @@ import type { Group } from '@opendatacapture/schemas/group'; import { request as apiRequestFactory, test as base, expect } from '@playwright/test'; -import type { APIRequestContext } from '@playwright/test'; +import type { APIRequestContext, Page } from '@playwright/test'; import { SettingsPage } from '../pages/_app/admin/settings.page'; import { DashboardPage } from '../pages/_app/dashboard.page'; @@ -32,6 +32,16 @@ const pageModels = { type PageModels = typeof pageModels; +/** Injects the token and first-run state the web app reads on boot; must run before navigation. */ +const injectAuth = (page: Page, accessToken: string, state: AppState) => + page.addInitScript( + (injected) => { + window.__PLAYWRIGHT_ACCESS_TOKEN__ = injected.accessToken; + localStorage.setItem('app', JSON.stringify({ state: injected.state, version: 1 })); + }, + { accessToken, state } + ); + type GetPageModel = >( key: TKey, ...args: NavigateVariadicArgs @@ -100,13 +110,7 @@ export const test = base.extend({ authenticateAs: async ({ appState, page, roleAccount }, use) => { await use(async (role) => { const { accessToken } = await roleAccount(role); - await page.addInitScript( - (injected) => { - window.__PLAYWRIGHT_ACCESS_TOKEN__ = injected.accessToken; - localStorage.setItem('app', JSON.stringify({ state: injected.state, version: 1 })); - }, - { accessToken, state: appState } - ); + await injectAuth(page, accessToken, appState); }); }, getPageModel: async ({ actingRole, authenticateAs, page }, use) => { @@ -126,13 +130,7 @@ export const test = base.extend({ const group = await api.createGroup(); const { credentials } = await api.createUser({ basePermissionLevel: 'GROUP_MANAGER', groupIds: [group.id] }); const accessToken = await ApiClient.login(apiRequestContext, credentials); - await page.addInitScript( - (injected) => { - window.__PLAYWRIGHT_ACCESS_TOKEN__ = injected.accessToken; - localStorage.setItem('app', JSON.stringify({ state: injected.state, version: 1 })); - }, - { accessToken, state: appState } - ); + await injectAuth(page, accessToken, appState); return group; }); }, From 5b50df6df37b84231334a4080e2e122e4a3de321 Mon Sep 17 00:00:00 2001 From: "Gabriel A. Devenyi" Date: Tue, 28 Jul 2026 18:09:23 -0400 Subject: [PATCH 5/5] test(e2e): read the exported file from disk instead of hand-rolling the stream The stream concatenation needed a chunk cast that lied about string-mode streams; playwright already has the download on disk, so readFile does the same job in one line with no cast. Co-Authored-By: Claude Fable 5 --- testing/src/specs/datahub.spec.ts | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/testing/src/specs/datahub.spec.ts b/testing/src/specs/datahub.spec.ts index 57e262975..89b36ac9d 100644 --- a/testing/src/specs/datahub.spec.ts +++ b/testing/src/specs/datahub.spec.ts @@ -1,3 +1,5 @@ +import { readFile } from 'node:fs/promises'; + import { DatahubPage } from '../pages/_app/datahub/index.page'; import { expect, test } from '../support/fixtures'; @@ -42,18 +44,9 @@ test.describe('data hub', () => { await expect(datahubPage.rows).toHaveCount(1); const download = await datahubPage.exportAs('JSON'); - const payload = JSON.parse(await readAll(download)) as { subjectId: string }[]; + const payload = JSON.parse(await readFile(await download.path(), 'utf8')) as { subjectId: string }[]; expect(payload.length).toBeGreaterThan(0); expect([...new Set(payload.map((row) => row.subjectId))]).toStrictEqual([listed]); }); }); - -async function readAll(download: Awaited>): Promise { - const stream = await download.createReadStream(); - const chunks: Buffer[] = []; - for await (const chunk of stream) { - chunks.push(chunk as Buffer); - } - return Buffer.concat(chunks).toString('utf8'); -}