abstract email validator, update test cases

opengovsg · Nov 22, 2024 · 5d1626a · 5d1626a
1 parent 576578f
commit 5d1626a
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 30 deletions.
diff --git a/backend/src/email/routes/email-transactional.routes.ts b/backend/src/email/routes/email-transactional.routes.ts
@@ -21,14 +21,16 @@ export const InitEmailTransactionalRoute = (
   const router = Router({ mergeParams: true })
 
   // Validators
+  const emailValidator = Joi.string()
+    .trim()
+    .email()
+    .options({ convert: true })
+    .lowercase()
+
   const emailArrayValidation = (fieldName: 'cc' | 'bcc') => {
     return Joi.alternatives().try(
       // array
-      Joi.array()
-        .unique()
-        .items(
-          Joi.string().trim().email().options({ convert: true }).lowercase()
-        ),
+      Joi.array().unique().items(emailValidator),
 
       // stringified array (form-data)
       Joi.string().custom((value: string) => {
@@ -46,9 +48,7 @@ export const InitEmailTransactionalRoute = (
         }
         const { value: validatedEmails, error } = Joi.array()
           .unique()
-          .items(
-            Joi.string().email().trim().lowercase().options({ convert: true })
-          )
+          .items(emailValidator)
           .validate(parsed)
 
         if (error) {
@@ -82,11 +82,7 @@ export const InitEmailTransactionalRoute = (
           )
         ),
       from: fromAddressValidator,
-      reply_to: Joi.string()
-        .trim()
-        .email()
-        .options({ convert: true })
-        .lowercase(),
+      reply_to: emailValidator,
       attachments: Joi.array().items(Joi.object().keys().required()),
       classification: Joi.string()
         .uppercase()

diff --git a/backend/src/email/routes/tests/email-transactional.routes.test.ts b/backend/src/email/routes/tests/email-transactional.routes.test.ts
@@ -866,14 +866,11 @@ describe(`${emailTransactionalRoute}/send`, () => {
   })
 
   const ccValidTests = [
-    ['[email protected]', '[email protected]'],
-    JSON.stringify([
-      '[email protected]',
-      '[email protected]',
-    ]),
+    [['[email protected]']],
+    [['[email protected]', '[email protected]']],
   ]
   test.each(ccValidTests)(
-    'Should throw api validation error if cc is not valid array or stringified array - JSON payload',
+    'Should send email with cc from valid array or stringified array - JSON payload',
     async (cc) => {
       mockSendEmail = jest
         .spyOn(EmailService, 'sendEmail')
@@ -887,10 +884,8 @@ describe(`${emailTransactionalRoute}/send`, () => {
           reply_to: user.email,
         })
 
-      const arrayToCheck = Array.isArray(cc) ? cc : JSON.parse(cc)
-
       expect(res.status).toBe(201)
-      expect(res.body.cc.sort()).toStrictEqual(arrayToCheck.sort())
+      expect(res.body.cc.sort()).toStrictEqual(cc.sort())
       expect(mockSendEmail).toBeCalledTimes(1)
       const transactionalEmail = await EmailMessageTransactional.findOne({
         where: { id: res.body.id },
@@ -906,11 +901,14 @@ describe(`${emailTransactionalRoute}/send`, () => {
   )
 
   test.each(ccValidTests)(
-    'Should throw api validation error if cc is not valid array or stringified array - form-data',
+    'Should send email with cc from valid array or stringified array - form-data',
     async (cc) => {
       mockSendEmail = jest
         .spyOn(EmailService, 'sendEmail')
         .mockResolvedValue(true)
+      // in the case where single cc is sent, stringify the cc list
+      const ccSend = cc.length === 1 ? JSON.stringify(cc) : cc
+
       const res = await request(app)
         .post(endpoint)
         .set('Authorization', `Bearer ${apiKey}`)
@@ -919,12 +917,10 @@ describe(`${emailTransactionalRoute}/send`, () => {
         .field('body', validApiCall.body)
         .field('from', 'Postman <[email protected]>')
         .field('reply_to', validApiCall.reply_to)
-        .field('cc', cc)
-
-      const arrayToCheck = Array.isArray(cc) ? cc : JSON.parse(cc)
+        .field('cc', ccSend)
 
       expect(res.status).toBe(201)
-      expect(res.body.cc.sort()).toStrictEqual(arrayToCheck.sort())
+      expect(res.body.cc.sort()).toStrictEqual(cc.sort())
       expect(mockSendEmail).toBeCalledTimes(1)
       const transactionalEmail = await EmailMessageTransactional.findOne({
         where: { id: res.body.id },
@@ -949,7 +945,7 @@ describe(`${emailTransactionalRoute}/send`, () => {
         transactionalEmail?.emailMessageTransactionalCc.map(
           (item) => item.email
         )
-      expect(transactionalCcEmails?.sort()).toStrictEqual(arrayToCheck.sort())
+      expect(transactionalCcEmails?.sort()).toStrictEqual(cc.sort())
     }
   )
 
@@ -995,14 +991,13 @@ describe(`${emailTransactionalRoute}/send`, () => {
   )
 
   test.each(ccInvalidTests)(
-    'Should throw api validation error if cc is not valid array or stringified array - JSON payload',
+    'Should throw api validation error if cc is not valid array or stringified array - form-data',
     async ({ cc, errMsg }) => {
       mockSendEmail = jest.spyOn(EmailService, 'sendEmail')
 
       const res = await request(app)
         .post(endpoint)
         .set('Authorization', `Bearer ${apiKey}`)
-        .set('Authorization', `Bearer ${apiKey}`)
         .field('recipient', validApiCall.recipient)
         .field('subject', validApiCall.subject)
         .field('body', validApiCall.body)