Merge branch 'master' into new-data-ui

packages/server/src/api/routes/tests/automation.spec.ts

@@ -15,6 +15,7 @@ import { Automation, FieldType, Table } from "@budibase/types"
 import { mocks } from "@budibase/backend-core/tests"
 import { FilterConditions } from "../../../automations/steps/filter"
 import { removeDeprecated } from "../../../automations/utils"
+import { createAutomationBuilder } from "../../../automations/tests/utilities/AutomationTestBuilder"
 
 const MAX_RETRIES = 4
 let {
@@ -121,6 +122,104 @@ describe("/automations", () => {
       expect(events.automation.stepCreated).toHaveBeenCalledTimes(2)
     })
 
+    it("Should ensure you can't have a branch as not a last step", async () => {
+      const automation = createAutomationBuilder({
+        name: "String Equality Branching",
+        appId: config.getAppId(),
+      })
+        .appAction({ fields: { status: "active" } })
+        .branch({
+          activeBranch: {
+            steps: stepBuilder =>
+              stepBuilder.serverLog({ text: "Active user" }),
+            condition: {
+              equal: { "trigger.fields.status": "active" },
+            },
+          },
+        })
+        .serverLog({ text: "Inactive user" })
+        .build()
+
+      await config.api.automation.post(automation, {
+        status: 400,
+        body: {
+          message:
+            "Invalid body - Branch steps are only allowed as the last step",
+        },
+      })
+    })
+
+    it("Should check validation on an automation that has a branch step with no children", async () => {
+      const automation = createAutomationBuilder({
+        name: "String Equality Branching",
+        appId: config.getAppId(),
+      })
+        .appAction({ fields: { status: "active" } })
+        .branch({})
+        .serverLog({ text: "Inactive user" })
+        .build()
+
+      await config.api.automation.post(automation, {
+        status: 400,
+        body: {
+          message:
+            'Invalid body - "definition.steps[0].inputs.branches" must contain at least 1 items',
+        },
+      })
+    })
+
+    it("Should check validation on a branch step with empty conditions", async () => {
+      const automation = createAutomationBuilder({
+        name: "String Equality Branching",
+        appId: config.getAppId(),
+      })
+        .appAction({ fields: { status: "active" } })
+        .branch({
+          activeBranch: {
+            steps: stepBuilder =>
+              stepBuilder.serverLog({ text: "Active user" }),
+            condition: {},
+          },
+        })
+        .build()
+
+      await config.api.automation.post(automation, {
+        status: 400,
+        body: {
+          message:
+            'Invalid body - "definition.steps[0].inputs.branches[0].condition" must have at least 1 key',
+        },
+      })
+    })
+
+    it("Should check validation on an branch that has a condition that is not valid", async () => {
+      const automation = createAutomationBuilder({
+        name: "String Equality Branching",
+        appId: config.getAppId(),
+      })
+        .appAction({ fields: { status: "active" } })
+        .branch({
+          activeBranch: {
+            steps: stepBuilder =>
+              stepBuilder.serverLog({ text: "Active user" }),
+            condition: {
+              //@ts-ignore
+              INCORRECT: { "trigger.fields.status": "active" },
+            },
+          },
+        })
+        .serverLog({ text: "Inactive user" })
+        .build()
+
+      await config.api.automation.post(automation, {
+        status: 400,
+        body: {
+          message:
+            'Invalid body - "definition.steps[0].inputs.branches[0].condition.INCORRECT" is not allowed',
+        },
+      })
+    })
+
     it("should apply authorization to endpoint", async () => {
       const automation = newAutomation()
       await checkBuilderEndpoint({

packages/server/src/api/routes/utils/validators.ts

@@ -1,6 +1,9 @@
 import { auth, permissions } from "@budibase/backend-core"
 import { DataSourceOperation } from "../../../constants"
 import {
+  AutomationActionStepId,
+  AutomationStep,
+  AutomationStepType,
   EmptyFilterOption,
   SearchFilters,
   Table,
@@ -88,7 +91,8 @@ export function datasourceValidator() {
   )
 }
 
-function filterObject() {
+function filterObject(opts?: { unknown: boolean }) {
+  const { unknown = true } = opts || {}
   const conditionalFilteringObject = () =>
     Joi.object({
       conditions: Joi.array().items(Joi.link("#schema")).required(),
@@ -115,7 +119,7 @@ function filterObject() {
     fuzzyOr: Joi.forbidden(),
     documentType: Joi.forbidden(),
   }
-  return Joi.object(filtersValidators).unknown(true).id("schema")
+  return Joi.object(filtersValidators).unknown(unknown).id("schema")
 }
 
 export function internalSearchValidator() {
@@ -259,6 +263,11 @@ export function screenValidator() {
 }
 
 function generateStepSchema(allowStepTypes: string[]) {
+  const branchSchema = Joi.object({
+    name: Joi.string().required(),
+    condition: filterObject({ unknown: false }).required().min(1),
+  })
+
   return Joi.object({
     stepId: Joi.string().required(),
     id: Joi.string().required(),
@@ -267,11 +276,35 @@ function generateStepSchema(allowStepTypes: string[]) {
     tagline: Joi.string().required(),
     icon: Joi.string().required(),
     params: Joi.object(),
+    inputs: Joi.when("stepId", {
+      is: AutomationActionStepId.BRANCH,
+      then: Joi.object({
+        branches: Joi.array().items(branchSchema).min(1).required(),
+        children: Joi.object()
+          .pattern(Joi.string(), Joi.array().items(Joi.link("#step")))
+          .required(),
+      }).required(),
+      otherwise: Joi.object(),
+    }),
+
     args: Joi.object(),
     type: Joi.string()
       .required()
       .valid(...allowStepTypes),
-  }).unknown(true)
+  })
+    .unknown(true)
+    .id("step")
+}
+
+const validateStepsArray = (
+  steps: AutomationStep[],
+  helpers: Joi.CustomHelpers
+) => {
+  for (const step of steps.slice(0, -1)) {
+    if (step.stepId === AutomationActionStepId.BRANCH) {
+      return helpers.error("branchStepPosition")
+    }
+  }
 }
 
 export function automationValidator(existing = false) {
@@ -284,9 +317,20 @@ export function automationValidator(existing = false) {
       definition: Joi.object({
         steps: Joi.array()
           .required()
-          .items(generateStepSchema(["ACTION", "LOGIC"])),
+          .items(
-        trigger: generateStepSchema(["TRIGGER"]).allow(null),
+            generateStepSchema([
+              AutomationStepType.ACTION,
+              AutomationStepType.LOGIC,
+            ])
+          )
+          .custom(validateStepsArray)
+          .messages({
+            branchStepPosition:
+              "Branch steps are only allowed as the last step",
+          }),
+        trigger: generateStepSchema([AutomationStepType.TRIGGER]).allow(null),
       })
+
         .required()
         .unknown(true),
     }).unknown(true)

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

@@ -63,8 +63,8 @@ describe("Automation Scenarios", () => {
           },
         })
         .run()
-
+      expect(results.steps[3].outputs.status).toContain("branch1 branch taken")
-      expect(results.steps[2].outputs.message).toContain("Branch 1.1")
+      expect(results.steps[4].outputs.message).toContain("Branch 1.1")
     })
 
     it("should execute correct branch based on string equality", async () => {
@@ -91,8 +91,10 @@ describe("Automation Scenarios", () => {
           },
         })
         .run()
-
+      expect(results.steps[0].outputs.status).toContain(
-      expect(results.steps[0].outputs.message).toContain("Active user")
+        "activeBranch branch taken"
+      )
+      expect(results.steps[1].outputs.message).toContain("Active user")
     })
 
     it("should handle multiple conditions with AND operator", async () => {
@@ -124,7 +126,7 @@ describe("Automation Scenarios", () => {
         })
         .run()
 
-      expect(results.steps[0].outputs.message).toContain("Active admin user")
+      expect(results.steps[1].outputs.message).toContain("Active admin user")
     })
 
     it("should handle multiple conditions with OR operator", async () => {
@@ -162,7 +164,7 @@ describe("Automation Scenarios", () => {
         })
         .run()
 
-      expect(results.steps[0].outputs.message).toContain("Special user")
+      expect(results.steps[1].outputs.message).toContain("Special user")
     })
   })
 
@@ -362,6 +364,32 @@ describe("Automation Scenarios", () => {
         }
       )
     })
+
+    it("should run an automation where a loop is used twice to ensure context correctness further down the tree", async () => {
+      const builder = createAutomationBuilder({
+        name: "Test Trigger with Loop and Create Row",
+      })
+
+      const results = await builder
+        .appAction({ fields: {} })
+        .loop({
+          option: LoopStepType.ARRAY,
+          binding: [1, 2, 3],
+        })
+        .serverLog({ text: "Message {{loop.currentItem}}" })
+        .serverLog({ text: "{{steps.1.iterations}}" })
+        .loop({
+          option: LoopStepType.ARRAY,
+          binding: [1, 2, 3],
+        })
+        .serverLog({ text: "{{loop.currentItem}}" })
+        .serverLog({ text: "{{steps.3.iterations}}" })
+        .run()
+
+      // We want to ensure that bindings are corr
+      expect(results.steps[1].outputs.message).toContain("- 3")
+      expect(results.steps[3].outputs.message).toContain("- 3")
+    })
   })
 
   describe("Row Automations", () => {

packages/server/src/automations/tests/utilities/AutomationTestBuilder.ts

@@ -179,7 +179,7 @@ class AutomationBuilder extends BaseStepBuilder {
   private triggerOutputs: any
   private triggerSet: boolean = false
 
-  constructor(options: { name?: string } = {}) {
+  constructor(options: { name?: string; appId?: string } = {}) {
     super()
     this.automationConfig = {
       name: options.name || `Test Automation ${uuidv4()}`,
@@ -188,7 +188,7 @@ class AutomationBuilder extends BaseStepBuilder {
         trigger: {} as AutomationTrigger,
       },
       type: "automation",
-      appId: setup.getConfig().getAppId(),
+      appId: options.appId ?? setup.getConfig().getAppId(),
     }
     this.config = setup.getConfig()
   }
@@ -261,13 +261,14 @@ class AutomationBuilder extends BaseStepBuilder {
     return this
   }
 
-  branch(branchConfig: BranchConfig): {
+  branch(branchConfig: BranchConfig): this {
-    run: () => Promise<AutomationResults>
-  } {
     this.addBranchStep(branchConfig)
-    return {
+    return this
-      run: () => this.run(),
+  }
-    }
+
+  build(): Automation {
+    this.automationConfig.definition.steps = this.steps
+    return this.automationConfig
   }
 
   async run() {
@@ -275,7 +276,7 @@ class AutomationBuilder extends BaseStepBuilder {
       throw new Error("Please add a trigger to this automation test")
     }
     this.automationConfig.definition.steps = this.steps
-    const automation = await this.config.createAutomation(this.automationConfig)
+    const automation = await this.config.createAutomation(this.build())
     const results = await testAutomation(
       this.config,
       automation,
@@ -295,6 +296,9 @@ class AutomationBuilder extends BaseStepBuilder {
   }
 }
 
-export function createAutomationBuilder(options?: { name?: string }) {
+export function createAutomationBuilder(options?: {
+  name?: string
+  appId?: string
+}) {
   return new AutomationBuilder(options)
 }

packages/server/src/tests/utilities/api/automation.ts

@@ -14,4 +14,14 @@ export class AutomationAPI extends TestAPI {
     )
     return result
   }
+  post = async (
+    body: Automation,
+    expectations?: Expectations
+  ): Promise<Automation> => {
+    const result = await this._post<Automation>(`/api/automations`, {
+      body,
+      expectations,
+    })
+    return result
+  }
 }

packages/server/src/threads/automation.ts

@@ -449,7 +449,11 @@ class Orchestrator {
         outputs: tempOutput,
         inputs: steps[stepToLoopIndex].inputs,
       })
-      this.context.steps[currentIndex + 1] = tempOutput
+      this.context.steps[this.context.steps.length] = tempOutput
+      this.context.steps = this.context.steps.filter(
+        item => !item.hasOwnProperty.call(item, "currentItem")
+      )
+
       this.loopStepOutputs = []
     }
 
@@ -461,6 +465,19 @@ class Orchestrator {
     for (const branch of branches) {
       const condition = await this.evaluateBranchCondition(branch.condition)
       if (condition) {
+        let branchStatus = {
+          status: `${branch.name} branch taken`,
+          success: true,
+        }
+
+        this.updateExecutionOutput(
+          branchStep.id,
+          branchStep.stepId,
+          branchStep.inputs,
+          branchStatus
+        )
+        this.context.steps[this.context.steps.length] = branchStatus
+
         const branchSteps = children?.[branch.name] || []
         await this.executeSteps(branchSteps)
         break
@@ -569,8 +586,8 @@ class Orchestrator {
       this.loopStepOutputs.push(outputs)
     } else {
       this.updateExecutionOutput(step.id, step.stepId, step.inputs, outputs)
+      this.context.steps[this.context.steps.length] = outputs
     }
-    this.context.steps[this.context.steps.length] = outputs
   }
 }