Fix else branches not properly skipped in nested if/else workflows (#20938)
## Summary - Extract `findParentSteps` utility that recognizes IF-ELSE steps as parents of their branch children (via `settings.input.branches[].nextStepIds`), used in all parent detection sites (`shouldSkipStepExecution`, `shouldExecuteStep`, `shouldFailSafely`, and their iterator variants) - Centralize next-step resolution in `getNextStepIdsToExecute` via extracted `getNextStepIdsForIterator` and `getNextStepIdsForIfElse` utils — Iterator now properly returns loop children as `nextStepIdsToSkip`/`nextStepIdsToFailSafely` when skipped - Refactor `skipAndFailSafelyStepsThenContinue` to delegate to `getNextStepIdsToExecute` instead of duplicating type-specific propagation logic Fixes #20934 ## Test plan - [x] New unit tests for `findParentSteps` (7 tests covering IF-ELSE branch parent detection) - [x] New IF-ELSE-specific tests added to `shouldSkipStepExecution`, `shouldExecuteStep`, `shouldFailSafely` test suites - [x] Updated Iterator skip/fail-safely tests in `workflow-executor.workspace-service.spec.ts` - [x] All 300 workflow executor tests pass - [x] `lint:ci` passes
This commit is contained in:
+93
@@ -0,0 +1,93 @@
|
||||
import {
|
||||
createMockCodeStep,
|
||||
createMockIfElseStep,
|
||||
} from 'src/modules/workflow/workflow-executor/utils/create-mock-workflow-steps.util';
|
||||
import { findParentSteps } from 'src/modules/workflow/workflow-executor/utils/find-parent-steps.util';
|
||||
import { type WorkflowAction } from 'src/modules/workflow/workflow-executor/workflow-actions/types/workflow-action.type';
|
||||
|
||||
describe('findParentSteps', () => {
|
||||
it('should find parent via nextStepIds', () => {
|
||||
const parent = createMockCodeStep('parent', ['child']);
|
||||
const child = createMockCodeStep('child');
|
||||
const steps = [parent, child];
|
||||
|
||||
const result = findParentSteps({ step: child, steps });
|
||||
|
||||
expect(result).toEqual([parent]);
|
||||
});
|
||||
|
||||
it('should return empty array when no parent exists', () => {
|
||||
const stepA = createMockCodeStep('a');
|
||||
const stepB = createMockCodeStep('b');
|
||||
const steps = [stepA, stepB];
|
||||
|
||||
const result = findParentSteps({ step: stepB, steps });
|
||||
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
|
||||
it('should find IF-ELSE parent via branch nextStepIds', () => {
|
||||
const branchChild = createMockCodeStep('branch-child');
|
||||
const ifElseStep = createMockIfElseStep('if-else', [
|
||||
{ id: 'true-branch', nextStepIds: ['branch-child'] },
|
||||
{ id: 'false-branch', nextStepIds: ['other-child'] },
|
||||
]);
|
||||
const steps: WorkflowAction[] = [ifElseStep, branchChild];
|
||||
|
||||
const result = findParentSteps({ step: branchChild, steps });
|
||||
|
||||
expect(result).toEqual([ifElseStep]);
|
||||
});
|
||||
|
||||
it('should find IF-ELSE parent for else branch child', () => {
|
||||
const elseChild = createMockCodeStep('else-child');
|
||||
const ifElseStep = createMockIfElseStep('if-else', [
|
||||
{ id: 'true-branch', nextStepIds: ['true-child'] },
|
||||
{ id: 'false-branch', nextStepIds: ['else-child'] },
|
||||
]);
|
||||
const steps: WorkflowAction[] = [ifElseStep, elseChild];
|
||||
|
||||
const result = findParentSteps({ step: elseChild, steps });
|
||||
|
||||
expect(result).toEqual([ifElseStep]);
|
||||
});
|
||||
|
||||
it('should find nested IF-ELSE parent via branch', () => {
|
||||
const nestedIfElse = createMockIfElseStep('nested-if-else', [
|
||||
{ id: 'nested-true', nextStepIds: ['step-y'] },
|
||||
{ id: 'nested-false', nextStepIds: ['step-z'] },
|
||||
]);
|
||||
const outerIfElse = createMockIfElseStep('outer-if-else', [
|
||||
{ id: 'outer-true', nextStepIds: ['step-x'] },
|
||||
{ id: 'outer-false', nextStepIds: ['nested-if-else'] },
|
||||
]);
|
||||
const steps: WorkflowAction[] = [outerIfElse, nestedIfElse];
|
||||
|
||||
const result = findParentSteps({ step: nestedIfElse, steps });
|
||||
|
||||
expect(result).toEqual([outerIfElse]);
|
||||
});
|
||||
|
||||
it('should handle undefined steps gracefully', () => {
|
||||
const parent = createMockCodeStep('parent', ['child']);
|
||||
const child = createMockCodeStep('child');
|
||||
const steps = [parent, undefined as unknown as WorkflowAction, child];
|
||||
|
||||
const result = findParentSteps({ step: child, steps });
|
||||
|
||||
expect(result).toEqual([parent]);
|
||||
});
|
||||
|
||||
it('should find multiple parents from different sources', () => {
|
||||
const child = createMockCodeStep('child');
|
||||
const standardParent = createMockCodeStep('standard', ['child']);
|
||||
const ifElseParent = createMockIfElseStep('if-else', [
|
||||
{ id: 'branch', nextStepIds: ['child'] },
|
||||
]);
|
||||
const steps: WorkflowAction[] = [standardParent, ifElseParent, child];
|
||||
|
||||
const result = findParentSteps({ step: child, steps });
|
||||
|
||||
expect(result).toEqual([standardParent, ifElseParent]);
|
||||
});
|
||||
});
|
||||
+46
@@ -1,6 +1,10 @@
|
||||
import { StepStatus } from 'twenty-shared/workflow';
|
||||
|
||||
import { WorkflowRunStatus } from 'src/modules/workflow/common/standard-objects/workflow-run.workspace-entity';
|
||||
import {
|
||||
createMockCodeStep,
|
||||
createMockIfElseStep,
|
||||
} from 'src/modules/workflow/workflow-executor/utils/create-mock-workflow-steps.util';
|
||||
import { shouldExecuteStep } from 'src/modules/workflow/workflow-executor/utils/should-execute-step.util';
|
||||
import {
|
||||
type WorkflowAction,
|
||||
@@ -466,4 +470,46 @@ describe('shouldExecuteStep', () => {
|
||||
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it('should return true when IF-ELSE parent succeeded and step is a branch child', () => {
|
||||
const ifElseStep = createMockIfElseStep('if-else', [
|
||||
{ id: 'true-branch', nextStepIds: ['step-a'] },
|
||||
{ id: 'false-branch', nextStepIds: ['step-b'] },
|
||||
]);
|
||||
const stepA = createMockCodeStep('step-a');
|
||||
const allSteps: WorkflowAction[] = [ifElseStep, stepA];
|
||||
|
||||
const result = shouldExecuteStep({
|
||||
step: stepA,
|
||||
steps: allSteps,
|
||||
stepInfos: {
|
||||
'if-else': { status: StepStatus.SUCCESS },
|
||||
'step-a': { status: StepStatus.NOT_STARTED },
|
||||
},
|
||||
workflowRunStatus: WorkflowRunStatus.RUNNING,
|
||||
});
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it('should return false when IF-ELSE parent is skipped and step is a branch child', () => {
|
||||
const ifElseStep = createMockIfElseStep('if-else', [
|
||||
{ id: 'true-branch', nextStepIds: ['step-a'] },
|
||||
{ id: 'false-branch', nextStepIds: ['step-b'] },
|
||||
]);
|
||||
const stepA = createMockCodeStep('step-a');
|
||||
const allSteps: WorkflowAction[] = [ifElseStep, stepA];
|
||||
|
||||
const result = shouldExecuteStep({
|
||||
step: stepA,
|
||||
steps: allSteps,
|
||||
stepInfos: {
|
||||
'if-else': { status: StepStatus.SKIPPED },
|
||||
'step-a': { status: StepStatus.NOT_STARTED },
|
||||
},
|
||||
workflowRunStatus: WorkflowRunStatus.RUNNING,
|
||||
});
|
||||
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
+39
@@ -2,9 +2,11 @@ import { StepStatus } from 'twenty-shared/workflow';
|
||||
|
||||
import {
|
||||
createMockCodeStep,
|
||||
createMockIfElseStep,
|
||||
createMockIteratorStep,
|
||||
} from 'src/modules/workflow/workflow-executor/utils/create-mock-workflow-steps.util';
|
||||
import { shouldFailSafely } from 'src/modules/workflow/workflow-executor/utils/should-fail-safely.util';
|
||||
import { type WorkflowAction } from 'src/modules/workflow/workflow-executor/workflow-actions/types/workflow-action.type';
|
||||
|
||||
describe('shouldFailSafely', () => {
|
||||
it('should return false when step has no parents', () => {
|
||||
@@ -107,6 +109,43 @@ describe('shouldFailSafely', () => {
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it('should return true when IF-ELSE parent is FAILED_SAFELY and step is a branch child', () => {
|
||||
const childStep = createMockCodeStep('child');
|
||||
const ifElseStep = createMockIfElseStep('if-else', [
|
||||
{ id: 'true-branch', nextStepIds: ['child'] },
|
||||
{ id: 'false-branch', nextStepIds: ['other'] },
|
||||
]);
|
||||
const steps: WorkflowAction[] = [ifElseStep, childStep];
|
||||
|
||||
const result = shouldFailSafely({
|
||||
step: childStep,
|
||||
steps,
|
||||
stepInfos: {
|
||||
'if-else': { status: StepStatus.FAILED_SAFELY, error: 'err' },
|
||||
},
|
||||
});
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it('should return false when IF-ELSE parent succeeded and step is a branch child', () => {
|
||||
const childStep = createMockCodeStep('child');
|
||||
const ifElseStep = createMockIfElseStep('if-else', [
|
||||
{ id: 'branch', nextStepIds: ['child'] },
|
||||
]);
|
||||
const steps: WorkflowAction[] = [ifElseStep, childStep];
|
||||
|
||||
const result = shouldFailSafely({
|
||||
step: childStep,
|
||||
steps,
|
||||
stepInfos: {
|
||||
'if-else': { status: StepStatus.SUCCESS },
|
||||
},
|
||||
});
|
||||
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('shouldFailSafely for iterator steps', () => {
|
||||
|
||||
+46
-1
@@ -1,6 +1,9 @@
|
||||
import { StepStatus } from 'twenty-shared/workflow';
|
||||
|
||||
import { createMockCodeStep } from 'src/modules/workflow/workflow-executor/utils/create-mock-workflow-steps.util';
|
||||
import {
|
||||
createMockCodeStep,
|
||||
createMockIfElseStep,
|
||||
} from 'src/modules/workflow/workflow-executor/utils/create-mock-workflow-steps.util';
|
||||
import { shouldSkipStepExecution } from 'src/modules/workflow/workflow-executor/utils/should-skip-step-execution.util';
|
||||
import { type WorkflowAction } from 'src/modules/workflow/workflow-executor/workflow-actions/types/workflow-action.type';
|
||||
|
||||
@@ -295,4 +298,46 @@ describe('shouldSkipStepExecution', () => {
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('should return true when IF-ELSE parent is skipped and step is a branch child', () => {
|
||||
const ifElseStep = createMockIfElseStep('if-else', [
|
||||
{ id: 'true-branch', nextStepIds: ['step-a'] },
|
||||
{ id: 'false-branch', nextStepIds: ['step-b'] },
|
||||
]);
|
||||
const stepA = createMockCodeStep('step-a');
|
||||
const stepB = createMockCodeStep('step-b');
|
||||
const steps: WorkflowAction[] = [ifElseStep, stepA, stepB];
|
||||
|
||||
const result = shouldSkipStepExecution({
|
||||
step: stepA,
|
||||
steps,
|
||||
stepInfos: {
|
||||
'if-else': { status: StepStatus.SKIPPED },
|
||||
'step-a': { status: StepStatus.NOT_STARTED },
|
||||
'step-b': { status: StepStatus.NOT_STARTED },
|
||||
},
|
||||
});
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it('should return false when IF-ELSE parent succeeded and step is a branch child', () => {
|
||||
const ifElseStep = createMockIfElseStep('if-else', [
|
||||
{ id: 'true-branch', nextStepIds: ['step-a'] },
|
||||
{ id: 'false-branch', nextStepIds: ['step-b'] },
|
||||
]);
|
||||
const stepA = createMockCodeStep('step-a');
|
||||
const steps: WorkflowAction[] = [ifElseStep, stepA];
|
||||
|
||||
const result = shouldSkipStepExecution({
|
||||
step: stepA,
|
||||
steps,
|
||||
stepInfos: {
|
||||
'if-else': { status: StepStatus.SUCCESS },
|
||||
'step-a': { status: StepStatus.NOT_STARTED },
|
||||
},
|
||||
});
|
||||
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
+30
@@ -0,0 +1,30 @@
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
import { isWorkflowIfElseAction } from 'src/modules/workflow/workflow-executor/workflow-actions/if-else/guards/is-workflow-if-else-action.guard';
|
||||
import { type WorkflowAction } from 'src/modules/workflow/workflow-executor/workflow-actions/types/workflow-action.type';
|
||||
|
||||
export const findParentSteps = ({
|
||||
step,
|
||||
steps,
|
||||
}: {
|
||||
step: WorkflowAction;
|
||||
steps: WorkflowAction[];
|
||||
}): WorkflowAction[] => {
|
||||
return steps.filter((candidateParent) => {
|
||||
if (!isDefined(candidateParent)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (candidateParent.nextStepIds?.includes(step.id)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (isWorkflowIfElseAction(candidateParent)) {
|
||||
return candidateParent.settings.input.branches.some((branch) =>
|
||||
branch.nextStepIds?.includes(step.id),
|
||||
);
|
||||
}
|
||||
|
||||
return false;
|
||||
});
|
||||
};
|
||||
+2
-5
@@ -1,7 +1,7 @@
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { type WorkflowRunStepInfos } from 'twenty-shared/workflow';
|
||||
|
||||
import { WorkflowRunStatus } from 'src/modules/workflow/common/standard-objects/workflow-run.workspace-entity';
|
||||
import { findParentSteps } from 'src/modules/workflow/workflow-executor/utils/find-parent-steps.util';
|
||||
import { shouldExecuteChildStep } from 'src/modules/workflow/workflow-executor/utils/should-execute-child-step.util';
|
||||
import { stepHasBeenStarted } from 'src/modules/workflow/workflow-executor/utils/step-has-been-started.util';
|
||||
import { isWorkflowIteratorAction } from 'src/modules/workflow/workflow-executor/workflow-actions/iterator/guards/is-workflow-iterator-action.guard';
|
||||
@@ -35,10 +35,7 @@ export const shouldExecuteStep = ({
|
||||
return false;
|
||||
}
|
||||
|
||||
const parentSteps = steps.filter(
|
||||
(parentStep) =>
|
||||
isDefined(parentStep) && parentStep.nextStepIds?.includes(step.id),
|
||||
);
|
||||
const parentSteps = findParentSteps({ step, steps });
|
||||
|
||||
return shouldExecuteChildStep({
|
||||
parentSteps,
|
||||
|
||||
+2
-5
@@ -1,7 +1,7 @@
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { StepStatus, type WorkflowRunStepInfos } from 'twenty-shared/workflow';
|
||||
|
||||
import { TERMINAL_STEP_STATUSES } from 'src/modules/workflow/workflow-executor/constants/terminal-step-statuses.constant';
|
||||
import { findParentSteps } from 'src/modules/workflow/workflow-executor/utils/find-parent-steps.util';
|
||||
import { isWorkflowIteratorAction } from 'src/modules/workflow/workflow-executor/workflow-actions/iterator/guards/is-workflow-iterator-action.guard';
|
||||
import { shouldFailSafelyIteratorStep } from 'src/modules/workflow/workflow-executor/workflow-actions/iterator/utils/should-fail-safely-iterator-step.util';
|
||||
import { type WorkflowAction } from 'src/modules/workflow/workflow-executor/workflow-actions/types/workflow-action.type';
|
||||
@@ -23,10 +23,7 @@ export const shouldFailSafely = ({
|
||||
});
|
||||
}
|
||||
|
||||
const parentSteps = steps.filter(
|
||||
(parentStep) =>
|
||||
isDefined(parentStep) && parentStep.nextStepIds?.includes(step.id),
|
||||
);
|
||||
const parentSteps = findParentSteps({ step, steps });
|
||||
|
||||
if (parentSteps.length === 0) {
|
||||
return false;
|
||||
|
||||
+2
-5
@@ -1,6 +1,6 @@
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { StepStatus, type WorkflowRunStepInfos } from 'twenty-shared/workflow';
|
||||
|
||||
import { findParentSteps } from 'src/modules/workflow/workflow-executor/utils/find-parent-steps.util';
|
||||
import { isWorkflowIteratorAction } from 'src/modules/workflow/workflow-executor/workflow-actions/iterator/guards/is-workflow-iterator-action.guard';
|
||||
import { shouldSkipIteratorStepExecution } from 'src/modules/workflow/workflow-executor/workflow-actions/iterator/utils/should-skip-iterator-step-execution.util';
|
||||
import { type WorkflowAction } from 'src/modules/workflow/workflow-executor/workflow-actions/types/workflow-action.type';
|
||||
@@ -22,10 +22,7 @@ export const shouldSkipStepExecution = ({
|
||||
});
|
||||
}
|
||||
|
||||
const parentSteps = steps.filter(
|
||||
(parentStep) =>
|
||||
isDefined(parentStep) && parentStep.nextStepIds?.includes(step.id),
|
||||
);
|
||||
const parentSteps = findParentSteps({ step, steps });
|
||||
|
||||
if (parentSteps.length === 0) {
|
||||
return false;
|
||||
|
||||
+48
@@ -0,0 +1,48 @@
|
||||
import { type WorkflowActionOutput } from 'src/modules/workflow/workflow-executor/types/workflow-action-output.type';
|
||||
import { type WorkflowIfElseResult } from 'src/modules/workflow/workflow-executor/workflow-actions/if-else/types/workflow-if-else-result.type';
|
||||
import { type WorkflowIfElseAction } from 'src/modules/workflow/workflow-executor/workflow-actions/types/workflow-action.type';
|
||||
|
||||
export const getNextStepIdsForIfElse = ({
|
||||
executedStep,
|
||||
executedStepOutput,
|
||||
}: {
|
||||
executedStep: WorkflowIfElseAction;
|
||||
executedStepOutput: WorkflowActionOutput;
|
||||
}): {
|
||||
nextStepIdsToExecute?: string[];
|
||||
nextStepIdsToSkip?: string[];
|
||||
nextStepIdsToFailSafely?: string[];
|
||||
} => {
|
||||
const ifElseResult = executedStepOutput.result as
|
||||
| WorkflowIfElseResult
|
||||
| undefined;
|
||||
|
||||
const branches = executedStep.settings.input.branches;
|
||||
|
||||
if (ifElseResult?.matchingBranchId) {
|
||||
const matchingBranch = branches.find(
|
||||
(branch) => branch.id === ifElseResult.matchingBranchId,
|
||||
);
|
||||
|
||||
const nonMatchingBranches = branches.filter(
|
||||
(branch) => branch.id !== ifElseResult.matchingBranchId,
|
||||
);
|
||||
|
||||
return {
|
||||
nextStepIdsToExecute: matchingBranch?.nextStepIds,
|
||||
nextStepIdsToSkip: nonMatchingBranches.flatMap(
|
||||
(branch) => branch.nextStepIds,
|
||||
),
|
||||
};
|
||||
}
|
||||
|
||||
if (executedStepOutput.shouldFailSafely) {
|
||||
return {
|
||||
nextStepIdsToFailSafely: branches.flatMap((branch) => branch.nextStepIds),
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
nextStepIdsToSkip: branches.flatMap((branch) => branch.nextStepIds),
|
||||
};
|
||||
};
|
||||
+45
@@ -0,0 +1,45 @@
|
||||
import { isString } from '@sniptt/guards';
|
||||
|
||||
import { type WorkflowActionOutput } from 'src/modules/workflow/workflow-executor/types/workflow-action-output.type';
|
||||
import { type WorkflowIteratorResult } from 'src/modules/workflow/workflow-executor/workflow-actions/iterator/types/workflow-iterator-result.type';
|
||||
import { type WorkflowIteratorAction } from 'src/modules/workflow/workflow-executor/workflow-actions/types/workflow-action.type';
|
||||
|
||||
type NextStepIds = {
|
||||
nextStepIdsToExecute?: string[];
|
||||
nextStepIdsToSkip?: string[];
|
||||
nextStepIdsToFailSafely?: string[];
|
||||
};
|
||||
|
||||
// Returns next step IDs for the Iterator's children, or undefined if the
|
||||
// Iterator has processed all items (caller should fall through to nextStepIds).
|
||||
export const getNextStepIdsForIterator = ({
|
||||
executedStep,
|
||||
executedStepOutput,
|
||||
}: {
|
||||
executedStep: WorkflowIteratorAction;
|
||||
executedStepOutput: WorkflowActionOutput;
|
||||
}): NextStepIds | undefined => {
|
||||
const initialLoopStepIds = isString(
|
||||
executedStep.settings.input.initialLoopStepIds,
|
||||
)
|
||||
? JSON.parse(executedStep.settings.input.initialLoopStepIds)
|
||||
: (executedStep.settings.input.initialLoopStepIds ?? []);
|
||||
|
||||
if (executedStepOutput.shouldSkipStepExecution) {
|
||||
return { nextStepIdsToSkip: initialLoopStepIds };
|
||||
}
|
||||
|
||||
if (executedStepOutput.shouldFailSafely) {
|
||||
return { nextStepIdsToFailSafely: initialLoopStepIds };
|
||||
}
|
||||
|
||||
const iteratorStepResult = executedStepOutput.result as
|
||||
| WorkflowIteratorResult
|
||||
| undefined;
|
||||
|
||||
if (!iteratorStepResult?.hasProcessedAllItems) {
|
||||
return { nextStepIdsToExecute: initialLoopStepIds };
|
||||
}
|
||||
|
||||
return undefined;
|
||||
};
|
||||
+5
-7
@@ -2,6 +2,7 @@ import { isDefined } from 'twenty-shared/utils';
|
||||
import { StepStatus, type WorkflowRunStepInfos } from 'twenty-shared/workflow';
|
||||
|
||||
import { TERMINAL_STEP_STATUSES } from 'src/modules/workflow/workflow-executor/constants/terminal-step-statuses.constant';
|
||||
import { findParentSteps } from 'src/modules/workflow/workflow-executor/utils/find-parent-steps.util';
|
||||
import { shouldExecuteChildStep } from 'src/modules/workflow/workflow-executor/utils/should-execute-child-step.util';
|
||||
import { stepHasBeenStarted } from 'src/modules/workflow/workflow-executor/utils/step-has-been-started.util';
|
||||
import { getAllStepIdsInLoop } from 'src/modules/workflow/workflow-executor/workflow-actions/iterator/utils/get-all-step-ids-in-loop.util';
|
||||
@@ -19,10 +20,7 @@ export const shouldExecuteIteratorStep = ({
|
||||
steps: WorkflowAction[];
|
||||
stepInfos: WorkflowRunStepInfos;
|
||||
}) => {
|
||||
const stepsTargetingIterator = steps.filter(
|
||||
(parentStep) =>
|
||||
isDefined(parentStep) && parentStep.nextStepIds?.includes(step.id),
|
||||
);
|
||||
const allParentSteps = findParentSteps({ step, steps });
|
||||
|
||||
const initialLoopStepIds = step.settings.input.initialLoopStepIds;
|
||||
|
||||
@@ -34,13 +32,13 @@ export const shouldExecuteIteratorStep = ({
|
||||
})
|
||||
: [];
|
||||
|
||||
const parentSteps = stepsTargetingIterator.filter(
|
||||
const externalParentSteps = allParentSteps.filter(
|
||||
(parentStep) => !stepIdsInLoop.includes(parentStep.id),
|
||||
);
|
||||
|
||||
const stepsToCheck = stepHasBeenStarted(step.id, stepInfos)
|
||||
? stepsTargetingIterator
|
||||
: parentSteps;
|
||||
? allParentSteps
|
||||
: externalParentSteps;
|
||||
|
||||
// When iterator has been started and has the continue-on-failure flag,
|
||||
// allow re-execution even if all loop-back parents are FAILED_SAFELY/SKIPPED
|
||||
|
||||
+5
-7
@@ -2,6 +2,7 @@ import { isDefined } from 'twenty-shared/utils';
|
||||
import { StepStatus, type WorkflowRunStepInfos } from 'twenty-shared/workflow';
|
||||
|
||||
import { TERMINAL_STEP_STATUSES } from 'src/modules/workflow/workflow-executor/constants/terminal-step-statuses.constant';
|
||||
import { findParentSteps } from 'src/modules/workflow/workflow-executor/utils/find-parent-steps.util';
|
||||
import { stepHasBeenStarted } from 'src/modules/workflow/workflow-executor/utils/step-has-been-started.util';
|
||||
import { getAllStepIdsInLoop } from 'src/modules/workflow/workflow-executor/workflow-actions/iterator/utils/get-all-step-ids-in-loop.util';
|
||||
import {
|
||||
@@ -18,10 +19,7 @@ export const shouldFailSafelyIteratorStep = ({
|
||||
steps: WorkflowAction[];
|
||||
stepInfos: WorkflowRunStepInfos;
|
||||
}): boolean => {
|
||||
const stepsTargetingIterator = steps.filter(
|
||||
(parentStep) =>
|
||||
isDefined(parentStep) && parentStep.nextStepIds?.includes(step.id),
|
||||
);
|
||||
const allParentSteps = findParentSteps({ step, steps });
|
||||
|
||||
const initialLoopStepIds = step.settings.input.initialLoopStepIds;
|
||||
|
||||
@@ -33,7 +31,7 @@ export const shouldFailSafelyIteratorStep = ({
|
||||
})
|
||||
: [];
|
||||
|
||||
const externalParentSteps = stepsTargetingIterator.filter(
|
||||
const externalParentSteps = allParentSteps.filter(
|
||||
(parentStep) => !stepIdsInLoop.includes(parentStep.id),
|
||||
);
|
||||
|
||||
@@ -55,7 +53,7 @@ export const shouldFailSafelyIteratorStep = ({
|
||||
return areAllExternalParentsTerminal && hasFailedSafelyExternalParent;
|
||||
}
|
||||
|
||||
const areAllParentsTerminal = stepsTargetingIterator.every((parentStep) =>
|
||||
const areAllParentsTerminal = allParentSteps.every((parentStep) =>
|
||||
TERMINAL_STEP_STATUSES.includes(stepInfos[parentStep.id]?.status),
|
||||
);
|
||||
|
||||
@@ -63,7 +61,7 @@ export const shouldFailSafelyIteratorStep = ({
|
||||
return false;
|
||||
}
|
||||
|
||||
const hasFailedSafelyParent = stepsTargetingIterator.some(
|
||||
const hasFailedSafelyParent = allParentSteps.some(
|
||||
(parentStep) =>
|
||||
stepInfos[parentStep.id]?.status === StepStatus.FAILED_SAFELY,
|
||||
);
|
||||
|
||||
+4
-6
@@ -1,6 +1,7 @@
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { StepStatus, type WorkflowRunStepInfos } from 'twenty-shared/workflow';
|
||||
|
||||
import { findParentSteps } from 'src/modules/workflow/workflow-executor/utils/find-parent-steps.util';
|
||||
import { stepHasBeenStarted } from 'src/modules/workflow/workflow-executor/utils/step-has-been-started.util';
|
||||
import { getAllStepIdsInLoop } from 'src/modules/workflow/workflow-executor/workflow-actions/iterator/utils/get-all-step-ids-in-loop.util';
|
||||
import {
|
||||
@@ -17,10 +18,7 @@ export const shouldSkipIteratorStepExecution = ({
|
||||
steps: WorkflowAction[];
|
||||
stepInfos: WorkflowRunStepInfos;
|
||||
}) => {
|
||||
const stepsTargetingIterator = steps.filter(
|
||||
(parentStep) =>
|
||||
isDefined(parentStep) && parentStep.nextStepIds?.includes(step.id),
|
||||
);
|
||||
const allParentSteps = findParentSteps({ step, steps });
|
||||
|
||||
const initialLoopStepIds = step.settings.input.initialLoopStepIds;
|
||||
|
||||
@@ -32,8 +30,8 @@ export const shouldSkipIteratorStepExecution = ({
|
||||
})
|
||||
: [];
|
||||
|
||||
const parentSteps = stepsTargetingIterator.filter(
|
||||
(step) => !stepIdsInLoop.includes(step.id),
|
||||
const parentSteps = allParentSteps.filter(
|
||||
(parentStep) => !stepIdsInLoop.includes(parentStep.id),
|
||||
);
|
||||
|
||||
if (stepHasBeenStarted(step.id, stepInfos) || parentSteps.length === 0) {
|
||||
|
||||
+4
-4
@@ -542,7 +542,7 @@ describe('WorkflowExecutorWorkspaceService', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('should return nextStepIds for a fail-safe iterator instead of entering the loop', async () => {
|
||||
it('should return loop children as nextStepIdsToFailSafely for a fail-safe iterator', async () => {
|
||||
const step = {
|
||||
id: 'iterator-1',
|
||||
type: WorkflowActionType.ITERATOR,
|
||||
@@ -562,11 +562,11 @@ describe('WorkflowExecutorWorkspaceService', () => {
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
nextStepIdsToExecute: ['after-loop'],
|
||||
nextStepIdsToFailSafely: ['loop-step-1'],
|
||||
});
|
||||
});
|
||||
|
||||
it('should return nextStepIds for a skipped iterator instead of entering the loop', async () => {
|
||||
it('should return loop children as nextStepIdsToSkip for a skipped iterator', async () => {
|
||||
const step = {
|
||||
id: 'iterator-1',
|
||||
type: WorkflowActionType.ITERATOR,
|
||||
@@ -586,7 +586,7 @@ describe('WorkflowExecutorWorkspaceService', () => {
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
nextStepIdsToExecute: ['after-loop'],
|
||||
nextStepIdsToSkip: ['loop-step-1'],
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
+70
-60
@@ -1,6 +1,5 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
|
||||
import { isString } from '@sniptt/guards';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import {
|
||||
getWorkflowRunContext,
|
||||
@@ -42,10 +41,10 @@ import { shouldSkipStepExecution } from 'src/modules/workflow/workflow-executor/
|
||||
import { workflowShouldFail } from 'src/modules/workflow/workflow-executor/utils/workflow-should-fail.util';
|
||||
import { workflowShouldKeepRunning } from 'src/modules/workflow/workflow-executor/utils/workflow-should-keep-running.util';
|
||||
import { isWorkflowIfElseAction } from 'src/modules/workflow/workflow-executor/workflow-actions/if-else/guards/is-workflow-if-else-action.guard';
|
||||
import { type WorkflowIfElseResult } from 'src/modules/workflow/workflow-executor/workflow-actions/if-else/types/workflow-if-else-result.type';
|
||||
import { getNextStepIdsForIfElse } from 'src/modules/workflow/workflow-executor/workflow-actions/if-else/utils/get-next-step-ids-for-if-else.util';
|
||||
import { isWorkflowIteratorAction } from 'src/modules/workflow/workflow-executor/workflow-actions/iterator/guards/is-workflow-iterator-action.guard';
|
||||
import { WorkflowIteratorResult } from 'src/modules/workflow/workflow-executor/workflow-actions/iterator/types/workflow-iterator-result.type';
|
||||
import { findEnclosingIteratorWithContinueOnFailure } from 'src/modules/workflow/workflow-executor/workflow-actions/iterator/utils/find-enclosing-iterator-with-continue-on-failure.util';
|
||||
import { getNextStepIdsForIterator } from 'src/modules/workflow/workflow-executor/workflow-actions/iterator/utils/get-next-step-ids-for-iterator.util';
|
||||
import { WorkflowAction } from 'src/modules/workflow/workflow-executor/workflow-actions/types/workflow-action.type';
|
||||
import { RUN_WORKFLOW_JOB_NAME } from 'src/modules/workflow/workflow-runner/constants/run-workflow-job-name';
|
||||
import { type RunWorkflowJobData } from 'src/modules/workflow/workflow-runner/types/run-workflow-job-data.type';
|
||||
@@ -250,61 +249,22 @@ export class WorkflowExecutorWorkspaceService {
|
||||
nextStepIdsToSkip?: string[];
|
||||
nextStepIdsToFailSafely?: string[];
|
||||
}> {
|
||||
const isIteratorStep = isWorkflowIteratorAction(executedStep);
|
||||
if (isWorkflowIteratorAction(executedStep)) {
|
||||
const result = getNextStepIdsForIterator({
|
||||
executedStep,
|
||||
executedStepOutput,
|
||||
});
|
||||
|
||||
if (isIteratorStep) {
|
||||
const iteratorStepResult = executedStepOutput.result as
|
||||
| WorkflowIteratorResult
|
||||
| undefined;
|
||||
|
||||
if (
|
||||
!iteratorStepResult?.hasProcessedAllItems &&
|
||||
!executedStepOutput.shouldFailSafely &&
|
||||
!executedStepOutput.shouldSkipStepExecution
|
||||
) {
|
||||
const nextStepIdsToExecute = isString(
|
||||
executedStep.settings.input.initialLoopStepIds,
|
||||
)
|
||||
? JSON.parse(executedStep.settings.input.initialLoopStepIds)
|
||||
: executedStep.settings.input.initialLoopStepIds;
|
||||
|
||||
return { nextStepIdsToExecute };
|
||||
if (result) {
|
||||
return result;
|
||||
}
|
||||
}
|
||||
|
||||
if (isWorkflowIfElseAction(executedStep)) {
|
||||
const ifElseResult = executedStepOutput.result as
|
||||
| WorkflowIfElseResult
|
||||
| undefined;
|
||||
|
||||
const branches = executedStep.settings.input.branches;
|
||||
|
||||
if (ifElseResult?.matchingBranchId) {
|
||||
const matchingBranch = branches.find(
|
||||
(branch) => branch.id === ifElseResult.matchingBranchId,
|
||||
);
|
||||
|
||||
const nonMatchingBranches = branches.filter(
|
||||
(branch) => branch.id !== ifElseResult.matchingBranchId,
|
||||
);
|
||||
|
||||
return {
|
||||
nextStepIdsToExecute: matchingBranch?.nextStepIds,
|
||||
nextStepIdsToSkip: nonMatchingBranches.flatMap(
|
||||
(branch) => branch.nextStepIds,
|
||||
),
|
||||
};
|
||||
} else if (executedStepOutput.shouldFailSafely) {
|
||||
return {
|
||||
nextStepIdsToFailSafely: branches.flatMap(
|
||||
(branch) => branch.nextStepIds,
|
||||
),
|
||||
};
|
||||
} else {
|
||||
return {
|
||||
nextStepIdsToSkip: branches.flatMap((branch) => branch.nextStepIds),
|
||||
};
|
||||
}
|
||||
return getNextStepIdsForIfElse({
|
||||
executedStep,
|
||||
executedStepOutput,
|
||||
});
|
||||
}
|
||||
|
||||
return { nextStepIdsToExecute: executedStep.nextStepIds };
|
||||
@@ -563,19 +523,69 @@ export class WorkflowExecutorWorkspaceService {
|
||||
workspaceId,
|
||||
});
|
||||
|
||||
const nextStepIds = new Set<string>();
|
||||
const nextStepIdsToExecute = new Set<string>();
|
||||
const cascadedStepIdsToSkip: string[] = [];
|
||||
const cascadedStepIdsToFailSafely: string[] = [];
|
||||
|
||||
for (const stepId of [...stepIdsToSkip, ...stepIdsToFailSafely]) {
|
||||
const step = steps.find((step) => step.id === stepId);
|
||||
for (const stepId of stepIdsToSkip) {
|
||||
const step = steps.find((candidate) => candidate.id === stepId);
|
||||
|
||||
for (const nextStepId of step?.nextStepIds ?? []) {
|
||||
nextStepIds.add(nextStepId);
|
||||
if (!step) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const result = await this.getNextStepIdsToExecute({
|
||||
executedStep: step,
|
||||
executedStepOutput: { shouldSkipStepExecution: true },
|
||||
});
|
||||
|
||||
for (const id of result.nextStepIdsToExecute ?? []) {
|
||||
nextStepIdsToExecute.add(id);
|
||||
}
|
||||
cascadedStepIdsToSkip.push(...(result.nextStepIdsToSkip ?? []));
|
||||
cascadedStepIdsToFailSafely.push(
|
||||
...(result.nextStepIdsToFailSafely ?? []),
|
||||
);
|
||||
}
|
||||
|
||||
if (nextStepIds.size > 0) {
|
||||
for (const stepId of stepIdsToFailSafely) {
|
||||
const step = steps.find((candidate) => candidate.id === stepId);
|
||||
|
||||
if (!step) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const result = await this.getNextStepIdsToExecute({
|
||||
executedStep: step,
|
||||
executedStepOutput: { shouldFailSafely: true },
|
||||
});
|
||||
|
||||
for (const id of result.nextStepIdsToExecute ?? []) {
|
||||
nextStepIdsToExecute.add(id);
|
||||
}
|
||||
cascadedStepIdsToSkip.push(...(result.nextStepIdsToSkip ?? []));
|
||||
cascadedStepIdsToFailSafely.push(
|
||||
...(result.nextStepIdsToFailSafely ?? []),
|
||||
);
|
||||
}
|
||||
|
||||
if (
|
||||
cascadedStepIdsToSkip.length > 0 ||
|
||||
cascadedStepIdsToFailSafely.length > 0
|
||||
) {
|
||||
await this.skipAndFailSafelyStepsThenContinue({
|
||||
stepIdsToSkip: cascadedStepIdsToSkip,
|
||||
stepIdsToFailSafely: cascadedStepIdsToFailSafely,
|
||||
steps,
|
||||
workflowRunId,
|
||||
workspaceId,
|
||||
executedStepsCount,
|
||||
});
|
||||
}
|
||||
|
||||
if (nextStepIdsToExecute.size > 0) {
|
||||
await this.executeFromSteps({
|
||||
stepIds: Array.from(nextStepIds),
|
||||
stepIds: Array.from(nextStepIdsToExecute),
|
||||
workflowRunId,
|
||||
workspaceId,
|
||||
shouldComputeWorkflowRunStatus: false,
|
||||
|
||||
Reference in New Issue
Block a user