From 56641e06c3cc5f599e2a8dc481ad63434fb43ef2 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Tue, 3 Sep 2024 11:10:30 +0100 Subject: [PATCH 01/11] re-add branch step to outputs --- .../automations/tests/scenarios/scenarios.spec.ts | 8 ++++---- packages/server/src/threads/automation.ts | 13 +++++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/packages/server/src/automations/tests/scenarios/scenarios.spec.ts b/packages/server/src/automations/tests/scenarios/scenarios.spec.ts index a0dab7f177..51d9ddc820 100644 --- a/packages/server/src/automations/tests/scenarios/scenarios.spec.ts +++ b/packages/server/src/automations/tests/scenarios/scenarios.spec.ts @@ -64,7 +64,7 @@ describe("Automation Scenarios", () => { }) .run() - 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 () => { @@ -92,7 +92,7 @@ describe("Automation Scenarios", () => { }) .run() - expect(results.steps[0].outputs.message).toContain("Active user") + expect(results.steps[1].outputs.message).toContain("Active user") }) it("should handle multiple conditions with AND operator", async () => { @@ -124,7 +124,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 +162,7 @@ describe("Automation Scenarios", () => { }) .run() - expect(results.steps[0].outputs.message).toContain("Special user") + expect(results.steps[1].outputs.message).toContain("Special user") }) }) diff --git a/packages/server/src/threads/automation.ts b/packages/server/src/threads/automation.ts index eff8407104..67e2937f3f 100644 --- a/packages/server/src/threads/automation.ts +++ b/packages/server/src/threads/automation.ts @@ -461,6 +461,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 From 839292b84d61d0ceb47d67692a1f1c8a2cdf7371 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Tue, 3 Sep 2024 16:16:52 +0100 Subject: [PATCH 02/11] add validators and tests for automation branching --- .../src/api/routes/tests/automation.spec.ts | 74 ++++++++++ .../server/src/api/routes/utils/validators.ts | 49 ++++++- .../server/src/tests/utilities/structures.ts | 126 ++++++++++++++++++ 3 files changed, 245 insertions(+), 4 deletions(-) diff --git a/packages/server/src/api/routes/tests/automation.spec.ts b/packages/server/src/api/routes/tests/automation.spec.ts index 1ade332f0a..3f35eb0c9a 100644 --- a/packages/server/src/api/routes/tests/automation.spec.ts +++ b/packages/server/src/api/routes/tests/automation.spec.ts @@ -25,6 +25,8 @@ let { collectAutomation, filterAutomation, updateRowAutomationWithFilters, + branchAutomationIncorrectPosition, + branchAutomation, } = setup.structures describe("/automations", () => { @@ -121,6 +123,78 @@ 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 = branchAutomationIncorrectPosition() + const res = await request + .post(`/api/automations`) + .set(config.defaultHeaders()) + .send(automation) + .expect("Content-Type", /json/) + .expect(400) + + expect(res.body.message).toContain("must contain at least 1 items") + }) + + it("Should check validation on an automation that has a branch step with no children", async () => { + const automation = branchAutomationIncorrectPosition() + automation.definition.steps[0].inputs.branches = [ + { name: "test", condition: { equal: { "steps.1.success": "true" } } }, + ] + automation.definition.steps[0].inputs.children = {} + + const res = await request + .post(`/api/automations`) + .set(config.defaultHeaders()) + .send(automation) + .expect("Content-Type", /json/) + .expect(400) + + expect(res.body.message).toContain( + "Branch steps are only allowed as the last step" + ) + }) + + it("Should check validation on a branch step with empty conditions", async () => { + const automation = branchAutomation() + + automation.definition.steps[1].inputs.branches = [ + { name: "test", condition: {} }, + ] + automation.definition.steps[1].inputs.children = {} + + const res = await request + .post(`/api/automations`) + .set(config.defaultHeaders()) + .send(automation) + .expect("Content-Type", /json/) + .expect(400) + + expect(res.body.message).toContain("must have at least 1 key") + }) + + it("Should check validation on an branch that has a condition that is not valid", async () => { + const automation = branchAutomation() + + automation.definition.steps[1].inputs.branches = [ + { + name: "test", + condition: { + INCORRECT: { "steps.1.success": true }, + }, + }, + ] + automation.definition.steps[1].inputs.children = {} + + const res = await request + .post(`/api/automations`) + .set(config.defaultHeaders()) + .send(automation) + .expect("Content-Type", /json/) + .expect(400) + + expect(res.body.message).toContain('INCORRECT" is not allowed') + }) + it("should apply authorization to endpoint", async () => { const automation = newAutomation() await checkBuilderEndpoint({ diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 9aa112cf4d..e68d5b3795 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/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,7 @@ export function datasourceValidator() { ) } -function filterObject() { +function filterObject(unknown = true) { const conditionalFilteringObject = () => Joi.object({ conditions: Joi.array().items(Joi.link("#schema")).required(), @@ -115,7 +118,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 +262,11 @@ export function screenValidator() { } function generateStepSchema(allowStepTypes: string[]) { + const branchSchema = Joi.object({ + name: Joi.string().required(), + condition: filterObject(false).required().min(1), + }) + return Joi.object({ stepId: Joi.string().required(), id: Joi.string().required(), @@ -267,6 +275,17 @@ 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() @@ -274,6 +293,17 @@ function generateStepSchema(allowStepTypes: string[]) { }).unknown(true) } +const validateStepsArray = ( + steps: AutomationStep[], + helpers: Joi.CustomHelpers +) => { + for (let i = 0; i < steps.length - 1; i++) { + if (steps[i].stepId === AutomationActionStepId.BRANCH) { + return helpers.error("branchStepPosition") + } + } +} + export function automationValidator(existing = false) { return auth.joiValidator.body( Joi.object({ @@ -284,9 +314,20 @@ export function automationValidator(existing = false) { definition: Joi.object({ steps: Joi.array() .required() - .items(generateStepSchema(["ACTION", "LOGIC"])), - trigger: generateStepSchema(["TRIGGER"]).allow(null), + .items( + 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) diff --git a/packages/server/src/tests/utilities/structures.ts b/packages/server/src/tests/utilities/structures.ts index 2e501932b8..6655444a13 100644 --- a/packages/server/src/tests/utilities/structures.ts +++ b/packages/server/src/tests/utilities/structures.ts @@ -292,6 +292,132 @@ export function serverLogAutomation(appId?: string): Automation { } } +export function branchAutomationIncorrectPosition(appId?: string): Automation { + return { + name: "My Automation", + screenId: "kasdkfldsafkl", + live: true, + uiTree: {}, + definition: { + trigger: { + stepId: AutomationTriggerStepId.APP, + name: "test", + tagline: "test", + icon: "test", + description: "test", + type: AutomationStepType.TRIGGER, + id: "test", + inputs: { fields: {} }, + schema: { + inputs: { + properties: {}, + }, + outputs: { + properties: {}, + }, + }, + }, + steps: [ + { + stepId: AutomationActionStepId.BRANCH, + name: "Branch", + tagline: "Console log a value in the backend", + icon: "Monitoring", + description: "Logs the given text to the server (using console.log)", + inputs: { + branches: [], + }, + schema: { inputs: { properties: {} }, outputs: { properties: {} } }, + id: "y8lkZbeSe", + type: AutomationStepType.LOGIC, + }, + { + stepId: AutomationActionStepId.SERVER_LOG, + name: "Backend log", + tagline: "Console log a value in the backend", + icon: "Monitoring", + description: "Logs the given text to the server (using console.log)", + internal: true, + features: { + LOOPING: true, + }, + inputs: { + text: "log statement", + }, + schema: BUILTIN_ACTION_DEFINITIONS.SERVER_LOG.schema, + id: "y8lkZbeSe", + type: AutomationStepType.ACTION, + }, + ], + }, + type: "automation", + appId: appId!, + } +} + +export function branchAutomation(appId?: string): Automation { + return { + name: "My Automation", + screenId: "kasdkfldsafkl", + live: true, + uiTree: {}, + definition: { + trigger: { + stepId: AutomationTriggerStepId.APP, + name: "test", + tagline: "test", + icon: "test", + description: "test", + type: AutomationStepType.TRIGGER, + id: "test", + inputs: { fields: {} }, + schema: { + inputs: { + properties: {}, + }, + outputs: { + properties: {}, + }, + }, + }, + steps: [ + { + stepId: AutomationActionStepId.SERVER_LOG, + name: "Backend log", + tagline: "Console log a value in the backend", + icon: "Monitoring", + description: "Logs the given text to the server (using console.log)", + internal: true, + features: { + LOOPING: true, + }, + inputs: { + text: "log statement", + }, + schema: BUILTIN_ACTION_DEFINITIONS.SERVER_LOG.schema, + id: "y8lkZbeSe", + type: AutomationStepType.ACTION, + }, + { + stepId: AutomationActionStepId.BRANCH, + name: "Branch", + tagline: "Console log a value in the backend", + icon: "Monitoring", + description: "Logs the given text to the server (using console.log)", + inputs: { + branches: [], + }, + schema: { inputs: { properties: {} }, outputs: { properties: {} } }, + id: "y8lkZbeSe", + type: AutomationStepType.LOGIC, + }, + ], + }, + type: "automation", + appId: appId!, + } +} + export function loopAutomation( tableId: string, loopOpts?: LoopInput From 484b329bc893c1ba130a2be015d31ad1948009f9 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Tue, 3 Sep 2024 16:29:52 +0100 Subject: [PATCH 03/11] ensure branch step is in output --- .../src/automations/tests/scenarios/scenarios.spec.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/server/src/automations/tests/scenarios/scenarios.spec.ts b/packages/server/src/automations/tests/scenarios/scenarios.spec.ts index 51d9ddc820..3f86cefc58 100644 --- a/packages/server/src/automations/tests/scenarios/scenarios.spec.ts +++ b/packages/server/src/automations/tests/scenarios/scenarios.spec.ts @@ -63,7 +63,7 @@ describe("Automation Scenarios", () => { }, }) .run() - + expect(results.steps[3].outputs.status).toContain("branch1 branch taken") expect(results.steps[4].outputs.message).toContain("Branch 1.1") }) @@ -91,7 +91,9 @@ describe("Automation Scenarios", () => { }, }) .run() - + expect(results.steps[0].outputs.status).toContain( + "activeBranch branch taken" + ) expect(results.steps[1].outputs.message).toContain("Active user") }) From f6f6120e706df84ab64df6881a570631636a5a47 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Tue, 3 Sep 2024 21:14:05 +0100 Subject: [PATCH 04/11] Update packages/server/src/api/routes/utils/validators.ts Co-authored-by: Sam Rose --- packages/server/src/api/routes/utils/validators.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index e68d5b3795..9218223d08 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -91,7 +91,8 @@ export function datasourceValidator() { ) } -function filterObject(unknown = true) { +function filterObject(opts?: { unknown: boolean }) { + const { unknown = true } = opts || {} const conditionalFilteringObject = () => Joi.object({ conditions: Joi.array().items(Joi.link("#schema")).required(), From 8c7aecfa22f5850dfccd103004adc38c8aefed6f Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Tue, 3 Sep 2024 21:14:28 +0100 Subject: [PATCH 05/11] Update packages/server/src/api/routes/utils/validators.ts Co-authored-by: Sam Rose --- packages/server/src/api/routes/utils/validators.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 9218223d08..d83569673f 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -298,8 +298,8 @@ const validateStepsArray = ( steps: AutomationStep[], helpers: Joi.CustomHelpers ) => { - for (let i = 0; i < steps.length - 1; i++) { - if (steps[i].stepId === AutomationActionStepId.BRANCH) { + for (const step of steps.slice(0, -1)) { + if (step.stepId === AutomationActionStepId.BRANCH) { return helpers.error("branchStepPosition") } } From 071670264681f1e64963df38b2dc56980c45f550 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Tue, 3 Sep 2024 21:31:38 +0100 Subject: [PATCH 06/11] use opts param --- packages/server/src/api/routes/utils/validators.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index d83569673f..334b3bac26 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -265,7 +265,7 @@ export function screenValidator() { function generateStepSchema(allowStepTypes: string[]) { const branchSchema = Joi.object({ name: Joi.string().required(), - condition: filterObject(false).required().min(1), + condition: filterObject({ unknown: false }).required().min(1), }) return Joi.object({ From d80123bbcb559a63f9f0681fa48d8a18bfccfee2 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Wed, 4 Sep 2024 09:31:18 +0100 Subject: [PATCH 07/11] use utilities api --- .../src/api/routes/tests/automation.spec.ts | 61 +++++++++---------- .../src/tests/utilities/api/automation.ts | 10 +++ 2 files changed, 38 insertions(+), 33 deletions(-) diff --git a/packages/server/src/api/routes/tests/automation.spec.ts b/packages/server/src/api/routes/tests/automation.spec.ts index 3f35eb0c9a..9a31cce9a6 100644 --- a/packages/server/src/api/routes/tests/automation.spec.ts +++ b/packages/server/src/api/routes/tests/automation.spec.ts @@ -125,14 +125,14 @@ describe("/automations", () => { it("Should ensure you can't have a branch as not a last step", async () => { const automation = branchAutomationIncorrectPosition() - const res = await request - .post(`/api/automations`) - .set(config.defaultHeaders()) - .send(automation) - .expect("Content-Type", /json/) - .expect(400) - expect(res.body.message).toContain("must contain at least 1 items") + 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 an automation that has a branch step with no children", async () => { @@ -142,16 +142,13 @@ describe("/automations", () => { ] automation.definition.steps[0].inputs.children = {} - const res = await request - .post(`/api/automations`) - .set(config.defaultHeaders()) - .send(automation) - .expect("Content-Type", /json/) - .expect(400) - - expect(res.body.message).toContain( - "Branch steps are only allowed as the last step" - ) + 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 a branch step with empty conditions", async () => { @@ -162,14 +159,13 @@ describe("/automations", () => { ] automation.definition.steps[1].inputs.children = {} - const res = await request - .post(`/api/automations`) - .set(config.defaultHeaders()) - .send(automation) - .expect("Content-Type", /json/) - .expect(400) - - expect(res.body.message).toContain("must have at least 1 key") + await config.api.automation.post(automation, { + status: 400, + body: { + message: + 'Invalid body - "definition.steps[1].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 () => { @@ -185,14 +181,13 @@ describe("/automations", () => { ] automation.definition.steps[1].inputs.children = {} - const res = await request - .post(`/api/automations`) - .set(config.defaultHeaders()) - .send(automation) - .expect("Content-Type", /json/) - .expect(400) - - expect(res.body.message).toContain('INCORRECT" is not allowed') + await config.api.automation.post(automation, { + status: 400, + body: { + message: + 'Invalid body - "definition.steps[1].inputs.branches[0].condition.INCORRECT" is not allowed', + }, + }) }) it("should apply authorization to endpoint", async () => { diff --git a/packages/server/src/tests/utilities/api/automation.ts b/packages/server/src/tests/utilities/api/automation.ts index 9620e2011c..61bd915647 100644 --- a/packages/server/src/tests/utilities/api/automation.ts +++ b/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 => { + const result = await this._post(`/api/automations`, { + body, + expectations, + }) + return result + } } From 3a8a8b1195b3c8560b58d724bc98fb666a94549a Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Wed, 4 Sep 2024 14:54:47 +0100 Subject: [PATCH 08/11] fix issue with multiple loops breaking automation context --- .../tests/scenarios/scenarios.spec.ts | 26 +++++++++++++++++++ packages/server/src/threads/automation.ts | 8 ++++-- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/packages/server/src/automations/tests/scenarios/scenarios.spec.ts b/packages/server/src/automations/tests/scenarios/scenarios.spec.ts index a0dab7f177..a4fd473ac9 100644 --- a/packages/server/src/automations/tests/scenarios/scenarios.spec.ts +++ b/packages/server/src/automations/tests/scenarios/scenarios.spec.ts @@ -362,6 +362,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", () => { diff --git a/packages/server/src/threads/automation.ts b/packages/server/src/threads/automation.ts index eff8407104..5e95ab9d89 100644 --- a/packages/server/src/threads/automation.ts +++ b/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("currentItem") + ) + this.loopStepOutputs = [] } @@ -569,8 +573,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 } } From 5e4b2fa500ba012011a7d68a885aba2e015e5871 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Wed, 4 Sep 2024 15:13:11 +0100 Subject: [PATCH 09/11] use .call --- packages/server/src/threads/automation.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/threads/automation.ts b/packages/server/src/threads/automation.ts index 5e95ab9d89..668599bdcc 100644 --- a/packages/server/src/threads/automation.ts +++ b/packages/server/src/threads/automation.ts @@ -451,7 +451,7 @@ class Orchestrator { }) this.context.steps[this.context.steps.length] = tempOutput this.context.steps = this.context.steps.filter( - item => !item.hasOwnProperty("currentItem") + item => !item.hasOwnProperty.call("currentItem") ) this.loopStepOutputs = [] From 9782ddb9eeacf813a6ba9b58ff189315c1c377b7 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Wed, 4 Sep 2024 15:29:07 +0100 Subject: [PATCH 10/11] missing param --- packages/server/src/threads/automation.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/threads/automation.ts b/packages/server/src/threads/automation.ts index 668599bdcc..478dade56f 100644 --- a/packages/server/src/threads/automation.ts +++ b/packages/server/src/threads/automation.ts @@ -451,7 +451,7 @@ class Orchestrator { }) this.context.steps[this.context.steps.length] = tempOutput this.context.steps = this.context.steps.filter( - item => !item.hasOwnProperty.call("currentItem") + item => !item.hasOwnProperty.call(item, "currentItem") ) this.loopStepOutputs = [] From 2135dbca67bff7449f60b76dc2d9de3c03565af7 Mon Sep 17 00:00:00 2001 From: Peter Clement Date: Wed, 4 Sep 2024 16:37:33 +0100 Subject: [PATCH 11/11] update automation test builder to support ...building --- .../src/api/routes/tests/automation.spec.ts | 104 ++++++++++----- .../server/src/api/routes/utils/validators.ts | 4 +- .../tests/utilities/AutomationTestBuilder.ts | 24 ++-- .../server/src/tests/utilities/structures.ts | 126 ------------------ 4 files changed, 84 insertions(+), 174 deletions(-) diff --git a/packages/server/src/api/routes/tests/automation.spec.ts b/packages/server/src/api/routes/tests/automation.spec.ts index 9a31cce9a6..b6233b24ad 100644 --- a/packages/server/src/api/routes/tests/automation.spec.ts +++ b/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 { @@ -25,8 +26,6 @@ let { collectAutomation, filterAutomation, updateRowAutomationWithFilters, - branchAutomationIncorrectPosition, - branchAutomation, } = setup.structures describe("/automations", () => { @@ -124,23 +123,22 @@ describe("/automations", () => { }) it("Should ensure you can't have a branch as not a last step", async () => { - const automation = branchAutomationIncorrectPosition() - - await config.api.automation.post(automation, { - status: 400, - body: { - message: - 'Invalid body - "definition.steps[0].inputs.branches" must contain at least 1 items', - }, + const automation = createAutomationBuilder({ + name: "String Equality Branching", + appId: config.getAppId(), }) - }) - - it("Should check validation on an automation that has a branch step with no children", async () => { - const automation = branchAutomationIncorrectPosition() - automation.definition.steps[0].inputs.branches = [ - { name: "test", condition: { equal: { "steps.1.success": "true" } } }, - ] - automation.definition.steps[0].inputs.children = {} + .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, @@ -151,41 +149,73 @@ describe("/automations", () => { }) }) - it("Should check validation on a branch step with empty conditions", async () => { - const automation = branchAutomation() - - automation.definition.steps[1].inputs.branches = [ - { name: "test", condition: {} }, - ] - automation.definition.steps[1].inputs.children = {} + 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[1].inputs.branches[0].condition" must have at least 1 key', + '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 = branchAutomation() - - automation.definition.steps[1].inputs.branches = [ - { - name: "test", - condition: { - INCORRECT: { "steps.1.success": true }, + 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" }, + }, }, - }, - ] - automation.definition.steps[1].inputs.children = {} + }) + .serverLog({ text: "Inactive user" }) + .build() await config.api.automation.post(automation, { status: 400, body: { message: - 'Invalid body - "definition.steps[1].inputs.branches[0].condition.INCORRECT" is not allowed', + 'Invalid body - "definition.steps[0].inputs.branches[0].condition.INCORRECT" is not allowed', }, }) }) diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 334b3bac26..5e2a585b4a 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -291,7 +291,9 @@ function generateStepSchema(allowStepTypes: string[]) { type: Joi.string() .required() .valid(...allowStepTypes), - }).unknown(true) + }) + .unknown(true) + .id("step") } const validateStepsArray = ( diff --git a/packages/server/src/automations/tests/utilities/AutomationTestBuilder.ts b/packages/server/src/automations/tests/utilities/AutomationTestBuilder.ts index 16cab73b75..f477efabe4 100644 --- a/packages/server/src/automations/tests/utilities/AutomationTestBuilder.ts +++ b/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): { - run: () => Promise - } { + branch(branchConfig: BranchConfig): this { this.addBranchStep(branchConfig) - return { - run: () => this.run(), - } + return this + } + + 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) } diff --git a/packages/server/src/tests/utilities/structures.ts b/packages/server/src/tests/utilities/structures.ts index 6655444a13..2e501932b8 100644 --- a/packages/server/src/tests/utilities/structures.ts +++ b/packages/server/src/tests/utilities/structures.ts @@ -292,132 +292,6 @@ export function serverLogAutomation(appId?: string): Automation { } } -export function branchAutomationIncorrectPosition(appId?: string): Automation { - return { - name: "My Automation", - screenId: "kasdkfldsafkl", - live: true, - uiTree: {}, - definition: { - trigger: { - stepId: AutomationTriggerStepId.APP, - name: "test", - tagline: "test", - icon: "test", - description: "test", - type: AutomationStepType.TRIGGER, - id: "test", - inputs: { fields: {} }, - schema: { - inputs: { - properties: {}, - }, - outputs: { - properties: {}, - }, - }, - }, - steps: [ - { - stepId: AutomationActionStepId.BRANCH, - name: "Branch", - tagline: "Console log a value in the backend", - icon: "Monitoring", - description: "Logs the given text to the server (using console.log)", - inputs: { - branches: [], - }, - schema: { inputs: { properties: {} }, outputs: { properties: {} } }, - id: "y8lkZbeSe", - type: AutomationStepType.LOGIC, - }, - { - stepId: AutomationActionStepId.SERVER_LOG, - name: "Backend log", - tagline: "Console log a value in the backend", - icon: "Monitoring", - description: "Logs the given text to the server (using console.log)", - internal: true, - features: { - LOOPING: true, - }, - inputs: { - text: "log statement", - }, - schema: BUILTIN_ACTION_DEFINITIONS.SERVER_LOG.schema, - id: "y8lkZbeSe", - type: AutomationStepType.ACTION, - }, - ], - }, - type: "automation", - appId: appId!, - } -} - -export function branchAutomation(appId?: string): Automation { - return { - name: "My Automation", - screenId: "kasdkfldsafkl", - live: true, - uiTree: {}, - definition: { - trigger: { - stepId: AutomationTriggerStepId.APP, - name: "test", - tagline: "test", - icon: "test", - description: "test", - type: AutomationStepType.TRIGGER, - id: "test", - inputs: { fields: {} }, - schema: { - inputs: { - properties: {}, - }, - outputs: { - properties: {}, - }, - }, - }, - steps: [ - { - stepId: AutomationActionStepId.SERVER_LOG, - name: "Backend log", - tagline: "Console log a value in the backend", - icon: "Monitoring", - description: "Logs the given text to the server (using console.log)", - internal: true, - features: { - LOOPING: true, - }, - inputs: { - text: "log statement", - }, - schema: BUILTIN_ACTION_DEFINITIONS.SERVER_LOG.schema, - id: "y8lkZbeSe", - type: AutomationStepType.ACTION, - }, - { - stepId: AutomationActionStepId.BRANCH, - name: "Branch", - tagline: "Console log a value in the backend", - icon: "Monitoring", - description: "Logs the given text to the server (using console.log)", - inputs: { - branches: [], - }, - schema: { inputs: { properties: {} }, outputs: { properties: {} } }, - id: "y8lkZbeSe", - type: AutomationStepType.LOGIC, - }, - ], - }, - type: "automation", - appId: appId!, - } -} - export function loopAutomation( tableId: string, loopOpts?: LoopInput