fix: count every instrument edition on the dashboard

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

apps/api/src/instruments/instruments.service.ts

@@ -83,8 +83,23 @@ export class InstrumentsService {
     private readonly virtualizationService: VirtualizationService<InstrumentVirtualizationContext>
   ) {}
 
-  async count(options: EntityOperationOptions = {}): Promise<number> {
-    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<number> {
+    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(

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,21 @@ 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, {
+        groupId: 'group-1',
+        recordTrendWindow: 'pastYear',
+        subjectTrendWindow: 'default'
+      })
+    ).resolves.toBe(summary);
+    expect(summaryService.getSummary).toHaveBeenCalledWith(currentUser, {
+      groupId: 'group-1',
+      recordTrendWindow: 'pastYear',
+      subjectTrendWindow: 'default'
+    });
   });
 });

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<InstrumentRecordsService>;
@@ -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, { groupId: 'group-1' });
       expect(instrumentRecordsService.count).toHaveBeenCalledWith(
         expect.objectContaining({ groupId: 'group-1' }),
         expect.anything()
@@ -112,6 +116,78 @@ describe('SummaryService', () => {
       }
     });
 
+    it('should count instruments with the same edition rule and group scope as the instruments dialog', async () => {
+      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.
@@ -136,7 +212,7 @@ describe('SummaryService', () => {
           return 0;
         });
       }
-      await summaryService.getSummary();
+      await summaryService.getSummary(currentUser);
       expect(started).toBe(EXPECTED_QUERIES);
     });
   });

apps/api/src/summary/summary.controller.ts

@@ -1,8 +1,9 @@
-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 type { AppAbility } from '@/auth/auth.types';
 import { RouteAccess } from '@/core/decorators/route-access.decorator';
 
 import { SummaryService } from './summary.service';
@@ -19,7 +20,10 @@ export class SummaryController {
     { action: 'read', subject: 'Subject' },
     { action: 'read', subject: 'User' }
   ])
-  async getSummary(@CurrentUser('ability') ability: AppAbility, @Query('groupId') groupId?: string): Promise<Summary> {
-    return this.summaryService.getSummary(groupId, { ability });
+  async getSummary(
+    @CurrentUser() currentUser: RequestUser,
+    @Query(new ParseSchemaPipe({ schema: $SummaryQueryParams })) query: SummaryQueryParams
+  ): Promise<Summary> {
+    return this.summaryService.getSummary(currentUser, query);
   }
 }