diff --git a/packages/instrument-utils/AGENTS.md b/packages/instrument-utils/AGENTS.md index e8f38dcc0..01f3f90e4 100644 --- a/packages/instrument-utils/AGENTS.md +++ b/packages/instrument-utils/AGENTS.md @@ -18,8 +18,10 @@ handled. That coupling is the point — do not add a catch-all pattern to make i fails `$InstrumentMeasureValue`, or whose label cannot be resolved, is `console.error`ed and dropped from the returned record; non-object `data` returns `{}`. The visible symptom is a summary missing a row, not an error. Check a measure's `ref` and `label` rather than trusting that the page rendered. -A `computed` measure is different: its author-supplied `value(data)` function is called bare, so if -it throws, the whole computation throws. +A `const` measure on a dynamic form field that the data keeps hidden (`isFieldHidden`) is dropped +without a log, label or not: the respondent never saw that question. A `computed` measure is +different: its author-supplied `value(data)` function is called bare, so if it throws, the whole +computation throws. **`isScalarInstrument` tests `Object.hasOwn(instrument, 'internal')`, not `kind`.** See `.agents/docs/architecture/instrument-pipeline.md` for why. @@ -29,6 +31,6 @@ appearing in two groups silently keeps the last one. ## Tests -`pnpm exec vitest --project instrument-utils`. `src/__tests__/` covers `getFormFields`; everything -else here is still covered only through its consumers' suites (`apps/api`, `apps/web`) and the -Playwright suite in `testing/`. +`pnpm exec vitest --project instrument-utils`. `src/__tests__/` holds one file per module. The +results summary that `computeInstrumentMeasures` feeds is exercised end to end by +`testing/src/specs/playground.spec.ts`. diff --git a/packages/instrument-utils/src/__tests__/form.test.ts b/packages/instrument-utils/src/__tests__/form.test.ts index 9eeb6dc73..6e83315d6 100644 --- a/packages/instrument-utils/src/__tests__/form.test.ts +++ b/packages/instrument-utils/src/__tests__/form.test.ts @@ -1,7 +1,7 @@ import type { FormInstrument, Language } from '@opendatacapture/runtime-core'; import { describe, expect, it } from 'vitest'; -import { extractFieldLabel, getFormFields } from '../form.js'; +import { extractFieldLabel, getFormFields, isFieldHidden } from '../form.js'; type TData = { a: number; b: number }; @@ -65,3 +65,28 @@ describe('extractFieldLabel', () => { expect(extractFieldLabel(dynamicForm, 'a', { a: 1, b: 2 })).toBeUndefined(); }); }); + +describe('isFieldHidden', () => { + const form = { + content: { + a: fieldA, + b: { + deps: ['a'], + kind: 'dynamic', + render: (data: { a?: number }) => (data.a ? { kind: 'number', label: 'B', variant: 'input' } : null) + } + } + } as unknown as FormInstrument; + + it('should report a dynamic field hidden while its render function returns null for the data', () => { + expect(isFieldHidden(form, 'b', {})).toBe(true); + }); + + it('should report a dynamic field shown once its render function returns a field for the data', () => { + expect(isFieldHidden(form, 'b', { a: 1 })).toBe(false); + }); + + it('should never report a static field hidden, since the respondent always sees it', () => { + expect(isFieldHidden(form, 'a', {})).toBe(false); + }); +}); diff --git a/packages/instrument-utils/src/__tests__/measures.test.ts b/packages/instrument-utils/src/__tests__/measures.test.ts index ec1e4fa64..05199f34a 100644 --- a/packages/instrument-utils/src/__tests__/measures.test.ts +++ b/packages/instrument-utils/src/__tests__/measures.test.ts @@ -16,6 +16,28 @@ const formInstrument = { } } as unknown as AnyUnilingualScalarInstrument & { content: FormInstrument.Content }; +type TDynamicData = { hasPet: boolean; petName?: string }; + +const dynamicFormInstrument = { + content: { + hasPet: { kind: 'boolean', label: 'Has Pet', variant: 'radio' }, + petName: { + deps: ['hasPet'], + kind: 'dynamic', + render: (data: FormInstrument.PartialData) => { + return data.hasPet ? { kind: 'string', label: 'Pet Name', variant: 'input' } : null; + } + } + }, + internal: { edition: 1, name: 'STUB_DYNAMIC_FORM' }, + kind: 'FORM', + language: 'en', + measures: { + labeledPetName: { kind: 'const', label: 'Name of Pet', ref: 'petName' }, + petName: { kind: 'const', ref: 'petName' } + } +} as unknown as AnyUnilingualScalarInstrument; + const interactiveInstrument = { internal: { edition: 1, name: 'STUB_INTERACTIVE' }, kind: 'INTERACTIVE', @@ -47,6 +69,15 @@ describe('computeInstrumentMeasures', () => { expect(result.favoriteNumber).toEqual({ label: 'Favorite Number', value: 7 }); }); + it("should read a 'const' measure's value and label through its ref, so a measure keyed apart from its field still shows that field", () => { + const instrument = { + ...formInstrument, + measures: { favorite: { kind: 'const', ref: 'favoriteNumber' } } + } as unknown as AnyUnilingualScalarInstrument; + const result = computeInstrumentMeasures(instrument, { favoriteNumber: 7 }); + expect(result.favorite).toEqual({ label: 'Favorite Number', value: 7 }); + }); + it("should use a 'const' measure's own label when it declares one", () => { const instrument = { ...formInstrument, @@ -71,4 +102,25 @@ describe('computeInstrumentMeasures', () => { expect(result.message).toBeUndefined(); expect(console.error).toHaveBeenCalledOnce(); }); + + it('should leave out a labeled measure on a dynamic field its data keeps hidden, since the respondent never saw it', () => { + const result = computeInstrumentMeasures(dynamicFormInstrument, { hasPet: false }); + expect(result.labeledPetName).toBeUndefined(); + }); + + it('should keep a labeled measure on a dynamic field once its data reveals it', () => { + const result = computeInstrumentMeasures(dynamicFormInstrument, { hasPet: true, petName: 'Rex' }); + expect(result.labeledPetName).toEqual({ label: 'Name of Pet', value: 'Rex' }); + }); + + it('should leave out an unlabeled measure on a hidden dynamic field without logging, since nothing failed', () => { + const result = computeInstrumentMeasures(dynamicFormInstrument, { hasPet: false }); + expect(result.petName).toBeUndefined(); + expect(console.error).not.toHaveBeenCalled(); + }); + + it('should keep a measure on an unanswered static field, so the summary still shows it as missing', () => { + const result = computeInstrumentMeasures(formInstrument, {}); + expect(result.favoriteNumber).toEqual({ label: 'Favorite Number', value: undefined }); + }); }); diff --git a/packages/instrument-utils/src/form.ts b/packages/instrument-utils/src/form.ts index 507034633..827e3d430 100644 --- a/packages/instrument-utils/src/form.ts +++ b/packages/instrument-utils/src/form.ts @@ -15,6 +15,16 @@ export function getFormFields( }, {}) as FormInstrument.Fields; } +/** Whether the field is a dynamic one that `data` keeps hidden, so the respondent never saw it */ +export function isFieldHidden( + form: FormInstrument, + key: string, + data: { [key: string]: unknown } +) { + const field = getFormFields(form.content)[key]; + return field?.kind === 'dynamic' && field.render(data as FormInstrument.PartialData) === null; +} + export function extractFieldLabel( form: FormInstrument, key: string, diff --git a/packages/instrument-utils/src/measures.ts b/packages/instrument-utils/src/measures.ts index adcbc14d4..7c18b3390 100644 --- a/packages/instrument-utils/src/measures.ts +++ b/packages/instrument-utils/src/measures.ts @@ -1,9 +1,14 @@ import { isPlainObject } from '@douglasneuroinformatics/libjs'; -import type { AnyUnilingualScalarInstrument, InstrumentMeasureValue } from '@opendatacapture/runtime-core'; +import type { + AnyUnilingualScalarInstrument, + InstrumentMeasures, + InstrumentMeasureValue, + Language +} from '@opendatacapture/runtime-core'; import { $InstrumentMeasureValue } from '@opendatacapture/schemas/instrument'; import { match } from 'ts-pattern'; -import { extractFieldLabel } from './form.js'; +import { extractFieldLabel, isFieldHidden } from './form.js'; import { isFormInstrument } from './guards.js'; export type ComputedMeasures = { [key: string]: { label: string; value: InstrumentMeasureValue } }; @@ -14,14 +19,19 @@ export function computeInstrumentMeasures(instrument: AnyUnilingualScalarInstrum console.error(`Cannot compute measures from data: ${JSON.stringify(data)} is not an object`); return computedMeasures; } - for (const key in instrument.measures) { - const result = match(instrument.measures[key]!) + // Widened because `data` is untyped: under an interactive instrument's `Json` data, `ConditionalKeys` + // makes `ref` a union of `Json[]` members (numbers, array methods), which cannot index `data`. + const measures: InstrumentMeasures | null = instrument.measures; + for (const key in measures) { + const result = match(measures[key]!) .with({ kind: 'computed' }, (measure) => { - // @ts-expect-error - this is ignored because it is safer than the previous (any) solution return { label: measure.label, value: measure.value(data) }; }) .with({ kind: 'const' }, (measure) => { - const result = $InstrumentMeasureValue.safeParse(data[key]); + if (isFormInstrument(instrument) && isFieldHidden(instrument, measure.ref, data)) { + return null; + } + const result = $InstrumentMeasureValue.safeParse(data[measure.ref]); if (!result.success) { console.error('Failed to Parse Constant Measure', result.error); return null; @@ -32,10 +42,10 @@ export function computeInstrumentMeasures(instrument: AnyUnilingualScalarInstrum label = measure.label; } else if (isFormInstrument(instrument)) { // @ts-expect-error - this is ignored because it is safer than the previous (any) solution - label = extractFieldLabel(instrument, key, data); + label = extractFieldLabel(instrument, measure.ref, data); } if (!label) { - console.error(`Failed to extract label for key '${key}' from data '${JSON.stringify(data)}'`); + console.error(`Failed to extract label for key '${measure.ref}' from data '${JSON.stringify(data)}'`); return; } return { label, value: result.data }; diff --git a/packages/react-core/src/components/InstrumentSummary/__tests__/InstrumentSummary.test.tsx b/packages/react-core/src/components/InstrumentSummary/__tests__/InstrumentSummary.test.tsx index bf093937f..b3f110203 100644 --- a/packages/react-core/src/components/InstrumentSummary/__tests__/InstrumentSummary.test.tsx +++ b/packages/react-core/src/components/InstrumentSummary/__tests__/InstrumentSummary.test.tsx @@ -7,6 +7,7 @@ import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; import { InstrumentSummary } from '../InstrumentSummary'; const baseInstrument = { + content: {}, details: { title: 'Stub Form' }, internal: { edition: 1, name: 'STUB_FORM' }, kind: 'FORM', diff --git a/testing/src/specs/playground.spec.ts b/testing/src/specs/playground.spec.ts index 2002a09a9..e12d111cc 100644 --- a/testing/src/specs/playground.spec.ts +++ b/testing/src/specs/playground.spec.ts @@ -61,6 +61,41 @@ export default defineInstrument({ `; } +/** + * A form that asks for a pet's name only once the respondent says they have a pet. Both questions + * carry a labeled measure, so the toggle's measure is always in the summary and the name's only when + * the question was shown. + */ +function dynamicMeasureInstrumentSource(title: string): string { + return ` +import { defineInstrument } from '/runtime/v1/@opendatacapture/runtime-core'; +import { z } from '/runtime/v1/zod@3.x'; + +export default defineInstrument({ + kind: 'FORM', + language: 'en', + tags: ['Measures'], + internal: { edition: 1, name: 'DYNAMIC_MEASURE' }, + clientDetails: { estimatedDuration: 1, instructions: ['Submit the form'] }, + content: { + hasPet: { kind: 'boolean', label: 'Has Pet', variant: 'radio' }, + petName: { + kind: 'dynamic', + deps: ['hasPet'], + render: (data) => (data.hasPet ? { kind: 'string', label: 'Pet Name', variant: 'input' } : null) + } + }, + defaultMeasureVisibility: 'visible', + details: { description: 'Dynamic field measure', license: 'Apache-2.0', title: '${title}' }, + measures: { + hasPet: { kind: 'const', label: 'Has Pet Measure', ref: 'hasPet' }, + petName: { kind: 'const', label: 'Pet Name Measure', ref: 'petName' } + }, + validationSchema: z.object({ hasPet: z.boolean(), petName: z.string().optional() }) +}); +`; +} + // The first paint waits on the 11 MB esbuild download and the toolchain boot; the preview then // compiles on a 2 s poll, so the whole chain is slower than the suite's default expect timeout. const PREVIEW_TIMEOUT = 60_000; @@ -165,4 +200,49 @@ test.describe('playground', () => { await expect(playground.preview.getByText('Shown Measure')).toBeVisible(); await expect(playground.preview.getByText('Hidden Measure')).toHaveCount(0); }); + + test('should leave the measure on a dynamic question the respondent never saw out of the summary', async ({ + page, + uniqueId + }) => { + const title = `Dynamic Measure ${uniqueId}`; + const playground = new PlaygroundPage(page); + await playground.goto( + generatePlaygroundURL({ + baseURL: playgroundURL, + files: [{ content: dynamicMeasureInstrumentSource(title), name: 'index.ts' }], + label: title + }) + ); + + await playground.preview.getByRole('button', { name: 'Begin' }).click({ timeout: PREVIEW_TIMEOUT }); + // The radio items carry stable ids (`-true`), unlike their labels, which libui translates. + await playground.preview.locator('#hasPet-false').click(); + await playground.preview.getByRole('button', { name: 'Submit' }).click(); + + await expect(playground.preview.getByText('Has Pet Measure')).toBeVisible(); + await expect(playground.preview.getByText('Pet Name Measure')).toHaveCount(0); + }); + + test('should list the measure on a dynamic question in the summary once the respondent reveals it', async ({ + page, + uniqueId + }) => { + const title = `Dynamic Measure ${uniqueId}`; + const playground = new PlaygroundPage(page); + await playground.goto( + generatePlaygroundURL({ + baseURL: playgroundURL, + files: [{ content: dynamicMeasureInstrumentSource(title), name: 'index.ts' }], + label: title + }) + ); + + await playground.preview.getByRole('button', { name: 'Begin' }).click({ timeout: PREVIEW_TIMEOUT }); + await playground.preview.locator('#hasPet-true').click(); + await playground.preview.getByLabel('Pet Name', { exact: true }).fill('Rex'); + await playground.preview.getByRole('button', { name: 'Submit' }).click(); + + await expect(playground.preview.getByText('Pet Name Measure')).toBeVisible(); + }); });