fix issues and add tests for delete from s3

Author: kevinkim-ogp

packages/backend/src/graphql/__tests__/mutations/delete-from-s3.itest.ts

@@ -6,6 +6,7 @@ import Flow from '@/models/flow'
 import Context from '@/types/express/context'
 
 import { generateMockContext } from './tiles/table.mock'
+import { generateMockFlow, generateMockStep } from './flow.mock'
 
 const mockFlowId = '8c2a70d1-e78b-431e-9069-a4d8f97883f6'
 const mockBucket = 'test-bucket'
@@ -38,6 +39,29 @@ vi.mock('@aws-sdk/client-s3', () => ({
   DeleteObjectsCommand: vi.fn(),
 }))
 
+function createMockS3Id(mockFileName: string) {
+  return `s3:${mockBucket}:${mockFlowId}/${mockFileName}.txt`
+}
+
+async function createMockStep(
+  context: Context,
+  flowId: string,
+  position: number,
+  params: Record<string, any>,
+  config: Record<string, any> = {},
+) {
+  return generateMockStep(
+    context,
+    'sendTransactionalEmail',
+    'postman',
+    'action',
+    flowId,
+    position,
+    params,
+    config,
+  )
+}
+
 describe('deleteFromS3', () => {
   let context: Context
   beforeEach(async () => {
@@ -46,17 +70,35 @@ describe('deleteFromS3', () => {
   })
 
   it('should successfully delete an object when user owns the flow', async () => {
-    await Flow.query().insert({
-      id: mockFlowId,
-      name: 'Test Flow',
-      userId: context.currentUser.id,
-    })
+    await generateMockFlow(context, mockFlowId)
 
     await expect(deleteFromS3(null, { id: mockS3Id }, context)).resolves.toBe(
       true,
     )
   })
 
+  it('should delete object from all other steps within a flow', async () => {
+    const fileToDelete = createMockS3Id('test_1')
+    await generateMockFlow(context, mockFlowId)
+    await createMockStep(context, mockFlowId, 2, {
+      body: 'Test body',
+      attachments: [fileToDelete],
+    })
+    await createMockStep(context, mockFlowId, 2, {
+      body: 'Test body',
+      attachments: [fileToDelete, createMockS3Id('test_2')],
+    })
+
+    await deleteFromS3(null, { id: fileToDelete }, context)
+    const postDeleteSteps = await context.currentUser
+      .$relatedQuery('steps')
+      .where({ flow_id: mockFlowId })
+
+    postDeleteSteps.forEach((step) => {
+      expect(step.parameters.attachments).not.toContain(fileToDelete)
+    })
+  })
+
   it('should throw an error if user does not have access to the flow', async () => {
     vi.fn()
       .mockRejectedValue(Flow.hasAccess)

packages/backend/src/graphql/__tests__/mutations/flow.mock.ts

@@ -0,0 +1,72 @@
+import Flow from '@/models/flow'
+import Step from '@/models/step'
+import Context from '@/types/express/context'
+
+export async function generateMockFlow(
+  context: Context,
+  id: string,
+  config?: Record<string, any>,
+) {
+  await Flow.query().insert({
+    id,
+    name: 'Test Flow',
+    userId: context.currentUser.id,
+    config: config ?? {},
+  })
+}
+
+export async function generateMockStep(
+  context: Context,
+  key:
+    | 'delayFor'
+    | 'everyHour'
+    | 'getCellValues'
+    | 'NULL'
+    | 'findMessage'
+    | 'ifThen'
+    | 'everyDay'
+    | 'getTableRow'
+    | 'createTableRow'
+    | 'sendTransactionalEmail'
+    | 'onlyContinueIf'
+    | 'createTileRow'
+    | 'dateTime'
+    | 'httpRequest'
+    | 'everyMonth'
+    | 'createLetter'
+    | 'updateSingleRow'
+    | 'catchRawWebhook'
+    | 'newSubmission'
+    | 'findSingleRow'
+    | 'performCalculation',
+  appKey:
+    | null
+    | 'slack'
+    | 'm365-excel'
+    | 'custom-api'
+    | 'webhook'
+    | 'delay'
+    | 'lettersg'
+    | 'formatter'
+    | 'tiles'
+    | 'scheduler'
+    | 'postman'
+    | 'formsg'
+    | 'calculator'
+    | 'toolbox',
+  type: 'action' | 'trigger',
+  flowId: string,
+  position: number,
+  parameters?: Record<string, any>,
+  config?: Record<string, any>,
+) {
+  await Step.query().insert({
+    key,
+    appKey,
+    type,
+    flowId,
+    position,
+    parameters: parameters ?? {},
+    config: config ?? {},
+  })
+}

packages/backend/src/graphql/mutations/delete-from-s3.ts

@@ -25,7 +25,7 @@ const deleteFromS3: MutationResolvers['deleteFromS3'] = async (
     .orderBy('steps.position', 'asc')
 
   // remove attachment from all steps to prevent execution failure
-  steps.map(
+  const deletePromises = steps.map(
     async (step: { id: string; parameters: { attachments?: string[] } }) => {
       const { id: stepId, parameters } = step
       const { attachments = [] } = parameters
@@ -43,6 +43,8 @@ const deleteFromS3: MutationResolvers['deleteFromS3'] = async (
       }
     },
   )
+  await Promise.allSettled(deletePromises)
+
   return await deleteObjects(COMMON_S3_BUCKET, [{ Key: objectKey }])
 }
 

packages/backend/src/helpers/csp.ts

@@ -18,7 +18,6 @@ const helmetOptions: HelmetOptions = {
         // For proxying datadog rum
         'https://rum-proxy.plumber.gov.sg',
         // For S3 bucket
-        `https://upload-${appConfig.appEnv}.plumber.gov.sg`,
         'https://plumber-uat-attachment-bucket-private-0d9400e.s3.ap-southeast-1.amazonaws.com',
         'https://plumber-staging-attachment-bucket-private-ab28487.s3.ap-southeast-1.amazonaws.com',
         'https://plumber-prod-attachment-bucket-private-beb3aa3.s3.ap-southeast-1.amazonaws.com',

packages/frontend/src/hooks/useS3Operations.ts

@@ -56,7 +56,7 @@ export const useS3Operations = (
 
   const deleteFromS3 = async (file: any) => {
     try {
-      const { name: filename, value } = file
+      const { name: filename, value, displayedValue } = file
       const flowId = getValues('flowId')
       setIsDeleting(true)
       await deleteFile({ variables: { id: value } })
@@ -71,7 +71,7 @@ export const useS3Operations = (
 
       await refetchFlow()
 
-      triggerToast(`${filename} deleted successfully`, 'success')
+      triggerToast(`${displayedValue} deleted successfully`, 'success')
       setIsDeleting(false)
       options.onSuccess?.(filename)
       return true