From 2486336d6b2cf1274a38a892d5585b8727cd2f6c Mon Sep 17 00:00:00 2001 From: thomasbeaudry Date: Mon, 6 Jul 2026 13:50:40 -0400 Subject: [PATCH 1/2] Fix runtime-core exports: point ./constants types to built .d.ts The ./constants subpath export had its `types` condition pointing at the source `./src/constants.ts` instead of the built declaration. A source .ts is not a valid target for the `types` condition, so a fresh CI build failed to resolve `@opendatacapture/runtime-core` exports, cascading into module-not-found and implicit-any errors in the instrument-bundler fixtures. Local builds passed only because a stale runtime/v1/dist masked it. Co-Authored-By: Claude Opus 4.8 --- packages/runtime-core/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/runtime-core/package.json b/packages/runtime-core/package.json index 2a9f03c8c..d326f1214 100644 --- a/packages/runtime-core/package.json +++ b/packages/runtime-core/package.json @@ -10,7 +10,7 @@ "import": "./dist/index.js" }, "./constants": { - "types": "./src/constants.ts", + "types": "./lib/constants.d.ts", "import": "./lib/constants.js" }, "./package.json": "./package.json" From c2478b9d295c8ed671c289f7d9c8542bba0a8200 Mon Sep 17 00:00:00 2001 From: thomasbeaudry Date: Wed, 8 Jul 2026 00:42:17 -0400 Subject: [PATCH 2/2] Address review feedback on series-instrument creation Backend - Derive $CreateSeriesInstrumentData from the master instrument details/clientDetails schemas (multilingual title/description/instructions) and take language from the client instead of hardcoding en/fr and duplicating text. - Remove the create-series DTO; the controller annotates the body with the $CreateSeriesInstrumentData schema value directly. - Share a discriminated-union result contract (CreateSeriesInstrumentResult) between the service, controller and web hook (outcome: 'created' | 'duplicate'). - Make InstrumentInfo a discriminated union over kind (scalar carries `internal`, series carries `seriesItems`); build the correct variant in findInfo and resolve series items from the full scalar set even under a kind filter. - deleteById: restrict deletion to series instruments (series never collect records) and drop the record-count guard; scope the group-manager delete ability to non-repo instruments. Add an explicit test that deleting a non-series is refused. - Handle multilingual titles throughout (uniqueness search now spans all instruments, duplicate detection returns the stored title); rename getFormSetKey -> getSeriesItemSetKey and give the generated definition an explicit SeriesInstrument type. Frontend - Rewrite useCreateSeriesInstrumentMutation to reuse the shared request/result types. - Remove pruneDeletedInstrument from the auth store; track deleted ids as local manage state and filter them from the save payload. - Fix instrument provenance to derive from the repo id (not name) so legacy repo instruments are never treated as manually-uploaded. - Make the shared submitButtonLabel prop a localizable value resolved with t(). Co-Authored-By: Claude Opus 4.8 --- apps/api/src/auth/ability.factory.ts | 8 +- .../__tests__/instruments.service.spec.ts | 97 +++++--- .../dto/create-series-instrument.dto.ts | 12 - .../src/instruments/instruments.controller.ts | 15 +- .../src/instruments/instruments.service.ts | 226 +++++++++--------- .../InstrumentCard/InstrumentCard.tsx | 2 +- .../InstrumentShowcase.stories.tsx | 4 +- .../useCreateSeriesInstrumentMutation.ts | 36 +-- .../src/hooks/useDeleteInstrumentMutation.ts | 8 +- .../_app/admin/instrument-repos/index.tsx | 4 +- apps/web/src/routes/_app/group/manage.tsx | 79 +++--- apps/web/src/store/slices/auth.slice.ts | 13 - apps/web/src/store/types.ts | 1 - .../components/FormContent/FormContent.tsx | 7 +- .../InstrumentRenderer/InstrumentRenderer.tsx | 5 +- .../ScalarInstrumentRenderer.tsx | 5 +- .../SeriesInstrumentRenderer.tsx | 5 +- packages/react-core/src/types.ts | 5 + .../schemas/src/instrument/instrument.base.ts | 113 ++++++--- 19 files changed, 360 insertions(+), 285 deletions(-) delete mode 100644 apps/api/src/instruments/dto/create-series-instrument.dto.ts diff --git a/apps/api/src/auth/ability.factory.ts b/apps/api/src/auth/ability.factory.ts index 40f875250..ab5c6f4cd 100644 --- a/apps/api/src/auth/ability.factory.ts +++ b/apps/api/src/auth/ability.factory.ts @@ -27,10 +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 remove ones they created - // (the service enforces that an instrument with collected records cannot be deleted). + // 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'); + 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/instruments/__tests__/instruments.service.spec.ts b/apps/api/src/instruments/__tests__/instruments.service.spec.ts index 7b0af6fd0..dc52ae06b 100644 --- a/apps/api/src/instruments/__tests__/instruments.service.spec.ts +++ b/apps/api/src/instruments/__tests__/instruments.service.spec.ts @@ -5,6 +5,8 @@ import type { MockedInstance } from '@douglasneuroinformatics/libnest/testing'; import { ConflictException, ForbiddenException, NotFoundException } from '@nestjs/common'; import { Test } from '@nestjs/testing'; 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'; @@ -16,7 +18,7 @@ vi.mock('@opendatacapture/instrument-bundler', () => ({ })); // A multilingual series instance as returned by `find`, containing two forms. -const existingSeries = { +const existingSeries: WithID = { __runtimeVersion: 1, content: { items: [ @@ -24,11 +26,16 @@ const existingSeries = { { edition: 1, name: 'FORM_B' } ] }, - details: { title: { en: 'Existing Series', fr: 'Série existante' } }, + 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'] -} as any; + language: ['en', 'fr'], + tags: { en: ['Series'], fr: ['Série'] } +}; describe('InstrumentsService', () => { let instrumentsService: InstrumentsService; @@ -58,6 +65,8 @@ describe('InstrumentsService', () => { 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', () => { @@ -72,14 +81,18 @@ describe('InstrumentsService', () => { // 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' } ], - title: 'My New Series' + language: 'en' }); - expect(result).toEqual({ existingTitle: 'Existing Series', requiresConfirmation: true }); + expect(result).toEqual({ + existingTitle: { en: 'Existing Series', fr: 'Série existante' }, + outcome: 'duplicate' + }); expect(createSpy).not.toHaveBeenCalled(); }); @@ -89,15 +102,16 @@ describe('InstrumentsService', () => { const result = await instrumentsService.createSeries({ confirmDuplicate: true, + details: { title: 'My New Series' }, items: [ { edition: 1, name: 'FORM_A' }, { edition: 1, name: 'FORM_B' } ], - title: 'My New Series' + language: 'en' }); expect(createSpy).toHaveBeenCalledWith({ bundle: '__BUNDLE__' }); - expect(result).toEqual({ id: 'new-id' }); + expect(result).toEqual({ instrumentId: 'new-id', outcome: 'created' }); }); it('creates the series directly when no existing series shares its forms', async () => { @@ -105,29 +119,30 @@ describe('InstrumentsService', () => { 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' } ], - title: 'Totally New' + language: 'en' }); expect(createSpy).toHaveBeenCalledWith({ bundle: '__BUNDLE__' }); - expect(result).toEqual({ id: 'fresh-id' }); + expect(result).toEqual({ instrumentId: 'fresh-id', outcome: 'created' }); }); - it('generates a valid series source payload with selected items in order', async () => { + 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({ - description: 'Optional description', - instructions: 'Complete the instruments in order.', + 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' } ], - title: 'Generated Series' + language: 'en' }); expect(bundle).toHaveBeenCalledTimes(1); @@ -143,13 +158,15 @@ describe('InstrumentsService', () => { const definition = JSON.parse(match![1]!); expect(definition.kind).toBe('SERIES'); - expect(definition.clientDetails).toEqual({ - instructions: { en: ['Complete the instruments in order.'], fr: ['Complete the instruments in order.'] } - }); + // 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: { en: 'Optional description', fr: 'Optional description' }, - title: { en: 'Generated Series', fr: 'Generated Series' } + 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' } @@ -165,13 +182,13 @@ describe('InstrumentsService', () => { __runtimeVersion: 1, content: { items }, details: { - description: { en: 'Stored Series', fr: 'Stored Series' }, + description: 'Stored Series', license: 'UNLICENSED', - title: { en: 'Stored Series', fr: 'Stored Series' } + title: 'Stored Series' }, kind: 'SERIES', - language: ['en', 'fr'], - tags: { en: ['Series'], fr: ['Série'] } + language: 'en', + tags: ['Series'] } as const; const id = `hash:${JSON.stringify({ content: instance.content, title: instance.details.title })}`; @@ -182,8 +199,9 @@ describe('InstrumentsService', () => { const result = await instrumentsService.createSeries({ confirmDuplicate: true, + details: { title: 'Stored Series' }, items, - title: 'Stored Series' + language: 'en' }); expect(virtualizationService.eval).toHaveBeenCalledWith('__BUNDLE__'); @@ -191,7 +209,7 @@ describe('InstrumentsService', () => { 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({ ...instance, id }); + expect(result).toEqual({ instrumentId: id, outcome: 'created' }); }); it('rejects a title that is already used (case-insensitively) by another instrument', async () => { @@ -200,11 +218,12 @@ describe('InstrumentsService', () => { await expect( instrumentsService.createSeries({ + details: { title: 'existing series' }, items: [ { edition: 1, name: 'FORM_X' }, { edition: 1, name: 'FORM_Y' } ], - title: 'existing series' + language: 'en' }) ).rejects.toThrow(ConflictException); expect(createSpy).not.toHaveBeenCalled(); @@ -217,11 +236,11 @@ describe('InstrumentsService', () => { const first = { ...existingSeries, - details: { title: 'First Series' } + details: { ...existingSeries.details, title: 'First Series' } }; const second = { ...existingSeries, - details: { title: 'Second Series' } + details: { ...existingSeries.details, title: 'Second Series' } }; expect(instrumentsService.generateSeriesInstrumentId(first)).not.toBe( @@ -239,17 +258,23 @@ describe('InstrumentsService', () => { await expect(instrumentsService.deleteById('missing')).rejects.toThrow(NotFoundException); }); - it('refuses to delete an instrument that has collected records', async () => { - instrumentModel.findFirst.mockResolvedValue({ id: 'has-records' }); - instrumentRecordModel.count.mockResolvedValue(3); + 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('has-records')).rejects.toThrow(ForbiddenException); + await expect(instrumentsService.deleteById('scalar')).rejects.toThrow(ForbiddenException); expect(instrumentModel.delete).not.toHaveBeenCalled(); }); - it('deletes the instrument and detaches it from every group when it has no records', async () => { - instrumentModel.findFirst.mockResolvedValue({ id: 'target' }); - instrumentRecordModel.count.mockResolvedValue(0); + 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'); @@ -260,6 +285,8 @@ describe('InstrumentsService', () => { }); 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/dto/create-series-instrument.dto.ts b/apps/api/src/instruments/dto/create-series-instrument.dto.ts deleted file mode 100644 index e0febd6ff..000000000 --- a/apps/api/src/instruments/dto/create-series-instrument.dto.ts +++ /dev/null @@ -1,12 +0,0 @@ -import { ValidationSchema } from '@douglasneuroinformatics/libnest'; -import type { ScalarInstrumentInternal } from '@opendatacapture/runtime-core'; -import { $CreateSeriesInstrumentData } from '@opendatacapture/schemas/instrument'; - -@ValidationSchema($CreateSeriesInstrumentData) -export class CreateSeriesInstrumentDto { - confirmDuplicate?: boolean; - description?: string; - instructions?: string; - items: ScalarInstrumentInternal[]; - title: string; -} diff --git a/apps/api/src/instruments/instruments.controller.ts b/apps/api/src/instruments/instruments.controller.ts index 52b85005b..6efc2085e 100644 --- a/apps/api/src/instruments/instruments.controller.ts +++ b/apps/api/src/instruments/instruments.controller.ts @@ -2,13 +2,19 @@ import { CurrentUser } from '@douglasneuroinformatics/libnest'; 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'; import { CreateInstrumentDto } from './dto/create-instrument.dto'; -import { CreateSeriesInstrumentDto } from './dto/create-series-instrument.dto'; import { InstrumentsService } from './instruments.service'; @ApiTags('Instruments') @@ -26,7 +32,10 @@ export class InstrumentsController { @ApiOperation({ summary: 'Create Series Instrument' }) @Post('series') @RouteAccess({ action: 'create', subject: 'Instrument' }) - createSeries(@Body() data: CreateSeriesInstrumentDto, @CurrentUser('ability') ability: AppAbility): Promise { + createSeries( + @Body() data: $CreateSeriesInstrumentData, + @CurrentUser('ability') ability: AppAbility + ): Promise { return this.instrumentsService.createSeries(data, { ability }); } diff --git a/apps/api/src/instruments/instruments.service.ts b/apps/api/src/instruments/instruments.service.ts index c30292432..a8c7cfa0b 100644 --- a/apps/api/src/instruments/instruments.service.ts +++ b/apps/api/src/instruments/instruments.service.ts @@ -13,7 +13,11 @@ import { getSeriesInstrumentItems, isScalarInstrument, isSeriesInstrument } from import type { AnyInstrument, AnyScalarInstrument, + ClientInstrumentDetails, InstrumentKind, + InstrumentLanguage, + InstrumentUIOption, + Language, ScalarInstrumentInternal, SeriesInstrument, SomeInstrument @@ -21,9 +25,13 @@ import type { 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'; @@ -32,22 +40,15 @@ import type { AppAbility } from '@/auth/auth.types'; import type { EntityOperationOptions } from '@/core/types'; import { CreateInstrumentDto } from './dto/create-instrument.dto'; -import { CreateSeriesInstrumentDto } from './dto/create-series-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>; }; -/** - * Returned by `createSeries` instead of creating an instrument when a series with the same set of forms - * already exists and the caller has not confirmed they want a duplicate. - */ -type SeriesDuplicateConfirmation = { - existingTitle: string; - requiresConfirmation: true; -}; - type InstrumentQuery = { kind?: TKind; subjectId?: string; @@ -122,40 +123,37 @@ export class InstrumentsService { /** * Assemble a new series instrument from existing scalar instruments. The title must be unique among - * instruments the caller can see; the series bundle is generated server-side and run through the same - * validation/creation path as any other instrument. + * 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 forms (regardless of order or name) and the - * caller has not opted to proceed, no instrument is created — instead a confirmation prompt is returned - * naming the existing series, so the client can ask the user whether they really want a duplicate. + * 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( - { confirmDuplicate, description, instructions, items, title }: CreateSeriesInstrumentDto, + { clientDetails, confirmDuplicate, details, items, language }: $CreateSeriesInstrumentData, options: EntityOperationOptions = {} - ): Promise> { - const trimmedTitle = title.trim(); - if (await this.isInstrumentTitleTaken(trimmedTitle, options)) { - throw new ConflictException(`An instrument named '${trimmedTitle}' already exists`); + ): Promise { + if (await this.isInstrumentTitleTaken(details.title)) { + throw new ConflictException('An instrument with the same title already exists'); } if (!confirmDuplicate) { - const existingTitle = await this.findSeriesTitleWithSameForms(items, options); + const existingTitle = await this.findSeriesTitleWithSameItems(items, options); if (existingTitle !== null) { - return { existingTitle, requiresConfirmation: true }; + return { existingTitle, outcome: 'duplicate' }; } } - const source = this.generateSeriesInstrumentSource({ - description: description?.trim(), - instructions: instructions?.trim(), - items, - title: trimmedTitle - }); + const source = this.generateSeriesInstrumentSource({ clientDetails, details, items, language }); const generatedBundle = await bundle({ inputs: [{ content: source, name: 'index.ts' }], minify: true }); - return this.create({ bundle: generatedBundle }); + const created = await this.create({ bundle: generatedBundle }); + return { instrumentId: created.id, outcome: 'created' }; } /** - * Delete an instrument the caller is permitted to remove, but only when no records have been collected - * with it (so historical data is never orphaned). Also detaches it from any group that exposes it. + * 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({ @@ -164,11 +162,9 @@ export class InstrumentsService { if (!instrument) { throw new NotFoundException(`Failed to find instrument with ID: ${id}`); } - const recordCount = await this.instrumentRecordModel.count({ where: { instrumentId: id } }); - if (recordCount > 0) { - throw new ForbiddenException( - `Cannot delete instrument: ${recordCount} record(s) have already been collected with it` - ); + 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. @@ -260,49 +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( - instances.flatMap((instance) => { - if (!isScalarInstrument(instance) || !instance.internal) { - return []; - } - return [[`${instance.internal.name}:${instance.internal.edition}`, instance.id] as const]; - }) + 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: InstrumentInfo = pick(instance, [ - '__runtimeVersion', - 'clientDetails', - 'details', - 'id', - 'internal', - 'kind', - 'language', - 'tags' - ]); - if (isSeriesInstrument(instance)) { - info.seriesItems = getSeriesInstrumentItems(instance.content) - .map(({ edition, name }) => scalarInstrumentIds.get(`${name}:${edition}`)) - .filter((id): id is string => typeof id === 'string') - .map((id) => ({ id })); - } + 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).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); } } @@ -378,23 +375,34 @@ 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 forms as `items` + * 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 findSeriesTitleWithSameForms( + private async findSeriesTitleWithSameItems( items: ScalarInstrumentInternal[], { ability }: EntityOperationOptions = {} - ): Promise { - const targetKey = this.getFormSetKey(items); + ): 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.getFormSetKey(getSeriesInstrumentItems(series.content)) === targetKey) { - const value = series.details.title; - return typeof value === 'string' ? value : (Object.values(value)[0] ?? null); + if (this.getSeriesItemSetKey(getSeriesInstrumentItems(series.content)) === targetKey) { + return series.details.title; } } return null; @@ -402,45 +410,41 @@ export class InstrumentsService { /** * 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. + * 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({ - description, - instructions, + clientDetails, + details, items, - title - }: { - description?: string; - instructions?: string; - items: ScalarInstrumentInternal[]; - title: string; - }): string { - const text = description && description.length > 0 ? description : title; - const definition = { + 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, - ...(instructions && instructions.length > 0 - ? { clientDetails: { instructions: { en: [instructions], fr: [instructions] } } } - : {}), + ...(clientDetails?.instructions ? { clientDetails: { instructions: clientDetails.instructions } } : {}), content: { items: items.map(({ edition, name }) => ({ edition, name })) }, details: { - description: { en: text, fr: text }, + description: details.description ?? details.title, license: 'UNLICENSED', - title: { en: title, fr: title } + title: details.title }, kind: 'SERIES', - language: ['en', 'fr'], - tags: { en: ['Series'], fr: ['Série'] } + language, + tags: this.buildSeriesTags(language) }; return `export default ${JSON.stringify(definition)};`; } /** - * A stable, order-independent key identifying the set of forms in a series. Two series with the same - * forms produce the same key even if the forms were added in a different order. + * 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 getFormSetKey(items: ScalarInstrumentInternal[]): string { + private getSeriesItemSetKey(items: ScalarInstrumentInternal[]): string { return items .map(({ edition, name }) => `${name}-${edition}`) .sort() @@ -456,17 +460,23 @@ export class InstrumentsService { } /** - * Whether an instrument visible to the caller already uses the given title (case-insensitive). Used to - * enforce that newly-created series instruments have a unique, recognizable name. + * 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: string, { ability }: EntityOperationOptions = {}): Promise { - const normalized = title.trim().toLowerCase(); - const instances = await this.find({}, { ability }); - return instances.some((instance) => { - const value = instance.details.title; - const titles = typeof value === 'string' ? [value] : Object.values(value); - return titles.some((candidate) => typeof candidate === 'string' && candidate.trim().toLowerCase() === normalized); - }); + 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) { @@ -488,4 +498,4 @@ export class InstrumentsService { } } -export type { InstrumentVirtualizationContext, SeriesDuplicateConfirmation }; +export type { InstrumentVirtualizationContext }; 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 index 50d2efa28..3f16e653b 100644 --- a/apps/web/src/hooks/useCreateSeriesInstrumentMutation.ts +++ b/apps/web/src/hooks/useCreateSeriesInstrumentMutation.ts @@ -1,39 +1,21 @@ import { useNotificationsStore, useTranslation } from '@douglasneuroinformatics/libui/hooks'; -import type { ScalarInstrumentInternal } from '@opendatacapture/runtime-core'; +import type { $CreateSeriesInstrumentData, CreateSeriesInstrumentResult } from '@opendatacapture/schemas/instrument'; import { useMutation, useQueryClient } from '@tanstack/react-query'; import axios from 'axios'; import { getApiErrorMessage } from '@/utils/error'; -type CreateSeriesInstrumentInput = { - confirmDuplicate?: boolean; - description?: string; - instructions?: string; - items: ScalarInstrumentInternal[]; - title: string; -}; - -// The server either creates the instrument (returning it with an id) or, when another series already -// contains the same forms and creation was not confirmed, asks the client to confirm the duplicate. -type SeriesDuplicateConfirmation = { - existingTitle: string; - requiresConfirmation: true; -}; -type CreateSeriesInstrumentResponse = SeriesDuplicateConfirmation | { id: string }; - -export function isSeriesDuplicateConfirmation( - response: CreateSeriesInstrumentResponse -): response is SeriesDuplicateConfirmation { - return 'requiresConfirmation' in response && response.requiresConfirmation; -} - +// 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: CreateSeriesInstrumentInput) => { - const response = await axios.post('/v1/instruments/series', data, { + mutationFn: async (data: $CreateSeriesInstrumentData) => { + const response = await axios.post('/v1/instruments/series', data, { meta: { disableDefaultErrorNotification: true } }); return response.data; @@ -48,8 +30,8 @@ export function useCreateSeriesInstrumentMutation() { }); }, onSuccess(data) { - // A confirmation prompt is not a creation: do not announce success or refresh the list yet. - if (isSeriesDuplicateConfirmation(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' }); diff --git a/apps/web/src/hooks/useDeleteInstrumentMutation.ts b/apps/web/src/hooks/useDeleteInstrumentMutation.ts index e42e95817..ca1099a84 100644 --- a/apps/web/src/hooks/useDeleteInstrumentMutation.ts +++ b/apps/web/src/hooks/useDeleteInstrumentMutation.ts @@ -2,13 +2,11 @@ import { useNotificationsStore, useTranslation } from '@douglasneuroinformatics/ import { useMutation, useQueryClient } from '@tanstack/react-query'; import axios from 'axios'; -import { useAppStore } from '@/store'; import { getApiErrorMessage } from '@/utils/error'; export function useDeleteInstrumentMutation() { const queryClient = useQueryClient(); const addNotification = useNotificationsStore((store) => store.addNotification); - const pruneDeletedInstrument = useAppStore((store) => store.pruneDeletedInstrument); const { t } = useTranslation(); return useMutation({ mutationFn: ({ id }: { id: string }) => @@ -22,10 +20,10 @@ export function useDeleteInstrumentMutation() { type: 'error' }); }, - onSuccess(_, { id }) { + onSuccess() { addNotification({ type: 'success' }); - // Drop the deleted id from the cached group so the manage page does not re-send it on save. - pruneDeletedInstrument(id); + // 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 b3a225c69..6161ed89b 100644 --- a/apps/web/src/routes/_app/group/manage.tsx +++ b/apps/web/src/routes/_app/group/manage.tsx @@ -17,7 +17,7 @@ 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 { InstrumentBundleContainer } from '@opendatacapture/schemas/instrument'; +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'; @@ -27,10 +27,7 @@ import { z } from 'zod/v4'; import { PageHeader } from '@/components/PageHeader'; import { WithFallback } from '@/components/WithFallback'; -import { - isSeriesDuplicateConfirmation, - useCreateSeriesInstrumentMutation -} from '@/hooks/useCreateSeriesInstrumentMutation'; +import { useCreateSeriesInstrumentMutation } from '@/hooks/useCreateSeriesInstrumentMutation'; import { useDeleteInstrumentMutation } from '@/hooks/useDeleteInstrumentMutation'; import { useInstrumentBundle } from '@/hooks/useInstrumentBundle'; import { useInstrumentInfoQuery } from '@/hooks/useInstrumentInfoQuery'; @@ -216,7 +213,8 @@ const InstrumentPreviewDialog = ({ const { t } = useTranslation(); const [showForm, setShowForm] = useState(false); const bundleQuery = useInstrumentBundle(showForm || item.kind === 'SERIES' ? item.id : null); - const previewSubmitLabel = t({ en: 'Preview Submit', fr: 'Soumettre l’aperçu' }); + // 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, @@ -329,7 +327,7 @@ const CreateSeriesInstrumentDialog = ({ forms: InstrumentItem[]; onClose: () => void; }) => { - const { t } = useTranslation(); + const { resolvedLanguage, t } = useTranslation(); const createMutation = useCreateSeriesInstrumentMutation(); const [title, setTitle] = useState(''); const [instructions, setInstructions] = useState(''); @@ -366,18 +364,24 @@ const CreateSeriesInstrumentDialog = ({ .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({ - instructions: instructions.trim() || undefined, - items, - title: title.trim() - }); - if (isSeriesDuplicateConfirmation(result)) { - setDuplicateOf(result.existingTitle); + 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(); @@ -388,12 +392,7 @@ const CreateSeriesInstrumentDialog = ({ if (items.length < 2) { return; } - await createMutation.mutateAsync({ - confirmDuplicate: true, - instructions: instructions.trim() || undefined, - items, - title: title.trim() - }); + await createMutation.mutateAsync({ ...buildPayload(items), confirmDuplicate: true }); onClose(); }; @@ -531,12 +530,21 @@ const CreateSeriesInstrumentDialog = ({ ); }; -const DeleteInstrumentDialog = ({ item, onClose }: { item: InstrumentItem; onClose: () => void }) => { +const DeleteInstrumentDialog = ({ + item, + onClose, + onDeleted +}: { + item: InstrumentItem; + onClose: () => void; + onDeleted: (id: string) => void; +}) => { const { t } = useTranslation(); const deleteMutation = useDeleteInstrumentMutation(); const handleDelete = async () => { await deleteMutation.mutateAsync({ id: item.id }); + onDeleted(item.id); onClose(); }; @@ -583,6 +591,10 @@ const ManageGroupForm = ({ data, onSubmit, readOnly }: ManageGroupFormProps) => const [previewItem, setPreviewItem] = useState(null); const [showCreateSeries, setShowCreateSeries] = useState(false); const [deletingItem, setDeletingItem] = useState(null); + // Ids deleted during this session. The server has already detached them from the group, but our copy of + // the group's accessible ids (seeded from the auth store) may still list them; we drop them at save time + // so a just-deleted instrument is never re-sent as a dangling relation. + const [deletedIds, setDeletedIds] = useState>(() => new Set()); // Resync the selection when the upstream group data changes (e.g. after a save refetches the group). // `initialSelectedIds` is a stable reference from a useMemo, so this only fires on real changes — @@ -624,7 +636,13 @@ const ManageGroupForm = ({ data, onSubmit, readOnly }: ManageGroupFormProps) => onClose={() => setShowCreateSeries(false)} /> )} - {deletingItem && setDeletingItem(null)} />} + {deletingItem && ( + setDeletingItem(null)} + onDeleted={(id) => setDeletedIds((prev) => new Set(prev).add(id))} + /> + )} {t('group.manage.accessibleInstruments')}

{description}

@@ -818,7 +836,7 @@ const ManageGroupForm = ({ data, onSubmit, readOnly }: ManageGroupFormProps) => selectedIds, series: instruments.series }) - ], + ].filter((id) => !deletedIds.has(id)), settings: { defaultIdentificationMethod: formData.defaultIdentificationMethod, idValidationRegex: formData.idValidationRegex, @@ -870,17 +888,19 @@ const RouteComponent = () => { continue; } visibleIds.add(instrument.id); - // Show the repo name when we have one; legacy repo instruments without a stored name render as - // "uploaded manually" (but are still filtered above as repo-sourced via their id). - const repoName = instrument.sourceRepo?.name ?? null; - const source: InstrumentSource = repoName ? { kind: 'repo', name: repoName } : { kind: 'manual' }; + // Provenance is determined by the repo id, not the name: a legacy repo instrument may have a null + // name but is still repo-sourced (and so must not get the "manual" delete affordance). We fall back + // to a placeholder label only for display. + const source: InstrumentSource = repoId + ? { kind: 'repo', name: instrument.sourceRepo?.name ?? t({ en: 'Unknown repository', fr: 'Dépôt inconnu' }) } + : { kind: 'manual' }; const item: InstrumentItem = { authors: instrument.details.authors, description: instrument.details.description, id: instrument.id, - internal: instrument.internal ?? null, + internal: instrument.kind === 'SERIES' ? null : instrument.internal, kind: instrument.kind, - seriesItems: instrument.seriesItems, + seriesItems: instrument.kind === 'SERIES' ? instrument.seriesItems : undefined, source, title: instrument.details.title }; @@ -919,6 +939,7 @@ const RouteComponent = () => { defaultIdentificationMethod, instrumentRepoIds, resolvedLanguage, + t, currentGroup?.settings ]); diff --git a/apps/web/src/store/slices/auth.slice.ts b/apps/web/src/store/slices/auth.slice.ts index f78e301fb..c1869e094 100644 --- a/apps/web/src/store/slices/auth.slice.ts +++ b/apps/web/src/store/slices/auth.slice.ts @@ -39,19 +39,6 @@ export const createAuthSlice: SliceCreator = (set) => { }, logout: () => { window.location.reload(); - }, - pruneDeletedInstrument: (instrumentId) => { - // Keep the cached group(s) in sync after an instrument is deleted: the deleted id would otherwise - // linger in accessibleInstrumentIds and be re-sent on the next group save, failing the relation set. - set((state) => { - const remove = (ids: string[]) => ids.filter((id) => id !== instrumentId); - if (state.currentGroup) { - state.currentGroup.accessibleInstrumentIds = remove(state.currentGroup.accessibleInstrumentIds); - } - state.currentUser?.groups.forEach((group) => { - group.accessibleInstrumentIds = remove(group.accessibleInstrumentIds); - }); - }); } }; }; diff --git a/apps/web/src/store/types.ts b/apps/web/src/store/types.ts index 88dd8bdd8..e2d78fe31 100644 --- a/apps/web/src/store/types.ts +++ b/apps/web/src/store/types.ts @@ -16,7 +16,6 @@ export type AuthSlice = { currentUser: CurrentUser | null; login: (accessToken: string) => void; logout: () => void; - pruneDeletedInstrument: (instrumentId: string) => void; }; export type DisclaimerSlice = { diff --git a/packages/react-core/src/components/FormContent/FormContent.tsx b/packages/react-core/src/components/FormContent/FormContent.tsx index 5276b1fe6..ffeec04a8 100644 --- a/packages/react-core/src/components/FormContent/FormContent.tsx +++ b/packages/react-core/src/components/FormContent/FormContent.tsx @@ -4,12 +4,15 @@ import type { AnyUnilingualFormInstrument, FormInstrument, InstrumentKind } from import { InfoIcon } from 'lucide-react'; import type { Promisable } from 'type-fest'; +import type { LocalizedText } from '../../types'; + export type FormContentSubmitResult = { data: FormInstrument.Data; kind: Extract }; export type FormContentProps = { instrument: AnyUnilingualFormInstrument; onSubmit: (result: FormContentSubmitResult) => Promisable; - submitButtonLabel?: string; + /** A localizable submit-button label; resolved to the active language here. */ + submitButtonLabel?: LocalizedText; }; export const FormContent = ({ instrument, onSubmit, submitButtonLabel }: FormContentProps) => { @@ -45,7 +48,7 @@ export const FormContent = ({ instrument, onSubmit, submitButtonLabel }: FormCon content={instrument.content} data-testid="form-content" initialValues={instrument.initialValues} - submitBtnLabel={submitButtonLabel} + submitBtnLabel={submitButtonLabel ? t(submitButtonLabel) : undefined} validationSchema={instrument.validationSchema} onSubmit={(data) => void onSubmit({ data, kind: 'FORM' })} /> diff --git a/packages/react-core/src/components/InstrumentRenderer/InstrumentRenderer.tsx b/packages/react-core/src/components/InstrumentRenderer/InstrumentRenderer.tsx index 6e0d676fa..d1ebe01ba 100644 --- a/packages/react-core/src/components/InstrumentRenderer/InstrumentRenderer.tsx +++ b/packages/react-core/src/components/InstrumentRenderer/InstrumentRenderer.tsx @@ -5,7 +5,7 @@ import type { InstrumentBundleContainer } from '@opendatacapture/schemas/instrum import { ScalarInstrumentRenderer } from './ScalarInstrumentRenderer'; import { SeriesInstrumentRenderer } from './SeriesInstrumentRenderer'; -import type { SubjectDisplayInfo } from '../../types'; +import type { LocalizedText, SubjectDisplayInfo } from '../../types'; import type { NavigationBlockerComponent } from '../NavigationBlockerDialog'; import type { InstrumentSubmitHandler } from './types'; @@ -21,7 +21,8 @@ export type InstrumentRendererProps = { NavigationBlocker?: NavigationBlockerComponent; onSubmit: InstrumentSubmitHandler; subject?: SubjectDisplayInfo; - submitButtonLabel?: string; + /** A localizable label for the submit button shown on the instrument's form(s). */ + submitButtonLabel?: LocalizedText; target: InstrumentBundleContainer; }; diff --git a/packages/react-core/src/components/InstrumentRenderer/ScalarInstrumentRenderer.tsx b/packages/react-core/src/components/InstrumentRenderer/ScalarInstrumentRenderer.tsx index a98a9ec7b..cf5a0e190 100644 --- a/packages/react-core/src/components/InstrumentRenderer/ScalarInstrumentRenderer.tsx +++ b/packages/react-core/src/components/InstrumentRenderer/ScalarInstrumentRenderer.tsx @@ -18,7 +18,7 @@ import { InteractiveContent } from '../InteractiveContent'; import { ContentPlaceholder } from './ContentPlaceholder'; import { InstrumentRendererContainer } from './InstrumentRendererContainer'; -import type { SubjectDisplayInfo } from '../../types'; +import type { LocalizedText, SubjectDisplayInfo } from '../../types'; import type { NavigationBlockerComponent } from '../NavigationBlockerDialog'; import type { AnyContentResult, InstrumentSubmitHandler } from './types'; @@ -37,7 +37,8 @@ export type ScalarInstrumentRendererProps = { /** @deprecated */ options?: InterpretOptions; subject?: SubjectDisplayInfo; - submitButtonLabel?: string; + /** A localizable label for the form's submit button. */ + submitButtonLabel?: LocalizedText; target: Pick; }; diff --git a/packages/react-core/src/components/InstrumentRenderer/SeriesInstrumentRenderer.tsx b/packages/react-core/src/components/InstrumentRenderer/SeriesInstrumentRenderer.tsx index c0b36d8af..3ddde66aa 100644 --- a/packages/react-core/src/components/InstrumentRenderer/SeriesInstrumentRenderer.tsx +++ b/packages/react-core/src/components/InstrumentRenderer/SeriesInstrumentRenderer.tsx @@ -17,7 +17,7 @@ import { InteractiveContent } from '../InteractiveContent'; import { ContentPlaceholder } from './ContentPlaceholder'; import { InstrumentRendererContainer } from './InstrumentRendererContainer'; -import type { SubjectDisplayInfo } from '../../types'; +import type { LocalizedText, SubjectDisplayInfo } from '../../types'; import type { FormContentSubmitResult } from '../FormContent'; import type { InteractiveContentSubmitResult } from '../InteractiveContent'; import type { InstrumentSubmitHandler } from './types'; @@ -31,7 +31,8 @@ export type SeriesInstrumentRendererProps = { initialSeriesIndex?: number; onSubmit: InstrumentSubmitHandler<'SERIES'>; subject?: SubjectDisplayInfo; - submitButtonLabel?: string; + /** A localizable label for each constituent form's submit button. */ + submitButtonLabel?: LocalizedText; target: SeriesInstrumentBundleContainer; }; diff --git a/packages/react-core/src/types.ts b/packages/react-core/src/types.ts index a4d116e22..70ba9afaf 100644 --- a/packages/react-core/src/types.ts +++ b/packages/react-core/src/types.ts @@ -19,4 +19,9 @@ export type InterpretedInstrumentState; diff --git a/packages/schemas/src/instrument/instrument.base.ts b/packages/schemas/src/instrument/instrument.base.ts index 709833d5d..66fb2a02c 100644 --- a/packages/schemas/src/instrument/instrument.base.ts +++ b/packages/schemas/src/instrument/instrument.base.ts @@ -224,18 +224,13 @@ const $UnilingualScalarInstrument = $ScalarInstrument.extend({ }) satisfies z.ZodType>; /** - * An object containing the essential data describing an instrument, but omitting the content - * and validation schema required to actually complete the instrument. This may be used for, - * among other things, displaying available instruments to the user. + * The fields common to every instrument's "info" (essential describing data, omitting the content and + * validation schema needed to actually complete the instrument). Used, among other things, for + * displaying available instruments to the user. */ -type InstrumentInfo = Omit & { +type BaseInstrumentInfo = Omit & { id: string; - internal?: { - edition: number; - name: string; - }; // Provenance: null when uploaded manually; otherwise the source repository id (always present) and - seriesItems?: { id: string }[]; // its name (may be null for legacy instruments imported before names were stored). sourceRepo?: null | { id: string; @@ -243,26 +238,48 @@ type InstrumentInfo = Omit; +/** Info for a scalar (standalone) instrument, uniquely identified by its internal name + edition. */ +type ScalarInstrumentInfo = BaseInstrumentInfo & { + internal: { + edition: number; + name: string; + }; + kind: Exclude; +}; + +/** Info for a series instrument, which bundles the scalar instruments referenced by `seriesItems`. */ +type SeriesInstrumentInfo = BaseInstrumentInfo & { + kind: 'SERIES'; + seriesItems: { id: string }[]; +}; + +/** A discriminated union over `kind`: scalar instruments carry `internal`, series carry `seriesItems`. */ +type InstrumentInfo = ScalarInstrumentInfo | SeriesInstrumentInfo; + +const $BaseInstrumentInfo = $BaseInstrument.omit({ content: true, kind: true }).extend({ + id: z.string(), + sourceRepo: z + .object({ + id: z.string(), + name: z.string().nullable() + }) + .nullish() +}); + +const $ScalarInstrumentInfo = $BaseInstrumentInfo.extend({ + internal: $ScalarInstrumentInternal, + kind: z.enum(['FILE', 'FORM', 'INTERACTIVE']) +}) satisfies z.ZodType; + +const $SeriesInstrumentInfo = $BaseInstrumentInfo.extend({ + kind: z.literal('SERIES'), + seriesItems: z.object({ id: z.string() }).array() +}) satisfies z.ZodType; + +const $InstrumentInfo = z.discriminatedUnion('kind', [ + $ScalarInstrumentInfo, + $SeriesInstrumentInfo +]) satisfies z.ZodType; /** @internal */ type UnilingualInstrumentInfo = Simplify>>; @@ -280,20 +297,38 @@ const $CreateInstrumentData = z.object({ bundle: z.string().min(1) }); -type CreateSeriesInstrumentData = z.infer; +/** + * The data a client submits to assemble a new series instrument from existing scalar instruments. The + * multilingual `details`/`clientDetails` are picked directly from the master instrument schemas (single + * source of truth); everything else about the stored instrument (id, content, kind, ...) is computed + * server-side. Exported as both a value (the schema) and a type so it can annotate a controller body + * without a dedicated DTO class. + */ +type $CreateSeriesInstrumentData = z.infer; const $CreateSeriesInstrumentData = z.object({ - // When true, proceed even though another series already contains the same set of forms. + // Optional instructions shown before the series begins (same shape as any instrument's clientDetails). + clientDetails: $ClientInstrumentDetails.pick({ instructions: true }).optional(), + // When true, proceed even though another series already contains the same set of items. confirmDuplicate: z.boolean().optional(), - // Optional human-readable description; falls back to the title when omitted. - description: z.string().min(1).optional(), - // Optional instructions shown to participants before the series begins. - instructions: z.string().min(1).optional(), + // The display title (required) and optional description, in the same multilingual shape as an instrument. + details: $InstrumentDetails.pick({ title: true }).extend({ + description: $InstrumentDetails.shape.description.optional() + }), // The ordered list of scalar instruments (by name + edition) that make up the series. items: z.array($ScalarInstrumentInternal).min(2), - // The unique display name entered by the group manager. - title: z.string().min(1) + // The language(s) the details above are written in — mirrors an instrument's own `language` field. + language: $InstrumentLanguage }); +/** + * The result of a create-series request. A discriminated union over `outcome`: either the series was + * created, or another series already contains the same set of items and the caller must confirm the + * duplicate before it is created. + */ +type CreateSeriesInstrumentResult = + | { existingTitle: NonNullable; outcome: 'duplicate' } + | { instrumentId: string; outcome: 'created' }; + const $BaseInstrumentBundleContainer = z.object({ id: z.string() }); @@ -344,12 +379,14 @@ export { export type { CreateInstrumentData, - CreateSeriesInstrumentData, + CreateSeriesInstrumentResult, InstrumentBundleContainer, InstrumentInfo, MultilingualInstrumentInfo, ScalarInstrumentBundleContainer, + ScalarInstrumentInfo, SeriesInstrumentBundleContainer, + SeriesInstrumentInfo, TranslatedInstrumentInfo, UnilingualInstrumentInfo };