From 332d3ce8b7e44827d5e01b1cccef5b9cf03b7728 Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Tue, 8 Jun 2021 04:34:28 -0700 Subject: [PATCH 01/56] feat: Add webhook support for storage mode attachments (#1713) * feat: Add webhook support for storage mode attachments * fix tests post rebase * remove merge error --- .../__tests__/submission.server.model.spec.ts | 6 + src/app/models/field/attachmentField.ts | 23 +-- src/app/models/submission.server.model.ts | 5 + .../webhook/__tests__/webhook.service.spec.ts | 3 + src/app/modules/webhook/webhook.errors.ts | 14 ++ src/app/modules/webhook/webhook.service.ts | 172 +++++++++++------- .../settings-form.client.view.html | 11 +- .../directives/edit-form.client.directive.js | 5 +- .../settings-form.client.directive.js | 8 - src/types/submission.ts | 1 + 10 files changed, 141 insertions(+), 107 deletions(-) diff --git a/src/app/models/__tests__/submission.server.model.spec.ts b/src/app/models/__tests__/submission.server.model.spec.ts index f726745928..8ee24f5d5d 100644 --- a/src/app/models/__tests__/submission.server.model.spec.ts +++ b/src/app/models/__tests__/submission.server.model.spec.ts @@ -68,6 +68,7 @@ describe('Submission Model', () => { isRetryEnabled: true, webhookView: { data: { + attachmentDownloadUrls: {}, formId: String(form._id), submissionId: String(submission._id), encryptedContent: MOCK_ENCRYPTED_CONTENT, @@ -120,6 +121,7 @@ describe('Submission Model', () => { isRetryEnabled: false, webhookView: { data: { + attachmentDownloadUrls: {}, formId: String(form._id), submissionId: String(submission._id), encryptedContent: MOCK_ENCRYPTED_CONTENT, @@ -155,6 +157,7 @@ describe('Submission Model', () => { isRetryEnabled: false, webhookView: { data: { + attachmentDownloadUrls: {}, formId: String(form._id), submissionId: String(submission._id), encryptedContent: MOCK_ENCRYPTED_CONTENT, @@ -268,6 +271,7 @@ describe('Submission Model', () => { created: expect.any(Date), encryptedContent: MOCK_ENCRYPTED_CONTENT, verifiedContent: undefined, + attachmentDownloadUrls: {}, version: 1, }, }) @@ -294,6 +298,7 @@ describe('Submission Model', () => { // Assert expect(actualWebhookView).toEqual({ data: { + attachmentDownloadUrls: {}, formId: expect.any(String), submissionId: expect.any(String), created: expect.any(Date), @@ -336,6 +341,7 @@ describe('Submission Model', () => { // Assert expect(actualWebhookView).toEqual({ data: { + attachmentDownloadUrls: {}, formId: expect.any(String), submissionId: expect.any(String), created: expect.any(Date), diff --git a/src/app/models/field/attachmentField.ts b/src/app/models/field/attachmentField.ts index 49ee6eadfe..80fe25ca69 100644 --- a/src/app/models/field/attachmentField.ts +++ b/src/app/models/field/attachmentField.ts @@ -1,11 +1,6 @@ import { Document, Schema } from 'mongoose' -import { - AttachmentSize, - IAttachmentField, - IFormSchema, - ResponseMode, -} from '../../../types' +import { AttachmentSize, IAttachmentField, IFormSchema } from '../../../types' // Manual override since mongoose types don't have generics yet. interface IAttachmentFieldSchema extends IAttachmentField, Document { @@ -24,22 +19,6 @@ const createAttachmentFieldSchema = () => { }, }) - // Prevent attachments from being saved on a webhooked form. - AttachmentFieldSchema.pre( - 'validate', - function (next) { - const { webhook, responseMode } = this.parent() - - if (responseMode === ResponseMode.Encrypt && webhook?.url) { - return next( - Error('Attachments are not allowed when a form has a webhook url'), - ) - } - - return next() - }, - ) - return AttachmentFieldSchema } diff --git a/src/app/models/submission.server.model.ts b/src/app/models/submission.server.model.ts index 2e37daae76..8c949bbedc 100644 --- a/src/app/models/submission.server.model.ts +++ b/src/app/models/submission.server.model.ts @@ -187,6 +187,10 @@ EncryptSubmissionSchema.methods.getWebhookView = function ( const formId = this.populated('form') ? String(this.form._id) : String(this.form) + const attachmentRecords = Object.fromEntries( + this.attachmentMetadata ?? new Map(), + ) + const webhookData: WebhookData = { formId, submissionId: String(this._id), @@ -194,6 +198,7 @@ EncryptSubmissionSchema.methods.getWebhookView = function ( verifiedContent: this.verifiedContent, version: this.version, created: this.created, + attachmentDownloadUrls: attachmentRecords, } return { diff --git a/src/app/modules/webhook/__tests__/webhook.service.spec.ts b/src/app/modules/webhook/__tests__/webhook.service.spec.ts index c763f183aa..8ecb70678c 100644 --- a/src/app/modules/webhook/__tests__/webhook.service.spec.ts +++ b/src/app/modules/webhook/__tests__/webhook.service.spec.ts @@ -94,6 +94,9 @@ describe('webhook.service', () => { formId: MOCK_FORM_ID, submissionId: MOCK_SUBMISSION_ID, verifiedContent: 'mockVerifiedContent', + attachmentDownloadUrls: { + 'some-field-id': 'https://mock.s3.url/some/s3/url/timeout=3600', + }, version: 1, }, } diff --git a/src/app/modules/webhook/webhook.errors.ts b/src/app/modules/webhook/webhook.errors.ts index 426d32f170..6ecc52f3d7 100644 --- a/src/app/modules/webhook/webhook.errors.ts +++ b/src/app/modules/webhook/webhook.errors.ts @@ -12,6 +12,20 @@ export class WebhookValidationError extends ApplicationError { } } +/** + * Webhook failed to generate S3 presigned URLs for attachments + */ +export class WebhookFailedWithPresignedUrlGenerationError extends ApplicationError { + meta: { + originalError: unknown + } + + constructor(error: unknown, message = 'Presigned Url Generation failed') { + super(message) + this.meta = { originalError: error } + } +} + /** * Webhook returned non-200 status, but error is not instance of AxiosError */ diff --git a/src/app/modules/webhook/webhook.service.ts b/src/app/modules/webhook/webhook.service.ts index be3f7feb19..ce6e16b69f 100644 --- a/src/app/modules/webhook/webhook.service.ts +++ b/src/app/modules/webhook/webhook.service.ts @@ -1,4 +1,5 @@ import axios from 'axios' +import Bluebird from 'bluebird' import { get } from 'lodash' import mongoose from 'mongoose' import { errAsync, okAsync, ResultAsync } from 'neverthrow' @@ -9,6 +10,7 @@ import { IWebhookResponse, WebhookView, } from '../../../types' +import { aws as AwsConfig } from '../../config/config' import formsgSdk from '../../config/formsg-sdk' import { createLoggerWithLabel } from '../../config/logger' import { getEncryptSubmissionModel } from '../../models/submission.server.model' @@ -18,6 +20,7 @@ import { SubmissionNotFoundError } from '../submission/submission.errors' import { WebhookFailedWithAxiosError, + WebhookFailedWithPresignedUrlGenerationError, WebhookFailedWithUnknownError, WebhookPushToQueueError, WebhookValidationError, @@ -72,10 +75,35 @@ export const saveWebhookRecord = ( }) } +const createWebhookSubmissionView = ( + submissionWebhookView: WebhookView, +): Promise => { + // Generate S3 signed urls + const signedUrlPromises: Record> = {} + for (const key in submissionWebhookView.data.attachmentDownloadUrls) { + signedUrlPromises[key] = AwsConfig.s3.getSignedUrlPromise('getObject', { + Bucket: AwsConfig.attachmentS3Bucket, + Key: submissionWebhookView.data.attachmentDownloadUrls[key], + Expires: 60 * 60, // one hour expiry + }) + } + + return Bluebird.props(signedUrlPromises).then((signedUrls) => { + submissionWebhookView.data.attachmentDownloadUrls = signedUrls + return submissionWebhookView + }) +} + export const sendWebhook = ( webhookView: WebhookView, webhookUrl: string, -): ResultAsync => { +): ResultAsync< + IWebhookResponse, + | WebhookValidationError + | WebhookFailedWithAxiosError + | WebhookFailedWithPresignedUrlGenerationError + | WebhookFailedWithUnknownError +> => { const now = Date.now() const { submissionId, formId } = webhookView.data @@ -104,77 +132,93 @@ export const sendWebhook = ( return error instanceof WebhookValidationError ? error : new WebhookValidationError() - }) - .andThen(() => - ResultAsync.fromPromise( - axios.post(webhookUrl, webhookView, { - headers: { - 'X-FormSG-Signature': formsgSdk.webhooks.constructHeader({ - epoch: now, - submissionId, - formId, - signature, - }), - }, - maxRedirects: 0, - // Timeout after 10 seconds to allow for cold starts in receiver, - // e.g. Lambdas - timeout: 10 * 1000, - }), - (error) => { - logger.error({ - message: 'Webhook POST failed', - meta: { - ...logMeta, - isAxiosError: axios.isAxiosError(error), - status: get(error, 'response.status'), - }, - error, - }) - if (axios.isAxiosError(error)) { - return new WebhookFailedWithAxiosError(error) - } - return new WebhookFailedWithUnknownError(error) - }, - ), + }).andThen(() => { + return ResultAsync.fromPromise( + createWebhookSubmissionView(webhookView), + (error) => { + logger.error({ + message: 'S3 attachment presigned URL generation failed', + meta: logMeta, + error, + }) + return new WebhookFailedWithPresignedUrlGenerationError(error) + }, ) - .map((response) => { - // Capture response for logging purposes - logger.info({ - message: 'Webhook POST succeeded', - meta: { - ...logMeta, - status: get(response, 'status'), - }, + .andThen((submissionWebhookView) => + ResultAsync.fromPromise( + axios.post(webhookUrl, submissionWebhookView, { + headers: { + 'X-FormSG-Signature': formsgSdk.webhooks.constructHeader({ + epoch: now, + submissionId, + formId, + signature, + }), + }, + maxRedirects: 0, + // Timeout after 10 seconds to allow for cold starts in receiver, + // e.g. Lambdas + timeout: 10 * 1000, + }), + (error) => { + logger.error({ + message: 'Webhook POST failed', + meta: { + ...logMeta, + isAxiosError: axios.isAxiosError(error), + status: get(error, 'response.status'), + }, + error, + }) + if (axios.isAxiosError(error)) { + return new WebhookFailedWithAxiosError(error) + } + return new WebhookFailedWithUnknownError(error) + }, + ), + ) + .map((response) => { + // Capture response for logging purposes + logger.info({ + message: 'Webhook POST succeeded', + meta: { + ...logMeta, + status: get(response, 'status'), + }, + }) + return { + signature, + webhookUrl, + response: formatWebhookResponse(response), + } }) - return { - signature, - webhookUrl, - response: formatWebhookResponse(response), - } - }) - .orElse((error) => { - // Webhook was not posted - if (error instanceof WebhookValidationError) return errAsync(error) + .orElse((error) => { + // Webhook was not posted + if (error instanceof WebhookValidationError) return errAsync(error) + + // S3 pre-signed URL generation failed + if (error instanceof WebhookFailedWithPresignedUrlGenerationError) + return errAsync(error) + + // Webhook was posted but failed + if (error instanceof WebhookFailedWithUnknownError) { + return okAsync({ + signature, + webhookUrl, + // Not Axios error so no guarantee of having response. + // Hence allow formatting function to return default shape. + response: formatWebhookResponse(), + }) + } - // Webhook was posted but failed - if (error instanceof WebhookFailedWithUnknownError) { + const axiosError = error.meta.originalError return okAsync({ signature, webhookUrl, - // Not Axios error so no guarantee of having response. - // Hence allow formatting function to return default shape. - response: formatWebhookResponse(), + response: formatWebhookResponse(axiosError.response), }) - } - - const axiosError = error.meta.originalError - return okAsync({ - signature, - webhookUrl, - response: formatWebhookResponse(axiosError.response), }) - }) + }) } /** diff --git a/src/public/modules/forms/admin/directiveViews/settings-form.client.view.html b/src/public/modules/forms/admin/directiveViews/settings-form.client.view.html index 5b6f67c24e..7169ccec1c 100644 --- a/src/public/modules/forms/admin/directiveViews/settings-form.client.view.html +++ b/src/public/modules/forms/admin/directiveViews/settings-form.client.view.html @@ -379,15 +379,6 @@ (optional) -
- - Webhook is not available for forms with attachment fields. -
{ - return ( - $scope.myform.form_fields.filter( - (field) => field.fieldType == 'attachment', - ).length > 0 - ) - } - // Warning message when turning off SP with MyInfo fields $scope.myInfoSPWarning = () => { return ( diff --git a/src/types/submission.ts b/src/types/submission.ts index b1aaabee5c..3c2e640352 100644 --- a/src/types/submission.ts +++ b/src/types/submission.ts @@ -51,6 +51,7 @@ export interface WebhookData { verifiedContent: IEncryptedSubmissionSchema['verifiedContent'] version: IEncryptedSubmissionSchema['version'] created: IEncryptedSubmissionSchema['created'] + attachmentDownloadUrls: Record } export interface WebhookView { From d3313ff7f481c2261699f3ec35b539b60077d808 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 8 Jun 2021 17:20:31 +0000 Subject: [PATCH 02/56] fix(deps): bump aws-sdk from 2.922.0 to 2.923.0 (#2105) Bumps [aws-sdk](https://github.com/aws/aws-sdk-js) from 2.922.0 to 2.923.0. - [Release notes](https://github.com/aws/aws-sdk-js/releases) - [Changelog](https://github.com/aws/aws-sdk-js/blob/master/CHANGELOG.md) - [Commits](https://github.com/aws/aws-sdk-js/compare/v2.922.0...v2.923.0) --- updated-dependencies: - dependency-name: aws-sdk dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- package-lock.json | 6 +++--- package.json | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index 12dff19c87..d998d83f92 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7098,9 +7098,9 @@ "integrity": "sha512-24q5Rh3bno7ldoyCq99d6hpnLI+PAMocdeVaaGt/5BTQMprvDwQToHfNnruqN11odCHZZIQbRBw+nZo1lTCH9g==" }, "aws-sdk": { - "version": "2.922.0", - "resolved": "https://registry.npmjs.org/aws-sdk/-/aws-sdk-2.922.0.tgz", - "integrity": "sha512-SufbR5TTCK94Zk/xIv4v/m0MM9z+KW999XnjXOyNWGFGHP9/FArjtHtq69+a3KpohYBR1dBj8wUhVjbClmQIBA==", + "version": "2.923.0", + "resolved": "https://registry.npmjs.org/aws-sdk/-/aws-sdk-2.923.0.tgz", + "integrity": "sha512-93YK9Mx32qvH5eLrDRVkvwpEc7GJ6mces1qQugmNZCuQT68baILyDs0IMRtHS6NYxxiM6XNRFc7ubgMlnafYjg==", "requires": { "buffer": "4.9.2", "events": "1.1.1", diff --git a/package.json b/package.json index 3a0d8fb3dc..0938b74ce5 100644 --- a/package.json +++ b/package.json @@ -84,7 +84,7 @@ "angular-ui-bootstrap": "~2.5.6", "angular-ui-router": "~1.0.29", "aws-info": "^1.2.0", - "aws-sdk": "^2.922.0", + "aws-sdk": "^2.923.0", "axios": "^0.21.1", "bcrypt": "^5.0.1", "bluebird": "^3.5.2", From 41a61c3f4aeb821a1c96392f4a4867579956c65e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 8 Jun 2021 17:25:06 +0000 Subject: [PATCH 03/56] fix(deps): bump zod from 3.0.0 to 3.1.0 (#2107) Bumps [zod](https://github.com/colinhacks/zod) from 3.0.0 to 3.1.0. - [Release notes](https://github.com/colinhacks/zod/releases) - [Changelog](https://github.com/colinhacks/zod/blob/master/CHANGELOG.md) - [Commits](https://github.com/colinhacks/zod/compare/v3.0.0...v3.1.0) --- updated-dependencies: - dependency-name: zod dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- package-lock.json | 6 +++--- package.json | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index d998d83f92..1c542be844 100644 --- a/package-lock.json +++ b/package-lock.json @@ -27124,9 +27124,9 @@ "dev": true }, "zod": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/zod/-/zod-3.0.0.tgz", - "integrity": "sha512-4DBG6siN02ooPB1yvEEqoe32maHzKEdGgtQ2HEz6FnFtgTjwZtzJ3ScuiDgtssWfDyLnQ3MvtSj6ff5ANL4STw==" + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/zod/-/zod-3.1.0.tgz", + "integrity": "sha512-qS0an8oo9EvVLVqIVxMZrQrfR2pVwBtlPp+BzTB/F19IyPTRaLLoFfdXRzgh626pxFR1efuTWV8bPoEE58KwqA==" }, "zwitch": { "version": "1.0.5", diff --git a/package.json b/package.json index 0938b74ce5..2ca79ab364 100644 --- a/package.json +++ b/package.json @@ -157,7 +157,7 @@ "whatwg-fetch": "^3.6.2", "winston": "^3.3.3", "winston-cloudwatch": "^2.5.2", - "zod": "^3.0.0" + "zod": "^3.1.0" }, "devDependencies": { "@babel/core": "^7.14.3", From a43b0dbc0574d68c6be6ba50282dbf3ed3c8ce9d Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 8 Jun 2021 17:36:08 +0000 Subject: [PATCH 04/56] chore(deps-dev): bump @types/express-rate-limit from 5.1.1 to 5.1.2 (#2109) Bumps [@types/express-rate-limit](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/express-rate-limit) from 5.1.1 to 5.1.2. - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/express-rate-limit) --- updated-dependencies: - dependency-name: "@types/express-rate-limit" dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- package-lock.json | 6 +++--- package.json | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index 1c542be844..57dac7de4e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5338,9 +5338,9 @@ } }, "@types/express-rate-limit": { - "version": "5.1.1", - "resolved": "https://registry.npmjs.org/@types/express-rate-limit/-/express-rate-limit-5.1.1.tgz", - "integrity": "sha512-6oMYZBLlhxC5sdcRXXz528QyfGz3zTy9YdHwqlxLfgx5Cd3zwYaUjjPpJcaTtHmRefLi9P8kLBPz2wB7yz4JtQ==", + "version": "5.1.2", + "resolved": "https://registry.npmjs.org/@types/express-rate-limit/-/express-rate-limit-5.1.2.tgz", + "integrity": "sha512-loN1dcr0WEKsbVtXNQKDef4Fmh25prfy+gESrwTDofIhAt17ngTxrsDiEZxLemmfHH279x206CdUB9XHXS9E6Q==", "dev": true, "requires": { "@types/express": "*" diff --git a/package.json b/package.json index 2ca79ab364..6e286b8138 100644 --- a/package.json +++ b/package.json @@ -173,7 +173,7 @@ "@types/dedent": "^0.7.0", "@types/ejs": "^3.0.6", "@types/express": "^4.17.12", - "@types/express-rate-limit": "^5.1.1", + "@types/express-rate-limit": "^5.1.2", "@types/express-request-id": "^1.4.1", "@types/express-session": "^1.17.0", "@types/has-ansi": "^3.0.0", From da58b26cb37488ce42dfac208489e4829af7ba4f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 8 Jun 2021 17:50:01 +0000 Subject: [PATCH 05/56] chore(deps-dev): bump ts-essentials from 7.0.1 to 7.0.2 (#2110) Bumps [ts-essentials](https://github.com/krzkaczor/ts-essentials) from 7.0.1 to 7.0.2. - [Release notes](https://github.com/krzkaczor/ts-essentials/releases) - [Commits](https://github.com/krzkaczor/ts-essentials/commits) --- updated-dependencies: - dependency-name: ts-essentials dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- package-lock.json | 6 +++--- package.json | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index 57dac7de4e..03fa70aca2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -24961,9 +24961,9 @@ } }, "ts-essentials": { - "version": "7.0.1", - "resolved": "https://registry.npmjs.org/ts-essentials/-/ts-essentials-7.0.1.tgz", - "integrity": "sha512-8lwh3QJtIc1UWhkQtr9XuksXu3O0YQdEE5g79guDfhCaU1FWTDIEDZ1ZSx4HTHUmlJZ8L812j3BZQ4a0aOUkSA==", + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/ts-essentials/-/ts-essentials-7.0.2.tgz", + "integrity": "sha512-qWPVC1xZGdefbsgFP7tPo+bsgSA2ZIXL1XeEe5M2WoMZxIOr/HbsHxP/Iv75IFhiMHMDGL7cOOwi5SXcgx9mHw==", "dev": true }, "ts-jest": { diff --git a/package.json b/package.json index 6e286b8138..bd763e71ae 100644 --- a/package.json +++ b/package.json @@ -243,7 +243,7 @@ "supertest-session": "^4.1.0", "terser-webpack-plugin": "^1.2.3", "testcafe": "^1.14.2", - "ts-essentials": "^7.0.1", + "ts-essentials": "^7.0.2", "ts-jest": "^26.5.6", "ts-loader": "^7.0.5", "ts-node": "^10.0.0", From 0a0b0de606b6fe15258c7e6c51442e6276b92bed Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 9 Jun 2021 02:25:36 +0000 Subject: [PATCH 06/56] chore(deps-dev): bump @types/node from 14.17.2 to 14.17.3 (#2108) Bumps [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node) from 14.17.2 to 14.17.3. - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node) --- updated-dependencies: - dependency-name: "@types/node" dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- package-lock.json | 6 +++--- package.json | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index 03fa70aca2..600f48ea48 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5534,9 +5534,9 @@ "dev": true }, "@types/node": { - "version": "14.17.2", - "resolved": "https://registry.npmjs.org/@types/node/-/node-14.17.2.tgz", - "integrity": "sha512-sld7b/xmFum66AAKuz/rp/CUO8+98fMpyQ3SBfzzBNGMd/1iHBTAg9oyAvcYlAj46bpc74r91jSw2iFdnx29nw==" + "version": "14.17.3", + "resolved": "https://registry.npmjs.org/@types/node/-/node-14.17.3.tgz", + "integrity": "sha512-e6ZowgGJmTuXa3GyaPbTGxX17tnThl2aSSizrFthQ7m9uLGZBXiGhgE55cjRZTF5kjZvYn9EOPOMljdjwbflxw==" }, "@types/nodemailer": { "version": "6.4.2", diff --git a/package.json b/package.json index bd763e71ae..f6a4a6e396 100644 --- a/package.json +++ b/package.json @@ -183,7 +183,7 @@ "@types/json-stringify-safe": "^5.0.0", "@types/mongodb": "^3.6.17", "@types/mongodb-uri": "^0.9.0", - "@types/node": "^14.17.2", + "@types/node": "^14.17.3", "@types/nodemailer": "^6.4.2", "@types/opossum": "^4.1.1", "@types/promise-retry": "^1.1.3", From 95b6297c62dbec6b656a38bda9da098c6e901343 Mon Sep 17 00:00:00 2001 From: tshuli <63710093+tshuli@users.noreply.github.com> Date: Wed, 9 Jun 2021 10:35:59 +0800 Subject: [PATCH 07/56] chore: remove redundant ValidationOption object properties for short text, long text and number fields (#2040) * chore: script to verify that db is consistent * chore: remove references to customMin, customMax, Range for short text, long text and number fields in backend * chore: remove references to customMin, customMax for short text, long text and number fields in frontend * chore: add customMin and customMax virtuals for backward compatibility * chore: script to unset customMin and customMax for text and number fields * chore: remove Number casting for customVal * chore: add todos for virtuals * chore: correct types for virtuals * refactor: combine TextValidationOptionsSchema * chore: add elemMatch for field type at start of aggregation * chore: add check for non-text/number fields * style: lint * chore: refine filter for update (doesn't affect outcome) * chore: improve query syntax * chore: rename folder to common --- scripts/20210517_validation-object/script.js | 369 ++++++++++++++++++ .../common/textValidationOptionsSchema.ts | 39 ++ src/app/models/field/longTextField.ts | 28 +- src/app/models/field/numberField.ts | 58 ++- src/app/models/field/shortTextField.ts | 27 +- .../__tests__/number-validation.spec.ts | 42 +- .../__tests__/text-validator.spec.ts | 136 +------ .../validators/numberValidator.ts | 8 +- .../validators/textValidator.ts | 14 +- .../modules/forms/admin/constants/covid19.js | 26 -- .../edit-fields-modal.client.controller.js | 13 - .../admin/views/edit-fields.client.modal.html | 1 - .../field-number.client.view.html | 12 +- .../field-textarea.client.view.html | 16 +- .../field-textfield.client.view.html | 12 +- .../modules/forms/viewmodels/Fields/MixIns.js | 2 - src/types/field/baseField.ts | 1 - src/types/field/longTextField.ts | 14 +- src/types/field/numberField.ts | 3 - src/types/field/shortTextField.ts | 14 +- src/types/field/utils/textField.ts | 15 + src/types/field/utils/virtuals.ts | 9 + .../backend/helpers/generate-form-data.ts | 4 - 23 files changed, 549 insertions(+), 314 deletions(-) create mode 100644 scripts/20210517_validation-object/script.js create mode 100644 src/app/models/field/common/textValidationOptionsSchema.ts create mode 100644 src/types/field/utils/textField.ts create mode 100644 src/types/field/utils/virtuals.ts diff --git a/scripts/20210517_validation-object/script.js b/scripts/20210517_validation-object/script.js new file mode 100644 index 0000000000..5f1483bffc --- /dev/null +++ b/scripts/20210517_validation-object/script.js @@ -0,0 +1,369 @@ +/* eslint-disable */ +// Scripts for #408 + +/** + * Part 1: Verify that the database is internally consistent today + * for all short text, long text and number fields. + * Expect 0 records for all scripts + */ + +// a. customVal === customMin when selectedValidation === 'Minimum' +// Expect 0 records + +db.getCollection('forms').aggregate([ + { + $match: { + 'form_fields.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + }, + }, + { $unwind: '$form_fields' }, + { + $match: { + 'form_fields.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + }, + }, + { + $project: { + form_fields: 1, + selectedValidation: '$form_fields.ValidationOptions.selectedValidation', + customVal: '$form_fields.ValidationOptions.customVal', + customMin: '$form_fields.ValidationOptions.customMin', + customMax: '$form_fields.ValidationOptions.customMax', + }, + }, + { + $match: { + $and: [ + { + selectedValidation: 'Minimum', + }, + { + $expr: { $ne: ['$customMin', '$customVal'] }, + }, + ], + }, + }, +]) + +// b. customVal === customMax when selectedValidation === 'Maximum' +// Expect 0 records + +db.getCollection('forms').aggregate([ + { + $match: { + 'form_fields.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + }, + }, + { $unwind: '$form_fields' }, + { + $match: { + 'form_fields.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + }, + }, + { + $project: { + form_fields: 1, + selectedValidation: '$form_fields.ValidationOptions.selectedValidation', + customVal: '$form_fields.ValidationOptions.customVal', + customMin: '$form_fields.ValidationOptions.customMin', + customMax: '$form_fields.ValidationOptions.customMax', + }, + }, + { + $match: { + $and: [ + { + selectedValidation: 'Maximum', + }, + { + $expr: { $ne: ['$customMax', '$customVal'] }, + }, + ], + }, + }, +]) + +// c. customVal === customMin === customMax when selectedValidation === 'Exact' +// Expect 0 records + +db.getCollection('forms').aggregate([ + { + $match: { + 'form_fields.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + }, + }, + { $unwind: '$form_fields' }, + { + $match: { + 'form_fields.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + }, + }, + { + $project: { + form_fields: 1, + selectedValidation: '$form_fields.ValidationOptions.selectedValidation', + customVal: '$form_fields.ValidationOptions.customVal', + customMin: '$form_fields.ValidationOptions.customMin', + customMax: '$form_fields.ValidationOptions.customMax', + }, + }, + { + $match: { + $and: [ + { + selectedValidation: 'Exact', + }, + { + $or: [ + { + $expr: { $ne: ['$customMax', '$customVal'] }, + }, + { + $expr: { $ne: ['$customMin', '$customVal'] }, + }, + ], + }, + ], + }, + }, +]) + +// d. selectedValidation === 'Range' should not exist +// Expect 0 records + +db.getCollection('forms').aggregate([ + { + $match: { + 'form_fields.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + }, + }, + { $unwind: '$form_fields' }, + { + $match: { + 'form_fields.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + }, + }, + { + $project: { + form_fields: 1, + selectedValidation: '$form_fields.ValidationOptions.selectedValidation', + }, + }, + { + $match: { + selectedValidation: 'Range', + }, + }, +]) + +/** + * Part 2: Unset form_fields.ValidationOptions.customMin and + * form_fields.ValidationOptions.customMax + * for all short text, long text and number fields. + */ + +// ********** BEFORE ********** + +// Count number of forms with (short text, long text and number fields) and +// with ValidationOptions.customMin +// Should match number of updated records later + +db.forms.count({ + form_fields: { + $elemMatch: { + fieldType: { $in: ['textfield', 'textarea', 'number'] }, + 'ValidationOptions.customMin': { $exists: true }, + }, + }, +}) + +// Count number of forms with (short text, long text and number fields) and +// with ValidationOptions.customMax +// Should match number of updated records later + +db.forms.count({ + form_fields: { + $elemMatch: { + fieldType: { $in: ['textfield', 'textarea', 'number'] }, + 'ValidationOptions.customMax': { $exists: true }, + }, + }, +}) + +// Count number of form FIELDS which are NOT (short text, long text and number fields) and +// with ValidationOptions.customMin +// Expect unchanged after update + +db.forms.aggregate([ + { $unwind: '$form_fields' }, + { + $match: { + 'form_fields.fieldType': { $nin: ['textfield', 'textarea', 'number'] }, + }, + }, + { + $match: { 'form_fields.ValidationOptions.customMin': { $exists: true } }, + }, + { $count: 'numFormFields' }, +]) + +// Count number of form FIELDS which are NOT (short text, long text and number fields) and +// with ValidationOptions.customMax +// Expect unchanged after update + +db.forms.aggregate([ + { $unwind: '$form_fields' }, + { + $match: { + 'form_fields.fieldType': { $nin: ['textfield', 'textarea', 'number'] }, + }, + }, + { + $match: { 'form_fields.ValidationOptions.customMax': { $exists: true } }, + }, + { $count: 'numFormFields' }, +]) + +// ********** UPDATE ********** + +// Remove customMin + +db.forms.update( + { + form_fields: { + $elemMatch: { + fieldType: { $in: ['textfield', 'textarea', 'number'] }, + 'ValidationOptions.customMin': { $exists: true }, + }, + }, + }, + { $unset: { 'form_fields.$[elem].ValidationOptions.customMin': '' } }, + { + arrayFilters: [ + { + 'elem.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + 'elem.ValidationOptions.customMin': { $exists: true }, + }, + ], + multi: true, + }, +) + +// Remove customMax + +db.forms.update( + { + form_fields: { + $elemMatch: { + fieldType: { $in: ['textfield', 'textarea', 'number'] }, + 'ValidationOptions.customMax': { $exists: true }, + }, + }, + }, + { $unset: { 'form_fields.$[elem].ValidationOptions.customMax': '' } }, + { + arrayFilters: [ + { + 'elem.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + 'elem.ValidationOptions.customMax': { $exists: true }, + }, + ], + multi: true, + }, +) + +// ********** AFTER ********** + +// Count number of forms with (short text, long text and number fields) and +// with ValidationOptions.customMin +// Expect 0 after running update + +db.forms.count({ + form_fields: { + $elemMatch: { + fieldType: { $in: ['textfield', 'textarea', 'number'] }, + 'ValidationOptions.customMin': { $exists: true }, + }, + }, +}) + +// Count number of forms with (short text, long text and number fields) and +// with ValidationOptions.customMax +// Expect 0 after running update + +db.forms.count({ + form_fields: { + $elemMatch: { + fieldType: { $in: ['textfield', 'textarea', 'number'] }, + 'ValidationOptions.customMax': { $exists: true }, + }, + }, +}) + +// Count number of form FIELDS with (short text, long text and number fields) and +// with ValidationOptions.customMin +// Expect 0 after running update + +db.forms.aggregate([ + { $unwind: '$form_fields' }, + { + $match: { + 'form_fields.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + }, + }, + { + $match: { 'form_fields.ValidationOptions.customMin': { $exists: true } }, + }, + { $count: 'numFormFields' }, +]) + +// Count number of form FIELDS with (short text, long text and number fields) and +// with ValidationOptions.customMax +// Expect 0 after running update + +db.forms.aggregate([ + { $unwind: '$form_fields' }, + { + $match: { + 'form_fields.fieldType': { $in: ['textfield', 'textarea', 'number'] }, + }, + }, + { + $match: { 'form_fields.ValidationOptions.customMax': { $exists: true } }, + }, + { $count: 'numFormFields' }, +]) + +// Count number of form FIELDS which are NOT (short text, long text and number fields) and +// with ValidationOptions.customMin +// Expect unchanged after update + +db.forms.aggregate([ + { $unwind: '$form_fields' }, + { + $match: { + 'form_fields.fieldType': { $nin: ['textfield', 'textarea', 'number'] }, + }, + }, + { + $match: { 'form_fields.ValidationOptions.customMin': { $exists: true } }, + }, + { $count: 'numFormFields' }, +]) + +// Count number of form FIELDS which are NOT (short text, long text and number fields) and +// with ValidationOptions.customMax +// Expect unchanged after update + +db.forms.aggregate([ + { $unwind: '$form_fields' }, + { + $match: { + 'form_fields.fieldType': { $nin: ['textfield', 'textarea', 'number'] }, + }, + }, + { + $match: { 'form_fields.ValidationOptions.customMax': { $exists: true } }, + }, + { $count: 'numFormFields' }, +]) diff --git a/src/app/models/field/common/textValidationOptionsSchema.ts b/src/app/models/field/common/textValidationOptionsSchema.ts new file mode 100644 index 0000000000..582da4d87c --- /dev/null +++ b/src/app/models/field/common/textValidationOptionsSchema.ts @@ -0,0 +1,39 @@ +import { Schema } from 'mongoose' + +import { TextSelectedValidation } from '../../../../types/field/baseField' +import { TextValidationOptions } from '../../../../types/field/utils/textField' +import { WithCustomMinMax } from '../../../../types/field/utils/virtuals' + +export const TextValidationOptionsSchema = new Schema< + WithCustomMinMax +>( + { + customVal: { + type: Number, + }, + selectedValidation: { + type: String, + enum: [...Object.values(TextSelectedValidation), null], + }, + }, + { + // TODO: Remove virtuals (#2039) + toJSON: { + virtuals: true, + }, + }, +) + +// Virtuals to allow for backwards compatibility after customMin and customMax were removed as part of #408 +// TODO: Remove virtuals (#2039) +TextValidationOptionsSchema.virtual('customMin').get(function ( + this: TextValidationOptions, +) { + return this.customVal +}) + +TextValidationOptionsSchema.virtual('customMax').get(function ( + this: TextValidationOptions, +) { + return this.customVal +}) diff --git a/src/app/models/field/longTextField.ts b/src/app/models/field/longTextField.ts index a893fe5861..ff289ab134 100644 --- a/src/app/models/field/longTextField.ts +++ b/src/app/models/field/longTextField.ts @@ -1,29 +1,21 @@ import { Schema } from 'mongoose' import { ILongTextFieldSchema } from '../../../types' -import { TextSelectedValidation } from '../../../types/field/baseField' + +import { TextValidationOptionsSchema } from './common/textValidationOptionsSchema' const createLongTextFieldSchema = () => { - return new Schema({ + const LongTextFieldSchema = new Schema({ ValidationOptions: { - customMax: { - type: Number, - default: null, - }, - customMin: { - type: Number, - default: null, - }, - customVal: { - type: Number, - default: null, - }, - selectedValidation: { - type: String, - enum: [...Object.values(TextSelectedValidation), null], - default: null, + type: TextValidationOptionsSchema, + default: { + // Defaults are defined here because subdocument paths are undefined by default, and Mongoose does not apply subdocument defaults unless you set the subdocument path to a non-nullish value (see https://mongoosejs.com/docs/subdocs.html) + customVal: null, + selectedValidation: null, }, }, }) + + return LongTextFieldSchema } export default createLongTextFieldSchema diff --git a/src/app/models/field/numberField.ts b/src/app/models/field/numberField.ts index 759fb45ebe..8ed27f99d1 100644 --- a/src/app/models/field/numberField.ts +++ b/src/app/models/field/numberField.ts @@ -1,31 +1,61 @@ import { Schema } from 'mongoose' -import { INumberFieldSchema, NumberSelectedValidation } from '../../../types' +import { + INumberFieldSchema, + NumberSelectedValidation, + NumberValidationOptions, +} from '../../../types' +import { WithCustomMinMax } from '../../../types/field/utils/virtuals' import { MyInfoSchema } from './baseField' const createNumberFieldSchema = () => { - return new Schema({ - myInfo: MyInfoSchema, - ValidationOptions: { - customMax: { - type: Number, - default: null, - }, - customMin: { - type: Number, - default: null, - }, + const ValidationOptionsSchema = new Schema< + WithCustomMinMax + >( + { customVal: { type: Number, - default: null, }, selectedValidation: { type: String, enum: [...Object.values(NumberSelectedValidation), null], - default: null, + }, + }, + { + // TODO: Remove virtuals (#2039) + toJSON: { + virtuals: true, + }, + }, + ) + + // Virtuals to allow for backwards compatibility after customMin and customMax were removed as part of #408 + // TODO: Remove virtuals (#2039) + ValidationOptionsSchema.virtual('customMin').get(function ( + this: NumberValidationOptions, + ) { + return this.customVal + }) + + ValidationOptionsSchema.virtual('customMax').get(function ( + this: NumberValidationOptions, + ) { + return this.customVal + }) + + const NumberFieldSchema = new Schema({ + myInfo: MyInfoSchema, + ValidationOptions: { + type: ValidationOptionsSchema, + default: { + // Defaults are defined here because subdocument paths are undefined by default, and Mongoose does not apply subdocument defaults unless you set the subdocument path to a non-nullish value (see https://mongoosejs.com/docs/subdocs.html) + customVal: null, + selectedValidation: null, }, }, }) + + return NumberFieldSchema } export default createNumberFieldSchema diff --git a/src/app/models/field/shortTextField.ts b/src/app/models/field/shortTextField.ts index 73eb99cdd8..3d46ff939d 100644 --- a/src/app/models/field/shortTextField.ts +++ b/src/app/models/field/shortTextField.ts @@ -1,30 +1,19 @@ import { Schema } from 'mongoose' import { IShortTextFieldSchema } from '../../../types' -import { TextSelectedValidation } from '../../../types/field/baseField' +import { TextValidationOptionsSchema } from './common/textValidationOptionsSchema' import { MyInfoSchema } from './baseField' const createShortTextFieldSchema = () => { - return new Schema({ + const ShortTextFieldSchema = new Schema({ myInfo: MyInfoSchema, ValidationOptions: { - customMax: { - type: Number, - default: null, - }, - customMin: { - type: Number, - default: null, - }, - customVal: { - type: Number, - default: null, - }, - selectedValidation: { - type: String, - enum: [...Object.values(TextSelectedValidation), null], - default: null, + type: TextValidationOptionsSchema, + default: { + // Defaults are defined here because subdocument paths are undefined by default, and Mongoose does not apply subdocument defaults unless you set the subdocument path to a non-nullish value (see https://mongoosejs.com/docs/subdocs.html) + customVal: null, + selectedValidation: null, }, }, allowPrefill: { @@ -33,6 +22,8 @@ const createShortTextFieldSchema = () => { default: false, }, }) + + return ShortTextFieldSchema } export default createShortTextFieldSchema diff --git a/src/app/utils/field-validation/validators/__tests__/number-validation.spec.ts b/src/app/utils/field-validation/validators/__tests__/number-validation.spec.ts index 5df3bd3bec..be26319617 100644 --- a/src/app/utils/field-validation/validators/__tests__/number-validation.spec.ts +++ b/src/app/utils/field-validation/validators/__tests__/number-validation.spec.ts @@ -12,9 +12,7 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: NumberSelectedValidation.Max, - customMin: null, - customMax: 2, - customVal: null, + customVal: 2, }, }) const response = generateNewSingleAnswerResponse(BasicField.Number, { @@ -30,9 +28,7 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: NumberSelectedValidation.Max, - customMin: null, - customMax: 2, - customVal: null, + customVal: 2, }, }) const response = generateNewSingleAnswerResponse(BasicField.Number, { @@ -47,9 +43,7 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: NumberSelectedValidation.Max, - customMin: null, - customMax: 2, - customVal: null, + customVal: 2, }, }) const response = generateNewSingleAnswerResponse(BasicField.Number, { @@ -66,9 +60,7 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: NumberSelectedValidation.Min, - customMin: 2, - customMax: null, - customVal: null, + customVal: 2, }, }) const response = generateNewSingleAnswerResponse(BasicField.Number, { @@ -83,9 +75,7 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: NumberSelectedValidation.Min, - customMin: 2, - customMax: null, - customVal: null, + customVal: 2, }, }) const response = generateNewSingleAnswerResponse(BasicField.Number, { @@ -100,8 +90,6 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: NumberSelectedValidation.Exact, - customMin: null, - customMax: null, customVal: 2, }, }) @@ -117,8 +105,6 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: NumberSelectedValidation.Exact, - customMin: null, - customMax: null, customVal: 2, }, }) @@ -136,8 +122,6 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: NumberSelectedValidation.Max, - customMin: null, - customMax: null, customVal: null, }, }) @@ -153,8 +137,6 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: NumberSelectedValidation.Min, - customMin: null, - customMax: null, customVal: null, }, }) @@ -170,8 +152,6 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: NumberSelectedValidation.Exact, - customMin: null, - customMax: null, customVal: null, }, }) @@ -187,8 +167,6 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, }) @@ -204,8 +182,6 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, required: false, @@ -222,8 +198,6 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, }) @@ -239,8 +213,6 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, }) @@ -258,8 +230,6 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, }) @@ -274,8 +244,6 @@ describe('Number field validation', () => { const formField = generateDefaultField(BasicField.Number, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, }) diff --git a/src/app/utils/field-validation/validators/__tests__/text-validator.spec.ts b/src/app/utils/field-validation/validators/__tests__/text-validator.spec.ts index ab60ce4a27..6d8894e91a 100644 --- a/src/app/utils/field-validation/validators/__tests__/text-validator.spec.ts +++ b/src/app/utils/field-validation/validators/__tests__/text-validator.spec.ts @@ -56,13 +56,11 @@ describe('Text validation', () => { ) }) - it('should disallow fewer characters than customMin if selectedValidation is Exact', () => { + it('should disallow fewer characters than customVal if selectedValidation is Exact', () => { const formField = generateDefaultField(BasicField.ShortText, { ValidationOptions: { selectedValidation: TextSelectedValidation.Exact, - customMin: 10, - customMax: null, - customVal: null, + customVal: 10, }, }) const response = generateNewSingleAnswerResponse(BasicField.ShortText, { @@ -75,13 +73,11 @@ describe('Text validation', () => { ) }) - it('should disallow more characters than customMin if selectedValidation is Exact', () => { + it('should disallow more characters than customVal if selectedValidation is Exact', () => { const formField = generateDefaultField(BasicField.ShortText, { ValidationOptions: { selectedValidation: TextSelectedValidation.Exact, - customMin: 10, - customMax: null, - customVal: null, + customVal: 10, }, }) const response = generateNewSingleAnswerResponse(BasicField.ShortText, { @@ -95,51 +91,11 @@ describe('Text validation', () => { ) }) - it('should disallow fewer characters than customMax if selectedValidation is Exact', () => { - const formField = generateDefaultField(BasicField.ShortText, { - ValidationOptions: { - selectedValidation: TextSelectedValidation.Exact, - customMin: null, - customMax: 10, - customVal: null, - }, - }) - const response = generateNewSingleAnswerResponse(BasicField.ShortText, { - answer: 'fewer', - }) - const validateResult = validateField('formId', formField, response) - expect(validateResult.isErr()).toBe(true) - expect(validateResult._unsafeUnwrapErr()).toEqual( - new ValidateFieldError('Invalid answer submitted'), - ) - }) - - it('should disallow more characters than customMax if selectedValidation is Exact', () => { - const formField = generateDefaultField(BasicField.ShortText, { - ValidationOptions: { - selectedValidation: TextSelectedValidation.Exact, - customMin: null, - customMax: 10, - customVal: null, - }, - }) - const response = generateNewSingleAnswerResponse(BasicField.ShortText, { - answer: 'more than 10 chars', - }) - const validateResult = validateField('formId', formField, response) - expect(validateResult.isErr()).toBe(true) - expect(validateResult._unsafeUnwrapErr()).toEqual( - new ValidateFieldError('Invalid answer submitted'), - ) - }) - - it('should disallow fewer characters than customMin if selectedValidation is Minimum', () => { + it('should disallow fewer characters than customVal if selectedValidation is Minimum', () => { const formField = generateDefaultField(BasicField.ShortText, { ValidationOptions: { selectedValidation: TextSelectedValidation.Minimum, - customMin: 10, - customMax: null, - customVal: null, + customVal: 10, }, }) const response = generateNewSingleAnswerResponse(BasicField.ShortText, { @@ -152,13 +108,11 @@ describe('Text validation', () => { ) }) - it('should disallow more characters than customMax if selectedValidation is Maximum', () => { + it('should disallow more characters than customVal if selectedValidation is Maximum', () => { const formField = generateDefaultField(BasicField.ShortText, { ValidationOptions: { selectedValidation: TextSelectedValidation.Maximum, - customMin: null, - customMax: 10, - customVal: null, + customVal: 10, }, }) const response = generateNewSingleAnswerResponse(BasicField.ShortText, { @@ -174,8 +128,6 @@ describe('Text validation', () => { const formField = generateDefaultField(BasicField.ShortText, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, }) @@ -198,8 +150,6 @@ describe('Text validation', () => { const formField = generateDefaultField(BasicField.LongText, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, }) @@ -217,8 +167,6 @@ describe('Text validation', () => { const formField = generateDefaultField(BasicField.LongText, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, required: false, @@ -235,8 +183,6 @@ describe('Text validation', () => { const formField = generateDefaultField(BasicField.LongText, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, }) @@ -252,8 +198,6 @@ describe('Text validation', () => { const formField = generateDefaultField(BasicField.LongText, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, }) @@ -267,51 +211,11 @@ describe('Text validation', () => { ) }) - it('should disallow fewer characters than customMin if selectedValidation is Exact', () => { + it('should disallow fewer characters than customVal if selectedValidation is Exact', () => { const formField = generateDefaultField(BasicField.LongText, { ValidationOptions: { selectedValidation: TextSelectedValidation.Exact, - customMin: 10, - customMax: null, - customVal: null, - }, - }) - const response = generateNewSingleAnswerResponse(BasicField.LongText, { - answer: 'less', - }) - const validateResult = validateField('formId', formField, response) - expect(validateResult.isErr()).toBe(true) - expect(validateResult._unsafeUnwrapErr()).toEqual( - new ValidateFieldError('Invalid answer submitted'), - ) - }) - - it('should disallow more characters than customMin if selectedValidation is Exact', () => { - const formField = generateDefaultField(BasicField.LongText, { - ValidationOptions: { - selectedValidation: TextSelectedValidation.Exact, - customMin: 10, - customMax: null, - customVal: null, - }, - }) - const response = generateNewSingleAnswerResponse(BasicField.LongText, { - answer: 'more than 10 chars', - }) - const validateResult = validateField('formId', formField, response) - expect(validateResult.isErr()).toBe(true) - expect(validateResult._unsafeUnwrapErr()).toEqual( - new ValidateFieldError('Invalid answer submitted'), - ) - }) - - it('should disallow fewer characters than customMax if selectedValidation is Exact', () => { - const formField = generateDefaultField(BasicField.LongText, { - ValidationOptions: { - selectedValidation: TextSelectedValidation.Exact, - customMin: null, - customMax: 10, - customVal: null, + customVal: 10, }, }) const response = generateNewSingleAnswerResponse(BasicField.LongText, { @@ -324,13 +228,11 @@ describe('Text validation', () => { ) }) - it('should disallow more characters than customMax if selectedValidation is Exact', () => { + it('should disallow more characters than customVal if selectedValidation is Exact', () => { const formField = generateDefaultField(BasicField.LongText, { ValidationOptions: { selectedValidation: TextSelectedValidation.Exact, - customMin: null, - customMax: 10, - customVal: null, + customVal: 10, }, }) const response = generateNewSingleAnswerResponse(BasicField.LongText, { @@ -343,13 +245,11 @@ describe('Text validation', () => { ) }) - it('should disallow fewer characters than customMin if selectedValidation is Minimum', () => { + it('should disallow fewer characters than customVal if selectedValidation is Minimum', () => { const formField = generateDefaultField(BasicField.LongText, { ValidationOptions: { selectedValidation: TextSelectedValidation.Minimum, - customMin: 10, - customMax: null, - customVal: null, + customVal: 10, }, }) const response = generateNewSingleAnswerResponse(BasicField.LongText, { @@ -362,13 +262,11 @@ describe('Text validation', () => { ) }) - it('should disallow more characters than customMax if selectedValidation is Maximum', () => { + it('should disallow more characters than customVal if selectedValidation is Maximum', () => { const formField = generateDefaultField(BasicField.LongText, { ValidationOptions: { selectedValidation: TextSelectedValidation.Maximum, - customMin: null, - customMax: 10, - customVal: null, + customVal: 10, }, }) const response = generateNewSingleAnswerResponse(BasicField.LongText, { @@ -384,8 +282,6 @@ describe('Text validation', () => { const formField = generateDefaultField(BasicField.LongText, { ValidationOptions: { selectedValidation: null, - customMin: null, - customMax: null, customVal: null, }, }) diff --git a/src/app/utils/field-validation/validators/numberValidator.ts b/src/app/utils/field-validation/validators/numberValidator.ts index 23307d9fa1..6568e5f0f9 100644 --- a/src/app/utils/field-validation/validators/numberValidator.ts +++ b/src/app/utils/field-validation/validators/numberValidator.ts @@ -29,8 +29,8 @@ const numberFormatValidator: NumberValidator = (response) => { const minLengthValidator: NumberValidatorConstructor = (numberField) => (response) => { const { answer } = response - const { customMin } = numberField.ValidationOptions - return !customMin || answer.length >= customMin + const { customVal } = numberField.ValidationOptions + return !customVal || answer.length >= customVal ? right(response) : left(`NumberValidator:\t answer is shorter than custom minimum length`) } @@ -42,8 +42,8 @@ const minLengthValidator: NumberValidatorConstructor = const maxLengthValidator: NumberValidatorConstructor = (numberField) => (response) => { const { answer } = response - const { customMax } = numberField.ValidationOptions - return !customMax || answer.length <= customMax + const { customVal } = numberField.ValidationOptions + return !customVal || answer.length <= customVal ? right(response) : left(`NumberValidator:\t answer is longer than custom maximum length`) } diff --git a/src/app/utils/field-validation/validators/textValidator.ts b/src/app/utils/field-validation/validators/textValidator.ts index b58aea807c..83d5ef8dcb 100644 --- a/src/app/utils/field-validation/validators/textValidator.ts +++ b/src/app/utils/field-validation/validators/textValidator.ts @@ -19,8 +19,7 @@ type TextFieldValidatorConstructor = ( */ const minLengthValidator: TextFieldValidatorConstructor = (textField) => (response) => { - const { customMin } = textField.ValidationOptions - const min = customMin !== null ? Number(customMin) : null + const { customVal: min } = textField.ValidationOptions if (min === null) return right(response) return response.answer.length >= min ? right(response) @@ -33,8 +32,7 @@ const minLengthValidator: TextFieldValidatorConstructor = */ const maxLengthValidator: TextFieldValidatorConstructor = (textField) => (response) => { - const { customMax } = textField.ValidationOptions - const max = customMax !== null ? Number(customMax) : null + const { customVal: max } = textField.ValidationOptions if (max === null) return right(response) return response.answer.length <= max ? right(response) @@ -49,13 +47,7 @@ const maxLengthValidator: TextFieldValidatorConstructor = */ const exactLengthValidator: TextFieldValidatorConstructor = (textField) => (response) => { - const { customMin, customMax } = textField.ValidationOptions - const exact = - customMin !== null - ? Number(customMin) - : customMax !== null - ? Number(customMax) - : null + const { customVal: exact } = textField.ValidationOptions if (exact === null) return right(response) return response.answer.length === exact ? right(response) diff --git a/src/public/modules/forms/admin/constants/covid19.js b/src/public/modules/forms/admin/constants/covid19.js index 29a39dca5c..e3a6c39fa6 100644 --- a/src/public/modules/forms/admin/constants/covid19.js +++ b/src/public/modules/forms/admin/constants/covid19.js @@ -99,8 +99,6 @@ const templates = [ }, { ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, @@ -115,8 +113,6 @@ const templates = [ }, { ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, @@ -184,8 +180,6 @@ const templates = [ }, { ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, @@ -246,8 +240,6 @@ const templates = [ }, { ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, @@ -261,8 +253,6 @@ const templates = [ }, { ValidationOptions: { - customMax: 6, - customMin: 6, customVal: 6, selectedValidation: 'Exact', }, @@ -323,8 +313,6 @@ const templates = [ }, { ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, @@ -357,8 +345,6 @@ const templates = [ }, { ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, @@ -543,8 +529,6 @@ const templates = [ form_fields: [ { ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, @@ -624,8 +608,6 @@ const templates = [ }, { ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, @@ -692,8 +674,6 @@ const templates = [ }, { ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, @@ -735,8 +715,6 @@ const templates = [ }, { ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, @@ -750,8 +728,6 @@ const templates = [ }, { ValidationOptions: { - customMax: 6, - customMin: 6, customVal: 6, selectedValidation: 'Exact', }, @@ -897,8 +873,6 @@ const templates = [ }, { ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, diff --git a/src/public/modules/forms/admin/controllers/edit-fields-modal.client.controller.js b/src/public/modules/forms/admin/controllers/edit-fields-modal.client.controller.js index a7e076bbf7..997c3ed742 100644 --- a/src/public/modules/forms/admin/controllers/edit-fields-modal.client.controller.js +++ b/src/public/modules/forms/admin/controllers/edit-fields-modal.client.controller.js @@ -227,8 +227,6 @@ function EditFieldsModalController( vm.clearCustomValidation = function (field) { field.ValidationOptions = { selectedValidation: null, - customMax: null, - customMin: null, customVal: null, } } @@ -236,22 +234,11 @@ function EditFieldsModalController( let temp = field.ValidationOptions.selectedValidation // Reset all custom validation params to null, keep selected validation option field.ValidationOptions = { - customMax: null, - customMin: null, customVal: null, selectedValidation: temp, } } - vm.changeFxn = function (field) { - let selected = field.ValidationOptions.selectedValidation - let val = field.ValidationOptions.customVal - field.ValidationOptions.customMax = - selected === 'Maximum' || selected === 'Exact' ? val : null - field.ValidationOptions.customMin = - selected === 'Minimum' || selected === 'Exact' ? val : null - } - vm.setFocus = function () { angular.element('#customValInputBox').focus() } diff --git a/src/public/modules/forms/admin/views/edit-fields.client.modal.html b/src/public/modules/forms/admin/views/edit-fields.client.modal.html index 1d0472b426..70aadb680b 100644 --- a/src/public/modules/forms/admin/views/edit-fields.client.modal.html +++ b/src/public/modules/forms/admin/views/edit-fields.client.modal.html @@ -1081,7 +1081,6 @@

class="input-custom input-medium" placeholder="No. of Characters" ng-disabled="!vm.field.ValidationOptions.selectedValidation" - ng-change="vm.changeFxn(vm.field)" />

diff --git a/src/public/modules/forms/base/componentViews/field-number.client.view.html b/src/public/modules/forms/base/componentViews/field-number.client.view.html index a6ba22465b..27abe19ed0 100644 --- a/src/public/modules/forms/base/componentViews/field-number.client.view.html +++ b/src/public/modules/forms/base/componentViews/field-number.client.view.html @@ -31,8 +31,8 @@ ng-required="vm.field.required" ng-disabled="vm.field.disabled" ng-pattern="/^\d*$/" - ng-maxlength="{{ vm.field.ValidationOptions ? vm.field.ValidationOptions.customMax : '' }}" - ng-minlength="{{ vm.field.ValidationOptions ? vm.field.ValidationOptions.customMin : ''}}" + ng-maxlength="{{ vm.field.ValidationOptions && ['Exact', 'Maximum'].includes(vm.field.ValidationOptions.selectedValidation) ? vm.field.ValidationOptions.customVal : ''}}" + ng-minlength="{{ vm.field.ValidationOptions && ['Exact', 'Minimum'].includes(vm.field.ValidationOptions.selectedValidation) ? vm.field.ValidationOptions.customVal : ''}}" ng-model-options="{ allowInvalid: true }" ng-keyup="vm.forms.myForm[(vm.field._id || 'defaultID')].$setTouched()" ng-attr-placeholder="{{ vm.placeholder }}" @@ -62,17 +62,17 @@
- Minimum {{vm.field.ValidationOptions.customMin}} characters + Minimum {{vm.field.ValidationOptions.customVal}} characters ({{vm.forms.myForm[(vm.field._id || - 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customMin}}) + 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customVal}})
- Maximum {{vm.field.ValidationOptions.customMax}} characters + Maximum {{vm.field.ValidationOptions.customVal}} characters ({{vm.forms.myForm[(vm.field._id || - 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customMax}}) + 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customVal}})
diff --git a/src/public/modules/forms/base/componentViews/field-textarea.client.view.html b/src/public/modules/forms/base/componentViews/field-textarea.client.view.html index 59d7e05f2d..118a0ec194 100644 --- a/src/public/modules/forms/base/componentViews/field-textarea.client.view.html +++ b/src/public/modules/forms/base/componentViews/field-textarea.client.view.html @@ -28,8 +28,8 @@ name="{{ vm.field._id || 'defaultID' }}" class="input-custom input-large" ng-model="vm.field.fieldValue" - ng-maxlength="{{ vm.field.ValidationOptions ? vm.field.ValidationOptions.customMax : ''}}" - ng-minlength="{{ vm.field.ValidationOptions ? vm.field.ValidationOptions.customMin : ''}}" + ng-maxlength="{{ vm.field.ValidationOptions && ['Exact', 'Maximum'].includes(vm.field.ValidationOptions.selectedValidation) ? vm.field.ValidationOptions.customVal : ''}}" + ng-minlength="{{ vm.field.ValidationOptions && ['Exact', 'Minimum'].includes(vm.field.ValidationOptions.selectedValidation) ? vm.field.ValidationOptions.customVal : ''}}" ng-model-options="{ allowInvalid: true }" ng-required="vm.field.required" ng-disabled="vm.field.disabled" @@ -58,18 +58,18 @@
- Minimum {{vm.field.ValidationOptions.customMin}} characters - ({{vm.field.fieldValue.length}}/{{vm.field.ValidationOptions.customMin}}) + Minimum {{vm.field.ValidationOptions.customVal}} characters + ({{vm.field.fieldValue.length}}/{{vm.field.ValidationOptions.customVal}})
- Maximum {{vm.field.ValidationOptions.customMax}} characters - ({{vm.field.fieldValue.length}}/{{vm.field.ValidationOptions.customMax}}) + Maximum {{vm.field.ValidationOptions.customVal}} characters + ({{vm.field.fieldValue.length}}/{{vm.field.ValidationOptions.customVal}})
- Must be exactly {{vm.field.ValidationOptions.customMax}} characters - ({{vm.field.fieldValue.length}}/{{vm.field.ValidationOptions.customMax}}) + Must be exactly {{vm.field.ValidationOptions.customVal}} characters + ({{vm.field.fieldValue.length}}/{{vm.field.ValidationOptions.customVal}})
diff --git a/src/public/modules/forms/base/componentViews/field-textfield.client.view.html b/src/public/modules/forms/base/componentViews/field-textfield.client.view.html index da39ac5557..6d898ff599 100644 --- a/src/public/modules/forms/base/componentViews/field-textfield.client.view.html +++ b/src/public/modules/forms/base/componentViews/field-textfield.client.view.html @@ -30,8 +30,8 @@ ng-disabled="vm.field.disabled" ng-pattern="vm.pattern" ng-class="vm.field.allowPrefill && vm.field.isPrefilled && 'prefill-highlight'" - ng-maxlength="{{vm.field.ValidationOptions ? vm.field.ValidationOptions.customMax : ''}}" - ng-minlength="{{vm.field.ValidationOptions ? vm.field.ValidationOptions.customMin : ''}}" + ng-maxlength="{{ vm.field.ValidationOptions && ['Exact', 'Maximum'].includes(vm.field.ValidationOptions.selectedValidation) ? vm.field.ValidationOptions.customVal : ''}}" + ng-minlength="{{ vm.field.ValidationOptions && ['Exact', 'Minimum'].includes(vm.field.ValidationOptions.selectedValidation) ? vm.field.ValidationOptions.customVal : ''}}" ng-model-options="{ allowInvalid: true }" ng-keyup="vm.forms.myForm[(vm.field._id || 'defaultID')].$setTouched()" ng-attr-placeholder="{{ vm.placeholder }}" @@ -54,17 +54,17 @@
- Minimum {{vm.field.ValidationOptions.customMin}} characters + Minimum {{vm.field.ValidationOptions.customVal}} characters ({{vm.forms.myForm[(vm.field._id || - 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customMin}}) + 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customVal}})
- Maximum {{vm.field.ValidationOptions.customMax}} characters + Maximum {{vm.field.ValidationOptions.customVal}} characters ({{vm.forms.myForm[(vm.field._id || - 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customMax}}) + 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customVal}})
diff --git a/src/public/modules/forms/viewmodels/Fields/MixIns.js b/src/public/modules/forms/viewmodels/Fields/MixIns.js index 3b2e0ffd0f..aba9eb7554 100644 --- a/src/public/modules/forms/viewmodels/Fields/MixIns.js +++ b/src/public/modules/forms/viewmodels/Fields/MixIns.js @@ -32,8 +32,6 @@ const CustomValidation = (Base) => getDefaultBasicData() { const fieldData = super.getDefaultBasicData() fieldData.ValidationOptions = { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, } diff --git a/src/types/field/baseField.ts b/src/types/field/baseField.ts index 7301e622c4..a66bfe4dcf 100644 --- a/src/types/field/baseField.ts +++ b/src/types/field/baseField.ts @@ -54,5 +54,4 @@ export enum TextSelectedValidation { Maximum = 'Maximum', Minimum = 'Minimum', Exact = 'Exact', - Range = 'Range', // TODO(#408) - questionable value } diff --git a/src/types/field/longTextField.ts b/src/types/field/longTextField.ts index 6957939890..59b8eac9f5 100644 --- a/src/types/field/longTextField.ts +++ b/src/types/field/longTextField.ts @@ -1,14 +1,6 @@ -import { IField, IFieldSchema, TextSelectedValidation } from './baseField' +import { ITextField } from './utils/textField' +import { IFieldSchema } from './baseField' -export type LongTextValidationOptions = { - customMax: number | null - customMin: number | null - customVal: number | null - selectedValidation: TextSelectedValidation | null -} - -export interface ILongTextField extends IField { - ValidationOptions: LongTextValidationOptions -} +export type ILongTextField = ITextField export interface ILongTextFieldSchema extends ILongTextField, IFieldSchema {} diff --git a/src/types/field/numberField.ts b/src/types/field/numberField.ts index 1fe8cf7f1f..f8c4ca908c 100644 --- a/src/types/field/numberField.ts +++ b/src/types/field/numberField.ts @@ -4,12 +4,9 @@ export enum NumberSelectedValidation { Max = 'Maximum', Min = 'Minimum', Exact = 'Exact', - Range = 'Range', } export type NumberValidationOptions = { - customMax: number | null - customMin: number | null customVal: number | null selectedValidation: NumberSelectedValidation | null } diff --git a/src/types/field/shortTextField.ts b/src/types/field/shortTextField.ts index a1d5e4dcfb..b2ab928a3a 100644 --- a/src/types/field/shortTextField.ts +++ b/src/types/field/shortTextField.ts @@ -1,15 +1,7 @@ -import { IField, IFieldSchema, TextSelectedValidation } from './baseField' +import { ITextField } from './utils/textField' +import { IFieldSchema } from './baseField' -export type ShortTextValidationOptions = { - customMax: number | null - customMin: number | null - customVal: number | null - selectedValidation: TextSelectedValidation | null -} - -export interface IShortTextField extends IField { - ValidationOptions: ShortTextValidationOptions -} +export type IShortTextField = ITextField export interface IShortTextFieldSchema extends IShortTextField, IFieldSchema { // Prefill flag diff --git a/src/types/field/utils/textField.ts b/src/types/field/utils/textField.ts new file mode 100644 index 0000000000..8678c70d1f --- /dev/null +++ b/src/types/field/utils/textField.ts @@ -0,0 +1,15 @@ +import { IField, IFieldSchema, TextSelectedValidation } from '../baseField' + +/** + * Utility types for short and long text field + */ +export type TextValidationOptions = { + customVal: number | null + selectedValidation: TextSelectedValidation | null +} + +export interface ITextField extends IField { + ValidationOptions: TextValidationOptions +} + +export interface ITextFieldSchema extends ITextField, IFieldSchema {} diff --git a/src/types/field/utils/virtuals.ts b/src/types/field/utils/virtuals.ts new file mode 100644 index 0000000000..fe670d2485 --- /dev/null +++ b/src/types/field/utils/virtuals.ts @@ -0,0 +1,9 @@ +import { Document } from 'mongoose' + +// Types for virtuals for backwards compatibility after customMin and customMax were removed as part of #408 +// TODO: Remove virtual type (#2039) +export type WithCustomMinMax = T & + Document & { + customMin: number | null + customMax: number | null + } diff --git a/tests/unit/backend/helpers/generate-form-data.ts b/tests/unit/backend/helpers/generate-form-data.ts index 2fcc97e109..fc3f4e2649 100644 --- a/tests/unit/backend/helpers/generate-form-data.ts +++ b/tests/unit/backend/helpers/generate-form-data.ts @@ -376,8 +376,6 @@ export const generateTableShortTextColumn = ( required: true, _id: new ObjectId().toHexString(), ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, @@ -390,8 +388,6 @@ export const generateTableShortTextColumn = ( required: true, _id: new ObjectId().toHexString(), ValidationOptions: { - customMax: null, - customMin: null, customVal: null, selectedValidation: null, }, From fe35c553fefe4694ee28920d8f04a772d1428691 Mon Sep 17 00:00:00 2001 From: seaerchin <44049504+seaerchin@users.noreply.github.com> Date: Wed, 9 Jun 2021 11:35:49 +0800 Subject: [PATCH 08/56] refactor(ts-migration): ndjsonstream and process-decrypted-content (#2111) * refactor(process-decrypted-content): ported over process-decrypted-content to typescript * refactor(public/modules/forms/admin): changed callsites for process-decrypted-content * refactor(process-decrypted-content): updated type predicate * refactor(process-decrypted-content): inlined typeguard for neatness * refactor(ts-migration): migrates ndjsonstream to typescript (#2053) * feat(ndjsonstream): refactored ndjsonstream and removed old js version * refactor(submissions.client.factory): updated callsite to use new version of ndjsonstream * revert(ndjsonstream): adds data_buf back to prevent regression * fix(ndjsonstream): fixed errorneous initial conditional * fix(ndjsontream): fixed insidious bug causing occasional off by 1 * revert(ndjsonstream): reverts slice/map to for loops * revert(ndjsonstream): reverts single line return --- .../view-responses.client.controller.js | 4 +- .../forms/helpers/decryption.worker.js | 4 +- .../modules/forms/helpers/ndjsonStream.js | 70 -------------- .../modules/forms/helpers/ndjsonStream.ts | 73 +++++++++++++++ .../helpers/process-decrypted-content.js | 87 ------------------ .../helpers/process-decrypted-content.ts | 91 +++++++++++++++++++ .../services/submissions.client.factory.js | 2 +- 7 files changed, 171 insertions(+), 160 deletions(-) delete mode 100644 src/public/modules/forms/helpers/ndjsonStream.js create mode 100644 src/public/modules/forms/helpers/ndjsonStream.ts delete mode 100644 src/public/modules/forms/helpers/process-decrypted-content.js create mode 100644 src/public/modules/forms/helpers/process-decrypted-content.ts diff --git a/src/public/modules/forms/admin/controllers/view-responses.client.controller.js b/src/public/modules/forms/admin/controllers/view-responses.client.controller.js index 98889fe6a3..22d65c2804 100644 --- a/src/public/modules/forms/admin/controllers/view-responses.client.controller.js +++ b/src/public/modules/forms/admin/controllers/view-responses.client.controller.js @@ -1,6 +1,8 @@ 'use strict' -const processDecryptedContent = require('../../helpers/process-decrypted-content') +const { + processDecryptedContent, +} = require('../../helpers/process-decrypted-content') const { triggerFileDownload } = require('../../helpers/util') const SHOW_PROGRESS_DELAY_MS = 3000 diff --git a/src/public/modules/forms/helpers/decryption.worker.js b/src/public/modules/forms/helpers/decryption.worker.js index 647fa0f0bf..9f8b3abe7a 100644 --- a/src/public/modules/forms/helpers/decryption.worker.js +++ b/src/public/modules/forms/helpers/decryption.worker.js @@ -8,7 +8,9 @@ const JSZip = require('jszip') const moment = require('moment-timezone') const { default: PQueue } = require('p-queue') -const processDecryptedContent = require('../helpers/process-decrypted-content') +const { + processDecryptedContent, +} = require('../helpers/process-decrypted-content') const { TRANSACTION_EXPIRE_AFTER_SECONDS, } = require('../../../../shared/util/verification') diff --git a/src/public/modules/forms/helpers/ndjsonStream.js b/src/public/modules/forms/helpers/ndjsonStream.js deleted file mode 100644 index 4df867a980..0000000000 --- a/src/public/modules/forms/helpers/ndjsonStream.js +++ /dev/null @@ -1,70 +0,0 @@ -/* eslint-disable camelcase */ -'use strict' - -// Modified fork of https://github.com/canjs/can-ndjson-stream to enqueue -// the string immediately without a JSON.parse() step, as the stream payload -// is to be decrypted by the decryption worker. - -// Note that this code assumes a polyfill of TextDecoder is available to run in IE11. - -const ndjsonStream = function (response) { - // For cancellation - var is_reader - var cancellationRequest = false - return new ReadableStream({ - start: function (controller) { - var reader = response.getReader() - is_reader = reader - var decoder = new TextDecoder() - var data_buf = '' - - reader.read().then(function processResult(result) { - if (result.done) { - if (cancellationRequest) { - // Immediately exit - return - } - - data_buf = data_buf.trim() - if (data_buf.length !== 0) { - try { - controller.enqueue(data_buf) - } catch (e) { - controller.error(e) - return - } - } - controller.close() - return - } - - var data = decoder.decode(result.value, { stream: true }) - data_buf += data - var lines = data_buf.split('\n') - for (var i = 0; i < lines.length - 1; ++i) { - var l = lines[i].trim() - if (l.length > 0) { - try { - controller.enqueue(l) - } catch (e) { - controller.error(e) - cancellationRequest = true - reader.cancel() - return - } - } - } - data_buf = lines[lines.length - 1] - - return reader.read().then(processResult) - }) - }, - cancel: function (reason) { - console.log('Cancel registered due to ', reason) - cancellationRequest = true - is_reader.cancel() - }, - }) -} - -module.exports = ndjsonStream diff --git a/src/public/modules/forms/helpers/ndjsonStream.ts b/src/public/modules/forms/helpers/ndjsonStream.ts new file mode 100644 index 0000000000..829cb3c91c --- /dev/null +++ b/src/public/modules/forms/helpers/ndjsonStream.ts @@ -0,0 +1,73 @@ +// Modified fork of https://github.com/canjs/can-ndjson-stream to enqueue +// the string immediately without a JSON.parse() step, as the stream payload +// is to be decrypted by the decryption worker. + +// Note that this code assumes a polyfill of TextDecoder is available to run in IE11. + +export const ndjsonStream = ( + response: ReadableStream, +): ReadableStream => { + // For cancellation + let maybeReader: ReadableStreamDefaultReader | undefined + let shouldCancel = false + return new ReadableStream({ + start: function (controller) { + const reader = response.getReader() + maybeReader = reader + const decoder = new TextDecoder() + let data_buf = '' + + return reader + .read() + .then(function processResult(result): Promise | undefined | void { + if (result.done && shouldCancel) { + return + } + + if (result.done) { + data_buf = data_buf.trim() + if (data_buf.length !== 0) { + try { + controller.enqueue(data_buf) + } catch (e) { + controller.error(e) + return + } + } + controller.close() + return + } + + // Read the input in as a stream and split by newline and trim + data_buf += decoder.decode(result.value, { stream: true }) + const lines = data_buf.split('\n') + + // Reads in every line BUT the last + // Trims the line and queues it in the controller if there is content in the line + for (let i = 0; i < lines.length - 1; ++i) { + const l = lines[i].trim() + if (l.length > 0) { + try { + controller.enqueue(l) + } catch (e) { + controller.error(e) + shouldCancel = true + void reader.cancel() + return + } + } + } + data_buf = lines[lines.length - 1] + + return reader.read().then(processResult) + }) + }, + cancel: function (reason) { + console.log('Cancel registered due to ', reason) + shouldCancel = true + if (maybeReader) { + void maybeReader.cancel() + } + }, + }) +} diff --git a/src/public/modules/forms/helpers/process-decrypted-content.js b/src/public/modules/forms/helpers/process-decrypted-content.js deleted file mode 100644 index a1e4cb2afa..0000000000 --- a/src/public/modules/forms/helpers/process-decrypted-content.js +++ /dev/null @@ -1,87 +0,0 @@ -const { SPCPFieldTitle } = require('../../../../types') -const { - CURRENT_VERIFIED_FIELDS, - VerifiedKeys, -} = require('../../../../shared/util/verified-content') -/** - * Returns a response matching the given type containing the given value. - * @param {string} type the field type to match - * @param {any} value the value to insert into the response to be returned - * @returns the desired response object if type is valid. Else returns null. - */ -const getResponseFromVerifiedField = (type, value) => { - switch (type) { - case VerifiedKeys.SpUinFin: - return { - question: SPCPFieldTitle.SpNric, - fieldType: 'nric', - answer: value, - // Just a unique identifier for CSV header uniqueness - _id: SPCPFieldTitle.SpNric, - } - case VerifiedKeys.CpUen: - return { - question: SPCPFieldTitle.CpUen, - fieldType: 'textfield', - answer: value, - _id: SPCPFieldTitle.CpUen, - } - case VerifiedKeys.CpUid: - return { - question: SPCPFieldTitle.CpUid, - fieldType: 'nric', - answer: value, - _id: SPCPFieldTitle.CpUid, - } - default: - return null - } -} - -/** - * Converts a decrypted verified object into an array with the same shape as the - * current decrypted content to be concatenated with the decrypted content. - * NOTE: This function assumes verifiedObj is an object with simple string - * key-value pairs. - * @param {Object} verifiedObj the object to convert - * @returns the converted array. - */ -const convertToResponseArray = (verifiedObj) => { - let verifiedResponses = [] - CURRENT_VERIFIED_FIELDS.forEach((fieldType) => { - if (Object.prototype.hasOwnProperty.call(verifiedObj, fieldType)) { - const verifiedResponse = getResponseFromVerifiedField( - fieldType, - verifiedObj[fieldType], - ) - - if (!verifiedResponse) return - verifiedResponses.push(verifiedResponse) - } - }) - - return verifiedResponses -} - -/** - * Processes the decrypted content containing the previously encrypted responses - * and verified content, and combines them into a single response array. - * @param {Object} decrypted the decrypted content to process - * @param {Object[]} decrypted.responses the previously encrypted responses content - * @param {Object} decrypted.verified the previously encrypted verified content,if it exists - * @returns {Object[]} the processed content - */ -const processDecryptedContent = (decrypted) => { - const { responses, verified } = decrypted - let displayedContent - // Convert decrypted content into displayable object. - displayedContent = responses - - if (verified) { - displayedContent = displayedContent.concat(convertToResponseArray(verified)) - } - - return displayedContent -} - -module.exports = processDecryptedContent diff --git a/src/public/modules/forms/helpers/process-decrypted-content.ts b/src/public/modules/forms/helpers/process-decrypted-content.ts new file mode 100644 index 0000000000..8bcdc64696 --- /dev/null +++ b/src/public/modules/forms/helpers/process-decrypted-content.ts @@ -0,0 +1,91 @@ +import { + DecryptedContent, + FormField as VerifiedFormField, +} from '@opengovsg/formsg-sdk/dist/types' +import has from 'lodash/has' + +import { + CURRENT_VERIFIED_FIELDS, + VerifiedKeys, +} from '../../../../shared/util/verified-content' +import { BasicField, SPCPFieldTitle } from '../../../../types' + +/** + * Returns a response matching the given type containing the given value. + * @param type the field type to match + * @param value the value to insert into the response to be returned + * @returns the desired response object if type is valid. Else returns null. + */ +const getResponseFromVerifiedField = ( + type: VerifiedKeys, + value: string, +): VerifiedFormField | null => { + switch (type) { + case VerifiedKeys.SpUinFin: + return { + question: SPCPFieldTitle.SpNric, + fieldType: BasicField.Nric, + answer: value, + // Just a unique identifier for CSV header uniqueness + _id: SPCPFieldTitle.SpNric, + } + + case VerifiedKeys.CpUen: + return { + question: SPCPFieldTitle.CpUen, + fieldType: BasicField.ShortText, + answer: value, + _id: SPCPFieldTitle.CpUen, + } + + case VerifiedKeys.CpUid: + return { + question: SPCPFieldTitle.CpUid, + fieldType: BasicField.Nric, + answer: value, + _id: SPCPFieldTitle.CpUid, + } + default: + return null + } +} + +/** + * Converts a decrypted verified object into an array with the same shape as the + * current decrypted content to be concatenated with the decrypted content. + * NOTE: This function assumes verifiedObj is an object with simple string + * key-value pairs. + * @param verifiedObj the object to convert + * @returns the converted array. + */ +const convertToResponseArray = ( + verifiedObj: Record, +): VerifiedFormField[] => { + return CURRENT_VERIFIED_FIELDS.filter((fieldType) => + has(verifiedObj, fieldType), + ) + .map((fieldType) => + getResponseFromVerifiedField(fieldType, verifiedObj[fieldType]), + ) + .filter((field): field is VerifiedFormField => { + return !!field + }) +} + +/** + * Processes the decrypted content containing the previously encrypted responses + * and verified content, and combines them into a single response array. + * @param decrypted.responses the previously encrypted responses content + * @param decrypted.verified the previously encrypted verified content,if it exists + * @returns the processed content + */ +export const processDecryptedContent = ( + decrypted: DecryptedContent, +): VerifiedFormField[] => { + const { responses: displayedContent, verified } = decrypted + // Convert decrypted content into displayable object. + + return verified + ? displayedContent.concat(convertToResponseArray(verified)) + : displayedContent +} diff --git a/src/public/modules/forms/services/submissions.client.factory.js b/src/public/modules/forms/services/submissions.client.factory.js index c8fdacf55f..5d339ba185 100644 --- a/src/public/modules/forms/services/submissions.client.factory.js +++ b/src/public/modules/forms/services/submissions.client.factory.js @@ -3,7 +3,7 @@ const CsvMHGenerator = require('../helpers/CsvMergedHeadersGenerator') const DecryptionWorker = require('../helpers/decryption.worker.js') const { fixParamsToUrl, triggerFileDownload } = require('../helpers/util') -const ndjsonStream = require('../helpers/ndjsonStream') +const { ndjsonStream } = require('../helpers/ndjsonStream') const fetchStream = require('fetch-readablestream') const { decode: decodeBase64 } = require('@stablelib/base64') const JSZip = require('jszip') From e8f6c65ff0e0c517d67ddf71aef6843592b7fe1d Mon Sep 17 00:00:00 2001 From: Chow Yi Yin <54089291+chowyiyin@users.noreply.github.com> Date: Wed, 9 Jun 2021 12:27:09 +0800 Subject: [PATCH 09/56] refactor: replace $resource in angularjs form-api.client.factory.js with typescript FormService (#1947) * refactor: replace object with typescript FormService * test: add test for FormService * refactor: edit calls to fit new function signature and remove angularjs * rfix: add missing parameter in transferOwner and cfix reading error message in collaborators client controller * refactor: add .when to all FormApi calls * refactor: extract interceptors from FormService and replace original interceptors * test: remove interceptor from FormService tests * refactor: add .when to FormApi calls * docs: add documentation and reaname some functions * refactor: edit calls to fit new function signature and remove angularjs * refactor: add .when to all FormApi calls * refactor: add .when to FormApi calls * refactor: rename getDashboardViews to getDashboardView * fix: rename getDashboardViews in form-api.client.factory.js * refactor: remove headers for IE11 support * refactor: split FormService into different services * split test into relevant files * refactor: update other files after rename of AdminFormService * refactor: change services and rename formapi functions * refactor: remove outdated comment * refactor: remove unnecessary * docs: update jsdocs regarding AdminFormService * refactor: remove unnecessary q.when * refactor: change from absolute to relative import in AdminViewFormService * refactor: shift deleteForm to UpdateFormService * fix: change service deleteForm * refactor: make tcalls to transformations explicit * test: edit tests to improve readability * refactor: update imports * test: fix imports in tests * style: fix eslint style in tests * style: fix prettier errors * fix: fix import statement in AdminSubmissionService * fix: fix import statement in test * fix: replace formApi.template with formApi.queryTemplate * fix: fix incorrect endpoint for query and use of template --- .../components/edit-logic.client.component.js | 4 +- .../admin-form.client.controller.js | 26 +- .../collaborator-modal.client.controller.js | 18 +- .../create-form-modal.client.controller.js | 26 +- .../delete-form-modal.client.controller.js | 12 +- .../edit-logic-modal.client.controller.js | 6 +- .../list-forms.client.controller.js | 10 +- .../base/directives/submit-form.directive.js | 6 +- .../forms/config/forms.client.routes.js | 14 +- .../forms/services/form-api.client.factory.js | 281 ++++++++++-------- .../services/form-factory.client.service.js | 6 +- .../examples-card.client.directive.js | 8 +- .../services/AdminSubmissionsService.ts | 2 +- src/public/services/AdminViewFormService.ts | 43 +++ src/public/services/CreateFormService.ts | 38 +++ src/public/services/ExamplesService.ts | 31 +- src/public/services/PublicFormService.ts | 15 + ...minFormService.ts => UpdateFormService.ts} | 48 ++- .../__tests__/AdminSubmissionsService.test.ts | 2 +- .../__tests__/AdminViewFormService.test.ts | 147 +++++++++ .../__tests__/CreateFormService.test.ts | 114 +++++++ .../__tests__/ExamplesService.test.ts | 105 +++++++ .../__tests__/PublicFormService.test.ts | 40 +++ ...vice.test.ts => UpdateFormService.test.ts} | 134 ++++++++- 24 files changed, 948 insertions(+), 188 deletions(-) create mode 100644 src/public/services/AdminViewFormService.ts create mode 100644 src/public/services/CreateFormService.ts rename src/public/services/{AdminFormService.ts => UpdateFormService.ts} (84%) create mode 100644 src/public/services/__tests__/AdminViewFormService.test.ts create mode 100644 src/public/services/__tests__/CreateFormService.test.ts rename src/public/services/__tests__/{AdminFormService.test.ts => UpdateFormService.test.ts} (55%) diff --git a/src/public/modules/forms/admin/components/edit-logic.client.component.js b/src/public/modules/forms/admin/components/edit-logic.client.component.js index 78f5a88e7f..0cdea8675b 100644 --- a/src/public/modules/forms/admin/components/edit-logic.client.component.js +++ b/src/public/modules/forms/admin/components/edit-logic.client.component.js @@ -1,7 +1,7 @@ 'use strict' const { LogicType } = require('../../../../../types') -const AdminFormService = require('../../../../services/AdminFormService') +const UpdateFormService = require('../../../../services/UpdateFormService') angular.module('forms').component('editLogicComponent', { templateUrl: 'modules/forms/admin/componentViews/edit-logic.client.view.html', @@ -82,7 +82,7 @@ function editLogicComponentController($uibModal, FormFields, Toastr, $q) { vm.deleteLogic = function (logicIndex) { const logicIdToDelete = vm.myform.form_logics[logicIndex]._id - $q.when(AdminFormService.deleteFormLogic(vm.myform._id, logicIdToDelete)) + $q.when(UpdateFormService.deleteFormLogic(vm.myform._id, logicIdToDelete)) .then(() => { vm.myform.form_logics.splice(logicIndex, 1) }) diff --git a/src/public/modules/forms/admin/controllers/admin-form.client.controller.js b/src/public/modules/forms/admin/controllers/admin-form.client.controller.js index 07f57002c9..51f148f7b9 100644 --- a/src/public/modules/forms/admin/controllers/admin-form.client.controller.js +++ b/src/public/modules/forms/admin/controllers/admin-form.client.controller.js @@ -3,7 +3,7 @@ const { StatusCodes } = require('http-status-codes') const get = require('lodash/get') const { LogicType } = require('../../../../../types') -const AdminFormService = require('../../../../services/AdminFormService') +const UpdateFormService = require('../../../../services/UpdateFormService') const FieldFactory = require('../../helpers/field-factory') const { UPDATE_FORM_TYPES } = require('../constants/update-form-types') @@ -191,7 +191,9 @@ function AdminFormController( case UPDATE_FORM_TYPES.CreateField: { const { body } = update return $q - .when(AdminFormService.createSingleFormField($scope.myform._id, body)) + .when( + UpdateFormService.createSingleFormField($scope.myform._id, body), + ) .then((updatedFormField) => { // !!! Convert retrieved form field objects into their class counterparts. const updatedFieldClass = @@ -210,7 +212,7 @@ function AdminFormController( const { fieldId } = update return $q .when( - AdminFormService.deleteSingleFormField($scope.myform._id, fieldId), + UpdateFormService.deleteSingleFormField($scope.myform._id, fieldId), ) .then(() => { // Success, remove deleted form field @@ -225,7 +227,7 @@ function AdminFormController( const { fieldId, body } = update return $q .when( - AdminFormService.updateSingleFormField( + UpdateFormService.updateSingleFormField( $scope.myform._id, fieldId, body, @@ -253,7 +255,7 @@ function AdminFormController( const { fieldId } = update return $q .when( - AdminFormService.duplicateSingleFormField( + UpdateFormService.duplicateSingleFormField( $scope.myform._id, fieldId, ), @@ -277,7 +279,7 @@ function AdminFormController( return $q .when( - AdminFormService.reorderSingleFormField( + UpdateFormService.reorderSingleFormField( $scope.myform._id, fieldId, newPosition, @@ -295,8 +297,10 @@ function AdminFormController( .catch(handleUpdateError) } default: - return FormApi.update({ formId: $scope.myform._id }, { form: update }) - .$promise.then((savedForm) => { + // This block should not be reached. All updateForm calls should have an update type. + return $q + .when(FormApi.updateForm($scope.myform._id, update)) + .then((savedForm) => { // Updating this form updates lastModified // and also updates myform if a formToUse is passed in $scope.myform = savedForm @@ -307,7 +311,7 @@ function AdminFormController( $scope.updateFormEndPage = (newEndPage) => { return $q - .when(AdminFormService.updateFormEndPage($scope.myform._id, newEndPage)) + .when(UpdateFormService.updateFormEndPage($scope.myform._id, newEndPage)) .then((updatedEndPage) => { $scope.myform.endPage = updatedEndPage }) @@ -317,7 +321,7 @@ function AdminFormController( $scope.updateFormStartPage = (newStartPage) => { return $q .when( - AdminFormService.updateFormStartPage($scope.myform._id, newStartPage), + UpdateFormService.updateFormStartPage($scope.myform._id, newStartPage), ) .then((updatedStartPage) => { $scope.myform.startPage = updatedStartPage @@ -333,7 +337,7 @@ function AdminFormController( $scope.updateFormSettings = (settingsToUpdate) => { return $q .when( - AdminFormService.updateFormSettings( + UpdateFormService.updateFormSettings( $scope.myform._id, settingsToUpdate, ), diff --git a/src/public/modules/forms/admin/controllers/collaborator-modal.client.controller.js b/src/public/modules/forms/admin/controllers/collaborator-modal.client.controller.js index 797c0419f4..a9ff67bcde 100644 --- a/src/public/modules/forms/admin/controllers/collaborator-modal.client.controller.js +++ b/src/public/modules/forms/admin/controllers/collaborator-modal.client.controller.js @@ -2,7 +2,7 @@ const { get } = require('lodash') const { StatusCodes } = require('http-status-codes') -const AdminFormService = require('../../../../services/AdminFormService') +const UpdateFormService = require('../../../../services/UpdateFormService') angular .module('forms') @@ -73,17 +73,14 @@ function CollaboratorModalController( return } - FormApi.transferOwner( - { formId: $scope.myform._id }, - { email: $scope.transferOwnerEmail }, - ) - .$promise.then((res) => { + $q.when(FormApi.transferOwner($scope.myform._id, $scope.transferOwnerEmail)) + .then((res) => { $scope.myform = res.form externalScope.refreshFormDataFromCollab($scope.myform) Toastr.success('Form ownership transferred. You are now an Editor.') }) .catch((err) => { - Toastr.error(err.data.message) + Toastr.error(err.response.data.message) return }) .finally(() => { @@ -92,13 +89,16 @@ function CollaboratorModalController( } /** - * Calls AdminFormService to update the permission list (collaborators) of a form + * Calls UpdateFormService to update the permission list (collaborators) of a form * @param {Array} permissionList - New permission list for the form */ $scope.updatePermissionList = (permissionList) => { return $q .when( - AdminFormService.updateCollaborators($scope.myform._id, permissionList), + UpdateFormService.updateCollaborators( + $scope.myform._id, + permissionList, + ), ) .then((updatedCollaborators) => { $scope.myform.permissionList = updatedCollaborators diff --git a/src/public/modules/forms/admin/controllers/create-form-modal.client.controller.js b/src/public/modules/forms/admin/controllers/create-form-modal.client.controller.js index 63f9ac5888..a82b2e615c 100644 --- a/src/public/modules/forms/admin/controllers/create-form-modal.client.controller.js +++ b/src/public/modules/forms/admin/controllers/create-form-modal.client.controller.js @@ -43,6 +43,7 @@ angular 'FormSgSdk', 'externalScope', 'MailTo', + '$q', CreateFormModalController, ]) @@ -61,6 +62,7 @@ function CreateFormModalController( FormSgSdk, externalScope, MailTo, + $q, ) { const vm = this @@ -255,11 +257,13 @@ function CreateFormModalController( const formMode = vm.mode switch (formMode) { case 'duplicate': { - FormFactory.generateForm( - formMode, - formParams, - FormToDuplicate._id, - ).$promise.then((data) => { + $q.when( + FormFactory.generateForm( + formMode, + formParams, + FormToDuplicate._id, + ), + ).then((data) => { vm.closeCreateModal() externalScope.onDuplicateSuccess(data) }, handleCreateFormError) @@ -267,11 +271,9 @@ function CreateFormModalController( } case 'useTemplate': { const { form } = externalScope - FormFactory.generateForm( - formMode, - formParams, - form._id, - ).$promise.then((data) => { + $q.when( + FormFactory.generateForm(formMode, formParams, form._id), + ).then((data) => { vm.closeCreateModal() vm.goToWithId('viewForm', data._id + '') GTag.examplesClickCreateNewForm(form) @@ -281,7 +283,7 @@ function CreateFormModalController( case 'createFromTemplate': { // Create new form from template selected const newForm = Object.assign({}, vm.template, formParams) - FormFactory.generateForm('create', newForm).$promise.then( + $q.when(FormFactory.generateForm('create', newForm)).then( (data) => { vm.closeCreateModal() vm.goToWithId('viewForm', data._id + '') @@ -291,7 +293,7 @@ function CreateFormModalController( break } case 'create': // Create form - FormFactory.generateForm(formMode, formParams).$promise.then( + $q.when(FormFactory.generateForm(formMode, formParams)).then( (data) => { vm.closeCreateModal() vm.goToWithId('viewForm', data._id + '') diff --git a/src/public/modules/forms/admin/controllers/delete-form-modal.client.controller.js b/src/public/modules/forms/admin/controllers/delete-form-modal.client.controller.js index 3b1b47788d..b737747ae6 100644 --- a/src/public/modules/forms/admin/controllers/delete-form-modal.client.controller.js +++ b/src/public/modules/forms/admin/controllers/delete-form-modal.client.controller.js @@ -6,10 +6,16 @@ angular '$uibModalInstance', 'externalScope', 'FormApi', + '$q', DeleteFormModalController, ]) -function DeleteFormModalController($uibModalInstance, externalScope, FormApi) { +function DeleteFormModalController( + $uibModalInstance, + externalScope, + FormApi, + $q, +) { const vm = this vm.cancel = function () { @@ -28,9 +34,7 @@ function DeleteFormModalController($uibModalInstance, externalScope, FormApi) { }`, ) } - FormApi.delete({ - formId: vm.myforms[formIndex]._id, - }).$promise.then( + $q.when(FormApi.deleteForm(vm.myforms[formIndex]._id)).then( function () { vm.myforms.splice(formIndex, 1) vm.cancel() diff --git a/src/public/modules/forms/admin/controllers/edit-logic-modal.client.controller.js b/src/public/modules/forms/admin/controllers/edit-logic-modal.client.controller.js index 19f88c5561..c6d2d5c164 100644 --- a/src/public/modules/forms/admin/controllers/edit-logic-modal.client.controller.js +++ b/src/public/modules/forms/admin/controllers/edit-logic-modal.client.controller.js @@ -3,7 +3,7 @@ const { range } = require('lodash') const { LogicType } = require('../../../../../types') const FormLogic = require('../../services/form-logic/form-logic.client.service') -const AdminFormService = require('../../../../services/AdminFormService') +const UpdateFormService = require('../../../../services/UpdateFormService') angular .module('forms') @@ -272,7 +272,7 @@ function EditLogicModalController( } vm.createNewLogic = function (newLogic) { - $q.when(AdminFormService.createFormLogic(vm.myform._id, newLogic)) + $q.when(UpdateFormService.createFormLogic(vm.myform._id, newLogic)) .then((createdLogic) => { vm.formLogics.push(createdLogic) externalScope.myform.form_logics.push(createdLogic) // update global myform @@ -290,7 +290,7 @@ function EditLogicModalController( vm.updateExistingLogic = function (logicIndex, updatedLogic) { const logicIdToUpdate = vm.formLogics[logicIndex]._id $q.when( - AdminFormService.updateFormLogic( + UpdateFormService.updateFormLogic( vm.myform._id, logicIdToUpdate, updatedLogic, diff --git a/src/public/modules/forms/admin/controllers/list-forms.client.controller.js b/src/public/modules/forms/admin/controllers/list-forms.client.controller.js index 5ad9d65bac..f9ed175d8f 100644 --- a/src/public/modules/forms/admin/controllers/list-forms.client.controller.js +++ b/src/public/modules/forms/admin/controllers/list-forms.client.controller.js @@ -16,6 +16,7 @@ angular '$timeout', '$window', 'Toastr', + '$q', ListFormsController, ]) @@ -29,6 +30,7 @@ function ListFormsController( $timeout, $window, Toastr, + $q, ) { const vm = this @@ -74,7 +76,7 @@ function ListFormsController( // Massage user email into a name turnEmailToName() - FormApi.query(function (_forms) { + $q.when(FormApi.getDashboardView()).then((_forms) => { vm.myforms = _forms }) } @@ -192,9 +194,9 @@ function ListFormsController( resolve: { FormToDuplicate: () => { // Retrieve the form so that we can populate the modal with any existing email recipients - return FormApi.preview({ - formId: vm.myforms[formIndex]._id, - }).$promise.then((res) => res.form) + return $q + .when(FormApi.previewForm(vm.myforms[formIndex]._id)) + .then((res) => res.form) }, createFormModalOptions: () => ({ mode: 'duplicate' }), externalScope: () => ({ diff --git a/src/public/modules/forms/base/directives/submit-form.directive.js b/src/public/modules/forms/base/directives/submit-form.directive.js index 1527f177c0..fd686c819e 100644 --- a/src/public/modules/forms/base/directives/submit-form.directive.js +++ b/src/public/modules/forms/base/directives/submit-form.directive.js @@ -5,7 +5,7 @@ const get = require('lodash/get') const FieldVerificationService = require('../../../../services/FieldVerificationService') const PublicFormAuthService = require('../../../../services/PublicFormAuthService') const PublicFormService = require('../../../../services/PublicFormService') -const AdminFormService = require('../../../../services/AdminFormService') +const UpdateFormService = require('../../../../services/UpdateFormService') const { getVisibleFieldIds, getLogicUnitPreventingSubmit, @@ -324,7 +324,7 @@ function submitFormDirective( const content = { responses: submissionContent.responses } const submitFn = form.isPreview - ? AdminFormService.submitEmailModeFormPreview + ? UpdateFormService.submitEmailModeFormPreview : PublicFormService.submitEmailModeForm return $q @@ -341,7 +341,7 @@ function submitFormDirective( } case responseModeEnum.ENCRYPT: { const submitFn = form.isPreview - ? AdminFormService.submitStorageModeFormPreview + ? UpdateFormService.submitStorageModeFormPreview : PublicFormService.submitStorageModeForm return $q diff --git a/src/public/modules/forms/config/forms.client.routes.js b/src/public/modules/forms/config/forms.client.routes.js index 2a58b6a757..220e2ece7f 100644 --- a/src/public/modules/forms/config/forms.client.routes.js +++ b/src/public/modules/forms/config/forms.client.routes.js @@ -22,7 +22,7 @@ angular.module('forms').config([ 'FormApi', '$transition$', function (FormApi, $transition$) { - return FormApi.getPublic($transition$.params()).$promise + return FormApi.getPublicForm($transition$.params().formId) }, ], }, @@ -37,7 +37,7 @@ angular.module('forms').config([ 'FormApi', '$transition$', function (FormApi, $transition$) { - return FormApi.preview($transition$.params()).$promise.then( + return FormApi.previewForm($transition$.params().formId).then( (FormData) => { FormData.isTemplate = true FormData.isPreview = true @@ -58,7 +58,7 @@ angular.module('forms').config([ 'FormApi', '$transition$', function (FormApi, $transition$) { - return FormApi.template($transition$.params()).$promise.then( + return FormApi.queryTemplate($transition$.params().formId).then( (FormData) => { FormData.isTemplate = true return FormData @@ -79,17 +79,15 @@ angular.module('forms').config([ // If the user is logged in, this field will contain the form data of the provided formId, // otherwise it will only contain the formId itself. FormData: [ - '$q', 'Auth', 'FormErrorService', '$stateParams', - function ($q, Auth, FormErrorService, $stateParams) { + function (Auth, FormErrorService, $stateParams) { if (!Auth.getUser()) { return $stateParams.formId } - return $q - .when(ExamplesService.getSingleExampleForm($stateParams.formId)) + return ExamplesService.getSingleExampleForm($stateParams.formId) .then(function (response) { response.form.isTemplate = true return response.form @@ -114,7 +112,7 @@ angular.module('forms').config([ 'FormApi', '$transition$', function (FormApi, $transition$) { - return FormApi.getAdmin($transition$.params()).$promise + return FormApi.getAdminForm($transition$.params().formId) }, ], }, diff --git a/src/public/modules/forms/services/form-api.client.factory.js b/src/public/modules/forms/services/form-api.client.factory.js index c7e223b54b..c26de14407 100644 --- a/src/public/modules/forms/services/form-api.client.factory.js +++ b/src/public/modules/forms/services/form-api.client.factory.js @@ -1,12 +1,16 @@ 'use strict' - const { get } = require('lodash') const Form = require('../viewmodels/Form.class') +const CreateFormService = require('../../../services/CreateFormService') +const UpdateFormService = require('../../../services/UpdateFormService') +const ExamplesService = require('../../../services/ExamplesService') +const AdminViewFormService = require('../../../services/AdminViewFormService') +const PublicFormService = require('../../../services/PublicFormService') // Forms service used for communicating with the forms REST endpoints angular .module('forms') - .factory('FormApi', ['$resource', 'FormErrorService', 'FormFields', FormApi]) + .factory('FormApi', ['FormErrorService', 'FormFields', FormApi]) // Helper function for getting formID from path starting with /:formId/. // If form ID is not found, returns an empty string. @@ -25,129 +29,170 @@ const extractFormId = (path) => { * Service for making API calls to /:formId/:accessMode endpoint, which is used * for all CRUD operations for forms. */ -function FormApi($resource, FormErrorService, FormFields) { +function FormApi(FormErrorService, FormFields) { + /** + * Handles data passed into API calls. In charge of stripping of + * MyInfo data when saving forms to the database. + * @param {*} input API service data input + * @returns Transformed input + */ + const stripMyInfo = (input) => { + FormFields.removeMyInfoFromForm(input) + return input + } + /** - * Used to generate interceptors for all API calls. These interceptors have the - * following responsibilities: - * 1) Stripping MyInfo data when saving forms to the database. - * 2) Injecting MyInfo data when forms are received from the database. - * 3) Converting plain form objects to smart Form class instances before returning + * Handles data returned from API calls. Responsibilities include: + * 1) Injecting MyInfo data when forms are received from the database. + * 2) Converting plain form objects to smart Form class instances before returning * them to controllers. - * 4) Redirecting users using FormErrorService if redirectOnError is true. - * @param {boolean} redirectOnError Whether to use FormErrorService to redirect - * to errorTargetState when an error is encountered. - * @param {string} [errorTargetState] If redirectOnError is true, this is the - * redirect state which will be passed to FormErrorService + * @param {*} data Data returned from API call + * @returns Transformed data */ - const getInterceptor = (redirectOnError, errorTargetState) => { - const interceptor = { - request: (config) => { - if (get(config, 'data.form')) { - FormFields.removeMyInfoFromForm(config.data.form) - } - return config - }, - response: (response) => { - // The backend returns different shapes for different request types. For GET - // requests, it returns a $resource instance with a form attribute containing - // all the form data; for PUT requests, it returns a $resource instance with - // all the form data at the top level. We need to ensure that the postprocessing - // is done in both cases. - if (get(response, 'resource.form.form_fields')) { - FormFields.injectMyInfoIntoForm(response.resource.form) - // Convert plain form object to smart Form instance - response.resource.form = new Form(response.resource.form) - } else if (get(response, 'resource.form_fields')) { - FormFields.injectMyInfoIntoForm(response.resource) - response.resource = new Form(response.resource) - } - return response.resource - }, + const injectMyInfo = (data) => { + // The backend returns different shapes for different request types. For GET + // requests, it returns an object with a form attribute containing + // all the form data; for PUT requests, it returns an object with + // all the form data at the top level. We need to ensure that the postprocessing + // is done in both cases. + if (get(data, 'form.form_fields')) { + FormFields.injectMyInfoIntoForm(data.form) + // Convert plain form object to smart Form instance + data.form = new Form(data.form) + } else if (get(data, 'form_fields')) { + FormFields.injectMyInfoIntoForm(data) + data = new Form(data) } + return data + } + + /** + * Handles error returned from API calls. In charge of redirecting users using FormErrorService + * if redirectOnError is true. + * @param {Error} err Error returned from API calls + * @param {{redirectOnError: boolean, errorTargetState: string }} errorParams redirectOnError specifies + * whether to use FormErrorService to redirect to errorTargetState when an error is encountered. + * If redirectOnError is true, this is the redirect state which will be passed to FormErrorService + */ + const handleError = (err, errorParams) => { + const { redirectOnError, errorTargetState } = errorParams if (redirectOnError) { - interceptor.responseError = (response) => { - return FormErrorService.redirect({ - response, - targetState: errorTargetState, - targetFormId: extractFormId(get(response, 'config.url')), - }) - } + FormErrorService.redirect({ + response: err.response, + targetState: errorTargetState, + targetFormId: extractFormId(get(err.response, 'config.url')), + }) + } else { + throw err // just pass error on } - return interceptor } - // accessMode is either adminForm or publicForm - let resourceUrl = '/:formId/:accessMode' - const V3_PUBLICFORM_URL = '/api/v3/forms/:formId' + const generateService = ( + service, + handleInput, + handleResponse, + errorParams, + ...inputs + ) => { + const input = handleInput + ? inputs.map((input) => handleInput(input)) + : inputs + return service(...input) + .then((data) => { + return handleResponse ? handleResponse(data) : data + }) + .catch((err) => handleError(err, errorParams)) + } - return $resource( - resourceUrl, - // default to admin for access mode, since that applies to most methods - { accessMode: 'adminform' }, - { - query: { - url: '/api/v3/admin/forms', - method: 'GET', - isArray: true, - headers: { 'If-Modified-Since': '0' }, - interceptor: getInterceptor(true, 'listForms'), - // disable IE ajax request caching (so new forms show on dashboard) - }, - getAdmin: { - method: 'GET', - headers: { 'If-Modified-Since': '0' }, - interceptor: getInterceptor(true, 'viewForm'), - // disable IE ajax request caching (so new fields show on build panel) - }, - getPublic: { - url: V3_PUBLICFORM_URL, - method: 'GET', - // disable IE ajax request caching (so new fields show on build panel) - headers: { 'If-Modified-Since': '0' }, - interceptor: getInterceptor(true), - }, - update: { - method: 'PUT', - interceptor: getInterceptor(false), - }, - save: { - url: '/api/v3/admin/forms/:formId/duplicate', - method: 'POST', - interceptor: getInterceptor(false), - }, - delete: { - url: '/api/v3/admin/forms/:formId', - method: 'DELETE', - interceptor: getInterceptor(false), - }, - // create is called without formId, so the endpoint is just /adminform - create: { - url: '/api/v3/admin/forms', - method: 'POST', - interceptor: getInterceptor(true, 'listForms'), - }, - // Used for viewing templates with use-template or examples listing. Any logged in officer is authorized. - template: { - url: resourceUrl + '/template', - method: 'GET', - interceptor: getInterceptor(true, 'templateForm'), - }, - // Used for previewing the form from the form admin page. Must be a viewer, collaborator or admin. - preview: { - url: '/api/v3/admin/forms/:formId/preview', - method: 'GET', - interceptor: getInterceptor(true, 'previewForm'), - }, - useTemplate: { - url: resourceUrl + '/copy', - method: 'POST', - interceptor: getInterceptor(true, 'useTemplate'), - }, - transferOwner: { - url: '/api/v3/admin/forms/:formId/collaborators/transfer-owner', - method: 'POST', - interceptor: getInterceptor(false), - }, - }, - ) + return { + getDashboardView: () => + generateService(AdminViewFormService.getDashboardView, null, null, { + redirectOnError: true, + errorTargetState: 'listForms', + }), + getAdminForm: (formId) => + generateService( + AdminViewFormService.getAdminFormView, + null, + injectMyInfo, + { redirectOnError: true, errorTargetState: 'viewForm' }, + formId, + ), + getPublicForm: (formId) => + generateService( + PublicFormService.getPublicFormView, + null, + injectMyInfo, + { redirectOnError: true }, + formId, + ), + updateForm: (formId, update) => + generateService( + UpdateFormService.updateForm, + stripMyInfo, + injectMyInfo, + { redirectOnError: false }, + formId, + update, + ), + duplicateForm: (formId, duplicateFormBody) => + generateService( + CreateFormService.duplicateForm, + null, + injectMyInfo, + { redirectOnError: false }, + formId, + duplicateFormBody, + ), + deleteForm: (formId) => + generateService( + UpdateFormService.deleteForm, + null, + null, + { redirectOnError: false }, + formId, + ), + createForm: (newForm) => + generateService( + CreateFormService.createForm, + stripMyInfo, + injectMyInfo, + { redirectOnError: true, errorTargetState: 'listForms' }, + newForm, + ), + queryTemplate: (formId) => + generateService( + ExamplesService.queryTemplate, + null, + injectMyInfo, + { redirectOnError: true, errorTargetState: 'templateForm' }, + formId, + ), + previewForm: (formId) => + generateService( + AdminViewFormService.previewForm, + null, + injectMyInfo, + { redirectOnError: true, errorTargetState: 'previewForm' }, + formId, + ), + useTemplate: (formId, overrideParams) => + generateService( + ExamplesService.useTemplate, + null, + null, + { redirectOnError: true, errorTargetState: 'useTemplate' }, + formId, + overrideParams, + ), + transferOwner: (formId, newOwner) => + generateService( + UpdateFormService.transferOwner, + null, + injectMyInfo, + { redirectOnError: false }, + formId, + newOwner, + ), + } } diff --git a/src/public/modules/forms/services/form-factory.client.service.js b/src/public/modules/forms/services/form-factory.client.service.js index 235e9cf345..79ba86aae2 100644 --- a/src/public/modules/forms/services/form-factory.client.service.js +++ b/src/public/modules/forms/services/form-factory.client.service.js @@ -15,11 +15,11 @@ function FormFactory(FormApi) { function generateForm(mode, params, formId) { switch (mode) { case 'create': - return FormApi.create({}, { form: params }) + return FormApi.createForm(params) case 'duplicate': - return FormApi.save({ formId }, params) + return FormApi.duplicateForm(formId, params) case 'useTemplate': - return FormApi.useTemplate({ formId }, params) + return FormApi.useTemplate(formId, params) default: throw new Error('Unsupported mode of form generation.') } diff --git a/src/public/modules/users/controllers/examples-card.client.directive.js b/src/public/modules/users/controllers/examples-card.client.directive.js index 2331969a89..d0f9faf37a 100644 --- a/src/public/modules/users/controllers/examples-card.client.directive.js +++ b/src/public/modules/users/controllers/examples-card.client.directive.js @@ -23,6 +23,7 @@ function examplesCard() { 'Auth', '$location', 'Toastr', + '$q', examplesCardController, ], } @@ -38,6 +39,7 @@ function examplesCardController( Auth, $location, Toastr, + $q, ) { $scope.user = Auth.getUser() @@ -98,9 +100,9 @@ function examplesCardController( controllerAs: 'vm', resolve: { FormToDuplicate: () => { - return FormApi.template({ - formId: $scope.form._id, - }).$promise.then((res) => res.form) + return $q + .when(FormApi.queryTemplate($scope.form._id)) + .then((res) => res.form) }, createFormModalOptions: () => ({ mode: 'useTemplate' }), externalScope: () => ({ diff --git a/src/public/services/AdminSubmissionsService.ts b/src/public/services/AdminSubmissionsService.ts index 2139651ab8..29ef413308 100644 --- a/src/public/services/AdminSubmissionsService.ts +++ b/src/public/services/AdminSubmissionsService.ts @@ -8,7 +8,7 @@ import { SubmissionResponseQueryDto, } from 'src/types/api' -import { ADMIN_FORM_ENDPOINT } from './AdminFormService' +import { ADMIN_FORM_ENDPOINT } from './UpdateFormService' /** * Counts the number of submissions for a given form diff --git a/src/public/services/AdminViewFormService.ts b/src/public/services/AdminViewFormService.ts new file mode 100644 index 0000000000..35f8abefb9 --- /dev/null +++ b/src/public/services/AdminViewFormService.ts @@ -0,0 +1,43 @@ +import axios from 'axios' + +import { FormViewDto } from '../..//types/api' +import { FormMetaView, PublicForm } from '../../types' + +// endpoint exported for testing +export const ADMIN_FORM_ENDPOINT = '/api/v3/admin/forms' + +/** + * Gets metadata for all forms in dashboard view i.e. forms which user + * owns or collaborates on + * @returns Metadata required for forms on dashboard view + */ +export const getDashboardView = async (): Promise => { + return axios + .get(`${ADMIN_FORM_ENDPOINT}`) + .then(({ data }) => data) +} + +/** + * Gets admin view of form. + * @param formId formId of form in question + * @returns Admin view of form + */ +export const getAdminFormView = async ( + formId: string, +): Promise => { + return axios.get(`/${formId}/adminform`).then(({ data }) => data) +} + +/** + * Gets the public view of a form. Used for previewing the form from the form admin page. + * Must be a viewer, collaborator or admin. + * @param formId formId of form in question + * @returns Public view of a form + */ +export const previewForm = async ( + formId: string, +): Promise<{ form: PublicForm }> => { + return axios + .get<{ form: PublicForm }>(`${ADMIN_FORM_ENDPOINT}/${formId}/preview`) + .then(({ data }) => data) +} diff --git a/src/public/services/CreateFormService.ts b/src/public/services/CreateFormService.ts new file mode 100644 index 0000000000..13482e85d5 --- /dev/null +++ b/src/public/services/CreateFormService.ts @@ -0,0 +1,38 @@ +import axios from 'axios' + +import { FormMetaView, IForm, IFormSchema } from '../../types' +import { DuplicateFormBody } from '../../types/api' + +// endpoints exported for testing +export const ADMIN_FORM_ENDPOINT = '/api/v3/admin/forms' + +/** + * Duplicates the form + * @param formId formId of form to be duplicated + * @param duplicateFormBody Title, response mode and relevant information for new form + * @returns Metadata of duplicated form for dashboard view + */ +export const duplicateForm = async ( + formId: string, + duplicateFormBody: DuplicateFormBody, +): Promise => { + return axios + .post( + `${ADMIN_FORM_ENDPOINT}/${formId}/duplicate`, + duplicateFormBody, + ) + .then(({ data }) => data) +} + +/** + * Creates a new form. This function is called without formId, so the endpoint is just /adminform. + * @param newForm Form fields to newly created form + * @returns Newly created form. + */ +export const createForm = async ( + newForm: Omit, +): Promise => { + return axios + .post(`${ADMIN_FORM_ENDPOINT}`, { form: newForm }) + .then(({ data }) => data) +} diff --git a/src/public/services/ExamplesService.ts b/src/public/services/ExamplesService.ts index 0575cdc711..46e26927c4 100644 --- a/src/public/services/ExamplesService.ts +++ b/src/public/services/ExamplesService.ts @@ -1,6 +1,8 @@ import axios from 'axios' +import { FormMetaView, PublicForm } from '../../types' import { + DuplicateFormBody, ExampleFormsQueryDto, ExampleFormsResult, ExampleSingleFormResult, @@ -20,7 +22,6 @@ export const getExampleForms = ( return axios .get(EXAMPLES_ENDPOINT, { params: exampleFormsSearchParams, - // disable IE ajax request caching (so search requests don't get cached) headers: { 'If-Modified-Since': '0' }, }) .then(({ data }) => data) @@ -35,8 +36,34 @@ export const getSingleExampleForm = ( ): Promise => { return axios .get(`${EXAMPLES_ENDPOINT}/${formId}`, { - // disable IE ajax request caching (so search requests don't get cached) headers: { 'If-Modified-Since': '0' }, }) .then(({ data }) => data) } + +/** + * Used to create a new form from an existing template. + * @param formId formId of template to base the new form on + * @returns Metadata for newly created form in dashboard view + */ +export const useTemplate = async ( + formId: string, + overrideParams: DuplicateFormBody, +): Promise => { + return axios + .post(`${formId}/adminform/copy`, overrideParams) + .then(({ data }) => data) +} + +/** + * Queries templates with use-template or examples listings. Any logged in officer is authorized. + * @param formId formId of template in question + * @returns Public view of a template + */ +export const queryTemplate = async ( + formId: string, +): Promise<{ form: PublicForm }> => { + return axios + .get<{ form: PublicForm }>(`${formId}/adminform/template`) + .then(({ data }) => data) +} diff --git a/src/public/services/PublicFormService.ts b/src/public/services/PublicFormService.ts index cba1f12c96..ea2a59f69a 100644 --- a/src/public/services/PublicFormService.ts +++ b/src/public/services/PublicFormService.ts @@ -3,6 +3,7 @@ import axios from 'axios' import { EmailSubmissionDto, EncryptSubmissionDto, + PublicFormViewDto, SubmissionResponseDto, } from '../../types/api' import { createEmailSubmissionFormData } from '../utils/submission' @@ -76,3 +77,17 @@ export const submitStorageModeForm = async ({ ) .then(({ data }) => data) } + +/** + * Gets public view of form, along with any + * identify information obtained from Singpass/Corppass/MyInfo. + * @param formId FormId of form in question + * @returns Public view of form, with additional identify information + */ +export const getPublicFormView = async ( + formId: string, +): Promise => { + return axios + .get(`${PUBLIC_FORMS_ENDPOINT}/${formId}`) + .then(({ data }) => data) +} diff --git a/src/public/services/AdminFormService.ts b/src/public/services/UpdateFormService.ts similarity index 84% rename from src/public/services/AdminFormService.ts rename to src/public/services/UpdateFormService.ts index b6a7063d56..3e7a04f96d 100644 --- a/src/public/services/AdminFormService.ts +++ b/src/public/services/UpdateFormService.ts @@ -1,6 +1,6 @@ import axios from 'axios' -import { FormSettings, LogicDto } from '../../types' +import { FormSettings, IPopulatedForm, LogicDto } from '../../types' import { EmailSubmissionDto, EncryptSubmissionDto, @@ -8,6 +8,8 @@ import { FieldCreateDto, FieldUpdateDto, FormFieldDto, + FormUpdateParams, + FormViewDto, PermissionsUpdateDto, SettingsUpdateDto, StartPageUpdateDto, @@ -255,3 +257,47 @@ export const submitStorageModeFormPreview = async ({ ) .then(({ data }) => data) } + +/** + * Deletes the form with the corresponding formId + * @param formId formId of form to delete + */ +export const deleteForm = async ( + formId: string, +): Promise<{ message: string }> => { + return axios + .delete(`${ADMIN_FORM_ENDPOINT}/${formId}`) + .then(({ data }) => data) +} + +/** + * Updates FormUpdateParams attribute(s) in the corresponding form. + * @deprecated This function should no longer be called + * @param formId formId of form in question + * @returns Updated form + */ +export const updateForm = async ( + formId: string, + update: FormUpdateParams, +): Promise => { + return axios + .put(`${formId}/adminform`, { form: update }) + .then(({ data }) => data) +} + +/** + * Transfers ownership of form to another user with the given email. + * @param newOwnerEmail Email of new owner + * @returns Updated form with new ownership. + */ +export const transferOwner = async ( + formId: string, + newOwnerEmail: string, +): Promise => { + return axios + .post( + `${ADMIN_FORM_ENDPOINT}/${formId}/collaborators/transfer-owner`, + { email: newOwnerEmail }, + ) + .then(({ data }) => data) +} diff --git a/src/public/services/__tests__/AdminSubmissionsService.test.ts b/src/public/services/__tests__/AdminSubmissionsService.test.ts index 4647b848a7..cd98a7883c 100644 --- a/src/public/services/__tests__/AdminSubmissionsService.test.ts +++ b/src/public/services/__tests__/AdminSubmissionsService.test.ts @@ -3,13 +3,13 @@ import { mocked } from 'ts-jest/utils' import { SubmissionMetadataList } from 'src/types' -import { ADMIN_FORM_ENDPOINT } from '../AdminFormService' import { countFormSubmissions, getEncryptedResponse, getSubmissionMetadataById, getSubmissionsMetadataByPage, } from '../AdminSubmissionsService' +import { ADMIN_FORM_ENDPOINT } from '../UpdateFormService' jest.mock('axios') const MockAxios = mocked(axios, true) diff --git a/src/public/services/__tests__/AdminViewFormService.test.ts b/src/public/services/__tests__/AdminViewFormService.test.ts new file mode 100644 index 0000000000..1390ee746c --- /dev/null +++ b/src/public/services/__tests__/AdminViewFormService.test.ts @@ -0,0 +1,147 @@ +import MockAxios from 'jest-mock-axios' + +import { IPopulatedUser } from 'src/types' +import { + FormMetaView, + IPopulatedForm, + PublicForm, + ResponseMode, +} from 'src/types/form' + +import { + ADMIN_FORM_ENDPOINT, + getAdminFormView, + getDashboardView, + previewForm, +} from '../AdminViewFormService' + +jest.mock('axios', () => MockAxios) + +const MOCK_USER = { + _id: 'mock-user-id', +} as IPopulatedUser + +describe('AdminViewFormService', () => { + afterEach(() => MockAxios.reset()) + describe('getDashboardView', () => { + it('should successfully return all available forms if GET request succeeds', async () => { + // Arrange + const expected: FormMetaView[] = [ + { + title: 'title', + lastModified: new Date(), + _id: 'mock-form-id', + responseMode: ResponseMode.Email, + admin: MOCK_USER, + }, + ] + MockAxios.get.mockResolvedValueOnce({ data: expected }) + // Act + const actual = await getDashboardView() + + // Assert + expect(actual).toEqual(expected) + expect(MockAxios.get).toHaveBeenCalledWith(`${ADMIN_FORM_ENDPOINT}`) + }) + + it('should successfully return empty array if GET request succeeds and there are no forms', async () => { + // Arrange + const expected: FormMetaView[] = [] + MockAxios.get.mockResolvedValueOnce({ data: expected }) + + // Act + const actual = await getDashboardView() + + // Assert + expect(actual).toEqual(expected) + expect(MockAxios.get).toHaveBeenCalledWith(`${ADMIN_FORM_ENDPOINT}`) + }) + + it('should reject with error message if GET request fails', async () => { + // Arrange + const expected = new Error('error') + MockAxios.get.mockRejectedValueOnce(expected) + + // Act + const actualPromise = getDashboardView() + + //Assert + await expect(actualPromise).rejects.toEqual(expected) + expect(MockAxios.get).toHaveBeenCalledWith(`${ADMIN_FORM_ENDPOINT}`) + }) + }) + + describe('getAdminFormView', () => { + it('should return admin form if GET request succeeds', async () => { + // Arrange + const expected = _generateMockFullForm() + MockAxios.get.mockResolvedValueOnce({ data: expected }) + + // Act + const actual = await getAdminFormView(expected._id) + + // Assert + expect(actual).toEqual(expected) + expect(MockAxios.get).toHaveBeenCalledWith(`/${expected._id}/adminform`) + }) + + it('should reject with error message when GET request fails', async () => { + // Arrange + const expected = new Error('error') + const MOCK_FORM = _generateMockFullForm() + MockAxios.get.mockRejectedValueOnce(expected) + + // Act + const actualPromise = getAdminFormView(MOCK_FORM._id) + + // Assert + await expect(actualPromise).rejects.toEqual(expected) + expect(MockAxios.get).toHaveBeenCalledWith(`/${MOCK_FORM._id}/adminform`) + }) + }) + + describe('previewForm', () => { + it('should return public form if GET request succeeds', async () => { + // Arrange + const MOCK_FORM_ID = 'mock-form-id' + const expected = { + _id: MOCK_FORM_ID, + title: 'mock preview title', + admin: MOCK_USER, + } as unknown as PublicForm + MockAxios.get.mockResolvedValueOnce({ data: expected }) + + // Act + const actual = await previewForm(MOCK_FORM_ID) + + // Assert + expect(actual).toEqual(expected) + expect(MockAxios.get).toHaveBeenCalledWith( + `${ADMIN_FORM_ENDPOINT}/${MOCK_FORM_ID}/preview`, + ) + }) + + it('should reject with error message if GET request fails', async () => { + // Arrange + const expected = new Error('error') + const MOCK_FORM_ID = 'mock-form-id' + MockAxios.get.mockRejectedValueOnce(expected) + + // Act + const actualPromise = previewForm(MOCK_FORM_ID) + + // Assert + await expect(actualPromise).rejects.toEqual(expected) + expect(MockAxios.get).toHaveBeenCalledWith( + `${ADMIN_FORM_ENDPOINT}/${MOCK_FORM_ID}/preview`, + ) + }) + }) +}) + +// Utils +const _generateMockFullForm = (): IPopulatedForm => { + return { + _id: 'mock-form-id', + } as IPopulatedForm +} diff --git a/src/public/services/__tests__/CreateFormService.test.ts b/src/public/services/__tests__/CreateFormService.test.ts new file mode 100644 index 0000000000..56a0380615 --- /dev/null +++ b/src/public/services/__tests__/CreateFormService.test.ts @@ -0,0 +1,114 @@ +import { ObjectId } from 'bson' +import MockAxios from 'jest-mock-axios' + +import { IPopulatedUser, IYesNoFieldSchema } from '../../../types' +import { DuplicateFormBody } from '../../../types/api' +import { ResponseMode } from '../../../types/form' +import { + ADMIN_FORM_ENDPOINT, + createForm, + duplicateForm, +} from '../CreateFormService' + +jest.mock('axios', () => MockAxios) + +const MOCK_USER = { + _id: new ObjectId(), +} as IPopulatedUser + +describe('CreateFormService', () => { + afterEach(() => MockAxios.reset()) + describe('duplicateForm', () => { + it('should return saved form if POST request succeeds', async () => { + // Arrange + const expected = { + title: 'title', + lastModified: new Date(), + _id: new ObjectId().toHexString(), + responseMode: ResponseMode.Email, + admin: MOCK_USER, + } + const MOCK_FORM_ID = expected._id + const MOCK_DUPLICATE_FORM_BODY = _generateDuplicateFormBody() + MockAxios.post.mockResolvedValueOnce({ data: expected }) + + // Act + const actual = await duplicateForm(MOCK_FORM_ID, MOCK_DUPLICATE_FORM_BODY) + + // Assert + expect(actual).toEqual(expected) + expect(MockAxios.post).toHaveBeenCalledWith( + `${ADMIN_FORM_ENDPOINT}/${MOCK_FORM_ID}/duplicate`, + MOCK_DUPLICATE_FORM_BODY, + ) + }) + + it('should reject with error message if POST request fails', async () => { + // Arrange + const expected = new Error('error') + const MOCK_FORM_ID = new ObjectId().toHexString() + const MOCK_DUPLICATE_FORM_BODY = _generateDuplicateFormBody() + MockAxios.post.mockRejectedValueOnce(expected) + + // Act + const actualPromise = duplicateForm( + MOCK_FORM_ID, + MOCK_DUPLICATE_FORM_BODY, + ) + + // Assert + await expect(actualPromise).rejects.toEqual(expected) + expect(MockAxios.post).toHaveBeenCalledWith( + `${ADMIN_FORM_ENDPOINT}/${MOCK_FORM_ID}/duplicate`, + MOCK_DUPLICATE_FORM_BODY, + ) + }) + }) + + describe('createForm', () => { + it('should return created form if POST request succeeds', async () => { + // Arrange + const expected = { form_fields: [{} as IYesNoFieldSchema] } + const MOCK_FORM_PARAMS = { + title: 'title', + responseMode: ResponseMode.Email, + } + MockAxios.post.mockResolvedValueOnce({ data: expected }) + + // Act + const actual = await createForm(MOCK_FORM_PARAMS) + + // Assert + expect(actual).toEqual(expected) + expect(MockAxios.post).toHaveBeenCalledWith(`${ADMIN_FORM_ENDPOINT}`, { + form: MOCK_FORM_PARAMS, + }) + }) + + it('should reject with error message if POST request fails', async () => { + // Arrange + const expected = new Error('error') + const MOCK_FORM_PARAMS = { + title: 'title', + responseMode: ResponseMode.Email, + } + MockAxios.post.mockRejectedValueOnce(expected) + // Act + const actualPromise = createForm(MOCK_FORM_PARAMS) + + // Assert + await expect(actualPromise).rejects.toEqual(expected) + expect(MockAxios.post).toHaveBeenCalledWith(`${ADMIN_FORM_ENDPOINT}`, { + form: MOCK_FORM_PARAMS, + }) + }) + }) +}) + +const _generateDuplicateFormBody = (): DuplicateFormBody => { + return { + title: 'title', + responseMode: ResponseMode.Email, + emails: 'test@example.com', + } as DuplicateFormBody +} diff --git a/src/public/services/__tests__/ExamplesService.test.ts b/src/public/services/__tests__/ExamplesService.test.ts index b50f0edcc5..a88815cbe5 100644 --- a/src/public/services/__tests__/ExamplesService.test.ts +++ b/src/public/services/__tests__/ExamplesService.test.ts @@ -1,5 +1,7 @@ import MockAxios from 'jest-mock-axios' +import { IPopulatedUser, PublicForm, ResponseMode } from '../../../types' +import { DuplicateFormBody } from '../../../types/api' import * as ExamplesService from '../ExamplesService' jest.mock('axios', () => MockAxios) @@ -88,4 +90,107 @@ describe('ExamplesService', () => { }) }) }) + + describe('useTemplate', () => { + it('should return template if POST request succeeds', async () => { + // Arrange + const MOCK_USER = { + _id: 'mock-user-id', + } as IPopulatedUser + const MOCK_FORM_ID = 'mock-form-id' + const expected = { + title: 'title', + lastModified: new Date(), + _id: MOCK_FORM_ID, + responseMode: ResponseMode.Email, + admin: MOCK_USER, + } + const MOCK_DUPLICATE_FORM_BODY = _generateDuplicateFormBody() + MockAxios.post.mockResolvedValueOnce({ data: expected }) + + // Act + const actual = await ExamplesService.useTemplate( + MOCK_FORM_ID, + MOCK_DUPLICATE_FORM_BODY, + ) + + // Assert + expect(actual).toEqual(expected) + expect(MockAxios.post).toHaveBeenCalledWith( + `${MOCK_FORM_ID}/adminform/copy`, + MOCK_DUPLICATE_FORM_BODY, + ) + }) + + it('should reject with error message if POST request fails', async () => { + // Arrange + const expected = new Error('error') + const MOCK_FORM_ID = 'mock-form-id' + const MOCK_DUPLICATE_FORM_BODY = _generateDuplicateFormBody() + MockAxios.post.mockRejectedValueOnce(expected) + + // Act + const actualPromise = ExamplesService.useTemplate( + MOCK_FORM_ID, + MOCK_DUPLICATE_FORM_BODY, + ) + + // Assert + await expect(actualPromise).rejects.toEqual(expected) + expect(MockAxios.post).toHaveBeenCalledWith( + `${MOCK_FORM_ID}/adminform/copy`, + MOCK_DUPLICATE_FORM_BODY, + ) + }) + }) + + describe('queryTemplate', () => { + it('should return template if GET request succeeds', async () => { + // Arrange + const MOCK_USER = { + _id: 'mock-user-id', + } as IPopulatedUser + const MOCK_FORM_ID = 'mock-form-id' + const expected = { + _id: MOCK_FORM_ID, + title: 'mock preview title', + admin: MOCK_USER, + } as unknown as PublicForm + MockAxios.get.mockResolvedValueOnce({ data: expected }) + + // Act + const actual = await ExamplesService.queryTemplate(MOCK_FORM_ID) + + // Assert + expect(actual).toEqual(expected) + expect(MockAxios.get).toHaveBeenCalledWith( + `${MOCK_FORM_ID}/adminform/template`, + ) + }) + + it('should reject with error message if GET request fails', async () => { + // Arrange + const expected = new Error('error') + const MOCK_FORM_ID = 'mock-form-id' + MockAxios.get.mockRejectedValueOnce(expected) + + // Act + const actualPromise = ExamplesService.queryTemplate(MOCK_FORM_ID) + + // Assert + await expect(actualPromise).rejects.toEqual(expected) + expect(MockAxios.get).toHaveBeenCalledWith( + `${MOCK_FORM_ID}/adminform/template`, + ) + }) + }) }) + +// Utils +const _generateDuplicateFormBody = (): DuplicateFormBody => { + return { + title: 'title', + responseMode: ResponseMode.Email, + emails: 'test@example.com', + } as DuplicateFormBody +} diff --git a/src/public/services/__tests__/PublicFormService.test.ts b/src/public/services/__tests__/PublicFormService.test.ts index 559ebd30a2..6d24283735 100644 --- a/src/public/services/__tests__/PublicFormService.test.ts +++ b/src/public/services/__tests__/PublicFormService.test.ts @@ -10,6 +10,7 @@ import { import * as SubmissionUtil from '../../utils/submission' import { + getPublicFormView, submitEmailModeForm, submitStorageModeForm, } from '../PublicFormService' @@ -155,4 +156,43 @@ describe('PublicFormService', () => { ) }) }) + + describe('getPublicFormView', () => { + it('should return public form if GET request succeeds', async () => { + // Arrange + const MOCK_FORM_ID = 'mock-form-id' + const expected = { + form: { _id: MOCK_FORM_ID, form_fields: [] }, + spcpSession: { username: 'username' }, + isIntranetUser: false, + myInfoError: true, + } + MockAxios.get.mockResolvedValueOnce({ data: expected }) + + // Act + const actual = await getPublicFormView(MOCK_FORM_ID) + + // Assert + expect(actual).toEqual(expected) + expect(MockAxios.get).toHaveBeenCalledWith( + `/api/v3/forms/${MOCK_FORM_ID}`, + ) + }) + + it('should reject with error message if GET request fails', async () => { + // Arrange + const MOCK_FORM_ID = 'mock-form-id' + const expected = new Error('error') + MockAxios.get.mockRejectedValueOnce(expected) + + // Act + const actualPromise = getPublicFormView(MOCK_FORM_ID) + + // Assert + await expect(actualPromise).rejects.toEqual(expected) + expect(MockAxios.get).toHaveBeenCalledWith( + `/api/v3/forms/${MOCK_FORM_ID}`, + ) + }) + }) }) diff --git a/src/public/services/__tests__/AdminFormService.test.ts b/src/public/services/__tests__/UpdateFormService.test.ts similarity index 55% rename from src/public/services/__tests__/AdminFormService.test.ts rename to src/public/services/__tests__/UpdateFormService.test.ts index f8e80438fb..2e32f8ef40 100644 --- a/src/public/services/__tests__/AdminFormService.test.ts +++ b/src/public/services/__tests__/UpdateFormService.test.ts @@ -1,22 +1,29 @@ /* eslint-disable @typescript-eslint/ban-ts-comment */ +import { ObjectId } from 'bson' +import { StatusCodes } from 'http-status-codes' import MockAxios from 'jest-mock-axios' -import { BasicField } from 'src/types' import { EmailSubmissionDto, EncryptSubmissionDto, SubmissionResponseDto, } from 'src/types/api' +import { BasicField, IPopulatedForm, IYesNoFieldSchema } from '../../../types' +import { FormUpdateParams } from '../../../types/api' import * as SubmissionUtil from '../../utils/submission' import { + ADMIN_FORM_ENDPOINT, + deleteForm, submitEmailModeFormPreview, submitStorageModeFormPreview, -} from '../AdminFormService' + transferOwner, + updateForm, +} from '../UpdateFormService' jest.mock('axios', () => MockAxios) -describe('AdminFormService', () => { +describe('UpdateFormService', () => { describe('submitEmailModeFormPreview', () => { const MOCK_FORM_ID = 'mock–form-id' const MOCK_RESPONSE: SubmissionResponseDto = { @@ -155,4 +162,125 @@ describe('AdminFormService', () => { ) }) }) + + describe('deleteForm', () => { + it('should successfully call delete endpoint', async () => { + // Arrange + const MOCK_FORM_ID = new ObjectId().toHexString() + MockAxios.delete.mockResolvedValueOnce({ + status: StatusCodes.OK, + data: { message: 'Form has been archived' }, + }) + + // Act + await deleteForm(MOCK_FORM_ID) + + // Assert + expect(MockAxios.delete).toHaveBeenCalledWith( + `${ADMIN_FORM_ENDPOINT}/${MOCK_FORM_ID}`, + ) + }) + + it('should reject with error message if DELETE request fails', async () => { + // Arrange + const expected = new Error('error') + const MOCK_FORM_ID = new ObjectId().toHexString() + MockAxios.delete.mockRejectedValueOnce(expected) + + // Act + const actualPromise = deleteForm(MOCK_FORM_ID) + + await expect(actualPromise).rejects.toEqual(expected) + // Assert + expect(MockAxios.delete).toHaveBeenCalledWith( + `${ADMIN_FORM_ENDPOINT}/${MOCK_FORM_ID}`, + ) + }) + }) + + describe('updateForm', () => { + it('should return updated form if PUT request succeeds', async () => { + // Arrange + const expected = [{} as IYesNoFieldSchema] + const MOCK_FORM_ID = new ObjectId().toHexString() + const update = { + editFormField: { + action: { name: 'REORDER' }, + field: expected[0], + }, + } as FormUpdateParams + MockAxios.put.mockResolvedValueOnce({ data: expected }) + + // Act + const actual = await updateForm(MOCK_FORM_ID, update) + + // Assert + expect(actual).toEqual(expected) + expect(MockAxios.put).toHaveBeenCalledWith(`${MOCK_FORM_ID}/adminform`, { + form: update, + }) + }) + + it('should reject with error message if PUT request fails', async () => { + // Arrange + const expected = new Error('error') + const MOCK_FORM_ID = new ObjectId().toHexString() + const update = { + editFormField: { + action: { name: 'REORDER' }, + field: {} as IYesNoFieldSchema, + }, + } as FormUpdateParams + MockAxios.put.mockRejectedValueOnce(expected) + + // Act + const actualPromise = updateForm(MOCK_FORM_ID, update) + + // Assert + await expect(actualPromise).rejects.toEqual(expected) + expect(MockAxios.put).toHaveBeenCalledWith(`${MOCK_FORM_ID}/adminform`, { + form: update, + }) + }) + }) + + describe('transferOwner', () => { + it('should return updated form if POST request succeeds', async () => { + // Arrange + const MOCK_FORM_ID = 'mock-form-id' + const expected = { + _id: MOCK_FORM_ID, + } as IPopulatedForm + const MOCK_NEW_OWNER = 'test@open.gov.sg' + MockAxios.post.mockResolvedValueOnce({ data: expected }) + + // Act + const actual = await transferOwner(MOCK_FORM_ID, MOCK_NEW_OWNER) + + // Assert + expect(actual).toEqual(expected) + expect(MockAxios.post).toHaveBeenCalledWith( + `${ADMIN_FORM_ENDPOINT}/${MOCK_FORM_ID}/collaborators/transfer-owner`, + { email: MOCK_NEW_OWNER }, + ) + }) + + it('should reject with error message if POST request fails', async () => { + // Arrange + const expected = new Error('error') + const MOCK_FORM_ID = 'mock-form-id' + const MOCK_NEW_OWNER = 'test@open.gov.sg' + MockAxios.post.mockRejectedValueOnce(expected) + + // Act + const actualPromise = transferOwner(MOCK_FORM_ID, MOCK_NEW_OWNER) + + // Assert + await expect(actualPromise).rejects.toEqual(expected) + expect(MockAxios.post).toHaveBeenCalledWith( + `${ADMIN_FORM_ENDPOINT}/${MOCK_FORM_ID}/collaborators/transfer-owner`, + { email: MOCK_NEW_OWNER }, + ) + }) + }) }) From 88cf5f42efac8dd86ff49b672bed9c61829590a3 Mon Sep 17 00:00:00 2001 From: Kar Rui Lau Date: Wed, 9 Jun 2021 14:09:10 +0800 Subject: [PATCH 10/56] fix: return storage mode submission version when when retrieving from server (#2112) * fix: add version key when retrieving storage mode submission data * fix(tests): update tests to account for version key * feat: add version key, make types primitive for EncryptedSubmissionDto * feat: pass in version param when decrypting on client * test: update tests for change in EncryptSubmissionDto shape --- .../encrypt-submission.server.model.spec.ts | 78 +++++++++---------- src/app/models/submission.server.model.ts | 2 + .../__tests__/admin-form.routes.spec.ts | 29 ++++--- .../encrypt-submission.utils.ts | 1 + .../admin-forms.submissions.routes.spec.ts | 32 ++++---- .../view-responses.client.controller.js | 3 +- .../forms/helpers/decryption.worker.js | 1 + src/types/submission.ts | 14 ++-- 8 files changed, 87 insertions(+), 73 deletions(-) diff --git a/src/app/models/__tests__/encrypt-submission.server.model.spec.ts b/src/app/models/__tests__/encrypt-submission.server.model.spec.ts index f48656cb2f..4d554fe663 100644 --- a/src/app/models/__tests__/encrypt-submission.server.model.spec.ts +++ b/src/app/models/__tests__/encrypt-submission.server.model.spec.ts @@ -4,12 +4,11 @@ import moment from 'moment-timezone' import mongoose from 'mongoose' import getSubmissionModel, { + getEmailSubmissionModel, getEncryptSubmissionModel, } from 'src/app/models/submission.server.model' import { - IEmailSubmissionSchema, IEncryptedSubmissionSchema, - ISubmissionSchema, SubmissionMetadata, SubmissionType, } from 'src/types' @@ -17,6 +16,7 @@ import { import dbHandler from 'tests/unit/backend/helpers/jest-db' const Submission = getSubmissionModel(mongoose) +const EmailSubmission = getEmailSubmissionModel(mongoose) const EncryptSubmission = getEncryptSubmissionModel(mongoose) describe('Encrypt Submission Model', () => { @@ -33,15 +33,14 @@ describe('Encrypt Submission Model', () => { const validFormId = new ObjectId().toHexString() const createdDate = new Date() // Add valid encrypt submission. - const validSubmission = - await Submission.create({ - form: validFormId, - myInfoFields: [], - submissionType: SubmissionType.Encrypt, - encryptedContent: MOCK_ENCRYPTED_CONTENT, - version: 1, - created: createdDate, - }) + const validSubmission = await EncryptSubmission.create({ + form: validFormId, + myInfoFields: [], + submissionType: SubmissionType.Encrypt, + encryptedContent: MOCK_ENCRYPTED_CONTENT, + version: 1, + created: createdDate, + }) // Act const result = await EncryptSubmission.findSingleMetadata( @@ -110,7 +109,7 @@ describe('Encrypt Submission Model', () => { // Arrange // Add 3 valid encrypt submission. const validSubmissionPromises = times(3, (idx) => - Submission.create({ + EncryptSubmission.create({ form: VALID_FORM_ID, myInfoFields: [], submissionType: SubmissionType.Encrypt, @@ -119,9 +118,8 @@ describe('Encrypt Submission Model', () => { created: MOCK_CREATED_DATES_ASC[idx], }), ) - const validSubmissions: ISubmissionSchema[] = await Promise.all( - validSubmissionPromises, - ) + const validSubmissions: IEncryptedSubmissionSchema[] = + await Promise.all(validSubmissionPromises) // Act const actual = await EncryptSubmission.findAllMetadataByFormId( @@ -149,7 +147,7 @@ describe('Encrypt Submission Model', () => { // Arrange // Add 3 valid encrypt submission. const validSubmissionPromises = times(3, (idx) => - Submission.create({ + EncryptSubmission.create({ form: VALID_FORM_ID, myInfoFields: [], submissionType: SubmissionType.Encrypt, @@ -158,9 +156,8 @@ describe('Encrypt Submission Model', () => { created: MOCK_CREATED_DATES_ASC[idx], }), ) - const validSubmissions: ISubmissionSchema[] = await Promise.all( - validSubmissionPromises, - ) + const validSubmissions: IEncryptedSubmissionSchema[] = + await Promise.all(validSubmissionPromises) // Act const actual = await EncryptSubmission.findAllMetadataByFormId( @@ -192,7 +189,7 @@ describe('Encrypt Submission Model', () => { // Arrange // Add 3 valid encrypt submission. const validSubmissionPromises = times(3, (idx) => - Submission.create({ + EncryptSubmission.create({ form: VALID_FORM_ID, myInfoFields: [], submissionType: SubmissionType.Encrypt, @@ -201,9 +198,8 @@ describe('Encrypt Submission Model', () => { created: MOCK_CREATED_DATES_ASC[idx], }), ) - const validSubmissions: ISubmissionSchema[] = await Promise.all( - validSubmissionPromises, - ) + const validSubmissions: IEncryptedSubmissionSchema[] = + await Promise.all(validSubmissionPromises) // Act const actual = await EncryptSubmission.findAllMetadataByFormId( @@ -235,7 +231,7 @@ describe('Encrypt Submission Model', () => { // Arrange // Add 3 valid encrypt submission. const validSubmissionPromises = times(3, (idx) => - Submission.create({ + EncryptSubmission.create({ form: VALID_FORM_ID, myInfoFields: [], submissionType: SubmissionType.Encrypt, @@ -244,9 +240,8 @@ describe('Encrypt Submission Model', () => { created: MOCK_CREATED_DATES_ASC[idx], }), ) - const validSubmissions: ISubmissionSchema[] = await Promise.all( - validSubmissionPromises, - ) + const validSubmissions: IEncryptedSubmissionSchema[] = + await Promise.all(validSubmissionPromises) // Act const actual = await EncryptSubmission.findAllMetadataByFormId( @@ -288,11 +283,11 @@ describe('Encrypt Submission Model', () => { it('should return cursor that contains all the submissions', async () => { // Arrange const validFormId = new ObjectId().toHexString() - const validSubmission = await Submission.create({ + const validSubmission = await EncryptSubmission.create({ submissionType: SubmissionType.Encrypt, form: validFormId, encryptedContent: 'mock encrypted content abc', - version: 1, + version: 3, }) const expectedSubmission = pick( validSubmission, @@ -301,6 +296,7 @@ describe('Encrypt Submission Model', () => { 'verifiedContent', 'encryptedContent', 'submissionType', + 'version', ) // Act @@ -344,11 +340,11 @@ describe('Encrypt Submission Model', () => { it('should return correct submission by its id', async () => { // Arrange const validFormId = new ObjectId().toHexString() - const validSubmission = await Submission.create({ + const validSubmission = await EncryptSubmission.create({ submissionType: SubmissionType.Encrypt, form: validFormId, encryptedContent: 'mock encrypted content abc', - version: 1, + version: 33, attachmentMetadata: { someFileName: 'some url of attachment' }, }) @@ -360,15 +356,16 @@ describe('Encrypt Submission Model', () => { // Assert const expected = pick( - validSubmission.toObject(), + validSubmission.toJSON(), '_id', 'attachmentMetadata', 'created', 'encryptedContent', 'submissionType', + 'version', ) expect(actual).not.toBeNull() - expect(actual?.toObject()).toEqual(expected) + expect(actual?.toJSON()).toEqual(expected) }) it('should return null when submission id does not exist', async () => { @@ -390,14 +387,13 @@ describe('Encrypt Submission Model', () => { it('should return null when type of submission with given id is not SubmissionType.Encrypt', async () => { // Arrange const validFormId = new ObjectId().toHexString() - const validEmailSubmission = - await Submission.create({ - submissionType: SubmissionType.Email, - form: validFormId, - recipientEmails: ['any@example.com'], - responseHash: 'any hash', - responseSalt: 'any salt', - }) + const validEmailSubmission = await EmailSubmission.create({ + submissionType: SubmissionType.Email, + form: validFormId, + recipientEmails: ['any@example.com'], + responseHash: 'any hash', + responseSalt: 'any salt', + }) // Act const actual = await EncryptSubmission.findEncryptedSubmissionById( diff --git a/src/app/models/submission.server.model.ts b/src/app/models/submission.server.model.ts index 8c949bbedc..7f96c38887 100644 --- a/src/app/models/submission.server.model.ts +++ b/src/app/models/submission.server.model.ts @@ -360,6 +360,7 @@ EncryptSubmissionSchema.statics.getSubmissionCursorByFormId = function ( verifiedContent: 1, attachmentMetadata: 1, created: 1, + version: 1, id: 1, }) .batchSize(2000) @@ -384,6 +385,7 @@ EncryptSubmissionSchema.statics.findEncryptedSubmissionById = function ( verifiedContent: 1, attachmentMetadata: 1, created: 1, + version: 1, }) .exec() } diff --git a/src/app/modules/form/admin-form/__tests__/admin-form.routes.spec.ts b/src/app/modules/form/admin-form/__tests__/admin-form.routes.spec.ts index 7256c76e3f..07fea92b2c 100644 --- a/src/app/modules/form/admin-form/__tests__/admin-form.routes.spec.ts +++ b/src/app/modules/form/admin-form/__tests__/admin-form.routes.spec.ts @@ -3609,7 +3609,7 @@ describe('admin-form.routes', () => { encryptedContent: 'any encrypted content', verifiedContent: 'any verified content', } - const submission = await createSubmission({ + const submission = await createEncryptSubmission({ form: defaultForm, ...expectedSubmissionParams, }) @@ -3629,6 +3629,7 @@ describe('admin-form.routes', () => { refNo: String(submission._id), submissionTime: expect.any(String), verified: expectedSubmissionParams.verifiedContent, + version: submission.version, }) }) @@ -3642,7 +3643,7 @@ describe('admin-form.routes', () => { ['fieldId2', 'some.other.attachment.url'], ]), } - const submission = await createSubmission({ + const submission = await createEncryptSubmission({ form: defaultForm, ...expectedSubmissionParams, }) @@ -3669,6 +3670,7 @@ describe('admin-form.routes', () => { refNo: String(submission._id), submissionTime: expect.any(String), verified: expectedSubmissionParams.verifiedContent, + version: submission.version, }) }) @@ -3819,7 +3821,7 @@ describe('admin-form.routes', () => { jest .spyOn(EncryptSubmissionModel, 'findEncryptedSubmissionById') .mockRejectedValueOnce(new Error('ohno')) - const submission = await createSubmission({ + const submission = await createEncryptSubmission({ form: defaultForm, encryptedContent: 'any encrypted content', verifiedContent: 'any verified content', @@ -3848,7 +3850,7 @@ describe('admin-form.routes', () => { .spyOn(aws.s3, 'getSignedUrlPromise') .mockRejectedValueOnce(new Error('something went wrong')) - const submission = await createSubmission({ + const submission = await createEncryptSubmission({ form: defaultForm, encryptedContent: 'any encrypted content', verifiedContent: 'any verified content', @@ -4349,7 +4351,7 @@ describe('admin-form.routes', () => { // Create 11 submissions const submissions = await Promise.all( times(11, (count) => - createSubmission({ + createEncryptSubmission({ form: defaultForm, encryptedContent: `any encrypted content ${count}`, verifiedContent: `any verified content ${count}`, @@ -4384,7 +4386,7 @@ describe('admin-form.routes', () => { it('should return 200 with empty results if query.page does not have metadata', async () => { // Arrange // Create single submission - await createSubmission({ + await createEncryptSubmission({ form: defaultForm, encryptedContent: `any encrypted content`, verifiedContent: `any verified content`, @@ -4412,7 +4414,7 @@ describe('admin-form.routes', () => { // Create 3 submissions const submissions = await Promise.all( times(3, (count) => - createSubmission({ + createEncryptSubmission({ form: defaultForm, encryptedContent: `any encrypted content ${count}`, verifiedContent: `any verified content ${count}`, @@ -4603,7 +4605,7 @@ describe('admin-form.routes', () => { // Arrange const submissions = await Promise.all( times(11, (count) => - createSubmission({ + createEncryptSubmission({ form: defaultForm, encryptedContent: `any encrypted content ${count}`, verifiedContent: `any verified content ${count}`, @@ -4639,6 +4641,7 @@ describe('admin-form.routes', () => { encryptedContent: s.encryptedContent, verifiedContent: s.verifiedContent, created: s.created, + version: s.version, }), ) .sort((a, b) => String(a._id).localeCompare(String(b._id))) @@ -4659,7 +4662,7 @@ describe('admin-form.routes', () => { // Arrange const submissions = await Promise.all( times(5, (count) => - createSubmission({ + createEncryptSubmission({ form: defaultForm, encryptedContent: `any encrypted content ${count}`, verifiedContent: `any verified content ${count}`, @@ -4695,6 +4698,7 @@ describe('admin-form.routes', () => { encryptedContent: s.encryptedContent, verifiedContent: s.verifiedContent, created: s.created, + version: s.version, }), ) .sort((a, b) => String(a._id).localeCompare(String(b._id))) @@ -4723,7 +4727,7 @@ describe('admin-form.routes', () => { // Arrange const submissions = await Promise.all( times(5, (count) => - createSubmission({ + createEncryptSubmission({ form: defaultForm, encryptedContent: `any encrypted content ${count}`, verifiedContent: `any verified content ${count}`, @@ -4774,6 +4778,7 @@ describe('admin-form.routes', () => { encryptedContent: s.encryptedContent, verifiedContent: s.verifiedContent, created: s.created, + version: s.version, }), ) .filter((s) => expectedSubmissionIds.includes(s._id)) @@ -5385,7 +5390,7 @@ describe('admin-form.routes', () => { }) // Helper utils -const createSubmission = ({ +const createEncryptSubmission = ({ form, encryptedContent, verifiedContent, @@ -5398,7 +5403,7 @@ const createSubmission = ({ verifiedContent?: string created?: Date }) => { - return SubmissionModel.create({ + return EncryptSubmissionModel.create({ submissionType: SubmissionType.Encrypt, form: form._id, authType: form.authType, diff --git a/src/app/modules/submission/encrypt-submission/encrypt-submission.utils.ts b/src/app/modules/submission/encrypt-submission/encrypt-submission.utils.ts index c1f99b8203..0a61cd2641 100644 --- a/src/app/modules/submission/encrypt-submission/encrypt-submission.utils.ts +++ b/src/app/modules/submission/encrypt-submission/encrypt-submission.utils.ts @@ -208,5 +208,6 @@ export const createEncryptedSubmissionDto = ( content: submissionData.encryptedContent, verified: submissionData.verifiedContent, attachmentMetadata: attachmentPresignedUrls, + version: submissionData.version, } } diff --git a/src/app/routes/api/v3/admin/forms/__tests__/admin-forms.submissions.routes.spec.ts b/src/app/routes/api/v3/admin/forms/__tests__/admin-forms.submissions.routes.spec.ts index 4b1f5fa894..b83cfc8f02 100644 --- a/src/app/routes/api/v3/admin/forms/__tests__/admin-forms.submissions.routes.spec.ts +++ b/src/app/routes/api/v3/admin/forms/__tests__/admin-forms.submissions.routes.spec.ts @@ -535,7 +535,7 @@ describe('admin-form.submissions.routes', () => { // Arrange const submissions = await Promise.all( times(11, (count) => - createSubmission({ + createEncryptSubmission({ form: defaultForm, encryptedContent: `any encrypted content ${count}`, verifiedContent: `any verified content ${count}`, @@ -571,6 +571,7 @@ describe('admin-form.submissions.routes', () => { encryptedContent: s.encryptedContent, verifiedContent: s.verifiedContent, created: s.created, + version: s.version, }), ) .sort((a, b) => String(a._id).localeCompare(String(b._id))) @@ -591,7 +592,7 @@ describe('admin-form.submissions.routes', () => { // Arrange const submissions = await Promise.all( times(5, (count) => - createSubmission({ + createEncryptSubmission({ form: defaultForm, encryptedContent: `any encrypted content ${count}`, verifiedContent: `any verified content ${count}`, @@ -627,6 +628,7 @@ describe('admin-form.submissions.routes', () => { encryptedContent: s.encryptedContent, verifiedContent: s.verifiedContent, created: s.created, + version: s.version, }), ) .sort((a, b) => String(a._id).localeCompare(String(b._id))) @@ -655,7 +657,7 @@ describe('admin-form.submissions.routes', () => { // Arrange const submissions = await Promise.all( times(5, (count) => - createSubmission({ + createEncryptSubmission({ form: defaultForm, encryptedContent: `any encrypted content ${count}`, verifiedContent: `any verified content ${count}`, @@ -705,6 +707,7 @@ describe('admin-form.submissions.routes', () => { encryptedContent: s.encryptedContent, verifiedContent: s.verifiedContent, created: s.created, + version: s.version, }), ) .filter((s) => expectedSubmissionIds.includes(s._id)) @@ -726,7 +729,7 @@ describe('admin-form.submissions.routes', () => { // Arrange const submissions = await Promise.all( times(5, (count) => - createSubmission({ + createEncryptSubmission({ form: defaultForm, encryptedContent: `any encrypted content ${count}`, verifiedContent: `any verified content ${count}`, @@ -777,6 +780,7 @@ describe('admin-form.submissions.routes', () => { encryptedContent: s.encryptedContent, verifiedContent: s.verifiedContent, created: s.created, + version: s.version, }), ) .filter((s) => expectedSubmissionIds.includes(s._id)) @@ -928,7 +932,7 @@ describe('admin-form.submissions.routes', () => { encryptedContent: 'any encrypted content', verifiedContent: 'any verified content', } - const submission = await createSubmission({ + const submission = await createEncryptSubmission({ form: defaultForm, ...expectedSubmissionParams, }) @@ -948,6 +952,7 @@ describe('admin-form.submissions.routes', () => { refNo: String(submission._id), submissionTime: expect.any(String), verified: expectedSubmissionParams.verifiedContent, + version: submission.version, }) }) @@ -961,7 +966,7 @@ describe('admin-form.submissions.routes', () => { ['fieldId2', 'some.other.attachment.url'], ]), } - const submission = await createSubmission({ + const submission = await createEncryptSubmission({ form: defaultForm, ...expectedSubmissionParams, }) @@ -988,6 +993,7 @@ describe('admin-form.submissions.routes', () => { refNo: String(submission._id), submissionTime: expect.any(String), verified: expectedSubmissionParams.verifiedContent, + version: submission.version, }) }) @@ -1139,7 +1145,7 @@ describe('admin-form.submissions.routes', () => { jest .spyOn(EncryptSubmissionModel, 'findEncryptedSubmissionById') .mockRejectedValueOnce(new Error('ohno')) - const submission = await createSubmission({ + const submission = await createEncryptSubmission({ form: defaultForm, encryptedContent: 'any encrypted content', verifiedContent: 'any verified content', @@ -1168,7 +1174,7 @@ describe('admin-form.submissions.routes', () => { .spyOn(aws.s3, 'getSignedUrlPromise') .mockRejectedValueOnce(new Error('something went wrong')) - const submission = await createSubmission({ + const submission = await createEncryptSubmission({ form: defaultForm, encryptedContent: 'any encrypted content', verifiedContent: 'any verified content', @@ -1223,7 +1229,7 @@ describe('admin-form.submissions.routes', () => { // Create 11 submissions const submissions = await Promise.all( times(11, (count) => - createSubmission({ + createEncryptSubmission({ form: defaultForm, encryptedContent: `any encrypted content ${count}`, verifiedContent: `any verified content ${count}`, @@ -1258,7 +1264,7 @@ describe('admin-form.submissions.routes', () => { it('should return 200 with empty results if query.page does not have metadata', async () => { // Arrange // Create single submission - await createSubmission({ + await createEncryptSubmission({ form: defaultForm, encryptedContent: `any encrypted content`, verifiedContent: `any verified content`, @@ -1286,7 +1292,7 @@ describe('admin-form.submissions.routes', () => { // Create 3 submissions const submissions = await Promise.all( times(3, (count) => - createSubmission({ + createEncryptSubmission({ form: defaultForm, encryptedContent: `any encrypted content ${count}`, verifiedContent: `any verified content ${count}`, @@ -1463,7 +1469,7 @@ describe('admin-form.submissions.routes', () => { }) // Helper utils -const createSubmission = ({ +const createEncryptSubmission = ({ form, encryptedContent, verifiedContent, @@ -1476,7 +1482,7 @@ const createSubmission = ({ verifiedContent?: string created?: Date }) => { - return SubmissionModel.create({ + return EncryptSubmissionModel.create({ submissionType: SubmissionType.Encrypt, form: form._id, authType: form.authType, diff --git a/src/public/modules/forms/admin/controllers/view-responses.client.controller.js b/src/public/modules/forms/admin/controllers/view-responses.client.controller.js index 22d65c2804..3b8ff8d9fd 100644 --- a/src/public/modules/forms/admin/controllers/view-responses.client.controller.js +++ b/src/public/modules/forms/admin/controllers/view-responses.client.controller.js @@ -211,7 +211,7 @@ function ViewResponsesController( if (vm.encryptionKey !== null) { vm.attachmentDownloadUrls = new Map() - const { content, verified, attachmentMetadata } = response + const { content, verified, attachmentMetadata, version } = response let displayedContent try { @@ -220,6 +220,7 @@ function ViewResponsesController( { encryptedContent: content, verifiedContent: verified, + version, }, ) diff --git a/src/public/modules/forms/helpers/decryption.worker.js b/src/public/modules/forms/helpers/decryption.worker.js index 9f8b3abe7a..d5fcf1a050 100644 --- a/src/public/modules/forms/helpers/decryption.worker.js +++ b/src/public/modules/forms/helpers/decryption.worker.js @@ -192,6 +192,7 @@ async function decryptIntoCsv(data) { formsgSdk.crypto.decrypt(secretKey, { encryptedContent: submission.encryptedContent, verifiedContent: submission.verifiedContent, + version: submission.version, }), ) diff --git a/src/types/submission.ts b/src/types/submission.ts index 3c2e640352..be9f3c7541 100644 --- a/src/types/submission.ts +++ b/src/types/submission.ts @@ -15,11 +15,12 @@ export type SubmissionMetadata = { } export type EncryptedSubmissionDto = { - refNo: IEncryptedSubmissionSchema['_id'] + refNo: string submissionTime: string - content: IEncryptedSubmissionSchema['encryptedContent'] - verified: IEncryptedSubmissionSchema['verifiedContent'] + content: string + verified?: string attachmentMetadata: Record + version: number } export type SubmissionMetadataList = { @@ -132,7 +133,7 @@ export interface IWebhookResponse { // Due to schema changes, some objects may not have attachmentMetadata key. export type SubmissionCursorData = Pick< IEncryptedSubmissionSchema, - 'encryptedContent' | 'verifiedContent' | 'created' | 'id' + 'encryptedContent' | 'verifiedContent' | 'created' | 'id' | 'version' > & { attachmentMetadata?: Record } & Document export type SubmissionData = Pick< @@ -141,8 +142,9 @@ export type SubmissionData = Pick< | 'verifiedContent' | 'attachmentMetadata' | 'created' - | '_id' -> + | 'version' +> & + Document export type IEmailSubmissionModel = Model & ISubmissionModel From f381be8e32ca1468a2c2da2472a6c615bbc5dd35 Mon Sep 17 00:00:00 2001 From: Jiayee Lim Date: Wed, 9 Jun 2021 14:24:18 +0800 Subject: [PATCH 11/56] feat: store only user ID in session (#1849) * feat: store only user ID in session * fix: fix test breakage * fix: update schema correctly * chore: remove TODO because done * fix: update login logger to reference sessionUser._id instead of email email is now removed from the logged in user Co-authored-by: Kar Rui Lau --- src/app/modules/auth/__tests__/auth.controller.spec.ts | 2 +- src/app/modules/auth/auth.controller.ts | 9 ++++----- src/app/modules/auth/auth.utils.ts | 1 - src/app/modules/billing/billing.controller.ts | 2 +- src/types/vendor/express.d.ts | 8 ++++++-- 5 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/app/modules/auth/__tests__/auth.controller.spec.ts b/src/app/modules/auth/__tests__/auth.controller.spec.ts index 94f7d8fe05..b75a6d001d 100644 --- a/src/app/modules/auth/__tests__/auth.controller.spec.ts +++ b/src/app/modules/auth/__tests__/auth.controller.spec.ts @@ -187,7 +187,7 @@ describe('auth.controller', () => { // Assert expect(mockRes.status).toBeCalledWith(200) - expect(mockRes.json).toBeCalledWith(mockUser.toObject()) + expect(mockRes.json).toBeCalledWith(mockUser) }) it('should return 401 when retrieving agency returns InvalidDomainError', async () => { diff --git a/src/app/modules/auth/auth.controller.ts b/src/app/modules/auth/auth.controller.ts index 1365dace36..81b2993b79 100644 --- a/src/app/modules/auth/auth.controller.ts +++ b/src/app/modules/auth/auth.controller.ts @@ -158,16 +158,15 @@ export const handleLoginVerifyOtp: ControllerHandler< .json(coreErrorMessage) } - // TODO(#212): Should store only userId in session. // Add user info to session. - const userObj = user.toObject() as SessionUser - req.session.user = userObj + const { _id } = user.toObject() as SessionUser + req.session.user = { _id } logger.info({ - message: `Successfully logged in user ${user.email}`, + message: `Successfully logged in user ${user._id}`, meta: logMeta, }) - return res.status(StatusCodes.OK).json(userObj) + return res.status(StatusCodes.OK).json(user) }) // Step 3b: Error occured in one of the steps. .mapErr((error) => { diff --git a/src/app/modules/auth/auth.utils.ts b/src/app/modules/auth/auth.utils.ts index b6de7e1d51..829cb8d7f3 100644 --- a/src/app/modules/auth/auth.utils.ts +++ b/src/app/modules/auth/auth.utils.ts @@ -57,7 +57,6 @@ export const isUserInSession = ( return !!session?.user?._id } -// TODO(#212): Save userId instead of entire user collection in session. export const getUserIdFromSession = ( session?: Express.Session, ): string | undefined => { diff --git a/src/app/modules/billing/billing.controller.ts b/src/app/modules/billing/billing.controller.ts index 23c24fce8c..959b00035a 100644 --- a/src/app/modules/billing/billing.controller.ts +++ b/src/app/modules/billing/billing.controller.ts @@ -55,7 +55,7 @@ export const handleGetBillInfo: ControllerHandler< // Retrieved login stats successfully. logger.info({ - message: `Billing search for ${esrvcId} by ${authedUser.email}`, + message: `Billing search for ${esrvcId} by ${authedUser._id}`, meta: { action: 'handleGetBillInfo', ...createReqMeta(req), diff --git a/src/types/vendor/express.d.ts b/src/types/vendor/express.d.ts index eab0c7af59..64ad20ce90 100644 --- a/src/types/vendor/express.d.ts +++ b/src/types/vendor/express.d.ts @@ -7,11 +7,15 @@ declare global { } export interface Session { - user?: IUserSchema + user?: { + _id: IUserSchema['_id'] + } } export interface AuthedSession extends Session { - user: IUserSchema + user: { + _id: IUserSchema['_id'] + } } } } From 04f5270a3dbee6455c045a7c7a6297b9394f8238 Mon Sep 17 00:00:00 2001 From: Kar Rui Lau Date: Wed, 9 Jun 2021 14:49:22 +0800 Subject: [PATCH 12/56] refactor(auth.client): (1) extract email validation and send login otp flow to Typescript (#2084) * feat(AdminAuthService): add checkIsEmailAllowed service function * ref: condense vm.checkEmail and vm.checkUser into single vm.login fn * feat(AdminAuthService): add sendLoginOtp function * ref: use AdminAuthService.sendLoginOtp * ref: rename AdminAuthService to AuthService better fit API prefix * feat: remove all catch transformations from AuthService not needed, may interfere with React migration. Should just handle it in the caller # Conflicts: # src/public/modules/users/controllers/authentication.client.controller.js # src/public/services/AuthService.ts * test(AuthService): unit tests for checkIsEmailAllowed + sendLoginOtp * feat: remove unused functions from old auth.client * fix(auth.client.ctl): correctly retrieve error msg when sendOtp fails * test(AuthService): add clarity to test block Co-authored-by: tshuli <63710093+tshuli@users.noreply.github.com> * feat(AuthService): remove throw documentation on sendLoginOtp Co-authored-by: tshuli <63710093+tshuli@users.noreply.github.com> --- .../authentication.client.controller.js | 93 +++++++++---------- .../users/services/auth.client.service.js | 29 ------ .../authentication/signin.client.view.html | 2 +- src/public/services/AuthService.ts | 33 +++++++ .../services/__tests__/AuthService.test.ts | 60 ++++++++++++ 5 files changed, 140 insertions(+), 77 deletions(-) create mode 100644 src/public/services/AuthService.ts create mode 100644 src/public/services/__tests__/AuthService.test.ts diff --git a/src/public/modules/users/controllers/authentication.client.controller.js b/src/public/modules/users/controllers/authentication.client.controller.js index 403c9b2861..4ac92def2b 100755 --- a/src/public/modules/users/controllers/authentication.client.controller.js +++ b/src/public/modules/users/controllers/authentication.client.controller.js @@ -1,10 +1,14 @@ 'use strict' const HttpStatus = require('http-status-codes') +const get = require('lodash/get') +const validator = require('validator').default +const AuthService = require('../../../services/AuthService') angular .module('users') .controller('AuthenticationController', [ + '$q', '$scope', '$state', '$timeout', @@ -15,6 +19,7 @@ angular ]) function AuthenticationController( + $q, $scope, $state, $timeout, @@ -54,7 +59,7 @@ function AuthenticationController( vm.handleEmailKeyUp = function (e) { if (e.keyCode == 13) { - vm.isSubmitEmailDisabled === false && vm.checkEmail() + vm.isSubmitEmailDisabled === false && vm.login() // condition vm.isSubmitEmailDisabled == false is necessary to prevent retries using enter key // when submit button is disabled } else { @@ -115,6 +120,16 @@ function AuthenticationController( } } + const setEmailSignInError = (errorMsg) => { + vm.buttonClicked = false + vm.signInMsg = { + isMsg: true, + isError: true, + msg: errorMsg, + } + vm.isSubmitEmailDisabled = true + } + // Steps of sign-in process // 1 - Check if user email is in valid format // 2 - Check if user's email domain (i.e. agency) is valid @@ -122,45 +137,28 @@ function AuthenticationController( // 4 - Verify OTP /** - * Conducts front-end check of user email format + * Checks validity of email domain (i.e. agency) and sends login OTP if email + * is valid. */ - vm.checkEmail = function () { + vm.login = function () { vm.buttonClicked = true - if (/\S+@\S+\.\S+/.test(vm.credentials.email)) { - vm.credentials.email = vm.credentials.email.toLowerCase() - vm.checkUser() - } else { - // Invalid email - vm.buttonClicked = false - vm.signInMsg = { - isMsg: true, - isError: true, - msg: 'Please enter a valid email', - } - vm.isSubmitEmailDisabled = true + + const email = vm.credentials.email + if (!validator.isEmail(email)) { + setEmailSignInError('Please enter a valid email address') + return } - } - /** - * Checks validity of email domain (i.e. agency) - * Directs user to otp input page - */ - vm.checkUser = function () { - Auth.checkUser(vm.credentials).then( - function () { - vm.sendOtp() - }, - function (error) { - // Invalid agency - vm.buttonClicked = false - vm.signInMsg = { - isMsg: true, - isError: true, - msg: error, - } - vm.isSubmitEmailDisabled = true - }, - ) + $q.when(AuthService.checkIsEmailAllowed(vm.credentials.email)) + .then(() => vm.sendOtp()) + .catch((error) => { + const errorMsg = get( + error, + 'response.data', + 'Something went wrong while validating your email. Please refresh and try again', + ) + setEmailSignInError(errorMsg) + }) } /** @@ -173,9 +171,9 @@ function AuthenticationController( vm.isOtpSending = true vm.buttonClicked = true vm.showOtpDelayNotification = false - const { email } = vm.credentials - Auth.sendOtp({ email }).then( - function (success) { + + $q.when(AuthService.sendLoginOtp(vm.credentials.email)) + .then((success) => { vm.isOtpSending = false vm.buttonClicked = false // Configure message to be show @@ -195,21 +193,22 @@ function AuthenticationController( vm.signInMsg.isMsg = false vm.showOtpDelayNotification = true }, 20000) - }, - function (error) { + }) + .catch((error) => { + const errorMsg = get( + error, + 'response.data.message', + 'Failed to send login OTP. Please try again later and if the problem persists, contact us.', + ) vm.isOtpSending = false vm.buttonClicked = false // Configure message to be shown - const msg = - (error && error.data && error.data.message) || - 'Failed to send login OTP. Please try again later and if the problem persists, contact us.' vm.signInMsg = { isMsg: true, isError: true, - msg, + msg: errorMsg, } - }, - ) + }) } /** diff --git a/src/public/modules/users/services/auth.client.service.js b/src/public/modules/users/services/auth.client.service.js index 2ca5083b7d..a5bf7f719e 100644 --- a/src/public/modules/users/services/auth.client.service.js +++ b/src/public/modules/users/services/auth.client.service.js @@ -21,8 +21,6 @@ function Auth($q, $http, $state, $window) { */ let authService = { - checkUser, - sendOtp, verifyOtp, getUser, setUser, @@ -66,33 +64,6 @@ function Auth($q, $http, $state, $window) { return null }) } - - function checkUser(credentials) { - let deferred = $q.defer() - $http.post('/api/v3/auth/email/validate', credentials).then( - function (response) { - deferred.resolve(response.data) - }, - function (error) { - deferred.reject(error.data) - }, - ) - return deferred.promise - } - - function sendOtp(credentials) { - let deferred = $q.defer() - $http.post('/api/v3/auth/otp/generate', credentials).then( - function (response) { - deferred.resolve(response.data) - }, - function (error) { - deferred.reject(error) - }, - ) - return deferred.promise - } - function verifyOtp(credentials) { let deferred = $q.defer() $http.post('/api/v3/auth/otp/verify', credentials).then( diff --git a/src/public/modules/users/views/authentication/signin.client.view.html b/src/public/modules/users/views/authentication/signin.client.view.html index 48d6f7fb90..189d2e3c89 100755 --- a/src/public/modules/users/views/authentication/signin.client.view.html +++ b/src/public/modules/users/views/authentication/signin.client.view.html @@ -40,7 +40,7 @@
+
+ + Webhook is not available for forms with attachment fields. +
{ + return ( + $scope.myform.form_fields.filter( + (field) => field.fieldType == 'attachment', + ).length > 0 + ) + } + // Warning message when turning off SP with MyInfo fields $scope.myInfoSPWarning = () => { return ( diff --git a/src/public/modules/forms/admin/views/edit-fields.client.modal.html b/src/public/modules/forms/admin/views/edit-fields.client.modal.html index 98f1c99808..2a22004546 100644 --- a/src/public/modules/forms/admin/views/edit-fields.client.modal.html +++ b/src/public/modules/forms/admin/views/edit-fields.client.modal.html @@ -1069,6 +1069,7 @@

class="input-custom input-medium" placeholder="No. of Characters" ng-disabled="!vm.field.ValidationOptions.selectedValidation" + ng-change="vm.changeFxn(vm.field)" />

diff --git a/src/public/modules/forms/base/componentViews/field-number.client.view.html b/src/public/modules/forms/base/componentViews/field-number.client.view.html index 27abe19ed0..a6ba22465b 100644 --- a/src/public/modules/forms/base/componentViews/field-number.client.view.html +++ b/src/public/modules/forms/base/componentViews/field-number.client.view.html @@ -31,8 +31,8 @@ ng-required="vm.field.required" ng-disabled="vm.field.disabled" ng-pattern="/^\d*$/" - ng-maxlength="{{ vm.field.ValidationOptions && ['Exact', 'Maximum'].includes(vm.field.ValidationOptions.selectedValidation) ? vm.field.ValidationOptions.customVal : ''}}" - ng-minlength="{{ vm.field.ValidationOptions && ['Exact', 'Minimum'].includes(vm.field.ValidationOptions.selectedValidation) ? vm.field.ValidationOptions.customVal : ''}}" + ng-maxlength="{{ vm.field.ValidationOptions ? vm.field.ValidationOptions.customMax : '' }}" + ng-minlength="{{ vm.field.ValidationOptions ? vm.field.ValidationOptions.customMin : ''}}" ng-model-options="{ allowInvalid: true }" ng-keyup="vm.forms.myForm[(vm.field._id || 'defaultID')].$setTouched()" ng-attr-placeholder="{{ vm.placeholder }}" @@ -62,17 +62,17 @@
- Minimum {{vm.field.ValidationOptions.customVal}} characters + Minimum {{vm.field.ValidationOptions.customMin}} characters ({{vm.forms.myForm[(vm.field._id || - 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customVal}}) + 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customMin}})
- Maximum {{vm.field.ValidationOptions.customVal}} characters + Maximum {{vm.field.ValidationOptions.customMax}} characters ({{vm.forms.myForm[(vm.field._id || - 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customVal}}) + 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customMax}})
diff --git a/src/public/modules/forms/base/componentViews/field-textarea.client.view.html b/src/public/modules/forms/base/componentViews/field-textarea.client.view.html index 118a0ec194..59d7e05f2d 100644 --- a/src/public/modules/forms/base/componentViews/field-textarea.client.view.html +++ b/src/public/modules/forms/base/componentViews/field-textarea.client.view.html @@ -28,8 +28,8 @@ name="{{ vm.field._id || 'defaultID' }}" class="input-custom input-large" ng-model="vm.field.fieldValue" - ng-maxlength="{{ vm.field.ValidationOptions && ['Exact', 'Maximum'].includes(vm.field.ValidationOptions.selectedValidation) ? vm.field.ValidationOptions.customVal : ''}}" - ng-minlength="{{ vm.field.ValidationOptions && ['Exact', 'Minimum'].includes(vm.field.ValidationOptions.selectedValidation) ? vm.field.ValidationOptions.customVal : ''}}" + ng-maxlength="{{ vm.field.ValidationOptions ? vm.field.ValidationOptions.customMax : ''}}" + ng-minlength="{{ vm.field.ValidationOptions ? vm.field.ValidationOptions.customMin : ''}}" ng-model-options="{ allowInvalid: true }" ng-required="vm.field.required" ng-disabled="vm.field.disabled" @@ -58,18 +58,18 @@
- Minimum {{vm.field.ValidationOptions.customVal}} characters - ({{vm.field.fieldValue.length}}/{{vm.field.ValidationOptions.customVal}}) + Minimum {{vm.field.ValidationOptions.customMin}} characters + ({{vm.field.fieldValue.length}}/{{vm.field.ValidationOptions.customMin}})
- Maximum {{vm.field.ValidationOptions.customVal}} characters - ({{vm.field.fieldValue.length}}/{{vm.field.ValidationOptions.customVal}}) + Maximum {{vm.field.ValidationOptions.customMax}} characters + ({{vm.field.fieldValue.length}}/{{vm.field.ValidationOptions.customMax}})
- Must be exactly {{vm.field.ValidationOptions.customVal}} characters - ({{vm.field.fieldValue.length}}/{{vm.field.ValidationOptions.customVal}}) + Must be exactly {{vm.field.ValidationOptions.customMax}} characters + ({{vm.field.fieldValue.length}}/{{vm.field.ValidationOptions.customMax}})
diff --git a/src/public/modules/forms/base/componentViews/field-textfield.client.view.html b/src/public/modules/forms/base/componentViews/field-textfield.client.view.html index 6d898ff599..da39ac5557 100644 --- a/src/public/modules/forms/base/componentViews/field-textfield.client.view.html +++ b/src/public/modules/forms/base/componentViews/field-textfield.client.view.html @@ -30,8 +30,8 @@ ng-disabled="vm.field.disabled" ng-pattern="vm.pattern" ng-class="vm.field.allowPrefill && vm.field.isPrefilled && 'prefill-highlight'" - ng-maxlength="{{ vm.field.ValidationOptions && ['Exact', 'Maximum'].includes(vm.field.ValidationOptions.selectedValidation) ? vm.field.ValidationOptions.customVal : ''}}" - ng-minlength="{{ vm.field.ValidationOptions && ['Exact', 'Minimum'].includes(vm.field.ValidationOptions.selectedValidation) ? vm.field.ValidationOptions.customVal : ''}}" + ng-maxlength="{{vm.field.ValidationOptions ? vm.field.ValidationOptions.customMax : ''}}" + ng-minlength="{{vm.field.ValidationOptions ? vm.field.ValidationOptions.customMin : ''}}" ng-model-options="{ allowInvalid: true }" ng-keyup="vm.forms.myForm[(vm.field._id || 'defaultID')].$setTouched()" ng-attr-placeholder="{{ vm.placeholder }}" @@ -54,17 +54,17 @@
- Minimum {{vm.field.ValidationOptions.customVal}} characters + Minimum {{vm.field.ValidationOptions.customMin}} characters ({{vm.forms.myForm[(vm.field._id || - 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customVal}}) + 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customMin}})
- Maximum {{vm.field.ValidationOptions.customVal}} characters + Maximum {{vm.field.ValidationOptions.customMax}} characters ({{vm.forms.myForm[(vm.field._id || - 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customVal}}) + 'defaultID')].$viewValue.length}}/{{vm.field.ValidationOptions.customMax}})
diff --git a/src/public/modules/forms/base/componentViews/field-uen.client.view.html b/src/public/modules/forms/base/componentViews/field-uen.client.view.html deleted file mode 100644 index 794bcbc0c4..0000000000 --- a/src/public/modules/forms/base/componentViews/field-uen.client.view.html +++ /dev/null @@ -1,56 +0,0 @@ -
- - - - -
- -
- - -
- -
- - Please enter a valid UEN -
-
-
diff --git a/src/public/modules/forms/base/components/field-uen.client.component.js b/src/public/modules/forms/base/components/field-uen.client.component.js deleted file mode 100644 index 758960d2ea..0000000000 --- a/src/public/modules/forms/base/components/field-uen.client.component.js +++ /dev/null @@ -1,10 +0,0 @@ -'use strict' - -angular.module('forms').component('uenFieldComponent', { - templateUrl: 'modules/forms/base/componentViews/field-uen.client.view.html', - bindings: { - field: '<', - forms: '<', - }, - controllerAs: 'vm', -}) diff --git a/src/public/modules/forms/base/directiveViews/field.client.directive.view.html b/src/public/modules/forms/base/directiveViews/field.client.directive.view.html index c1e4c3675b..3c817c28a1 100644 --- a/src/public/modules/forms/base/directiveViews/field.client.directive.view.html +++ b/src/public/modules/forms/base/directiveViews/field.client.directive.view.html @@ -101,6 +101,4 @@ forms="forms" > - - diff --git a/src/public/modules/forms/base/directives/validate-uen.client.directive.js b/src/public/modules/forms/base/directives/validate-uen.client.directive.js deleted file mode 100644 index 4991ef3833..0000000000 --- a/src/public/modules/forms/base/directives/validate-uen.client.directive.js +++ /dev/null @@ -1,17 +0,0 @@ -'use strict' - -const { isUenValid } = require('../../../../../shared/util/uen-validation') - -angular.module('forms').directive('validateUen', validateUen) - -function validateUen() { - return { - restrict: 'A', - require: 'ngModel', - link: function (_scope, _elem, _attrs, ctrl) { - ctrl.$validators.uenValidator = function (modelValue) { - return ctrl.$isEmpty(modelValue) ? true : isUenValid(modelValue) - } - }, - } -} diff --git a/src/public/modules/forms/base/resources/icon-types.js b/src/public/modules/forms/base/resources/icon-types.js index 1cb8b46398..bdcdfa28c9 100644 --- a/src/public/modules/forms/base/resources/icon-types.js +++ b/src/public/modules/forms/base/resources/icon-types.js @@ -15,7 +15,6 @@ const iconTypeMap = { decimal: 'bx bx-calculator', image: 'bx bx-image', nric: 'bx bx-user', - uen: 'bx bx-building', attachment: 'bx bx-cloud-download', radiobutton: 'bx bx-radio-circle-marked', table: 'bx bx-table', diff --git a/src/public/modules/forms/helpers/field-factory.js b/src/public/modules/forms/helpers/field-factory.js index 55efd5fe3f..81791e627b 100644 --- a/src/public/modules/forms/helpers/field-factory.js +++ b/src/public/modules/forms/helpers/field-factory.js @@ -73,7 +73,6 @@ const getClass = (fieldType) => { case 'textfield': return TextField case 'nric': - case 'uen': case 'yes_no': return SingleAnswerField case 'image': diff --git a/src/public/modules/forms/viewmodels/Fields/MixIns.js b/src/public/modules/forms/viewmodels/Fields/MixIns.js index aba9eb7554..3b2e0ffd0f 100644 --- a/src/public/modules/forms/viewmodels/Fields/MixIns.js +++ b/src/public/modules/forms/viewmodels/Fields/MixIns.js @@ -32,6 +32,8 @@ const CustomValidation = (Base) => getDefaultBasicData() { const fieldData = super.getDefaultBasicData() fieldData.ValidationOptions = { + customMax: null, + customMin: null, customVal: null, selectedValidation: null, } diff --git a/src/public/services/AdminViewFormService.ts b/src/public/services/AdminViewFormService.ts index 190dad63bb..35f8abefb9 100644 --- a/src/public/services/AdminViewFormService.ts +++ b/src/public/services/AdminViewFormService.ts @@ -25,9 +25,7 @@ export const getDashboardView = async (): Promise => { export const getAdminFormView = async ( formId: string, ): Promise => { - return axios - .get(`${ADMIN_FORM_ENDPOINT}/${formId}`) - .then(({ data }) => data) + return axios.get(`/${formId}/adminform`).then(({ data }) => data) } /** diff --git a/src/public/services/__tests__/AdminViewFormService.test.ts b/src/public/services/__tests__/AdminViewFormService.test.ts index 75fafdb4aa..1390ee746c 100644 --- a/src/public/services/__tests__/AdminViewFormService.test.ts +++ b/src/public/services/__tests__/AdminViewFormService.test.ts @@ -82,9 +82,7 @@ describe('AdminViewFormService', () => { // Assert expect(actual).toEqual(expected) - expect(MockAxios.get).toHaveBeenCalledWith( - `${ADMIN_FORM_ENDPOINT}/${expected._id}`, - ) + expect(MockAxios.get).toHaveBeenCalledWith(`/${expected._id}/adminform`) }) it('should reject with error message when GET request fails', async () => { @@ -98,9 +96,7 @@ describe('AdminViewFormService', () => { // Assert await expect(actualPromise).rejects.toEqual(expected) - expect(MockAxios.get).toHaveBeenCalledWith( - `${ADMIN_FORM_ENDPOINT}/${MOCK_FORM._id}`, - ) + expect(MockAxios.get).toHaveBeenCalledWith(`/${MOCK_FORM._id}/adminform`) }) }) diff --git a/src/shared/resources/basic/index.ts b/src/shared/resources/basic/index.ts index ad84dffcc5..2befcefc8d 100644 --- a/src/shared/resources/basic/index.ts +++ b/src/shared/resources/basic/index.ts @@ -116,12 +116,6 @@ export const types: IBasicFieldType[] = [ submitted: true, answerArray: false, }, - { - name: BasicField.Uen, - value: 'UEN', - submitted: true, - answerArray: false, - }, { name: BasicField.Table, value: 'Table', diff --git a/src/shared/util/uen-validation.ts b/src/shared/util/uen-validation.ts deleted file mode 100644 index 32c866749e..0000000000 --- a/src/shared/util/uen-validation.ts +++ /dev/null @@ -1,129 +0,0 @@ -/** - * Validate entity-type indicators, as per - * https://www.uen.gov.sg/ueninternet/faces/pages/admin/aboutUEN.jspx - */ -const VALID_ENTITY_TYPE_INDICATORS = new Set([ - 'LP', - 'LL', - 'FC', - 'PF', - 'RF', - 'MQ', - 'MM', - 'NB', - 'CC', - 'CS', - 'MB', - 'FM', - 'GS', - 'GA', - 'GB', - 'DP', - 'CP', - 'NR', - 'CM', - 'CD', - 'MD', - 'HS', - 'VH', - 'CH', - 'MH', - 'CL', - 'XL', - 'CX', - 'RP', - 'TU', - 'TC', - 'FB', - 'FN', - 'PA', - 'PB', - 'SS', - 'MC', - 'SM', -]) - -/** - * Helper to check whether a string is numeric - * @param s String - * @returns True if string is numeric - */ -const isNumeric = (s: string): boolean => !!s.match(/^[0-9]+$/) - -/** - * Helper to check whether a string is alphabetic - * @param s string - * @returns True if string is alphabetic - */ -const isAlphabetic = (s: string): boolean => !!s.match(/^[a-zA-Z]+$/) - -/** - * Validates whether a provided string value adheres to the UIN/FIN format - * as provided on the Singapore Government's National Registration Identity Card. - * @param value The value to be validated - */ -export const isUenValid = (uen: string): boolean => { - // allow lowercase strings - uen = uen.toUpperCase() - - // check if uen is 9 or 10 digits - if (uen.length < 9 || uen.length > 10) { - return false - } - - // (A) Businesses registered with ACRA - if (uen.length === 9) { - // check that last character is a letter - const lastChar = uen[uen.length - 1] - if (!isAlphabetic(lastChar)) { - return false - } - - // check that first 8 letters are all numbers - const first8Chars = uen.slice(0, 8) - if (!isNumeric(first8Chars)) { - return false - } - - // (A) Businesses registered with ACRA (SUCCESS) - return true - } - - // Length is 10 - // check that last character is a letter - const lastChar = uen[uen.length - 1] - if (!isAlphabetic(lastChar)) { - return false - } - - // (B) Local companies registered with ACRA - const first4Chars = uen.slice(0, 4) - if (isNumeric(first4Chars)) { - // if first 4 are digits then next 5 must be digits too - const next5Chars = uen.slice(4, 9) - return isNumeric(next5Chars) - } - - // (C) All other entities which will be issued new UEN - // check that 1st letter is either T or S or R - const firstChar = uen[0] - if (!['T', 'S', 'R'].includes(firstChar)) { - return false - } - - // check that 2nd and 3rd letters are numbers only - const chars2And3 = uen.slice(1, 3) - if (!isNumeric(chars2And3)) { - return false - } - - // check entity-type indicator - const entityTypeIndicator = uen.slice(3, 5) - if (!VALID_ENTITY_TYPE_INDICATORS.has(entityTypeIndicator)) { - return false - } - - // check that 6th to 9th letters are numbers only - const chars5To8 = uen.slice(5, 9) - return isNumeric(chars5To8) -} diff --git a/src/types/field/baseField.ts b/src/types/field/baseField.ts index a66bfe4dcf..7301e622c4 100644 --- a/src/types/field/baseField.ts +++ b/src/types/field/baseField.ts @@ -54,4 +54,5 @@ export enum TextSelectedValidation { Maximum = 'Maximum', Minimum = 'Minimum', Exact = 'Exact', + Range = 'Range', // TODO(#408) - questionable value } diff --git a/src/types/field/fieldTypes.ts b/src/types/field/fieldTypes.ts index e72862f865..0e7eda7360 100644 --- a/src/types/field/fieldTypes.ts +++ b/src/types/field/fieldTypes.ts @@ -18,7 +18,6 @@ export enum BasicField { Rating = 'rating', Nric = 'nric', Table = 'table', - Uen = 'uen', } export enum MyInfoAttribute { diff --git a/src/types/field/index.ts b/src/types/field/index.ts index eb195970f4..52df4bc36f 100644 --- a/src/types/field/index.ts +++ b/src/types/field/index.ts @@ -16,7 +16,6 @@ import { ISectionField, ISectionFieldSchema } from './sectionField' import { IShortTextField, IShortTextFieldSchema } from './shortTextField' import { IStatementField, IStatementFieldSchema } from './statementField' import { ITableField, ITableFieldSchema } from './tableField' -import { IUenField, IUenFieldSchema } from './uenField' import { IYesNoField, IYesNoFieldSchema } from './yesNoField' export * from './fieldTypes' @@ -40,7 +39,6 @@ export * from './sectionField' export * from './shortTextField' export * from './statementField' export * from './tableField' -export * from './uenField' export * from './yesNoField' export type FormFieldSchema = @@ -62,7 +60,6 @@ export type FormFieldSchema = | IShortTextFieldSchema | IStatementFieldSchema | ITableFieldSchema - | IUenFieldSchema | IYesNoFieldSchema export type FormField = @@ -84,7 +81,6 @@ export type FormField = | IShortTextField | IStatementField | ITableField - | IUenField | IYesNoField /** diff --git a/src/types/field/longTextField.ts b/src/types/field/longTextField.ts index 59b8eac9f5..6957939890 100644 --- a/src/types/field/longTextField.ts +++ b/src/types/field/longTextField.ts @@ -1,6 +1,14 @@ -import { ITextField } from './utils/textField' -import { IFieldSchema } from './baseField' +import { IField, IFieldSchema, TextSelectedValidation } from './baseField' -export type ILongTextField = ITextField +export type LongTextValidationOptions = { + customMax: number | null + customMin: number | null + customVal: number | null + selectedValidation: TextSelectedValidation | null +} + +export interface ILongTextField extends IField { + ValidationOptions: LongTextValidationOptions +} export interface ILongTextFieldSchema extends ILongTextField, IFieldSchema {} diff --git a/src/types/field/numberField.ts b/src/types/field/numberField.ts index f8c4ca908c..1fe8cf7f1f 100644 --- a/src/types/field/numberField.ts +++ b/src/types/field/numberField.ts @@ -4,9 +4,12 @@ export enum NumberSelectedValidation { Max = 'Maximum', Min = 'Minimum', Exact = 'Exact', + Range = 'Range', } export type NumberValidationOptions = { + customMax: number | null + customMin: number | null customVal: number | null selectedValidation: NumberSelectedValidation | null } diff --git a/src/types/field/shortTextField.ts b/src/types/field/shortTextField.ts index b2ab928a3a..a1d5e4dcfb 100644 --- a/src/types/field/shortTextField.ts +++ b/src/types/field/shortTextField.ts @@ -1,7 +1,15 @@ -import { ITextField } from './utils/textField' -import { IFieldSchema } from './baseField' +import { IField, IFieldSchema, TextSelectedValidation } from './baseField' -export type IShortTextField = ITextField +export type ShortTextValidationOptions = { + customMax: number | null + customMin: number | null + customVal: number | null + selectedValidation: TextSelectedValidation | null +} + +export interface IShortTextField extends IField { + ValidationOptions: ShortTextValidationOptions +} export interface IShortTextFieldSchema extends IShortTextField, IFieldSchema { // Prefill flag diff --git a/src/types/field/uenField.ts b/src/types/field/uenField.ts deleted file mode 100644 index b2638ff3fd..0000000000 --- a/src/types/field/uenField.ts +++ /dev/null @@ -1,5 +0,0 @@ -import { IField, IFieldSchema } from './baseField' - -export type IUenField = IField - -export interface IUenFieldSchema extends IUenField, IFieldSchema {} diff --git a/src/types/field/utils/guards.ts b/src/types/field/utils/guards.ts index 19efe8b723..25e50e7efc 100644 --- a/src/types/field/utils/guards.ts +++ b/src/types/field/utils/guards.ts @@ -19,7 +19,6 @@ import { ISectionFieldSchema, IShortTextField, ITableFieldSchema, - IUenField, IYesNoField, } from '..' @@ -89,10 +88,6 @@ export const isDecimalField = ( return formField.fieldType === BasicField.Decimal } -export const isUenField = (formField: IField): formField is IUenField => { - return formField.fieldType === BasicField.Uen -} - export const isYesNoField = (formField: IField): formField is IYesNoField => { return formField.fieldType === BasicField.YesNo } diff --git a/src/types/field/utils/textField.ts b/src/types/field/utils/textField.ts deleted file mode 100644 index 8678c70d1f..0000000000 --- a/src/types/field/utils/textField.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { IField, IFieldSchema, TextSelectedValidation } from '../baseField' - -/** - * Utility types for short and long text field - */ -export type TextValidationOptions = { - customVal: number | null - selectedValidation: TextSelectedValidation | null -} - -export interface ITextField extends IField { - ValidationOptions: TextValidationOptions -} - -export interface ITextFieldSchema extends ITextField, IFieldSchema {} diff --git a/src/types/field/utils/virtuals.ts b/src/types/field/utils/virtuals.ts deleted file mode 100644 index fe670d2485..0000000000 --- a/src/types/field/utils/virtuals.ts +++ /dev/null @@ -1,9 +0,0 @@ -import { Document } from 'mongoose' - -// Types for virtuals for backwards compatibility after customMin and customMax were removed as part of #408 -// TODO: Remove virtual type (#2039) -export type WithCustomMinMax = T & - Document & { - customMin: number | null - customMax: number | null - } diff --git a/src/types/submission.ts b/src/types/submission.ts index be9f3c7541..708427f284 100644 --- a/src/types/submission.ts +++ b/src/types/submission.ts @@ -52,7 +52,6 @@ export interface WebhookData { verifiedContent: IEncryptedSubmissionSchema['verifiedContent'] version: IEncryptedSubmissionSchema['version'] created: IEncryptedSubmissionSchema['created'] - attachmentDownloadUrls: Record } export interface WebhookView { diff --git a/tests/unit/backend/helpers/generate-form-data.ts b/tests/unit/backend/helpers/generate-form-data.ts index fc3f4e2649..2fcc97e109 100644 --- a/tests/unit/backend/helpers/generate-form-data.ts +++ b/tests/unit/backend/helpers/generate-form-data.ts @@ -376,6 +376,8 @@ export const generateTableShortTextColumn = ( required: true, _id: new ObjectId().toHexString(), ValidationOptions: { + customMax: null, + customMin: null, customVal: null, selectedValidation: null, }, @@ -388,6 +390,8 @@ export const generateTableShortTextColumn = ( required: true, _id: new ObjectId().toHexString(), ValidationOptions: { + customMax: null, + customMin: null, customVal: null, selectedValidation: null, }, From a3f6291e0800c18024b635344aafcddcd7c0a567 Mon Sep 17 00:00:00 2001 From: Antariksh Date: Tue, 15 Jun 2021 13:53:42 +0800 Subject: [PATCH 56/56] chore: bump version to 5.14.0 --- CHANGELOG.md | 64 +++++++++++++++++++++++++++++++++++++++++++++++ package-lock.json | 2 +- package.json | 2 +- 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d2d6e9791f..382ff31957 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,72 @@ All notable changes to this project will be documented in this file. Dates are d Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog). +#### [v5.14.0](https://github.com/opengovsg/FormSG/compare/v5.13.1...v5.14.0) + +- feat: revert backwards-incompatible changes for v5.14.0 [`#2169`](https://github.com/opengovsg/FormSG/pull/2169) +- fix(auth.client.controller): add missing undefinedness check [`#2168`](https://github.com/opengovsg/FormSG/pull/2168) +- refactor: simplify isUenValid logic [`#2156`](https://github.com/opengovsg/FormSG/pull/2156) +- fix(deps): bump @opengovsg/spcp-auth-client from 1.4.7 to 1.4.8 [`#2164`](https://github.com/opengovsg/FormSG/pull/2164) +- fix(deps): bump @sentry/integrations from 6.6.0 to 6.7.0 [`#2163`](https://github.com/opengovsg/FormSG/pull/2163) +- chore(deps-dev): bump @typescript-eslint/parser from 4.26.1 to 4.27.0 [`#2162`](https://github.com/opengovsg/FormSG/pull/2162) +- refactor(encrypt-submission): introduce IncomingEncryptSubmission [`#1987`](https://github.com/opengovsg/FormSG/pull/1987) +- chore(deps-dev): bump @typescript-eslint/eslint-plugin [`#2161`](https://github.com/opengovsg/FormSG/pull/2161) +- fix(deps): bump @sentry/browser from 6.6.0 to 6.7.0 [`#2160`](https://github.com/opengovsg/FormSG/pull/2160) +- refactor(angularjs): remove angular-moment [`#2154`](https://github.com/opengovsg/FormSG/pull/2154) +- feat: add UEN field [`#2100`](https://github.com/opengovsg/FormSG/pull/2100) +- build: mute Localstack logs [`#2146`](https://github.com/opengovsg/FormSG/pull/2146) +- feat(feature-manager): remove GoogleAnalytics feature [`#2127`](https://github.com/opengovsg/FormSG/pull/2127) +- feat(feature-manager): hardcode /features API response [`#2149`](https://github.com/opengovsg/FormSG/pull/2149) +- fix(deps): bump zod from 3.1.0 to 3.2.0 [`#2151`](https://github.com/opengovsg/FormSG/pull/2151) +- fix(deps): bump aws-sdk from 2.925.0 to 2.927.0 [`#2153`](https://github.com/opengovsg/FormSG/pull/2153) +- chore(deps-dev): bump @types/mongodb from 3.6.17 to 3.6.18 [`#2152`](https://github.com/opengovsg/FormSG/pull/2152) +- chore(deps-dev): bump htmlhint from 0.14.2 to 0.15.1 [`#2150`](https://github.com/opengovsg/FormSG/pull/2150) +- chore: merge v5.13.1 into develop [`#2144`](https://github.com/opengovsg/FormSG/pull/2144) +- docs(feature-manager): update docs to reflect FeatureManager removal [`#2145`](https://github.com/opengovsg/FormSG/pull/2145) +- feat(feature-manager): remove Sentry from feature manager [`#2130`](https://github.com/opengovsg/FormSG/pull/2130) +- fix(submissions): remove captcha dependence on feature toggle [`#2143`](https://github.com/opengovsg/FormSG/pull/2143) +- chore(deps-dev): bump @babel/plugin-transform-runtime [`#2141`](https://github.com/opengovsg/FormSG/pull/2141) +- fix(deps): bump aws-sdk from 2.924.0 to 2.925.0 [`#2136`](https://github.com/opengovsg/FormSG/pull/2136) +- fix(deps): bump @sentry/browser from 6.5.1 to 6.6.0 [`#2135`](https://github.com/opengovsg/FormSG/pull/2135) +- chore(deps-dev): bump @babel/preset-env from 7.14.4 to 7.14.5 [`#2140`](https://github.com/opengovsg/FormSG/pull/2140) +- fix(deps): bump @babel/runtime from 7.14.0 to 7.14.5 [`#2139`](https://github.com/opengovsg/FormSG/pull/2139) +- chore(deps-dev): bump @babel/core from 7.14.3 to 7.14.5 [`#2138`](https://github.com/opengovsg/FormSG/pull/2138) +- fix(deps): bump @sentry/integrations from 6.5.1 to 6.6.0 [`#2137`](https://github.com/opengovsg/FormSG/pull/2137) +- test(adminsubmissionservice): unit tests for download methods [`#2129`](https://github.com/opengovsg/FormSG/pull/2129) +- feat(feature-manager): remove Intranet from feature manager [`#2131`](https://github.com/opengovsg/FormSG/pull/2131) +- refactor(submissions.client.factory): refactored download methods [`#2054`](https://github.com/opengovsg/FormSG/pull/2054) +- refactor(auth.client.service): refactor to Typescript [`#2132`](https://github.com/opengovsg/FormSG/pull/2132) +- test: remove basic and full e2e test separation [`#2128`](https://github.com/opengovsg/FormSG/pull/2128) +- fix(auth): make login emails case-insensitive [`#2125`](https://github.com/opengovsg/FormSG/pull/2125) +- feat(feature-manager): tear out AggregateStats feature [`#2120`](https://github.com/opengovsg/FormSG/pull/2120) +- feat(feature-manager): remove feature toggles from frontend [`#2118`](https://github.com/opengovsg/FormSG/pull/2118) +- refactor: formFactoryClientService [`#2117`](https://github.com/opengovsg/FormSG/pull/2117) +- chore(deps-dev): bump csv-parse from 4.15.4 to 4.16.0 [`#2124`](https://github.com/opengovsg/FormSG/pull/2124) +- fix(deps): bump aws-sdk from 2.923.0 to 2.924.0 [`#2123`](https://github.com/opengovsg/FormSG/pull/2123) +- chore(deps-dev): bump @types/validator from 13.1.3 to 13.1.4 [`#2122`](https://github.com/opengovsg/FormSG/pull/2122) +- test: loosen URL check in e2e test [`#2119`](https://github.com/opengovsg/FormSG/pull/2119) +- feat: add and call v3 API for retrieving individual admin form [`#2113`](https://github.com/opengovsg/FormSG/pull/2113) +- chore: remove blocking of SP and RP admin updates [`#2114`](https://github.com/opengovsg/FormSG/pull/2114) +- refactor(auth.client): (1) extract email validation and send login otp flow to Typescript [`#2084`](https://github.com/opengovsg/FormSG/pull/2084) +- feat: store only user ID in session [`#1849`](https://github.com/opengovsg/FormSG/pull/1849) +- fix: return storage mode submission version when when retrieving from server [`#2112`](https://github.com/opengovsg/FormSG/pull/2112) +- refactor: replace $resource in angularjs form-api.client.factory.js with typescript FormService [`#1947`](https://github.com/opengovsg/FormSG/pull/1947) +- refactor(ts-migration): ndjsonstream and process-decrypted-content [`#2111`](https://github.com/opengovsg/FormSG/pull/2111) +- chore: remove redundant ValidationOption object properties for short text, long text and number fields [`#2040`](https://github.com/opengovsg/FormSG/pull/2040) +- chore(deps-dev): bump @types/node from 14.17.2 to 14.17.3 [`#2108`](https://github.com/opengovsg/FormSG/pull/2108) +- chore(deps-dev): bump ts-essentials from 7.0.1 to 7.0.2 [`#2110`](https://github.com/opengovsg/FormSG/pull/2110) +- chore(deps-dev): bump @types/express-rate-limit from 5.1.1 to 5.1.2 [`#2109`](https://github.com/opengovsg/FormSG/pull/2109) +- fix(deps): bump zod from 3.0.0 to 3.1.0 [`#2107`](https://github.com/opengovsg/FormSG/pull/2107) +- fix(deps): bump aws-sdk from 2.922.0 to 2.923.0 [`#2105`](https://github.com/opengovsg/FormSG/pull/2105) +- chore: merge release v5.13.0 into develop [`#2102`](https://github.com/opengovsg/FormSG/pull/2102) +- feat: Add webhook support for storage mode attachments [`#1713`](https://github.com/opengovsg/FormSG/pull/1713) +- refactor(auth.client.service): refactor to Typescript (#2132) [`#2066`](https://github.com/opengovsg/FormSG/issues/2066) + #### [v5.13.1](https://github.com/opengovsg/FormSG/compare/v5.13.0...v5.13.1) +> 11 June 2021 + +- chore: bump version to 5.13.1 [`7c87bf3`](https://github.com/opengovsg/FormSG/commit/7c87bf3af16347b630581427bc9223a84846dea3) - feat(verification): up expiry time to 30min [`34b28c8`](https://github.com/opengovsg/FormSG/commit/34b28c87c14e8e0b55276e5a21b6f5473c436e24) #### [v5.13.0](https://github.com/opengovsg/FormSG/compare/v5.12.1...v5.13.0) diff --git a/package-lock.json b/package-lock.json index 320d909aa4..aa7a79dde4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "FormSG", - "version": "5.13.1", + "version": "5.14.0", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 5682bbe094..7a0da50b6c 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "FormSG", "description": "Form Manager for Government", - "version": "5.13.1", + "version": "5.14.0", "homepage": "https://form.gov.sg", "authors": [ "FormSG "