refactor: standardise usage of getUniqueMyInfoAttrs (#636)

* ref: rename uniq function * ref: remove extractRequestedAttributes * ref: remove getRequestedAttrs middleware
opengovsg · Nov 19, 2020 · 3880f6e · 3880f6e
1 parent 9e657df
commit 3880f6e
Show file tree

Hide file tree

Showing 9 changed files with 13 additions and 51 deletions.
diff --git a/src/app/controllers/email-submissions.server.controller.js b/src/app/controllers/email-submissions.server.controller.js
@@ -572,12 +572,11 @@ function concatResponse(formData, attachments) {
  */
 exports.saveMetadataToDb = function (req, res, next) {
   const { form, attachments, formData } = req
-  const { requestedAttributes } = res.locals
 
   let submission = new emailSubmission({
     form: form._id,
     authType: form.authType,
-    myInfoFields: requestedAttributes,
+    myInfoFields: form.getUniqueMyInfoAttrs(),
     recipientEmails: form.emails,
   })
 

diff --git a/src/app/controllers/encrypt-submissions.server.controller.js b/src/app/controllers/encrypt-submissions.server.controller.js
@@ -150,7 +150,7 @@ function onEncryptSubmissionFailure(err, req, res, submission) {
  */
 exports.saveResponseToDb = function (req, res, next) {
   const { form, formData, attachmentData } = req
-  const { verified, requestedAttributes } = res.locals
+  const { verified } = res.locals
   let attachmentMetadata = new Map()
   let attachmentUploadPromises = []
 
@@ -193,7 +193,7 @@ exports.saveResponseToDb = function (req, res, next) {
   const submission = new EncryptSubmission({
     form: form._id,
     authType: form.authType,
-    myInfoFields: requestedAttributes,
+    myInfoFields: form.getUniqueMyInfoAttrs(),
     encryptedContent: formData,
     verifiedContent: verified,
     attachmentMetadata,

diff --git a/src/app/controllers/myinfo.server.controller.ts b/src/app/controllers/myinfo.server.controller.ts
@@ -16,10 +16,7 @@ import {
   WithForm,
 } from '../../types'
 import { MyInfoFactory } from '../services/myinfo/myinfo.factory'
-import {
-  extractRequestedAttributes,
-  mapVerifyMyInfoError,
-} from '../services/myinfo/myinfo.util'
+import { mapVerifyMyInfoError } from '../services/myinfo/myinfo.util'
 import { createReqMeta } from '../utils/request'
 
 const logger = createLoggerWithLabel(module)
@@ -40,7 +37,9 @@ export const addMyInfo: RequestHandler<ParamsDictionary> = async (
   const { esrvcId, authType, form_fields: formFields, _id: formId } = form
 
   // Early return if nothing needs to be done.
-  const requestedAttributes = extractRequestedAttributes(formFields)
+  const requestedAttributes = (req as WithForm<
+    typeof req
+  >).form.getUniqueMyInfoAttrs()
   if (!uinFin || authType !== AuthType.SP || requestedAttributes.length === 0) {
     return next()
   }
@@ -94,11 +93,11 @@ export const verifyMyInfoVals: RequestHandler<
   { parsedResponses: ProcessedFieldResponse[] }
 > = async (req, res, next) => {
   // TODO (#42): add proper types here when migrating away from middleware pattern
-  const { authType, _id: formId, form_fields: formFields } = (req as WithForm<
-    typeof req
-  >).form.toJSON()
+  const { authType, _id: formId } = (req as WithForm<typeof req>).form.toJSON()
   const uinFin = (res as ResWithUinFin<typeof res>).locals.uinFin
-  const requestedAttributes = extractRequestedAttributes(formFields)
+  const requestedAttributes = (req as WithForm<
+    typeof req
+  >).form.getUniqueMyInfoAttrs()
   if (authType !== AuthType.SP || requestedAttributes.length === 0) {
     return next()
   } else if (!uinFin) {

diff --git a/src/app/controllers/spcp.server.controller.js b/src/app/controllers/spcp.server.controller.js
@@ -373,17 +373,6 @@ exports.addSpcpSessionInfo = (authClients) => {
   }
 }
 
-/**
- * Get MyInfo attributes that are requested in this particular submission
- * @param  {Object} req - Express request object
- * @param  {Object} res - Express response object
- */
-exports.getRequestedAttributes = function (req, res, next) {
-  const { form } = req
-  res.locals.requestedAttributes = form.getUniqMyinfoAttrs() || []
-  return next()
-}
-
 /**
  * Encrypt and sign verified fields if exist
  * @param  {Object} req - Express request object

diff --git a/src/app/factories/spcp.factory.js b/src/app/factories/spcp.factory.js
@@ -65,7 +65,6 @@ const spcpFactory = ({ isEnabled, props }) => {
     }
 
     return {
-      getRequestedAttributes: spcp.getRequestedAttributes,
       appendVerifiedSPCPResponses: spcp.appendVerifiedSPCPResponses,
       passThroughSpcp: admin.passThroughSpcp,
       returnSpcpRedirectURL: spcp.returnSpcpRedirectURL,
@@ -79,10 +78,6 @@ const spcpFactory = ({ isEnabled, props }) => {
   } else {
     const errMsg = 'SPCP/MyInfo feature is not enabled'
     return {
-      getRequestedAttributes: (req, res, next) => {
-        res.locals.requestedAttributes = []
-        return next()
-      },
       appendVerifiedSPCPResponses: (req, res, next) => next(),
       passThroughSpcp: (req, res, next) => next(),
       returnSpcpRedirectURL: (req, res) =>

diff --git a/src/app/models/form.server.model.ts b/src/app/models/form.server.model.ts
@@ -400,7 +400,7 @@ const compileFormModel = (db: Mongoose): IFormModel => {
   }
 
   // Method to return myInfo attributes
-  FormSchema.methods.getUniqMyinfoAttrs = function (this: IFormSchema) {
+  FormSchema.methods.getUniqueMyInfoAttrs = function (this: IFormSchema) {
     if (this.authType !== AuthType.SP) {
       return []
     }

diff --git a/src/app/routes/public-forms.server.routes.js b/src/app/routes/public-forms.server.routes.js
@@ -193,7 +193,6 @@ module.exports = function (app) {
     submissions.injectAutoReplyInfo,
     spcpFactory.appendVerifiedSPCPResponses,
     emailSubmissions.prepareEmailSubmission,
-    spcpFactory.getRequestedAttributes,
     emailSubmissions.saveMetadataToDb,
     emailSubmissions.sendAdminEmail,
     submissions.sendAutoReply,
@@ -272,7 +271,6 @@ module.exports = function (app) {
     submissions.injectAutoReplyInfo,
     webhookVerifiedContentFactory.encryptedVerifiedFields,
     encryptSubmissions.prepareEncryptSubmission,
-    spcpFactory.getRequestedAttributes,
     encryptSubmissions.saveResponseToDb,
     webhookVerifiedContentFactory.post,
     submissions.sendAutoReply,

diff --git a/src/app/services/myinfo/myinfo.util.ts b/src/app/services/myinfo/myinfo.util.ts
@@ -11,7 +11,6 @@ import moment from 'moment'
 import { createLoggerWithLabel } from '../../../config/logger'
 import {
   BasicField,
-  IFieldSchema,
   IHashes,
   MapRouteError,
   MyInfoAttribute,
@@ -142,23 +141,6 @@ export const hashFieldValues = (
   return readOnlyHashPromises
 }
 
-/**
- * Extracts the MyInfo attributes in an array of form fields.
- * @param formFields Array of form fields
- * @returns array of the MyInfo attributes present in the form fields
- */
-export const extractRequestedAttributes = (
-  formFields: IFieldSchema[],
-): MyInfoAttribute[] => {
-  const attrs: MyInfoAttribute[] = []
-  formFields.forEach((field) => {
-    if (field.myInfo?.attr) {
-      attrs.push(field.myInfo.attr)
-    }
-  })
-  return attrs
-}
-
 /**
  * Whether a field contains a MyInfo response
  * @param field a processed response with the isVisible attribute

diff --git a/src/types/form.ts b/src/types/form.ts
@@ -104,7 +104,7 @@ export interface IForm {
 
 export interface IFormSchema extends IForm, Document {
   getMainFields(): Pick<IFormSchema, '_id' | 'title' | 'status'>
-  getUniqMyinfoAttrs(): MyInfoAttribute[]
+  getUniqueMyInfoAttrs(): MyInfoAttribute[]
   duplicate(overrideProps: Partial<IForm>): Partial<IFormSchema>
   transferOwner(currentOwner: IUserSchema, newOwnerEmail: string): void
 }