diff --git a/packages/twenty-server/src/modules/workflow/workflow-builder/workflow-version-step/utils/__tests__/remove-step.spec.ts b/packages/twenty-server/src/modules/workflow/workflow-builder/workflow-version-step/utils/__tests__/remove-step.spec.ts index 7bce655a55..faffdb5c17 100644 --- a/packages/twenty-server/src/modules/workflow/workflow-builder/workflow-version-step/utils/__tests__/remove-step.spec.ts +++ b/packages/twenty-server/src/modules/workflow/workflow-builder/workflow-version-step/utils/__tests__/remove-step.spec.ts @@ -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 = { diff --git a/packages/twenty-server/src/modules/workflow/workflow-builder/workflow-version-step/utils/remove-step.ts b/packages/twenty-server/src/modules/workflow/workflow-builder/workflow-version-step/utils/remove-step.ts index 4cdb509db6..b3d1c9ba76 100644 --- a/packages/twenty-server/src/modules/workflow/workflow-builder/workflow-version-step/utils/remove-step.ts +++ b/packages/twenty-server/src/modules/workflow/workflow-builder/workflow-version-step/utils/remove-step.ts @@ -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)) { diff --git a/packages/twenty-server/src/modules/workflow/workflow-builder/workflow-version-step/workflow-version-step-update.workspace-service.ts b/packages/twenty-server/src/modules/workflow/workflow-builder/workflow-version-step/workflow-version-step-update.workspace-service.ts index 160ca8a51a..12dee29501 100644 --- a/packages/twenty-server/src/modules/workflow/workflow-builder/workflow-version-step/workflow-version-step-update.workspace-service.ts +++ b/packages/twenty-server/src/modules/workflow/workflow-builder/workflow-version-step/workflow-version-step-update.workspace-service.ts @@ -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 { + }): 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({