chore: add tests, address bugbot comments

kevinkim-ogp · kevinkim-ogp · commit 1d9d5947deff · 2025-06-10T22:21:07.000+08:00
diff --git a/packages/backend/src/apps/toolbox/__tests__/actions/for-each/index.test.ts b/packages/backend/src/apps/toolbox/__tests__/actions/for-each/index.test.ts
@@ -0,0 +1,312 @@
+import { IGlobalVariable } from '@plumber/types'
+
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+
+import {
+  isCheckboxItems,
+  processItems,
+} from '@/apps/toolbox/common/get-for-each-variables'
+import StepError from '@/errors/step'
+
+import action from '../../../actions/for-each'
+import {
+  FOR_EACH_INPUT_SOURCE,
+  FOR_EACH_ITERATION_KEY,
+} from '../../../common/constants'
+
+const mocks = vi.hoisted(() => ({
+  setActionItem: vi.fn(),
+}))
+
+vi.mock('@/apps/toolbox/common/get-for-each-variables', () => ({
+  isCheckboxItems: vi.fn(),
+  processItems: vi.fn(),
+}))
+
+const mockedIsCheckboxItems = vi.mocked(isCheckboxItems)
+const mockedProcessItems = vi.mocked(processItems)
+
+const MOCK_FLOW = [
+  {
+    id: 'trigger',
+    appKey: 'formsg',
+    key: 'newSubmission',
+    position: 1,
+  },
+  {
+    id: 'test-step',
+    appKey: 'tiles',
+    key: 'findMultipleRows',
+    position: 2,
+  },
+  {
+    id: 'for-each',
+    appKey: 'toolbox',
+    key: action.key,
+    position: 3,
+  },
+  {
+    id: 'for-each-action',
+    appKey: 'postman',
+    key: 'sendTransactionalEmail',
+    position: 4,
+  },
+]
+
+describe('For each action', () => {
+  let $: IGlobalVariable
+
+  beforeEach(() => {
+    vi.clearAllMocks()
+
+    $ = {
+      flow: {
+        id: 'fake-pipe',
+        steps: MOCK_FLOW,
+      },
+      step: {
+        id: 'for-each',
+        appKey: 'toolbox',
+        key: action.key,
+        position: 3,
+        parameters: {},
+      },
+      app: {
+        name: 'Toolbox',
+      },
+      setActionItem: mocks.setActionItem,
+    } as unknown as IGlobalVariable
+  })
+
+  afterEach(() => {
+    vi.restoreAllMocks()
+  })
+
+  describe('checkbox input', () => {
+    it('should handle valid checkbox input', async () => {
+      const checkboxItems = ['item1', 'item2', 'item3']
+      $.step.parameters.items = 'item1,item2,item3'
+
+      mockedIsCheckboxItems.mockReturnValue(true)
+
+      const result = await action.run($)
+
+      expect(mocks.setActionItem).toHaveBeenCalledWith({
+        raw: {
+          iterations: 3,
+          items: checkboxItems,
+          inputSource: FOR_EACH_INPUT_SOURCE.CHECKBOX,
+          item: `items.${FOR_EACH_ITERATION_KEY}`,
+        },
+      })
+
+      expect(result).toEqual({
+        nextStep: {
+          command: 'start-for-each',
+          stepId: 'for-each',
+        },
+      })
+    })
+
+    it('should handle checkbox input with whitespace', async () => {
+      const checkboxItems = ['item1', 'item2', 'item3']
+      $.step.parameters.items = '  item1,item2,item3  '
+
+      mockedIsCheckboxItems.mockReturnValue(true)
+
+      await action.run($)
+
+      expect(mocks.setActionItem).toHaveBeenCalledWith({
+        raw: {
+          iterations: 3,
+          items: checkboxItems,
+          inputSource: FOR_EACH_INPUT_SOURCE.CHECKBOX,
+          item: `items.${FOR_EACH_ITERATION_KEY}`,
+        },
+      })
+    })
+
+    it('should handle single checkbox item', async () => {
+      const checkboxItems = ['single-item']
+      $.step.parameters.items = 'single-item'
+
+      mockedIsCheckboxItems.mockReturnValue(true)
+
+      await action.run($)
+
+      expect(mocks.setActionItem).toHaveBeenCalledWith({
+        raw: {
+          iterations: 1,
+          items: checkboxItems,
+          inputSource: FOR_EACH_INPUT_SOURCE.CHECKBOX,
+          item: `items.${FOR_EACH_ITERATION_KEY}`,
+        },
+      })
+    })
+
+    it('should throw error when checkbox items validation fails', async () => {
+      $.step.parameters.items = ''
+
+      await expect(action.run($)).rejects.toThrow(StepError)
+      await expect(action.run($)).rejects.toThrow('Invalid input list')
+    })
+  })
+
+  describe('table input', () => {
+    const validTableData = {
+      rows: [
+        {
+          data: {
+            col1: 'value1',
+            col2: 'value2',
+          },
+          rowId: 'row-1',
+        },
+        {
+          data: {
+            col1: 'value3',
+            col2: 'value4',
+          },
+          rowId: 'row-2',
+        },
+      ],
+      columns: [
+        {
+          id: 'col1',
+          name: 'Column 1',
+          value: 'col1-value',
+        },
+        {
+          id: 'col2',
+          name: 'Column 2',
+          value: 'col2-value',
+        },
+      ],
+    }
+
+    it('should handle valid table input (Tiles)', async () => {
+      $.step.parameters.items = JSON.stringify(validTableData)
+
+      const processedResult = {
+        processedItems: {
+          rows: validTableData.rows,
+          columns: validTableData.columns,
+        },
+        iterations: 2,
+        inputSource: FOR_EACH_INPUT_SOURCE.TILES,
+      }
+
+      mockedProcessItems.mockReturnValue(processedResult)
+
+      const result = await action.run($)
+
+      expect(mockedProcessItems).toHaveBeenCalledWith(validTableData)
+      expect(mocks.setActionItem).toHaveBeenCalledWith({
+        raw: {
+          iterations: 2,
+          items: processedResult.processedItems,
+          inputSource: FOR_EACH_INPUT_SOURCE.TILES,
+        },
+      })
+      expect(result).toEqual({
+        nextStep: {
+          command: 'start-for-each',
+          stepId: 'for-each',
+        },
+      })
+    })
+
+    it('should handle valid table input (M365 Excel)', async () => {
+      const excelData = {
+        ...validTableData,
+        rows: validTableData.rows.map((row) => ({ data: row.data })), // No rowId for Excel
+      }
+
+      $.step.parameters.items = JSON.stringify(excelData)
+
+      const processedResult = {
+        processedItems: {
+          rows: excelData.rows,
+          columns: validTableData.columns,
+        },
+        iterations: 2,
+        inputSource: FOR_EACH_INPUT_SOURCE.M365_EXCEL,
+      }
+
+      mockedProcessItems.mockReturnValue(processedResult)
+
+      await action.run($)
+
+      expect(mockedProcessItems).toHaveBeenCalledWith(excelData)
+      expect(mocks.setActionItem).toHaveBeenCalledWith({
+        raw: {
+          iterations: 2,
+          items: processedResult.processedItems,
+          inputSource: FOR_EACH_INPUT_SOURCE.M365_EXCEL,
+        },
+      })
+    })
+
+    it('should handle table input with empty rows', async () => {
+      const emptyTableData = {
+        rows: [] as any[],
+        columns: validTableData.columns,
+      }
+
+      $.step.parameters.items = JSON.stringify(emptyTableData)
+
+      const processedResult = {
+        processedItems: {
+          rows: [] as any[],
+          columns: validTableData.columns,
+        },
+        iterations: 0,
+        inputSource: null as any,
+      }
+
+      mockedProcessItems.mockReturnValue(processedResult)
+
+      await action.run($)
+
+      expect(mocks.setActionItem).toHaveBeenCalledWith({
+        raw: {
+          iterations: 0,
+          items: processedResult.processedItems,
+          inputSource: null,
+        },
+      })
+    })
+  })
+
+  describe('error handling', () => {
+    it('should throw StepError for empty input', async () => {
+      $.step.parameters.items = ''
+
+      await expect(action.run($)).rejects.toThrow(StepError)
+      await expect(action.run($)).rejects.toThrow('Invalid input list')
+    })
+
+    it('should throw StepError for whitespace-only input', async () => {
+      $.step.parameters.items = '   '
+
+      await expect(action.run($)).rejects.toThrow(StepError)
+      await expect(action.run($)).rejects.toThrow('Invalid input list')
+    })
+
+    it('should throw StepError for invalid JSON', async () => {
+      $.step.parameters.items = '{ invalid json'
+
+      await expect(action.run($)).rejects.toThrow(StepError)
+      await expect(action.run($)).rejects.toThrow('Invalid input list')
+    })
+
+    it('should throw StepError for JSON missing required fields', async () => {
+      $.step.parameters.items = JSON.stringify({
+        invalidField: 'value',
+      })
+
+      await expect(action.run($)).rejects.toThrow(StepError)
+      await expect(action.run($)).rejects.toThrow('Invalid input list')
+    })
+  })
+})
diff --git a/packages/backend/src/helpers/__tests__/compute-for-each-parameters.test.ts b/packages/backend/src/helpers/__tests__/compute-for-each-parameters.test.ts
@@ -1,9 +1,10 @@
-import { IExecutionStep, IJSONObject } from '@plumber/types'
+import { IExecutionStep } from '@plumber/types'
 
 import { randomUUID } from 'crypto'
 import { describe, expect, it } from 'vitest'
 
 import {
+  FOR_EACH_ITERATION_KEY,
   TOOLBOX_ACTIONS,
   TOOLBOX_APP_KEY,
 } from '@/apps/toolbox/common/constants'
@@ -480,13 +481,10 @@ describe('computeForEachParameters', () => {
   })
 
   it('should handle missing forEachContext gracefully', () => {
-    const data: IJSONObject = {
-      stringProp: 'test string',
-    }
-    const keyPath = 'stringProp'
+    const keyPath = `items.columns.${FOR_EACH_ITERATION_KEY}.value`
     const executionStep = mockExecutionStepsCheckbox[2]
     const result = computeForEachParameters({
-      data,
+      data: executionStep.dataOut,
       keyPath,
       executionSteps: mockExecutionStepsCheckbox,
       executionStep,
@@ -496,4 +494,23 @@ describe('computeForEachParameters', () => {
 
     expect(result).toBe(keyPath) // returns keyPath when context is missing
   })
+
+  it('should handle undefined metadata.iteration gracefully', () => {
+    const keyPath = `items.columns.${FOR_EACH_ITERATION_KEY}.value`
+    const executionStep = mockExecutionStepsTable[1] // for-each step
+
+    const result = computeForEachParameters({
+      data: executionStep.dataOut,
+      keyPath,
+      executionSteps: mockExecutionStepsTable,
+      executionStep,
+      stepId: randomForEachStepId,
+      forEachContext: {
+        ...baseForEachContext,
+        executionStepMetadata: {},
+      },
+    })
+
+    expect(result).toBe('')
+  })
 })
diff --git a/packages/backend/src/helpers/compute-for-each-parameters.ts b/packages/backend/src/helpers/compute-for-each-parameters.ts
@@ -93,11 +93,21 @@ export function computeForEachParameters({
   let forEachKeyPath = get(data, keyPath)
   const currentStepIndex = stepPositions?.[executionStep?.stepId] || -1
 
-  // handling of checkboxes which is just a flat array
-  forEachKeyPath = String(forEachKeyPath).replace(
-    FOR_EACH_ITERATION_KEY,
-    `${testRun ? 0 : metadata?.iteration - 1}`,
-  )
+  if (testRun) {
+    forEachKeyPath = String(forEachKeyPath).replace(FOR_EACH_ITERATION_KEY, '0')
+  } else if (metadata?.iteration) {
+    forEachKeyPath = String(forEachKeyPath).replace(
+      FOR_EACH_ITERATION_KEY,
+      `${metadata.iteration - 1}`,
+    )
+  }
+
+  if (testRun || metadata?.iteration) {
+    forEachKeyPath = String(forEachKeyPath).replace(
+      FOR_EACH_ITERATION_KEY,
+      `${testRun ? 0 : metadata.iteration - 1}`,
+    )
+  }
 
   if (
     executionStep.appKey === TOOLBOX_APP_KEY &&
diff --git a/packages/backend/src/services/action.ts b/packages/backend/src/services/action.ts
