From 84b7d6b033737ac992a1316a65477a5308a2cd78 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 22 Sep 2020 15:38:15 +0800 Subject: [PATCH 01/14] fix(deps): bump http-status-codes from 2.1.2 to 2.1.4 (#354) Bumps [http-status-codes](https://github.com/prettymuchbryce/node-http-status) from 2.1.2 to 2.1.4. - [Release notes](https://github.com/prettymuchbryce/node-http-status/releases) - [Commits](https://github.com/prettymuchbryce/node-http-status/compare/2.1.2...v2.1.4) 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 f0d35de817..6b06184418 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13209,9 +13209,9 @@ } }, "http-status-codes": { - "version": "2.1.2", - "resolved": "https://registry.npmjs.org/http-status-codes/-/http-status-codes-2.1.2.tgz", - "integrity": "sha512-zpZ1nBcoR0j1FLQ7xbXXBy1z/yUfAi+0a5IZBoZnmOseYkaljdzQ17ZeVXFlK23IbLxMJn6aWI0uU92DQQrG0g==" + "version": "2.1.4", + "resolved": "https://registry.npmjs.org/http-status-codes/-/http-status-codes-2.1.4.tgz", + "integrity": "sha512-MZVIsLKGVOVE1KEnldppe6Ij+vmemMuApDfjhVSLzyYP+td0bREEYyAoIw9yFePoBXManCuBqmiNP5FqJS5Xkg==" }, "httpntlm": { "version": "1.6.1", diff --git a/package.json b/package.json index 69b25a260c..445cb925fa 100644 --- a/package.json +++ b/package.json @@ -121,7 +121,7 @@ "glob": "^7.1.2", "has-ansi": "^4.0.0", "helmet": "^4.1.0", - "http-status-codes": "^2.1.2", + "http-status-codes": "^2.1.4", "intl-tel-input": "~12.1.6", "json-stringify-deterministic": "^1.0.1", "json-stringify-safe": "^5.0.1", From 54e8de593ab0f6ece177af68c8e3c13636b07a23 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 22 Sep 2020 15:38:35 +0800 Subject: [PATCH 02/14] chore(deps-dev): bump prettier from 2.1.1 to 2.1.2 (#356) Bumps [prettier](https://github.com/prettier/prettier) from 2.1.1 to 2.1.2. - [Release notes](https://github.com/prettier/prettier/releases) - [Changelog](https://github.com/prettier/prettier/blob/master/CHANGELOG.md) - [Commits](https://github.com/prettier/prettier/compare/2.1.1...2.1.2) 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 6b06184418..a0f3674cc8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -20968,9 +20968,9 @@ "integrity": "sha1-IZMqVJ9eUv/ZqCf1cOBL5iqX2lQ=" }, "prettier": { - "version": "2.1.1", - "resolved": "https://registry.npmjs.org/prettier/-/prettier-2.1.1.tgz", - "integrity": "sha512-9bY+5ZWCfqj3ghYBLxApy2zf6m+NJo5GzmLTpr9FsApsfjriNnS2dahWReHMi7qNPhhHl9SYHJs2cHZLgexNIw==", + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/prettier/-/prettier-2.1.2.tgz", + "integrity": "sha512-16c7K+x4qVlJg9rEbXl7HEGmQyZlG4R9AgP+oHKRMsMsuk8s+ATStlf1NpDqyBI1HpVyfjLOeMhH2LvuNvV5Vg==", "dev": true }, "prettier-linter-helpers": { diff --git a/package.json b/package.json index 445cb925fa..1d473eef17 100644 --- a/package.json +++ b/package.json @@ -228,7 +228,7 @@ "mongodb-memory-server-core": "^6.7.5", "ngrok": "^3.2.7", "optimize-css-assets-webpack-plugin": "^5.0.1", - "prettier": "^2.1.1", + "prettier": "^2.1.2", "regenerator": "^0.14.4", "sinon": "^9.0.3", "stylelint": "^13.3.3", From e4bcc89aecf65eaf4c3f99f4a9f96eef96a8b054 Mon Sep 17 00:00:00 2001 From: Antariksh Mahajan Date: Tue, 22 Sep 2020 15:58:06 +0800 Subject: [PATCH 03/14] refactor: make bounceType required (#360) --- src/types/bounce.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/types/bounce.ts b/src/types/bounce.ts index a01ce8d754..0a180595dd 100644 --- a/src/types/bounce.ts +++ b/src/types/bounce.ts @@ -8,9 +8,7 @@ export type ISingleBounce = | { email: string hasBounced: true - // TODO (private #44): this key should be required, - // but is currently optional for backwards compatibility reasons. - bounceType?: BounceType + bounceType: BounceType } | { email: string From f7770be98bbea008c02ead1021af671adadf37db Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 23 Sep 2020 22:11:05 +0800 Subject: [PATCH 04/14] chore(deps-dev): bump @types/validator from 13.0.0 to 13.1.0 (#366) Bumps [@types/validator](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/validator) from 13.0.0 to 13.1.0. - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/validator) 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 d81b447b0d..34e4bd178b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4957,9 +4957,9 @@ "dev": true }, "@types/validator": { - "version": "13.0.0", - "resolved": "https://registry.npmjs.org/@types/validator/-/validator-13.0.0.tgz", - "integrity": "sha512-WAy5txG7aFX8Vw3sloEKp5p/t/Xt8jD3GRD9DacnFv6Vo8ubudAsRTXgxpQwU0mpzY/H8U4db3roDuCMjShBmw==", + "version": "13.1.0", + "resolved": "https://registry.npmjs.org/@types/validator/-/validator-13.1.0.tgz", + "integrity": "sha512-gHUHI6pJaANIO2r6WcbT7+WMgbL9GZooR4tWpuBOETpDIqFNxwaJluE+6rj6VGYe8k6OkfhbHz2Fkm8kl06Igw==", "dev": true }, "@types/yargs": { diff --git a/package.json b/package.json index 08c35bacf6..d49899e349 100644 --- a/package.json +++ b/package.json @@ -192,7 +192,7 @@ "@types/triple-beam": "^1.3.2", "@types/uid-generator": "^2.0.2", "@types/uuid": "^8.0.0", - "@types/validator": "^13.0.0", + "@types/validator": "^13.1.0", "@typescript-eslint/eslint-plugin": "^4.0.1", "@typescript-eslint/parser": "^4.0.0", "auto-changelog": "^2.2.0", From bd7d9742aa2513fa33afffaa80c25bfc3f4269fd Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 24 Sep 2020 15:31:39 +0800 Subject: [PATCH 05/14] chore(deps-dev): bump auto-changelog from 2.2.0 to 2.2.1 (#365) Bumps [auto-changelog](https://github.com/CookPete/auto-changelog) from 2.2.0 to 2.2.1. - [Release notes](https://github.com/CookPete/auto-changelog/releases) - [Changelog](https://github.com/CookPete/auto-changelog/blob/master/CHANGELOG.md) - [Commits](https://github.com/CookPete/auto-changelog/compare/v2.2.0...v2.2.1) 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 34e4bd178b..a98d2d4ace 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5843,9 +5843,9 @@ "dev": true }, "auto-changelog": { - "version": "2.2.0", - "resolved": "https://registry.npmjs.org/auto-changelog/-/auto-changelog-2.2.0.tgz", - "integrity": "sha512-RBY0hhVNXstggOQL0SyUaCfSiVD11CVXEHvDwB+mEt9UnhXPqhdpQ7nIVGDEog7JopTdYbydULLLt6v//qrWjw==", + "version": "2.2.1", + "resolved": "https://registry.npmjs.org/auto-changelog/-/auto-changelog-2.2.1.tgz", + "integrity": "sha512-XlykJfZrXlWUAADBqGoN1elmntrRcx7oEymyYB3NRPEZxv0TfYHfivmwzejUMnwAdXKCgbQPo7GV5ULs3jwpfw==", "dev": true, "requires": { "commander": "^5.0.0", diff --git a/package.json b/package.json index d49899e349..a236de0954 100644 --- a/package.json +++ b/package.json @@ -195,7 +195,7 @@ "@types/validator": "^13.1.0", "@typescript-eslint/eslint-plugin": "^4.0.1", "@typescript-eslint/parser": "^4.0.0", - "auto-changelog": "^2.2.0", + "auto-changelog": "^2.2.1", "axios-mock-adapter": "^1.18.1", "babel-loader": "^8.0.5", "concurrently": "^5.3.0", From 015f3196ce274ea60861a0bc51e5df40a2f6b496 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 24 Sep 2020 17:54:37 +0800 Subject: [PATCH 06/14] fix(deps): bump @sentry/integrations from 5.22.3 to 5.24.2 (#373) Bumps [@sentry/integrations](https://github.com/getsentry/sentry-javascript) from 5.22.3 to 5.24.2. - [Release notes](https://github.com/getsentry/sentry-javascript/releases) - [Changelog](https://github.com/getsentry/sentry-javascript/blob/master/CHANGELOG.md) - [Commits](https://github.com/getsentry/sentry-javascript/compare/5.22.3...5.24.2) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- package-lock.json | 26 +++++++++++++++++++++----- package.json | 2 +- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index a98d2d4ace..f3e7935e81 100644 --- a/package-lock.json +++ b/package-lock.json @@ -4191,14 +4191,30 @@ } }, "@sentry/integrations": { - "version": "5.22.3", - "resolved": "https://registry.npmjs.org/@sentry/integrations/-/integrations-5.22.3.tgz", - "integrity": "sha512-Fx6h8DTDvUpEOymx8Wi49LBdVcNYHwaI6NqApm1qVU9qn/I50Q29KWoZTCGBjBwmkJud+DOAHWYWoU2qRrIvcQ==", + "version": "5.24.2", + "resolved": "https://registry.npmjs.org/@sentry/integrations/-/integrations-5.24.2.tgz", + "integrity": "sha512-b0upZS+xvONwxkLL6apSSgseR1e6dtq7wAGHefnPa5ckTwIoUkboL/dqiTNmFj1xXnWb87WDX1ZcIx7nfEqw6A==", "requires": { - "@sentry/types": "5.22.3", - "@sentry/utils": "5.22.3", + "@sentry/types": "5.24.2", + "@sentry/utils": "5.24.2", "localforage": "1.8.1", "tslib": "^1.9.3" + }, + "dependencies": { + "@sentry/types": { + "version": "5.24.2", + "resolved": "https://registry.npmjs.org/@sentry/types/-/types-5.24.2.tgz", + "integrity": "sha512-HcOK00R0tQG5vzrIrqQ0jC28+z76jWSgQCzXiessJ5SH/9uc6NzdO7sR7K8vqMP2+nweCHckFohC8G0T1DLzuQ==" + }, + "@sentry/utils": { + "version": "5.24.2", + "resolved": "https://registry.npmjs.org/@sentry/utils/-/utils-5.24.2.tgz", + "integrity": "sha512-oPGde4tNEDHKk0Cg9q2p0qX649jLDUOwzJXHKpd0X65w3A6eJByDevMr8CSzKV9sesjrUpxqAv6f9WWlz185tA==", + "requires": { + "@sentry/types": "5.24.2", + "tslib": "^1.9.3" + } + } } }, "@sentry/minimal": { diff --git a/package.json b/package.json index a236de0954..547206f470 100644 --- a/package.json +++ b/package.json @@ -69,7 +69,7 @@ "@opengovsg/ng-file-upload": "^12.2.14", "@opengovsg/spcp-auth-client": "^1.3.5", "@sentry/browser": "^5.22.3", - "@sentry/integrations": "^5.22.3", + "@sentry/integrations": "^5.24.2", "@stablelib/base64": "^1.0.0", "JSONStream": "^1.3.5", "angular": "~1.8.0", From b884c6f52c0881de6a2b1a2739f54303eb14fb65 Mon Sep 17 00:00:00 2001 From: shuli-ogp <63710093+shuli-ogp@users.noreply.github.com> Date: Thu, 24 Sep 2020 21:02:56 +0800 Subject: [PATCH 07/14] fix(deps): upgrade node, alpine and chromium (#363) * fix(deps): upgrade node, alpine and chromium * fix(deps): downgrade puppeteer-core to 3.1.0 to correspond to chromium 83 * fix: update package-lock * chore: upgrade travis node version to 14 to be in sync * chore: revert to using node12.18.4 as vulnerability has been fixed in that version * chore: static version for puppeteer-core Co-authored-by: Kar Rui Lau * chore: python3 instead of python2 * docs: update troubleshooting guide * chore: update package-lock Co-authored-by: Kar Rui Lau --- Dockerfile.development | 9 +++-- Dockerfile.production | 26 ++++++------ docs/TROUBLESHOOTING.md | 5 +-- package-lock.json | 88 +++++++---------------------------------- package.json | 2 +- 5 files changed, 38 insertions(+), 92 deletions(-) diff --git a/Dockerfile.development b/Dockerfile.development index bf52cc744f..5957da139a 100644 --- a/Dockerfile.development +++ b/Dockerfile.development @@ -1,4 +1,4 @@ -FROM node:12.18.0-alpine3.11 +FROM node:12.18.4-alpine3.12 LABEL maintainer=FormSG WORKDIR /opt/formsg @@ -8,12 +8,15 @@ ENV NODE_ENV=development RUN apk update && apk upgrade && \ # Build dependencies for node_modules apk add --virtual native-deps \ - g++ gcc libgcc libstdc++ linux-headers autoconf automake make nasm python git curl \ + # Python version must be specified starting in alpine3.12 + g++ gcc libgcc libstdc++ linux-headers autoconf automake make nasm python3 git curl \ # Runtime dependencies + # Note that each alpine version supports a specific version of chromium # Note that chromium and puppeteer-core are released together and it is the only version # that is guaranteed to work. Upgrades must be done in lockstep. # https://github.com/puppeteer/puppeteer/blob/master/docs/troubleshooting.md#running-on-alpine - chromium=81.0.4044.113-r0 \ + # https://www.npmjs.com/package/puppeteer-core?activeTab=versions for corresponding versions + chromium=83.0.4103.116-r0 \ nss \ freetype \ freetype-dev \ diff --git a/Dockerfile.production b/Dockerfile.production index f6a23ef96c..069aa31e45 100644 --- a/Dockerfile.production +++ b/Dockerfile.production @@ -1,8 +1,9 @@ -FROM node:12.18.0-alpine3.11 AS node-modules-builder +FROM node:12.18.4-alpine3.12 AS node-modules-builder # node-modules-builder stage installs/compiles the node_modules folder +# Python version must be specified starting in alpine3.12 RUN apk update && apk upgrade && \ apk --no-cache add --virtual native-deps \ - g++ gcc libgcc libstdc++ linux-headers autoconf automake make nasm python git curl && \ + g++ gcc libgcc libstdc++ linux-headers autoconf automake make nasm python3 git curl && \ npm install --quiet node-gyp -g WORKDIR /opt/formsg COPY package* /opt/formsg/ @@ -10,7 +11,7 @@ RUN npm ci COPY . /opt/formsg # This stage builds the final container -FROM node:12.18.0-alpine3.11 +FROM node:12.18.4-alpine3.12 LABEL maintainer=FormSG WORKDIR /opt/formsg @@ -19,17 +20,20 @@ COPY --from=node-modules-builder /opt/formsg /opt/formsg # Install chromium from official docs # https://github.com/puppeteer/puppeteer/blob/master/docs/troubleshooting.md#running-on-alpine +# Note that each alpine version supports a specific version of chromium # Note that chromium and puppeteer-core are released together and it is the only version # that is guaranteed to work. Upgrades must be done in lockstep. +# https://www.npmjs.com/package/puppeteer-core?activeTab=versions for corresponding versions + RUN apk add --no-cache \ - chromium=81.0.4044.113-r0 \ - nss \ - freetype \ - freetype-dev \ - harfbuzz \ - ca-certificates \ - ttf-freefont \ - tini + chromium=83.0.4103.116-r0 \ + nss \ + freetype \ + freetype-dev \ + harfbuzz \ + ca-certificates \ + ttf-freefont \ + tini # This package is needed to render Chinese characters in autoreply PDFs RUN echo @edge http://nl.alpinelinux.org/alpine/edge/testing >> /etc/apk/repositories && apk add wqy-zenhei@edge ENV CHROMIUM_BIN=/usr/bin/chromium-browser diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index 06abbf6300..1bb4bed1ce 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -6,10 +6,6 @@ A list of common issues that developers face and how to resolve them. This could happen if node modules were compiled with a different version of node, or if node modules fail to compile due to other configuration errors. -### [Configuration Errors](https://stackoverflow.com/questions/21656420/failed-to-load-c-bson-extension) - -Some modules such as `node-gyp` require Python 2.x and if your system's Python points to 3.x, it will fail to compile bson, without warning. Fix this by setting Python 2.x to be the default on your system or ensuring that the Python global key in your npm config points to the 2.x executable on your system. - ### [Node Versioning Error](https://stackoverflow.com/questions/28486891/uncaught-error-module-did-not-self-register) Run the following commands to set the node version and then re-install the node modules: @@ -21,4 +17,5 @@ npm install ``` ## JavaScript out of memory error + On your Docker application, go to Preferences > Resources and increase the amount of memory allocated for Docker. diff --git a/package-lock.json b/package-lock.json index f3e7935e81..216638c3a8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10190,11 +10190,6 @@ "integrity": "sha512-fYXbFSeilT7bnKWFi4OERSPHdtaEoDGn4aUhV5Nly6/I+Tp6JZ/6Icmd7LVIF5euyodGpxz2e/bfUmDnIdSIDw==", "dev": true }, - "devtools-protocol": { - "version": "0.0.781568", - "resolved": "https://registry.npmjs.org/devtools-protocol/-/devtools-protocol-0.0.781568.tgz", - "integrity": "sha512-9Uqnzy6m6zEStluH9iyJ3iHyaQziFnMnLeC8vK0eN6smiJmIx7+yB64d67C2lH/LZra+5cGscJAJsNXO+MdPMg==" - }, "dicer": { "version": "0.3.0", "resolved": "https://registry.npmjs.org/dicer/-/dicer-0.3.0.tgz", @@ -11763,11 +11758,11 @@ }, "dependencies": { "debug": { - "version": "4.1.1", - "resolved": "https://registry.npmjs.org/debug/-/debug-4.1.1.tgz", - "integrity": "sha512-pYAIzeRo8J6KPEaJ0VWOh5Pzkbw/RetuzehGM7QRRX5he4fPHx2rdKMB256ehJCkX+XRQm16eZLqLNS8RSZXZw==", + "version": "4.2.0", + "resolved": "https://registry.npmjs.org/debug/-/debug-4.2.0.tgz", + "integrity": "sha512-IX2ncY78vDTjZMFUdmsvIRFY2Cf4FnD0wRs+nQwJU8Lu99/tPFdb0VybiiMTPe3I6rQmwsqQqRBvxU+bZ/I8sg==", "requires": { - "ms": "^2.1.1" + "ms": "2.1.2" } } } @@ -13259,11 +13254,11 @@ }, "dependencies": { "debug": { - "version": "4.1.1", - "resolved": "https://registry.npmjs.org/debug/-/debug-4.1.1.tgz", - "integrity": "sha512-pYAIzeRo8J6KPEaJ0VWOh5Pzkbw/RetuzehGM7QRRX5he4fPHx2rdKMB256ehJCkX+XRQm16eZLqLNS8RSZXZw==", + "version": "4.2.0", + "resolved": "https://registry.npmjs.org/debug/-/debug-4.2.0.tgz", + "integrity": "sha512-IX2ncY78vDTjZMFUdmsvIRFY2Cf4FnD0wRs+nQwJU8Lu99/tPFdb0VybiiMTPe3I6rQmwsqQqRBvxU+bZ/I8sg==", "requires": { - "ms": "^2.1.1" + "ms": "2.1.2" } } } @@ -21258,16 +21253,14 @@ "integrity": "sha512-XRsRjdf+j5ml+y/6GKHPZbrF/8p2Yga0JPtdqTIY2Xe5ohJPD9saDJJLPvp9+NSBprVvevdXZybnj2cv8OEd0A==" }, "puppeteer-core": { - "version": "5.2.1", - "resolved": "https://registry.npmjs.org/puppeteer-core/-/puppeteer-core-5.2.1.tgz", - "integrity": "sha512-gLjEOrzwgcnwRH+sm4hS1TBqe2/DN248nRb2hYB7+lZ9kCuLuACNvuzlXILlPAznU3Ob+mEvVEBDcLuFa0zq3g==", + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/puppeteer-core/-/puppeteer-core-3.1.0.tgz", + "integrity": "sha512-o0dlA3g+Dzc8mrKvCrlmn79upNhLfTQwtnPbS4hxuH6tWOwFFEbet8WHtYjQbUPZOqIt0GmoyM93VQKm6ogO+Q==", "requires": { "debug": "^4.1.0", - "devtools-protocol": "0.0.781568", "extract-zip": "^2.0.0", "https-proxy-agent": "^4.0.0", "mime": "^2.0.3", - "pkg-dir": "^4.2.0", "progress": "^2.0.1", "proxy-from-env": "^1.0.0", "rimraf": "^3.0.2", @@ -21277,68 +21270,17 @@ }, "dependencies": { "debug": { - "version": "4.1.1", - "resolved": "https://registry.npmjs.org/debug/-/debug-4.1.1.tgz", - "integrity": "sha512-pYAIzeRo8J6KPEaJ0VWOh5Pzkbw/RetuzehGM7QRRX5he4fPHx2rdKMB256ehJCkX+XRQm16eZLqLNS8RSZXZw==", - "requires": { - "ms": "^2.1.1" - } - }, - "find-up": { - "version": "4.1.0", - "resolved": "https://registry.npmjs.org/find-up/-/find-up-4.1.0.tgz", - "integrity": "sha512-PpOwAdQ/YlXQ2vj8a3h8IipDuYRi3wceVQQGYWxNINccq40Anw7BlsEXCMbt1Zt+OLA6Fq9suIpIWD0OsnISlw==", - "requires": { - "locate-path": "^5.0.0", - "path-exists": "^4.0.0" - } - }, - "locate-path": { - "version": "5.0.0", - "resolved": "https://registry.npmjs.org/locate-path/-/locate-path-5.0.0.tgz", - "integrity": "sha512-t7hw9pI+WvuwNJXwk5zVHpyhIqzg2qTlklJOf0mVxGSbe3Fp2VieZcduNYjaLDoy6p9uGpQEGWG87WpMKlNq8g==", + "version": "4.2.0", + "resolved": "https://registry.npmjs.org/debug/-/debug-4.2.0.tgz", + "integrity": "sha512-IX2ncY78vDTjZMFUdmsvIRFY2Cf4FnD0wRs+nQwJU8Lu99/tPFdb0VybiiMTPe3I6rQmwsqQqRBvxU+bZ/I8sg==", "requires": { - "p-locate": "^4.1.0" + "ms": "2.1.2" } }, "mime": { "version": "2.4.6", "resolved": "https://registry.npmjs.org/mime/-/mime-2.4.6.tgz", "integrity": "sha512-RZKhC3EmpBchfTGBVb8fb+RL2cWyw/32lshnsETttkBAyAUXSGHxbEJWWRXc751DrIxG1q04b8QwMbAwkRPpUA==" - }, - "p-limit": { - "version": "2.3.0", - "resolved": "https://registry.npmjs.org/p-limit/-/p-limit-2.3.0.tgz", - "integrity": "sha512-//88mFWSJx8lxCzwdAABTJL2MyWB12+eIY7MDL2SqLmAkeKU9qxRvWuSyTjm3FUmpBEMuFfckAIqEaVGUDxb6w==", - "requires": { - "p-try": "^2.0.0" - } - }, - "p-locate": { - "version": "4.1.0", - "resolved": "https://registry.npmjs.org/p-locate/-/p-locate-4.1.0.tgz", - "integrity": "sha512-R79ZZ/0wAxKGu3oYMlz8jy/kbhsNrS7SKZ7PxEHBgJ5+F2mtFW2fK2cOtBh1cHYkQsbzFV7I+EoRKe6Yt0oK7A==", - "requires": { - "p-limit": "^2.2.0" - } - }, - "p-try": { - "version": "2.2.0", - "resolved": "https://registry.npmjs.org/p-try/-/p-try-2.2.0.tgz", - "integrity": "sha512-R4nPAVTAU0B9D35/Gk3uJf/7XYbQcyohSKdvAxIRSNghFl4e71hVoGnBNQz9cWaXxO2I10KTC+3jMdvvoKw6dQ==" - }, - "path-exists": { - "version": "4.0.0", - "resolved": "https://registry.npmjs.org/path-exists/-/path-exists-4.0.0.tgz", - "integrity": "sha512-ak9Qy5Q7jYb2Wwcey5Fpvg2KoAc/ZIhLSLOSBmRmygPsGwkVVt0fZa0qrtMz+m6tJTAHfZQ8FnmB4MG4LWy7/w==" - }, - "pkg-dir": { - "version": "4.2.0", - "resolved": "https://registry.npmjs.org/pkg-dir/-/pkg-dir-4.2.0.tgz", - "integrity": "sha512-HRDzbaKjC+AOWVXxAU/x54COGeIv9eb+6CkDSQoNTt4XyWoIJvuPsXizxu/Fr23EiekbtZwmh1IcIG/l/a10GQ==", - "requires": { - "find-up": "^4.0.0" - } } } }, diff --git a/package.json b/package.json index 547206f470..30c8a17d76 100644 --- a/package.json +++ b/package.json @@ -145,7 +145,7 @@ "opossum": "^5.0.1", "promise-retry": "^2.0.1", "proxyquire": "^2.1.0", - "puppeteer-core": "^5.2.1", + "puppeteer-core": "3.1.0", "rimraf": "^3.0.2", "selectize": "0.12.4", "slick-carousel": "1.8.1", From 43092b1f08d43a11a23a2f7b02bef195d75a1d0c Mon Sep 17 00:00:00 2001 From: Jiayee Lim Date: Fri, 25 Sep 2020 14:57:36 +0800 Subject: [PATCH 08/14] feat: implement email domain validation with unit tests (#143) * feat: implement email domain validation with unit tests * refactor: refactor schema validation and separate validation directives from controllers * test: add tests for EmailValidator for cases where isVerifiable is true * chore: run prettier write * refactor: Use spyOn mockImplementation instead of manual __mocks__ * fix: hoist directives * style: disable toggle when email isVerifiable is false and update tooltip for email domain fields * feat: use user's email domain and @agency.gov.sg as allowed email domains placeholder and migrate email validation tests * fix: update label for email domain textarea field, add horizontal line below the field and prevent error when textarea value is empty (cannot call .split on empty string) * fix: set hasAllowedEmailDomains to false when isVerifiable is false pre-save * fix: fix email schema pre-save hook and add directive to intercept isVerifiable save * feat: trigger toast when isVerifiable is switched off * fix: hide textarea for email domains if switch is off * fix: resolve error TS2339: Property 'hasAllowedEmailDomains' does not exist on type 'IEmailFieldSchema'. * fix: remove unneeded pre-save interceptor for hasAllowedEmailDomains * fix: fix directive logic and refactor label names so that they follow the mockups * fix: update toggle name in test as well * refactor: update line CSS class name, use import from instead of import require and add a warning to prevent changes to onclick="" * fix: fix CSS selector clash and handle Auth.getUser() if it's null * fix: fix error TS2320: Interface 'IEmailFieldSchema' cannot simultaneously extend types 'IEmailField' and 'IFieldSchema'. * feat: implement dynamic tooltip text depending on emptiness check for allowedEmailDomainsFromText * refactor: rename vm.tooltipText to vm.tooltipHtml for more clarity --- src/app/models/field/emailField.ts | 25 +++ .../validators/EmailValidator.class.js | 17 +- src/public/main.js | 3 + .../edit-fields-modal.client.controller.js | 37 +++++ .../modules/forms/admin/css/edit-form.css | 6 + ...s-verifiable-save-interceptor.directive.js | 25 +++ ...lidate-email-domain-from-text.directive.js | 31 ++++ .../admin/views/edit-fields.client.modal.html | 91 ++++++++++- .../field-email.client.view.html | 5 + .../validate-email-domain.client.directive.js | 23 +++ .../viewmodels/Fields/EmailField.class.js | 2 + src/types/field/emailField.ts | 2 + tests/end-to-end/helpers/util.js | 2 +- .../field-validation/email-validation.spec.js | 47 ------ .../field-validation/email-validation.spec.ts | 149 ++++++++++++++++++ 15 files changed, 413 insertions(+), 52 deletions(-) create mode 100644 src/public/modules/forms/admin/directives/is-verifiable-save-interceptor.directive.js create mode 100644 src/public/modules/forms/admin/directives/validate-email-domain-from-text.directive.js create mode 100644 src/public/modules/forms/base/directives/validate-email-domain.client.directive.js delete mode 100644 tests/unit/backend/utils/field-validation/email-validation.spec.js create mode 100644 tests/unit/backend/utils/field-validation/email-validation.spec.ts diff --git a/src/app/models/field/emailField.ts b/src/app/models/field/emailField.ts index a4137052e3..7d00543b91 100644 --- a/src/app/models/field/emailField.ts +++ b/src/app/models/field/emailField.ts @@ -1,3 +1,4 @@ +import { isEmpty } from 'lodash' import { Schema } from 'mongoose' import { IEmailFieldSchema, ResponseMode } from '../../../types' @@ -39,6 +40,30 @@ const createEmailFieldSchema = () => { type: Boolean, default: false, }, + hasAllowedEmailDomains: { + type: Boolean, + default: false, + }, + allowedEmailDomains: { + type: [ + { + type: String, + trim: true, + match: [/.+\..+/, 'There are one or more invalid email domains.'], + }, + ], + // If there allowedEmailDomains is empty, then all email domains should be allowed. + default: [], + validate: { + validator: (emailDomains: string[]) => { + return ( + isEmpty(emailDomains) || + new Set(emailDomains).size === emailDomains.length + ) + }, + message: 'There are one or more duplicate email domains.', + }, + }, }) // PDF response not allowed if autoreply is set in encrypted forms. If diff --git a/src/app/utils/field-validation/validators/EmailValidator.class.js b/src/app/utils/field-validation/validators/EmailValidator.class.js index a03cf8ef85..941f924b43 100644 --- a/src/app/utils/field-validation/validators/EmailValidator.class.js +++ b/src/app/utils/field-validation/validators/EmailValidator.class.js @@ -4,7 +4,22 @@ const validator = require('validator') class EmailValidator extends BaseFieldValidator { _isFilledAnswerValid() { const { answer } = this.response - const isValid = validator.isEmail(String(answer)) + const { + isVerifiable, + hasAllowedEmailDomains, + allowedEmailDomains, + } = this.formField + const emailAddress = String(answer) + let isValid = validator.isEmail(emailAddress) + if ( + isValid && + isVerifiable && + hasAllowedEmailDomains && + allowedEmailDomains.length + ) { + const emailDomain = '@' + emailAddress.split('@').pop() + isValid = isValid && new Set(allowedEmailDomains).has(emailDomain) + } this.logIfInvalid(isValid, 'EmailValidator._isFilledAnswerValid') return isValid } diff --git a/src/public/main.js b/src/public/main.js index d5fc80ce9a..fcaa8f8ec9 100644 --- a/src/public/main.js +++ b/src/public/main.js @@ -187,6 +187,8 @@ require('./modules/forms/admin/controllers/pop-up-modal.client.controller.js') // forms admin directives require('./modules/forms/admin/directives/settings-form.client.directive.js') require('./modules/forms/admin/directives/edit-form.client.directive.js') +require('./modules/forms/admin/directives/is-verifiable-save-interceptor.directive.js') +require('./modules/forms/admin/directives/validate-email-domain-from-text.directive.js') require('./modules/forms/admin/directives/view-feedback.client.directive.js') require('./modules/forms/admin/directives/configure-form.client.directive.js') require('./modules/forms/admin/directives/configure-mobile.client.directive.js') @@ -248,6 +250,7 @@ require('./modules/forms/base/directives/field.client.directive.js') require('./modules/forms/base/directives/iframe-onload.client.directive.js') require('./modules/forms/base/directives/rating-stars.client.directive.js') require('./modules/forms/base/directives/validate-checkbox.client.directive.js') +require('./modules/forms/base/directives/validate-email-domain.client.directive.js') require('./modules/forms/base/directives/validate-nric.client.directive.js') require('./modules/forms/base/directives/validate-url.client.directive.js') require('./modules/forms/base/directives/ng-intl-tel-input.js') 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 2ddb1afd91..a108fab5ce 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 @@ -17,6 +17,7 @@ const DATE_VALIDATION_OPTIONS = { angular .module('forms') .controller('EditFieldsModalController', [ + '$scope', '$uibModalInstance', 'externalScope', 'responseModeEnum', @@ -27,10 +28,12 @@ angular '$q', 'Betas', 'Auth', + '$state', EditFieldsModalController, ]) function EditFieldsModalController( + $scope, $uibModalInstance, externalScope, responseModeEnum, @@ -41,6 +44,7 @@ function EditFieldsModalController( $q, Betas, Auth, + $state, ) { const vm = this @@ -57,6 +61,26 @@ function EditFieldsModalController( vm.field.manualOptions = vm.field.fieldOptions } + // Serialize allowed email domains + vm.user = Auth.getUser() || $state.go('signin') + if (vm.field.fieldType === 'email') { + const userEmailDomain = '@' + vm.user.email.split('@').pop() + vm.field.allowedEmailDomainsPlaceholder = `${userEmailDomain}\n@agency.gov.sg` + if (vm.field.allowedEmailDomains.length > 0) { + vm.field.allowedEmailDomainsFromText = vm.field.allowedEmailDomains.join( + '\n', + ) + } + $scope.$watch('vm.field.allowedEmailDomainsFromText', (newValue) => { + if (newValue) { + vm.tooltipHtml = 'e.g. @mom.gov.sg, @moe.gov.sg' + } else { + vm.tooltipHtml = + 'e.g. @mom.gov.sg, @moe.gov.sg
OTP verification needs to be activated first.' + } + }) + } + // Set Validation Options on older paragraph fields - This ensures backward compatibility // TODO: Is this safe to remove? vm.enforceCompatibility = function (field) { @@ -418,6 +442,19 @@ function EditFieldsModalController( } else { field.fieldOptions = field.manualOptions } + + // Deserialize allowed email domains + if (field.fieldType === 'email') { + if (!field.allowedEmailDomainsFromText) { + field.allowedEmailDomains = [] + } else { + field.allowedEmailDomains = field.allowedEmailDomainsFromText + .split('\n') + .map((s) => s.trim()) + .filter((s) => s) + } + } + // set total attachment size left if (field.fieldType === 'attachment') { Attachment.attachmentsTotal = diff --git a/src/public/modules/forms/admin/css/edit-form.css b/src/public/modules/forms/admin/css/edit-form.css index 1f1ef33a27..529a16780d 100644 --- a/src/public/modules/forms/admin/css/edit-form.css +++ b/src/public/modules/forms/admin/css/edit-form.css @@ -1253,3 +1253,9 @@ a.modal-cancel-btn:hover { font-size: 24px; cursor: pointer; } + +.edit-fields-line { + border: solid 1px #9b9b9b; + height: 2px; + margin: 30px 0; +} diff --git a/src/public/modules/forms/admin/directives/is-verifiable-save-interceptor.directive.js b/src/public/modules/forms/admin/directives/is-verifiable-save-interceptor.directive.js new file mode 100644 index 0000000000..013c49b77c --- /dev/null +++ b/src/public/modules/forms/admin/directives/is-verifiable-save-interceptor.directive.js @@ -0,0 +1,25 @@ +'use strict' + +angular + .module('forms') + .directive('isVerifiableSaveInterceptor', [ + 'Toastr', + isVerifiableSaveInterceptor, + ]) + +function isVerifiableSaveInterceptor(Toastr) { + return { + require: 'ngModel', + link: (scope, elem, attr, ngModel) => { + ngModel.$parsers.push((inputValue) => { + if (!inputValue) { + Toastr.error( + 'Turn on OTP verification again if you wish to restrict email domains.', + ) + } + scope.vm.field.hasAllowedEmailDomains = false + return inputValue + }) + }, + } +} diff --git a/src/public/modules/forms/admin/directives/validate-email-domain-from-text.directive.js b/src/public/modules/forms/admin/directives/validate-email-domain-from-text.directive.js new file mode 100644 index 0000000000..4db7718a63 --- /dev/null +++ b/src/public/modules/forms/admin/directives/validate-email-domain-from-text.directive.js @@ -0,0 +1,31 @@ +'use strict' + +angular + .module('forms') + .directive('validateEmailDomainFromText', validateEmailDomainFromText) + +function validateEmailDomainFromText() { + return { + restrict: 'A', + require: 'ngModel', + link: (scope, elem, attr, ngModel) => { + ngModel.$validators.emailDomainFromTextValidator = ( + allowedEmailDomainsFromText, + ) => { + if (!allowedEmailDomainsFromText) { + return true + } + // The last `filter` chain is needed to remove falsey values from the array e.g. ['']. + const emailDomains = allowedEmailDomainsFromText + .split('\n') + .map((s) => s.trim()) + .filter((s) => s) + if (emailDomains.length === 0) return true + const validDedupedDomainsSet = new Set( + emailDomains.filter((s) => s.match(/@.+\..+/)), + ) + return validDedupedDomainsSet.size === emailDomains.length + } + }, + } +} 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 8d518d7997..af1915fabd 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 @@ -565,7 +565,7 @@


- Verified? + OTP verification ng-class="vm.field.isVerifiable? 'toggle-selector-on' : ''" onclick="" > - +
-

+
+
+
+
+ Restrict email domains +
+
+ + + +
+
+
+ +
+
+
+
Domains allowed
+
+
+
+ +
+
+
+
+
+
+ + + You may have duplicate or invalid email domains. Please + check. + +
+
+
+
+
+ +
+
diff --git a/src/public/modules/forms/base/componentViews/field-email.client.view.html b/src/public/modules/forms/base/componentViews/field-email.client.view.html index 39ed01292b..97d21e75a6 100644 --- a/src/public/modules/forms/base/componentViews/field-email.client.view.html +++ b/src/public/modules/forms/base/componentViews/field-email.client.view.html @@ -36,6 +36,7 @@ ng-required="vm.field.required" ng-disabled="vm.field.disabled" ng-pattern="vm.pattern" + validate-email-domain ng-model-options="{ allowInvalid: true }" ng-keyup="vm.forms.myForm[(vm.field._id || 'defaultID')].$setTouched()" placeholder="me@example.com" @@ -59,6 +60,10 @@ Please enter a valid email.
+
+ + Not an allowed email domain +
Please verify your email. diff --git a/src/public/modules/forms/base/directives/validate-email-domain.client.directive.js b/src/public/modules/forms/base/directives/validate-email-domain.client.directive.js new file mode 100644 index 0000000000..f4caabca69 --- /dev/null +++ b/src/public/modules/forms/base/directives/validate-email-domain.client.directive.js @@ -0,0 +1,23 @@ +'use strict' + +angular.module('forms').directive('validateEmailDomain', validateEmailDomain) + +function validateEmailDomain() { + return { + restrict: 'A', + require: 'ngModel', + link: (scope, elem, attr, ngModel) => { + const allowedEmailDomains = + scope.vm.field.isVerifiable && scope.vm.field.hasAllowedEmailDomains + ? new Set(scope.vm.field.allowedEmailDomains) + : new Set() + ngModel.$validators.emailDomainValidator = (emailAddress) => { + if (allowedEmailDomains.size) { + const emailDomain = emailAddress.split('@').pop() + return allowedEmailDomains.has('@' + emailDomain) + } + return true + } + }, + } +} diff --git a/src/public/modules/forms/viewmodels/Fields/EmailField.class.js b/src/public/modules/forms/viewmodels/Fields/EmailField.class.js index 8839141b0f..05787f109d 100644 --- a/src/public/modules/forms/viewmodels/Fields/EmailField.class.js +++ b/src/public/modules/forms/viewmodels/Fields/EmailField.class.js @@ -12,6 +12,8 @@ class EmailField extends VerifiableField { includeFormSummary: false, } fieldData.isVerifiable = false + fieldData.hasAllowedEmailDomains = false + fieldData.allowedEmailDomains = [] return fieldData } } diff --git a/src/types/field/emailField.ts b/src/types/field/emailField.ts index 0e076d3e35..a531d42c83 100644 --- a/src/types/field/emailField.ts +++ b/src/types/field/emailField.ts @@ -11,6 +11,8 @@ export type AutoReplyOptions = { export interface IEmailField extends IField { autoReplyOptions: AutoReplyOptions isVerifiable: boolean + hasAllowedEmailDomains: boolean + allowedEmailDomains: string[] } export interface IEmailFieldSchema extends IEmailField, IFieldSchema { diff --git a/tests/end-to-end/helpers/util.js b/tests/end-to-end/helpers/util.js index f8e76f56fe..52b153025f 100644 --- a/tests/end-to-end/helpers/util.js +++ b/tests/end-to-end/helpers/util.js @@ -464,7 +464,7 @@ async function createBasicField(t, field) { switch (field.fieldType) { case 'email': if (field.isVerifiable) { - await t.click(editFieldModal.getToggle('Verified')) + await t.click(editFieldModal.getToggle('OTP verification')) } break case 'radiobutton': diff --git a/tests/unit/backend/utils/field-validation/email-validation.spec.js b/tests/unit/backend/utils/field-validation/email-validation.spec.js deleted file mode 100644 index ee59778beb..0000000000 --- a/tests/unit/backend/utils/field-validation/email-validation.spec.js +++ /dev/null @@ -1,47 +0,0 @@ -const validateField = require('../../../../../dist/backend/app/utils/field-validation') - -describe('Email field validation', () => { - it('should allow valid emails', () => { - const formField = { - _id: 'abc123', - fieldType: 'email', - } - const response = { - _id: 'abc123', - fieldType: 'email', - answer: 'valid@email.com', - } - const testFunc = () => validateField('formId', formField, response) - expect(testFunc).not.toThrow() - }) - - it('should disallow invalid emails', () => { - const formField = { - _id: 'abc123', - fieldType: 'email', - } - const response = { - _id: 'abc123', - fieldType: 'email', - answer: 'invalidemail.com', - } - const testFunc = () => validateField('formId', formField, response) - expect(testFunc).toThrow() - }) - - it('should allow empty answer for required logic field that is not visible', () => { - const formField = { - _id: 'abc123', - fieldType: 'email', - required: true, - } - const response = { - _id: 'abc123', - fieldType: 'email', - isVisible: false, - answer: '', - } - const testFunc = () => validateField('formId', formField, response) - expect(testFunc).not.toThrow() - }) -}) diff --git a/tests/unit/backend/utils/field-validation/email-validation.spec.ts b/tests/unit/backend/utils/field-validation/email-validation.spec.ts new file mode 100644 index 0000000000..9befdf63b7 --- /dev/null +++ b/tests/unit/backend/utils/field-validation/email-validation.spec.ts @@ -0,0 +1,149 @@ +import validateField from 'src/app/utils/field-validation' +import EmailValidator from 'src/app/utils/field-validation/validators/EmailValidator.class' + +describe('Email field validation', () => { + beforeEach(() => { + jest + .spyOn(EmailValidator.prototype, '_isSignatureValid') + .mockImplementation(() => true) + }) + + it('should allow valid emails', () => { + const formField = { + _id: 'abc123', + fieldType: 'email', + } + const response = { + _id: 'abc123', + fieldType: 'email', + answer: 'valid@email.com', + } + const testFunc = () => validateField('formId', formField, response) + expect(testFunc).not.toThrow() + }) + + it('should disallow invalid emails', () => { + const formField = { + _id: 'abc123', + fieldType: 'email', + } + const response = { + _id: 'abc123', + fieldType: 'email', + answer: 'invalidemail.com', + } + const testFunc = () => validateField('formId', formField, response) + expect(testFunc).toThrow() + }) + + it('should allow empty answer for required logic field that is not visible', () => { + const formField = { + _id: 'abc123', + fieldType: 'email', + required: true, + } + const response = { + _id: 'abc123', + fieldType: 'email', + isVisible: false, + answer: '', + } + const testFunc = () => validateField('formId', formField, response) + expect(testFunc).not.toThrow() + }) + + it('should allow email addresses whose email domain belongs to allowedEmailDomains when isVerifiable is true, hasAllowedEmailDomains is true and allowedEmailDomains is not empty', () => { + const formField = { + _id: 'abc123', + fieldType: 'email', + required: true, + isVerifiable: true, + hasAllowedEmailDomains: true, + allowedEmailDomains: ['@test.gov.sg'], + } + const response = { + _id: 'abc123', + fieldType: 'email', + isVisible: true, + answer: 'volunteer-testing@test.gov.sg', + } + const testFunc = () => validateField('formId', formField, response) + expect(testFunc).not.toThrow() + }) + + it('should not allow email addresses whose email domain does not belong to allowedEmailDomains when isVerifiable is true, hasAllowedEmailDomains is true and allowedEmailDomains is not empty', () => { + const formField = { + _id: 'abc123', + fieldType: 'email', + required: true, + isVerifiable: true, + hasAllowedEmailDomains: true, + allowedEmailDomains: ['@example.com'], + } + const response = { + _id: 'abc123', + fieldType: 'email', + isVisible: true, + answer: 'volunteer-testing@test.gov.sg', + } + const testFunc = () => validateField('formId', formField, response) + expect(testFunc).toThrow() + }) + + it('should allow any valid email address when isVerifiable is true, hasAllowedEmailDomains is true but allowedEmailDomains is empty', () => { + const formField = { + _id: 'abc123', + fieldType: 'email', + required: true, + isVerifiable: true, + hasAllowedEmailDomains: true, + allowedEmailDomains: [], + } + const response = { + _id: 'abc123', + fieldType: 'email', + isVisible: true, + answer: 'volunteer-testing@test.gov.sg', + } + const testFunc = () => validateField('formId', formField, response) + expect(testFunc).not.toThrow() + }) + + it('should allow any valid email address when isVerifiable is true and hasAllowedEmailDomains is false, regardless of the cardinality of allowedEmailDomains', () => { + const formField = { + _id: 'abc123', + fieldType: 'email', + required: true, + isVerifiable: true, + hasAllowedEmailDomains: false, + allowedEmailDomains: ['@example.com'], + } + const response = { + _id: 'abc123', + fieldType: 'email', + isVisible: true, + answer: 'volunteer-testing@test.gov.sg', + } + const testFunc = () => validateField('formId', formField, response) + expect(testFunc).not.toThrow() + }) + + it('should allow any valid email address when isVerifiable is false and hasAllowedEmailDomains is true, regardless of the cardinality of allowedEmailDomains', () => { + const formField = { + _id: 'abc123', + fieldType: 'email', + required: true, + isVerifiable: false, + hasAllowedEmailDomains: true, + allowedEmailDomains: ['@example.com'], + } + const response = { + _id: 'abc123', + fieldType: 'email', + isVisible: true, + answer: 'volunteer-testing@test.gov.sg', + } + const testFunc = () => validateField('formId', formField, response) + expect(testFunc).not.toThrow() + }) +}) From e7f2549a8915f61259d39a2c69d52827f75f8fca Mon Sep 17 00:00:00 2001 From: shuli-ogp <63710093+shuli-ogp@users.noreply.github.com> Date: Mon, 28 Sep 2020 16:28:39 +0800 Subject: [PATCH 09/14] feat: filter storage mode responses by submission id (#362) * feat: Filter Storage Mode Responses by Submission Id (#174) * feat: Filter Storage Mode Responses by Submission Id (updated 2020-09-13) * Update page design so that it is compatible with tablets and everything is on one row * Update CSS for iPad * fix: add else on Submission.aggregate to prevent indefinite query * refactor: fix logging format Co-authored-by: Frank Chen Co-authored-by: Antariksh Mahajan --- .../encrypt-submissions.server.controller.js | 191 +++++++++++------- .../view-responses.client.controller.js | 29 ++- .../modules/forms/admin/css/export-button.css | 1 - .../forms/admin/css/view-responses.css | 22 +- .../views/view-responses.client.view.html | 56 ++++- .../services/submissions.client.factory.js | 6 +- 6 files changed, 220 insertions(+), 85 deletions(-) diff --git a/src/app/controllers/encrypt-submissions.server.controller.js b/src/app/controllers/encrypt-submissions.server.controller.js index d364e21bfe..74eb058f73 100644 --- a/src/app/controllers/encrypt-submissions.server.controller.js +++ b/src/app/controllers/encrypt-submissions.server.controller.js @@ -25,6 +25,8 @@ const { getProcessedResponses, } = require('../modules/submission/submission.service') +const HttpStatus = require('http-status-codes') + /** * Extracts relevant fields, injects questions, verifies visibility of field and validates answers * to produce req.body.parsedResponses @@ -223,88 +225,129 @@ exports.saveResponseToDb = function (req, res, next) { */ exports.getMetadata = function (req, res) { let pageSize = 10 - let { page } = req.query || {} + let { page, submissionId } = req.query || {} let numToSkip = parseInt(page - 1 || 0) * pageSize - Submission.aggregate([ - { - $match: { - form: req.form._id, - submissionType: 'encryptSubmission', - }, - }, - { - $sort: { created: -1 }, - }, - { - $facet: { - pageResults: [ - { - $skip: numToSkip, - }, - { - $limit: pageSize, - }, - { - $project: { - _id: 1, - created: 1, - }, - }, - ], - allResults: [ - { - $group: { - _id: null, - count: { - $sum: 1, + let matchClause = { + form: req.form._id, + submissionType: 'encryptSubmission', + } + + if (submissionId) { + if (mongoose.Types.ObjectId.isValid(submissionId)) { + matchClause._id = mongoose.Types.ObjectId(submissionId) + // Reading from primary to avoid any contention issues with bulk queries on secondary servers + Submission.findOne(matchClause, { created: 1 }) + .read('primary') + .exec((err, result) => { + if (err) { + logger.error({ + message: 'Failure retrieving metadata from database', + meta: { + action: 'getMetadata', + ip: getRequestIp(req), + url: req.url, + headers: req.headers, }, - }, - }, - { - $project: { - _id: 0, - }, - }, - ], - }, - }, - ]) - .allowDiskUse(true) // prevents out-of-memory for large search results (max 100MB) - .exec((err, result) => { - if (err || !result) { - logger.error({ - message: 'Failure retrieving metadata from database', - meta: { - action: 'getMetadata', - ip: getRequestIp(req), - url: req.url, - headers: req.headers, - }, - error: err, - }) - return res.status(StatusCodes.INTERNAL_SERVER_ERROR).send({ - message: errorHandler.getMongoErrorMessage(err), - }) - } else { - const [{ pageResults, allResults }] = result - let [numResults] = allResults - let count = (numResults && numResults.count) || 0 - let number = count - numToSkip - let metadata = pageResults.map((data) => { + error: err, + }) + return res.status(HttpStatus.INTERNAL_SERVER_ERROR).send({ + message: errorHandler.getMongoErrorMessage(err), + }) + } + if (!result) { + return res.status(HttpStatus.OK).send({ metadata: [], count: 0 }) + } let entry = { - number, - refNo: data._id, - submissionTime: moment(data.created) + number: 1, + refNo: result._id, + submissionTime: moment(result.created) .tz('Asia/Singapore') .format('Do MMM YYYY, h:mm:ss a'), } - number-- - return entry + return res.status(HttpStatus.OK).send({ metadata: [entry], count: 1 }) }) - return res.status(StatusCodes.OK).send({ metadata, count }) - } - }) + } else { + return res.status(HttpStatus.OK).send({ metadata: [], count: 0 }) + } + } else { + Submission.aggregate([ + { + $match: matchClause, + }, + { + $sort: { created: -1 }, + }, + { + $facet: { + pageResults: [ + { + $skip: numToSkip, + }, + { + $limit: pageSize, + }, + { + $project: { + _id: 1, + created: 1, + }, + }, + ], + allResults: [ + { + $group: { + _id: null, + count: { + $sum: 1, + }, + }, + }, + { + $project: { + _id: 0, + }, + }, + ], + }, + }, + ]) + .allowDiskUse(true) // prevents out-of-memory for large search results (max 100MB) + .exec((err, result) => { + if (err || !result) { + logger.error({ + message: 'Failure retrieving metadata from database', + meta: { + action: 'getMetadata', + ip: getRequestIp(req), + url: req.url, + headers: req.headers, + }, + error: err, + }) + return res.status(StatusCodes.INTERNAL_SERVER_ERROR).send({ + message: errorHandler.getMongoErrorMessage(err), + }) + } else { + const [{ pageResults, allResults }] = result + let [numResults] = allResults + let count = (numResults && numResults.count) || 0 + let number = count - numToSkip + let metadata = pageResults.map((data) => { + let entry = { + number, + refNo: data._id, + submissionTime: moment(data.created) + .tz('Asia/Singapore') + .format('Do MMM YYYY, h:mm:ss a'), + } + number-- + return entry + }) + return res.status(StatusCodes.OK).send({ metadata, count }) + } + }) + } } /** 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 eceab09d5d..ab0d81f37c 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 @@ -40,7 +40,12 @@ function ViewResponsesController( vm.isEncryptResponseMode = vm.myform.responseMode === responseModeEnum.ENCRYPT vm.encryptionKey = null // will be set to an instance of EncryptionKey when form is unlocked successfully vm.csvDownloading = false // whether CSV export is in progress + vm.attachmentDownloadUrls = new Map() + vm.filterBySubmissionRefId = '' // whether to filter submissions by a specific ID + vm.filterBySubmissionRefIdTextbox = '' + vm.filterBySubmissionRefIdMatcher = /^[0-9A-Fa-f]{24}$/ + vm.filterBySubmissionShowFilterBox = false // Three views: // 1 - Unlock view for verifying form password @@ -318,12 +323,29 @@ function ViewResponsesController( } }) + vm.filterBySubmissionChanged = function () { + // We only reload the table if the text box has changed. This prevents excessive + // requests being sent by users clicking on the "Filter" button repeatedly. + if ( + vm.filterBySubmissionRefIdTextbox !== '' && + vm.filterBySubmissionRefId !== vm.filterBySubmissionRefIdTextbox + ) { + vm.filterBySubmissionRefId = vm.filterBySubmissionRefIdTextbox + vm.tableParams.reload() + } + } + + vm.filterBySubmissionReset = function () { + vm.filterBySubmissionShowFilterBox = false + vm.filterBySubmissionRefId = '' + vm.filterBySubmissionRefIdTextbox = '' + vm.tableParams.reload() + } + // Called by child directive unlockResponsesForm after key is verified to get responses - vm.loadResponses = function (formPassword) { - vm.formPassword = formPassword + vm.loadResponses = function () { vm.currentView = 2 vm.loading = true - vm.tableParams = new NgTableParams( { page: 1, // show first page @@ -334,6 +356,7 @@ function ViewResponsesController( let { page } = params.url() return Submissions.getMetadata({ formId: vm.myform._id, + filterBySubmissionRefId: vm.filterBySubmissionRefId, page, }) .then((data) => { diff --git a/src/public/modules/forms/admin/css/export-button.css b/src/public/modules/forms/admin/css/export-button.css index 602f23432c..942afd499f 100644 --- a/src/public/modules/forms/admin/css/export-button.css +++ b/src/public/modules/forms/admin/css/export-button.css @@ -7,7 +7,6 @@ display: flex; justify-content: center; align-items: center; - min-width: 164px; margin-left: auto; } diff --git a/src/public/modules/forms/admin/css/view-responses.css b/src/public/modules/forms/admin/css/view-responses.css index 662eebac1a..20836838fe 100644 --- a/src/public/modules/forms/admin/css/view-responses.css +++ b/src/public/modules/forms/admin/css/view-responses.css @@ -101,7 +101,8 @@ } #responses-tab .response-stats { - width: 100%; + white-space: nowrap; + min-width: 240px; } #responses-tab .datepicker-export-container { @@ -715,3 +716,22 @@ padding-left: 16px !important; border: solid 1px #ccc; } + +#filter-by-submission-reset-link { + padding-top: 13px; + padding-left: 13px; + white-space: nowrap; + cursor: pointer; +} + +#filter-by-submission-filter-link { + padding-top: 13px; + padding-left: 5px; + white-space: nowrap; + cursor: pointer; +} + +#filter-by-submission-filter-textbox-container { + display: flex; + min-width: 240px; +} diff --git a/src/public/modules/forms/admin/views/view-responses.client.view.html b/src/public/modules/forms/admin/views/view-responses.client.view.html index 5e9671e07c..f6f3081780 100644 --- a/src/public/modules/forms/admin/views/view-responses.client.view.html +++ b/src/public/modules/forms/admin/views/view-responses.client.view.html @@ -31,7 +31,9 @@
-
+
-
+
-
+
{{vm.responsesCount}} response(s) to date + + + + +
+
+ + + View All
-
+
+
+
+
-
+
+
+
+ +
No results found
+
+ Did you enter the right reference number? We can't seem to find + the response. +
+
+
Date: Tue, 29 Sep 2020 11:13:24 +0800 Subject: [PATCH 10/14] feat: move notification banner back to top and add banner type capabilities (#359) * feat: add top aligned banner that does not block anything * style: update styling of app to allow for sticky banner/navbar * feat: remove scroll-to-id directive and use $anchorScroll * style: update MyInfo field tab styling * style: update form builder styling for mobile * feat: add function to process banner message encoding to display * style: add banner styling for the various types of banners * style: update styling for signin view to accommodate banner * style: correct link styling for banner * chore: add documentation and example to banner env vars * style: update avatar styling to center on iOS * fix: typo in banner component comment * style: add vendor prefixes to avatar-dropdown.css * style: remove section.content styling add add .page-container class * feat: add page-container class to all route htmls for correct styling * feat: add back overlay to trigger * feat: add id to banner component * feat: add dynamic padding to form header when banner is shown * feat: make retrieval of banner type more forgiving Co-authored-by: shuli-ogp <63710093+shuli-ogp@users.noreply.github.com> * style: fix wrong paddings on myinfo disabled overlay in add fields * style: update add myinfo field padding to be back to original style Co-authored-by: shuli-ogp <63710093+shuli-ogp@users.noreply.github.com> --- docs/DEPLOYMENT_SETUP.md | 39 +++ src/public/main.css | 1 + src/public/main.js | 1 - .../modules/core/componentViews/banner.html | 14 +- .../components/banner.client.component.js | 24 ++ .../modules/core/css/admin-form-header.css | 33 ++ .../modules/core/css/avatar-dropdown.css | 17 +- src/public/modules/core/css/banner.css | 92 +++++ src/public/modules/core/css/core.css | 69 +++- src/public/modules/core/css/landing.css | 46 --- .../list-forms.client.controller.js | 6 - .../view-responses.client.controller.js | 18 + .../modules/forms/admin/css/edit-form.css | 161 ++++++++- .../modules/forms/admin/css/list-forms.css | 31 +- .../directiveViews/edit-form.client.view.html | 8 +- .../scroll-to-id.client.directive.js | 18 - .../admin/views/admin-form.client.view.html | 106 +++--- .../admin/views/list-forms.client.view.html | 204 +++++------ .../views/view-responses.client.view.html | 4 - .../components/start-page.client.component.js | 7 +- .../modules/forms/base/css/base-form.css | 49 ++- .../modules/forms/base/css/start-end-page.css | 3 +- src/public/modules/users/css/billing.css | 2 +- .../modules/users/css/examples-list.css | 2 +- src/public/modules/users/css/sign-in.css | 70 +++- .../authentication/signin.client.view.html | 182 +++++----- .../users/views/billing.client.view.html | 330 +++++++++--------- .../views/examples-list.client.view.html | 196 ++++++----- 28 files changed, 1115 insertions(+), 618 deletions(-) create mode 100644 src/public/modules/core/css/banner.css delete mode 100644 src/public/modules/forms/admin/directives/scroll-to-id.client.directive.js diff --git a/docs/DEPLOYMENT_SETUP.md b/docs/DEPLOYMENT_SETUP.md index ffa33d004d..6fd821164e 100644 --- a/docs/DEPLOYMENT_SETUP.md +++ b/docs/DEPLOYMENT_SETUP.md @@ -130,6 +130,45 @@ The following env variables are set in Travis: #### Banners +These environment variables allow us to set notification banners in the +application without a full redeployment of the application. Note the hierarchy +of the banner content. + +In addition, you can change the color of the banner by adding a type encoding in +the environment variable string. The default banner type will be `info` if no encoding is +provided. + +The possible banner type prefixes are: `info:`, `warn:`, and `error:`. Other +prefixes will not work and the invalid prefixes will be shown in the banner +text. + +Examples: + +``` +SITE_BANNER_CONTENT=info:This is an info banner. You can also add links in the text like https://example.com. There is also a dismiss button to the right of the text. +``` + +![Info banner +example](https://user-images.githubusercontent.com/22133008/93852946-8a867780-fce5-11ea-929f-a0ce1c6796b9.png) + +``` +SITE_BANNER_CONTENT=warn:This is a warning banner. You can also add links in the text like https://example.com +``` + +![Warning banner example](https://user-images.githubusercontent.com/22133008/93852559-cec54800-fce4-11ea-9376-9b2802e8ac62.png) + +``` +SITE_BANNER_CONTENT=error:This is an error banner. You can also add links in the text like https://example.com +``` + +![Error banner example](https://user-images.githubusercontent.com/22133008/93852689-1055f300-fce5-11ea-956d-d5966cbe86d8.png) + +``` +SITE_BANNER_CONTENT=hello:This is an invalid banner type, and the full text will be shown. The default banner type of `info` will used. +``` + +![Invalid banner default example](https://user-images.githubusercontent.com/22133008/93853306-392ab800-fce6-11ea-9891-f752bdad236e.png) + | Variable | Description | | :----------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `SITE_BANNER_CONTENT` | If set, displays a banner message on both private routes that `ADMIN_BANNER_CONTENT` covers **and** public form routes that `IS_GENERAL_MAINTENANCE` covers. Overrides all other banner environment variables | diff --git a/src/public/main.css b/src/public/main.css index d0ad4fefe1..bf28a358e8 100644 --- a/src/public/main.css +++ b/src/public/main.css @@ -18,6 +18,7 @@ @import './modules/core/css/admin-form-header.css'; @import './modules/core/css/sg-govt-banner.css'; @import './modules/core/css/landing.css'; +@import './modules/core/css/banner.css'; @import './modules/core/css/how-it-works.css'; @import './modules/forms/admin/css/pop-up-modal.css'; @import './modules/forms/admin/css/edit-form.css'; diff --git a/src/public/main.js b/src/public/main.js index fcaa8f8ec9..e2bf4e6f68 100644 --- a/src/public/main.js +++ b/src/public/main.js @@ -194,7 +194,6 @@ require('./modules/forms/admin/directives/configure-form.client.directive.js') require('./modules/forms/admin/directives/configure-mobile.client.directive.js') require('./modules/forms/admin/directives/verify-secret-key.client.directive.js') require('./modules/forms/admin/directives/daterangepicker.client.directive.js') -require('./modules/forms/admin/directives/scroll-to-id.client.directive.js') require('./modules/forms/admin/directives/edit-captcha.client.directive.js') require('./modules/forms/admin/controllers/create-form-template-modal.client.controller.js') require('./modules/forms/admin/controllers/collaborator-modal.client.controller.js') diff --git a/src/public/modules/core/componentViews/banner.html b/src/public/modules/core/componentViews/banner.html index decb9e1a3b..09c2f8dfc4 100644 --- a/src/public/modules/core/componentViews/banner.html +++ b/src/public/modules/core/componentViews/banner.html @@ -1,8 +1,16 @@ -