From 671152306ad958bb73934099df7970d7481d9afc Mon Sep 17 00:00:00 2001 From: thomasbeaudry Date: Mon, 9 Nov 2026 15:25:21 -0500 Subject: [PATCH 1/7] fix: count every instrument edition on the dashboard The records dialog matched records only against each instrument's latest edition, dropping the records of older editions; it now counts each record under its instrument's name. The Total Instruments card counted every edition in the database; it now counts the latest edition of each instrument the group may administer, the same set its dialog lists. Fixes #1886 Co-Authored-By: Claude Opus 5.5 --- .../__tests__/instruments.service.test.ts | 37 ++++++++++-- .../src/instruments/instruments.service.ts | 19 +++++- .../__tests__/summary.controller.test.ts | 7 ++- .../summary/__tests__/summary.service.test.ts | 21 +++++-- apps/api/src/summary/summary.controller.ts | 6 +- apps/api/src/summary/summary.service.ts | 7 ++- .../routes/_app/__tests__/dashboard.test.tsx | 33 ++++++++++- apps/web/src/routes/_app/dashboard.tsx | 14 ++++- testing/src/pages/_app/dashboard.page.ts | 4 ++ testing/src/specs/dashboard.spec.ts | 58 +++++++++++++++++++ 10 files changed, 180 insertions(+), 26 deletions(-) diff --git a/apps/api/src/instruments/__tests__/instruments.service.test.ts b/apps/api/src/instruments/__tests__/instruments.service.test.ts index 84ae054e80..28ae548db1 100644 --- a/apps/api/src/instruments/__tests__/instruments.service.test.ts +++ b/apps/api/src/instruments/__tests__/instruments.service.test.ts @@ -1244,11 +1244,40 @@ describe('InstrumentsService', () => { }); describe('count', () => { - it('should count every instrument', async () => { - const find = vi.spyOn(instrumentsService, 'find').mockResolvedValue([existingSeries, formInstance('FORM_A', 1)]); + beforeEach(() => { + vi.spyOn(instrumentsService, 'find').mockResolvedValue([ + formInstance('FORM_A', 1), + formInstance('FORM_A', 2), + formInstance('FORM_B', 1) + ]); + instrumentModel.findMany.mockResolvedValue([]); + }); + + it('should count each instrument once whatever its number of editions, as the instruments dialog lists it', async () => { + await expect(instrumentsService.count(adminUser)).resolves.toBe(2); + }); + + it('should count only the instruments the group may administer, so the card matches its dialog', async () => { + groupModel.findFirst.mockResolvedValue({ accessibleInstrumentIds: ['hash:FORM_A-1', 'hash:FORM_A-2'] }); + + await expect(instrumentsService.count(adminUser, 'group-1')).resolves.toBe(1); + }); + + it('should look the group up within what the caller may read, so another group cannot be counted', async () => { + groupModel.findFirst.mockResolvedValue({ accessibleInstrumentIds: [] }); + + await instrumentsService.count(adminUser, 'group-1'); + + expect(groupModel.findFirst).toHaveBeenCalledWith({ + select: { accessibleInstrumentIds: true }, + where: { AND: [accessibleQuery(adminUser.ability, 'read', 'Group')], id: 'group-1' } + }); + }); + + it('should reject a group that cannot be found rather than count nothing', async () => { + groupModel.findFirst.mockResolvedValue(null); - await expect(instrumentsService.count()).resolves.toBe(2); - expect(find).toHaveBeenCalledWith({}, {}); + await expect(instrumentsService.count(adminUser, 'group-1')).rejects.toThrow(NotFoundException); }); }); diff --git a/apps/api/src/instruments/instruments.service.ts b/apps/api/src/instruments/instruments.service.ts index 9e06aa95d4..150f676450 100644 --- a/apps/api/src/instruments/instruments.service.ts +++ b/apps/api/src/instruments/instruments.service.ts @@ -83,8 +83,23 @@ export class InstrumentsService { private readonly virtualizationService: VirtualizationService ) {} - async count(options: EntityOperationOptions = {}): Promise { - return (await this.find({}, options)).length; + /** + * The number of instruments the dashboard lists for the group: the latest edition of each, as + * `findInfo` reports them, narrowed to those the group may administer. + */ + async count(currentUser: RequestUser, requestedGroupId?: string): Promise { + const infos = await this.findInfo({}, currentUser, requestedGroupId); + if (!requestedGroupId) { + return infos.length; + } + const group = await this.groupModel.findFirst({ + select: { accessibleInstrumentIds: true }, + where: { AND: [accessibleQuery(currentUser.ability, 'read', 'Group')], id: requestedGroupId } + }); + if (!group) { + throw new NotFoundException(`Failed to find group with ID: ${requestedGroupId}`); + } + return infos.filter((info) => group.accessibleInstrumentIds.includes(info.id)).length; } async create( diff --git a/apps/api/src/summary/__tests__/summary.controller.test.ts b/apps/api/src/summary/__tests__/summary.controller.test.ts index 5d22db1209..d6c2fb5c3b 100644 --- a/apps/api/src/summary/__tests__/summary.controller.test.ts +++ b/apps/api/src/summary/__tests__/summary.controller.test.ts @@ -1,3 +1,4 @@ +import type { RequestUser } from '@douglasneuroinformatics/libnest'; import { MockFactory } from '@douglasneuroinformatics/libnest/testing'; import type { MockedInstance } from '@douglasneuroinformatics/libnest/testing'; import { Test } from '@nestjs/testing'; @@ -22,11 +23,11 @@ describe('SummaryController', () => { }); it('should summarize the requested group within what the caller may read', async () => { - const ability = createAppAbility([{ action: 'read', subject: 'all' }]); + const currentUser = { ability: createAppAbility([{ action: 'read', subject: 'all' }]) } as RequestUser; const summary = { counts: { instruments: 1, records: 2, sessions: 3, subjects: 4, users: 5 } }; summaryService.getSummary.mockResolvedValue(summary); - await expect(summaryController.getSummary(ability, 'group-1')).resolves.toBe(summary); - expect(summaryService.getSummary).toHaveBeenCalledWith('group-1', { ability }); + await expect(summaryController.getSummary(currentUser, 'group-1')).resolves.toBe(summary); + expect(summaryService.getSummary).toHaveBeenCalledWith(currentUser, 'group-1'); }); }); diff --git a/apps/api/src/summary/__tests__/summary.service.test.ts b/apps/api/src/summary/__tests__/summary.service.test.ts index c15593b4f0..23922addfb 100644 --- a/apps/api/src/summary/__tests__/summary.service.test.ts +++ b/apps/api/src/summary/__tests__/summary.service.test.ts @@ -1,8 +1,10 @@ +import type { RequestUser } from '@douglasneuroinformatics/libnest'; import { MockFactory } from '@douglasneuroinformatics/libnest/testing'; import type { MockedInstance } from '@douglasneuroinformatics/libnest/testing'; import { Test } from '@nestjs/testing'; import { beforeEach, describe, expect, it } from 'vitest'; +import { createAppAbility } from '@/auth/ability.utils'; import { InstrumentRecordsService } from '@/instrument-records/instrument-records.service'; import { InstrumentsService } from '@/instruments/instruments.service'; import { SessionsService } from '@/sessions/sessions.service'; @@ -13,6 +15,8 @@ import { SummaryService } from '../summary.service'; const THIRTY_DAYS_MS = 30 * 24 * 60 * 60 * 1000; +const currentUser = { ability: createAppAbility([{ action: 'read', subject: 'all' }]) } as RequestUser; + describe('SummaryService', () => { let summaryService: SummaryService; let instrumentRecordsService: MockedInstance; @@ -58,12 +62,12 @@ describe('SummaryService', () => { sessionsService.count.mockResolvedValue(3); subjectsService.count.mockResolvedValue(4); usersService.count.mockResolvedValue(5); - const { counts } = await summaryService.getSummary(); + const { counts } = await summaryService.getSummary(currentUser); expect(counts).toStrictEqual({ instruments: 1, records: 2, sessions: 3, subjects: 4, users: 5 }); }); it('should return five trend points per metric, spaced thirty days apart and ordered most recent first', async () => { - const { trends } = await summaryService.getSummary(); + const { trends } = await summaryService.getSummary(currentUser); for (const trend of [trends.records, trends.sessions, trends.subjects]) { expect(trend).toHaveLength(5); for (let i = 1; i < trend.length; i++) { @@ -78,14 +82,14 @@ describe('SummaryService', () => { sessionsService.count.mockImplementation((where: { date?: { lte: Date } }) => { return Promise.resolve(where.date ? where.date.lte.getTime() : 0); }); - const { trends } = await summaryService.getSummary(); + const { trends } = await summaryService.getSummary(currentUser); for (const point of trends.sessions) { expect(point.value).toBe(point.timestamp); } }); it('should use a single clock reading for every metric', async () => { - const { trends } = await summaryService.getSummary(); + const { trends } = await summaryService.getSummary(currentUser); expect(trends.sessions.map((point) => point.timestamp)).toStrictEqual( trends.records.map((point) => point.timestamp) ); @@ -95,7 +99,7 @@ describe('SummaryService', () => { }); it('should scope every query to the group when a groupId is provided', async () => { - await summaryService.getSummary('group-1'); + await summaryService.getSummary(currentUser, 'group-1'); expect(instrumentRecordsService.count).toHaveBeenCalledWith( expect.objectContaining({ groupId: 'group-1' }), expect.anything() @@ -112,6 +116,11 @@ describe('SummaryService', () => { } }); + it('should count instruments with the same edition rule and group scope as the instruments dialog', async () => { + await summaryService.getSummary(currentUser, 'group-1'); + expect(instrumentsService.count).toHaveBeenCalledWith(currentUser, 'group-1'); + }); + it('should issue every query concurrently rather than awaiting them in sequence', async () => { // Hold every count open until all of them have been called. If any query waits on the result // of another, the expected number is never reached and this test times out. @@ -136,7 +145,7 @@ describe('SummaryService', () => { return 0; }); } - await summaryService.getSummary(); + await summaryService.getSummary(currentUser); expect(started).toBe(EXPECTED_QUERIES); }); }); diff --git a/apps/api/src/summary/summary.controller.ts b/apps/api/src/summary/summary.controller.ts index 6804c6c059..8f5fe7af9c 100644 --- a/apps/api/src/summary/summary.controller.ts +++ b/apps/api/src/summary/summary.controller.ts @@ -1,8 +1,8 @@ import { CurrentUser } from '@douglasneuroinformatics/libnest'; +import type { RequestUser } from '@douglasneuroinformatics/libnest'; import { Controller, Get, Query } from '@nestjs/common'; import type { Summary } from '@opendatacapture/schemas/summary'; -import type { AppAbility } from '@/auth/auth.types'; import { RouteAccess } from '@/core/decorators/route-access.decorator'; import { SummaryService } from './summary.service'; @@ -19,7 +19,7 @@ export class SummaryController { { action: 'read', subject: 'Subject' }, { action: 'read', subject: 'User' } ]) - async getSummary(@CurrentUser('ability') ability: AppAbility, @Query('groupId') groupId?: string): Promise { - return this.summaryService.getSummary(groupId, { ability }); + async getSummary(@CurrentUser() currentUser: RequestUser, @Query('groupId') groupId?: string): Promise { + return this.summaryService.getSummary(currentUser, groupId); } } diff --git a/apps/api/src/summary/summary.service.ts b/apps/api/src/summary/summary.service.ts index d2d0f91e74..bd0aef2356 100644 --- a/apps/api/src/summary/summary.service.ts +++ b/apps/api/src/summary/summary.service.ts @@ -1,7 +1,7 @@ +import type { RequestUser } from '@douglasneuroinformatics/libnest'; import { Injectable } from '@nestjs/common'; import type { Summary } from '@opendatacapture/schemas/summary'; -import type { EntityOperationOptions } from '@/core/types'; import { InstrumentRecordsService } from '@/instrument-records/instrument-records.service'; import { InstrumentsService } from '@/instruments/instruments.service'; import { SessionsService } from '@/sessions/sessions.service'; @@ -21,7 +21,8 @@ export class SummaryService { private readonly subjectsService: SubjectsService ) {} - async getSummary(groupId?: string, { ability }: EntityOperationOptions = {}): Promise { + async getSummary(currentUser: RequestUser, groupId?: string): Promise { + const { ability } = currentUser; const filter = groupId ? { groupIds: { has: groupId } } : ({} as { [key: string]: unknown }); const sessionFilter = groupId ? { groupId } : {}; const recordFilter = groupId ? { groupId } : {}; @@ -39,7 +40,7 @@ export class SummaryService { // the slowest query rather than the sum of all of them. const [instruments, records, sessions, subjects, users, recordTrend, sessionTrend, subjectTrend] = await Promise.all([ - this.instrumentsService.count(), + this.instrumentsService.count(currentUser, groupId), this.instrumentRecordsService.count(recordFilter, { ability }), this.sessionsService.count(sessionFilter, { ability }), this.subjectsService.count(filter, { ability }), diff --git a/apps/web/src/routes/_app/__tests__/dashboard.test.tsx b/apps/web/src/routes/_app/__tests__/dashboard.test.tsx index 979e2c8a7d..46742c290b 100644 --- a/apps/web/src/routes/_app/__tests__/dashboard.test.tsx +++ b/apps/web/src/routes/_app/__tests__/dashboard.test.tsx @@ -40,13 +40,20 @@ const SUMMARY: Summary = { }; const INSTRUMENTS = [ - { details: { title: 'Alpha Scale' }, id: 'alpha', kind: 'FORM' }, - { details: { title: 'Beta Task' }, id: 'beta', kind: 'INTERACTIVE' } + { details: { title: 'Alpha Scale' }, id: 'alpha', internal: { edition: 2, name: 'ALPHA' }, kind: 'FORM' }, + { details: { title: 'Beta Task' }, id: 'beta', internal: { edition: 1, name: 'BETA' }, kind: 'INTERACTIVE' } +]; + +const ALL_EDITIONS = [ + { details: { title: 'Alpha Scale' }, id: 'alpha-1', internal: { edition: 1, name: 'ALPHA' }, kind: 'FORM' }, + ...INSTRUMENTS, + { details: { title: 'Gamma Series' }, id: 'gamma', kind: 'SERIES' } ]; const RESEARCH_GROUP: CurrentGroup = { accessibleInstrumentIds: ['alpha'], id: 'group-1', type: 'RESEARCH' }; type DashboardMocks = { + allEditions: QueryResult; ensureQueryData: Mock<(options: unknown) => Promise>; instruments: QueryResult; navigate: Mock<(options: unknown) => void>; @@ -64,6 +71,7 @@ type DashboardMocks = { }; const mocks = vi.hoisted((): DashboardMocks => ({ + allEditions: {}, ensureQueryData: vi.fn(), instruments: {}, navigate: vi.fn(), @@ -93,7 +101,11 @@ vi.mock('@/hooks/useSummaryQuery', async (importOriginal) => ({ return mocks.summary; } })); -vi.mock('@/hooks/useInstrumentInfoQuery', () => ({ useInstrumentInfoQuery: () => mocks.instruments })); +vi.mock('@/hooks/useInstrumentInfoQuery', () => ({ + useInstrumentInfoQuery: (options?: { params?: { allEditions?: boolean } }) => { + return options?.params?.allEditions ? mocks.allEditions : mocks.instruments; + } +})); vi.mock('@/hooks/useInstrumentRecords', () => ({ useInstrumentRecords: (options: unknown) => { mocks.useInstrumentRecords(options); @@ -188,6 +200,7 @@ describe('Route', () => { mocks.store.currentGroup = null; mocks.summary = { data: SUMMARY }; mocks.instruments = { data: INSTRUMENTS }; + mocks.allEditions = { data: ALL_EDITIONS }; mocks.records = { data: [{ instrumentId: 'alpha' }, { instrumentId: 'alpha' }, { instrumentId: 'beta' }] }; mocks.users = { data: [{ username: 'alice' }, { username: 'bob' }] }; mocks.tooltipProps = { active: false }; @@ -278,6 +291,20 @@ describe('Route', () => { expect(listItems(dialog)).toEqual(['Alpha Scale 2', 'Beta Task 1']); }); + it('should count the records of every edition, so older editions are not dropped from the dialog', () => { + mocks.records = { data: [{ instrumentId: 'alpha-1' }, { instrumentId: 'alpha-1' }, { instrumentId: 'alpha' }] }; + renderPage(); + const dialog = openDialog('statistic-records', 'dashboard-record-Modal-dialog'); + expect(listItems(dialog)).toEqual(['Alpha Scale 3', 'Beta Task 0']); + }); + + it('should count each record under its own edition while the other editions are unavailable', () => { + mocks.allEditions = { data: undefined }; + renderPage(); + const dialog = openDialog('statistic-records', 'dashboard-record-Modal-dialog'); + expect(listItems(dialog)).toEqual(['Alpha Scale 2', 'Beta Task 1']); + }); + it('should count zero records for each instrument while records are unavailable', () => { mocks.records = { data: undefined }; renderPage(); diff --git a/apps/web/src/routes/_app/dashboard.tsx b/apps/web/src/routes/_app/dashboard.tsx index 3d9253b9e4..c5b8635f72 100644 --- a/apps/web/src/routes/_app/dashboard.tsx +++ b/apps/web/src/routes/_app/dashboard.tsx @@ -26,6 +26,7 @@ const RouteComponent = () => { const [isUserModalOpen, setIsUserModalOpen] = useState(false); const [isRecordModalOpen, setIsRecordModalOpen] = useState(false); const instrumentInfoQuery = useInstrumentInfoQuery(); + const allEditionsQuery = useInstrumentInfoQuery({ params: { allEditions: true } }); const userInfoQuery = useUsersQuery({ params: { groupId: currentGroup?.id @@ -53,12 +54,21 @@ const RouteComponent = () => { }; }); - const recordIds = recordsQuery.data?.map((record) => record.instrumentId); + // A record keeps the id of the edition it was collected with, so records are counted under the + // instrument's name to include every edition in the row for its latest one. + const instrumentKeyById = new Map( + allEditionsQuery.data?.map((instrument) => { + return [instrument.id, instrument.kind === 'SERIES' ? instrument.id : instrument.internal.name]; + }) + ); + const instrumentKey = (instrumentId: string) => instrumentKeyById.get(instrumentId) ?? instrumentId; + + const recordKeys = recordsQuery.data?.map((record) => instrumentKey(record.instrumentId)); const recordCounter = instrumentInfo?.map((title) => { return { - count: recordIds?.filter((val) => val === title.id).length ?? 0, + count: recordKeys?.filter((key) => key === instrumentKey(title.id)).length ?? 0, id: title.id, instrumentTitle: title.title }; diff --git a/testing/src/pages/_app/dashboard.page.ts b/testing/src/pages/_app/dashboard.page.ts index 797172dc09..d8e43bee30 100644 --- a/testing/src/pages/_app/dashboard.page.ts +++ b/testing/src/pages/_app/dashboard.page.ts @@ -3,7 +3,9 @@ import type { Locator, Page } from '@playwright/test'; import { AppPage } from './route.page'; export class DashboardPage extends AppPage { + readonly instrumentsDialog: Locator; readonly pageHeader: Locator; + readonly recordsDialog: Locator; readonly recordsSessionsChart: Locator; readonly statisticInstruments: Locator; readonly statisticRecords: Locator; @@ -24,5 +26,7 @@ export class DashboardPage extends AppPage { this.recordsSessionsChart = page.getByTestId('dashboard-chart-records-sessions'); this.subjectsGrowthChart = page.getByTestId('dashboard-chart-subjects-growth'); this.usersDialog = page.getByTestId('dashboard-users-Modal-dialog'); + this.instrumentsDialog = page.getByTestId('dashboard-instruments-Modal-dialog'); + this.recordsDialog = page.getByTestId('dashboard-record-Modal-dialog'); } } diff --git a/testing/src/specs/dashboard.spec.ts b/testing/src/specs/dashboard.spec.ts index dca1c56de9..16e71a129e 100644 --- a/testing/src/specs/dashboard.spec.ts +++ b/testing/src/specs/dashboard.spec.ts @@ -1,5 +1,24 @@ +import { DashboardPage } from '../pages/_app/dashboard.page'; import { expect, test } from '../support/fixtures'; +function scoreFormBundle({ edition, name, title }: { edition: number; name: string; title: string }) { + return `(async () => { + const runtimeImport = (specifier) => import(globalThis.__resolveImport?.(specifier) ?? specifier); + const { z } = await runtimeImport('/runtime/v1/zod@3.x/index.js'); + return { + __runtimeVersion: 1, + content: { score: { kind: 'number', label: 'Score', variant: 'input' } }, + details: { description: 'A form with one number field', license: 'Apache-2.0', title: ${JSON.stringify(title)} }, + internal: { edition: ${edition}, name: ${JSON.stringify(name)} }, + kind: 'FORM', + language: 'en', + measures: null, + tags: [], + validationSchema: z.object({ score: z.number() }) + }; + })()`; +} + test.describe('dashboard', () => { test('should display the dashboard header @smoke', async ({ getPageModel }) => { const dashboardPage = await getPageModel('/dashboard'); @@ -46,6 +65,45 @@ test.describe('dashboard', () => { await expect(dashboardPage.usersDialog).toContainText('Users'); }); + // A record keeps the id of the edition it was collected with, so the records dialog once counted + // only the latest edition's records, and the instruments card counted every edition in the database. + // Edition 2 is stored after the group exists, so the group gains it the way an upgrade does. + test('should count the records of every edition of an instrument, matching both cards to their dialogs', async ({ + api, + authenticateAs, + page, + uniqueId + }) => { + const name = `DASHBOARD_EDITIONS_${uniqueId}`; + const title = `Dashboard Editions ${uniqueId}`; + const editionOneId = await api.createInstrument(scoreFormBundle({ edition: 1, name, title })); + const group = await api.createGroup(); + const editionTwoId = await api.createInstrument(scoreFormBundle({ edition: 2, name, title })); + const session = await api.createSession(group.id, { id: `dashboard_editions_${uniqueId}` }); + for (const instrumentId of [editionOneId, editionOneId, editionOneId, editionTwoId, editionTwoId]) { + await api.createRecord(group.id, instrumentId, session, { score: 1 }); + } + const { credentials } = await api.createUser({ basePermissionLevel: 'GROUP_MANAGER', groupIds: [group.id] }); + await authenticateAs(credentials); + + const dashboardPage = new DashboardPage(page); + await dashboardPage.goto('/dashboard'); + + await expect(dashboardPage.statisticRecords.getByRole('heading')).toHaveText('5'); + await dashboardPage.statisticRecords.click(); + await expect(dashboardPage.recordsDialog.getByRole('listitem').filter({ hasText: title })).toHaveText( + new RegExp(`${title}\\s*5$`) + ); + await page.keyboard.press('Escape'); + + await dashboardPage.statisticInstruments.click(); + const instrumentRows = dashboardPage.instrumentsDialog.getByRole('listitem'); + await expect(instrumentRows.filter({ hasText: title })).toHaveCount(1); + const instrumentRowCount = await instrumentRows.count(); + await page.keyboard.press('Escape'); + await expect(dashboardPage.statisticInstruments.getByRole('heading')).toHaveText(String(instrumentRowCount)); + }); + test.describe('as an administrator', () => { test.use({ actingRole: 'ADMIN' }); From 8fd2d6aa6e52b390bbdfe1c32568c0b8c3bbf9f4 Mon Sep 17 00:00:00 2001 From: thomasbeaudry Date: Mon, 9 Nov 2026 16:05:57 -0500 Subject: [PATCH 2/7] feat(web): search and sort the dashboard records dialog The Number of Records dialog is now a DataTable: a search bar, sortable Title and Number columns, and the instruments with the most records first. Co-Authored-By: Claude Opus 5.5 --- .../routes/_app/__tests__/dashboard.test.tsx | 60 +++++++++++++- apps/web/src/routes/_app/dashboard.tsx | 80 +++++++++---------- testing/src/specs/dashboard.spec.ts | 9 ++- 3 files changed, 99 insertions(+), 50 deletions(-) diff --git a/apps/web/src/routes/_app/__tests__/dashboard.test.tsx b/apps/web/src/routes/_app/__tests__/dashboard.test.tsx index 46742c290b..8fea27744a 100644 --- a/apps/web/src/routes/_app/__tests__/dashboard.test.tsx +++ b/apps/web/src/routes/_app/__tests__/dashboard.test.tsx @@ -182,6 +182,16 @@ const listItems = (dialog: HTMLElement) => { .map((item) => item.textContent); }; +const recordRows = (dialog: HTMLElement) => { + return within(dialog) + .getAllByTestId('data-table-row') + .map((row) => Array.from(row.children, (cell) => cell.textContent)); +}; + +const recordsHeader = (dialog: HTMLElement, label: string) => { + return within(within(dialog).getByTestId('data-table-head')).getByText(label).closest('button')!; +}; + const runLoader = async () => { const loader = Route.options.loader as (opts: object) => Promise; try { @@ -288,28 +298,70 @@ describe('Route', () => { it('should count the records of each instrument in the records dialog', () => { renderPage(); const dialog = openDialog('statistic-records', 'dashboard-record-Modal-dialog'); - expect(listItems(dialog)).toEqual(['Alpha Scale 2', 'Beta Task 1']); + expect(recordRows(dialog)).toEqual([ + ['Alpha Scale', '2'], + ['Beta Task', '1'] + ]); + }); + + it('should list the instrument with the most records first, so the busiest are seen without sorting', () => { + mocks.records = { data: [{ instrumentId: 'beta' }, { instrumentId: 'beta' }, { instrumentId: 'alpha' }] }; + renderPage(); + const dialog = openDialog('statistic-records', 'dashboard-record-Modal-dialog'); + expect(recordRows(dialog).map(([title]) => title)).toEqual(['Beta Task', 'Alpha Scale']); + }); + + it('should sort the records dialog by title when its header is clicked', () => { + mocks.records = { data: [{ instrumentId: 'beta' }, { instrumentId: 'beta' }, { instrumentId: 'alpha' }] }; + renderPage(); + const dialog = openDialog('statistic-records', 'dashboard-record-Modal-dialog'); + fireEvent.click(recordsHeader(dialog, 'Title')); + expect(recordRows(dialog).map(([title]) => title)).toEqual(['Alpha Scale', 'Beta Task']); + }); + + it('should reverse the record count order when the number header is clicked', () => { + renderPage(); + const dialog = openDialog('statistic-records', 'dashboard-record-Modal-dialog'); + fireEvent.click(recordsHeader(dialog, 'Number')); + expect(recordRows(dialog).map(([title]) => title)).toEqual(['Beta Task', 'Alpha Scale']); + }); + + it('should narrow the records dialog to the instruments matching its search', () => { + renderPage(); + const dialog = openDialog('statistic-records', 'dashboard-record-Modal-dialog'); + const search = within(within(dialog).getByTestId('data-table-search-bar')).getByRole('searchbox'); + fireEvent.change(search, { target: { value: 'Beta' } }); + expect(recordRows(dialog)).toEqual([['Beta Task', '1']]); }); it('should count the records of every edition, so older editions are not dropped from the dialog', () => { mocks.records = { data: [{ instrumentId: 'alpha-1' }, { instrumentId: 'alpha-1' }, { instrumentId: 'alpha' }] }; renderPage(); const dialog = openDialog('statistic-records', 'dashboard-record-Modal-dialog'); - expect(listItems(dialog)).toEqual(['Alpha Scale 3', 'Beta Task 0']); + expect(recordRows(dialog)).toEqual([ + ['Alpha Scale', '3'], + ['Beta Task', '0'] + ]); }); it('should count each record under its own edition while the other editions are unavailable', () => { mocks.allEditions = { data: undefined }; renderPage(); const dialog = openDialog('statistic-records', 'dashboard-record-Modal-dialog'); - expect(listItems(dialog)).toEqual(['Alpha Scale 2', 'Beta Task 1']); + expect(recordRows(dialog)).toEqual([ + ['Alpha Scale', '2'], + ['Beta Task', '1'] + ]); }); it('should count zero records for each instrument while records are unavailable', () => { mocks.records = { data: undefined }; renderPage(); const dialog = openDialog('statistic-records', 'dashboard-record-Modal-dialog'); - expect(listItems(dialog)).toEqual(['Alpha Scale 0', 'Beta Task 0']); + expect(recordRows(dialog)).toEqual([ + ['Alpha Scale', '0'], + ['Beta Task', '0'] + ]); }); it('should report a failure to load instruments in the records dialog, since records are counted per instrument', () => { diff --git a/apps/web/src/routes/_app/dashboard.tsx b/apps/web/src/routes/_app/dashboard.tsx index c5b8635f72..e08e88cbdb 100644 --- a/apps/web/src/routes/_app/dashboard.tsx +++ b/apps/web/src/routes/_app/dashboard.tsx @@ -1,6 +1,6 @@ import React, { useState } from 'react'; -import { Dialog, Heading, Spinner, StatisticCard } from '@douglasneuroinformatics/libui/components'; +import { DataTable, Dialog, Heading, Spinner, StatisticCard } from '@douglasneuroinformatics/libui/components'; import { useTheme, useTranslation } from '@douglasneuroinformatics/libui/hooks'; import type { Theme } from '@douglasneuroinformatics/libui/hooks'; import { ClipboardDocumentIcon, DocumentTextIcon, UserIcon, UsersIcon } from '@heroicons/react/24/solid'; @@ -10,6 +10,8 @@ import { AnimatePresence, motion } from 'motion/react'; import { Area, AreaChart, CartesianGrid, Legend, ResponsiveContainer, Tooltip, XAxis, YAxis } from 'recharts'; import { PageHeader } from '@/components/PageHeader'; +import { SortableHeader } from '@/components/SortableHeader'; +import { TruncatedCell } from '@/components/TruncatedCell'; import { useInstrumentInfoQuery } from '@/hooks/useInstrumentInfoQuery'; import { useInstrumentRecords } from '@/hooks/useInstrumentRecords'; import { summaryQueryOptions, useSummaryQuery } from '@/hooks/useSummaryQuery'; @@ -365,48 +367,40 @@ const RouteComponent = () => { })} -
    - {!instrumentInfo && ( -

    - {t({ - en: 'Error finding records', - es: 'Error al buscar los registros', - fr: "Erreur lors de la recherche d'enregistrements" - })} -

    - )} - {instrumentInfo && ( - -
    -

    - {t({ - en: 'Title', - es: 'Título', - fr: 'Titre' - })} -

    {' '} -

    {t({ en: 'Number', es: 'Número', fr: 'Nombre' })}

    -
    -
    - {recordCounter.map((instrument, i) => { - return ( - -
    -

    {instrument.instrumentTitle}

    {instrument.count}

    -
    -     - ); - })} -     - )} -
+ {instrumentInfo ? ( + ( + ()} value={getValue()} /> + ), + header: ({ column }) => ( + + ), + id: 'instrumentTitle' + }, + { + accessorKey: 'count', + header: ({ column }) => ( + + ), + id: 'count', + size: 110 + } + ]} + data={recordCounter} + initialState={{ columnPinning: { right: ['count'] }, sorting: [{ desc: true, id: 'count' }] }} + /> + ) : ( +

+ {t({ + en: 'Error finding records', + es: 'Error al buscar los registros', + fr: "Erreur lors de la recherche d'enregistrements" + })} +

+ )} diff --git a/testing/src/specs/dashboard.spec.ts b/testing/src/specs/dashboard.spec.ts index 16e71a129e..9afb724030 100644 --- a/testing/src/specs/dashboard.spec.ts +++ b/testing/src/specs/dashboard.spec.ts @@ -91,9 +91,12 @@ test.describe('dashboard', () => { await expect(dashboardPage.statisticRecords.getByRole('heading')).toHaveText('5'); await dashboardPage.statisticRecords.click(); - await expect(dashboardPage.recordsDialog.getByRole('listitem').filter({ hasText: title })).toHaveText( - new RegExp(`${title}\\s*5$`) - ); + // The only instrument holding records in this group, so the default most-records-first order puts it on top. + const recordRows = dashboardPage.recordsDialog.getByTestId('data-table-row'); + await expect(recordRows.first()).toHaveText(new RegExp(`^${title}\\s*5$`)); + await dashboardPage.recordsDialog.getByRole('searchbox').fill(title); + await expect(recordRows).toHaveCount(1); + await expect(recordRows).toHaveText(new RegExp(`^${title}\\s*5$`)); await page.keyboard.press('Escape'); await dashboardPage.statisticInstruments.click(); From 8124e2e4c416e07613ddb0a18b1b988e00dc9715 Mon Sep 17 00:00:00 2001 From: thomasbeaudry Date: Mon, 9 Nov 2026 16:18:40 -0500 Subject: [PATCH 3/7] feat(web): search and sort every dashboard dialog as a plain-text table The Users and Available Instruments dialogs become searchable, sortable tables like the records dialog, with each instrument kind shown as its colour tag. All three dialogs show their tables as plain text, without borders, under larger bold column headers. Co-Authored-By: Claude Opus 5.5 --- .../routes/_app/__tests__/dashboard.test.tsx | 88 ++++++-- apps/web/src/routes/_app/dashboard.tsx | 205 +++++++++--------- testing/src/pages/_app/dashboard.page.ts | 16 ++ testing/src/specs/dashboard.spec.ts | 49 ++++- 4 files changed, 232 insertions(+), 126 deletions(-) diff --git a/apps/web/src/routes/_app/__tests__/dashboard.test.tsx b/apps/web/src/routes/_app/__tests__/dashboard.test.tsx index 8fea27744a..fdc98e3267 100644 --- a/apps/web/src/routes/_app/__tests__/dashboard.test.tsx +++ b/apps/web/src/routes/_app/__tests__/dashboard.test.tsx @@ -112,6 +112,7 @@ vi.mock('@/hooks/useInstrumentRecords', () => ({ return mocks.records; } })); +vi.mock('@/hooks/useSetupStateQuery', () => ({ useSetupStateQuery: () => ({ data: { tableColorMode: 'tags' } }) })); vi.mock('@/hooks/useUsersQuery', () => ({ useUsersQuery: (options: unknown) => { mocks.useUsersQuery(options); @@ -176,19 +177,18 @@ const openDialog = (statisticTestId: string, dialogTestId: string) => { return screen.getByTestId(dialogTestId); }; -const listItems = (dialog: HTMLElement) => { - return within(dialog) - .getAllByRole('listitem') - .map((item) => item.textContent); -}; - -const recordRows = (dialog: HTMLElement) => { +const tableRows = (dialog: HTMLElement) => { return within(dialog) .getAllByTestId('data-table-row') .map((row) => Array.from(row.children, (cell) => cell.textContent)); }; -const recordsHeader = (dialog: HTMLElement, label: string) => { +const searchDialogTable = (dialog: HTMLElement, value: string) => { + const search = within(within(dialog).getByTestId('data-table-search-bar')).getByRole('searchbox'); + fireEvent.change(search, { target: { value } }); +}; + +const tableHeader = (dialog: HTMLElement, label: string) => { return within(within(dialog).getByTestId('data-table-head')).getByText(label).closest('button')!; }; @@ -258,7 +258,22 @@ describe('Route', () => { it('should list every user of the group in the users dialog', () => { renderPage(); const dialog = openDialog('statistic-users', 'dashboard-users-Modal-dialog'); - expect(listItems(dialog)).toEqual(['alice', 'bob']); + expect(tableRows(dialog)).toEqual([['alice'], ['bob']]); + }); + + it('should narrow the users dialog to the usernames matching its search', () => { + renderPage(); + const dialog = openDialog('statistic-users', 'dashboard-users-Modal-dialog'); + searchDialogTable(dialog, 'bo'); + expect(tableRows(dialog)).toEqual([['bob']]); + }); + + it('should sort the users dialog by username when its header is clicked', () => { + renderPage(); + const dialog = openDialog('statistic-users', 'dashboard-users-Modal-dialog'); + fireEvent.click(tableHeader(dialog, 'Username')); + fireEvent.click(tableHeader(dialog, 'Username')); + expect(tableRows(dialog)).toEqual([['bob'], ['alice']]); }); it('should show a spinner in the users dialog while users are loading', () => { @@ -279,13 +294,41 @@ describe('Route', () => { mocks.store.currentGroup = RESEARCH_GROUP; renderPage(); const dialog = openDialog('statistic-instruments', 'dashboard-instruments-Modal-dialog'); - expect(listItems(dialog)).toEqual(['Alpha Scale FORM']); + expect(tableRows(dialog)).toEqual([['Alpha Scale', 'Form']]); }); it('should list every instrument when no group is selected', () => { renderPage(); const dialog = openDialog('statistic-instruments', 'dashboard-instruments-Modal-dialog'); - expect(listItems(dialog)).toEqual(['Alpha Scale FORM', 'Beta Task INTERACTIVE']); + expect(tableRows(dialog)).toEqual([ + ['Alpha Scale', 'Form'], + ['Beta Task', 'Interactive'] + ]); + }); + + it('should tag each instrument kind with the badge the instrument tables use', () => { + renderPage(); + const dialog = openDialog('statistic-instruments', 'dashboard-instruments-Modal-dialog'); + expect( + within(dialog) + .getAllByTestId('instrument-cell-kind') + .map((tag) => tag.textContent) + ).toEqual(['Form', 'Interactive']); + }); + + it('should narrow the instruments dialog to the titles matching its search', () => { + renderPage(); + const dialog = openDialog('statistic-instruments', 'dashboard-instruments-Modal-dialog'); + searchDialogTable(dialog, 'Beta'); + expect(tableRows(dialog)).toEqual([['Beta Task', 'Interactive']]); + }); + + it('should sort the instruments dialog by kind when its header is clicked', () => { + renderPage(); + const dialog = openDialog('statistic-instruments', 'dashboard-instruments-Modal-dialog'); + fireEvent.click(tableHeader(dialog, 'Kind')); + fireEvent.click(tableHeader(dialog, 'Kind')); + expect(tableRows(dialog).map(([title]) => title)).toEqual(['Beta Task', 'Alpha Scale']); }); it('should report a failure to load instruments in the instruments dialog', () => { @@ -298,7 +341,7 @@ describe('Route', () => { it('should count the records of each instrument in the records dialog', () => { renderPage(); const dialog = openDialog('statistic-records', 'dashboard-record-Modal-dialog'); - expect(recordRows(dialog)).toEqual([ + expect(tableRows(dialog)).toEqual([ ['Alpha Scale', '2'], ['Beta Task', '1'] ]); @@ -308,37 +351,36 @@ describe('Route', () => { mocks.records = { data: [{ instrumentId: 'beta' }, { instrumentId: 'beta' }, { instrumentId: 'alpha' }] }; renderPage(); const dialog = openDialog('statistic-records', 'dashboard-record-Modal-dialog'); - expect(recordRows(dialog).map(([title]) => title)).toEqual(['Beta Task', 'Alpha Scale']); + expect(tableRows(dialog).map(([title]) => title)).toEqual(['Beta Task', 'Alpha Scale']); }); it('should sort the records dialog by title when its header is clicked', () => { mocks.records = { data: [{ instrumentId: 'beta' }, { instrumentId: 'beta' }, { instrumentId: 'alpha' }] }; renderPage(); const dialog = openDialog('statistic-records', 'dashboard-record-Modal-dialog'); - fireEvent.click(recordsHeader(dialog, 'Title')); - expect(recordRows(dialog).map(([title]) => title)).toEqual(['Alpha Scale', 'Beta Task']); + fireEvent.click(tableHeader(dialog, 'Title')); + expect(tableRows(dialog).map(([title]) => title)).toEqual(['Alpha Scale', 'Beta Task']); }); it('should reverse the record count order when the number header is clicked', () => { renderPage(); const dialog = openDialog('statistic-records', 'dashboard-record-Modal-dialog'); - fireEvent.click(recordsHeader(dialog, 'Number')); - expect(recordRows(dialog).map(([title]) => title)).toEqual(['Beta Task', 'Alpha Scale']); + fireEvent.click(tableHeader(dialog, 'Number')); + expect(tableRows(dialog).map(([title]) => title)).toEqual(['Beta Task', 'Alpha Scale']); }); it('should narrow the records dialog to the instruments matching its search', () => { renderPage(); const dialog = openDialog('statistic-records', 'dashboard-record-Modal-dialog'); - const search = within(within(dialog).getByTestId('data-table-search-bar')).getByRole('searchbox'); - fireEvent.change(search, { target: { value: 'Beta' } }); - expect(recordRows(dialog)).toEqual([['Beta Task', '1']]); + searchDialogTable(dialog, 'Beta'); + expect(tableRows(dialog)).toEqual([['Beta Task', '1']]); }); it('should count the records of every edition, so older editions are not dropped from the dialog', () => { mocks.records = { data: [{ instrumentId: 'alpha-1' }, { instrumentId: 'alpha-1' }, { instrumentId: 'alpha' }] }; renderPage(); const dialog = openDialog('statistic-records', 'dashboard-record-Modal-dialog'); - expect(recordRows(dialog)).toEqual([ + expect(tableRows(dialog)).toEqual([ ['Alpha Scale', '3'], ['Beta Task', '0'] ]); @@ -348,7 +390,7 @@ describe('Route', () => { mocks.allEditions = { data: undefined }; renderPage(); const dialog = openDialog('statistic-records', 'dashboard-record-Modal-dialog'); - expect(recordRows(dialog)).toEqual([ + expect(tableRows(dialog)).toEqual([ ['Alpha Scale', '2'], ['Beta Task', '1'] ]); @@ -358,7 +400,7 @@ describe('Route', () => { mocks.records = { data: undefined }; renderPage(); const dialog = openDialog('statistic-records', 'dashboard-record-Modal-dialog'); - expect(recordRows(dialog)).toEqual([ + expect(tableRows(dialog)).toEqual([ ['Alpha Scale', '0'], ['Beta Task', '0'] ]); diff --git a/apps/web/src/routes/_app/dashboard.tsx b/apps/web/src/routes/_app/dashboard.tsx index e08e88cbdb..b97984b2d4 100644 --- a/apps/web/src/routes/_app/dashboard.tsx +++ b/apps/web/src/routes/_app/dashboard.tsx @@ -3,12 +3,14 @@ import React, { useState } from 'react'; import { DataTable, Dialog, Heading, Spinner, StatisticCard } from '@douglasneuroinformatics/libui/components'; import { useTheme, useTranslation } from '@douglasneuroinformatics/libui/hooks'; import type { Theme } from '@douglasneuroinformatics/libui/hooks'; +import { cn } from '@douglasneuroinformatics/libui/utils'; import { ClipboardDocumentIcon, DocumentTextIcon, UserIcon, UsersIcon } from '@heroicons/react/24/solid'; +import type { InstrumentKind } from '@opendatacapture/runtime-core'; import type { AppSubjectName } from '@opendatacapture/schemas/core'; import { createFileRoute, redirect, useNavigate } from '@tanstack/react-router'; -import { AnimatePresence, motion } from 'motion/react'; import { Area, AreaChart, CartesianGrid, Legend, ResponsiveContainer, Tooltip, XAxis, YAxis } from 'recharts'; +import { InstrumentKindCell } from '@/components/InstrumentKindCell'; import { PageHeader } from '@/components/PageHeader'; import { SortableHeader } from '@/components/SortableHeader'; import { TruncatedCell } from '@/components/TruncatedCell'; @@ -18,6 +20,13 @@ import { summaryQueryOptions, useSummaryQuery } from '@/hooks/useSummaryQuery'; import { useUsersQuery } from '@/hooks/useUsersQuery'; import { useAppStore } from '@/store'; +// A dialog's table is shown as plain text: libui's frame, row and cell borders are removed, and the +// column headers are set larger and bold to stand apart from the rows without them. +const TEXT_ONLY_TABLE = cn( + '[&_[data-testid=data-table]>.border]:border-0 [&_[data-testid=data-table]>.border_*]:border-0', + '[&_[data-testid=data-table-head]]:text-base [&_[data-testid=data-table-head]]:font-bold' +); + const RouteComponent = () => { const currentGroup = useAppStore((store) => store.currentGroup); const { t } = useTranslation(); @@ -204,39 +213,38 @@ const RouteComponent = () => { })} -
-
    - {userInfoQuery.isLoading && } - {userInfoQuery.isError && ( -

    - {t({ - en: 'Error finding users', - es: 'Error al buscar los usuarios', - fr: "Erreur lors de la recherche d'utilisateurs" - })} -

    - )} - {userInfoQuery.data && ( - - {userInfoQuery.data.map((user, i) => { - return ( - -
    -

    {user.username}

    -
    -     - ); - })} -     - )} -
+ {userInfoQuery.isLoading && } + {userInfoQuery.isError && ( +

+ {t({ + en: 'Error finding users', + es: 'Error al buscar los usuarios', + fr: "Erreur lors de la recherche d'utilisateurs" + })} +

+ )} + {userInfoQuery.data && ( +
+ ( + ()} value={getValue()} /> + ), + header: ({ column }) => ( + + ), + id: 'username' + } + ]} + data={userInfoQuery.data} + /> +
+ )} @@ -294,48 +302,43 @@ const RouteComponent = () => { })} -
    - {!instrumentInfo && ( -

    - {t({ - en: 'Error finding instruments', - es: 'Error al buscar los instrumentos', - fr: 'Erreur lors de la recherche des instruments' - })} -

    - )} - {instrumentInfo && ( - -
    -

    - {t({ - en: 'Title', - es: 'Título', - fr: 'Titre' - })} -

    {' '} -

    {t({ en: 'Kind', es: 'Tipo', fr: 'Type' })}

    -
    -
    - {instrumentInfo.map((instrument, i) => { - return ( - -
    -

    {instrument.title}

    {instrument.kind}

    -
    -     - ); - })} -     - )} -
+ {instrumentInfo ? ( +
+ ( + ()} value={getValue()} /> + ), + header: ({ column }) => ( + + ), + id: 'title' + }, + { + accessorKey: 'kind', + cell: ({ getValue }) => ()} />, + header: ({ column }) => ( + + ), + id: 'kind', + size: 140 + } + ]} + data={instrumentInfo} + initialState={{ columnPinning: { right: ['kind'] } }} + /> +
+ ) : ( +

+ {t({ + en: 'Error finding instruments', + es: 'Error al buscar los instrumentos', + fr: 'Erreur lors de la recherche des instruments' + })} +

+ )} @@ -368,30 +371,32 @@ const RouteComponent = () => { {instrumentInfo ? ( - ( - ()} value={getValue()} /> - ), - header: ({ column }) => ( - - ), - id: 'instrumentTitle' - }, - { - accessorKey: 'count', - header: ({ column }) => ( - - ), - id: 'count', - size: 110 - } - ]} - data={recordCounter} - initialState={{ columnPinning: { right: ['count'] }, sorting: [{ desc: true, id: 'count' }] }} - /> +
+ ( + ()} value={getValue()} /> + ), + header: ({ column }) => ( + + ), + id: 'instrumentTitle' + }, + { + accessorKey: 'count', + header: ({ column }) => ( + + ), + id: 'count', + size: 110 + } + ]} + data={recordCounter} + initialState={{ columnPinning: { right: ['count'] }, sorting: [{ desc: true, id: 'count' }] }} + /> +
) : (

{t({ diff --git a/testing/src/pages/_app/dashboard.page.ts b/testing/src/pages/_app/dashboard.page.ts index d8e43bee30..df9079f611 100644 --- a/testing/src/pages/_app/dashboard.page.ts +++ b/testing/src/pages/_app/dashboard.page.ts @@ -1,3 +1,4 @@ +import { expect } from '@playwright/test'; import type { Locator, Page } from '@playwright/test'; import { AppPage } from './route.page'; @@ -29,4 +30,19 @@ export class DashboardPage extends AppPage { this.instrumentsDialog = page.getByTestId('dashboard-instruments-Modal-dialog'); this.recordsDialog = page.getByTestId('dashboard-record-Modal-dialog'); } + + /** Every row of a dialog's paginated table, counted by stepping through its pages. */ + async countDialogRows(dialog: Locator): Promise { + const rows = dialog.getByTestId('data-table-row'); + const nextButton = dialog.locator('button:has(svg.lucide-chevron-right)'); + await rows.first().waitFor(); + let count = await rows.count(); + while (await nextButton.isEnabled()) { + const firstRow = await rows.first().textContent(); + await nextButton.click(); + await expect(rows.first()).not.toHaveText(firstRow!); + count += await rows.count(); + } + return count; + } } diff --git a/testing/src/specs/dashboard.spec.ts b/testing/src/specs/dashboard.spec.ts index 9afb724030..7b1c78b338 100644 --- a/testing/src/specs/dashboard.spec.ts +++ b/testing/src/specs/dashboard.spec.ts @@ -65,6 +65,46 @@ test.describe('dashboard', () => { await expect(dashboardPage.usersDialog).toContainText('Users'); }); + test('should find a user of the group by searching the users dialog', async ({ api, authenticateAs, page }) => { + const group = await api.createGroup(); + const { credentials } = await api.createUser({ basePermissionLevel: 'GROUP_MANAGER', groupIds: [group.id] }); + const { credentials: colleague } = await api.createUser({ groupIds: [group.id] }); + await authenticateAs(credentials); + + const dashboardPage = new DashboardPage(page); + await dashboardPage.goto('/dashboard'); + await dashboardPage.statisticUsers.click(); + const userRows = dashboardPage.usersDialog.getByTestId('data-table-row'); + await expect(userRows).toHaveCount(2); + await dashboardPage.usersDialog.getByRole('searchbox').fill(colleague.username); + await expect(userRows).toHaveCount(1); + await expect(userRows).toHaveText(colleague.username); + }); + + // The dialogs show their tables as plain text, with larger bold headers in place of the borders. + test('should show the tables in the dashboard dialogs as text, without borders', async ({ getPageModel, page }) => { + const dashboardPage = await getPageModel('/dashboard'); + for (const [statistic, dialog] of [ + [dashboardPage.statisticUsers, dashboardPage.usersDialog], + [dashboardPage.statisticInstruments, dashboardPage.instrumentsDialog], + [dashboardPage.statisticRecords, dashboardPage.recordsDialog] + ] as const) { + await statistic.click(); + const frame = dialog.getByTestId('data-table').locator('> .border'); + await expect(frame).toHaveCSS('border-top-width', '0px'); + for (const cell of [ + dialog.getByTestId('data-table-row').first(), + frame.getByTestId('data-table-head').locator('> div > div').first() + ]) { + await expect(cell).toHaveCSS('border-bottom-width', '0px'); + await expect(cell).toHaveCSS('border-right-width', '0px'); + } + await expect(frame.getByTestId('data-table-head')).toHaveCSS('font-weight', '700'); + await expect(frame.getByTestId('data-table-head')).toHaveCSS('font-size', '16px'); + await page.keyboard.press('Escape'); + } + }); + // A record keeps the id of the edition it was collected with, so the records dialog once counted // only the latest edition's records, and the instruments card counted every edition in the database. // Edition 2 is stored after the group exists, so the group gains it the way an upgrade does. @@ -100,9 +140,12 @@ test.describe('dashboard', () => { await page.keyboard.press('Escape'); await dashboardPage.statisticInstruments.click(); - const instrumentRows = dashboardPage.instrumentsDialog.getByRole('listitem'); - await expect(instrumentRows.filter({ hasText: title })).toHaveCount(1); - const instrumentRowCount = await instrumentRows.count(); + const instrumentRows = dashboardPage.instrumentsDialog.getByTestId('data-table-row'); + await dashboardPage.instrumentsDialog.getByRole('searchbox').fill(title); + await expect(instrumentRows).toHaveCount(1); + await expect(instrumentRows.getByTestId('instrument-cell-kind')).toHaveText('Form'); + await dashboardPage.instrumentsDialog.getByRole('searchbox').fill(''); + const instrumentRowCount = await dashboardPage.countDialogRows(dashboardPage.instrumentsDialog); await page.keyboard.press('Escape'); await expect(dashboardPage.statisticInstruments.getByRole('heading')).toHaveText(String(instrumentRowCount)); }); From f9549380318fd4aa71352087892e526ab7801810 Mon Sep 17 00:00:00 2001 From: thomasbeaudry Date: Mon, 9 Nov 2026 16:18:50 -0500 Subject: [PATCH 4/7] style(web): centre the records and sessions legend under its chart The legend spans the whole card rather than the plot area, sits a line below the chart, and spaces its two items evenly apart. Co-Authored-By: Claude Opus 5.5 --- .../routes/_app/__tests__/dashboard.test.tsx | 21 ++++++++++++++++++- apps/web/src/routes/_app/dashboard.tsx | 7 +++++-- testing/src/specs/dashboard.spec.ts | 11 ++++++++++ 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/apps/web/src/routes/_app/__tests__/dashboard.test.tsx b/apps/web/src/routes/_app/__tests__/dashboard.test.tsx index fdc98e3267..8ddb130302 100644 --- a/apps/web/src/routes/_app/__tests__/dashboard.test.tsx +++ b/apps/web/src/routes/_app/__tests__/dashboard.test.tsx @@ -150,7 +150,14 @@ vi.mock('recharts', () => ({ ), CartesianGrid: ({ stroke }: { stroke: string }) => , - Legend: () => , + Legend: ({ wrapperStyle }: { wrapperStyle?: { left?: number; paddingTop?: number; width?: string } }) => ( + + ), ResponsiveContainer: ({ children }: { children: ReactNode }) =>

{children}
, Tooltip: ({ content }: { content: (props: TooltipProps) => ReactNode }) => ( {content(mocks.tooltipProps)} @@ -413,6 +420,18 @@ describe('Route', () => { expect(within(dialog).getByText('Error finding records')).toBeTruthy(); }); + it('should leave a line of space between the chart and its legend', () => { + renderPage(); + expect(recordsChart().getByTestId('chart-legend').dataset.paddingTop).toBe('16'); + }); + + it('should span the legend across the whole chart, so it centres on the card rather than the plot area', () => { + renderPage(); + const legend = recordsChart().getByTestId('chart-legend'); + expect(legend.dataset.left).toBe('0'); + expect(legend.dataset.width).toBe('100%'); + }); + it.each([ ['en', 'Records', 'Sessions'], ['es', 'Registros', 'Sesiones'], diff --git a/apps/web/src/routes/_app/dashboard.tsx b/apps/web/src/routes/_app/dashboard.tsx index b97984b2d4..f4b68c9470 100644 --- a/apps/web/src/routes/_app/dashboard.tsx +++ b/apps/web/src/routes/_app/dashboard.tsx @@ -439,7 +439,9 @@ const RouteComponent = () => { -
+ {/* recharts spaces legend items with an inline right margin, which also pushes the legend off + centre; only an important class can replace it with an even margin on both sides. */} +
@@ -504,7 +506,8 @@ const RouteComponent = () => { return null; }} /> - + {/* recharts sizes the legend to the plot area, which the chart's right margin shifts off the card's centre. */} + { await expect(recordsIcon).not.toHaveAttribute('stroke-dasharray', '6 4'); await expect(sessionsIcon).toHaveAttribute('stroke-dasharray', '6 4'); + + await expect(legend.locator('..')).toHaveCSS('padding-top', '16px'); + const legendItems = legend.locator('.recharts-legend-item'); + await expect(legendItems.first()).toHaveCSS('margin-right', '16px'); + await expect(legendItems.first()).toHaveCSS('margin-left', '16px'); + + const wrapperBox = (await dashboardPage.recordsSessionsChart.boundingBox())!; + const firstItemBox = (await legendItems.first().boundingBox())!; + const lastItemBox = (await legendItems.last().boundingBox())!; + const itemsCentre = (firstItemBox.x + lastItemBox.x + lastItemBox.width) / 2; + expect(Math.abs(itemsCentre - (wrapperBox.x + wrapperBox.width / 2))).toBeLessThan(2); }); test('should navigate to the data hub when the subjects statistic is clicked', async ({ getPageModel, page }) => { From 435457677fe26acf99191efc5b0b60a96356993b Mon Sep 17 00:00:00 2001 From: thomasbeaudry Date: Mon, 9 Nov 2026 16:35:18 -0500 Subject: [PATCH 5/7] feat: choose the time window of each dashboard chart Each trend card has a time window in its top right corner: Default (the original five points thirty days apart), the last month, 3 or 6 months, the last year, 2 or 5 years, or all time. Points are spaced to suit the window, all time takes the shortest window holding every item, and an axis reaching back more than a year is labelled by month and year. GET /v1/summary takes recordTrendWindow and subjectTrendWindow, so the two charts are independent. Co-Authored-By: Claude Opus 5.5 --- .../__tests__/summary.controller.test.ts | 14 ++- .../summary/__tests__/summary.service.test.ts | 71 +++++++++++- apps/api/src/summary/summary.controller.ts | 12 ++- apps/api/src/summary/summary.service.ts | 101 ++++++++++++++++-- .../TrendWindowSelect/TrendWindowSelect.tsx | 43 ++++++++ .../__tests__/TrendWindowSelect.test.tsx | 55 ++++++++++ .../src/components/TrendWindowSelect/index.ts | 1 + .../hooks/__tests__/useSummaryQuery.test.ts | 15 +++ apps/web/src/hooks/useSummaryQuery.ts | 11 +- .../routes/_app/__tests__/dashboard.test.tsx | 41 ++++++- apps/web/src/routes/_app/dashboard.tsx | 81 +++++++++----- packages/schemas/src/summary/summary.ts | 21 ++++ testing/src/pages/_app/dashboard.page.ts | 4 + testing/src/specs/dashboard.spec.ts | 34 ++++++ 14 files changed, 458 insertions(+), 46 deletions(-) create mode 100644 apps/web/src/components/TrendWindowSelect/TrendWindowSelect.tsx create mode 100644 apps/web/src/components/TrendWindowSelect/__tests__/TrendWindowSelect.test.tsx create mode 100644 apps/web/src/components/TrendWindowSelect/index.ts diff --git a/apps/api/src/summary/__tests__/summary.controller.test.ts b/apps/api/src/summary/__tests__/summary.controller.test.ts index d6c2fb5c3b..0d310bff15 100644 --- a/apps/api/src/summary/__tests__/summary.controller.test.ts +++ b/apps/api/src/summary/__tests__/summary.controller.test.ts @@ -27,7 +27,17 @@ describe('SummaryController', () => { const summary = { counts: { instruments: 1, records: 2, sessions: 3, subjects: 4, users: 5 } }; summaryService.getSummary.mockResolvedValue(summary); - await expect(summaryController.getSummary(currentUser, 'group-1')).resolves.toBe(summary); - expect(summaryService.getSummary).toHaveBeenCalledWith(currentUser, 'group-1'); + await expect( + summaryController.getSummary(currentUser, { + groupId: 'group-1', + recordTrendWindow: 'pastYear', + subjectTrendWindow: 'default' + }) + ).resolves.toBe(summary); + expect(summaryService.getSummary).toHaveBeenCalledWith(currentUser, { + groupId: 'group-1', + recordTrendWindow: 'pastYear', + subjectTrendWindow: 'default' + }); }); }); diff --git a/apps/api/src/summary/__tests__/summary.service.test.ts b/apps/api/src/summary/__tests__/summary.service.test.ts index 23922addfb..8f6af1ad21 100644 --- a/apps/api/src/summary/__tests__/summary.service.test.ts +++ b/apps/api/src/summary/__tests__/summary.service.test.ts @@ -99,7 +99,7 @@ describe('SummaryService', () => { }); it('should scope every query to the group when a groupId is provided', async () => { - await summaryService.getSummary(currentUser, 'group-1'); + await summaryService.getSummary(currentUser, { groupId: 'group-1' }); expect(instrumentRecordsService.count).toHaveBeenCalledWith( expect.objectContaining({ groupId: 'group-1' }), expect.anything() @@ -117,10 +117,77 @@ describe('SummaryService', () => { }); it('should count instruments with the same edition rule and group scope as the instruments dialog', async () => { - await summaryService.getSummary(currentUser, 'group-1'); + await summaryService.getSummary(currentUser, { groupId: 'group-1' }); expect(instrumentsService.count).toHaveBeenCalledWith(currentUser, 'group-1'); }); + it.each([ + ['pastMonth', 5, { days: 7 }], + ['past3Months', 4, { months: 1 }], + ['past6Months', 7, { months: 1 }], + ['pastYear', 13, { months: 1 }], + ['past2Years', 9, { months: 3 }], + ['past5Years', 6, { years: 1 }] + ] as const)('should space a %s trend into %i points by its own calendar step', async (window, count, step) => { + const { trends } = await summaryService.getSummary(currentUser, { + recordTrendWindow: window, + subjectTrendWindow: window + }); + for (const trend of [trends.records, trends.sessions, trends.subjects]) { + expect(trend).toHaveLength(count); + const expected = new Date(trend[0]!.timestamp - ('days' in step ? step.days * 24 * 60 * 60 * 1000 : 0)); + expected.setFullYear( + expected.getFullYear() - ('years' in step ? step.years : 0), + expected.getMonth() - ('months' in step ? step.months : 0) + ); + expect(trend[1]!.timestamp).toBe(expected.getTime()); + } + }); + + it('should window the records chart and the subjects chart independently', async () => { + const { trends } = await summaryService.getSummary(currentUser, { + recordTrendWindow: 'pastYear', + subjectTrendWindow: 'pastMonth' + }); + expect(trends.records).toHaveLength(13); + expect(trends.sessions).toHaveLength(13); + expect(trends.subjects).toHaveLength(5); + }); + + it('should draw all time over the shortest window reaching back past every record and session', async () => { + const SIX_MONTHS_MS = 183 * 24 * 60 * 60 * 1000; + const oldest = Date.now() - SIX_MONTHS_MS + THIRTY_DAYS_MS; + instrumentRecordsService.count.mockImplementation((where: { date?: { lte: Date } }) => { + return Promise.resolve(where.date && where.date.lte.getTime() < oldest ? 0 : 1); + }); + sessionsService.count.mockResolvedValue(0); + const { trends } = await summaryService.getSummary(currentUser, { recordTrendWindow: 'all' }); + expect(trends.records).toHaveLength(7); + expect(trends.sessions).toHaveLength(7); + }); + + it('should draw all time over the shortest window reaching back past every subject', async () => { + subjectsService.count.mockImplementation((where: { createdAt?: { lte: Date } }) => { + return Promise.resolve( + where.createdAt && where.createdAt.lte.getTime() < Date.now() - 20 * 24 * 60 * 60 * 1000 ? 0 : 1 + ); + }); + const { trends } = await summaryService.getSummary(currentUser, { subjectTrendWindow: 'all' }); + expect(trends.subjects).toHaveLength(5); + }); + + it('should widen all time past five years until it reaches the oldest record, in a fixed number of points', async () => { + const oldest = new Date(Date.now()).setFullYear(new Date().getFullYear() - 30); + instrumentRecordsService.count.mockImplementation((where: { date?: { lte: Date } }) => { + return Promise.resolve(where.date && where.date.lte.getTime() < oldest ? 0 : 1); + }); + const { trends } = await summaryService.getSummary(currentUser, { recordTrendWindow: 'all' }); + expect(trends.records).toHaveLength(6); + const spanYears = + new Date(trends.records[0]!.timestamp).getFullYear() - new Date(trends.records.at(-1)!.timestamp).getFullYear(); + expect(spanYears).toBe(40); + }); + it('should issue every query concurrently rather than awaiting them in sequence', async () => { // Hold every count open until all of them have been called. If any query waits on the result // of another, the expected number is never reached and this test times out. diff --git a/apps/api/src/summary/summary.controller.ts b/apps/api/src/summary/summary.controller.ts index 8f5fe7af9c..62cd2dc7f9 100644 --- a/apps/api/src/summary/summary.controller.ts +++ b/apps/api/src/summary/summary.controller.ts @@ -1,7 +1,8 @@ -import { CurrentUser } from '@douglasneuroinformatics/libnest'; +import { CurrentUser, ParseSchemaPipe } from '@douglasneuroinformatics/libnest'; import type { RequestUser } from '@douglasneuroinformatics/libnest'; import { Controller, Get, Query } from '@nestjs/common'; -import type { Summary } from '@opendatacapture/schemas/summary'; +import { $SummaryQueryParams } from '@opendatacapture/schemas/summary'; +import type { Summary, SummaryQueryParams } from '@opendatacapture/schemas/summary'; import { RouteAccess } from '@/core/decorators/route-access.decorator'; @@ -19,7 +20,10 @@ export class SummaryController { { action: 'read', subject: 'Subject' }, { action: 'read', subject: 'User' } ]) - async getSummary(@CurrentUser() currentUser: RequestUser, @Query('groupId') groupId?: string): Promise { - return this.summaryService.getSummary(currentUser, groupId); + async getSummary( + @CurrentUser() currentUser: RequestUser, + @Query(new ParseSchemaPipe({ schema: $SummaryQueryParams })) query: SummaryQueryParams + ): Promise { + return this.summaryService.getSummary(currentUser, query); } } diff --git a/apps/api/src/summary/summary.service.ts b/apps/api/src/summary/summary.service.ts index bd0aef2356..e1228b04a8 100644 --- a/apps/api/src/summary/summary.service.ts +++ b/apps/api/src/summary/summary.service.ts @@ -1,6 +1,6 @@ import type { RequestUser } from '@douglasneuroinformatics/libnest'; import { Injectable } from '@nestjs/common'; -import type { Summary } from '@opendatacapture/schemas/summary'; +import type { Summary, SummaryQueryParams, SummaryTrendWindow } from '@opendatacapture/schemas/summary'; import { InstrumentRecordsService } from '@/instrument-records/instrument-records.service'; import { InstrumentsService } from '@/instruments/instruments.service'; @@ -8,8 +8,41 @@ import { SessionsService } from '@/sessions/sessions.service'; import { SubjectsService } from '@/subjects/subjects.service'; import { UsersService } from '@/users/users.service'; -const THIRTY_DAYS_MS = 30 * 24 * 60 * 60 * 1000; -const TREND_POINT_COUNT = 5; +type TrendStep = { days?: number; months?: number; years?: number }; + +type TrendSpacing = { count: number; step: TrendStep }; + +type CountAsOf = (lte: Date) => Promise; + +const TREND_SPACINGS: { [K in Exclude]: TrendSpacing } = { + default: { count: 5, step: { days: 30 } }, + past2Years: { count: 9, step: { months: 3 } }, + past3Months: { count: 4, step: { months: 1 } }, + past5Years: { count: 6, step: { years: 1 } }, + past6Months: { count: 7, step: { months: 1 } }, + pastMonth: { count: 5, step: { days: 7 } }, + pastYear: { count: 13, step: { months: 1 } } +}; + +/** The bounded windows, shortest first: `all` takes the first one that reaches back past every item. */ +const BOUNDED_WINDOWS = ['pastMonth', 'past3Months', 'past6Months', 'pastYear', 'past2Years', 'past5Years'] as const; + +/** The number of points `all` draws once the data is older than the longest bounded window. */ +const ALL_TIME_POINT_COUNT = 6; + +const DAY_MS = 24 * 60 * 60 * 1000; + +/** + * Each point's timestamp, most recent first, stepping back from `now`. Days step by a fixed length, + * so a daylight saving change cannot stretch them; months and years step by the calendar. + */ +function trendTimestamps(now: number, { count, step }: TrendSpacing): number[] { + return Array.from({ length: count }, (_, i) => { + const date = new Date(now - (step.days ?? 0) * i * DAY_MS); + date.setFullYear(date.getFullYear() - (step.years ?? 0) * i, date.getMonth() - (step.months ?? 0) * i); + return date.getTime(); + }); +} @Injectable() export class SummaryService { @@ -21,16 +54,32 @@ export class SummaryService { private readonly subjectsService: SubjectsService ) {} - async getSummary(currentUser: RequestUser, groupId?: string): Promise { + async getSummary( + currentUser: RequestUser, + { groupId, recordTrendWindow = 'default', subjectTrendWindow = 'default' }: Partial = {} + ): Promise { const { ability } = currentUser; const filter = groupId ? { groupIds: { has: groupId } } : ({} as { [key: string]: unknown }); const sessionFilter = groupId ? { groupId } : {}; const recordFilter = groupId ? { groupId } : {}; + const countRecordsAsOf: CountAsOf = (lte) => { + return this.instrumentRecordsService.count({ date: { lte }, ...recordFilter }, { ability }); + }; + const countSessionsAsOf: CountAsOf = (lte) => { + return this.sessionsService.count({ date: { lte }, ...sessionFilter }, { ability }); + }; + const countSubjectsAsOf: CountAsOf = (lte) => { + return this.subjectsService.count({ createdAt: { lte }, ...filter }, { ability }); + }; + const now = Date.now(); - const timestamps = Array.from({ length: TREND_POINT_COUNT }, (_, i) => now - THIRTY_DAYS_MS * i); + const [recordTimestamps, subjectTimestamps] = await Promise.all([ + this.resolveTrendTimestamps(recordTrendWindow, now, [countRecordsAsOf, countSessionsAsOf]), + this.resolveTrendTimestamps(subjectTrendWindow, now, [countSubjectsAsOf]) + ]); - const countTrend = (countAsOf: (lte: Date) => Promise): Promise => { + const countTrend = (timestamps: number[], countAsOf: CountAsOf): Promise => { return Promise.all( timestamps.map(async (timestamp) => ({ timestamp, value: await countAsOf(new Date(timestamp)) })) ); @@ -45,9 +94,9 @@ export class SummaryService { this.sessionsService.count(sessionFilter, { ability }), this.subjectsService.count(filter, { ability }), this.usersService.count(filter, { ability }), - countTrend((lte) => this.instrumentRecordsService.count({ date: { lte }, ...recordFilter }, { ability })), - countTrend((lte) => this.sessionsService.count({ date: { lte }, ...sessionFilter }, { ability })), - countTrend((lte) => this.subjectsService.count({ createdAt: { lte }, ...filter }, { ability })) + countTrend(recordTimestamps, countRecordsAsOf), + countTrend(recordTimestamps, countSessionsAsOf), + countTrend(subjectTimestamps, countSubjectsAsOf) ]); return { @@ -55,4 +104,38 @@ export class SummaryService { trends: { records: recordTrend, sessions: sessionTrend, subjects: subjectTrend } }; } + + /** Whether nothing any of the counts measures existed as of `timestamp`. */ + private async isEmptyAsOf(timestamp: number, countsAsOf: CountAsOf[]): Promise { + const counts = await Promise.all(countsAsOf.map((countAsOf) => countAsOf(new Date(timestamp)))); + return counts.every((count) => count === 0); + } + + /** + * The timestamps of a trend over `window`. For `all`, the shortest bounded window that reaches back + * past every item the counts measure; past the longest, a span doubled from ten years until it does, + * drawn as a fixed number of points so an old instance does not draw one point per year. + */ + private async resolveTrendTimestamps( + window: SummaryTrendWindow, + now: number, + countsAsOf: CountAsOf[] + ): Promise { + if (window !== 'all') { + return trendTimestamps(now, TREND_SPACINGS[window]); + } + for (const bounded of BOUNDED_WINDOWS) { + const timestamps = trendTimestamps(now, TREND_SPACINGS[bounded]); + if (await this.isEmptyAsOf(timestamps.at(-1)!, countsAsOf)) { + return timestamps; + } + } + let spanYears = 10; + let timestamps = trendTimestamps(now, { count: ALL_TIME_POINT_COUNT, step: { years: spanYears / 5 } }); + while (!(await this.isEmptyAsOf(timestamps.at(-1)!, countsAsOf))) { + spanYears *= 2; + timestamps = trendTimestamps(now, { count: ALL_TIME_POINT_COUNT, step: { years: spanYears / 5 } }); + } + return timestamps; + } } diff --git a/apps/web/src/components/TrendWindowSelect/TrendWindowSelect.tsx b/apps/web/src/components/TrendWindowSelect/TrendWindowSelect.tsx new file mode 100644 index 0000000000..34c5b93745 --- /dev/null +++ b/apps/web/src/components/TrendWindowSelect/TrendWindowSelect.tsx @@ -0,0 +1,43 @@ +import { Select } from '@douglasneuroinformatics/libui/components'; +import { useTranslation } from '@douglasneuroinformatics/libui/hooks'; +import { $SummaryTrendWindow } from '@opendatacapture/schemas/summary'; +import type { SummaryTrendWindow } from '@opendatacapture/schemas/summary'; + +type TrendWindowSelectProps = { + 'data-testid'?: string; + onChange: (window: SummaryTrendWindow) => void; + value: SummaryTrendWindow; +}; + +/** Picks how far back a dashboard trend chart reaches. */ +export const TrendWindowSelect = ({ 'data-testid': testId, onChange, value }: TrendWindowSelectProps) => { + const { t } = useTranslation(); + const labels: { [K in SummaryTrendWindow]: string } = { + all: t({ en: 'All Time', es: 'Todo el tiempo', fr: 'Depuis le début' }), + default: t({ en: 'Default', es: 'Predeterminado', fr: 'Par défaut' }), + past2Years: t({ en: 'Last 2 Years', es: 'Últimos 2 años', fr: '2 dernières années' }), + past3Months: t({ en: 'Last 3 Months', es: 'Últimos 3 meses', fr: '3 derniers mois' }), + past5Years: t({ en: 'Last 5 Years', es: 'Últimos 5 años', fr: '5 dernières années' }), + past6Months: t({ en: 'Last 6 Months', es: 'Últimos 6 meses', fr: '6 derniers mois' }), + pastMonth: t({ en: 'Last Month', es: 'Último mes', fr: 'Dernier mois' }), + pastYear: t({ en: 'Last Year', es: 'Último año', fr: 'Dernière année' }) + }; + return ( + + ); +}; diff --git a/apps/web/src/components/TrendWindowSelect/__tests__/TrendWindowSelect.test.tsx b/apps/web/src/components/TrendWindowSelect/__tests__/TrendWindowSelect.test.tsx new file mode 100644 index 0000000000..269d975342 --- /dev/null +++ b/apps/web/src/components/TrendWindowSelect/__tests__/TrendWindowSelect.test.tsx @@ -0,0 +1,55 @@ +import { i18n } from '@douglasneuroinformatics/libui/i18n'; +import { cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { TrendWindowSelect } from '../TrendWindowSelect'; + +import '@/services/i18n'; + +const trigger = () => screen.getByTestId('trend-window'); + +const open = () => fireEvent.keyDown(trigger(), { key: 'Enter' }); + +describe('TrendWindowSelect', () => { + beforeEach(() => { + i18n.changeLanguage('en'); + }); + + afterEach(() => { + cleanup(); + }); + + it('should label the default window as the default', () => { + render(); + expect(trigger().textContent).toBe('Default'); + }); + + it('should offer every window, shortest first after the default', () => { + render(); + open(); + expect(screen.getAllByRole('option').map((option) => option.textContent)).toEqual([ + 'Default', + 'Last Month', + 'Last 3 Months', + 'Last 6 Months', + 'Last Year', + 'Last 2 Years', + 'Last 5 Years', + 'All Time' + ]); + }); + + it('should report the chosen window by its key', () => { + const onChange = vi.fn(); + render(); + open(); + fireEvent.click(screen.getByRole('option', { name: 'Last 2 Years' })); + expect(onChange).toHaveBeenCalledWith('past2Years'); + }); + + it('should label its options in the interface language', () => { + i18n.changeLanguage('fr'); + render(); + expect(trigger().textContent).toBe('Depuis le début'); + }); +}); diff --git a/apps/web/src/components/TrendWindowSelect/index.ts b/apps/web/src/components/TrendWindowSelect/index.ts new file mode 100644 index 0000000000..75097f8a85 --- /dev/null +++ b/apps/web/src/components/TrendWindowSelect/index.ts @@ -0,0 +1 @@ +export * from './TrendWindowSelect'; diff --git a/apps/web/src/hooks/__tests__/useSummaryQuery.test.ts b/apps/web/src/hooks/__tests__/useSummaryQuery.test.ts index fac5cd5746..333d4fb8b7 100644 --- a/apps/web/src/hooks/__tests__/useSummaryQuery.test.ts +++ b/apps/web/src/hooks/__tests__/useSummaryQuery.test.ts @@ -44,6 +44,21 @@ describe('useSummaryQuery', () => { expect(mockAxios.get).toHaveBeenCalledWith('/v1/summary', { params: undefined }); }); + it('should request the chosen trend windows', async () => { + const params = { groupId: 'group-1', recordTrendWindow: 'pastYear', subjectTrendWindow: 'all' } as const; + const { result } = renderSummaryQuery({ params }); + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + expect(mockAxios.get).toHaveBeenCalledWith('/v1/summary', { params }); + }); + + it('should key an omitted window as the default one, so the loader prefetch serves the page', () => { + expect(summaryQueryOptions({ params: { groupId: 'group-1' } }).queryKey).toEqual( + summaryQueryOptions({ + params: { groupId: 'group-1', recordTrendWindow: 'default', subjectTrendWindow: 'default' } + }).queryKey + ); + }); + it('should reject a summary with a negative count, so a corrupt total never reaches the dashboard', async () => { mockAxios.get.mockResolvedValueOnce({ data: { ...summary, counts: { ...summary.counts, users: -1 } } }); await expect(new QueryClient().fetchQuery(summaryQueryOptions())).rejects.toThrow(z.ZodError); diff --git a/apps/web/src/hooks/useSummaryQuery.ts b/apps/web/src/hooks/useSummaryQuery.ts index 2fa3e11d9c..eeebf8cc6a 100644 --- a/apps/web/src/hooks/useSummaryQuery.ts +++ b/apps/web/src/hooks/useSummaryQuery.ts @@ -1,9 +1,12 @@ import { $Summary } from '@opendatacapture/schemas/summary'; +import type { SummaryTrendWindow } from '@opendatacapture/schemas/summary'; import { keepPreviousData, queryOptions, useQuery } from '@tanstack/react-query'; import axios from 'axios'; type SummaryQueryParams = { groupId?: string; + recordTrendWindow?: SummaryTrendWindow; + subjectTrendWindow?: SummaryTrendWindow; }; export const summaryQueryOptions = ({ params }: { params?: SummaryQueryParams } = {}) => { @@ -15,7 +18,13 @@ export const summaryQueryOptions = ({ params }: { params?: SummaryQueryParams } }); return $Summary.parse(response.data); }, - queryKey: ['summary', params?.groupId] + // An omitted window is the default one, so the loader's prefetch and the page share a cache entry. + queryKey: [ + 'summary', + params?.groupId, + params?.recordTrendWindow ?? 'default', + params?.subjectTrendWindow ?? 'default' + ] }); }; diff --git a/apps/web/src/routes/_app/__tests__/dashboard.test.tsx b/apps/web/src/routes/_app/__tests__/dashboard.test.tsx index 8ddb130302..eda5c3a5c2 100644 --- a/apps/web/src/routes/_app/__tests__/dashboard.test.tsx +++ b/apps/web/src/routes/_app/__tests__/dashboard.test.tsx @@ -237,7 +237,9 @@ describe('Route', () => { it('should scope every query to the current group', () => { mocks.store.currentGroup = RESEARCH_GROUP; renderPage(); - expect(mocks.useSummaryQuery).toHaveBeenCalledWith({ params: { groupId: 'group-1' } }); + expect(mocks.useSummaryQuery).toHaveBeenCalledWith({ + params: { groupId: 'group-1', recordTrendWindow: 'default', subjectTrendWindow: 'default' } + }); expect(mocks.useUsersQuery).toHaveBeenCalledWith({ params: { groupId: 'group-1' } }); expect(mocks.useInstrumentRecords).toHaveBeenCalledWith({ enabled: true, params: { groupId: 'group-1' } }); }); @@ -477,6 +479,35 @@ describe('Route', () => { expect(subjectsChart().getByTestId('x-axis').textContent).toBe('Mar 2'); }); + it('should label the x axis ticks with month and year once a trend reaches back more than a year', () => { + const twoYearsEarlier = new Date(2023, 2, 2, 12).getTime(); + mocks.summary = { + data: { + ...SUMMARY, + trends: { + ...SUMMARY.trends, + records: [ + { timestamp: DAY_3, value: 3 }, + { timestamp: twoYearsEarlier, value: 0 } + ] + } + } + }; + renderPage(); + expect(recordsChart().getByTestId('x-axis').textContent).toBe('Mar 2025'); + expect(subjectsChart().getByTestId('x-axis').textContent).toBe('Mar 2'); + }); + + it.each([ + ['dashboard-records-trend-window', { recordTrendWindow: 'pastYear', subjectTrendWindow: 'default' }], + ['dashboard-subjects-trend-window', { recordTrendWindow: 'default', subjectTrendWindow: 'pastYear' }] + ] as const)('should request the window chosen in %s for its own chart only', (testId, windows) => { + renderPage(); + fireEvent.keyDown(screen.getByTestId(testId), { key: 'Enter' }); + fireEvent.click(screen.getByRole('option', { name: 'Last Year' })); + expect(mocks.useSummaryQuery).toHaveBeenLastCalledWith({ params: { groupId: undefined, ...windows } }); + }); + it('should show nothing in the tooltips while no point is hovered, even if recharts still holds a payload', () => { mocks.tooltipProps = { active: false, @@ -586,14 +617,18 @@ describe('Route', () => { it('should prefetch the summary of the current group for a user who can read everything', async () => { grantRead(READABLE_SUBJECTS); expect(await runLoader()).toBeNull(); - expect(mocks.ensureQueryData).toHaveBeenCalledWith(expect.objectContaining({ queryKey: ['summary', 'group-1'] })); + expect(mocks.ensureQueryData).toHaveBeenCalledWith( + expect.objectContaining({ queryKey: ['summary', 'group-1', 'default', 'default'] }) + ); }); it('should prefetch the application-wide summary when no group is selected', async () => { grantRead(READABLE_SUBJECTS); mocks.store.currentGroup = null; await runLoader(); - expect(mocks.ensureQueryData).toHaveBeenCalledWith(expect.objectContaining({ queryKey: ['summary', undefined] })); + expect(mocks.ensureQueryData).toHaveBeenCalledWith( + expect.objectContaining({ queryKey: ['summary', undefined, 'default', 'default'] }) + ); }); }); }); diff --git a/apps/web/src/routes/_app/dashboard.tsx b/apps/web/src/routes/_app/dashboard.tsx index f4b68c9470..7037fbd82c 100644 --- a/apps/web/src/routes/_app/dashboard.tsx +++ b/apps/web/src/routes/_app/dashboard.tsx @@ -7,12 +7,14 @@ import { cn } from '@douglasneuroinformatics/libui/utils'; import { ClipboardDocumentIcon, DocumentTextIcon, UserIcon, UsersIcon } from '@heroicons/react/24/solid'; import type { InstrumentKind } from '@opendatacapture/runtime-core'; import type { AppSubjectName } from '@opendatacapture/schemas/core'; +import type { Summary, SummaryTrendWindow } from '@opendatacapture/schemas/summary'; import { createFileRoute, redirect, useNavigate } from '@tanstack/react-router'; import { Area, AreaChart, CartesianGrid, Legend, ResponsiveContainer, Tooltip, XAxis, YAxis } from 'recharts'; import { InstrumentKindCell } from '@/components/InstrumentKindCell'; import { PageHeader } from '@/components/PageHeader'; import { SortableHeader } from '@/components/SortableHeader'; +import { TrendWindowSelect } from '@/components/TrendWindowSelect'; import { TruncatedCell } from '@/components/TruncatedCell'; import { useInstrumentInfoQuery } from '@/hooks/useInstrumentInfoQuery'; import { useInstrumentRecords } from '@/hooks/useInstrumentRecords'; @@ -27,11 +29,26 @@ const TEXT_ONLY_TABLE = cn( '[&_[data-testid=data-table-head]]:text-base [&_[data-testid=data-table-head]]:font-bold' ); +const ONE_YEAR_MS = 365 * 24 * 60 * 60 * 1000; + +/** A chart axis label: day and month within a year, month and year for a trend reaching further back. */ +const formatTrendTick = (timestamp: number, trend: Summary['trends']['records']) => { + const spansYears = trend[0]!.timestamp - trend.at(-1)!.timestamp > ONE_YEAR_MS; + return new Date(timestamp).toLocaleDateString( + undefined, + spansYears ? { month: 'short', year: 'numeric' } : { day: 'numeric', month: 'short' } + ); +}; + const RouteComponent = () => { const currentGroup = useAppStore((store) => store.currentGroup); const { t } = useTranslation(); const [theme] = useTheme(); - const summaryQuery = useSummaryQuery({ params: { groupId: currentGroup?.id } }); + const [recordTrendWindow, setRecordTrendWindow] = useState('default'); + const [subjectTrendWindow, setSubjectTrendWindow] = useState('default'); + const summaryQuery = useSummaryQuery({ + params: { groupId: currentGroup?.id, recordTrendWindow, subjectTrendWindow } + }); const navigate = useNavigate(); const [isInstrumentModalOpen, setIsInstrumentModalOpen] = useState(false); const [isUserModalOpen, setIsUserModalOpen] = useState(false); @@ -421,14 +438,24 @@ const RouteComponent = () => {
-
- - {t({ - en: 'Records & Sessions Trend', - es: 'Evolución de registros y sesiones', - fr: 'Tendance des enregistrements et des sessions' - })} - +
+ {/* The window picker shares the title row so its label lines up with the title. */} +
+ + {t({ + en: 'Records & Sessions Trend', + es: 'Evolución de registros y sesiones', + fr: 'Tendance des enregistrements et des sessions' + })} + +
+ +
+

{t({ en: 'Activity over time', @@ -465,10 +492,7 @@ const RouteComponent = () => { domain={['auto', 'auto']} stroke={strokeColors[theme]} tick={{ fontSize: 12 }} - tickFormatter={(timestamp: number) => { - const date = new Date(timestamp); - return date.toLocaleDateString(undefined, { day: 'numeric', month: 'short' }); - }} + tickFormatter={(timestamp: number) => formatTrendTick(timestamp, summaryQuery.data.trends.records)} tickLine={false} type="number" /> @@ -544,14 +568,24 @@ const RouteComponent = () => {

-
- - {t({ - en: 'Subjects Growth', - es: 'Crecimiento de sujetos', - fr: 'Croissance des sujets' - })} - +
+ {/* The window picker shares the title row so its label lines up with the title. */} +
+ + {t({ + en: 'Subjects Growth', + es: 'Crecimiento de sujetos', + fr: 'Croissance des sujets' + })} + +
+ +
+

{t({ en: 'Growth trajectory', @@ -585,10 +619,7 @@ const RouteComponent = () => { domain={['auto', 'auto']} stroke={strokeColors[theme]} tick={{ fontSize: 12 }} - tickFormatter={(timestamp: number) => { - const date = new Date(timestamp); - return date.toLocaleDateString(undefined, { day: 'numeric', month: 'short' }); - }} + tickFormatter={(timestamp: number) => formatTrendTick(timestamp, summaryQuery.data.trends.subjects)} tickLine={false} type="number" /> diff --git a/packages/schemas/src/summary/summary.ts b/packages/schemas/src/summary/summary.ts index d9437895b4..819160f12c 100644 --- a/packages/schemas/src/summary/summary.ts +++ b/packages/schemas/src/summary/summary.ts @@ -7,6 +7,27 @@ const $Trend = z.array( }) ); +/** How far back a dashboard trend reaches; `default` is the original five points thirty days apart. */ +export type SummaryTrendWindow = z.infer; +export const $SummaryTrendWindow = z.enum([ + 'default', + 'pastMonth', + 'past3Months', + 'past6Months', + 'pastYear', + 'past2Years', + 'past5Years', + 'all' +]); + +export type SummaryQueryParams = z.infer; +export const $SummaryQueryParams = z.object({ + groupId: z.string().optional(), + /** The window of the records and sessions trends, which share a chart */ + recordTrendWindow: $SummaryTrendWindow.default('default'), + subjectTrendWindow: $SummaryTrendWindow.default('default') +}); + export type Summary = z.infer; export const $Summary = z.object({ counts: z.object({ diff --git a/testing/src/pages/_app/dashboard.page.ts b/testing/src/pages/_app/dashboard.page.ts index df9079f611..e49d1f76ba 100644 --- a/testing/src/pages/_app/dashboard.page.ts +++ b/testing/src/pages/_app/dashboard.page.ts @@ -8,11 +8,13 @@ export class DashboardPage extends AppPage { readonly pageHeader: Locator; readonly recordsDialog: Locator; readonly recordsSessionsChart: Locator; + readonly recordsTrendWindow: Locator; readonly statisticInstruments: Locator; readonly statisticRecords: Locator; readonly statisticSubjects: Locator; readonly statisticUsers: Locator; readonly subjectsGrowthChart: Locator; + readonly subjectsTrendWindow: Locator; readonly usersDialog: Locator; readonly welcomeHeading: Locator; @@ -26,6 +28,8 @@ export class DashboardPage extends AppPage { this.statisticRecords = page.getByTestId('statistic-records'); this.recordsSessionsChart = page.getByTestId('dashboard-chart-records-sessions'); this.subjectsGrowthChart = page.getByTestId('dashboard-chart-subjects-growth'); + this.recordsTrendWindow = page.getByTestId('dashboard-records-trend-window'); + this.subjectsTrendWindow = page.getByTestId('dashboard-subjects-trend-window'); this.usersDialog = page.getByTestId('dashboard-users-Modal-dialog'); this.instrumentsDialog = page.getByTestId('dashboard-instruments-Modal-dialog'); this.recordsDialog = page.getByTestId('dashboard-record-Modal-dialog'); diff --git a/testing/src/specs/dashboard.spec.ts b/testing/src/specs/dashboard.spec.ts index fb7e52baff..64ae2e7e4b 100644 --- a/testing/src/specs/dashboard.spec.ts +++ b/testing/src/specs/dashboard.spec.ts @@ -63,6 +63,40 @@ test.describe('dashboard', () => { expect(Math.abs(itemsCentre - (wrapperBox.x + wrapperBox.width / 2))).toBeLessThan(2); }); + test('should redraw a chart over the time window chosen in its corner', async ({ getPageModel, page }) => { + const dashboardPage = await getPageModel('/dashboard'); + await expect(dashboardPage.recordsTrendWindow).toHaveText('Default'); + await expect(dashboardPage.subjectsTrendWindow).toHaveText('Default'); + + const response = page.waitForResponse((res) => { + return res.url().includes('/v1/summary') && res.url().includes('recordTrendWindow=past5Years'); + }); + await dashboardPage.recordsTrendWindow.click(); + await page.getByRole('option', { name: 'Last 5 Years' }).click(); + const summary = (await (await response).json()) as { trends: { records: unknown[]; subjects: unknown[] } }; + + expect(summary.trends.records).toHaveLength(6); + expect(summary.trends.subjects).toHaveLength(5); + await expect(dashboardPage.recordsTrendWindow).toHaveText('Last 5 Years'); + await expect(dashboardPage.subjectsTrendWindow).toHaveText('Default'); + // Only the x axis labels can hold a four-digit year; the y axis counts records. + await expect(dashboardPage.recordsSessionsChart.getByRole('application')).toContainText(/\d{4}/); + }); + + test('should line each time window up with its chart title, near the card edge', async ({ getPageModel }) => { + const dashboardPage = await getPageModel('/dashboard'); + for (const [chart, select] of [ + [dashboardPage.recordsSessionsChart, dashboardPage.recordsTrendWindow], + [dashboardPage.subjectsGrowthChart, dashboardPage.subjectsTrendWindow] + ] as const) { + const cardBox = (await chart.boundingBox())!; + const titleBox = (await chart.getByRole('heading').first().boundingBox())!; + const selectBox = (await select.boundingBox())!; + expect(Math.abs(selectBox.y + selectBox.height / 2 - (titleBox.y + titleBox.height / 2))).toBeLessThan(2); + expect(cardBox.x + cardBox.width - (selectBox.x + selectBox.width)).toBeLessThanOrEqual(17); + } + }); + test('should navigate to the data hub when the subjects statistic is clicked', async ({ getPageModel, page }) => { const dashboardPage = await getPageModel('/dashboard'); await dashboardPage.statisticSubjects.click(); From 652e9e3f89d8a93c88817fcfa884bc83cffac865 Mon Sep 17 00:00:00 2001 From: thomasbeaudry Date: Mon, 9 Nov 2026 16:35:19 -0500 Subject: [PATCH 6/7] style(web): enlarge the text of the dashboard dialog tables Co-Authored-By: Claude Opus 5.5 --- apps/web/src/routes/_app/dashboard.tsx | 3 ++- testing/src/specs/dashboard.spec.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/web/src/routes/_app/dashboard.tsx b/apps/web/src/routes/_app/dashboard.tsx index 7037fbd82c..00c780668a 100644 --- a/apps/web/src/routes/_app/dashboard.tsx +++ b/apps/web/src/routes/_app/dashboard.tsx @@ -26,7 +26,8 @@ import { useAppStore } from '@/store'; // column headers are set larger and bold to stand apart from the rows without them. const TEXT_ONLY_TABLE = cn( '[&_[data-testid=data-table]>.border]:border-0 [&_[data-testid=data-table]>.border_*]:border-0', - '[&_[data-testid=data-table-head]]:text-base [&_[data-testid=data-table-head]]:font-bold' + '[&_[data-testid=data-table]>.border>div]:text-base', + '[&_[data-testid=data-table-head]]:text-lg [&_[data-testid=data-table-head]]:font-bold' ); const ONE_YEAR_MS = 365 * 24 * 60 * 60 * 1000; diff --git a/testing/src/specs/dashboard.spec.ts b/testing/src/specs/dashboard.spec.ts index 64ae2e7e4b..045b11a5e2 100644 --- a/testing/src/specs/dashboard.spec.ts +++ b/testing/src/specs/dashboard.spec.ts @@ -145,7 +145,8 @@ test.describe('dashboard', () => { await expect(cell).toHaveCSS('border-right-width', '0px'); } await expect(frame.getByTestId('data-table-head')).toHaveCSS('font-weight', '700'); - await expect(frame.getByTestId('data-table-head')).toHaveCSS('font-size', '16px'); + await expect(frame.getByTestId('data-table-head')).toHaveCSS('font-size', '18px'); + await expect(dialog.getByTestId('data-table-row').first()).toHaveCSS('font-size', '16px'); await page.keyboard.press('Escape'); } }); From d9937758029d783ace6be2f2635e5063bb08afb9 Mon Sep 17 00:00:00 2001 From: thomasbeaudry Date: Mon, 9 Nov 2026 17:32:40 -0500 Subject: [PATCH 7/7] test(testing): search for the series the instrument hub test creates The hub list is paginated over the shared catalog, so an instrument another spec adds can push the new series off the first page. Co-Authored-By: Claude Opus 5.5 --- testing/src/specs/instrument-hub.spec.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/testing/src/specs/instrument-hub.spec.ts b/testing/src/specs/instrument-hub.spec.ts index de35da7b81..810e105b00 100644 --- a/testing/src/specs/instrument-hub.spec.ts +++ b/testing/src/specs/instrument-hub.spec.ts @@ -53,6 +53,9 @@ test.describe('instrument hub', () => { const hubPage = new InstrumentHubPage(page); await hubPage.goto('/datahub/instruments'); + // The list is paginated and every spec's instruments are in the shared catalog, so the new + // series is not guaranteed a place on the first page. + await hubPage.searchInput.fill(seriesTitle); const row = hubPage.row(seriesTitle).first(); await expect(row).toBeVisible();