Merge pull request #12906 from Budibase/fix-get-loop-iterations

Typing improvements around automation loop tests.

packages/server/scripts/test.sh

@@ -5,10 +5,10 @@ if [[ -n $CI ]]
 then
   # Running in ci, where resources are limited
   export NODE_OPTIONS="--max-old-space-size=4096"
-  echo "jest --coverage --maxWorkers=2 --forceExit --workerIdleMemoryLimit=2000MB --bail"
-  jest --coverage --maxWorkers=2 --forceExit --workerIdleMemoryLimit=2000MB --bail
+  echo "jest --coverage --maxWorkers=2 --forceExit --workerIdleMemoryLimit=2000MB --bail $@"
+  jest --coverage --maxWorkers=2 --forceExit --workerIdleMemoryLimit=2000MB --bail $@
 else
   # --maxWorkers performs better in development
-  echo "jest --coverage --maxWorkers=2 --forceExit"
-  jest --coverage --maxWorkers=2 --forceExit
+  echo "jest --coverage --maxWorkers=2 --forceExit $@"
+  jest --coverage --maxWorkers=2 --forceExit $@
 fi

packages/server/src/automations/automationUtils.ts

@@ -5,7 +5,7 @@ import {
 } from "@budibase/string-templates"
 import sdk from "../sdk"
 import { Row } from "@budibase/types"
-import { LoopStep, LoopStepType, LoopInput } from "../definitions/automations"
+import { LoopInput, LoopStep, LoopStepType } from "../definitions/automations"
 
 /**
  * When values are input to the system generally they will be of type string as this is required for template strings.
@@ -139,12 +139,12 @@ export function stringSplit(value: string | string[]) {
   return value
 }
 
-export function typecastForLooping(loopStep: LoopStep, input: LoopInput) {
+export function typecastForLooping(input: LoopInput) {
   if (!input || !input.binding) {
     return null
   }
   try {
-    switch (loopStep.inputs.option) {
+    switch (input.option) {
       case LoopStepType.ARRAY:
         if (typeof input.binding === "string") {
           return JSON.parse(input.binding)

packages/server/src/automations/tests/loop.spec.ts

@@ -3,11 +3,13 @@ import * as triggers from "../triggers"
 import { loopAutomation } from "../../tests/utilities/structures"
 import { context } from "@budibase/backend-core"
 import * as setup from "./utilities"
+import { Row, Table } from "@budibase/types"
+import { LoopInput, LoopStepType } from "../../definitions/automations"
 
 describe("Attempt to run a basic loop automation", () => {
   let config = setup.getConfig(),
-    table: any,
-    row: any
+    table: Table,
+    row: Row
 
   beforeEach(async () => {
     await automation.init()
@@ -18,12 +20,12 @@ describe("Attempt to run a basic loop automation", () => {
 
   afterAll(setup.afterAll)
 
-  async function runLoop(loopOpts?: any) {
+  async function runLoop(loopOpts?: LoopInput) {
     const appId = config.getAppId()
     return await context.doInAppContext(appId, async () => {
       const params = { fields: { appId } }
       return await triggers.externalTrigger(
-        loopAutomation(table._id, loopOpts),
+        loopAutomation(table._id!, loopOpts),
         params,
         { getResponses: true }
       )
@@ -37,7 +39,7 @@ describe("Attempt to run a basic loop automation", () => {
 
   it("test a loop with a string", async () => {
     const resp = await runLoop({
-      type: "String",
+      option: LoopStepType.STRING,
       binding: "a,b,c",
     })
     expect(resp.steps[2].outputs.iterations).toBe(3)

packages/server/src/automations/unitTests/automationUtils.spec.ts

@@ -1,10 +1,15 @@
-const automationUtils = require("../automationUtils")
+import { LoopStep, LoopStepType } from "../../definitions/automations"
+import {
+  typecastForLooping,
+  cleanInputValues,
+  substituteLoopStep,
+} from "../automationUtils"
 
 describe("automationUtils", () => {
   describe("substituteLoopStep", () => {
     it("should allow multiple loop binding substitutes", () => {
       expect(
-        automationUtils.substituteLoopStep(
+        substituteLoopStep(
           `{{ loop.currentItem._id }} {{ loop.currentItem._id }} {{ loop.currentItem._id }}`,
           "step.2"
         )
@@ -15,7 +20,7 @@ describe("automationUtils", () => {
 
     it("should handle not subsituting outside of curly braces", () => {
       expect(
-        automationUtils.substituteLoopStep(
+        substituteLoopStep(
           `loop {{ loop.currentItem._id }}loop loop{{ loop.currentItem._id }}loop`,
           "step.2"
         )
@@ -28,37 +33,20 @@ describe("automationUtils", () => {
   describe("typeCastForLooping", () => {
     it("should parse to correct type", () => {
       expect(
-        automationUtils.typecastForLooping(
-          { inputs: { option: "Array" } },
-          { binding: [1, 2, 3] }
-        )
+        typecastForLooping({ option: LoopStepType.ARRAY, binding: [1, 2, 3] })
       ).toEqual([1, 2, 3])
       expect(
-        automationUtils.typecastForLooping(
-          { inputs: { option: "Array" } },
-          { binding: "[1, 2, 3]" }
-        )
+        typecastForLooping({ option: LoopStepType.ARRAY, binding: "[1,2,3]" })
       ).toEqual([1, 2, 3])
       expect(
-        automationUtils.typecastForLooping(
-          { inputs: { option: "String" } },
-          { binding: [1, 2, 3] }
-        )
+        typecastForLooping({ option: LoopStepType.STRING, binding: [1, 2, 3] })
       ).toEqual("1,2,3")
     })
     it("should handle null values", () => {
       // expect it to handle where the binding is null
-      expect(
-        automationUtils.typecastForLooping(
-          { inputs: { option: "Array" } },
-          { binding: null }
-        )
-      ).toEqual(null)
+      expect(typecastForLooping({ option: LoopStepType.ARRAY })).toEqual(null)
       expect(() =>
-        automationUtils.typecastForLooping(
-          { inputs: { option: "Array" } },
-          { binding: "test" }
-        )
+        typecastForLooping({ option: LoopStepType.ARRAY, binding: "test" })
       ).toThrow()
     })
   })
@@ -80,7 +68,7 @@ describe("automationUtils", () => {
         },
       }
       expect(
-        automationUtils.cleanInputValues(
+        cleanInputValues(
           {
             row: {
               relationship: `[{"_id": "ro_ta_users_us_3"}]`,
@@ -113,7 +101,7 @@ describe("automationUtils", () => {
         },
       }
       expect(
-        automationUtils.cleanInputValues(
+        cleanInputValues(
           {
             row: {
               relationship: `ro_ta_users_us_3`,

packages/server/src/definitions/automations.ts

@@ -6,14 +6,14 @@ export enum LoopStepType {
 }
 
 export interface LoopStep extends AutomationStep {
-  inputs: {
-    option: LoopStepType
-    [key: string]: any
-  }
+  inputs: LoopInput
 }
 
 export interface LoopInput {
-  binding: string[] | string
+  option: LoopStepType
+  binding?: string[] | string | number[]
+  iterations?: string
+  failure?: any
 }
 
 export interface TriggerOutput {

packages/server/src/tests/utilities/structures.ts

@@ -23,6 +23,7 @@ import {
   TableSourceType,
   Query,
 } from "@budibase/types"
+import { LoopInput, LoopStepType } from "../../definitions/automations"
 
 const { BUILTIN_ROLE_IDS } = roles
 
@@ -204,10 +205,13 @@ export function serverLogAutomation(appId?: string): Automation {
   }
 }
 
-export function loopAutomation(tableId: string, loopOpts?: any): Automation {
+export function loopAutomation(
+  tableId: string,
+  loopOpts?: LoopInput
+): Automation {
   if (!loopOpts) {
     loopOpts = {
-      option: "Array",
+      option: LoopStepType.ARRAY,
       binding: "{{ steps.1.rows }}",
     }
   }

packages/server/src/threads/automation.ts

@@ -47,9 +47,8 @@ function getLoopIterations(loopStep: LoopStep) {
   if (!binding) {
     return 0
   }
-  const isString = typeof binding === "string"
   try {
-    if (isString) {
+    if (typeof binding === "string") {
       binding = JSON.parse(binding)
     }
   } catch (err) {
@@ -58,7 +57,7 @@ function getLoopIterations(loopStep: LoopStep) {
   if (Array.isArray(binding)) {
     return binding.length
   }
-  if (isString) {
+  if (typeof binding === "string") {
     return automationUtils.stringSplit(binding).length
   }
   return 0
@@ -256,7 +255,7 @@ class Orchestrator {
         this._context.env = await sdkUtils.getEnvironmentVariables()
         let automation = this._automation
         let stopped = false
-        let loopStep: AutomationStep | undefined = undefined
+        let loopStep: LoopStep | undefined = undefined
 
         let stepCount = 0
         let loopStepNumber: any = undefined
@@ -311,7 +310,7 @@ class Orchestrator {
 
             stepCount++
             if (step.stepId === LOOP_STEP_ID) {
-              loopStep = step
+              loopStep = step as LoopStep
               loopStepNumber = stepCount
               continue
             }
@@ -331,7 +330,6 @@ class Orchestrator {
                 }
                 try {
                   loopStep.inputs.binding = automationUtils.typecastForLooping(
-                    loopStep as LoopStep,
                     loopStep.inputs as LoopInput
                   )
                 } catch (err) {
@@ -348,7 +346,7 @@ class Orchestrator {
                   loopStep = undefined
                   break
                 }
-                let item = []
+                let item: any[] = []
                 if (
                   typeof loopStep.inputs.binding === "string" &&
                   loopStep.inputs.option === "String"
@@ -399,7 +397,8 @@ class Orchestrator {
 
                 if (
                   index === env.AUTOMATION_MAX_ITERATIONS ||
-                  index === parseInt(loopStep.inputs.iterations)
+                  (loopStep.inputs.iterations &&
+                    index === parseInt(loopStep.inputs.iterations))
                 ) {
                   this.updateContextAndOutput(
                     loopStepNumber,