fix failure condition

packages/builder/src/components/automation/AutomationBuilder/FlowChart/FlowItem.svelte

@@ -10,7 +10,7 @@
     Button,
     StatusLight,
     Select,
-    Label,
+    ActionButton,
     notifications,
   } from "@budibase/bbui"
   import AutomationBlockSetup from "../../SetupPanel/AutomationBlockSetup.svelte"
@@ -58,7 +58,15 @@
   $: showLooping = false
 
   async function deleteStep() {
+    let loopBlock =
+      $automationStore.selectedAutomation?.automation.definition.steps.find(
+        x => x.blockToLoop === block.id
+      )
+
     try {
+      if (loopBlock) {
+        automationStore.actions.deleteAutomationBlock(loopBlock)
+      }
       automationStore.actions.deleteAutomationBlock(block)
       await automationStore.actions.save(
         $automationStore.selectedAutomation?.automation
@@ -233,29 +241,24 @@
           <div>
             <div class="block-options">
               {#if !loopingSelected}
-                <div style="display: flex;" on:click={() => addLooping()}>
-                  <Icon name="Reuse" />
-                  <div style="margin-left:10px">
-                    <Label>Add looping</Label>
-                  </div>
-                </div>
+                <ActionButton on:click={() => addLooping()} icon="Reuse"
+                  >Add Looping</ActionButton
+                >
               {/if}
               {#if showBindingPicker}
-                <div>
-                  <Select
-                    on:change={toggleFieldControl}
-                    quiet
-                    defaultValue="Use values"
-                    autoWidth
-                    value={rowControl ? "Use bindings" : "Use values"}
-                    options={["Use values", "Use bindings"]}
-                    placeholder={null}
-                  />
-                </div>
+                <Select
+                  on:change={toggleFieldControl}
+                  defaultValue="Use values"
+                  autoWidth
+                  value={rowControl ? "Use bindings" : "Use values"}
+                  options={["Use values", "Use bindings"]}
+                  placeholder={null}
+                />
               {/if}
-              <div class="delete-padding" on:click={() => deleteStep()}>
-                <Icon name="DeleteOutline" />
-              </div>
+              <ActionButton
+                on:click={() => deleteStep()}
+                icon="DeleteOutline"
+              />
             </div>
           </div>
         {/if}
@@ -306,6 +309,7 @@
     justify-content: flex-end;
     align-items: center;
     display: flex;
+    gap: var(--spacing-m);
   }
   .center-items {
     display: flex;

packages/builder/src/components/automation/SetupPanel/AutomationBlockSetup.svelte

@@ -30,7 +30,6 @@
   import FilterDrawer from "components/design/PropertiesPanel/PropertyControls/FilterEditor/FilterDrawer.svelte"
   import { LuceneUtils } from "@budibase/frontend-core"
   import { getSchemaForTable } from "builderStore/dataBinding"
-  import { cloneDeep } from "lodash/fp"
 
   export let block
   export let testData
@@ -97,19 +96,23 @@
     }
     let blockIdx = allSteps.findIndex(step => step.id === block.id)
 
-    let loopBlockIdx = cloneDeep(allSteps)
-      .splice(0, blockIdx)
-      .findIndex(x => x.stepId === "LOOP")
-    // if a loop stepId exists in previous steps, we need to decerement the blockIdx
-    if (loopBlockIdx > -1 && blockIdx > loopBlockIdx) {
-      blockIdx--
-    }
     // Extract all outputs from all previous steps as available bindins
     let bindings = []
     for (let idx = 0; idx < blockIdx; idx++) {
-      let isLoopBlock = allSteps[idx + 1]?.blockToLoop === block.id
+      let wasLoopBlock = allSteps[idx]?.stepId === "LOOP"
+      let isLoopBlock =
+        allSteps[idx]?.stepId === "LOOP" &&
+        allSteps.find(x => x.blockToLoop === block.id)
+
+      // If the previous block was a loop block, decerement the index so the following
+      // steps are in the correct order
+      if (wasLoopBlock) {
+        blockIdx--
+      }
 
       let schema = allSteps[idx]?.schema?.outputs?.properties ?? {}
+
+      // If its a Loop Block, we need to add this custom schema
       if (isLoopBlock) {
         schema = {
           currentItem: {
@@ -118,14 +121,6 @@
           },
         }
       }
-
-      if (loopBlockIdx && allSteps[blockIdx - 1]?.stepId === "LOOP") {
-        schema = {
-          ...schema,
-          ...$automationStore.blockDefinitions.ACTION.LOOP.schema.outputs
-            .properties.properties,
-        }
-      }
       const outputs = Object.entries(schema)
 
       bindings = bindings.concat(

packages/server/src/threads/automation.js

@@ -77,6 +77,24 @@ class Orchestrator {
     this.executionOutput.steps.push(stepObj)
   }
 
+  updateContextAndOutput(loopStepNumber, step, output, result) {
+    this.executionOutput.steps.splice(loopStepNumber, 0, {
+      id: step.id,
+      stepId: step.stepId,
+      outputs: {
+        ...output,
+        success: result.success,
+        status: result.status,
+      },
+      inputs: step.inputs,
+    })
+    this._context.steps.splice(loopStepNumber, 0, {
+      ...output,
+      success: result.success,
+      status: result.status,
+    })
+  }
+
   async execute() {
     let automation = this._automation
     const app = await this.getApp()
@@ -118,62 +136,30 @@ class Orchestrator {
           this._context.steps[loopStepNumber] = {
             currentItem: newInput.binding[index],
           }
+
           let tempOutput = { items: loopSteps, iterations: iterationCount }
-
           if (
-            loopStep.inputs.option === "Array" &&
-            !Array.isArray(newInput.binding)
+            (loopStep.inputs.option === "Array" &&
+              !Array.isArray(newInput.binding)) ||
+            (loopStep.inputs.option === "String" &&
+              typeof newInput.binding !== "string")
           ) {
-            this.executionOutput.steps.splice(loopStepNumber, 0, {
-              id: step.id,
-              stepId: step.stepId,
-              outputs: {
-                ...tempOutput,
-                success: true,
-                status: "INCORRECT_TYPE",
-              },
-              inputs: step.inputs,
-            })
-            this._context.steps.splice(loopStepNumber, 0, {
-              ...tempOutput,
-              success: true,
+            this.updateContextAndOutput(loopStepNumber, step, tempOutput, {
               status: "INCORRECT_TYPE",
+              success: false,
             })
-
-            loopSteps = null
-            loopStep = null
-            break
-          } else if (
-            loopStep.inputs.option === "String" &&
-            typeof newInput.binding !== "string"
-          ) {
-            this.executionOutput.steps.splice(loopStepNumber, 0, {
-              id: step.id,
-              stepId: step.stepId,
-              outputs: {
-                ...tempOutput,
-                success: false,
-                status: "INCORRECT_TYPE",
-              },
-              inputs: step.inputs,
-            })
-            this._context.steps.splice(loopStepNumber, 0, {
-              ...tempOutput,
-              success: true,
-              status: "INCORRECT_TYPE",
-            })
-
             loopSteps = null
             loopStep = null
             break
           }
 
           // The "Loop" binding in the front end is "fake", so replace it here so the context can understand it
+          // Pretty hacky because we need to account for the row object
           for (let key in originalStepInput) {
             if (key === "row") {
-              for (let test in originalStepInput["row"]) {
-                originalStepInput["row"][test] = originalStepInput["row"][
-                  test
+              for (let val in originalStepInput["row"]) {
+                originalStepInput["row"][val] = originalStepInput["row"][
+                  val
                 ].replace(/loop/, `steps.${loopStepNumber}`)
               }
             } else {
@@ -185,18 +171,10 @@ class Orchestrator {
           }
 
           if (index >= loopStep.inputs.iterations) {
-            this.executionOutput.steps.splice(loopStepNumber, 0, {
-              id: step.id,
-              stepId: step.stepId,
-              outputs: { ...tempOutput, success: true, status: "LOOP_BROKEN" },
-              inputs: step.inputs,
-            })
-            this._context.steps.splice(loopStepNumber, 0, {
-              ...tempOutput,
+            this.updateContextAndOutput(loopStepNumber, step, tempOutput, {
+              status: "MAX_ITERATIONS_REACHED",
               success: true,
-              status: "LOOP_BROKEN",
             })
-
             loopSteps = null
             loopStep = null
             break
@@ -206,23 +184,10 @@ class Orchestrator {
             this._context.steps[loopStepNumber]?.currentItem ===
             loopStep.inputs.failure
           ) {
-            console.log("hello?????")
-            this.executionOutput.steps.splice(loopStepNumber, 0, {
-              id: step.id,
-              stepId: step.stepId,
-              outputs: {
-                ...tempOutput,
-                success: false,
-                status: "FAILURE_CONDITION_MET",
-              },
-              inputs: step.inputs,
-            })
-            this._context.steps.splice(loopStepNumber, 0, {
-              ...tempOutput,
-              success: false,
+            this.updateContextAndOutput(loopStepNumber, step, tempOutput, {
               status: "FAILURE_CONDITION_MET",
+              success: false,
             })
-
             loopSteps = null
             loopStep = null
             break