perf(datahub): match export rows against a set of listed subject ids

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(

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<Subject> | undefined;
+  const Capture = (props: { table: TanstackTable.Table<Subject> }) => {
+    table = props.table;
+    return null;
+  };
+  render(
+    <DataTable<Subject>
+      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);
+  });
+});

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<Subject>): Set<string> {
+  return new Set(table.getPrePaginationRowModel().rows.map((row) => removeSubjectIdScope(row.original.id)));
+}

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.
 

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<Download> {
+    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);
   }
 }

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]);
+  });
 });

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<string> {
+    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<void> {
+    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<T>(
     pending: ReturnType<APIRequestContext['post']>,
     status: number,

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 = <TKey extends Extract<keyof PageModels, RouteTo>>(
   key: TKey,
   ...args: NavigateVariadicArgs<TKey>
@@ -60,6 +71,15 @@ type TestFixtures = {
   authenticateAs: (role: Role) => Promise<void>;
   /** 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<Group>;
   /** Short run-unique suffix for naming seeded data in this test. */
   uniqueId: string;
 };
@@ -90,13 +110,7 @@ export const test = base.extend<TestFixtures, WorkerFixtures>({
   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<TestFixtures, WorkerFixtures>({
       }
     );
   },
+  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<Role, { accessToken: string; username: string }>([