feat: change nric mask toggle to nric collection toggle (#7566)

* feat: change nric mask toggle to nric collection toggle * feat: display submitter id collection status to respondent * feat: disable submitter id collection when toggle is enabled * feat: remove masking of nric in preview and form builder * feat: add chromatic tests * fix: change copy to singpass login id * feat: add encrypt submission unit test coverage for submitter id collection across various singpass authtypes * feat: add tests for email mode * fix: remove single quote from copy * fix: change copy for corppass * fix: adapt playwright tests for nric opt-out * fix: use custom object to override isSubmitterIdCollectionEnabled in e2e test instead of providing through argument * fix: move iife to outside component * fix: failing tc due to changes in errorcode formatting and hashing of caps * feat: add growthbook support for backend * feat: wrap opt in out feature in feature flag * feat: remove growthbook for test env, make env config export more precise, cleanup growthbook usage in FE * fix: update form defaults in test case to set isSubmitterIdCollected to true for rollout * feat: make storybook test env so that we can disable Growthbook for storybook runs based on test env * fix: remove unused useEffect dependency * fix: logic bug with disabling submitterIdCollection feature for test mode and based on feature flag * fix: failing e2e test due to mismatch default form setting for isSubmitterIdCollectionEnabled * fix: add exhaustive checks on switchcase --------- Co-authored-by: Ken <[email protected]>
opengovsg · Aug 20, 2024 · 1f42038 · 1f42038
1 parent 2b5ed0a
commit 1f42038
Show file tree

Hide file tree

Showing 66 changed files with 1,093 additions and 441 deletions.
diff --git a/__tests__/e2e/constants/settings.ts b/__tests__/e2e/constants/settings.ts
@@ -8,14 +8,19 @@ export type Collaborator = {
 export type E2eSettingsOptions = {
   status: FormStatus
   collaborators: Collaborator[]
+  isSubmitterIdCollectionEnabled: boolean
   responseLimit?: number
   closedFormMessage?: string
   emails?: string[]
   authType: FormAuthType
   /** If authType is SPCP/MyInfo, eserviceId is required. */
   esrvcId?: string
-  /** If authType is non-NIL, nric is required. */
+  /**
+   * Represents singpass/corppass validated nric.
+   * If authType is non-NIL, nric is required. */
   nric?: string
-  /** If authType is CP, uen is required */
+  /**
+   * Represents corppass validated uen.
+   * If authType is CP, uen is required. */
   uen?: string
 }
diff --git a/__tests__/e2e/helpers/verifySubmission.ts b/__tests__/e2e/helpers/verifySubmission.ts
@@ -15,6 +15,7 @@ import {
 import {
   expectAttachment,
   expectContains,
+  expectNotToContain,
   // expectToast,
   getAutoreplyEmail,
   getResponseArray,
@@ -127,7 +128,10 @@ export const verifyEmailSubmission = async (
     expectAttachment(field, submission.attachments)
   }
 
-  if (formSettings.authType !== FormAuthType.NIL) {
+  if (
+    formSettings.isSubmitterIdCollectionEnabled &&
+    formSettings.authType !== FormAuthType.NIL
+  ) {
     // Verify that form auth correctly returned NRIC (SPCP/SGID) and UEN (CP)
     if (!formSettings.nric) throw new Error('No nric provided!')
     switch (formSettings.authType) {
@@ -216,7 +220,10 @@ export const verifyEncryptSubmission = async (
       expectAttachment(field, submission.attachments)
     }
 
-    if (formSettings.authType !== FormAuthType.NIL) {
+    if (
+      formSettings.isSubmitterIdCollectionEnabled &&
+      formSettings.authType !== FormAuthType.NIL
+    ) {
       // Verify that form auth correctly returned NRIC (SPCP/SGID) and UEN (CP)
       if (!formSettings.nric) throw new Error('No nric provided!')
       switch (formSettings.authType) {
@@ -234,6 +241,23 @@ export const verifyEncryptSubmission = async (
           break
       }
     }
+
+    const expectSubmissionNotToContain = expectNotToContain(submission.html)
+    if (!formSettings.isSubmitterIdCollectionEnabled) {
+      // Verify that the submission does not contain any singpass or corppass validated fields
+      expectSubmissionNotToContain([
+        SPCPFieldTitle.SpNric,
+        SPCPFieldTitle.CpUid,
+        SPCPFieldTitle.CpUen,
+        SgidFieldTitle.SgidNric,
+      ])
+      if (formSettings.nric) {
+        expectSubmissionNotToContain([formSettings.nric])
+      }
+      if (formSettings.uen) {
+        expectSubmissionNotToContain([formSettings.uen])
+      }
+    }
   }
 
   // Step 2: Download the response using secret key

diff --git a/__tests__/e2e/utils/response.ts b/__tests__/e2e/utils/response.ts
@@ -149,6 +149,18 @@ export const expectContains =
     }
   }
 
+/**
+ * Tests that container does not contain any of the values in contained.
+ * @param {string} container string in which to search
+ * @param {string[]} containedArray Array of values to search for
+ */
+export const expectNotToContain =
+  (container: string) => (containedArray: string[]) => {
+    for (const contained of containedArray) {
+      expect(container).not.toContain(contained)
+    }
+  }
+
 /**
  * Checks that an attachment field's attachment is contained in the email.
  * @param {E2eFieldMetadata} field field used to create and fill form

diff --git a/__tests__/e2e/utils/settings.ts b/__tests__/e2e/utils/settings.ts
@@ -42,6 +42,7 @@ const _getSettings = (
     status: FormStatus.Public,
     collaborators: [],
     authType: FormAuthType.NIL,
+    isSubmitterIdCollectionEnabled: true, // TODO: (E-voting v1.0.1) Set back to false when form.server.model default is set to false
     // By default, if emails is undefined, only the admin (current user) will receive.
     ...custom,
   }

diff --git a/frontend/.storybook/main.js b/frontend/.storybook/main.js
@@ -41,4 +41,10 @@ module.exports = {
       },
     }
   },
+  // For injecitng environment variables into Storybook runs
+  // (see: https://github.com/storybookjs/storybook/issues/12270#issuecomment-1139104523)
+  env: (config) => ({
+    ...config,
+    NODE_ENV: 'test',
+  }),
 }
diff --git a/frontend/src/constants/links.ts b/frontend/src/constants/links.ts
@@ -75,6 +75,3 @@ export const OGP_SGID = 'https://go.gov.sg/sgid-formsg'
 export const OGP_FORMSG_REPO = 'https://github.com/opengovsg/formsg'
 
 export const FORMSG_UAT = 'https://uat.form.gov.sg'
-
-export const GROWTHBOOK_DEV_PROXY =
-  'https://proxy-growthbook-stg.formsg.workers.dev'
diff --git a/frontend/src/constants/routes.ts b/frontend/src/constants/routes.ts
@@ -54,6 +54,3 @@ export const ACTIVE_ADMINFORM_RESULTS_ROUTE_REGEX = new RegExp(
 
 export const PAYMENT_PAGE_SUBROUTE = 'payment/:paymentId'
 export const EDIT_SUBMISSION_PAGE_SUBROUTE = 'edit/:submissionId'
-
-// Path for growthbook api proxy, set up on cloudflare workers. Worker script: https://github.com/opengovsg/formsg-private/pull/171.
-export const GROWTHBOOK_API_HOST_PATH = '/api/v1/proxy/growthbook'
diff --git a/frontend/src/features/admin-form/common/AdminViewFormService.ts b/frontend/src/features/admin-form/common/AdminViewFormService.ts
@@ -24,10 +24,7 @@ import {
   filterHiddenInputs,
 } from '~features/public-form/utils'
 
-import {
-  PREVIEW_MASKED_MOCK_UINFIN,
-  PREVIEW_MOCK_UINFIN,
-} from '../preview/constants'
+import { PREVIEW_MOCK_UINFIN } from '../preview/constants'
 
 // endpoint exported for testing
 export const ADMIN_FORM_ENDPOINT = '/admin/forms'
@@ -62,9 +59,7 @@ export const previewForm = async (
       // and if server has not already sent back a mock authenticated state.
       if (data.form.authType !== FormAuthType.NIL && !data.spcpSession) {
         data.spcpSession = {
-          userName: data.form.isNricMaskEnabled
-            ? PREVIEW_MASKED_MOCK_UINFIN
-            : PREVIEW_MOCK_UINFIN,
+          userName: PREVIEW_MOCK_UINFIN,
         }
       }
 
@@ -95,9 +90,7 @@ export const viewFormTemplate = async (
       // and if server has not already sent back a mock authenticated state.
       if (data.form.authType !== FormAuthType.NIL && !data.spcpSession) {
         data.spcpSession = {
-          userName: data.form.isNricMaskEnabled
-            ? PREVIEW_MASKED_MOCK_UINFIN
-            : PREVIEW_MOCK_UINFIN,
+          userName: PREVIEW_MOCK_UINFIN,
         }
       }
 

diff --git a/...c/features/admin-form/create/builder-and-design/BuilderAndDesignContent/StartPageView.tsx b/...c/features/admin-form/create/builder-and-design/BuilderAndDesignContent/StartPageView.tsx
@@ -6,10 +6,7 @@ import { FormAuthType, FormLogoState, FormStartPage } from '~shared/types'
 
 import { useIsMobile } from '~hooks/useIsMobile'
 
-import {
-  PREVIEW_MASKED_MOCK_UINFIN as PREVIEW_MASKED_MOCK_UINFIN,
-  PREVIEW_MOCK_UINFIN,
-} from '~features/admin-form/preview/constants'
+import { PREVIEW_MOCK_UINFIN } from '~features/admin-form/preview/constants'
 import { useEnv } from '~features/env/queries'
 import { FormInstructions } from '~features/public-form/components/FormInstructions/FormInstructions'
 import {
@@ -200,9 +197,7 @@ export const StartPageView = () => {
           showHeader
           loggedInId={
             form && form.authType !== FormAuthType.NIL
-              ? form.isNricMaskEnabled
-                ? PREVIEW_MASKED_MOCK_UINFIN
-                : PREVIEW_MOCK_UINFIN
+              ? PREVIEW_MOCK_UINFIN
               : undefined
           }
           {...formHeaderProps}

diff --git a/...te/builder-and-design/BuilderAndDesignDrawer/FieldListDrawer/field-panels/MyInfoPanel.tsx b/...te/builder-and-design/BuilderAndDesignDrawer/FieldListDrawer/field-panels/MyInfoPanel.tsx
@@ -2,9 +2,8 @@ import { useCallback, useEffect, useMemo } from 'react'
 import { Droppable } from 'react-beautiful-dnd'
 import { Link as ReactLink } from 'react-router-dom'
 import { Box, Text } from '@chakra-ui/react'
-import { useFeatureIsOn, useGrowthBook } from '@growthbook/growthbook-react'
+import { useGrowthBook } from '@growthbook/growthbook-react'
 
-import { featureFlags } from '~shared/constants'
 import {
   AdminFormDto,
   FormAuthType,

diff --git a/frontend/src/features/admin-form/create/end-page/EndPageContent.tsx b/frontend/src/features/admin-form/create/end-page/EndPageContent.tsx
@@ -13,10 +13,7 @@ import { PaymentsThankYouSvgr } from '~components/FormEndPage/PaymentsThankYouSv
 import { ThankYouSvgr } from '~components/FormEndPage/ThankYouSvgr'
 
 import { useAdminForm } from '~features/admin-form/common/queries'
-import {
-  PREVIEW_MASKED_MOCK_UINFIN,
-  PREVIEW_MOCK_UINFIN,
-} from '~features/admin-form/preview/constants'
+import { PREVIEW_MOCK_UINFIN } from '~features/admin-form/preview/constants'
 import { useEnv } from '~features/env/queries'
 import {
   FormBannerLogo,
@@ -93,9 +90,7 @@ export const EndPageContent = (): JSX.Element => {
           onLogout={undefined}
           loggedInId={
             form && form.authType !== FormAuthType.NIL
-              ? form.isNricMaskEnabled
-                ? PREVIEW_MASKED_MOCK_UINFIN
-                : PREVIEW_MOCK_UINFIN
+              ? PREVIEW_MOCK_UINFIN
               : undefined
           }
         />

diff --git a/frontend/src/features/admin-form/preview/constants.ts b/frontend/src/features/admin-form/preview/constants.ts
@@ -1,2 +1 @@
 export const PREVIEW_MOCK_UINFIN = 'S1234567A'
-export const PREVIEW_MASKED_MOCK_UINFIN = '*****567A'
diff --git a/frontend/src/features/admin-form/responses/ResponsesPage/storage/useDecryptionWorkers.ts b/frontend/src/features/admin-form/responses/ResponsesPage/storage/useDecryptionWorkers.ts
@@ -1,7 +1,7 @@
 import { useCallback, useEffect, useRef, useState } from 'react'
 import { useMutation, UseMutationOptions } from 'react-query'
 import { datadogLogs } from '@datadog/browser-logs'
-import { useFeature } from '@growthbook/growthbook-react'
+import { useFeatureIsOn } from '@growthbook/growthbook-react'
 
 import { waitForMs } from '~utils/waitForMs'
 
@@ -82,10 +82,9 @@ const useDecryptionWorkers = ({
   const { data: adminForm } = useAdminForm()
   const { user } = useUser()
 
-  const isDev = process.env.NODE_ENV === 'development'
-
-  const fasterDownloadsFeature = useFeature('faster-downloads')
-  const fasterDownloads = fasterDownloadsFeature.on || isDev
+  const isTest = process.env.NODE_ENV === 'test'
+  const isFasterDownloadsFeatureOn = useFeatureIsOn('faster-downloads')
+  const isFasterDownloadsEnabled = isTest || isFasterDownloadsFeatureOn
 
   useEffect(() => {
     return () => killWorkers(workers)
@@ -191,7 +190,7 @@ const useDecryptionWorkers = ({
                     formId: adminForm._id,
                     hostOrigin: window.location.origin,
                   },
-                  fasterDownloads,
+                  isFasterDownloadsEnabled,
                 )
                 progress += 1
                 onProgress(progress)
@@ -373,7 +372,7 @@ const useDecryptionWorkers = ({
           })
       })
     },
-    [adminForm, onProgress, user?._id, workers, fasterDownloads],
+    [adminForm, onProgress, user?._id, workers, isFasterDownloadsEnabled],
   )
 
   const downloadEncryptedResponsesFaster = useCallback(
@@ -455,7 +454,7 @@ const useDecryptionWorkers = ({
             formId: adminForm._id,
             hostOrigin: window.location.origin,
           },
-          fasterDownloads,
+          isFasterDownloadsEnabled,
         )
 
         switch (decryptResult.status) {
@@ -679,12 +678,12 @@ const useDecryptionWorkers = ({
           })
       })
     },
-    [adminForm, onProgress, user?._id, workers, fasterDownloads],
+    [adminForm, onProgress, user?._id, workers, isFasterDownloadsEnabled],
   )
 
   const handleExportCsvMutation = useMutation(
     (params: DownloadEncryptedParams) =>
-      fasterDownloads
+      isFasterDownloadsEnabled
         ? downloadEncryptedResponsesFaster(params)
         : downloadEncryptedResponses(params),
     mutateProps,

diff --git a/frontend/src/features/admin-form/responses/ResponsesPage/storage/worker/decryption.worker.ts b/frontend/src/features/admin-form/responses/ResponsesPage/storage/worker/decryption.worker.ts
@@ -68,7 +68,7 @@ function verifySignature(
  */
 async function decryptIntoCsv(
   data: LineData,
-  fasterDownloads: boolean,
+  isFasterDownloadsEnabled: boolean,
 ): Promise<MaterializedCsvRecord> {
   // This needs to be dynamically imported due to sharing code between main app and worker code.
   // Fixes issue raised at https://stackoverflow.com/questions/66472945/referenceerror-refreshreg-is-not-defined
@@ -192,7 +192,7 @@ async function decryptIntoCsv(
             CsvRecordStatus.Ok,
             'Success (with Downloaded Attachment)',
           )
-          if (fasterDownloads) {
+          if (isFasterDownloadsEnabled) {
             csvRecord.downloadBlobURL = URL.createObjectURL(downloadBlob)
           } else {
             csvRecord.setDownloadBlob(downloadBlob)

diff --git a/frontend/src/features/admin-form/settings/SettingsAuthPage.stories.tsx b/frontend/src/features/admin-form/settings/SettingsAuthPage.stories.tsx
@@ -77,13 +77,13 @@ PublicStorageNilAuthForm.parameters = {
   }),
 }
 
-// purpose: tests that isNricMaskEnabled should not affect setting options available
-export const PublicStorageNilAuthFormNricMaskingEnabled = Template.bind({})
-PublicStorageNilAuthFormNricMaskingEnabled.parameters = {
+export const PublicStorageNilAuthFormSubmitterIdCollectionEnabled =
+  Template.bind({})
+PublicStorageNilAuthFormSubmitterIdCollectionEnabled.parameters = {
   msw: buildEncryptModeMswRoutes({
     responseMode: FormResponseMode.Encrypt,
     status: FormStatus.Public,
-    isNricMaskEnabled: true,
+    isSubmitterIdCollectionEnabled: true,
   }),
 }
 
@@ -143,22 +143,25 @@ PublicEmailMyInfoForm.parameters = {
   ],
 }
 
-export const PrivateEmailSingpassFormNricMaskingEnabled = Template.bind({})
-PrivateEmailSingpassFormNricMaskingEnabled.parameters = {
+export const PrivateEmailSingpassFormSubmitterIdCollectionEnabled =
+  Template.bind({})
+PrivateEmailSingpassFormSubmitterIdCollectionEnabled.parameters = {
   msw: buildEmailModeMswRoutes({
     status: FormStatus.Private,
     authType: FormAuthType.SGID,
-    isNricMaskEnabled: true,
+    isSubmitterIdCollectionEnabled: true,
   }),
 }
-export const PrivateEmailMyInfoFormNricMaskingEnabled = Template.bind({})
-PrivateEmailMyInfoFormNricMaskingEnabled.parameters = {
+export const PrivateEmailMyInfoFormSubmitterIdCollectionEnabled = Template.bind(
+  {},
+)
+PrivateEmailMyInfoFormSubmitterIdCollectionEnabled.parameters = {
   msw: [
     ...buildEmailModeMswRoutes({
       status: FormStatus.Private,
       authType: FormAuthType.MyInfo,
       esrvcId: 'STORYBOOK-TEST',
-      isNricMaskEnabled: true,
+      isSubmitterIdCollectionEnabled: true,
     }),
     ...createFormBuilderMocks({ form_fields: MOCK_FORM_FIELDS_WITH_MYINFO }),
   ],
@@ -180,7 +183,7 @@ PrivateStorageSingpassFormAllTogglesEnabled.parameters = {
     status: FormStatus.Private,
     authType: FormAuthType.SGID,
     isSingleSubmission: true,
-    isNricMaskEnabled: true,
+    isSubmitterIdCollectionEnabled: true,
   }),
 }
 
@@ -190,7 +193,7 @@ PublicEmailCorppassAllTogglesEnabledForm.parameters = {
     status: FormStatus.Public,
     authType: FormAuthType.CP,
     isSingleSubmission: true,
-    isNricMaskEnabled: true,
+    isSubmitterIdCollectionEnabled: true,
   }),
 }
 

diff --git a/frontend/src/features/admin-form/settings/SettingsService.ts b/frontend/src/features/admin-form/settings/SettingsService.ts
@@ -109,12 +109,11 @@ export const updateFormAuthType: UpdateFormFn<'authType'> = async (
   return updateFormSettings(formId, { authType: newAuthType })
 }
 
-export const updateFormNricMask: UpdateFormFn<'isNricMaskEnabled'> = async (
-  formId,
-  newIsNricMaskEnabled,
-) => {
+export const updateIsSubmitterIdCollectionEnabled: UpdateFormFn<
+  'isSubmitterIdCollectionEnabled'
+> = async (formId, nextIsSubmitterIdCollectionEnabled) => {
   return updateFormSettings(formId, {
-    isNricMaskEnabled: newIsNricMaskEnabled,
+    isSubmitterIdCollectionEnabled: nextIsSubmitterIdCollectionEnabled,
   })
 }