From 651e0abec526d0e5165b17aa3974f61ac0362897 Mon Sep 17 00:00:00 2001 From: seaerchin Date: Mon, 14 Jun 2021 12:16:51 +0800 Subject: [PATCH 01/23] feat(configure-mobile.client.directive): updated modal to use new messages with hard-coded data --- .../configure-mobile.client.directive.js | 22 ++++++++++++------- .../admin/views/pop-up.client.modal.html | 2 +- src/public/translations/en-SG/main.json | 1 + 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js index ee0bcec6a0..ed57ac9640 100644 --- a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js +++ b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js @@ -20,8 +20,8 @@ function configureMobileDirective() { '$translate', function ($uibModal, $scope, $translate) { // Get support form link from translation json. - $translate('LINKS.SUPPORT_FORM_LINK').then((supportFormLink) => { - $scope.supportFormLink = supportFormLink + $translate('LINKS.TWILIO_SETUP_LINK').then((twilioSetupLink) => { + $scope.twilioSetupLink = twilioSetupLink }) $scope.openVerifiedSMSModal = function () { @@ -39,13 +39,19 @@ function configureMobileDirective() { resolve: { externalScope: function () { return { - title: 'Verified SMS charges', - confirmButtonText: 'OK, Noted', + title: + 'OTP verification will be disabled at 10,000 responses', + confirmButtonText: 'Accept', description: ` - Under 10,000 form responses: Free verified SMS -

- Above 10,000 form responses: ~US$0.0395 per SMS - contact us - for billing. Forms exceeding the free tier without billing will be deactivated. + We provide SMS OTP verification for free up to 10,000 responses. OTP verification will be automatically disabled when your account reaches 10,000 responses. +

+ If you require OTP verification for more than 10,000 responses, + please arrange advance billing with us. + +

+ Current response count: ${0}/${0} `, } }, diff --git a/src/public/modules/forms/admin/views/pop-up.client.modal.html b/src/public/modules/forms/admin/views/pop-up.client.modal.html index ced8b7367c..39718f2501 100644 --- a/src/public/modules/forms/admin/views/pop-up.client.modal.html +++ b/src/public/modules/forms/admin/views/pop-up.client.modal.html @@ -6,7 +6,7 @@ diff --git a/src/public/translations/en-SG/main.json b/src/public/translations/en-SG/main.json index 746d70d70f..b84869dd4a 100644 --- a/src/public/translations/en-SG/main.json +++ b/src/public/translations/en-SG/main.json @@ -18,6 +18,7 @@ "GO_GOV": "https://go.gov.sg", "POSTMAN_GOV": "https://postman.gov.sg", "SUPPORT_FORM_LINK": "https://go.gov.sg/formsg-support", + "TWILIO_SETUP_LINK": "https://go.gov.sg/form-twilio-setup", "WHITELISTED_ATTACHMENT_TYPES": "https://go.gov.sg/formsg-cwl", "SINGPASS_ELIGIBILITY_FAQ": "https://www.ifaq.gov.sg/SINGPASS/apps/Fcd_faqmain.aspx#FAQ_2101385", "ESERVICE_ID_FAQ": "https://go.gov.sg/formsg-spcp", From dbc01269ca0935093b4fe7886e93707b6866c10d Mon Sep 17 00:00:00 2001 From: seaerchin Date: Mon, 14 Jun 2021 12:52:49 +0800 Subject: [PATCH 02/23] refactor(verification.constants): shifted sms_verification_limit to shared/util/verification --- src/app/services/mail/mail.service.ts | 5 ++++- src/shared/util/verification.ts | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/app/services/mail/mail.service.ts b/src/app/services/mail/mail.service.ts index 686dd9d915..babf4ab9c6 100644 --- a/src/app/services/mail/mail.service.ts +++ b/src/app/services/mail/mail.service.ts @@ -5,7 +5,10 @@ import Mail from 'nodemailer/lib/mailer' import promiseRetry from 'promise-retry' import validator from 'validator' -import { HASH_EXPIRE_AFTER_SECONDS } from '../../../shared/util/verification' +import { + HASH_EXPIRE_AFTER_SECONDS, + SMS_VERIFICATION_LIMIT, +} from '../../../shared/util/verification' import { BounceType, EmailAdminDataField, diff --git a/src/shared/util/verification.ts b/src/shared/util/verification.ts index 3f361a96a3..31dd1a5a83 100644 --- a/src/shared/util/verification.ts +++ b/src/shared/util/verification.ts @@ -21,3 +21,5 @@ export enum VfnErrors { TransactionNotFound = 'TRANSACTION_NOT_FOUND', InvalidMobileNumber = 'INVALID_MOBILE_NUMBER', } + +export const SMS_VERIFICATION_LIMIT = 10000 From 380fa4bfe7b2db5f7f4c72988716addb58997396 Mon Sep 17 00:00:00 2001 From: seaerchin Date: Mon, 14 Jun 2021 17:23:23 +0800 Subject: [PATCH 03/23] feat(configure-mobile.client): added frontend ui for toggling on/off --- .../configure-mobile.client.view.html | 20 ++++++- .../configure-mobile.client.directive.js | 54 ++++++++++++++++--- .../base/componentViews/verifiable-field.html | 5 +- src/shared/util/verification.ts | 6 +++ 4 files changed, 76 insertions(+), 9 deletions(-) diff --git a/src/public/modules/forms/admin/directiveViews/configure-mobile.client.view.html b/src/public/modules/forms/admin/directiveViews/configure-mobile.client.view.html index 44b68cf7fc..01e7ffcffe 100644 --- a/src/public/modules/forms/admin/directiveViews/configure-mobile.client.view.html +++ b/src/public/modules/forms/admin/directiveViews/configure-mobile.client.view.html @@ -11,7 +11,7 @@

+
+ + + + You have exceeded the limit for sms verification! + + Please arrange advance billing with us. + + + +
diff --git a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js index ed57ac9640..028cba891d 100644 --- a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js +++ b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js @@ -1,5 +1,18 @@ 'use strict' +const { + ADMIN_VERIFIED_SMS_STATES, + SMS_VERIFICATION_LIMIT, +} = require('../../../../../shared/util/verification') + +// TODO: Implement this +const getSmsCounts = () => { + return Promise.resolve({ + verifiedSmsCount: 1000000, + messageServiceId: 'asdf', + }) +} + angular .module('forms') .directive('configureMobileDirective', [configureMobileDirective]) @@ -10,24 +23,55 @@ function configureMobileDirective() { 'modules/forms/admin/directiveViews/configure-mobile.client.view.html', restrict: 'E', scope: { - field: '=', + field: '<', name: '=', characterLimit: '=', }, controller: [ + '$q', '$uibModal', '$scope', '$translate', - function ($uibModal, $scope, $translate) { + function ($q, $uibModal, $scope, $translate) { // Get support form link from translation json. $translate('LINKS.TWILIO_SETUP_LINK').then((twilioSetupLink) => { $scope.twilioSetupLink = twilioSetupLink }) + const getAdminVerifiedSmsState = (verifiedSmsCount, msgSrvcId) => { + if (msgSrvcId) { + return ADMIN_VERIFIED_SMS_STATES.hasMessageServiceId + } + if (verifiedSmsCount < SMS_VERIFICATION_LIMIT) { + return ADMIN_VERIFIED_SMS_STATES.belowLimit + } + return ADMIN_VERIFIED_SMS_STATES.limitExceeded + } + + $q.when(getSmsCounts()).then( + ({ verifiedSmsCount, messageServiceId }) => { + $scope.verifiedSmsCount = verifiedSmsCount + $scope.messageServiceId = messageServiceId + $scope.adminVerifiedSmsState = getAdminVerifiedSmsState( + verifiedSmsCount, + messageServiceId, + ) + // NOTE: This links into the verifiable field component and hence, is used by both email and mobile + $scope.field.hasAdminExceededSmsLimit = + $scope.adminVerifiedSmsState === + ADMIN_VERIFIED_SMS_STATES.limitExceeded + }, + ) + + // Only open if the admin has sms counts below the limit. + // If the admin has counts above limit without a message id, the toggle should be disabled anyway. + // Otherwise, if the admin has a message id, just enable it without the modal $scope.openVerifiedSMSModal = function () { const isTogglingOnVerifiedSms = !$scope.field.isVerifiable $scope.verifiedSMSModal = isTogglingOnVerifiedSms && + $scope.adminVerifiedSmsState === + ADMIN_VERIFIED_SMS_STATES.belowLimit && $uibModal.open({ animation: true, backdrop: 'static', @@ -46,12 +90,10 @@ function configureMobileDirective() { We provide SMS OTP verification for free up to 10,000 responses. OTP verification will be automatically disabled when your account reaches 10,000 responses.

If you require OTP verification for more than 10,000 responses, - please arrange advance billing with us. + please arrange advance billing with us.

- Current response count: ${0}/${0} + Current response count: ${$scope.verifiedSmsCount}/${SMS_VERIFICATION_LIMIT} `, } }, diff --git a/src/public/modules/forms/base/componentViews/verifiable-field.html b/src/public/modules/forms/base/componentViews/verifiable-field.html index 17e191a573..592dd13951 100644 --- a/src/public/modules/forms/base/componentViews/verifiable-field.html +++ b/src/public/modules/forms/base/componentViews/verifiable-field.html @@ -1,7 +1,10 @@ -
+

-
+
You have exceeded the limit for sms verification! From 809a8915b6f8252172599b1617550345081e0383 Mon Sep 17 00:00:00 2001 From: seaerchin Date: Wed, 23 Jun 2021 01:01:33 +0800 Subject: [PATCH 07/23] fix(configure-mobile.client.view): updated form twilio account link --- .../admin/directiveViews/configure-mobile.client.view.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/public/modules/forms/admin/directiveViews/configure-mobile.client.view.html b/src/public/modules/forms/admin/directiveViews/configure-mobile.client.view.html index 7a8487b5d2..84fe5b100f 100644 --- a/src/public/modules/forms/admin/directiveViews/configure-mobile.client.view.html +++ b/src/public/modules/forms/admin/directiveViews/configure-mobile.client.view.html @@ -37,7 +37,7 @@ You have exceeded the limit for sms verification! - + Please arrange advance billing with us. From 92f8a3a8a06a00abff4cfe0230cfbb3b1c196b20 Mon Sep 17 00:00:00 2001 From: seaerchin Date: Wed, 23 Jun 2021 01:35:27 +0800 Subject: [PATCH 08/23] feat(configure-mobile.client.directive): added loading state for toggle --- .../modules/forms/admin/css/edit-form.css | 6 +++++ .../configure-mobile.client.view.html | 4 +++ .../configure-mobile.client.directive.js | 25 ++++++++++++------- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/src/public/modules/forms/admin/css/edit-form.css b/src/public/modules/forms/admin/css/edit-form.css index ab84336a1d..25c1fa0882 100644 --- a/src/public/modules/forms/admin/css/edit-form.css +++ b/src/public/modules/forms/admin/css/edit-form.css @@ -1431,3 +1431,9 @@ a.modal-cancel-btn:hover { /* margin is set so that the error message appears to be in the same block as the toggle options */ margin: 0 15px; } + +/* Spinner */ +.loading-spinner { + display: flex; + justify-content: flex-end; +} diff --git a/src/public/modules/forms/admin/directiveViews/configure-mobile.client.view.html b/src/public/modules/forms/admin/directiveViews/configure-mobile.client.view.html index 84fe5b100f..f2d6d6f7c7 100644 --- a/src/public/modules/forms/admin/directiveViews/configure-mobile.client.view.html +++ b/src/public/modules/forms/admin/directiveViews/configure-mobile.client.view.html @@ -12,7 +12,11 @@ >
+

-
+
You have reached the free tier limit for SMS verification. To continue From 294024a6cc72b6de7496ccb531c4165ed8d0d561 Mon Sep 17 00:00:00 2001 From: seaerchin Date: Wed, 30 Jun 2021 15:28:47 +0800 Subject: [PATCH 11/23] feat(public/services): adds new smsservice --- src/public/services/SmsService.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 src/public/services/SmsService.ts diff --git a/src/public/services/SmsService.ts b/src/public/services/SmsService.ts new file mode 100644 index 0000000000..ae87ecdd8e --- /dev/null +++ b/src/public/services/SmsService.ts @@ -0,0 +1,19 @@ +import axios from 'axios' + +import { SmsCountsDto } from '../../types/api' + +/** Exported for testing */ +export const FORM_API_PREFIX = '/api/v3' +export const SMS_ENDPOINT = 'sms' + +/** + * Retrieves the sms verifications metadata from the backend for the admin of a specified form + * @param formId + * @returns + */ +export const getSmsVerificationStateForFormAdmin = ( + formId: string, +): Promise => + axios + .get(`${FORM_API_PREFIX}/${SMS_ENDPOINT}/${formId}`) + .then(({ data }) => data) From 985c1d6306d33dd0fcc1ae5dfbc3b349bc79a617 Mon Sep 17 00:00:00 2001 From: seaerchin Date: Wed, 30 Jun 2021 15:51:51 +0800 Subject: [PATCH 12/23] feat(configure-mobile.client.directive): connected sms counts to fetch from backend --- .../configure-mobile.client.directive.js | 43 ++++++++++--------- .../admin/views/edit-fields.client.modal.html | 1 + 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js index aff9935cb3..9b8bf9436b 100644 --- a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js +++ b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js @@ -1,10 +1,13 @@ 'use strict' +const { get } = require('lodash') const { ADMIN_VERIFIED_SMS_STATES, SMS_VERIFICATION_LIMIT, } = require('../../../../../shared/util/verification') +const SmsService = require('../../../../services/SmsService') + angular .module('forms') .directive('configureMobileDirective', [configureMobileDirective]) @@ -16,6 +19,7 @@ function configureMobileDirective() { restrict: 'E', scope: { field: '<', + form: '<', name: '=', characterLimit: '=', isLoading: '<', @@ -25,22 +29,13 @@ function configureMobileDirective() { '$uibModal', '$scope', '$translate', - '$timeout', - function ($q, $uibModal, $scope, $translate, $timeout) { + 'Toastr', + function ($q, $uibModal, $scope, $translate, Toastr) { // Get support form link from translation json. $translate('LINKS.TWILIO_SETUP_LINK').then((twilioSetupLink) => { $scope.twilioSetupLink = twilioSetupLink }) - // TODO: Implement this - const getSmsCounts = () => - $timeout(() => { - return { - verifiedSmsCount: 20000035, - messageServiceId: '', - } - }, 1000) - // NOTE: This is set on scope as it is used by the UI to determine if the toggle is loading $scope.isLoading = true @@ -54,21 +49,29 @@ function configureMobileDirective() { return ADMIN_VERIFIED_SMS_STATES.limitExceeded } - $q.when(getSmsCounts()).then( - ({ verifiedSmsCount, messageServiceId }) => { - $scope.verifiedSmsCount = verifiedSmsCount - $scope.messageServiceId = messageServiceId + $q.when(SmsService.getSmsVerificationStateForFormAdmin($scope.form._id)) + .then(({ msgSrvcSid, freeSmsCount }) => { + $scope.verifiedSmsCount = freeSmsCount + $scope.messageServiceId = msgSrvcSid $scope.adminVerifiedSmsState = getAdminVerifiedSmsState( - verifiedSmsCount, - messageServiceId, + freeSmsCount, + msgSrvcSid, ) // NOTE: This links into the verifiable field component and hence, is used by both email and mobile $scope.field.hasAdminExceededSmsLimit = $scope.adminVerifiedSmsState === ADMIN_VERIFIED_SMS_STATES.limitExceeded - $scope.isLoading = false - }, - ) + }) + .catch((error) => { + Toastr.error( + get( + error, + 'response.data.message', + 'Sorry, an error occurred. Please refresh the page and try again later.', + ), + ) + }) + .finally(() => ($scope.isLoading = false)) // Only open if the admin has sms counts below the limit. // If the admin has counts above limit without a message id, the toggle should be disabled anyway. 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..6dfd863430 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 @@ -849,6 +849,7 @@

From df66edd6fe01155338fe494073ae1ae84257e1b3 Mon Sep 17 00:00:00 2001 From: seaerchin Date: Thu, 1 Jul 2021 11:20:15 +0800 Subject: [PATCH 13/23] refactor(pop-up-modal): adds variable to check if we should display red button \ --- .../forms/admin/controllers/pop-up-modal.client.controller.js | 1 + .../admin/directives/configure-mobile.client.directive.js | 1 + src/public/modules/forms/admin/views/pop-up.client.modal.html | 3 ++- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/public/modules/forms/admin/controllers/pop-up-modal.client.controller.js b/src/public/modules/forms/admin/controllers/pop-up-modal.client.controller.js index 9c3c33a776..0692194d4d 100644 --- a/src/public/modules/forms/admin/controllers/pop-up-modal.client.controller.js +++ b/src/public/modules/forms/admin/controllers/pop-up-modal.client.controller.js @@ -15,6 +15,7 @@ function PopUpModalController($uibModalInstance, externalScope) { vm.description = externalScope.description vm.confirmButtonText = externalScope.confirmButtonText vm.cancelButtonText = externalScope.cancelButtonText + vm.isImportant = externalScope.isImportant vm.cancel = $uibModalInstance.close vm.confirm = function () { diff --git a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js index 9b8bf9436b..8d6b38a843 100644 --- a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js +++ b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js @@ -105,6 +105,7 @@ function configureMobileDirective() {

Current response count: ${$scope.verifiedSmsCount}/${SMS_VERIFICATION_LIMIT} `, + isImportant: true, } }, }, diff --git a/src/public/modules/forms/admin/views/pop-up.client.modal.html b/src/public/modules/forms/admin/views/pop-up.client.modal.html index 39718f2501..e034acf292 100644 --- a/src/public/modules/forms/admin/views/pop-up.client.modal.html +++ b/src/public/modules/forms/admin/views/pop-up.client.modal.html @@ -6,7 +6,8 @@ From 971984a176a4a256431b1abb3070c29235920fbe Mon Sep 17 00:00:00 2001 From: seaerchin Date: Thu, 1 Jul 2021 13:48:18 +0800 Subject: [PATCH 14/23] test(smsservice): adds tests for sms service --- .../services/__tests__/SmsService.test.ts | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 src/public/services/__tests__/SmsService.test.ts diff --git a/src/public/services/__tests__/SmsService.test.ts b/src/public/services/__tests__/SmsService.test.ts new file mode 100644 index 0000000000..ccf9b1884f --- /dev/null +++ b/src/public/services/__tests__/SmsService.test.ts @@ -0,0 +1,29 @@ +/* eslint-disable @typescript-eslint/ban-ts-comment */ +import axios from 'axios' +import { ObjectId } from 'bson-ext' +import { mocked } from 'ts-jest/utils' + +import * as SmsService from '../SmsService' + +jest.mock('axios') +const MockAxios = mocked(axios, true) + +describe('SmsService', () => { + describe('getSmsVerificationStateForFormAdmin', () => { + const MOCK_FORM_ID = new ObjectId().toHexString() + it('should call the endpoint successfully when parameters are provided', async () => { + // Arrange + MockAxios.get.mockResolvedValueOnce({ data: 'some data' }) + // Act + const actual = await SmsService.getSmsVerificationStateForFormAdmin( + MOCK_FORM_ID, + ) + + // Assert + expect(MockAxios.get).toBeCalledWith( + `${SmsService.FORM_API_PREFIX}/${SmsService.SMS_ENDPOINT}/${MOCK_FORM_ID}`, + ) + expect(actual).toBe('some data') + }) + }) +}) From 1bc1033364c9691073ceddb1395a0a9a7ade9ad2 Mon Sep 17 00:00:00 2001 From: seaerchin Date: Thu, 1 Jul 2021 13:49:19 +0800 Subject: [PATCH 15/23] refactor(configure-mobile.client): adds error reporting and handling on FE --- .../configure-mobile.client.view.html | 11 +++++++---- .../configure-mobile.client.directive.js | 16 ++++++++++++---- .../base/componentViews/verifiable-field.html | 2 +- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/public/modules/forms/admin/directiveViews/configure-mobile.client.view.html b/src/public/modules/forms/admin/directiveViews/configure-mobile.client.view.html index c508ee8474..6646a0d9c4 100644 --- a/src/public/modules/forms/admin/directiveViews/configure-mobile.client.view.html +++ b/src/public/modules/forms/admin/directiveViews/configure-mobile.client.view.html @@ -1,7 +1,7 @@
OTP verification @@ -18,7 +18,7 @@
diff --git a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js index 8d6b38a843..42dbda1e1d 100644 --- a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js +++ b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js @@ -38,6 +38,7 @@ function configureMobileDirective() { // NOTE: This is set on scope as it is used by the UI to determine if the toggle is loading $scope.isLoading = true + $scope.field.hasRetrievalError = false const getAdminVerifiedSmsState = (verifiedSmsCount, msgSrvcId) => { if (msgSrvcId) { @@ -61,13 +62,15 @@ function configureMobileDirective() { $scope.field.hasAdminExceededSmsLimit = $scope.adminVerifiedSmsState === ADMIN_VERIFIED_SMS_STATES.limitExceeded + $scope.field.hasRetrievalError = false }) .catch((error) => { + $scope.field.hasRetrievalError = true Toastr.error( get( error, 'response.data.message', - 'Sorry, an error occurred. Please refresh the page and try again later.', + 'Sorry, an error occurred. Please refresh the page to toggle OTP verification.', ), ) }) @@ -78,10 +81,15 @@ function configureMobileDirective() { // Otherwise, if the admin has a message id, just enable it without the modal $scope.openVerifiedSMSModal = function () { const isTogglingOnVerifiedSms = !$scope.field.isVerifiable - $scope.verifiedSMSModal = - isTogglingOnVerifiedSms && + const isAdminBelowLimit = $scope.adminVerifiedSmsState === - ADMIN_VERIFIED_SMS_STATES.belowLimit && + ADMIN_VERIFIED_SMS_STATES.belowLimit + const shouldShowModal = + isTogglingOnVerifiedSms && + isAdminBelowLimit && + !$scope.field.hasRetrievalError + $scope.verifiedSMSModal = + shouldShowModal && $uibModal.open({ animation: true, backdrop: 'static', diff --git a/src/public/modules/forms/base/componentViews/verifiable-field.html b/src/public/modules/forms/base/componentViews/verifiable-field.html index 592dd13951..fd97acf537 100644 --- a/src/public/modules/forms/base/componentViews/verifiable-field.html +++ b/src/public/modules/forms/base/componentViews/verifiable-field.html @@ -3,7 +3,7 @@ >
From bd33e1c57d7ef3051b66b7bd298b2d0f50ecdd3e Mon Sep 17 00:00:00 2001 From: seaerchin Date: Fri, 2 Jul 2021 13:02:37 +0800 Subject: [PATCH 16/23] refactor(smsservice): renamed SmsService to AdminMetaService --- .../configure-mobile.client.directive.js | 15 ++++++++------- src/public/services/AdminMetaService.ts | 18 ++++++++++++++++++ src/public/services/SmsService.ts | 19 ------------------- ...rvice.test.ts => AdminMetaService.test.ts} | 8 ++++---- 4 files changed, 30 insertions(+), 30 deletions(-) create mode 100644 src/public/services/AdminMetaService.ts delete mode 100644 src/public/services/SmsService.ts rename src/public/services/__tests__/{SmsService.test.ts => AdminMetaService.test.ts} (71%) diff --git a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js index 42dbda1e1d..0223299005 100644 --- a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js +++ b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js @@ -6,7 +6,7 @@ const { SMS_VERIFICATION_LIMIT, } = require('../../../../../shared/util/verification') -const SmsService = require('../../../../services/SmsService') +const AdminMetaService = require('../../../../services/AdminMetaService') angular .module('forms') @@ -50,13 +50,14 @@ function configureMobileDirective() { return ADMIN_VERIFIED_SMS_STATES.limitExceeded } - $q.when(SmsService.getSmsVerificationStateForFormAdmin($scope.form._id)) - .then(({ msgSrvcSid, freeSmsCount }) => { - $scope.verifiedSmsCount = freeSmsCount - $scope.messageServiceId = msgSrvcSid + $q.when( + AdminMetaService.getSmsVerificationStateForFormAdmin($scope.form._id), + ) + .then(({ smsCounts }) => { + $scope.verifiedSmsCount = smsCounts $scope.adminVerifiedSmsState = getAdminVerifiedSmsState( - freeSmsCount, - msgSrvcSid, + smsCounts, + $scope.form.msgSrvcName, ) // NOTE: This links into the verifiable field component and hence, is used by both email and mobile $scope.field.hasAdminExceededSmsLimit = diff --git a/src/public/services/AdminMetaService.ts b/src/public/services/AdminMetaService.ts new file mode 100644 index 0000000000..9c885ea28a --- /dev/null +++ b/src/public/services/AdminMetaService.ts @@ -0,0 +1,18 @@ +import axios from 'axios' + +import { SmsCountsDto } from '../../types/api' + +/** Exported for testing */ +export const FORM_API_PREFIX = '/api/v3/admin/forms' + +/** + * Retrieves the free sms counts used by the admin of a specified form + * @param formId + * @returns The amount of free sms counts used by the admin of the form + */ +export const getSmsVerificationStateForFormAdmin = ( + formId: string, +): Promise => + axios + .get(`${FORM_API_PREFIX}/${formId}/verified-sms/count/free`) + .then(({ data }) => data) diff --git a/src/public/services/SmsService.ts b/src/public/services/SmsService.ts deleted file mode 100644 index ae87ecdd8e..0000000000 --- a/src/public/services/SmsService.ts +++ /dev/null @@ -1,19 +0,0 @@ -import axios from 'axios' - -import { SmsCountsDto } from '../../types/api' - -/** Exported for testing */ -export const FORM_API_PREFIX = '/api/v3' -export const SMS_ENDPOINT = 'sms' - -/** - * Retrieves the sms verifications metadata from the backend for the admin of a specified form - * @param formId - * @returns - */ -export const getSmsVerificationStateForFormAdmin = ( - formId: string, -): Promise => - axios - .get(`${FORM_API_PREFIX}/${SMS_ENDPOINT}/${formId}`) - .then(({ data }) => data) diff --git a/src/public/services/__tests__/SmsService.test.ts b/src/public/services/__tests__/AdminMetaService.test.ts similarity index 71% rename from src/public/services/__tests__/SmsService.test.ts rename to src/public/services/__tests__/AdminMetaService.test.ts index ccf9b1884f..aadedd0b1f 100644 --- a/src/public/services/__tests__/SmsService.test.ts +++ b/src/public/services/__tests__/AdminMetaService.test.ts @@ -3,25 +3,25 @@ import axios from 'axios' import { ObjectId } from 'bson-ext' import { mocked } from 'ts-jest/utils' -import * as SmsService from '../SmsService' +import * as AdminMetaService from '../AdminMetaService' jest.mock('axios') const MockAxios = mocked(axios, true) -describe('SmsService', () => { +describe('AdminMetaService', () => { describe('getSmsVerificationStateForFormAdmin', () => { const MOCK_FORM_ID = new ObjectId().toHexString() it('should call the endpoint successfully when parameters are provided', async () => { // Arrange MockAxios.get.mockResolvedValueOnce({ data: 'some data' }) // Act - const actual = await SmsService.getSmsVerificationStateForFormAdmin( + const actual = await AdminMetaService.getSmsVerificationStateForFormAdmin( MOCK_FORM_ID, ) // Assert expect(MockAxios.get).toBeCalledWith( - `${SmsService.FORM_API_PREFIX}/${SmsService.SMS_ENDPOINT}/${MOCK_FORM_ID}`, + `${AdminMetaService.FORM_API_PREFIX}/${MOCK_FORM_ID}/verified-sms/count/free`, ) expect(actual).toBe('some data') }) From 722b33329b8fb47afe5527eb5f86f2813e511221 Mon Sep 17 00:00:00 2001 From: seaerchin Date: Fri, 2 Jul 2021 13:09:52 +0800 Subject: [PATCH 17/23] refactor(adminmetaservice): renamed method for greater clarity --- .../admin/directives/configure-mobile.client.directive.js | 2 +- src/public/services/AdminMetaService.ts | 2 +- src/public/services/__tests__/AdminMetaService.test.ts | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js index 0223299005..3caab9aa17 100644 --- a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js +++ b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js @@ -51,7 +51,7 @@ function configureMobileDirective() { } $q.when( - AdminMetaService.getSmsVerificationStateForFormAdmin($scope.form._id), + AdminMetaService.getFreeSmsCountsUsedByFormAdmin($scope.form._id), ) .then(({ smsCounts }) => { $scope.verifiedSmsCount = smsCounts diff --git a/src/public/services/AdminMetaService.ts b/src/public/services/AdminMetaService.ts index 9c885ea28a..f35eeea248 100644 --- a/src/public/services/AdminMetaService.ts +++ b/src/public/services/AdminMetaService.ts @@ -10,7 +10,7 @@ export const FORM_API_PREFIX = '/api/v3/admin/forms' * @param formId * @returns The amount of free sms counts used by the admin of the form */ -export const getSmsVerificationStateForFormAdmin = ( +export const getFreeSmsCountsUsedByFormAdmin = ( formId: string, ): Promise => axios diff --git a/src/public/services/__tests__/AdminMetaService.test.ts b/src/public/services/__tests__/AdminMetaService.test.ts index aadedd0b1f..7e42e31ab0 100644 --- a/src/public/services/__tests__/AdminMetaService.test.ts +++ b/src/public/services/__tests__/AdminMetaService.test.ts @@ -9,13 +9,13 @@ jest.mock('axios') const MockAxios = mocked(axios, true) describe('AdminMetaService', () => { - describe('getSmsVerificationStateForFormAdmin', () => { + describe('getFreeSmsCountsUsedByFormAdmin', () => { const MOCK_FORM_ID = new ObjectId().toHexString() it('should call the endpoint successfully when parameters are provided', async () => { // Arrange MockAxios.get.mockResolvedValueOnce({ data: 'some data' }) // Act - const actual = await AdminMetaService.getSmsVerificationStateForFormAdmin( + const actual = await AdminMetaService.getFreeSmsCountsUsedByFormAdmin( MOCK_FORM_ID, ) From 35e984b44462f3df6661ac6ad82e25c9acb510ce Mon Sep 17 00:00:00 2001 From: seaerchin Date: Mon, 5 Jul 2021 11:53:46 +0800 Subject: [PATCH 18/23] chore(configure-mobile.client): addressed PR comments --- .../configure-mobile.client.view.html | 2 +- .../directives/configure-mobile.client.directive.js | 13 +++++++------ src/public/translations/en-SG/main.json | 2 +- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/public/modules/forms/admin/directiveViews/configure-mobile.client.view.html b/src/public/modules/forms/admin/directiveViews/configure-mobile.client.view.html index 6646a0d9c4..989e0d131f 100644 --- a/src/public/modules/forms/admin/directiveViews/configure-mobile.client.view.html +++ b/src/public/modules/forms/admin/directiveViews/configure-mobile.client.view.html @@ -48,7 +48,7 @@ You have reached the free tier limit for SMS verification. To continue using SMS verification, - + please arrange for billing with us diff --git a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js index 3caab9aa17..427bf172f0 100644 --- a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js +++ b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js @@ -31,10 +31,12 @@ function configureMobileDirective() { '$translate', 'Toastr', function ($q, $uibModal, $scope, $translate, Toastr) { - // Get support form link from translation json. - $translate('LINKS.TWILIO_SETUP_LINK').then((twilioSetupLink) => { - $scope.twilioSetupLink = twilioSetupLink - }) + // Get the link for onboarding the form from the translation json + $translate('LINKS.VERIFIED_SMS_SETUP_LINK').then( + (verifiedSmsSetupLink) => { + $scope.verifiedSmsSetupLink = verifiedSmsSetupLink + }, + ) // NOTE: This is set on scope as it is used by the UI to determine if the toggle is loading $scope.isLoading = true @@ -63,7 +65,6 @@ function configureMobileDirective() { $scope.field.hasAdminExceededSmsLimit = $scope.adminVerifiedSmsState === ADMIN_VERIFIED_SMS_STATES.limitExceeded - $scope.field.hasRetrievalError = false }) .catch((error) => { $scope.field.hasRetrievalError = true @@ -109,7 +110,7 @@ function configureMobileDirective() { We provide SMS OTP verification for free up to 10,000 responses. OTP verification will be automatically disabled when your account reaches 10,000 responses.

If you require OTP verification for more than 10,000 responses, - please arrange advance billing with us. + please arrange advance billing with us.

Current response count: ${$scope.verifiedSmsCount}/${SMS_VERIFICATION_LIMIT} diff --git a/src/public/translations/en-SG/main.json b/src/public/translations/en-SG/main.json index b84869dd4a..297d254724 100644 --- a/src/public/translations/en-SG/main.json +++ b/src/public/translations/en-SG/main.json @@ -18,7 +18,7 @@ "GO_GOV": "https://go.gov.sg", "POSTMAN_GOV": "https://postman.gov.sg", "SUPPORT_FORM_LINK": "https://go.gov.sg/formsg-support", - "TWILIO_SETUP_LINK": "https://go.gov.sg/form-twilio-setup", + "VERIFIED_SMS_SETUP_LINK": "https://go.gov.sg/form-twilio-setup", "WHITELISTED_ATTACHMENT_TYPES": "https://go.gov.sg/formsg-cwl", "SINGPASS_ELIGIBILITY_FAQ": "https://www.ifaq.gov.sg/SINGPASS/apps/Fcd_faqmain.aspx#FAQ_2101385", "ESERVICE_ID_FAQ": "https://go.gov.sg/formsg-spcp", From 093d9e4db0a8c159a755a894c3c484fb7eeaa04b Mon Sep 17 00:00:00 2001 From: seaerchin Date: Wed, 7 Jul 2021 14:26:58 +0800 Subject: [PATCH 19/23] refactor(configure-mobile.client): uses env var instead of using constants --- .../configure-mobile.client.directive.js | 18 +++++++++++------- src/public/utils/injectedVariables.ts | 2 ++ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js index 427bf172f0..f20d30472b 100644 --- a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js +++ b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js @@ -3,11 +3,12 @@ const { get } = require('lodash') const { ADMIN_VERIFIED_SMS_STATES, - SMS_VERIFICATION_LIMIT, } = require('../../../../../shared/util/verification') const AdminMetaService = require('../../../../services/AdminMetaService') +const { injectedVariables } = require('../../../../utils/injectedVariables') + angular .module('forms') .directive('configureMobileDirective', [configureMobileDirective]) @@ -42,11 +43,15 @@ function configureMobileDirective() { $scope.isLoading = true $scope.field.hasRetrievalError = false + const formattedSmsVerificationLimit = + // Format so that it has commas; conversion is required because it's string initially + Number(injectedVariables.smsVerificationLimit).toLocaleString('en-US') + const getAdminVerifiedSmsState = (verifiedSmsCount, msgSrvcId) => { if (msgSrvcId) { return ADMIN_VERIFIED_SMS_STATES.hasMessageServiceId } - if (verifiedSmsCount < SMS_VERIFICATION_LIMIT) { + if (verifiedSmsCount < injectedVariables.smsVerificationLimit) { return ADMIN_VERIFIED_SMS_STATES.belowLimit } return ADMIN_VERIFIED_SMS_STATES.limitExceeded @@ -103,17 +108,16 @@ function configureMobileDirective() { resolve: { externalScope: function () { return { - title: - 'OTP verification will be disabled at 10,000 responses', + title: `OTP verification will be disabled at ${formattedSmsVerificationLimit} responses`, confirmButtonText: 'Accept', description: ` - We provide SMS OTP verification for free up to 10,000 responses. OTP verification will be automatically disabled when your account reaches 10,000 responses. + We provide SMS OTP verification for free up to ${formattedSmsVerificationLimit} responses. OTP verification will be automatically disabled when your account reaches ${formattedSmsVerificationLimit} responses.

- If you require OTP verification for more than 10,000 responses, + If you require OTP verification for more than ${formattedSmsVerificationLimit} responses, please arrange advance billing with us.

- Current response count: ${$scope.verifiedSmsCount}/${SMS_VERIFICATION_LIMIT} + Current response count: ${$scope.verifiedSmsCount}/${formattedSmsVerificationLimit} `, isImportant: true, } diff --git a/src/public/utils/injectedVariables.ts b/src/public/utils/injectedVariables.ts index 4909586a85..3a423b65ba 100644 --- a/src/public/utils/injectedVariables.ts +++ b/src/public/utils/injectedVariables.ts @@ -13,6 +13,7 @@ interface FrontendInjectedVariables { isCPMaintenance: string | null GATrackingID: string | null spcpCookieDomain: string | null + smsVerificationLimit: number } // NOTE: As these variables are not injected until runtime @@ -33,4 +34,5 @@ export const injectedVariables: FrontendInjectedVariables = { isCPMaintenance: formsgWindow.isCPMaintenance ?? null, GATrackingID: formsgWindow.GATrackingID ?? null, spcpCookieDomain: formsgWindow.spcpCookieDomain ?? null, + smsVerificationLimit: formsgWindow.smsVerificationLimit, } From 2d4491f54700b481867482dbf68dd5a46576121d Mon Sep 17 00:00:00 2001 From: seaerchin Date: Wed, 7 Jul 2021 14:42:49 +0800 Subject: [PATCH 20/23] refactor(mail.service): changes to use env var instead of const --- src/app/services/mail/mail.service.ts | 5 +---- src/shared/util/verification.ts | 2 -- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/app/services/mail/mail.service.ts b/src/app/services/mail/mail.service.ts index babf4ab9c6..686dd9d915 100644 --- a/src/app/services/mail/mail.service.ts +++ b/src/app/services/mail/mail.service.ts @@ -5,10 +5,7 @@ import Mail from 'nodemailer/lib/mailer' import promiseRetry from 'promise-retry' import validator from 'validator' -import { - HASH_EXPIRE_AFTER_SECONDS, - SMS_VERIFICATION_LIMIT, -} from '../../../shared/util/verification' +import { HASH_EXPIRE_AFTER_SECONDS } from '../../../shared/util/verification' import { BounceType, EmailAdminDataField, diff --git a/src/shared/util/verification.ts b/src/shared/util/verification.ts index 68cf109da6..233701cc4f 100644 --- a/src/shared/util/verification.ts +++ b/src/shared/util/verification.ts @@ -27,5 +27,3 @@ export enum ADMIN_VERIFIED_SMS_STATES { belowLimit = 'BELOW_LIMIT', hasMessageServiceId = 'MESSAGE_SERVICE_ID_OBTAINED', } - -export const SMS_VERIFICATION_LIMIT = 10000 From 5869bedfd24bcc422d08ea32e6972b6fb1fd5673 Mon Sep 17 00:00:00 2001 From: seaerchin Date: Wed, 7 Jul 2021 15:20:23 +0800 Subject: [PATCH 21/23] refactor(adminmetaservice): updated definitions to fit BE --- .../admin/directives/configure-mobile.client.directive.js | 2 +- src/public/services/AdminMetaService.ts | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js index f20d30472b..5d443ee3dc 100644 --- a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js +++ b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js @@ -60,7 +60,7 @@ function configureMobileDirective() { $q.when( AdminMetaService.getFreeSmsCountsUsedByFormAdmin($scope.form._id), ) - .then(({ smsCounts }) => { + .then((smsCounts) => { $scope.verifiedSmsCount = smsCounts $scope.adminVerifiedSmsState = getAdminVerifiedSmsState( smsCounts, diff --git a/src/public/services/AdminMetaService.ts b/src/public/services/AdminMetaService.ts index f35eeea248..8370c4ddbd 100644 --- a/src/public/services/AdminMetaService.ts +++ b/src/public/services/AdminMetaService.ts @@ -1,7 +1,5 @@ import axios from 'axios' -import { SmsCountsDto } from '../../types/api' - /** Exported for testing */ export const FORM_API_PREFIX = '/api/v3/admin/forms' @@ -12,7 +10,7 @@ export const FORM_API_PREFIX = '/api/v3/admin/forms' */ export const getFreeSmsCountsUsedByFormAdmin = ( formId: string, -): Promise => +): Promise => axios - .get(`${FORM_API_PREFIX}/${formId}/verified-sms/count/free`) + .get(`${FORM_API_PREFIX}/${formId}/verified-sms/count/free`) .then(({ data }) => data) From d90e4419f9ab4fc2dca501c65e9e1258677a9018 Mon Sep 17 00:00:00 2001 From: seaerchin Date: Wed, 14 Jul 2021 11:42:26 +0800 Subject: [PATCH 22/23] refactor(configure-mobile.client): changed condition to be same as BE --- .../forms/admin/directives/configure-mobile.client.directive.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js index 5d443ee3dc..93e6c0f826 100644 --- a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js +++ b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js @@ -51,7 +51,7 @@ function configureMobileDirective() { if (msgSrvcId) { return ADMIN_VERIFIED_SMS_STATES.hasMessageServiceId } - if (verifiedSmsCount < injectedVariables.smsVerificationLimit) { + if (verifiedSmsCount <= injectedVariables.smsVerificationLimit) { return ADMIN_VERIFIED_SMS_STATES.belowLimit } return ADMIN_VERIFIED_SMS_STATES.limitExceeded From cf5ff26316057d795c481a8e88c597532eeacf79 Mon Sep 17 00:00:00 2001 From: seaerchin Date: Wed, 14 Jul 2021 11:53:09 +0800 Subject: [PATCH 23/23] refactor(configure-mobile.client.directive): extracts formatting to own function --- .../configure-mobile.client.directive.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js index 93e6c0f826..7f2a4b25c3 100644 --- a/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js +++ b/src/public/modules/forms/admin/directives/configure-mobile.client.directive.js @@ -39,13 +39,18 @@ function configureMobileDirective() { }, ) + // Formats a given string as a number by setting it to US locale. + // Concretely, this adds commas between every thousand. + const formatStringAsNumber = (num) => + Number(num).toLocaleString('en-US') + // NOTE: This is set on scope as it is used by the UI to determine if the toggle is loading $scope.isLoading = true $scope.field.hasRetrievalError = false const formattedSmsVerificationLimit = // Format so that it has commas; conversion is required because it's string initially - Number(injectedVariables.smsVerificationLimit).toLocaleString('en-US') + formatStringAsNumber(injectedVariables.smsVerificationLimit) const getAdminVerifiedSmsState = (verifiedSmsCount, msgSrvcId) => { if (msgSrvcId) { @@ -114,10 +119,14 @@ function configureMobileDirective() { We provide SMS OTP verification for free up to ${formattedSmsVerificationLimit} responses. OTP verification will be automatically disabled when your account reaches ${formattedSmsVerificationLimit} responses.

If you require OTP verification for more than ${formattedSmsVerificationLimit} responses, - please arrange advance billing with us. + please arrange advance billing with us.

- Current response count: ${$scope.verifiedSmsCount}/${formattedSmsVerificationLimit} + Current response count: ${formatStringAsNumber( + $scope.verifiedSmsCount, + )}/${formattedSmsVerificationLimit} `, isImportant: true, }