chore: address PR comments, fix tests

Author: kevinkim-ogp

packages/backend/src/apps/postman/__tests__/actions/send-transactional-email.test.ts

@@ -626,7 +626,7 @@ describe('send transactional email', () => {
     })
   })
 
-  it('should only send one email if there are blacklisted recipients and invalid attachments', async () => {
+  it('should send two emails if there are blacklisted recipients and invalid attachments', async () => {
     const recipients = ['[email protected]', '[email protected]']
     $.step.parameters.destinationEmail = recipients.join(',')
     $.step.parameters.attachments = [
@@ -687,5 +687,15 @@ describe('send transactional email', () => {
       executionId: $.execution.id,
       blacklistedRecipients: [recipients[1]],
     })
+    expect(mocks.sendInvalidAttachmentsEmail).toHaveBeenCalledWith({
+      flowName: $.flow.name,
+      flowId: $.flow.id,
+      userEmail: $.user.email,
+      executionId: $.execution.id,
+      submissionId: 'abc',
+      formAdminLink: 'https://form.gov.sg/admin/form/123/results',
+      invalidAttachments: ['file-2.svg'],
+      hasInvalidAttachments: true,
+    })
   })
 })

packages/backend/src/apps/postman/actions/send-transactional-email/index.ts

@@ -21,6 +21,8 @@ import { sendBlacklistEmail } from '../../common/send-blacklist-email'
 import { sendInvalidAttachmentsEmail } from '../../common/send-invalid-attachments-email'
 import { throwPostmanStepError } from '../../common/throw-errors'
 
+const BASE_FORM_URL = 'https://form.gov.sg'
+
 const action: IRawAction = {
   name: 'Send email',
   key: 'sendTransactionalEmail',
@@ -165,7 +167,9 @@ const action: IRawAction = {
       executionId: $.execution.id,
     }
     const hasInvalidAttachments = invalidAttachments.length > 0
-    const formAdminLink = `https://form.gov.sg/admin/form/${formId}/results`
+    const formAdminLink = formId
+      ? `${BASE_FORM_URL}/admin/form/${formId}/results`
+      : BASE_FORM_URL
     const invalidAttachmentParams = {
       hasInvalidAttachments,
       submissionId,

packages/backend/src/apps/postman/common/parameters-helper.ts

@@ -14,14 +14,15 @@ export async function getDefaultReplyTo(flowId: string): Promise<string> {
   return flow.user.email
 }
 
-async function getFormId(executionId: string) {
+export async function getFormId(executionId: string): Promise<string | null> {
   const formData = await ExecutionStep.query()
     .where('execution_id', executionId)
     .where('app_key', 'formsg')
     .first()
     .select('data_out')
 
-  return String(formData?.dataOut?.formId)
+  const formId = formData?.dataOut?.formId
+  return formId ? String(formId) : null
 }
 
 export async function filterAttachments(
@@ -39,8 +40,8 @@ export async function filterAttachments(
       // maliciously/ manually injected by another user who does not have access to this attachment
       const obj = await getObjectFromS3Id(attachment, { flowId: $.flow.id }, $)
       const fileName = obj.name
-      const fileType = obj.name.split('.').pop()
-      if (!POSTMAN_ACCEPTED_EXTENSIONS.includes(fileType)) {
+      const fileType = obj.name.split('.').pop()?.toLowerCase()
+      if (!fileType || !POSTMAN_ACCEPTED_EXTENSIONS.includes(fileType)) {
         invalidAttachments.push(fileName)
 
         if (formId === null) {

packages/backend/src/apps/postman/common/send-invalid-attachments-email.ts

@@ -1,6 +1,5 @@
 import { truncateFlowName } from '@/helpers/generate-error-email'
 import { sendEmail } from '@/helpers/send-email'
-import ExecutionStep from '@/models/execution-step'
 
 interface SendInvalidAttachmentsEmailProps {
   flowName: string
@@ -19,16 +18,6 @@ interface CreateMessageProps {
   formAdminLink: string
 }
 
-export async function getFormId(executionId: string) {
-  const formData = await ExecutionStep.query()
-    .where('execution_id', executionId)
-    .where('app_key', 'formsg')
-    .first()
-    .select('data_out')
-
-  return String(formData?.dataOut?.formId)
-}
-
 export function createInvalidAttachmentsMessage(props: CreateMessageProps) {
   const {
     flowName,
@@ -62,7 +51,7 @@ export async function sendInvalidAttachmentsEmail(
 ) {
   const { flowName, userEmail, formAdminLink } = props
   const truncatedFlowName = truncateFlowName(flowName)
-  const bodyContent = await createInvalidAttachmentsMessage({
+  const bodyContent = createInvalidAttachmentsMessage({
     ...props,
     formAdminLink,
   })