Merge pull request #14508 from Budibase/feat/branching-api-validation

This commit is contained in:
Peter Clement 2024-09-04 17:53:56 +01:00 committed by GitHub
commit 2c9de045a5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 193 additions and 21 deletions

View File

@ -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({

View File

@ -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"])),
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)

View File

@ -63,8 +63,8 @@ describe("Automation Scenarios", () => {
},
})
.run()
expect(results.steps[2].outputs.message).toContain("Branch 1.1")
expect(results.steps[3].outputs.status).toContain("branch1 branch taken")
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.message).toContain("Active user")
expect(results.steps[0].outputs.status).toContain(
"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")
})
})

View File

@ -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<AutomationResults>
} {
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)
}

View File

@ -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
}
}

View File

@ -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