From 4c5a51811944222e7274a9c2dbad1ea280475146 Mon Sep 17 00:00:00 2001 From: "Gabriel A. Devenyi" Date: Mon, 20 Jul 2026 15:18:10 -0400 Subject: [PATCH 1/7] perf(api): group the has-record subject ids in the database The "with records only" datahub filter resolved its subject list with findMany({ distinct: ['subjectId'], select: { subjectId: true }, where }) but prisma applies `distinct` in the query engine rather than pushing it into mongodb, so the driver receives one row per *record* and collapses them to one row per subject only after they have arrived. On a group holding 100k records that is 100k rows transferred to produce a list of at most a few thousand ids. `groupBy` asks mongodb for the distinct values directly. Measured over HTTP against a live instance with 200,300 records, median of 7: GET /v1/subjects?hasRecord=true 366ms -> 42ms GET /v1/subjects?groupId=&hasRecord=true 67ms -> 37ms The unscoped case is where the difference shows, and it widens with the size of the collection: `groupBy` returns one row per subject regardless of how many records back it. The record query also now carries the caller's ability, which it previously ignored. The outer subject query already constrained the result, so this was not a data leak, but the inner query read records the caller had no permission to read. Note for anyone revisiting this: the issue also floated expressing the filter as `instrumentRecords: { some: ... }` on Subject. Measured on the same dataset that takes 26,849ms -- prisma compiles a relation filter on mongodb into a $lookup over the whole record collection. It is 270x slower than the code being replaced here, and carries the 100 MiB per-document ceiling that made the same construct fail outright in #1416. Refs #1413 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Bfen9VQemZanJLXinJ6MMG --- .../__tests__/subjects.service.spec.ts | 43 +++++++++++++------ apps/api/src/subjects/subjects.service.ts | 28 +++++++++--- 2 files changed, 51 insertions(+), 20 deletions(-) diff --git a/apps/api/src/subjects/__tests__/subjects.service.spec.ts b/apps/api/src/subjects/__tests__/subjects.service.spec.ts index df87ca6b4..2666b996c 100644 --- a/apps/api/src/subjects/__tests__/subjects.service.spec.ts +++ b/apps/api/src/subjects/__tests__/subjects.service.spec.ts @@ -32,7 +32,8 @@ describe('SubjectsService', () => { $transaction: vi.fn(), instrumentRecord: { deleteMany: vi.fn(), - findMany: vi.fn() + findMany: vi.fn(), + groupBy: vi.fn() }, session: { deleteMany: vi.fn() @@ -135,16 +136,9 @@ describe('SubjectsService', () => { await expect(subjectsService.find()).resolves.toMatchObject([{ id: '123' }]); }); it('should return the array of subjects with records', async () => { - prismaClient.instrumentRecord.findMany.mockResolvedValueOnce([{ subjectId: '123' }]); + prismaClient.instrumentRecord.groupBy.mockResolvedValueOnce([{ subjectId: '123' }]); subjectModel.findMany.mockResolvedValueOnce([{ id: '123' }]); await expect(subjectsService.find({ hasRecord: true })).resolves.toMatchObject([{ id: '123' }]); - expect(prismaClient.instrumentRecord.findMany).toHaveBeenCalledWith( - expect.objectContaining({ - distinct: ['subjectId'], - select: { subjectId: true }, - where: {} - }) - ); expect(subjectModel.findMany).toHaveBeenCalledWith( expect.objectContaining({ where: expect.objectContaining({ @@ -153,18 +147,41 @@ describe('SubjectsService', () => { }) ); }); + // `distinct` is applied by the prisma query engine, so it returns one row per record over the + // wire; grouping asks mongodb for one row per subject instead. + it('should group the ids in the database rather than deduplicating them after the fact', async () => { + prismaClient.instrumentRecord.groupBy.mockResolvedValueOnce([{ subjectId: '123' }]); + subjectModel.findMany.mockResolvedValueOnce([{ id: '123' }]); + await subjectsService.find({ hasRecord: true }); + expect(prismaClient.instrumentRecord.groupBy).toHaveBeenCalledWith( + expect.objectContaining({ by: ['subjectId'] }) + ); + expect(prismaClient.instrumentRecord.findMany).not.toHaveBeenCalled(); + }); it('should filter instrument records by groupId when provided', async () => { - prismaClient.instrumentRecord.findMany.mockResolvedValueOnce([{ subjectId: '123' }]); + prismaClient.instrumentRecord.groupBy.mockResolvedValueOnce([{ subjectId: '123' }]); subjectModel.findMany.mockResolvedValueOnce([{ id: '123' }]); await subjectsService.find({ groupId: 'group-1', hasRecord: true }); - expect(prismaClient.instrumentRecord.findMany).toHaveBeenCalledWith( + expect(prismaClient.instrumentRecord.groupBy).toHaveBeenCalledWith( expect.objectContaining({ - where: { groupId: 'group-1' } + where: { AND: [{}, { groupId: 'group-1' }] } }) ); }); + it('should constrain the record query to what the caller may read', async () => { + prismaClient.instrumentRecord.groupBy.mockResolvedValueOnce([]); + subjectModel.findMany.mockResolvedValueOnce([]); + const ability = createAppAbility([ + { action: 'read', subject: 'InstrumentRecord' }, + { action: 'read', subject: 'Subject' } + ]); + await subjectsService.find({ hasRecord: true }, { ability }); + const [call] = prismaClient.instrumentRecord.groupBy.mock.lastCall as [{ where: { AND: unknown[] } }]; + // the ability contributes the first clause; without it the query would be unconstrained + expect(call.where.AND[0]).not.toStrictEqual({}); + }); it('should pass all subject IDs returned by instrument records to the subject query', async () => { - prismaClient.instrumentRecord.findMany.mockResolvedValueOnce([{ subjectId: '123' }, { subjectId: '456' }]); + prismaClient.instrumentRecord.groupBy.mockResolvedValueOnce([{ subjectId: '123' }, { subjectId: '456' }]); subjectModel.findMany.mockResolvedValueOnce([{ id: '123' }, { id: '456' }]); await subjectsService.find({ hasRecord: true }); expect(subjectModel.findMany).toHaveBeenCalledWith( diff --git a/apps/api/src/subjects/subjects.service.ts b/apps/api/src/subjects/subjects.service.ts index 8c8e1aed2..9d90c0559 100644 --- a/apps/api/src/subjects/subjects.service.ts +++ b/apps/api/src/subjects/subjects.service.ts @@ -4,6 +4,7 @@ import { ConflictException, Injectable, NotFoundException } from '@nestjs/common import type { Prisma } from '@prisma/client'; import { accessibleQuery } from '@/auth/ability.utils'; +import type { AppAbility } from '@/auth/auth.types'; import type { RuntimePrismaClient } from '@/core/prisma'; import type { EntityOperationOptions } from '@/core/types'; @@ -146,7 +147,7 @@ export class SubjectsService { AND: [ accessibleQuery(ability, 'read', 'Subject'), groupInput, - { id: { in: await this.querySubjectIdsWithRecords(groupId) } } + { id: { in: await this.querySubjectIdsWithRecords(groupId, ability) } } ] } }); @@ -190,12 +191,25 @@ export class SubjectsService { return subjects.map((subject) => subject.id); } - private async querySubjectIdsWithRecords(groupId?: string): Promise { - const records = await this.prismaClient.instrumentRecord.findMany({ - distinct: ['subjectId'], - select: { subjectId: true }, - where: groupId ? { groupId } : {} + /** + * The ids of subjects having at least one record, for the "with records only" filter. + * + * Grouped by the database rather than deduplicated after the fact: `distinct` is applied by the + * prisma query engine, so it returns one row per *record* over the wire and collapses them only + * once they have arrived. + * + * Deliberately not expressed as a `instrumentRecords: { some: ... }` relation filter on Subject. + * On mongodb prisma compiles that into a $lookup over the whole record collection, which measured + * far slower than either form here and carries the 100 MiB per-document ceiling that made the same + * construct fail outright elsewhere. + */ + private async querySubjectIdsWithRecords(groupId?: string, ability?: AppAbility): Promise { + const groups = await this.prismaClient.instrumentRecord.groupBy({ + by: ['subjectId'], + where: { + AND: [accessibleQuery(ability, 'read', 'InstrumentRecord'), groupId ? { groupId } : {}] + } }); - return records.map((r) => r.subjectId); + return groups.map((group) => group.subjectId); } } From 8f2e1942ff4e666674eda72abb3614fe0d16480d Mon Sep 17 00:00:00 2001 From: "Gabriel A. Devenyi" Date: Mon, 27 Jul 2026 14:59:03 -0400 Subject: [PATCH 2/7] test(api): pin the has-record ability clause to accessibleQuery `not.toStrictEqual({})` did not prove the ability was applied: an unconditional read rule legitimately produces `{}`, so the assertion would have passed on a query that ignored the caller entirely. Constrain the ability with conditions and compare the clause against accessibleQuery directly. Verified by mutation: passing `undefined` in place of the ability now fails. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014ZEJLwa7jwD8sKzcE4PfSc --- .../src/subjects/__tests__/subjects.service.spec.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/apps/api/src/subjects/__tests__/subjects.service.spec.ts b/apps/api/src/subjects/__tests__/subjects.service.spec.ts index 2666b996c..bf6513684 100644 --- a/apps/api/src/subjects/__tests__/subjects.service.spec.ts +++ b/apps/api/src/subjects/__tests__/subjects.service.spec.ts @@ -168,17 +168,18 @@ describe('SubjectsService', () => { }) ); }); - it('should constrain the record query to what the caller may read', async () => { + it('should constrain the record query to what the caller may read, so the filter cannot be resolved from other groups records', async () => { prismaClient.instrumentRecord.groupBy.mockResolvedValueOnce([]); subjectModel.findMany.mockResolvedValueOnce([]); + // Conditions are what make this meaningful: an unconditional read rule yields `{}`, which is + // indistinguishable from the ability never having been applied. const ability = createAppAbility([ - { action: 'read', subject: 'InstrumentRecord' }, - { action: 'read', subject: 'Subject' } + { action: 'read', conditions: { groupId: { in: ['group-1'] } }, subject: 'InstrumentRecord' }, + { action: 'read', conditions: { groupIds: { hasSome: ['group-1'] } }, subject: 'Subject' } ]); await subjectsService.find({ hasRecord: true }, { ability }); const [call] = prismaClient.instrumentRecord.groupBy.mock.lastCall as [{ where: { AND: unknown[] } }]; - // the ability contributes the first clause; without it the query would be unconstrained - expect(call.where.AND[0]).not.toStrictEqual({}); + expect(call.where.AND[0]).toStrictEqual(accessibleQuery(ability, 'read', 'InstrumentRecord')); }); it('should pass all subject IDs returned by instrument records to the subject query', async () => { prismaClient.instrumentRecord.groupBy.mockResolvedValueOnce([{ subjectId: '123' }, { subjectId: '456' }]); From 53b54e75cc925684faf731d83003dddd69352167 Mon Sep 17 00:00:00 2001 From: "Gabriel A. Devenyi" Date: Tue, 28 Jul 2026 13:25:28 -0400 Subject: [PATCH 3/7] fix(api): answer the has-record filter for a caller who may read no records Applying the caller's ability to the record query introduced a new failure on this endpoint. `accessibleQuery` does not return an empty filter when the ability holds no rule for the subject at all -- only an `undefined` ability does that -- it throws a CASL ForbiddenError, which is not an HttpException, so libnest's filter renders it a 500. A STANDARD user holds `create` but not `read` on InstrumentRecord, and `GET /v1/subjects` is gated on `read Subject`, which they do hold. So `?hasRecord=true` returned an Internal Server Error where main returns 200. The web UI hides the datahub from those users, but the endpoint is documented and its guard admits them. Return an empty list for such a caller: the honest reading of "subjects whose records you can see". The scoping itself stays. Also from review: - a unit test for the no-rule caller, built through `AbilityFactory` with a STANDARD payload rather than by hand, since a hand-built ability tends to carry the very rule whose absence breaks this - e2e coverage in `datahub.spec.ts`, which had only asserted the page header: one case toggling "With records only" and asserting the table empties, one asserting the endpoint answers a standard user. Both verified by mutation - dropped `expect(findMany).not.toHaveBeenCalled()`, which pinned an implementation detail the `groupBy` assertion above it already covers The narrowing test needs to know exactly what its group holds, and the `roleAccount` group is shared by every spec in a worker, so this adds an `isolatedGroupManager` fixture that authenticates into a group of its own. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014ZEJLwa7jwD8sKzcE4PfSc --- .../__tests__/subjects.service.spec.ts | 23 ++++++++++- apps/api/src/subjects/subjects.service.ts | 7 ++++ apps/web/src/routes/_app/datahub/index.tsx | 7 +++- testing/src/pages/_app/datahub/index.page.ts | 10 +++++ testing/src/specs/datahub.spec.ts | 38 +++++++++++++++++++ testing/src/support/api-client.ts | 5 ++- 6 files changed, 86 insertions(+), 4 deletions(-) diff --git a/apps/api/src/subjects/__tests__/subjects.service.spec.ts b/apps/api/src/subjects/__tests__/subjects.service.spec.ts index bf6513684..0f0a8bd26 100644 --- a/apps/api/src/subjects/__tests__/subjects.service.spec.ts +++ b/apps/api/src/subjects/__tests__/subjects.service.spec.ts @@ -1,4 +1,4 @@ -import { CryptoService, getModelToken, PRISMA_CLIENT_TOKEN } from '@douglasneuroinformatics/libnest'; +import { CryptoService, getModelToken, LoggingService, PRISMA_CLIENT_TOKEN } from '@douglasneuroinformatics/libnest'; import type { Model } from '@douglasneuroinformatics/libnest'; import { MockFactory } from '@douglasneuroinformatics/libnest/testing'; import type { MockedInstance } from '@douglasneuroinformatics/libnest/testing'; @@ -8,6 +8,7 @@ import type { Subject } from '@prisma/client'; import { pick } from 'lodash-es'; import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { AbilityFactory } from '@/auth/ability.factory'; import { accessibleQuery, createAppAbility } from '@/auth/ability.utils'; import type { RuntimePrismaClient } from '@/core/prisma'; @@ -156,7 +157,6 @@ describe('SubjectsService', () => { expect(prismaClient.instrumentRecord.groupBy).toHaveBeenCalledWith( expect.objectContaining({ by: ['subjectId'] }) ); - expect(prismaClient.instrumentRecord.findMany).not.toHaveBeenCalled(); }); it('should filter instrument records by groupId when provided', async () => { prismaClient.instrumentRecord.groupBy.mockResolvedValueOnce([{ subjectId: '123' }]); @@ -181,6 +181,25 @@ describe('SubjectsService', () => { const [call] = prismaClient.instrumentRecord.groupBy.mock.lastCall as [{ where: { AND: unknown[] } }]; expect(call.where.AND[0]).toStrictEqual(accessibleQuery(ability, 'read', 'InstrumentRecord')); }); + + // A STANDARD user holds `create` but not `read` on InstrumentRecord, and this route's guard names + // `read Subject`, so they reach the service. `accessibleQuery` throws on an ability with no rule + // for the subject at all, which escapes as a 500 rather than the empty list they should see. + it('should return an empty list for a caller who may read no records, rather than throwing', async () => { + const abilityFactory = new AbilityFactory(MockFactory.createMock(LoggingService) as unknown as LoggingService); + const ability = abilityFactory.createForPayload({ + basePermissionLevel: 'STANDARD', + groups: [{ id: 'group-1' }], + id: 'user-1' + } as any); + subjectModel.findMany.mockResolvedValueOnce([]); + + await expect(subjectsService.find({ hasRecord: true }, { ability })).resolves.toStrictEqual([]); + + expect(prismaClient.instrumentRecord.groupBy).not.toHaveBeenCalled(); + const [call] = subjectModel.findMany.mock.lastCall as [{ where: { AND: unknown[] } }]; + expect(call.where.AND).toContainEqual({ id: { in: [] } }); + }); it('should pass all subject IDs returned by instrument records to the subject query', async () => { prismaClient.instrumentRecord.groupBy.mockResolvedValueOnce([{ subjectId: '123' }, { subjectId: '456' }]); subjectModel.findMany.mockResolvedValueOnce([{ id: '123' }, { id: '456' }]); diff --git a/apps/api/src/subjects/subjects.service.ts b/apps/api/src/subjects/subjects.service.ts index 9d90c0559..6f26d17e8 100644 --- a/apps/api/src/subjects/subjects.service.ts +++ b/apps/api/src/subjects/subjects.service.ts @@ -204,6 +204,13 @@ export class SubjectsService { * construct fail outright elsewhere. */ private async querySubjectIdsWithRecords(groupId?: string, ability?: AppAbility): Promise { + // `accessibleQuery` throws rather than returning a restrictive filter when the ability holds no + // rule for the subject at all, and a STANDARD user holds `create` but not `read` on + // InstrumentRecord. This route's guard names `read Subject`, so such a caller reaches here; no + // readable records means no subjects qualify. + if (ability && !ability.can('read', 'InstrumentRecord')) { + return []; + } const groups = await this.prismaClient.instrumentRecord.groupBy({ by: ['subjectId'], where: { diff --git a/apps/web/src/routes/_app/datahub/index.tsx b/apps/web/src/routes/_app/datahub/index.tsx index 8fdc2b0b6..4b2b785c1 100644 --- a/apps/web/src/routes/_app/datahub/index.tsx +++ b/apps/web/src/routes/_app/datahub/index.tsx @@ -60,7 +60,11 @@ const Filters: React.FC<{ return ( - @@ -170,6 +174,7 @@ const Filters: React.FC<{ e.preventDefault()} > diff --git a/testing/src/pages/_app/datahub/index.page.ts b/testing/src/pages/_app/datahub/index.page.ts index 1a87a5de4..e8e688c6d 100644 --- a/testing/src/pages/_app/datahub/index.page.ts +++ b/testing/src/pages/_app/datahub/index.page.ts @@ -4,6 +4,8 @@ import { AppPage } from '../route.page'; export class DatahubPage extends AppPage { readonly exportDropdown: Locator; + readonly filtersTrigger: Locator; + readonly hasRecordsFilter: Locator; readonly pageHeader: Locator; readonly rowActionsTrigger: Locator; readonly rows: Locator; @@ -11,6 +13,8 @@ export class DatahubPage extends AppPage { constructor(page: Page) { super(page); this.exportDropdown = page.getByTestId('datahub-export-dropdown'); + this.filtersTrigger = page.getByTestId('datahub-filters-trigger'); + this.hasRecordsFilter = page.getByTestId('datahub-filter-has-records'); this.pageHeader = page.getByTestId('page-header'); this.rowActionsTrigger = page.getByTestId('row-actions-trigger').first(); this.rows = page.getByTestId('data-table-body').getByTestId('data-table-row'); @@ -28,4 +32,10 @@ export class DatahubPage extends AppPage { await this.$ref.getByRole('menuitem', { exact: true, name: format }).click(); return started; } + + /** Opens the filter menu and toggles "With records only", which refetches with `hasRecord=true`. */ + async toggleWithRecordsOnly() { + await this.filtersTrigger.click(); + await this.hasRecordsFilter.click(); + } } diff --git a/testing/src/specs/datahub.spec.ts b/testing/src/specs/datahub.spec.ts index f39a3de68..462f1bb67 100644 --- a/testing/src/specs/datahub.spec.ts +++ b/testing/src/specs/datahub.spec.ts @@ -180,4 +180,42 @@ test.describe('data hub', () => { expect(payload.length).toBeGreaterThan(0); expect([...new Set(payload.map((row) => row.subjectId))]).toStrictEqual([listed]); }); + + test('should list only subjects holding records once "with records only" is applied', async ({ + api, + isolatedGroupManager, + page, + uniqueId + }) => { + // A group of its own, so the row count is exactly what this test seeds. Both subjects are created + // through a session and never given an instrument record, so the filter must empty the table. + const group = await isolatedGroupManager(); + for (const suffix of ['a', 'b']) { + await api.createSession(group.id, { id: `recordless-${uniqueId}-${suffix}` }); + } + + const datahubPage = new DatahubPage(page); + await datahubPage.goto('/datahub'); + await expect(datahubPage.rows).toHaveCount(2); + + await datahubPage.toggleWithRecordsOnly(); + + await expect(datahubPage.rows).toHaveCount(0); + }); + + // `GET /v1/subjects` is gated on `read Subject`, which a standard user holds, but resolving + // `hasRecord` reads instrument records, which they do not. The honest answer is an empty list. + test('should answer the with-records filter for a caller who may read no records', async ({ + apiRequestContext, + roleAccount + }) => { + const { accessToken } = await roleAccount('STANDARD'); + + const response = await apiRequestContext.get('/api/v1/subjects?hasRecord=true', { + headers: { Authorization: `Bearer ${accessToken}` } + }); + + expect(response.status()).toBe(200); + expect(await response.json()).toStrictEqual([]); + }); }); diff --git a/testing/src/support/api-client.ts b/testing/src/support/api-client.ts index 5ef6c67ab..69b567ce2 100644 --- a/testing/src/support/api-client.ts +++ b/testing/src/support/api-client.ts @@ -64,7 +64,10 @@ export class ApiClient { return group; } - /** Creates a session, and with it the subject it names. */ + /** + * Creates a session, and with it the subject it names. A subject seeded this way holds no + * instrument records, which is what distinguishes it under the "with records only" filter. + */ async createSession(groupId: null | string, subjectData: CreateSubjectData): Promise { const data: CreateSessionData = { date: new Date(), groupId, subjectData, type: 'IN_PERSON' }; return this.expectJson( From 9b0945b2ac548a3fb079c45891c2464b57ce2a19 Mon Sep 17 00:00:00 2001 From: "Gabriel A. Devenyi" Date: Fri, 11 Sep 2026 12:49:42 -0400 Subject: [PATCH 4/7] test(testing): keep a record-holding subject visible under the with-records filter The toggle test seeded only recordless subjects, so a filter that hid every subject unconditionally would have passed it. A third subject now holds an uploaded record and must be the one row that survives the toggle, pinning the filter in both directions. Co-Authored-By: Claude Fable 5 --- testing/src/specs/datahub.spec.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/testing/src/specs/datahub.spec.ts b/testing/src/specs/datahub.spec.ts index 462f1bb67..3deb89fe3 100644 --- a/testing/src/specs/datahub.spec.ts +++ b/testing/src/specs/datahub.spec.ts @@ -187,20 +187,28 @@ test.describe('data hub', () => { page, uniqueId }) => { - // A group of its own, so the row count is exactly what this test seeds. Both subjects are created - // through a session and never given an instrument record, so the filter must empty the table. + // A group of its own, so the row count is exactly what this test seeds. Two subjects exist only + // through sessions while a third holds a record, so the filter must drop exactly the recordless + // pair — hiding every subject or filtering none would both fail. const group = await isolatedGroupManager(); + const withRecord = `hasrecord-${uniqueId}`; for (const suffix of ['a', 'b']) { await api.createSession(group.id, { id: `recordless-${uniqueId}-${suffix}` }); } + await api.uploadRecords(group.id, await api.findInstrumentIdByName('DNP_HAPPINESS_QUESTIONNAIRE'), [ + { data: HAPPINESS_RECORD, date: new Date(), subjectId: withRecord } + ]); const datahubPage = new DatahubPage(page); await datahubPage.goto('/datahub'); - await expect(datahubPage.rows).toHaveCount(2); + await expect(datahubPage.rows).toHaveCount(3); await datahubPage.toggleWithRecordsOnly(); - await expect(datahubPage.rows).toHaveCount(0); + await expect(datahubPage.rows).toHaveCount(1); + // The cell renders at most the id's first nine characters (the subjectIdDisplayLength default), + // so the assertion matches the visible prefix rather than the full seeded id. + await expect(datahubPage.rows).toContainText(withRecord.slice(0, 9)); }); // `GET /v1/subjects` is gated on `read Subject`, which a standard user holds, but resolving From 2f6ab812ddbd78b9de5e2af027e9e6dd1c0d38b4 Mon Sep 17 00:00:00 2001 From: "Gabriel A. Devenyi" Date: Tue, 28 Jul 2026 18:08:03 -0400 Subject: [PATCH 5/7] docs(api): record that the record scoping clause drops ungrouped records A group manager's rule compiles to `groupId: { in: [...] }` and prisma's `in` never matches null, so admin-created records do not make their subject count as "with records" for a manager. Intended, but invisible at the call site. Co-Authored-By: Claude Fable 5 --- apps/api/src/subjects/subjects.service.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/api/src/subjects/subjects.service.ts b/apps/api/src/subjects/subjects.service.ts index 6f26d17e8..b3b1725f3 100644 --- a/apps/api/src/subjects/subjects.service.ts +++ b/apps/api/src/subjects/subjects.service.ts @@ -211,6 +211,10 @@ export class SubjectsService { if (ability && !ability.can('read', 'InstrumentRecord')) { return []; } + // The accessibleQuery clause also drops records whose groupId is null: a group manager's rule is + // `groupId: { in: [...] }`, and prisma's `in` never matches null. That is intended — an + // admin-created, ungrouped record should not make a subject count as "with records" for a + // manager — but it is invisible at this call site, so it is recorded here. const groups = await this.prismaClient.instrumentRecord.groupBy({ by: ['subjectId'], where: { From 9192c58347a14d339c91ae5321df0640e0c488f9 Mon Sep 17 00:00:00 2001 From: "Gabriel A. Devenyi" Date: Fri, 11 Sep 2026 14:05:01 -0400 Subject: [PATCH 6/7] test(api): build the standard caller from a complete token payload The payload was cast with `as any`, which hid required fields. One of them, mustResetPassword, changes which ability the factory builds. A complete payload keeps a later change to the token contract from weakening this test without a type error. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KLfKsdHT1arfrc2fdT1fWq --- .../src/subjects/__tests__/subjects.service.spec.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/apps/api/src/subjects/__tests__/subjects.service.spec.ts b/apps/api/src/subjects/__tests__/subjects.service.spec.ts index 0f0a8bd26..5d6c7545c 100644 --- a/apps/api/src/subjects/__tests__/subjects.service.spec.ts +++ b/apps/api/src/subjects/__tests__/subjects.service.spec.ts @@ -189,9 +189,14 @@ describe('SubjectsService', () => { const abilityFactory = new AbilityFactory(MockFactory.createMock(LoggingService) as unknown as LoggingService); const ability = abilityFactory.createForPayload({ basePermissionLevel: 'STANDARD', - groups: [{ id: 'group-1' }], - id: 'user-1' - } as any); + firstName: null, + groups: [], + id: 'user-1', + kind: 'login', + lastName: null, + mustResetPassword: false, + username: 'standard-user' + }); subjectModel.findMany.mockResolvedValueOnce([]); await expect(subjectsService.find({ hasRecord: true }, { ability })).resolves.toStrictEqual([]); From 804f7819230b2f9bdf07a6013b1d5cc3c61d586c Mon Sep 17 00:00:00 2001 From: "Gabriel A. Devenyi" Date: Fri, 11 Sep 2026 14:05:02 -0400 Subject: [PATCH 7/7] docs(api): fix the article before instrumentRecords Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KLfKsdHT1arfrc2fdT1fWq --- apps/api/src/subjects/subjects.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/api/src/subjects/subjects.service.ts b/apps/api/src/subjects/subjects.service.ts index b3b1725f3..f4d50936c 100644 --- a/apps/api/src/subjects/subjects.service.ts +++ b/apps/api/src/subjects/subjects.service.ts @@ -198,7 +198,7 @@ export class SubjectsService { * prisma query engine, so it returns one row per *record* over the wire and collapses them only * once they have arrived. * - * Deliberately not expressed as a `instrumentRecords: { some: ... }` relation filter on Subject. + * Deliberately not expressed as an `instrumentRecords: { some: ... }` relation filter on Subject. * On mongodb prisma compiles that into a $lookup over the whole record collection, which measured * far slower than either form here and carries the 100 MiB per-document ceiling that made the same * construct fail outright elsewhere.