From 6512a84c6456665e6f12a8fad5a7e8ccd9860d28 Mon Sep 17 00:00:00 2001 From: Foo Yong Jie Date: Tue, 22 Jun 2021 13:56:53 +0800 Subject: [PATCH] refactor(email-submission): encapsulate parsedResponses (#2206) --- .../__tests__/admin-form.controller.spec.ts | 49 +-- .../form/admin-form/admin-form.controller.ts | 38 +-- .../submission/submission.service.spec.ts | 295 +----------------- .../ParsedResponsesObject.class.ts | 153 +++++++++ .../ParsedResponsesObject.class.spec.ts | 124 ++++++++ .../email-submission.controller.ts | 46 +-- .../email-submission.types.ts | 6 +- .../modules/submission/submission.service.ts | 112 +------ 8 files changed, 355 insertions(+), 468 deletions(-) create mode 100644 src/app/modules/submission/email-submission/ParsedResponsesObject.class.ts create mode 100644 src/app/modules/submission/email-submission/__tests__/ParsedResponsesObject.class.spec.ts diff --git a/src/app/modules/form/admin-form/__tests__/admin-form.controller.spec.ts b/src/app/modules/form/admin-form/__tests__/admin-form.controller.spec.ts index 05a20e680d..3fefae6160 100644 --- a/src/app/modules/form/admin-form/__tests__/admin-form.controller.spec.ts +++ b/src/app/modules/form/admin-form/__tests__/admin-form.controller.spec.ts @@ -77,6 +77,7 @@ import { } from 'tests/unit/backend/helpers/generate-form-data' import expressHandler from 'tests/unit/backend/helpers/jest-express' +import ParsedResponsesObject from '../../../submission/email-submission/ParsedResponsesObject.class' import * as UserService from '../../../user/user.service' import { ForbiddenFormError, @@ -127,6 +128,8 @@ jest.mock('src/app/modules/submission/submission.utils') const MockSubmissionUtils = mocked(SubmissionUtils) jest.mock('../admin-form.service') const MockAdminFormService = mocked(AdminFormService) +jest.mock('../../../submission/email-submission/ParsedResponsesObject.class') +const MockParsedResponsesObject = mocked(ParsedResponsesObject) jest.mock('../../form.service') const MockFormService = mocked(FormService) jest.mock('../../../user/user.service') @@ -5130,8 +5133,12 @@ describe('admin-form.controller', () => { MockEmailSubmissionService.validateAttachments.mockReturnValue( okAsync(true), ) - MockSubmissionService.getProcessedResponses.mockReturnValue( - ok(MOCK_PARSED_RESPONSES), + MockParsedResponsesObject.mockClear() + MockParsedResponsesObject.parseResponses.mockReturnValue( + ok(new ParsedResponsesObject(MOCK_PARSED_RESPONSES)), + ) + MockParsedResponsesObject.mock.instances[0].getAllResponses.mockReturnValue( + MOCK_PARSED_RESPONSES, ) MockEmailSubmissionService.createEmailSubmissionWithoutSave.mockReturnValue( MOCK_SUBMISSION, @@ -5186,7 +5193,7 @@ describe('admin-form.controller', () => { expect( MockEmailSubmissionService.validateAttachments, ).toHaveBeenCalledWith(MOCK_RESPONSES) - expect(MockSubmissionService.getProcessedResponses).toHaveBeenCalledWith( + expect(MockParsedResponsesObject.parseResponses).toHaveBeenCalledWith( MOCK_FORM, MOCK_RESPONSES, ) @@ -5253,7 +5260,7 @@ describe('admin-form.controller', () => { expect( MockEmailSubmissionService.validateAttachments, ).not.toHaveBeenCalled() - expect(MockSubmissionService.getProcessedResponses).not.toHaveBeenCalled() + expect(MockParsedResponsesObject.parseResponses).not.toHaveBeenCalled() expect(MockAdminFormService.extractMyInfoFieldIds).not.toHaveBeenCalled() expect( MockEmailSubmissionService.createEmailSubmissionWithoutSave, @@ -5302,7 +5309,7 @@ describe('admin-form.controller', () => { expect( MockEmailSubmissionService.validateAttachments, ).not.toHaveBeenCalled() - expect(MockSubmissionService.getProcessedResponses).not.toHaveBeenCalled() + expect(MockParsedResponsesObject.parseResponses).not.toHaveBeenCalled() expect(MockAdminFormService.extractMyInfoFieldIds).not.toHaveBeenCalled() expect( MockEmailSubmissionService.createEmailSubmissionWithoutSave, @@ -5355,7 +5362,7 @@ describe('admin-form.controller', () => { expect( MockEmailSubmissionService.validateAttachments, ).not.toHaveBeenCalled() - expect(MockSubmissionService.getProcessedResponses).not.toHaveBeenCalled() + expect(MockParsedResponsesObject.parseResponses).not.toHaveBeenCalled() expect(MockAdminFormService.extractMyInfoFieldIds).not.toHaveBeenCalled() expect( MockEmailSubmissionService.createEmailSubmissionWithoutSave, @@ -5408,7 +5415,7 @@ describe('admin-form.controller', () => { expect( MockEmailSubmissionService.validateAttachments, ).not.toHaveBeenCalled() - expect(MockSubmissionService.getProcessedResponses).not.toHaveBeenCalled() + expect(MockParsedResponsesObject.parseResponses).not.toHaveBeenCalled() expect(MockAdminFormService.extractMyInfoFieldIds).not.toHaveBeenCalled() expect( MockEmailSubmissionService.createEmailSubmissionWithoutSave, @@ -5461,7 +5468,7 @@ describe('admin-form.controller', () => { expect( MockEmailSubmissionService.validateAttachments, ).not.toHaveBeenCalled() - expect(MockSubmissionService.getProcessedResponses).not.toHaveBeenCalled() + expect(MockParsedResponsesObject.parseResponses).not.toHaveBeenCalled() expect(MockAdminFormService.extractMyInfoFieldIds).not.toHaveBeenCalled() expect( MockEmailSubmissionService.createEmailSubmissionWithoutSave, @@ -5514,7 +5521,7 @@ describe('admin-form.controller', () => { expect( MockEmailSubmissionService.validateAttachments, ).not.toHaveBeenCalled() - expect(MockSubmissionService.getProcessedResponses).not.toHaveBeenCalled() + expect(MockParsedResponsesObject.parseResponses).not.toHaveBeenCalled() expect(MockAdminFormService.extractMyInfoFieldIds).not.toHaveBeenCalled() expect( MockEmailSubmissionService.createEmailSubmissionWithoutSave, @@ -5567,7 +5574,7 @@ describe('admin-form.controller', () => { expect( MockEmailSubmissionService.validateAttachments, ).not.toHaveBeenCalled() - expect(MockSubmissionService.getProcessedResponses).not.toHaveBeenCalled() + expect(MockParsedResponsesObject.parseResponses).not.toHaveBeenCalled() expect(MockAdminFormService.extractMyInfoFieldIds).not.toHaveBeenCalled() expect( MockEmailSubmissionService.createEmailSubmissionWithoutSave, @@ -5620,7 +5627,7 @@ describe('admin-form.controller', () => { expect( MockEmailSubmissionService.validateAttachments, ).toHaveBeenCalledWith(MOCK_RESPONSES) - expect(MockSubmissionService.getProcessedResponses).not.toHaveBeenCalled() + expect(MockParsedResponsesObject.parseResponses).not.toHaveBeenCalled() expect(MockAdminFormService.extractMyInfoFieldIds).not.toHaveBeenCalled() expect( MockEmailSubmissionService.createEmailSubmissionWithoutSave, @@ -5673,7 +5680,7 @@ describe('admin-form.controller', () => { expect( MockEmailSubmissionService.validateAttachments, ).toHaveBeenCalledWith(MOCK_RESPONSES) - expect(MockSubmissionService.getProcessedResponses).not.toHaveBeenCalled() + expect(MockParsedResponsesObject.parseResponses).not.toHaveBeenCalled() expect(MockAdminFormService.extractMyInfoFieldIds).not.toHaveBeenCalled() expect( MockEmailSubmissionService.createEmailSubmissionWithoutSave, @@ -5692,7 +5699,7 @@ describe('admin-form.controller', () => { }) it('should return 400 when responses cannot be processed', async () => { - MockSubmissionService.getProcessedResponses.mockReturnValueOnce( + MockParsedResponsesObject.parseResponses.mockReturnValueOnce( err(new ProcessingError()), ) const mockReq = expressHandler.mockRequest({ @@ -5726,7 +5733,7 @@ describe('admin-form.controller', () => { expect( MockEmailSubmissionService.validateAttachments, ).toHaveBeenCalledWith(MOCK_RESPONSES) - expect(MockSubmissionService.getProcessedResponses).toHaveBeenCalledWith( + expect(MockParsedResponsesObject.parseResponses).toHaveBeenCalledWith( MOCK_FORM, MOCK_RESPONSES, ) @@ -5748,7 +5755,7 @@ describe('admin-form.controller', () => { }) it('should return 409 when the submitted field IDs do not match the form field IDs', async () => { - MockSubmissionService.getProcessedResponses.mockReturnValueOnce( + MockParsedResponsesObject.parseResponses.mockReturnValueOnce( err(new ConflictError('')), ) const mockReq = expressHandler.mockRequest({ @@ -5782,7 +5789,7 @@ describe('admin-form.controller', () => { expect( MockEmailSubmissionService.validateAttachments, ).toHaveBeenCalledWith(MOCK_RESPONSES) - expect(MockSubmissionService.getProcessedResponses).toHaveBeenCalledWith( + expect(MockParsedResponsesObject.parseResponses).toHaveBeenCalledWith( MOCK_FORM, MOCK_RESPONSES, ) @@ -5804,7 +5811,7 @@ describe('admin-form.controller', () => { }) it('should return 400 when any answer is invalid', async () => { - MockSubmissionService.getProcessedResponses.mockReturnValueOnce( + MockParsedResponsesObject.parseResponses.mockReturnValueOnce( err(new ValidateFieldError()), ) const mockReq = expressHandler.mockRequest({ @@ -5838,7 +5845,7 @@ describe('admin-form.controller', () => { expect( MockEmailSubmissionService.validateAttachments, ).toHaveBeenCalledWith(MOCK_RESPONSES) - expect(MockSubmissionService.getProcessedResponses).toHaveBeenCalledWith( + expect(MockParsedResponsesObject.parseResponses).toHaveBeenCalledWith( MOCK_FORM, MOCK_RESPONSES, ) @@ -5894,7 +5901,7 @@ describe('admin-form.controller', () => { expect( MockEmailSubmissionService.validateAttachments, ).toHaveBeenCalledWith(MOCK_RESPONSES) - expect(MockSubmissionService.getProcessedResponses).toHaveBeenCalledWith( + expect(MockParsedResponsesObject.parseResponses).toHaveBeenCalledWith( MOCK_FORM, MOCK_RESPONSES, ) @@ -5959,7 +5966,7 @@ describe('admin-form.controller', () => { expect( MockEmailSubmissionService.validateAttachments, ).toHaveBeenCalledWith(MOCK_RESPONSES) - expect(MockSubmissionService.getProcessedResponses).toHaveBeenCalledWith( + expect(MockParsedResponsesObject.parseResponses).toHaveBeenCalledWith( MOCK_FORM, MOCK_RESPONSES, ) @@ -6024,7 +6031,7 @@ describe('admin-form.controller', () => { expect( MockEmailSubmissionService.validateAttachments, ).toHaveBeenCalledWith(MOCK_RESPONSES) - expect(MockSubmissionService.getProcessedResponses).toHaveBeenCalledWith( + expect(MockParsedResponsesObject.parseResponses).toHaveBeenCalledWith( MOCK_FORM, MOCK_RESPONSES, ) diff --git a/src/app/modules/form/admin-form/admin-form.controller.ts b/src/app/modules/form/admin-form/admin-form.controller.ts index 3be70728b3..311deac41a 100644 --- a/src/app/modules/form/admin-form/admin-form.controller.ts +++ b/src/app/modules/form/admin-form/admin-form.controller.ts @@ -50,10 +50,6 @@ import { } from '../../core/core.errors' import { ControllerHandler } from '../../core/core.types' import * as FeedbackService from '../../feedback/feedback.service' -import { - createCorppassParsedResponses, - createSingpassParsedResponses, -} from '../../spcp/spcp.util' import * as EmailSubmissionMiddleware from '../../submission/email-submission/email-submission.middleware' import * as EmailSubmissionService from '../../submission/email-submission/email-submission.service' import { @@ -61,6 +57,7 @@ import { mapRouteError as mapEmailSubmissionError, SubmissionEmailObj, } from '../../submission/email-submission/email-submission.util' +import ParsedResponsesObject from '../../submission/email-submission/ParsedResponsesObject.class' import * as EncryptSubmissionMiddleware from '../../submission/encrypt-submission/encrypt-submission.middleware' import * as EncryptSubmissionService from '../../submission/encrypt-submission/encrypt-submission.service' import { mapRouteError as mapEncryptSubmissionError } from '../../submission/encrypt-submission/encrypt-submission.utils' @@ -1512,7 +1509,7 @@ export const submitEmailPreview: ControllerHandler< const parsedResponsesResult = await EmailSubmissionService.validateAttachments(responses).andThen(() => - SubmissionService.getProcessedResponses(form, responses), + ParsedResponsesObject.parseResponses(form, responses), ) if (parsedResponsesResult.isErr()) { logger.error({ @@ -1529,21 +1526,22 @@ export const submitEmailPreview: ControllerHandler< const attachments = mapAttachmentsFromResponses(req.body.responses) // Handle SingPass, CorpPass and MyInfo authentication and validation - if (form.authType === AuthType.SP || form.authType === AuthType.MyInfo) { - parsedResponses.push( - ...createSingpassParsedResponses(PREVIEW_SINGPASS_UINFIN), - ) - } else if (form.authType === AuthType.CP) { - parsedResponses.push( - ...createCorppassParsedResponses( - PREVIEW_CORPPASS_UINFIN, - PREVIEW_CORPPASS_UID, - ), - ) + const { authType } = form + if (authType === AuthType.SP || authType === AuthType.MyInfo) { + parsedResponses.addNdiResponses({ + authType, + uinFin: PREVIEW_SINGPASS_UINFIN, + }) + } else if (authType === AuthType.CP) { + parsedResponses.addNdiResponses({ + authType, + uinFin: PREVIEW_CORPPASS_UINFIN, + userInfo: PREVIEW_CORPPASS_UID, + }) } const emailData = new SubmissionEmailObj( - parsedResponses, + parsedResponses.getAllResponses(), // All MyInfo fields are verified in preview new Set(AdminFormService.extractMyInfoFieldIds(form.form_fields)), form.authType, @@ -1556,7 +1554,9 @@ export const submitEmailPreview: ControllerHandler< ) const sendAdminEmailResult = await MailService.sendSubmissionToAdmin({ - replyToEmails: EmailSubmissionService.extractEmailAnswers(parsedResponses), + replyToEmails: EmailSubmissionService.extractEmailAnswers( + parsedResponses.getAllResponses(), + ), form, submission, attachments, @@ -1585,7 +1585,7 @@ export const submitEmailPreview: ControllerHandler< attachments, responsesData: emailData.autoReplyData, recipientData: extractEmailConfirmationData( - parsedResponses, + parsedResponses.getAllResponses(), form.form_fields, ), }).mapErr((error) => { diff --git a/src/app/modules/submission/__tests__/submission/submission.service.spec.ts b/src/app/modules/submission/__tests__/submission/submission.service.spec.ts index db1cf95366..93dd37a40d 100644 --- a/src/app/modules/submission/__tests__/submission/submission.service.spec.ts +++ b/src/app/modules/submission/__tests__/submission/submission.service.spec.ts @@ -9,40 +9,26 @@ import { MalformedParametersError, } from 'src/app/modules/core/core.errors' import * as SubmissionService from 'src/app/modules/submission/submission.service' -import { ProcessedFieldResponse } from 'src/app/modules/submission/submission.types' import MailService from 'src/app/services/mail/mail.service' import { createQueryWithDateParam } from 'src/app/utils/date' -import * as LogicUtil from 'src/shared/util/logic' import { AutoReplyOptions, BasicField, IAttachmentInfo, - IEmailFormSchema, IEmailSubmissionSchema, - IEncryptedFormSchema, IEncryptedSubmissionSchema, IFormSchema, - IPreventSubmitLogicSchema, ISubmissionSchema, - LogicType, - ResponseMode, SubmissionType, } from 'src/types' import { generateDefaultField, generateNewSingleAnswerResponse, - generateProcessedSingleAnswerResponse, - generateSingleAnswerResponse, } from 'tests/unit/backend/helpers/generate-form-data' import dbHandler from 'tests/unit/backend/helpers/jest-db' -import { - ConflictError, - ProcessingError, - SendEmailConfirmationError, - ValidateFieldError, -} from '../../submission.errors' +import { SendEmailConfirmationError } from '../../submission.errors' import { extractEmailConfirmationData } from '../../submission.utils' jest.mock('src/app/services/mail/mail.service') @@ -109,285 +95,6 @@ describe('submission.service', () => { afterAll(async () => await dbHandler.closeDatabase()) beforeEach(() => jest.clearAllMocks()) - describe('getProcessedResponses', () => { - it('should return list of parsed responses for encrypted form submission successfully', async () => { - // Arrange - // Only mobile and email fields are parsed, since the other fields are - // e2e encrypted from the browser. - const mobileField = generateDefaultField(BasicField.Mobile) - const emailField = generateDefaultField(BasicField.Email) - // Add answers to both mobile and email fields - const mobileResponse = generateSingleAnswerResponse( - mobileField, - '+6587654321', - ) - const emailResponse = generateSingleAnswerResponse( - emailField, - 'test@example.com', - ) - - const mobileProcessedResponse = generateProcessedSingleAnswerResponse( - mobileField, - '+6587654321', - ) - const emailProcessedResponse = generateProcessedSingleAnswerResponse( - emailField, - 'test@example.com', - ) - - // Act - const actualResult = SubmissionService.getProcessedResponses( - { - responseMode: ResponseMode.Encrypt, - form_fields: [mobileField, emailField], - } as unknown as IFormSchema, - [mobileResponse, emailResponse], - ) - - // Assert - const expectedParsed: ProcessedFieldResponse[] = [ - { ...mobileProcessedResponse, isVisible: true }, - { ...emailProcessedResponse, isVisible: true }, - ] - // Should only have email and mobile fields for encrypted forms. - expect(actualResult.isOk()).toEqual(true) - expect(actualResult._unsafeUnwrap()).toEqual(expectedParsed) - }) - - it('should return list of parsed responses for email form submission successfully', async () => { - // Arrange - // Add answer to subset of field types - const shortTextField = generateDefaultField(BasicField.ShortText) - const decimalField = generateDefaultField(BasicField.Decimal) - - // Add answers to both mobile and email fields - const shortTextResponse = generateSingleAnswerResponse( - shortTextField, - 'the quick brown fox jumps over the lazy dog', - ) - const decimalResponse = generateSingleAnswerResponse( - decimalField, - '3.142', - ) - - const shortTextProcessedResponse = generateProcessedSingleAnswerResponse( - shortTextField, - 'the quick brown fox jumps over the lazy dog', - ) - const decimalProcessedResponse = generateProcessedSingleAnswerResponse( - decimalField, - '3.142', - ) - - // Act - const actualResult = SubmissionService.getProcessedResponses( - { - responseMode: ResponseMode.Email, - form_fields: [shortTextField, decimalField], - } as unknown as IFormSchema, - [shortTextResponse, decimalResponse], - ) - - // Assert - const expectedParsed: ProcessedFieldResponse[] = [ - { ...shortTextProcessedResponse, isVisible: true }, - { ...decimalProcessedResponse, isVisible: true }, - ] - - expect(actualResult.isOk()).toEqual(true) - expect(actualResult._unsafeUnwrap()).toEqual(expectedParsed) - }) - - it('should return error when email form has more fields than responses', async () => { - // Arrange - const extraField = generateDefaultField(BasicField.Mobile) - - // Act + Assert - - const actualResult = SubmissionService.getProcessedResponses( - { - responseMode: ResponseMode.Email, - form_fields: [extraField], - } as unknown as IEmailFormSchema, - [], - ) - - expect(actualResult.isErr()).toEqual(true) - expect(actualResult._unsafeUnwrapErr()).toEqual( - new ConflictError('Some form fields are missing'), - ) - }) - - it('should return error when encrypt form has more fields than responses', async () => { - // Arrange - const extraField = generateDefaultField(BasicField.Mobile) - - // Act + Assert - - const actualResult = SubmissionService.getProcessedResponses( - { - responseMode: ResponseMode.Encrypt, - form_fields: [extraField], - } as unknown as IEncryptedFormSchema, - [], - ) - - expect(actualResult.isErr()).toEqual(true) - expect(actualResult._unsafeUnwrapErr()).toEqual( - new ConflictError('Some form fields are missing'), - ) - }) - it('should allow responses for encrypt mode hidden fields', async () => { - // Arrange - // Only check for mobile and email fields, since the other fields are - // e2e encrypted from the browser. - const mobileField = generateDefaultField(BasicField.Mobile) - const emailField = generateDefaultField(BasicField.Email) - // Add answers to both mobile and email fields - const mobileResponse = generateSingleAnswerResponse( - mobileField, - '+6587654321', - ) - - const emailResponse = generateSingleAnswerResponse( - emailField, - 'test@example.com', - ) - - const mobileProcessedResponse = generateProcessedSingleAnswerResponse( - mobileField, - '+6587654321', - ) - mobileProcessedResponse.isVisible = false - - const emailProcessedResponse = generateProcessedSingleAnswerResponse( - emailField, - 'test@example.com', - ) - emailProcessedResponse.isVisible = false - - // Act - const actualResult = SubmissionService.getProcessedResponses( - { - responseMode: ResponseMode.Encrypt, - form_fields: [mobileField, emailField], - } as unknown as IFormSchema, - [mobileResponse, emailResponse], - ) - - // Assert - const expectedParsed: ProcessedFieldResponse[] = [ - { ...mobileProcessedResponse, isVisible: true }, //getProcessedResponses sets isVisible to be true for encrypt mode - { ...emailProcessedResponse, isVisible: true }, - ] - // Should only have email and mobile fields for encrypted forms. - expect(actualResult.isOk()).toEqual(true) - expect(actualResult._unsafeUnwrap()).toEqual(expectedParsed) - }) - - it('should return error when any responses are not valid for encrypted form submission', async () => { - // Arrange - // Only mobile and email fields are parsed, since the other fields are - // e2e encrypted from the browser. - const mobileField = generateDefaultField(BasicField.Mobile) - const mobileResponse = generateSingleAnswerResponse( - mobileField, - 'invalid', - ) - - // Act + Assert - const actualResult = SubmissionService.getProcessedResponses( - { - responseMode: ResponseMode.Encrypt, - form_fields: [mobileField], - } as unknown as IEncryptedFormSchema, - [mobileResponse], - ) - - expect(actualResult.isErr()).toEqual(true) - expect(actualResult._unsafeUnwrapErr()).toEqual( - new ValidateFieldError('Invalid answer submitted'), - ) - }) - - it('should return error when any responses are not valid for email form submission', async () => { - // Arrange - // Set NRIC field in form as required. - const nricField = generateDefaultField(BasicField.Nric) - const nricResponse = generateSingleAnswerResponse(nricField, 'invalid') - - // Act + Assert - const actualResult = SubmissionService.getProcessedResponses( - { - responseMode: ResponseMode.Email, - form_fields: [nricField], - } as unknown as IEmailFormSchema, - [nricResponse], - ) - - expect(actualResult.isErr()).toEqual(true) - expect(actualResult._unsafeUnwrapErr()).toEqual( - new ValidateFieldError('Invalid answer submitted'), - ) - }) - - it('should return error when encrypted form submission is prevented by logic', async () => { - // Arrange - // Mock logic util to return non-empty to check if error is thrown - jest - .spyOn(LogicUtil, 'getLogicUnitPreventingSubmit') - .mockReturnValueOnce({ - preventSubmitMessage: 'mock prevent submit', - conditions: [], - logicType: LogicType.PreventSubmit, - _id: 'some id', - } as unknown as IPreventSubmitLogicSchema) - - // Act + Assert - const actualResult = SubmissionService.getProcessedResponses( - { - responseMode: ResponseMode.Encrypt, - form_fields: [], - } as unknown as IEncryptedFormSchema, - [], - ) - - expect(actualResult.isErr()).toEqual(true) - expect(actualResult._unsafeUnwrapErr()).toEqual( - new ProcessingError('Submission prevented by form logic'), - ) - }) - - it('should return error when email form submission is prevented by logic', async () => { - // Arrange - // Mock logic util to return non-empty to check if error is thrown. - const mockReturnLogicUnit = { - preventSubmitMessage: 'mock prevent submit', - conditions: [], - logicType: LogicType.PreventSubmit, - _id: 'some id', - } as unknown as IPreventSubmitLogicSchema - - jest - .spyOn(LogicUtil, 'getLogicUnitPreventingSubmit') - .mockReturnValueOnce(mockReturnLogicUnit) - - // Act + Assert - const actualResult = SubmissionService.getProcessedResponses( - { - responseMode: ResponseMode.Email, - form_fields: [], - } as unknown as IEmailFormSchema, - [], - ) - - expect(actualResult.isErr()).toEqual(true) - expect(actualResult._unsafeUnwrapErr()).toEqual( - new ProcessingError('Submission prevented by form logic'), - ) - }) - }) - describe('getFormSubmissionsCount', () => { const countSpy = jest.spyOn(Submission, 'countDocuments') const MOCK_FORM_ID = new ObjectId() diff --git a/src/app/modules/submission/email-submission/ParsedResponsesObject.class.ts b/src/app/modules/submission/email-submission/ParsedResponsesObject.class.ts new file mode 100644 index 0000000000..0a3d899220 --- /dev/null +++ b/src/app/modules/submission/email-submission/ParsedResponsesObject.class.ts @@ -0,0 +1,153 @@ +import { err, ok, Result } from 'neverthrow' + +import { + getLogicUnitPreventingSubmit, + getVisibleFieldIds, +} from '../../../../shared/util/logic' +import { + AuthType, + FieldResponse, + IFieldSchema, + IFormDocument, + ResponseMode, +} from '../../../../types' +import { validateField } from '../../../utils/field-validation' +import { + createCorppassParsedResponses, + createSingpassParsedResponses, +} from '../../spcp/spcp.util' +import { + ConflictError, + ProcessingError, + ValidateFieldError, +} from '../submission.errors' +import { ProcessedFieldResponse } from '../submission.types' +import { getFilteredResponses } from '../submission.utils' + +type NdiUserInfo = + | { authType: AuthType.SP | AuthType.MyInfo; uinFin: string } + | { authType: AuthType.CP; uinFin: string; userInfo: string } + +export default class ParsedResponsesObject { + public ndiResponses: ProcessedFieldResponse[] = [] + private constructor(public responses: ProcessedFieldResponse[]) {} + + addNdiResponses(info: NdiUserInfo): ParsedResponsesObject { + /** + * No typescript destructuring being done in switch statement + * because typescript isn't smart enough to do narrowing with + * destructured variable switch cases. + */ + switch (info.authType) { + case AuthType.SP: + case AuthType.MyInfo: + this.ndiResponses = createSingpassParsedResponses(info.uinFin) + break + case AuthType.CP: + this.ndiResponses = createCorppassParsedResponses( + info.uinFin, + info.userInfo, + ) + break + } + return this + } + + getAllResponses(): ProcessedFieldResponse[] { + return [...this.responses, ...this.ndiResponses] + } + + /** + * Injects response metadata such as the question, visibility state. In + * addition, validation such as input validation or signature validation on + * verified fields are also performed on the response. + * @param form The form document corresponding to the responses + * @param responses The responses to process and validate + * @returns neverthrow ok() with field responses with additional metadata injected. + * @returns neverthrow err() if response validation fails + */ + static parseResponses( + form: IFormDocument, + responses: FieldResponse[], + ): Result< + ParsedResponsesObject, + ProcessingError | ConflictError | ValidateFieldError + > { + const filteredResponsesResult = getFilteredResponses(form, responses) + if (filteredResponsesResult.isErr()) { + return err(filteredResponsesResult.error) + } + + const filteredResponses = filteredResponsesResult.value + + // Set of all visible fields + const visibleFieldIds = getVisibleFieldIds(filteredResponses, form) + + // Guard against invalid form submissions that should have been prevented by + // logic. + if ( + getLogicUnitPreventingSubmit(filteredResponses, form, visibleFieldIds) + ) { + return err(new ProcessingError('Submission prevented by form logic')) + } + + // Create a map keyed by field._id for easier access + + if (!form.form_fields) { + return err(new ProcessingError('Form fields are undefined')) + } + + const fieldMap = form.form_fields.reduce<{ + [fieldId: string]: IFieldSchema + }>((acc, field) => { + acc[field._id] = field + return acc + }, {}) + + // Validate each field in the form and inject metadata into the responses. + const processedResponses = [] + for (const response of filteredResponses) { + const responseId = response._id + const formField = fieldMap[responseId] + if (!formField) { + return err( + new ProcessingError('Response ID does not match form field IDs'), + ) + } + + const processingResponse: ProcessedFieldResponse = { + ...response, + isVisible: + // Set isVisible as true for Encrypt mode if there is a response for mobile and email field + // Because we cannot tell if the field is unhidden by logic + // This prevents downstream validateField from incorrectly preventing + // encrypt mode submissions with responses on unhidden fields + // TODO(#780): Remove this once submission service is separated into + // Email and Encrypted services + form.responseMode === ResponseMode.Encrypt + ? 'answer' in response && + typeof response.answer === 'string' && + response.answer.trim() !== '' + : visibleFieldIds.has(responseId), + question: formField.getQuestion(), + } + + if (formField.isVerifiable) { + processingResponse.isUserVerified = formField.isVerifiable + } + + // Error will be returned if the processed response is not valid. + const validateFieldResult = validateField( + form._id, + formField, + processingResponse, + ) + if (validateFieldResult.isErr()) { + return err(validateFieldResult.error) + } + processedResponses.push(processingResponse) + } + + return ok(new ParsedResponsesObject(processedResponses)) + } +} diff --git a/src/app/modules/submission/email-submission/__tests__/ParsedResponsesObject.class.spec.ts b/src/app/modules/submission/email-submission/__tests__/ParsedResponsesObject.class.spec.ts new file mode 100644 index 0000000000..b0a8bb01ae --- /dev/null +++ b/src/app/modules/submission/email-submission/__tests__/ParsedResponsesObject.class.spec.ts @@ -0,0 +1,124 @@ +import { + generateDefaultField, + generateProcessedSingleAnswerResponse, + generateSingleAnswerResponse, +} from '../../../../../../tests/unit/backend/helpers/generate-form-data' +import * as LogicUtil from '../../../../../shared/util/logic' +import { + BasicField, + IEmailFormSchema, + IFormSchema, + IPreventSubmitLogicSchema, + LogicType, + ResponseMode, +} from '../../../../../types' +import { + ConflictError, + ProcessingError, + ValidateFieldError, +} from '../../submission.errors' +import { ProcessedFieldResponse } from '../../submission.types' +import ParsedResponsesObject from '../ParsedResponsesObject.class' + +describe('ParsedResponsesObject', () => { + it('should return list of parsed responses for email form submission successfully', async () => { + // Add answer to subset of field types + const shortTextField = generateDefaultField(BasicField.ShortText) + const decimalField = generateDefaultField(BasicField.Decimal) + + // Add answers to both mobile and email fields + const shortTextResponse = generateSingleAnswerResponse( + shortTextField, + 'the quick brown fox jumps over the lazy dog', + ) + const decimalResponse = generateSingleAnswerResponse(decimalField, '3.142') + + const shortTextProcessedResponse = generateProcessedSingleAnswerResponse( + shortTextField, + 'the quick brown fox jumps over the lazy dog', + ) + const decimalProcessedResponse = generateProcessedSingleAnswerResponse( + decimalField, + '3.142', + ) + + const result = ParsedResponsesObject.parseResponses( + { + responseMode: ResponseMode.Email, + form_fields: [shortTextField, decimalField], + } as unknown as IFormSchema, + [shortTextResponse, decimalResponse], + ) + + const expectedParsed: ProcessedFieldResponse[] = [ + { ...shortTextProcessedResponse, isVisible: true }, + { ...decimalProcessedResponse, isVisible: true }, + ] + + expect(result.isOk()).toEqual(true) + expect(result._unsafeUnwrap().getAllResponses()).toEqual(expectedParsed) + }) + + it('should return error when email form has more fields than responses', async () => { + const extraField = generateDefaultField(BasicField.Mobile) + + const result = ParsedResponsesObject.parseResponses( + { + responseMode: ResponseMode.Email, + form_fields: [extraField], + } as unknown as IEmailFormSchema, + [], + ) + + expect(result.isErr()).toEqual(true) + expect(result._unsafeUnwrapErr()).toEqual( + new ConflictError('Some form fields are missing'), + ) + }) + + it('should return error when any responses are not valid for email form submission', async () => { + // Set NRIC field in form as required. + const nricField = generateDefaultField(BasicField.Nric) + const nricResponse = generateSingleAnswerResponse(nricField, 'invalid') + + const result = ParsedResponsesObject.parseResponses( + { + responseMode: ResponseMode.Email, + form_fields: [nricField], + } as unknown as IEmailFormSchema, + [nricResponse], + ) + + expect(result.isErr()).toEqual(true) + expect(result._unsafeUnwrapErr()).toEqual( + new ValidateFieldError('Invalid answer submitted'), + ) + }) + + it('should return error when email form submission is prevented by logic', async () => { + // Mock logic util to return non-empty to check if error is thrown. + const mockReturnLogicUnit = { + preventSubmitMessage: 'mock prevent submit', + conditions: [], + logicType: LogicType.PreventSubmit, + _id: 'some id', + } as unknown as IPreventSubmitLogicSchema + + jest + .spyOn(LogicUtil, 'getLogicUnitPreventingSubmit') + .mockReturnValueOnce(mockReturnLogicUnit) + + const result = ParsedResponsesObject.parseResponses( + { + responseMode: ResponseMode.Email, + form_fields: [], + } as unknown as IEmailFormSchema, + [], + ) + + expect(result.isErr()).toEqual(true) + expect(result._unsafeUnwrapErr()).toEqual( + new ProcessingError('Submission prevented by form logic'), + ) + }) +}) diff --git a/src/app/modules/submission/email-submission/email-submission.controller.ts b/src/app/modules/submission/email-submission/email-submission.controller.ts index f445414da1..4c8c215cc0 100644 --- a/src/app/modules/submission/email-submission/email-submission.controller.ts +++ b/src/app/modules/submission/email-submission/email-submission.controller.ts @@ -20,10 +20,6 @@ import { import { MyInfoFactory } from '../../myinfo/myinfo.factory' import * as MyInfoUtil from '../../myinfo/myinfo.util' import { SpcpFactory } from '../../spcp/spcp.factory' -import { - createCorppassParsedResponses, - createSingpassParsedResponses, -} from '../../spcp/spcp.util' import * as EmailSubmissionMiddleware from '../email-submission/email-submission.middleware' import * as SubmissionService from '../submission.service' import { extractEmailConfirmationData } from '../submission.utils' @@ -35,6 +31,7 @@ import { mapRouteError, SubmissionEmailObj, } from './email-submission.util' +import ParsedResponsesObject from './ParsedResponsesObject.class' const logger = createLoggerWithLabel(module) @@ -137,7 +134,7 @@ const submitEmailModeForm: ControllerHandler< // Validate responses EmailSubmissionService.validateAttachments(req.body.responses) .andThen(() => - SubmissionService.getProcessedResponses(form, req.body.responses), + ParsedResponsesObject.parseResponses(form, req.body.responses), ) .map((parsedResponses) => ({ parsedResponses, form })) .mapErr((error) => { @@ -157,10 +154,11 @@ const submitEmailModeForm: ControllerHandler< .asyncAndThen((jwt) => SpcpFactory.extractCorppassJwtPayload(jwt)) .map((jwt) => ({ form, - parsedResponses: [ - ...parsedResponses, - ...createCorppassParsedResponses(jwt.userName, jwt.userInfo), - ], + parsedResponses: parsedResponses.addNdiResponses({ + authType, + uinFin: jwt.userName, + userInfo: jwt.userInfo, + }), })) .mapErr((error) => { spcpSubmissionFailure = true @@ -176,10 +174,10 @@ const submitEmailModeForm: ControllerHandler< .asyncAndThen((jwt) => SpcpFactory.extractSingpassJwtPayload(jwt)) .map((jwt) => ({ form, - parsedResponses: [ - ...parsedResponses, - ...createSingpassParsedResponses(jwt.userName), - ], + parsedResponses: parsedResponses.addNdiResponses({ + authType, + uinFin: jwt.userName, + }), })) .mapErr((error) => { spcpSubmissionFailure = true @@ -199,16 +197,19 @@ const submitEmailModeForm: ControllerHandler< .asyncAndThen((uinFin) => MyInfoFactory.fetchMyInfoHashes(uinFin, formId) .andThen((hashes) => - MyInfoFactory.checkMyInfoHashes(parsedResponses, hashes), + MyInfoFactory.checkMyInfoHashes( + parsedResponses.responses, + hashes, + ), ) .map( (hashedFields) => ({ form, hashedFields, - parsedResponses: [ - ...parsedResponses, - ...createSingpassParsedResponses(uinFin), - ], + parsedResponses: parsedResponses.addNdiResponses({ + authType, + uinFin, + }), }), ), ) @@ -231,7 +232,7 @@ const submitEmailModeForm: ControllerHandler< .andThen(({ form, parsedResponses, hashedFields }) => { // Create data for response email as well as email confirmation const emailData = new SubmissionEmailObj( - parsedResponses, + parsedResponses.getAllResponses(), hashedFields, form.authType, ) @@ -274,8 +275,9 @@ const submitEmailModeForm: ControllerHandler< // NOTE: This should short circuit in the event of an error. // This is why sendSubmissionToAdmin is separated from sendEmailConfirmations in 2 blocks return MailService.sendSubmissionToAdmin({ - replyToEmails: - EmailSubmissionService.extractEmailAnswers(parsedResponses), + replyToEmails: EmailSubmissionService.extractEmailAnswers( + parsedResponses.getAllResponses(), + ), form, submission, attachments, @@ -313,7 +315,7 @@ const submitEmailModeForm: ControllerHandler< attachments, responsesData: emailData.autoReplyData, recipientData: extractEmailConfirmationData( - parsedResponses, + parsedResponses.getAllResponses(), form.form_fields, ), }).mapErr((error) => { diff --git a/src/app/modules/submission/email-submission/email-submission.types.ts b/src/app/modules/submission/email-submission/email-submission.types.ts index eae1936c14..d142ed7b60 100644 --- a/src/app/modules/submission/email-submission/email-submission.types.ts +++ b/src/app/modules/submission/email-submission/email-submission.types.ts @@ -4,7 +4,9 @@ import { IBaseResponse, IPopulatedEmailForm, } from '../../../../types' -import { ProcessedFieldResponse, ProcessedResponse } from '../submission.types' +import { ProcessedResponse } from '../submission.types' + +import ParsedResponsesObject from './ParsedResponsesObject.class' // When a response has been formatted for email, all answerArray // should have been converted to answer @@ -28,6 +30,6 @@ export interface SubmissionHash { export interface IPopulatedEmailFormWithResponsesAndHash { form: IPopulatedEmailForm - parsedResponses: ProcessedFieldResponse[] + parsedResponses: ParsedResponsesObject hashedFields?: Set } diff --git a/src/app/modules/submission/submission.service.ts b/src/app/modules/submission/submission.service.ts index 1dedd9b20f..281719ba1c 100644 --- a/src/app/modules/submission/submission.service.ts +++ b/src/app/modules/submission/submission.service.ts @@ -1,132 +1,24 @@ import mongoose from 'mongoose' -import { err, errAsync, ok, okAsync, Result, ResultAsync } from 'neverthrow' +import { errAsync, okAsync, ResultAsync } from 'neverthrow' -import { - getLogicUnitPreventingSubmit, - getVisibleFieldIds, -} from '../../../shared/util/logic' import { EmailRespondentConfirmationField, - FieldResponse, IAttachmentInfo, - IFieldSchema, - IFormDocument, IPopulatedForm, ISubmissionSchema, - ResponseMode, } from '../../../types' import { createLoggerWithLabel } from '../../config/logger' import getSubmissionModel from '../../models/submission.server.model' import MailService from '../../services/mail/mail.service' import { AutoReplyMailData } from '../../services/mail/mail.types' import { createQueryWithDateParam, isMalformedDate } from '../../utils/date' -import { validateField } from '../../utils/field-validation' import { DatabaseError, MalformedParametersError } from '../core/core.errors' -import { - ConflictError, - ProcessingError, - SendEmailConfirmationError, - ValidateFieldError, -} from './submission.errors' -import { ProcessedFieldResponse } from './submission.types' -import { getFilteredResponses } from './submission.utils' +import { SendEmailConfirmationError } from './submission.errors' const logger = createLoggerWithLabel(module) const SubmissionModel = getSubmissionModel(mongoose) -/** - * Injects response metadata such as the question, visibility state. In - * addition, validation such as input validation or signature validation on - * verified fields are also performed on the response. - * @param form The form document corresponding to the responses - * @param responses The responses to process and validate - * @returns neverthrow ok() with field responses with additional metadata injected. - * @returns neverthrow err() if response validation fails - */ -export const getProcessedResponses = ( - form: IFormDocument, - originalResponses: FieldResponse[], -): Result< - ProcessedFieldResponse[], - ProcessingError | ConflictError | ValidateFieldError -> => { - const filteredResponsesResult = getFilteredResponses(form, originalResponses) - if (filteredResponsesResult.isErr()) { - return err(filteredResponsesResult.error) - } - - const filteredResponses = filteredResponsesResult.value - - // Set of all visible fields - const visibleFieldIds = getVisibleFieldIds(filteredResponses, form) - - // Guard against invalid form submissions that should have been prevented by - // logic. - if (getLogicUnitPreventingSubmit(filteredResponses, form, visibleFieldIds)) { - return err(new ProcessingError('Submission prevented by form logic')) - } - - // Create a map keyed by field._id for easier access - - if (!form.form_fields) { - return err(new ProcessingError('Form fields are undefined')) - } - - const fieldMap = form.form_fields.reduce<{ - [fieldId: string]: IFieldSchema - }>((acc, field) => { - acc[field._id] = field - return acc - }, {}) - - // Validate each field in the form and inject metadata into the responses. - const processedResponses = [] - for (const response of filteredResponses) { - const responseId = response._id - const formField = fieldMap[responseId] - if (!formField) { - return err( - new ProcessingError('Response ID does not match form field IDs'), - ) - } - - const processingResponse: ProcessedFieldResponse = { - ...response, - isVisible: - // Set isVisible as true for Encrypt mode if there is a response for mobile and email field - // Because we cannot tell if the field is unhidden by logic - // This prevents downstream validateField from incorrectly preventing - // encrypt mode submissions with responses on unhidden fields - // TODO(#780): Remove this once submission service is separated into - // Email and Encrypted services - form.responseMode === ResponseMode.Encrypt - ? 'answer' in response && - typeof response.answer === 'string' && - response.answer.trim() !== '' - : visibleFieldIds.has(responseId), - question: formField.getQuestion(), - } - - if (formField.isVerifiable) { - processingResponse.isUserVerified = formField.isVerifiable - } - - // Error will be returned if the processed response is not valid. - const validateFieldResult = validateField( - form._id, - formField, - processingResponse, - ) - if (validateFieldResult.isErr()) { - return err(validateFieldResult.error) - } - processedResponses.push(processingResponse) - } - - return ok(processedResponses) -} - /** * Returns number of form submissions of given form id in the given date range. *