diff --git a/apps/api/src/auth/ability.factory.ts b/apps/api/src/auth/ability.factory.ts index c0306d40a..ab5c6f4cd 100644 --- a/apps/api/src/auth/ability.factory.ts +++ b/apps/api/src/auth/ability.factory.ts @@ -27,6 +27,12 @@ export class AbilityFactory { ability.can('manage', 'Assignment', { groupId: { in: groupIds } }); ability.can('manage', 'Group', { id: { in: groupIds } }); ability.can('read', 'Instrument'); + // Group managers may assemble series instruments on the fly and delete ones they created. A + // series is a bundle of other instruments, not a shared platform asset. The delete ability is + // scoped to manually-created (non-repo) instruments so repo instruments shared across groups can + // never be removed; the service further restricts deletion to series instruments specifically. + ability.can('create', 'Instrument'); + ability.can('delete', 'Instrument', { sourceRepoId: null }); ability.can('read', 'InstrumentRepo', { groupIds: { hasSome: groupIds } }); ability.can('create', 'InstrumentRecord'); ability.can('create', 'InstrumentRecordFile', { groupId: { in: groupIds } }); diff --git a/apps/api/src/groups/__tests__/groups.service.spec.ts b/apps/api/src/groups/__tests__/groups.service.spec.ts index 3e6d4a1e1..072ca83b2 100644 --- a/apps/api/src/groups/__tests__/groups.service.spec.ts +++ b/apps/api/src/groups/__tests__/groups.service.spec.ts @@ -59,6 +59,20 @@ describe('GroupsService', () => { }); }); + it('should drop accessibleInstrumentIds that no longer exist before setting the relation', async () => { + groupModel.findFirst.mockResolvedValueOnce({ name: 'Test Group', settings: {} }); + // 'deleted' was removed since the client loaded the group and must not reach the relation set. + instrumentModel.findMany.mockResolvedValueOnce([{ id: 'live-1' }, { id: 'live-2' }]); + await groupsService.updateById('123', { accessibleInstrumentIds: ['live-1', 'deleted', 'live-2'] }); + expect(instrumentModel.findMany).toHaveBeenCalledWith({ + select: { id: true }, + where: { id: { in: ['live-1', 'deleted', 'live-2'] } } + }); + expect(groupModel.update.mock.lastCall?.[0]).toMatchObject({ + data: { accessibleInstruments: { set: [{ id: 'live-1' }, { id: 'live-2' }] } } + }); + }); + it('should not throw when the name is unchanged', async () => { groupModel.findFirst.mockResolvedValueOnce({ name: 'Test Group', settings: {} }); await groupsService.updateById('123', { name: 'Test Group' }); diff --git a/apps/api/src/groups/groups.service.ts b/apps/api/src/groups/groups.service.ts index bcceebb02..248544e5c 100644 --- a/apps/api/src/groups/groups.service.ts +++ b/apps/api/src/groups/groups.service.ts @@ -82,11 +82,23 @@ export class GroupsService { throw new ConflictException(`Group with name '${data.name}' already exists!`); } + // Guard against stale client state: an instrument may have been deleted since the client loaded the + // group (the deleted id can linger in the client's accessible list). Connecting a non-existent + // instrument makes Prisma fail the relation update, so restrict the set to ids that still exist. + let validInstrumentIds: string[] | undefined; + if (accessibleInstrumentIds) { + const existingInstruments = await this.instrumentModel.findMany({ + select: { id: true }, + where: { id: { in: accessibleInstrumentIds } } + }); + validInstrumentIds = existingInstruments.map(({ id }) => id); + } + return this.groupModel.update({ data: { - accessibleInstruments: accessibleInstrumentIds + accessibleInstruments: validInstrumentIds ? { - set: accessibleInstrumentIds.map((id) => ({ id })) + set: validInstrumentIds.map((id) => ({ id })) } : undefined, instrumentRepos: instrumentRepoIds diff --git a/apps/api/src/instruments/__tests__/instruments.service.spec.ts b/apps/api/src/instruments/__tests__/instruments.service.spec.ts index 00961a654..dc52ae06b 100644 --- a/apps/api/src/instruments/__tests__/instruments.service.spec.ts +++ b/apps/api/src/instruments/__tests__/instruments.service.spec.ts @@ -2,32 +2,291 @@ import { CryptoService, getModelToken, LoggingService, VirtualizationService } f import type { Model } from '@douglasneuroinformatics/libnest'; import { MockFactory } from '@douglasneuroinformatics/libnest/testing'; import type { MockedInstance } from '@douglasneuroinformatics/libnest/testing'; +import { ConflictException, ForbiddenException, NotFoundException } from '@nestjs/common'; import { Test } from '@nestjs/testing'; -import { beforeEach, describe, expect, it } from 'vitest'; +import { bundle } from '@opendatacapture/instrument-bundler'; +import type { SeriesInstrument } from '@opendatacapture/runtime-core'; +import type { WithID } from '@opendatacapture/schemas/core'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import { InstrumentsService } from '../instruments.service'; +// Avoid bundling a real instrument (esbuild) during unit tests; `createSeries` only needs `create` to be +// called with whatever the bundler produces. +vi.mock('@opendatacapture/instrument-bundler', () => ({ + bundle: vi.fn(() => Promise.resolve('__BUNDLE__')) +})); + +// A multilingual series instance as returned by `find`, containing two forms. +const existingSeries: WithID = { + __runtimeVersion: 1, + content: { + items: [ + { edition: 1, name: 'FORM_A' }, + { edition: 1, name: 'FORM_B' } + ] + }, + details: { + description: { en: 'An existing series', fr: 'Une série existante' }, + license: 'UNLICENSED', + title: { en: 'Existing Series', fr: 'Série existante' } + }, + id: 'existing-series-id', + kind: 'SERIES', + language: ['en', 'fr'], + tags: { en: ['Series'], fr: ['Série'] } +}; + describe('InstrumentsService', () => { let instrumentsService: InstrumentsService; + let cryptoService: MockedInstance; let instrumentModel: MockedInstance>; + let instrumentRecordModel: MockedInstance>; + let groupModel: MockedInstance>; + let virtualizationService: MockedInstance>; beforeEach(async () => { + vi.mocked(bundle).mockClear(); + const moduleRef = await Test.createTestingModule({ providers: [ InstrumentsService, MockFactory.createForModelToken(getModelToken('Group')), MockFactory.createForModelToken(getModelToken('Instrument')), + MockFactory.createForModelToken(getModelToken('InstrumentRecord')), MockFactory.createForService(CryptoService), MockFactory.createForService(LoggingService), MockFactory.createForService(VirtualizationService) ] }).compile(); instrumentsService = moduleRef.get(InstrumentsService); + cryptoService = moduleRef.get(CryptoService); instrumentModel = moduleRef.get(getModelToken('Instrument')); + instrumentRecordModel = moduleRef.get(getModelToken('InstrumentRecord')); + groupModel = moduleRef.get(getModelToken('Group')); + virtualizationService = moduleRef.get(VirtualizationService); + // `getInstrumentInstance` reads/writes an instance cache on the virtualization context. + (virtualizationService as { context: unknown }).context = { __resolveImport: vi.fn(), instruments: new Map() }; }); it('should be defined', () => { expect(instrumentsService).toBeDefined(); expect(instrumentModel).toBeDefined(); }); + + describe('createSeries', () => { + it('asks for confirmation (without creating) when another series already has the same forms', async () => { + vi.spyOn(instrumentsService, 'find').mockResolvedValue([existingSeries]); + const createSpy = vi.spyOn(instrumentsService, 'create'); + + // Same two forms as `existingSeries` but in a different order and under a different name. + const result = await instrumentsService.createSeries({ + details: { title: 'My New Series' }, + items: [ + { edition: 1, name: 'FORM_B' }, + { edition: 1, name: 'FORM_A' } + ], + language: 'en' + }); + + expect(result).toEqual({ + existingTitle: { en: 'Existing Series', fr: 'Série existante' }, + outcome: 'duplicate' + }); + expect(createSpy).not.toHaveBeenCalled(); + }); + + it('creates the series when the duplicate is explicitly confirmed', async () => { + vi.spyOn(instrumentsService, 'find').mockResolvedValue([existingSeries]); + const createSpy = vi.spyOn(instrumentsService, 'create').mockResolvedValue({ id: 'new-id' } as any); + + const result = await instrumentsService.createSeries({ + confirmDuplicate: true, + details: { title: 'My New Series' }, + items: [ + { edition: 1, name: 'FORM_A' }, + { edition: 1, name: 'FORM_B' } + ], + language: 'en' + }); + + expect(createSpy).toHaveBeenCalledWith({ bundle: '__BUNDLE__' }); + expect(result).toEqual({ instrumentId: 'new-id', outcome: 'created' }); + }); + + it('creates the series directly when no existing series shares its forms', async () => { + vi.spyOn(instrumentsService, 'find').mockResolvedValue([existingSeries]); + const createSpy = vi.spyOn(instrumentsService, 'create').mockResolvedValue({ id: 'fresh-id' } as any); + + const result = await instrumentsService.createSeries({ + details: { title: 'Totally New' }, + items: [ + { edition: 1, name: 'FORM_C' }, + { edition: 1, name: 'FORM_D' } + ], + language: 'en' + }); + + expect(createSpy).toHaveBeenCalledWith({ bundle: '__BUNDLE__' }); + expect(result).toEqual({ instrumentId: 'fresh-id', outcome: 'created' }); + }); + + it('generates a valid, unilingual series source payload with selected items in order', async () => { + vi.spyOn(instrumentsService, 'find').mockResolvedValue([]); + vi.spyOn(instrumentsService, 'create').mockResolvedValue({ id: 'generated-id' } as any); + + await instrumentsService.createSeries({ + clientDetails: { instructions: ['Complete the instruments in order.'] }, + details: { description: 'Optional description', title: 'Generated Series' }, + items: [ + { edition: 2, name: 'FORM_B' }, + { edition: 1, name: 'FORM_A' } + ], + language: 'en' + }); + + expect(bundle).toHaveBeenCalledTimes(1); + + const [bundleOptions] = vi.mocked(bundle).mock.calls[0]!; + expect(bundleOptions.minify).toBe(true); + expect(bundleOptions.inputs).toHaveLength(1); + + expect(bundleOptions.inputs[0]!.content).toEqual(expect.any(String)); + const source = bundleOptions.inputs[0]!.content as string; + const match = /^export default (.*);$/.exec(source); + expect(match).not.toBeNull(); + + const definition = JSON.parse(match![1]!); + expect(definition.kind).toBe('SERIES'); + // The details/instructions/language are stored verbatim in the caller's language — nothing is + // duplicated across languages or hardcoded. + expect(definition.language).toBe('en'); + expect(definition.clientDetails).toEqual({ instructions: ['Complete the instruments in order.'] }); + expect(definition.details).toMatchObject({ + description: 'Optional description', + title: 'Generated Series' + }); + expect(definition.tags).toEqual(['Series']); + expect(definition.content.items).toEqual([ + { edition: 2, name: 'FORM_B' }, + { edition: 1, name: 'FORM_A' } + ]); + }); + + it('runs the generated bundle through create and stores the validated series instrument', async () => { + const items = [ + { edition: 1, name: 'FORM_A' }, + { edition: 1, name: 'FORM_B' } + ]; + const instance = { + __runtimeVersion: 1, + content: { items }, + details: { + description: 'Stored Series', + license: 'UNLICENSED', + title: 'Stored Series' + }, + kind: 'SERIES', + language: 'en', + tags: ['Series'] + } as const; + const id = `hash:${JSON.stringify({ content: instance.content, title: instance.details.title })}`; + + vi.spyOn(instrumentsService, 'find').mockResolvedValue([]); + vi.spyOn(cryptoService, 'hash').mockImplementation((value) => `hash:${value}`); + virtualizationService.eval.mockResolvedValue({ isErr: () => false, value: instance } as any); + instrumentModel.exists.mockResolvedValueOnce(false).mockResolvedValue(true); + + const result = await instrumentsService.createSeries({ + confirmDuplicate: true, + details: { title: 'Stored Series' }, + items, + language: 'en' + }); + + expect(virtualizationService.eval).toHaveBeenCalledWith('__BUNDLE__'); + expect(instrumentModel.exists).toHaveBeenNthCalledWith(1, { id }); + expect(instrumentModel.exists).toHaveBeenNthCalledWith(2, { id: 'hash:FORM_A-1' }); + expect(instrumentModel.exists).toHaveBeenNthCalledWith(3, { id: 'hash:FORM_B-1' }); + expect(instrumentModel.create).toHaveBeenCalledWith({ data: { bundle: '__BUNDLE__', id } }); + expect(result).toEqual({ instrumentId: id, outcome: 'created' }); + }); + + it('rejects a title that is already used (case-insensitively) by another instrument', async () => { + vi.spyOn(instrumentsService, 'find').mockResolvedValue([existingSeries]); + const createSpy = vi.spyOn(instrumentsService, 'create'); + + await expect( + instrumentsService.createSeries({ + details: { title: 'existing series' }, + items: [ + { edition: 1, name: 'FORM_X' }, + { edition: 1, name: 'FORM_Y' } + ], + language: 'en' + }) + ).rejects.toThrow(ConflictException); + expect(createSpy).not.toHaveBeenCalled(); + }); + }); + + describe('generateSeriesInstrumentId', () => { + it('includes the series title so confirmed duplicate form sets can be distinct', () => { + vi.spyOn(cryptoService, 'hash').mockImplementation((value) => value); + + const first = { + ...existingSeries, + details: { ...existingSeries.details, title: 'First Series' } + }; + const second = { + ...existingSeries, + details: { ...existingSeries.details, title: 'Second Series' } + }; + + expect(instrumentsService.generateSeriesInstrumentId(first)).not.toBe( + instrumentsService.generateSeriesInstrumentId(second) + ); + expect(cryptoService.hash).toHaveBeenCalledWith( + JSON.stringify({ content: first.content, title: first.details.title }) + ); + }); + }); + + describe('deleteById', () => { + it('throws when the instrument does not exist', async () => { + instrumentModel.findFirst.mockResolvedValue(null); + await expect(instrumentsService.deleteById('missing')).rejects.toThrow(NotFoundException); + }); + + it('refuses to delete a non-series (scalar) instrument', async () => { + instrumentModel.findFirst.mockResolvedValue({ bundle: '__BUNDLE__', id: 'scalar' }); + virtualizationService.eval.mockResolvedValue({ + isErr: () => false, + value: { internal: { edition: 1, name: 'FORM_A' }, kind: 'FORM' } + } as any); + + await expect(instrumentsService.deleteById('scalar')).rejects.toThrow(ForbiddenException); + expect(instrumentModel.delete).not.toHaveBeenCalled(); + }); + + it('deletes a series instrument and detaches it from every group', async () => { + instrumentModel.findFirst.mockResolvedValue({ bundle: '__BUNDLE__', id: 'target' }); + virtualizationService.eval.mockResolvedValue({ + isErr: () => false, + value: { content: { items: [] }, kind: 'SERIES' } + } as any); + groupModel.findMany.mockResolvedValue([{ accessibleInstrumentIds: ['other', 'target'], id: 'g1' }]); + + const result = await instrumentsService.deleteById('target'); + + expect(groupModel.update).toHaveBeenCalledWith({ + data: { accessibleInstrumentIds: { set: ['other'] } }, + where: { id: 'g1' } + }); + expect(instrumentModel.delete).toHaveBeenCalledWith({ where: { id: 'target' } }); + expect(result).toEqual({ id: 'target' }); + // Series instruments never have records of their own, so record counts are irrelevant here. + expect(instrumentRecordModel.count).not.toHaveBeenCalled(); + }); + }); }); diff --git a/apps/api/src/instruments/instruments.controller.ts b/apps/api/src/instruments/instruments.controller.ts index a5309a6ac..6efc2085e 100644 --- a/apps/api/src/instruments/instruments.controller.ts +++ b/apps/api/src/instruments/instruments.controller.ts @@ -1,8 +1,15 @@ import { CurrentUser } from '@douglasneuroinformatics/libnest'; -import { Body, Controller, Get, Param, Post, Query } from '@nestjs/common'; +import { Body, Controller, Delete, Get, Param, Post, Query } from '@nestjs/common'; import { ApiOperation, ApiTags } from '@nestjs/swagger'; import type { InstrumentKind } from '@opendatacapture/runtime-core'; -import type { InstrumentBundleContainer, InstrumentInfo } from '@opendatacapture/schemas/instrument'; +// Imported as a value (not a type-only import) so it doubles as the validation schema for the request +// body while also annotating its type — no dedicated DTO class is needed. +import { $CreateSeriesInstrumentData } from '@opendatacapture/schemas/instrument'; +import type { + CreateSeriesInstrumentResult, + InstrumentBundleContainer, + InstrumentInfo +} from '@opendatacapture/schemas/instrument'; import type { AppAbility } from '@/auth/auth.types'; import { RouteAccess } from '@/core/decorators/route-access.decorator'; @@ -22,6 +29,23 @@ export class InstrumentsController { return this.instrumentsService.create(data); } + @ApiOperation({ summary: 'Create Series Instrument' }) + @Post('series') + @RouteAccess({ action: 'create', subject: 'Instrument' }) + createSeries( + @Body() data: $CreateSeriesInstrumentData, + @CurrentUser('ability') ability: AppAbility + ): Promise { + return this.instrumentsService.createSeries(data, { ability }); + } + + @ApiOperation({ summary: 'Delete Instrument' }) + @Delete(':id') + @RouteAccess({ action: 'delete', subject: 'Instrument' }) + delete(@Param('id') id: string, @CurrentUser('ability') ability: AppAbility): Promise { + return this.instrumentsService.deleteById(id, { ability }); + } + @ApiOperation({ summary: 'Get Instrument Bundle' }) @Get('bundle/:id') @RouteAccess({ action: 'read', subject: 'Instrument' }) diff --git a/apps/api/src/instruments/instruments.service.ts b/apps/api/src/instruments/instruments.service.ts index 7c6a4b92d..a8c7cfa0b 100644 --- a/apps/api/src/instruments/instruments.service.ts +++ b/apps/api/src/instruments/instruments.service.ts @@ -2,25 +2,36 @@ import { CryptoService, InjectModel, LoggingService, VirtualizationService } fro import type { Model } from '@douglasneuroinformatics/libnest'; import { ConflictException, + ForbiddenException, Injectable, InternalServerErrorException, NotFoundException, UnprocessableEntityException } from '@nestjs/common'; +import { bundle } from '@opendatacapture/instrument-bundler'; import { getSeriesInstrumentItems, isScalarInstrument, isSeriesInstrument } from '@opendatacapture/instrument-utils'; import type { AnyInstrument, AnyScalarInstrument, + ClientInstrumentDetails, InstrumentKind, + InstrumentLanguage, + InstrumentUIOption, + Language, + ScalarInstrumentInternal, SeriesInstrument, SomeInstrument } from '@opendatacapture/runtime-core'; import type { WithID } from '@opendatacapture/schemas/core'; import { $AnyInstrument } from '@opendatacapture/schemas/instrument'; import type { + $CreateSeriesInstrumentData, + CreateSeriesInstrumentResult, InstrumentBundleContainer, InstrumentInfo, - ScalarInstrumentBundleContainer + ScalarInstrumentBundleContainer, + ScalarInstrumentInfo, + SeriesInstrumentInfo } from '@opendatacapture/schemas/instrument'; import { pick } from 'lodash-es'; @@ -30,6 +41,9 @@ import type { EntityOperationOptions } from '@/core/types'; import { CreateInstrumentDto } from './dto/create-instrument.dto'; +/** The localized "series" tag applied to every generated series instrument. */ +const seriesTag = (language: Language): string => (language === 'fr' ? 'Série' : 'Series'); + type InstrumentVirtualizationContext = { __resolveImport: (specifier: string) => string; instruments: Map>; @@ -45,6 +59,7 @@ export class InstrumentsService { constructor( @InjectModel('Group') private readonly groupModel: Model<'Group'>, @InjectModel('Instrument') private readonly instrumentModel: Model<'Instrument'>, + @InjectModel('InstrumentRecord') private readonly instrumentRecordModel: Model<'InstrumentRecord'>, private readonly cryptoService: CryptoService, private readonly loggingService: LoggingService, private readonly virtualizationService: VirtualizationService @@ -106,6 +121,69 @@ export class InstrumentsService { return { ...instance, id }; } + /** + * Assemble a new series instrument from existing scalar instruments. The title must be unique among + * every instrument on the platform; the series bundle is generated server-side and run through the + * same validation/creation path as any other instrument. + * + * If another series already contains the exact same set of items (regardless of order or name) and the + * caller has not opted to proceed, no instrument is created — instead a `duplicate` result is returned + * with the existing series' title, so the client can ask the user whether they really want a duplicate. + */ + async createSeries( + { clientDetails, confirmDuplicate, details, items, language }: $CreateSeriesInstrumentData, + options: EntityOperationOptions = {} + ): Promise { + if (await this.isInstrumentTitleTaken(details.title)) { + throw new ConflictException('An instrument with the same title already exists'); + } + if (!confirmDuplicate) { + const existingTitle = await this.findSeriesTitleWithSameItems(items, options); + if (existingTitle !== null) { + return { existingTitle, outcome: 'duplicate' }; + } + } + const source = this.generateSeriesInstrumentSource({ clientDetails, details, items, language }); + const generatedBundle = await bundle({ inputs: [{ content: source, name: 'index.ts' }], minify: true }); + const created = await this.create({ bundle: generatedBundle }); + return { instrumentId: created.id, outcome: 'created' }; + } + + /** + * Delete a series instrument the caller is permitted to remove, detaching it from any group that + * exposes it. Only series instruments may be deleted: they are on-the-fly bundles of other + * instruments and never have records of their own (records belong to their constituent members). + * Scalar instruments are shared platform assets and are never removed through this path. + */ + async deleteById(id: string, { ability }: EntityOperationOptions = {}): Promise<{ id: string }> { + const instrument = await this.instrumentModel.findFirst({ + where: { AND: [accessibleQuery(ability, 'delete', 'Instrument')], id } + }); + if (!instrument) { + throw new NotFoundException(`Failed to find instrument with ID: ${id}`); + } + const instance = await this.getInstrumentInstance(instrument); + if (!isSeriesInstrument(instance)) { + throw new ForbiddenException('Only series instruments can be deleted'); + } + // Mongo does not cascade scalar-list relations, so pull the id from every group that exposes it + // before removing the instrument itself. + const groups = await this.groupModel.findMany({ + select: { accessibleInstrumentIds: true, id: true }, + where: { accessibleInstrumentIds: { has: id } } + }); + await Promise.all( + groups.map((group) => + this.groupModel.update({ + data: { accessibleInstrumentIds: { set: group.accessibleInstrumentIds.filter((value) => value !== id) } }, + where: { id: group.id } + }) + ) + ); + await this.instrumentModel.delete({ where: { id } }); + return { id }; + } + async find( query: InstrumentQuery = {}, { ability }: EntityOperationOptions = {} @@ -178,35 +256,50 @@ export class InstrumentsService { const instances = await this.find(query, options); const sourceMap = await this.buildInstrumentSourceMap(instances.map((instance) => instance.id)); + // Series resolve their `seriesItems` against the scalar instruments they reference. A `kind` filter can + // exclude those scalars from `instances`, so when the result set contains series we build the lookup + // from the full instrument set instead — otherwise a series' items would resolve to nothing. + const scalarSource = query.kind && instances.some(isSeriesInstrument) ? await this.find({}, options) : instances; + const scalarInstrumentIds = new Map( + scalarSource.flatMap((instance) => + isScalarInstrument(instance) && instance.internal + ? [[`${instance.internal.name}:${instance.internal.edition}`, instance.id] as const] + : [] + ) + ); const results = new Map(); for (const instance of instances) { - const info = pick(instance, [ - '__runtimeVersion', - 'clientDetails', - 'details', - 'id', - 'internal', - 'kind', - 'language', - 'tags' - ]); + const base = pick(instance, ['__runtimeVersion', 'clientDetails', 'details', 'id', 'language', 'tags']); // Expose the source repo id whenever the instrument came from a repo (so it can be filtered per // group). The name may be null for legacy instruments imported before names were stored; the - // client renders those as "uploaded manually" while still treating them as repo-sourced. - const source = sourceMap.get(info.id); - if (source?.sourceRepoId) { - (info as InstrumentInfo).sourceRepo = { - id: source.sourceRepoId, - name: source.sourceRepoName ?? null + // client still treats those as repo-sourced via their id. + const source = sourceMap.get(instance.id); + const sourceRepo = source?.sourceRepoId ? { id: source.sourceRepoId, name: source.sourceRepoName ?? null } : null; + + if (isSeriesInstrument(instance)) { + const info: SeriesInstrumentInfo = { + ...base, + kind: 'SERIES', + seriesItems: getSeriesInstrumentItems(instance.content) + .map(({ edition, name }) => scalarInstrumentIds.get(`${name}:${edition}`)) + .filter((id): id is string => typeof id === 'string') + .map((id) => ({ id })), + sourceRepo }; - } - if (!info.internal) { results.set(info.id, info); continue; } + + const info: ScalarInstrumentInfo = { + ...base, + internal: instance.internal, + kind: instance.kind, + sourceRepo + }; + // Collapse editions of the same instrument to the newest one, keyed by its stable internal name. const currentEntry = results.get(info.internal.name); - if (!currentEntry || info.internal.edition > currentEntry.internal!.edition) { + if (!currentEntry || !('internal' in currentEntry) || info.internal.edition > currentEntry.internal.edition) { results.set(info.internal.name, info); } } @@ -226,9 +319,10 @@ export class InstrumentsService { generateSeriesInstrumentId(instrument: SeriesInstrument) { return this.cryptoService.hash( - getSeriesInstrumentItems(instrument.content) - .map(({ edition, name }) => `${name}-${edition}`) - .join('--') + JSON.stringify({ + content: instrument.content, + title: instrument.details.title + }) ); } @@ -281,6 +375,82 @@ export class InstrumentsService { return map; } + /** The localized `tags` for a series, matching the shape (unilingual vs multilingual) of `language`. */ + private buildSeriesTags(language: InstrumentLanguage): InstrumentUIOption { + if (typeof language === 'string') { + return [seriesTag(language)]; + } + return Object.fromEntries(language.map((lang) => [lang, [seriesTag(lang)]])) as InstrumentUIOption< + InstrumentLanguage, + string[] + >; + } + + /** + * The title of an existing series instrument that contains the exact same set of items as `items` + * (regardless of order), or `null` when none exists. Used to warn before creating a redundant series. + * The title is returned in its stored (possibly multilingual) form so the client can localize it. + */ + private async findSeriesTitleWithSameItems( + items: ScalarInstrumentInternal[], + { ability }: EntityOperationOptions = {} + ): Promise | null> { + const targetKey = this.getSeriesItemSetKey(items); + const seriesInstances = await this.find({ kind: 'SERIES' }, { ability }); + for (const series of seriesInstances) { + if (!isSeriesInstrument(series)) { + continue; + } + if (this.getSeriesItemSetKey(getSeriesInstrumentItems(series.content)) === targetKey) { + return series.details.title; + } + } + return null; + } + + /** + * Generate the TypeScript source for an on-the-fly series instrument. The definition is plain + * JSON-serializable data, so it is emitted as a single default export with no runtime imports. The + * multilingual details/instructions and the language(s) are taken verbatim from the caller — nothing + * is hardcoded — so the series is stored exactly as it was authored. + */ + private generateSeriesInstrumentSource({ + clientDetails, + details, + items, + language + }: Pick<$CreateSeriesInstrumentData, 'clientDetails' | 'details' | 'items' | 'language'>): string { + // A series is a `SeriesInstrument` minus the server-computed `id`; typing it explicitly keeps this + // in sync with the schema so a change to the instrument shape fails to compile here. + const definition: Omit = { + __runtimeVersion: 1, + ...(clientDetails?.instructions ? { clientDetails: { instructions: clientDetails.instructions } } : {}), + content: { + items: items.map(({ edition, name }) => ({ edition, name })) + }, + details: { + description: details.description ?? details.title, + license: 'UNLICENSED', + title: details.title + }, + kind: 'SERIES', + language, + tags: this.buildSeriesTags(language) + }; + return `export default ${JSON.stringify(definition)};`; + } + + /** + * A stable, order-independent key identifying the set of scalar instruments in a series. Two series + * with the same items produce the same key even if the items were added in a different order. + */ + private getSeriesItemSetKey(items: ScalarInstrumentInternal[]): string { + return items + .map(({ edition, name }) => `${name}-${edition}`) + .sort() + .join('--'); + } + private async instantiate(instruments: Pick[]) { return Promise.all( instruments.map((instrument) => { @@ -289,6 +459,26 @@ export class InstrumentsService { ); } + /** + * Whether any instrument on the platform already uses the given title (case-insensitive, across every + * language it is defined in). Used to enforce that newly-created series have a unique, recognizable + * name. Deliberately unscoped by ability: uniqueness must hold globally, not just for instruments the + * caller happens to see. + */ + private async isInstrumentTitleTaken(title: NonNullable): Promise { + const normalize = (value: string) => value.trim().toLowerCase(); + const candidates = new Set(this.titleValues(title).map(normalize)); + const instances = await this.find({}); + return instances.some((instance) => + this.titleValues(instance.details.title).some((value) => candidates.has(normalize(value))) + ); + } + + /** The individual language values of a (possibly multilingual) title. */ + private titleValues(title: InstrumentUIOption): string[] { + return typeof title === 'string' ? [title] : Object.values(title); + } + private async validateSeriesInstrument(instrument: SeriesInstrument) { const items = getSeriesInstrumentItems(instrument.content); if (items.length < 2) { diff --git a/apps/web/src/components/InstrumentCard/InstrumentCard.tsx b/apps/web/src/components/InstrumentCard/InstrumentCard.tsx index d2cadd379..cf865f71a 100644 --- a/apps/web/src/components/InstrumentCard/InstrumentCard.tsx +++ b/apps/web/src/components/InstrumentCard/InstrumentCard.tsx @@ -48,7 +48,7 @@ export const InstrumentCard = ({ instrument, onClick }: InstrumentCardProps) => en: 'Edition', fr: 'Édition' }), - text: instrument.internal?.edition.toString() + text: instrument.kind === 'SERIES' ? undefined : instrument.internal.edition.toString() }, { kind: 'text', diff --git a/apps/web/src/components/InstrumentShowcase/InstrumentShowcase.stories.tsx b/apps/web/src/components/InstrumentShowcase/InstrumentShowcase.stories.tsx index 6e8a19f6c..f633e4eb1 100644 --- a/apps/web/src/components/InstrumentShowcase/InstrumentShowcase.stories.tsx +++ b/apps/web/src/components/InstrumentShowcase/InstrumentShowcase.stories.tsx @@ -13,7 +13,9 @@ export default { component: InstrumentShowcase } as Meta - translateInstrumentInfo(instance, 'en') + // The stubs are full instruments; `translateInstrumentInfo` only reads the fields common to every + // `InstrumentInfo`, so a cast to its parameter type is sufficient for the story. + translateInstrumentInfo(instance as unknown as Parameters[0], 'en') ) } }; diff --git a/apps/web/src/hooks/useCreateSeriesInstrumentMutation.ts b/apps/web/src/hooks/useCreateSeriesInstrumentMutation.ts new file mode 100644 index 000000000..3f16e653b --- /dev/null +++ b/apps/web/src/hooks/useCreateSeriesInstrumentMutation.ts @@ -0,0 +1,41 @@ +import { useNotificationsStore, useTranslation } from '@douglasneuroinformatics/libui/hooks'; +import type { $CreateSeriesInstrumentData, CreateSeriesInstrumentResult } from '@opendatacapture/schemas/instrument'; +import { useMutation, useQueryClient } from '@tanstack/react-query'; +import axios from 'axios'; + +import { getApiErrorMessage } from '@/utils/error'; + +// The request/response contract is shared with the server (`@opendatacapture/schemas/instrument`) so the +// two cannot drift: `$CreateSeriesInstrumentData` is the request body and `CreateSeriesInstrumentResult` +// is a discriminated union over `outcome` (either the series was created, or the caller must confirm a +// duplicate before it is). +export function useCreateSeriesInstrumentMutation() { + const queryClient = useQueryClient(); + const addNotification = useNotificationsStore((store) => store.addNotification); + const { t } = useTranslation(); + return useMutation({ + mutationFn: async (data: $CreateSeriesInstrumentData) => { + const response = await axios.post('/v1/instruments/series', data, { + meta: { disableDefaultErrorNotification: true } + }); + return response.data; + }, + onError(err) { + addNotification({ + message: getApiErrorMessage( + err, + t({ en: 'Failed to create series instrument', fr: "Échec de la création de l'instrument en série" }) + ), + type: 'error' + }); + }, + onSuccess(data) { + // A duplicate prompt is not a creation: do not announce success or refresh the list yet. + if (data.outcome === 'duplicate') { + return; + } + addNotification({ type: 'success' }); + void queryClient.invalidateQueries({ queryKey: ['instrument-info'] }); + } + }); +} diff --git a/apps/web/src/hooks/useDeleteInstrumentMutation.ts b/apps/web/src/hooks/useDeleteInstrumentMutation.ts new file mode 100644 index 000000000..ca1099a84 --- /dev/null +++ b/apps/web/src/hooks/useDeleteInstrumentMutation.ts @@ -0,0 +1,30 @@ +import { useNotificationsStore, useTranslation } from '@douglasneuroinformatics/libui/hooks'; +import { useMutation, useQueryClient } from '@tanstack/react-query'; +import axios from 'axios'; + +import { getApiErrorMessage } from '@/utils/error'; + +export function useDeleteInstrumentMutation() { + const queryClient = useQueryClient(); + const addNotification = useNotificationsStore((store) => store.addNotification); + const { t } = useTranslation(); + return useMutation({ + mutationFn: ({ id }: { id: string }) => + axios.delete(`/v1/instruments/${id}`, { meta: { disableDefaultErrorNotification: true } }), + onError(err) { + addNotification({ + message: getApiErrorMessage( + err, + t({ en: 'Failed to delete instrument', fr: "Échec de la suppression de l'instrument" }) + ), + type: 'error' + }); + }, + onSuccess() { + addNotification({ type: 'success' }); + // The server has already detached the instrument from every group; callers that hold a stale copy + // of the group's accessible ids (e.g. the manage page) reconcile it locally via the mutation result. + void queryClient.invalidateQueries({ queryKey: ['instrument-info'] }); + } + }); +} diff --git a/apps/web/src/routes/_app/admin/instrument-repos/index.tsx b/apps/web/src/routes/_app/admin/instrument-repos/index.tsx index 5e2e8644b..6303d6376 100644 --- a/apps/web/src/routes/_app/admin/instrument-repos/index.tsx +++ b/apps/web/src/routes/_app/admin/instrument-repos/index.tsx @@ -317,7 +317,9 @@ const RouteComponent = () => { {instrument.details.title}

{instrument.kind}

-

{instrument.internal?.edition ?? '-'}

+

+ {instrument.kind === 'SERIES' ? '-' : instrument.internal.edition} +

))} diff --git a/apps/web/src/routes/_app/group/manage.tsx b/apps/web/src/routes/_app/group/manage.tsx index 543894a0b..6161ed89b 100644 --- a/apps/web/src/routes/_app/group/manage.tsx +++ b/apps/web/src/routes/_app/group/manage.tsx @@ -7,22 +7,28 @@ import { Dialog, Form, Heading, + Input, SearchBar, - Spinner + Spinner, + TextArea } from '@douglasneuroinformatics/libui/components'; import { useTranslation } from '@douglasneuroinformatics/libui/hooks'; import { InstrumentRenderer } from '@opendatacapture/react-core'; +import type { ScalarInstrumentInternal } from '@opendatacapture/runtime-core'; import { $RegexString } from '@opendatacapture/schemas/core'; import type { UpdateGroupData } from '@opendatacapture/schemas/group'; +import type { $CreateSeriesInstrumentData, InstrumentBundleContainer } from '@opendatacapture/schemas/instrument'; import { $SubjectIdentificationMethod } from '@opendatacapture/schemas/subject'; import type { SubjectIdentificationMethod } from '@opendatacapture/schemas/subject'; import { createFileRoute } from '@tanstack/react-router'; -import { EyeIcon } from 'lucide-react'; +import { EyeIcon, TrashIcon } from 'lucide-react'; import type { Promisable } from 'type-fest'; import { z } from 'zod/v4'; import { PageHeader } from '@/components/PageHeader'; import { WithFallback } from '@/components/WithFallback'; +import { useCreateSeriesInstrumentMutation } from '@/hooks/useCreateSeriesInstrumentMutation'; +import { useDeleteInstrumentMutation } from '@/hooks/useDeleteInstrumentMutation'; import { useInstrumentBundle } from '@/hooks/useInstrumentBundle'; import { useInstrumentInfoQuery } from '@/hooks/useInstrumentInfoQuery'; import { useSetupStateQuery } from '@/hooks/useSetupStateQuery'; @@ -35,7 +41,10 @@ type InstrumentItem = { authors?: null | string[]; description?: string; id: string; + // The scalar instrument identity (name + edition); null for series instruments, which have no edition. + internal: null | ScalarInstrumentInternal; kind: string; + seriesItems?: { id: string }[]; source: InstrumentSource; title: string; }; @@ -46,6 +55,36 @@ type CategorizedInstruments = { series: InstrumentItem[]; }; +const getSeriesPreviewItemTitles = ({ + bundle, + fallbackTitle, + items +}: { + bundle?: InstrumentBundleContainer; + fallbackTitle: (index: number) => string; + items: InstrumentItem[]; +}) => { + if (bundle?.kind !== 'SERIES') { + return []; + } + return bundle.items.map((seriesItem, index) => { + return items.find((item) => item.id === seriesItem.id)?.title ?? fallbackTitle(index); + }); +}; + +const expandSelectedSeriesIds = ({ selectedIds, series }: { selectedIds: Set; series: InstrumentItem[] }) => { + const expandedIds = new Set(selectedIds); + for (const item of series) { + if (!selectedIds.has(item.id)) { + continue; + } + for (const seriesItem of item.seriesItems ?? []) { + expandedIds.add(seriesItem.id); + } + } + return expandedIds; +}; + type SettingsValues = { defaultIdentificationMethod?: SubjectIdentificationMethod; idValidationRegex?: null | string; @@ -58,6 +97,8 @@ type SettingsValues = { type ManageGroupFormProps = { data: { + // The titles of every visible instrument, used to enforce unique new series instrument names. + existingTitles: string[]; // Accessible instrument ids that are not in the visible list; preserved as-is on save. hiddenAccessibleIds: string[]; initialSelectedIds: string[]; @@ -70,6 +111,7 @@ type ManageGroupFormProps = { const InstrumentSection = ({ items, + onDelete, onPreview, onToggle, readOnly, @@ -78,6 +120,8 @@ const InstrumentSection = ({ title }: { items: InstrumentItem[]; + // When provided, a delete affordance is shown for items where it returns true. + onDelete?: (item: InstrumentItem) => void; onPreview: (item: InstrumentItem) => void; onToggle: (id: string) => void; readOnly: boolean; @@ -94,7 +138,7 @@ const InstrumentSection = ({ return (
-

{title}

+ {title &&

{title}

} {filtered.length === 0 ? (

{t({ en: 'No instruments available.', fr: 'Aucun instrument disponible.' })} @@ -111,25 +155,44 @@ const InstrumentSection = ({ disabled={readOnly} onCheckedChange={() => onToggle(item.id)} /> - - {item.title} - - - {item.source.kind === 'repo' - ? item.source.name - : t({ en: 'Uploaded manually', fr: 'Téléversé manuellement' })} - + + {item.source.kind === 'repo' ? item.source.name : t({ en: 'No repo', fr: 'Aucun dépôt' })} + +

+ + {onDelete && !readOnly && item.source.kind === 'manual' && ( + + )} +
))} @@ -138,10 +201,27 @@ const InstrumentSection = ({ ); }; -const InstrumentPreviewDialog = ({ item, onClose }: { item: InstrumentItem; onClose: () => void }) => { +const InstrumentPreviewDialog = ({ + item, + items, + onClose +}: { + item: InstrumentItem; + items: InstrumentItem[]; + onClose: () => void; +}) => { const { t } = useTranslation(); const [showForm, setShowForm] = useState(false); - const bundleQuery = useInstrumentBundle(showForm ? item.id : null); + const bundleQuery = useInstrumentBundle(showForm || item.kind === 'SERIES' ? item.id : null); + // Passed to the renderer as a localizable value; the shared component resolves it to the active language. + const previewSubmitLabel = { en: 'Preview Submit', fr: 'Soumettre l’aperçu' }; + const seriesItemTitles = useMemo(() => { + return getSeriesPreviewItemTitles({ + bundle: bundleQuery.data, + fallbackTitle: (index) => t({ en: `Item ${index + 1}`, fr: `Élément ${index + 1}` }), + items + }); + }, [bundleQuery.data, items, t]); return ( - + {item.title} @@ -170,45 +250,65 @@ const InstrumentPreviewDialog = ({ item, onClose }: { item: InstrumentItem; onCl

)} {bundleQuery.data && ( - // Preview only: visually disable the submit button (pointer-events-none + dimmed) so no - // record can be created, and make submit a no-op as a defensive backstop. -
- { - // Intentionally does nothing: forms cannot be submitted in preview mode. - }} - /> -
+ { + // Intentionally does nothing: previews can advance without creating records. + }} + /> )} ) : ( -
-
- {t({ en: 'Kind', fr: 'Type' })}: - {item.kind} -
- {item.authors && item.authors.length > 0 && ( +
+
- {t({ en: 'Authors', fr: 'Auteurs' })}: - {item.authors.join(', ')} + {t({ en: 'Kind', fr: 'Type' })}: + {item.kind}
- )} - {item.description && ( + {item.kind === 'SERIES' && ( +
+ + {t({ en: 'Series order', fr: 'Ordre de la série' })} + {seriesItemTitles.length > 0 && ` (${seriesItemTitles.length})`}:{' '} + + {bundleQuery.isLoading ? ( + {t({ en: 'Loading...', fr: 'Chargement...' })} + ) : bundleQuery.isError ? ( + + {t({ en: 'Unable to load series items.', fr: 'Impossible de charger les éléments de la série.' })} + + ) : ( +
    + {seriesItemTitles.map((title, index) => ( +
  1. + {title} +
    2. + ))} +
+ )} +
+ )} + {item.authors && item.authors.length > 0 && ( +
+ {t({ en: 'Authors', fr: 'Auteurs' })}: + {item.authors.join(', ')} +
+ )} + {item.description && ( +
+ {t({ en: 'Description', fr: 'Description' })}: + {item.description} +
+ )}
- {t({ en: 'Description', fr: 'Description' })}: - {item.description} + {t({ en: 'Source', fr: 'Source' })}: + + {item.source.kind === 'repo' ? item.source.name : t({ en: 'No repo', fr: 'Aucun dépôt' })} +
- )} -
- {t({ en: 'Source', fr: 'Source' })}: - - {item.source.kind === 'repo' - ? item.source.name - : t({ en: 'Uploaded manually', fr: 'Téléversé manuellement' })} -
-
+
@@ -218,12 +318,283 @@ const InstrumentPreviewDialog = ({ item, onClose }: { item: InstrumentItem; onCl ); }; +const CreateSeriesInstrumentDialog = ({ + existingTitles, + forms, + onClose +}: { + existingTitles: string[]; + forms: InstrumentItem[]; + onClose: () => void; +}) => { + const { resolvedLanguage, t } = useTranslation(); + const createMutation = useCreateSeriesInstrumentMutation(); + const [title, setTitle] = useState(''); + const [instructions, setInstructions] = useState(''); + const [search, setSearch] = useState(''); + // Ordered list of selected instrument ids — order determines the sequence of the series. + const [selectedIds, setSelectedIds] = useState([]); + // Set when the server reports another series already uses the same forms; holds that series' name so + // we can ask the user whether they really want to create a duplicate. + const [duplicateOf, setDuplicateOf] = useState(null); + + // Only scalar instruments (which carry a name + edition) can be assembled into a series. + const selectableForms = useMemo(() => forms.filter((form) => form.internal !== null), [forms]); + + const filteredForms = useMemo(() => { + if (!search) return selectableForms; + const lower = search.toLowerCase(); + return selectableForms.filter((form) => form.title.toLowerCase().includes(lower)); + }, [selectableForms, search]); + + const normalizedTitle = title.trim().toLowerCase(); + const titleTaken = useMemo( + () => existingTitles.some((existing) => existing.trim().toLowerCase() === normalizedTitle), + [existingTitles, normalizedTitle] + ); + + const toggle = (id: string) => { + setSelectedIds((prev) => (prev.includes(id) ? prev.filter((value) => value !== id) : [...prev, id])); + }; + + const canCreate = title.trim().length > 0 && !titleTaken && selectedIds.length >= 2 && !createMutation.isPending; + + const buildItems = () => + selectedIds + .map((id) => selectableForms.find((form) => form.id === id)?.internal) + .filter((internal): internal is ScalarInstrumentInternal => internal !== null && internal !== undefined); + + // The series is authored in the creator's current language, so the details/instructions are stored as + // plain (unilingual) strings tagged with that language — the same shape any instrument would use. + const buildPayload = (items: ScalarInstrumentInternal[]): $CreateSeriesInstrumentData => ({ + clientDetails: instructions.trim() ? { instructions: [instructions.trim()] } : undefined, + details: { title: title.trim() }, + items, + language: resolvedLanguage + }); + + const handleCreate = async () => { + const items = buildItems(); + if (items.length < 2) { + return; + } + const result = await createMutation.mutateAsync(buildPayload(items)); + if (result.outcome === 'duplicate') { + // The existing title comes back in its stored form (a plain string, or multilingual object). + setDuplicateOf(typeof result.existingTitle === 'string' ? result.existingTitle : t(result.existingTitle)); + return; + } + onClose(); + }; + + const handleConfirmDuplicate = async () => { + const items = buildItems(); + if (items.length < 2) { + return; + } + await createMutation.mutateAsync({ ...buildPayload(items), confirmDuplicate: true }); + onClose(); + }; + + return ( + + { + if (!open) onClose(); + }} + > + + + {t({ en: 'Create Series Instrument', fr: 'Créer un instrument en série' })} + + {t({ + en: 'Give the series a unique name, then select at least two instruments to include in order.', + fr: 'Donnez un nom unique à la série, puis sélectionnez au moins deux instruments à inclure dans l’ordre.' + })} + + +
+
+ + setTitle(event.target.value)} + /> + {titleTaken && ( +

+ {t({ + en: 'An instrument with this name already exists.', + fr: 'Un instrument portant ce nom existe déjà.' + })} +

+ )} +
+
+ +