diff --git a/apps/web/src/routes/_app/datahub/index.tsx b/apps/web/src/routes/_app/datahub/index.tsx index 22317a4a1..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; @@ -239,11 +240,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( 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..c3b0bd48a --- /dev/null +++ b/apps/web/src/utils/__tests__/table.test.tsx @@ -0,0 +1,89 @@ +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; + +/** 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 + * 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(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))); +} 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 6f3bc1461..ba9f6d0e9 100644 --- a/testing/src/pages/_app/datahub/index.page.ts +++ b/testing/src/pages/_app/datahub/index.page.ts @@ -1,13 +1,32 @@ -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; + 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.getByTestId('data-table-search-bar').getByRole('searchbox'); + } + + /** 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..89b36ac9d 100644 --- a/testing/src/specs/datahub.spec.ts +++ b/testing/src/specs/datahub.spec.ts @@ -1,9 +1,52 @@ +import { readFile } from 'node:fs/promises'; + +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 readFile(await download.path(), 'utf8')) as { subjectId: string }[]; + + expect(payload.length).toBeGreaterThan(0); + expect([...new Set(payload.map((row) => row.subjectId))]).toStrictEqual([listed]); + }); }); 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..edc30fbf8 100644 --- a/testing/src/support/fixtures.ts +++ b/testing/src/support/fixtures.ts @@ -1,7 +1,8 @@ /* 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'; +import type { APIRequestContext, Page } from '@playwright/test'; import { SettingsPage } from '../pages/_app/admin/settings.page'; import { DashboardPage } from '../pages/_app/dashboard.page'; @@ -31,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 @@ -60,6 +71,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; }; @@ -90,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) => { @@ -111,6 +125,15 @@ 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 injectAuth(page, accessToken, appState); + return group; + }); + }, roleAccount: [ async ({ adminToken, api, apiRequestContext }, use) => { const cache = new Map([