Skip to content

Commit 8277e8f

Browse files
denniskigendenniskigen
authored
(feat) Flag quoted field IDs in calculateExpressions as errors (#673)
1 parent 7ede9d8 commit 8277e8f

File tree

3 files changed

+282
-3
lines changed

3 files changed

+282
-3
lines changed

‎src/components/processor-factory/form-processor-factory.component.tsx‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,15 @@ const FormProcessorFactory = ({
3131

3232
const processor = useMemo(() => {
3333
const ProcessorClass = formProcessors[formJson.processor];
34+
let processorInstance;
3435
if (ProcessorClass) {
35-
return new ProcessorClass(formJson);
36+
processorInstance = new ProcessorClass(formJson);
37+
} else {
38+
console.error(`Form processor ${formJson.processor} not found, defaulting to EncounterFormProcessor`);
39+
processorInstance = new EncounterFormProcessor(formJson);
3640
}
37-
console.error(`Form processor ${formJson.processor} not found, defaulting to EncounterFormProcessor`);
38-
return new EncounterFormProcessor(formJson);
41+
processorInstance.prepareFormSchema(formJson);
42+
return processorInstance;
3943
}, [formProcessors, formJson.processor]);
4044

4145
const [processorContext, setProcessorContext] = useState<FormProcessorContextProps>({

‎src/processors/encounter/encounter-form-processor.test.ts‎

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
1+
import { EncounterFormProcessor } from './encounter-form-processor';
2+
import { type FormSchema } from '../../types';
3+
4+
describe('EncounterFormProcessor', () => {
5+
describe('prepareFormSchema - validateCalculateExpressions', () => {
6+
let processor: EncounterFormProcessor;
7+
let consoleSpy: jest.SpyInstance;
8+
9+
beforeEach(() => {
10+
processor = new EncounterFormProcessor(null);
11+
consoleSpy = jest.spyOn(console, 'error').mockImplementation();
12+
});
13+
14+
afterEach(() => {
15+
consoleSpy.mockRestore();
16+
});
17+
18+
it('should warn when a calculateExpression contains a quoted string that matches a field ID', () => {
19+
const schema: FormSchema = {
20+
name: 'Test Form',
21+
pages: [
22+
{
23+
label: 'Page 1',
24+
sections: [
25+
{
26+
label: 'Section 1',
27+
isExpanded: 'true',
28+
questions: [
29+
{
30+
label: 'Last Menstrual Period',
31+
type: 'obs',
32+
id: 'lmp',
33+
questionOptions: {
34+
rendering: 'date',
35+
concept: 'test-concept',
36+
},
37+
},
38+
{
39+
label: 'Expected Date of Delivery',
40+
type: 'obs',
41+
id: 'edd',
42+
questionOptions: {
43+
rendering: 'date',
44+
concept: 'test-concept',
45+
calculate: {
46+
calculateExpression: "calcEDD('lmp')",
47+
},
48+
},
49+
},
50+
],
51+
},
52+
],
53+
},
54+
],
55+
processor: 'EncounterFormProcessor',
56+
encounterType: 'test-encounter-type',
57+
referencedForms: [],
58+
uuid: 'test-form-uuid',
59+
} as unknown as FormSchema;
60+
61+
processor.prepareFormSchema(schema);
62+
63+
expect(consoleSpy).toHaveBeenCalledWith(
64+
expect.stringContaining("incorrectly quotes the field ID 'lmp' as a string"),
65+
);
66+
});
67+
68+
it('should not warn when a calculateExpression uses bare variable references', () => {
69+
const schema: FormSchema = {
70+
name: 'Test Form',
71+
pages: [
72+
{
73+
label: 'Page 1',
74+
sections: [
75+
{
76+
label: 'Section 1',
77+
isExpanded: 'true',
78+
questions: [
79+
{
80+
label: 'Last Menstrual Period',
81+
type: 'obs',
82+
id: 'lmp',
83+
questionOptions: {
84+
rendering: 'date',
85+
concept: 'test-concept',
86+
},
87+
},
88+
{
89+
label: 'Expected Date of Delivery',
90+
type: 'obs',
91+
id: 'edd',
92+
questionOptions: {
93+
rendering: 'date',
94+
concept: 'test-concept',
95+
calculate: {
96+
calculateExpression: 'calcEDD(lmp)',
97+
},
98+
},
99+
},
100+
],
101+
},
102+
],
103+
},
104+
],
105+
processor: 'EncounterFormProcessor',
106+
encounterType: 'test-encounter-type',
107+
referencedForms: [],
108+
uuid: 'test-form-uuid',
109+
} as unknown as FormSchema;
110+
111+
processor.prepareFormSchema(schema);
112+
113+
expect(consoleSpy).not.toHaveBeenCalled();
114+
});
115+
116+
it('should not warn when a quoted string does not match any field ID', () => {
117+
const schema: FormSchema = {
118+
name: 'Test Form',
119+
pages: [
120+
{
121+
label: 'Page 1',
122+
sections: [
123+
{
124+
label: 'Section 1',
125+
isExpanded: 'true',
126+
questions: [
127+
{
128+
label: 'Duration',
129+
type: 'obs',
130+
id: 'duration',
131+
questionOptions: {
132+
rendering: 'number',
133+
concept: 'test-concept',
134+
calculate: {
135+
calculateExpression: "calcTimeDifference(onsetDate, 'd')",
136+
},
137+
},
138+
},
139+
],
140+
},
141+
],
142+
},
143+
],
144+
processor: 'EncounterFormProcessor',
145+
encounterType: 'test-encounter-type',
146+
referencedForms: [],
147+
uuid: 'test-form-uuid',
148+
} as unknown as FormSchema;
149+
150+
processor.prepareFormSchema(schema);
151+
152+
// 'd' is not a field ID, so no warning should be issued
153+
expect(consoleSpy).not.toHaveBeenCalled();
154+
});
155+
156+
it('should warn for nested questions in obsGroups', () => {
157+
const schema: FormSchema = {
158+
name: 'Test Form',
159+
pages: [
160+
{
161+
label: 'Page 1',
162+
sections: [
163+
{
164+
label: 'Section 1',
165+
isExpanded: 'true',
166+
questions: [
167+
{
168+
label: 'Height',
169+
type: 'obs',
170+
id: 'height',
171+
questionOptions: {
172+
rendering: 'number',
173+
concept: 'test-concept',
174+
},
175+
},
176+
{
177+
label: 'Weight',
178+
type: 'obs',
179+
id: 'weight',
180+
questionOptions: {
181+
rendering: 'number',
182+
concept: 'test-concept',
183+
},
184+
},
185+
{
186+
label: 'Vitals Group',
187+
type: 'obsGroup',
188+
id: 'vitalsGroup',
189+
questionOptions: {
190+
rendering: 'group',
191+
concept: 'test-concept',
192+
},
193+
questions: [
194+
{
195+
label: 'BMI',
196+
type: 'obs',
197+
id: 'bmi',
198+
questionOptions: {
199+
rendering: 'number',
200+
concept: 'test-concept',
201+
calculate: {
202+
calculateExpression: "calcBMI('height', 'weight')",
203+
},
204+
},
205+
},
206+
],
207+
},
208+
],
209+
},
210+
],
211+
},
212+
],
213+
processor: 'EncounterFormProcessor',
214+
encounterType: 'test-encounter-type',
215+
referencedForms: [],
216+
uuid: 'test-form-uuid',
217+
} as unknown as FormSchema;
218+
219+
processor.prepareFormSchema(schema);
220+
221+
expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining("incorrectly quotes the field ID 'height' as a string"));
222+
expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining("incorrectly quotes the field ID 'weight' as a string"));
223+
});
224+
});
225+
});

‎src/processors/encounter/encounter-form-processor.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ const contextInitializableTypes = [
8181

8282
export class EncounterFormProcessor extends FormProcessor {
8383
prepareFormSchema(schema: FormSchema) {
84+
const allFieldIds = new Set<string>();
85+
8486
schema.pages.forEach((page) => {
8587
page.sections.forEach((section) => {
8688
section.questions.forEach((question) => {
@@ -90,6 +92,11 @@ export class EncounterFormProcessor extends FormProcessor {
9092
});
9193

9294
function prepareFormField(field: FormField, section: FormSection, page: FormPage, schema: FormSchema) {
95+
// Collect field ID
96+
if (field.id) {
97+
allFieldIds.add(field.id);
98+
}
99+
93100
// inherit inlineRendering and readonly from parent section and page if not set
94101
field.inlineRendering =
95102
field.inlineRendering ?? section.inlineRendering ?? page.inlineRendering ?? schema.inlineRendering;
@@ -106,6 +113,9 @@ export class EncounterFormProcessor extends FormProcessor {
106113
}
107114
}
108115

116+
// Validate calculate expressions for common mistakes
117+
validateCalculateExpressions(schema, allFieldIds);
118+
109119
return schema;
110120
}
111121

@@ -368,3 +378,43 @@ async function evaluateCalculateExpression(
368378
values[field.id] = value;
369379
}
370380
}
381+
382+
/**
383+
* Validates calculate expressions to warn about common mistakes.
384+
* Specifically, checks if string literals in expressions match field IDs,
385+
* which usually indicates the user should use bare variable references instead.
386+
*
387+
* For example: calcEDD('lmp') should be calcEDD(lmp)
388+
*/
389+
function validateCalculateExpressions(schema: FormSchema, allFieldIds: Set<string>) {
390+
const stringLiteralPattern = /(['"])([a-zA-Z_][a-zA-Z0-9_]*)\1/g;
391+
392+
function checkExpression(expression: string, fieldId: string) {
393+
for (const match of expression.matchAll(stringLiteralPattern)) {
394+
const quotedValue = match[2];
395+
if (allFieldIds.has(quotedValue)) {
396+
console.error(
397+
`The calculateExpression for the field '${fieldId}' incorrectly quotes the field ID '${quotedValue}' as a string. ` +
398+
`Field IDs must be referenced as variables without quotes to access their values. ` +
399+
`Remove the quotes: use ${quotedValue} instead of '${quotedValue}'.`,
400+
);
401+
}
402+
}
403+
}
404+
405+
function processField(field: FormField) {
406+
if (field.questionOptions?.calculate?.calculateExpression) {
407+
checkExpression(field.questionOptions.calculate.calculateExpression, field.id);
408+
}
409+
// Process nested questions (for obsGroups)
410+
if (field.questions) {
411+
field.questions.forEach(processField);
412+
}
413+
}
414+
415+
schema.pages.forEach((page) => {
416+
page.sections.forEach((section) => {
417+
section.questions.forEach(processField);
418+
});
419+
});
420+
}

0 commit comments

Comments
 (0)