From d3b66110a51882a8475bc44bbffebac8c55c712a Mon Sep 17 00:00:00 2001 From: kevinkim-ogp Date: Tue, 24 Dec 2024 12:37:50 +0800 Subject: [PATCH] PLU-393: Variables: keys with . may be single key or nested path (#831) ### TL;DR New MyInfo Child fields from FormSG returns data with `.` as part of the key (e.g., `child.birthdate`). Libraries like `lodash.get` treat `.` as a path separator for nested properties (e.g., accessing `child.birthdate`), leading to incorrect value retrieval or failure. ### What changed? - Replaced the `.` in keys with `_` to avoid issues with obtaining values. ### How to test? 1. Run the test suite to verify all scenarios pass 2. Test with actual FormSG fields from MyInfo Child ### Why make this change? The standard lodash.get function doesn't properly handle cases where object keys contain dots, leading to incorrect property access. This implementation provides more accurate property resolution. --- .../auth/decrypt-form-response.test.ts | 92 +++++++++++++++++++ .../apps/formsg/auth/decrypt-form-response.ts | 4 +- .../__tests__/compute-parameters.test.ts | 30 ++++++ 3 files changed, 125 insertions(+), 1 deletion(-) diff --git a/packages/backend/src/apps/formsg/__tests__/auth/decrypt-form-response.test.ts b/packages/backend/src/apps/formsg/__tests__/auth/decrypt-form-response.test.ts index a4556dd6c..f7af4be4d 100644 --- a/packages/backend/src/apps/formsg/__tests__/auth/decrypt-form-response.test.ts +++ b/packages/backend/src/apps/formsg/__tests__/auth/decrypt-form-response.test.ts @@ -410,6 +410,98 @@ describe('decrypt form response', () => { }), ) }) + + it('should parse form fields and replace dots with underscores in keys', async () => { + mockDecryptedSubmission({ + responses: [ + { + _id: 'question1.field.answer', + fieldType: 'textarea', + question: 'What do you eat for breakfast?', + answer: 'i eat lorem dimsum for breakfast', + }, + { + _id: 'question2.field.answer', + fieldType: 'mobile', + question: 'What is your mobile number?', + answer: '+6591234567', + }, + ], + }) + await expect(decryptFormResponse($)).resolves.toEqual(true) + expect($.request.body).toEqual( + expect.objectContaining({ + fields: { + question1_field_answer: { + fieldType: 'textarea', + question: 'What do you eat for breakfast?', + answer: 'i eat lorem dimsum for breakfast', + order: 1, + }, + question2_field_answer: { + fieldType: 'mobile', + question: 'What is your mobile number?', + answer: '+6591234567', + order: 2, + }, + }, + }), + ) + expect($.request.headers).toBeUndefined() + expect($.request.query).toBeUndefined() + }) + + it('should parse form fields and replace dots with underscores in keys', async () => { + mockDecryptedSubmission({ + responses: [ + { + _id: 'childrenbirthrecords.abc.childdateofbirth.0', + fieldType: 'children', + question: 'Child Date of birth', + answer: '31/03/2017', + }, + { + _id: 'childrenbirthrecords.abc.childname.0', + fieldType: 'children', + question: 'Child Name', + answer: 'John Doe', + }, + { + _id: 'question2.field.answer', + fieldType: 'mobile', + question: 'What is your mobile number?', + answer: '+6591234567', + }, + ], + }) + await expect(decryptFormResponse($)).resolves.toEqual(true) + expect($.request.body).toEqual( + expect.objectContaining({ + fields: { + childrenbirthrecords_abc_childdateofbirth_0: { + fieldType: 'children', + question: 'Child Date of birth', + answer: '31/03/2017', + order: 1, + }, + childrenbirthrecords_abc_childname_0: { + fieldType: 'children', + question: 'Child Name', + answer: 'John Doe', + order: 2, + }, + question2_field_answer: { + fieldType: 'mobile', + question: 'What is your mobile number?', + answer: '+6591234567', + order: 3, + }, + }, + }), + ) + expect($.request.headers).toBeUndefined() + expect($.request.query).toBeUndefined() + }) }) describe('attachments', () => { diff --git a/packages/backend/src/apps/formsg/auth/decrypt-form-response.ts b/packages/backend/src/apps/formsg/auth/decrypt-form-response.ts index 2442a4c45..7a29a3134 100644 --- a/packages/backend/src/apps/formsg/auth/decrypt-form-response.ts +++ b/packages/backend/src/apps/formsg/auth/decrypt-form-response.ts @@ -121,7 +121,9 @@ export async function decryptFormResponse( // Note: the order may not be sequential; fields (e.g. NRIC) can be // omitted from the output. - parsedData[_id] = { + // Note: FormSG uses dot notation for field ids for MyInfo children + // we replace with underscore to avoid issues when using lodash get. + parsedData[_id.replaceAll('.', '_')] = { order: index + 1, ...rest, } diff --git a/packages/backend/src/helpers/__tests__/compute-parameters.test.ts b/packages/backend/src/helpers/__tests__/compute-parameters.test.ts index 0a23df935..0caaf1101 100644 --- a/packages/backend/src/helpers/__tests__/compute-parameters.test.ts +++ b/packages/backend/src/helpers/__tests__/compute-parameters.test.ts @@ -319,4 +319,34 @@ describe('compute parameters', () => { const result = computeParameters(params, executionStep) expect(result).toEqual(expected) }) + + it('should process parameters with underscores in keys', () => { + const executionStep = [ + { + stepId: randomStepID, + dataOut: { + childrenbirthrecords_abc_childdateofbirth_0: { + fieldType: 'children', + question: 'Child Date of birth', + answer: '31/03/2017', + order: 1, + }, + childrenbirthrecords_abc_childname_0: { + fieldType: 'children', + question: 'Child Name', + answer: 'John Doe', + order: 2, + }, + }, + } as unknown as ExecutionStep, + ] + const params = { + toSubstitute: `{{step.${randomStepID}.childrenbirthrecords_abc_childdateofbirth_0.answer}} {{step.${randomStepID}.childrenbirthrecords_abc_childname_0.answer}}`, + } + const expected = { + toSubstitute: `31/03/2017 John Doe`, + } + const result = computeParameters(params, executionStep) + expect(result).toEqual(expected) + }) })