houseassassin/twenty
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Prevent deletion of il-else branches (#18294)

Browse Source 
If-else branches cannot be recreated once deleted. Only else-if branches
can. On if-else branches removal, we now remplace the node by an empty
node instead of only deleting

Also fixing nested if-else.
This commit is contained in:
Thomas Trompette
2026-03-02 13:52:32 +01:00
committed by GitHub
parent ff3326a53b
commit 78a0197643
3 changed files with 421 additions and 98 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/twenty-server/src/modules/workflow/workflow-builder/workflow-version-step/utils/__tests__/remove-step.spec.ts
+189
View File
@@ -165,6 +165,195 @@ describe('removeStep', () => {
expect(result.updatedSteps).toEqual(null);
});
it('should replace with EMPTY node when deleting a leaf step on an If branch', () => {
const ifElseStep = {
id: '2',
name: 'If/Else',
type: WorkflowActionType.IF_ELSE,
settings: {
input: {
stepFilterGroups: [],
stepFilters: [],
branches: [
{
id: 'branch-if',
filterGroupId: 'fg-1',
nextStepIds: ['3'],
},
{
id: 'branch-else',
nextStepIds: ['4'],
},
],
},
outputSchema: {},
errorHandlingOptions: {
retryOnFailure: { value: false },
continueOnFailure: { value: false },
},
},
valid: true,
nextStepIds: [],
} as WorkflowAction;
const step1 = createMockAction('1', ['2']);
const step3 = createMockAction('3');
const step4 = createMockAction('4');
const result = removeStep({
existingTrigger: mockTrigger,
existingSteps: [step1, ifElseStep, step3, step4],
stepIdToDelete: '3',
stepToDeleteChildrenIds: [],
});
expect(result.updatedSteps).toHaveLength(4);
const updatedIfElse = result.updatedSteps?.find(
(step) => step.id === '2',
) as WorkflowAction & { type: WorkflowActionType.IF_ELSE };
const ifBranch = updatedIfElse.settings.input.branches[0];
expect(ifBranch.nextStepIds).toHaveLength(1);
const replacementNodeId = ifBranch.nextStepIds[0];
const replacementNode = result.updatedSteps?.find(
(step) => step.id === replacementNodeId,
);
expect(replacementNode).toBeDefined();
expect(replacementNode?.type).toBe(WorkflowActionType.EMPTY);
expect(replacementNode?.name).toBe('Add an Action');
const elseBranch = updatedIfElse.settings.input.branches[1];
expect(elseBranch.nextStepIds).toEqual(['4']);
});
it('should replace with EMPTY node when deleting a leaf step on an Else branch', () => {
const ifElseStep = {
id: '2',
name: 'If/Else',
type: WorkflowActionType.IF_ELSE,
settings: {
input: {
stepFilterGroups: [],
stepFilters: [],
branches: [
{
id: 'branch-if',
filterGroupId: 'fg-1',
nextStepIds: ['3'],
},
{
id: 'branch-else',
nextStepIds: ['4'],
},
],
},
outputSchema: {},
errorHandlingOptions: {
retryOnFailure: { value: false },
continueOnFailure: { value: false },
},
},
valid: true,
nextStepIds: [],
} as WorkflowAction;
const step1 = createMockAction('1', ['2']);
const step3 = createMockAction('3');
const step4 = createMockAction('4');
const result = removeStep({
existingTrigger: mockTrigger,
existingSteps: [step1, ifElseStep, step3, step4],
stepIdToDelete: '4',
stepToDeleteChildrenIds: [],
});
expect(result.updatedSteps).toHaveLength(4);
const updatedIfElse = result.updatedSteps?.find(
(step) => step.id === '2',
) as WorkflowAction & { type: WorkflowActionType.IF_ELSE };
const elseBranch = updatedIfElse.settings.input.branches[1];
expect(elseBranch.nextStepIds).toHaveLength(1);
const replacementNodeId = elseBranch.nextStepIds[0];
const replacementNode = result.updatedSteps?.find(
(step) => step.id === replacementNodeId,
);
expect(replacementNode).toBeDefined();
expect(replacementNode?.type).toBe(WorkflowActionType.EMPTY);
const ifBranch = updatedIfElse.settings.input.branches[0];
expect(ifBranch.nextStepIds).toEqual(['3']);
});
it('should not replace with EMPTY node when deleted step has children that get re-linked', () => {
const ifElseStep = {
id: '2',
name: 'If/Else',
type: WorkflowActionType.IF_ELSE,
settings: {
input: {
stepFilterGroups: [],
stepFilters: [],
branches: [
{
id: 'branch-if',
filterGroupId: 'fg-1',
nextStepIds: ['3'],
},
{
id: 'branch-else',
nextStepIds: ['5'],
},
],
},
outputSchema: {},
errorHandlingOptions: {
retryOnFailure: { value: false },
continueOnFailure: { value: false },
},
},
valid: true,
nextStepIds: [],
} as WorkflowAction;
const step1 = createMockAction('1', ['2']);
const step3 = createMockAction('3', ['4']);
const step4 = createMockAction('4');
const step5 = createMockAction('5');
const result = removeStep({
existingTrigger: mockTrigger,
existingSteps: [step1, ifElseStep, step3, step4, step5],
stepIdToDelete: '3',
stepToDeleteChildrenIds: ['4'],
});
const updatedIfElse = result.updatedSteps?.find(
(step) => step.id === '2',
) as WorkflowAction & { type: WorkflowActionType.IF_ELSE };
const ifBranch = updatedIfElse.settings.input.branches[0];
expect(ifBranch.nextStepIds).toEqual(['4']);
const hasEmptyNode = result.updatedSteps?.some(
(step) => step.type === WorkflowActionType.EMPTY,
);
expect(hasEmptyNode).toBe(false);
});
it('should handle removing a step that is part of iteratorLoopStepIds', () => {
const step1 = createMockAction('1', ['2']);
const iteratorStep = {
packages/twenty-server/src/modules/workflow/workflow-builder/workflow-version-step/utils/remove-step.ts
+201 -80
View File
@@ -1,11 +1,17 @@
import { isDefined } from 'twenty-shared/utils';
import { TRIGGER_STEP_ID, type StepIfElseBranch } from 'twenty-shared/workflow';
import {
IF_ELSE_BRANCH_POSITION_OFFSETS,
TRIGGER_STEP_ID,
type StepIfElseBranch,
} from 'twenty-shared/workflow';
import { v4 } from 'uuid';
import { isWorkflowEmptyAction } from 'src/modules/workflow/workflow-executor/workflow-actions/empty/guards/is-workflow-empty-action.guard';
import { isWorkflowIfElseAction } from 'src/modules/workflow/workflow-executor/workflow-actions/if-else/guards/is-workflow-if-else-action.guard';
import {
WorkflowActionType,
type WorkflowAction,
type WorkflowIteratorAction,
} from 'src/modules/workflow/workflow-executor/workflow-actions/types/workflow-action.type';
import { type WorkflowTrigger } from 'src/modules/workflow/workflow-trigger/types/workflow-trigger.type';
@@ -47,6 +53,179 @@ export const getEmptyChildStepIdsForIfElse = ({
});
};
const createReplacementEmptyNode = ({
ifElseStep,
isIfBranch,
deletedStepPosition,
}: {
ifElseStep: WorkflowAction;
isIfBranch: boolean;
deletedStepPosition?: { x: number; y: number };
}): WorkflowAction => {
const offset = isIfBranch
? IF_ELSE_BRANCH_POSITION_OFFSETS.IF
: IF_ELSE_BRANCH_POSITION_OFFSETS.ELSE;
return {
id: v4(),
name: 'Add an Action',
type: WorkflowActionType.EMPTY,
valid: true,
settings: {
outputSchema: {},
errorHandlingOptions: {
continueOnFailure: { value: false },
retryOnFailure: { value: false },
},
input: {},
},
position: deletedStepPosition ?? {
x: (ifElseStep.position?.x ?? 0) + offset.x,
y: (ifElseStep.position?.y ?? 0) + offset.y,
},
};
};
const updateIfElseStepOnDeletion = ({
step,
stepIdToDelete,
stepToDeleteChildrenIds,
allRemovedStepIds,
replacementEmptyNodes,
deletedStepPosition,
}: {
step: WorkflowAction;
stepIdToDelete: string;
stepToDeleteChildrenIds?: string[];
allRemovedStepIds: string[];
replacementEmptyNodes: WorkflowAction[];
deletedStepPosition?: { x: number; y: number };
}): WorkflowAction => {
if (!isWorkflowIfElseAction(step)) {
return step;
}
const totalBranches = step.settings.input.branches.length;
const updatedBranches = step.settings.input.branches.map(
(branch, branchIndex) => {
let updatedNextStepIds = branch.nextStepIds;
if (branch.nextStepIds.includes(stepIdToDelete)) {
updatedNextStepIds = computeUpdatedNextStepIds({
existingNextStepIds: branch.nextStepIds,
stepIdToRemove: stepIdToDelete,
stepToDeleteChildrenIds,
});
}
const finalNextStepIds = updatedNextStepIds.filter(
(id) => !allRemovedStepIds.includes(id),
);
const isIfBranch = branchIndex === 0;
const isElseBranch =
branchIndex === totalBranches - 1 && !isDefined(branch.filterGroupId);
if ((isIfBranch || isElseBranch) && finalNextStepIds.length === 0) {
const emptyNode = createReplacementEmptyNode({
ifElseStep: step,
isIfBranch,
deletedStepPosition,
});
replacementEmptyNodes.push(emptyNode);
return {
...branch,
nextStepIds: [emptyNode.id],
};
}
return {
...branch,
nextStepIds: finalNextStepIds,
};
},
);
const filteredBranches = updatedBranches.filter((branch, branchIndex) => {
const isIfBranch = branchIndex === 0;
const isElseBranch =
branchIndex === updatedBranches.length - 1 &&
!isDefined(branch.filterGroupId);
const isElseIfBranch =
branchIndex > 0 &&
branchIndex < updatedBranches.length - 1 &&
isDefined(branch.filterGroupId);
if (isIfBranch || isElseBranch) {
return true;
}
if (isElseIfBranch && branch.nextStepIds.length === 0) {
return false;
}
return true;
});
return {
...step,
settings: {
...step.settings,
input: {
...step.settings.input,
branches: filteredBranches,
},
},
};
};
const updateNextStepIdsOnDeletion = ({
step,
stepIdToDelete,
stepToDeleteChildrenIds,
}: {
step: WorkflowAction;
stepIdToDelete: string;
stepToDeleteChildrenIds?: string[];
}): WorkflowAction => {
return {
...step,
nextStepIds: computeUpdatedNextStepIds({
existingNextStepIds: step.nextStepIds ?? [],
stepIdToRemove: stepIdToDelete,
stepToDeleteChildrenIds,
}),
};
};
const updateIteratorStepOnDeletion = ({
step,
stepIdToDelete,
stepToDeleteChildrenIds,
}: {
step: WorkflowIteratorAction;
stepIdToDelete: string;
stepToDeleteChildrenIds?: string[];
}): WorkflowAction => {
return {
...step,
settings: {
...step.settings,
input: {
...step.settings.input,
initialLoopStepIds: computeUpdatedNextStepIds({
existingNextStepIds: step.settings.input.initialLoopStepIds ?? [],
stepIdToRemove: stepIdToDelete,
stepToDeleteChildrenIds,
}),
},
},
};
};
export const removeStep = ({
existingTrigger,
existingSteps,
@@ -84,80 +263,29 @@ export const removeStep = ({
}
const allRemovedStepIds = [stepIdToDelete, ...emptyChildStepIds];
const replacementEmptyNodes: WorkflowAction[] = [];
const updatedSteps =
existingSteps
?.filter((step) => !allRemovedStepIds.includes(step.id))
.map((step) => {
if (
step.type === WorkflowActionType.IF_ELSE &&
isWorkflowIfElseAction(step)
) {
const updatedBranches = step.settings.input.branches.map((branch) => {
let updatedNextStepIds = branch.nextStepIds;
if (branch.nextStepIds.includes(stepIdToDelete)) {
updatedNextStepIds = computeUpdatedNextStepIds({
existingNextStepIds: branch.nextStepIds,
stepIdToRemove: stepIdToDelete,
stepToDeleteChildrenIds,
});
}
const finalNextStepIds = updatedNextStepIds.filter(
(id) => !allRemovedStepIds.includes(id),
);
return {
...branch,
nextStepIds: finalNextStepIds,
};
if (step.type === WorkflowActionType.IF_ELSE) {
return updateIfElseStepOnDeletion({
step,
stepIdToDelete,
stepToDeleteChildrenIds,
allRemovedStepIds,
replacementEmptyNodes,
deletedStepPosition: stepToDelete?.position,
});
const filteredBranches = updatedBranches.filter(
(branch, branchIndex) => {
const isIfBranch = branchIndex === 0;
const isElseBranch =
branchIndex === updatedBranches.length - 1 &&
!isDefined(branch.filterGroupId);
const isElseIfBranch =
branchIndex > 0 &&
branchIndex < updatedBranches.length - 1 &&
isDefined(branch.filterGroupId);
if (isIfBranch || isElseBranch) {
return true;
}
if (isElseIfBranch && branch.nextStepIds.length === 0) {
return false;
}
return true;
},
);
return {
...step,
settings: {
...step.settings,
input: {
...step.settings.input,
branches: filteredBranches,
},
},
};
}
if (step.nextStepIds?.includes(stepIdToDelete)) {
return {
...step,
nextStepIds: computeUpdatedNextStepIds({
existingNextStepIds: step.nextStepIds,
stepIdToRemove: stepIdToDelete,
stepToDeleteChildrenIds,
}),
};
return updateNextStepIdsOnDeletion({
step,
stepIdToDelete,
stepToDeleteChildrenIds,
});
}
if (
@@ -165,25 +293,18 @@ export const removeStep = ({
isDefined(step.settings.input.initialLoopStepIds) &&
step.settings.input.initialLoopStepIds.includes(stepIdToDelete)
) {
return {
...step,
settings: {
...step.settings,
input: {
...step.settings.input,
initialLoopStepIds: computeUpdatedNextStepIds({
existingNextStepIds: step.settings.input.initialLoopStepIds,
stepIdToRemove: stepIdToDelete,
stepToDeleteChildrenIds,
}),
},
},
};
return updateIteratorStepOnDeletion({
step,
stepIdToDelete,
stepToDeleteChildrenIds,
});
}
return step;
}) ?? [];
updatedSteps.push(...replacementEmptyNodes);
let updatedTrigger = existingTrigger;
if (isDefined(existingTrigger)) {
packages/twenty-server/src/modules/workflow/workflow-builder/workflow-version-step/workflow-version-step-update.workspace-service.ts
+31 -18
View File
@@ -57,18 +57,21 @@ export class WorkflowVersionStepUpdateWorkspaceService {
const isStepTypeChanged = existingStep.type !== step.type;
const updatedStep = isStepTypeChanged
const { updatedStep, additionalCreatedSteps } = isStepTypeChanged
? await this.updateWorkflowVersionStepType({
existingStep,
newStep: step,
workspaceId,
workflowVersionId,
})
: await this.updateWorkflowVersionStepSettings({
newStep: step,
workspaceId,
workflowVersionId,
});
: {
updatedStep: await this.updateWorkflowVersionStepSettings({
newStep: step,
workspaceId,
workflowVersionId,
}),
additionalCreatedSteps: undefined,
};
const updatedSteps = workflowVersion.steps.map((existingStep) => {
if (existingStep.id === step.id) {
@@ -78,6 +81,10 @@ export class WorkflowVersionStepUpdateWorkspaceService {
}
});
if (isDefined(additionalCreatedSteps)) {
updatedSteps.push(...additionalCreatedSteps);
}
await this.workflowVersionStepHelpersWorkspaceService.updateWorkflowVersionStepsAndTrigger(
{
workspaceId,
@@ -99,7 +106,10 @@ export class WorkflowVersionStepUpdateWorkspaceService {
newStep: WorkflowAction;
workspaceId: string;
workflowVersionId: string;
}): Promise<WorkflowAction> {
}): Promise<{
updatedStep: WorkflowAction;
additionalCreatedSteps?: WorkflowAction[];
}> {
await this.workflowVersionStepOperationsWorkspaceService.runWorkflowVersionStepDeletionSideEffects(
{
step: existingStep,
@@ -107,7 +117,7 @@ export class WorkflowVersionStepUpdateWorkspaceService {
},
);
const { builtStep } =
const { builtStep, additionalCreatedSteps } =
await this.workflowVersionStepOperationsWorkspaceService.runStepCreationSideEffectsAndBuildStep(
{
type: newStep.type,
@@ -118,16 +128,19 @@ export class WorkflowVersionStepUpdateWorkspaceService {
},
);
return this.workflowSchemaWorkspaceService.enrichOutputSchema({
step: {
...builtStep,
id: existingStep.id,
nextStepIds: existingStep.nextStepIds,
position: existingStep.position,
},
workspaceId,
workflowVersionId,
});
const updatedStep =
await this.workflowSchemaWorkspaceService.enrichOutputSchema({
step: {
...builtStep,
id: existingStep.id,
nextStepIds: existingStep.nextStepIds,
position: existingStep.position,
},
workspaceId,
workflowVersionId,
});
return { updatedStep, additionalCreatedSteps };
}
private async updateWorkflowVersionStepSettings({