From fe7b632727904fbb0b6fb46b08837a4fdedf4278 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 24 Jan 2023 19:09:36 +0000 Subject: [PATCH 1/2] Quick updates for #8989 - this issue appeared to be fixed already but just adding a few extra bits of security to make sure that looping only occurs when valid data is found in the binding. --- packages/bbui/yarn.lock | 8 ++++---- packages/server/src/threads/automation.ts | 14 +++++++++----- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/packages/bbui/yarn.lock b/packages/bbui/yarn.lock index 3286c33e69..16f1feb920 100644 --- a/packages/bbui/yarn.lock +++ b/packages/bbui/yarn.lock @@ -114,10 +114,10 @@ resolved "https://registry.yarnpkg.com/@spectrum-css/accordion/-/accordion-3.0.24.tgz#f89066c120c57b0cfc9aba66d60c39fc1cf69f74" integrity sha512-jNOmUsxmiT3lRLButnN5KKHM94fd+87fjiF8L0c4uRNgJl6ZsBuxPXrM15lV4y1f8D2IACAw01/ZkGRAeaCOFA== -"@spectrum-css/actionbutton@^1.0.1": - version "1.0.2" - resolved "https://registry.yarnpkg.com/@spectrum-css/actionbutton/-/actionbutton-1.0.2.tgz#7753a94c64cebecfca6749ef20e37a5ea80c59be" - integrity sha512-laDWk7PCgy2I0AGsMjTmYKkiMVYVoF1B4tffJf4cIp66znTiqPHEbLDh5EDNU88JLTY2bWoCOY4cJxvXk5gERw== +"@spectrum-css/actionbutton@1.0.1": + version "1.0.1" + resolved "https://registry.yarnpkg.com/@spectrum-css/actionbutton/-/actionbutton-1.0.1.tgz#9c75da37ea6915919fb574c74bd60dacc03b6577" + integrity sha512-AUqtyNabHF451Aj9i3xz82TxS5Z6k1dttA68/1hMeU9kbPCSS4P6Viw3vaRGs9CSspuR8xnnhDgrq+F+zMy2Hw== "@spectrum-css/actiongroup@1.0.1": version "1.0.1" diff --git a/packages/server/src/threads/automation.ts b/packages/server/src/threads/automation.ts index 44951869f4..d765ef8472 100644 --- a/packages/server/src/threads/automation.ts +++ b/packages/server/src/threads/automation.ts @@ -37,9 +37,13 @@ function getLoopIterations(loopStep: LoopStep, input: LoopInput) { if (!loopStep || !binding) { return 1 } - return Array.isArray(binding) - ? binding.length - : automationUtils.stringSplit(binding).length + if (Array.isArray(binding)) { + return binding.length + } + if (typeof binding === "string") { + return automationUtils.stringSplit(binding).length + } + return 1 } /** @@ -280,13 +284,13 @@ class Orchestrator { break } - let item + let item = [] if ( typeof loopStep.inputs.binding === "string" && loopStep.inputs.option === "String" ) { item = automationUtils.stringSplit(newInput.binding) - } else { + } else if (Array.isArray(loopStep.inputs.binding)) { item = loopStep.inputs.binding } this._context.steps[loopStepNumber] = { From 697cd8b4eafe1b390b9bee9343f4f66ec2297b62 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 25 Jan 2023 18:27:58 +0000 Subject: [PATCH 2/2] Adding test cases as per PR comments. --- .../server/src/automations/tests/loop.spec.ts | 45 +++++++++++++ packages/server/src/automations/triggers.ts | 7 +- .../server/src/tests/utilities/structures.ts | 67 ++++++++++++++++++- .../types/src/documents/app/automation.ts | 1 + 4 files changed, 117 insertions(+), 3 deletions(-) create mode 100644 packages/server/src/automations/tests/loop.spec.ts diff --git a/packages/server/src/automations/tests/loop.spec.ts b/packages/server/src/automations/tests/loop.spec.ts new file mode 100644 index 0000000000..b64f7b16f8 --- /dev/null +++ b/packages/server/src/automations/tests/loop.spec.ts @@ -0,0 +1,45 @@ +import * as automation from "../index" +import * as triggers from "../triggers" +import { loopAutomation } from "../../tests/utilities/structures" +import { context } from "@budibase/backend-core" +import * as setup from "./utilities" + +describe("Attempt to run a basic loop automation", () => { + let config = setup.getConfig(), + table: any, + row: any + + beforeEach(async () => { + await automation.init() + await config.init() + table = await config.createTable() + row = await config.createRow() + }) + + afterAll(setup.afterAll) + + async function runLoop(loopOpts?: any) { + const appId = config.getAppId() + return await context.doInAppContext(appId, async () => { + const params = { fields: { appId } } + return await triggers.externalTrigger( + loopAutomation(table._id, loopOpts), + params, + { getResponses: true } + ) + }) + } + + it("attempt to run a basic loop", async () => { + const resp = await runLoop() + expect(resp.steps[2].outputs.iterations).toBe(1) + }) + + it("test a loop with a string", async () => { + const resp = await runLoop({ + type: "String", + binding: "a,b,c", + }) + expect(resp.steps[2].outputs.iterations).toBe(3) + }) +}) diff --git a/packages/server/src/automations/triggers.ts b/packages/server/src/automations/triggers.ts index f4b34bc9e8..78f8a87b0c 100644 --- a/packages/server/src/automations/triggers.ts +++ b/packages/server/src/automations/triggers.ts @@ -109,8 +109,13 @@ export async function externalTrigger( } params.fields = coercedFields } - const data = { automation, event: params } + const data: Record = { automation, event: params } if (getResponses) { + data.event = { + ...data.event, + appId: context.getAppId(), + automation, + } return utils.processEvent({ data }) } else { return automationQueue.add(data, JOB_OPTS) diff --git a/packages/server/src/tests/utilities/structures.ts b/packages/server/src/tests/utilities/structures.ts index a412be4931..9d66fecc5e 100644 --- a/packages/server/src/tests/utilities/structures.ts +++ b/packages/server/src/tests/utilities/structures.ts @@ -1,8 +1,14 @@ -import { roles, permissions } from "@budibase/backend-core" +import { permissions, roles } from "@budibase/backend-core" import { createHomeScreen } from "../../constants/screens" import { EMPTY_LAYOUT } from "../../constants/layouts" import { cloneDeep } from "lodash/fp" -import { TRIGGER_DEFINITIONS, ACTION_DEFINITIONS } from "../../automations" +import { ACTION_DEFINITIONS, TRIGGER_DEFINITIONS } from "../../automations" +import { + Automation, + AutomationActionStepId, + AutomationTriggerStepId, +} from "@budibase/types" + const { v4: uuidv4 } = require("uuid") export const TENANT_ID = "default" @@ -116,6 +122,63 @@ export function basicAutomation() { } } +export function loopAutomation(tableId: string, loopOpts?: any): Automation { + if (!loopOpts) { + loopOpts = { + option: "Array", + binding: "{{ steps.1.rows }}", + } + } + const automation: any = { + name: "looping", + type: "automation", + definition: { + steps: [ + { + id: "b", + type: "ACTION", + stepId: AutomationActionStepId.QUERY_ROWS, + internal: true, + inputs: { + tableId, + }, + schema: ACTION_DEFINITIONS.QUERY_ROWS.schema, + }, + { + id: "c", + type: "ACTION", + stepId: AutomationActionStepId.LOOP, + internal: true, + inputs: loopOpts, + blockToLoop: "d", + schema: ACTION_DEFINITIONS.LOOP.schema, + }, + { + id: "d", + type: "ACTION", + internal: true, + stepId: AutomationActionStepId.SERVER_LOG, + inputs: { + text: "log statement", + }, + schema: ACTION_DEFINITIONS.SERVER_LOG.schema, + }, + ], + trigger: { + id: "a", + type: "TRIGGER", + event: "row:save", + stepId: AutomationTriggerStepId.ROW_SAVED, + inputs: { + tableId, + }, + schema: TRIGGER_DEFINITIONS.ROW_SAVED.schema, + }, + }, + } + return automation as Automation +} + export function basicRow(tableId: string) { return { name: "Test Contact", diff --git a/packages/types/src/documents/app/automation.ts b/packages/types/src/documents/app/automation.ts index 184fed629d..0aa11d808b 100644 --- a/packages/types/src/documents/app/automation.ts +++ b/packages/types/src/documents/app/automation.ts @@ -50,6 +50,7 @@ export interface AutomationStepSchema { internal?: boolean deprecated?: boolean stepId: AutomationTriggerStepId | AutomationActionStepId + blockToLoop?: string inputs: { [key: string]: any }