Handle edge deletion from loop (#14489)

Update delete edge endpoint. It should handles the deletion between the
iterator and the loop step ids
This commit is contained in:
Thomas Trompette
2025-09-15 10:20:33 +02:00
committed by GitHub
parent 4d2c3b7431
commit 155aa4995e
6 changed files with 285 additions and 21 deletions
@@ -56,13 +56,19 @@ export class WorkflowVersionEdgeResolver {
async deleteWorkflowVersionEdge(
@AuthWorkspace() { id: workspaceId }: Workspace,
@Args('input')
{ source, target, workflowVersionId }: CreateWorkflowVersionEdgeInput,
{
source,
target,
workflowVersionId,
sourceConnectionOptions,
}: CreateWorkflowVersionEdgeInput,
): Promise<WorkflowVersionStepChangesDTO> {
return this.workflowVersionEdgeWorkspaceService.deleteWorkflowVersionEdge({
source,
target,
workflowVersionId,
workspaceId,
sourceConnectionOptions,
});
}
}
@@ -242,7 +242,7 @@ describe('WorkflowVersionEdgeWorkspaceService', () => {
);
});
it('should add target to initialLoopStepIds when shouldInsertToLoop is true', async () => {
it('should add target to initialLoopStepIds when isConnectedToLoop is true', async () => {
const result = await service.createWorkflowVersionEdge({
source: 'iterator-step',
target: 'step-3',
@@ -251,7 +251,7 @@ describe('WorkflowVersionEdgeWorkspaceService', () => {
sourceConnectionOptions: {
connectedStepType: WorkflowActionType.ITERATOR,
settings: {
shouldInsertToLoop: true,
isConnectedToLoop: true,
},
},
});
@@ -291,7 +291,7 @@ describe('WorkflowVersionEdgeWorkspaceService', () => {
sourceConnectionOptions: {
connectedStepType: WorkflowActionType.ITERATOR,
settings: {
shouldInsertToLoop: true,
isConnectedToLoop: true,
},
},
});
@@ -321,7 +321,7 @@ describe('WorkflowVersionEdgeWorkspaceService', () => {
sourceConnectionOptions: {
connectedStepType: WorkflowActionType.ITERATOR,
settings: {
shouldInsertToLoop: true,
isConnectedToLoop: true,
},
},
});
@@ -331,7 +331,7 @@ describe('WorkflowVersionEdgeWorkspaceService', () => {
);
});
it('should create normal edge when shouldInsertToLoop is false', async () => {
it('should create normal edge when isConnectedToLoop is false', async () => {
const result = await service.createWorkflowVersionEdge({
source: 'iterator-step',
target: 'step-3',
@@ -340,7 +340,7 @@ describe('WorkflowVersionEdgeWorkspaceService', () => {
sourceConnectionOptions: {
connectedStepType: WorkflowActionType.ITERATOR,
settings: {
shouldInsertToLoop: false,
isConnectedToLoop: false,
},
},
});
@@ -590,6 +590,163 @@ describe('WorkflowVersionEdgeWorkspaceService', () => {
});
});
describe('with sourceConnectionOptions', () => {
describe('with iterator step', () => {
const mockIteratorStep = {
id: 'iterator-step',
type: WorkflowActionType.ITERATOR,
settings: {
errorHandlingOptions: {
continueOnFailure: { value: false },
retryOnFailure: { value: false },
},
input: {
initialLoopStepIds: ['step-2', 'step-3'],
},
},
nextStepIds: ['step-1'],
} as WorkflowAction;
const mockStepsWithIterator = [mockIteratorStep, ...mockSteps];
const mockWorkflowVersionWithIterator = {
...mockWorkflowVersion,
steps: mockStepsWithIterator,
};
beforeEach(() => {
workflowCommonWorkspaceService.getWorkflowVersionOrFail.mockResolvedValue(
mockWorkflowVersionWithIterator,
);
});
it('should throw if source step is not an iterator when connectedStepType is ITERATOR', async () => {
const call = async () =>
await service.deleteWorkflowVersionEdge({
source: 'step-1',
target: 'step-2',
workflowVersionId: mockWorkflowVersionId,
workspaceId: mockWorkspaceId,
sourceConnectionOptions: {
connectedStepType: WorkflowActionType.ITERATOR,
settings: {
isConnectedToLoop: true,
},
},
});
await expect(call).rejects.toThrow(
`Source step 'step-1' is not an iterator`,
);
});
it('should remove target from initialLoopStepIds when isConnectedToLoop is true', async () => {
const result = await service.deleteWorkflowVersionEdge({
source: 'iterator-step',
target: 'step-2',
workflowVersionId: mockWorkflowVersionId,
workspaceId: mockWorkspaceId,
sourceConnectionOptions: {
connectedStepType: WorkflowActionType.ITERATOR,
settings: {
isConnectedToLoop: true,
},
},
});
expect(
mockWorkflowVersionWorkspaceRepository.update,
).toHaveBeenCalledWith(mockWorkflowVersionId, {
steps: expect.arrayContaining([
expect.objectContaining({
id: 'iterator-step',
settings: expect.objectContaining({
input: expect.objectContaining({
initialLoopStepIds: ['step-3'],
}),
}),
}),
]),
});
expect(result).toEqual({
triggerNextStepIds: ['step-1'],
stepsNextStepIds: {
'iterator-step': ['step-1'],
'step-1': ['step-2'],
'step-2': [],
'step-3': [],
},
});
});
it('should not update if target is not in initialLoopStepIds when isConnectedToLoop is true', async () => {
const result = await service.deleteWorkflowVersionEdge({
source: 'iterator-step',
target: 'step-1',
workflowVersionId: mockWorkflowVersionId,
workspaceId: mockWorkspaceId,
sourceConnectionOptions: {
connectedStepType: WorkflowActionType.ITERATOR,
settings: {
isConnectedToLoop: true,
},
},
});
expect(
mockWorkflowVersionWorkspaceRepository.update,
).not.toHaveBeenCalled();
expect(result).toEqual({
triggerNextStepIds: ['step-1'],
stepsNextStepIds: {
'iterator-step': ['step-1'],
'step-1': ['step-2'],
'step-2': [],
'step-3': [],
},
});
});
it('should remove target from nextStepIds when isConnectedToLoop is false', async () => {
const result = await service.deleteWorkflowVersionEdge({
source: 'iterator-step',
target: 'step-1',
workflowVersionId: mockWorkflowVersionId,
workspaceId: mockWorkspaceId,
sourceConnectionOptions: {
connectedStepType: WorkflowActionType.ITERATOR,
settings: {
isConnectedToLoop: false,
},
},
});
expect(
mockWorkflowVersionWorkspaceRepository.update,
).toHaveBeenCalledWith(mockWorkflowVersionId, {
steps: expect.arrayContaining([
expect.objectContaining({
id: 'iterator-step',
nextStepIds: [],
}),
]),
});
expect(result).toEqual({
triggerNextStepIds: ['step-1'],
stepsNextStepIds: {
'iterator-step': [],
'step-1': ['step-2'],
'step-2': [],
'step-3': [],
},
});
});
});
});
describe('with filter steps', () => {
it('should delete the filter step when deleting edge from trigger to target through filter', async () => {
const mockStepsWithFilter = [
@@ -96,11 +96,13 @@ export class WorkflowVersionEdgeWorkspaceService {
target,
workflowVersionId,
workspaceId,
sourceConnectionOptions,
}: {
source: string;
target: string;
workflowVersionId: string;
workspaceId: string;
sourceConnectionOptions?: WorkflowStepConnectionOptions;
}): Promise<WorkflowVersionStepChangesDTO> {
const workflowVersionRepository =
await this.twentyORMGlobalManager.getRepositoryForWorkspace<WorkflowVersionWorkspaceEntity>(
@@ -147,6 +149,7 @@ export class WorkflowVersionEdgeWorkspaceService {
target,
workflowVersion,
workflowVersionRepository,
sourceConnectionOptions,
});
}
}
@@ -270,7 +273,7 @@ export class WorkflowVersionEdgeWorkspaceService {
);
}
if (sourceConnectionOptions.settings.shouldInsertToLoop) {
if (sourceConnectionOptions.settings.isConnectedToLoop) {
const currentInitialLoopStepIds =
sourceStep.settings.input.initialLoopStepIds;
@@ -394,6 +397,7 @@ export class WorkflowVersionEdgeWorkspaceService {
target,
workflowVersion,
workflowVersionRepository,
sourceConnectionOptions,
}: {
trigger: WorkflowTrigger | null;
steps: WorkflowAction[];
@@ -401,6 +405,7 @@ export class WorkflowVersionEdgeWorkspaceService {
target: string;
workflowVersion: WorkflowVersionWorkspaceEntity;
workflowVersionRepository: WorkspaceRepository<WorkflowVersionWorkspaceEntity>;
sourceConnectionOptions?: WorkflowStepConnectionOptions;
}): Promise<WorkflowVersionStepChangesDTO> {
const sourceStep = steps.find((step) => step.id === source);
@@ -411,7 +416,17 @@ export class WorkflowVersionEdgeWorkspaceService {
);
}
if (!sourceStep.nextStepIds?.includes(target)) {
// TODO: Remove this once we start using filters as regular steps
const isIteratorWithLoopTarget =
isDefined(sourceConnectionOptions) &&
sourceConnectionOptions.connectedStepType ===
WorkflowActionType.ITERATOR &&
sourceConnectionOptions.settings.isConnectedToLoop;
if (
!sourceStep.nextStepIds?.includes(target) &&
!isIteratorWithLoopTarget
) {
return await this.handleFilterBetweenSourceAndTarget({
trigger,
steps,
@@ -422,12 +437,30 @@ export class WorkflowVersionEdgeWorkspaceService {
});
}
const updatedSourceStep = {
...sourceStep,
nextStepIds: sourceStep.nextStepIds?.filter(
(nextStepId: string) => nextStepId !== target,
),
};
const { updatedSourceStep, shouldPersist } = isDefined(
sourceConnectionOptions,
)
? this.buildUpdatedSourceStepWithOptions({
sourceStep,
target,
sourceConnectionOptions,
})
: {
updatedSourceStep: {
...sourceStep,
nextStepIds: sourceStep.nextStepIds?.filter(
(nextStepId: string) => nextStepId !== target,
),
},
shouldPersist: true,
};
if (!shouldPersist) {
return computeWorkflowVersionStepChanges({
trigger,
steps,
});
}
const updatedSteps = steps.map((step) => {
if (step.id === source) {
@@ -550,6 +583,74 @@ export class WorkflowVersionEdgeWorkspaceService {
});
}
private buildUpdatedSourceStepWithOptions({
sourceStep,
target,
sourceConnectionOptions,
}: {
sourceStep: WorkflowAction;
target: string;
sourceConnectionOptions: WorkflowStepConnectionOptions;
}): {
updatedSourceStep: WorkflowAction;
shouldPersist: boolean;
} {
switch (sourceConnectionOptions.connectedStepType) {
case WorkflowActionType.ITERATOR:
if (sourceStep.type !== WorkflowActionType.ITERATOR) {
throw new WorkflowVersionEdgeException(
`Source step '${sourceStep.id}' is not an iterator`,
WorkflowVersionEdgeExceptionCode.INVALID_REQUEST,
);
}
if (sourceConnectionOptions.settings.isConnectedToLoop) {
const currentInitialLoopStepIds =
sourceStep.settings.input.initialLoopStepIds;
if (!currentInitialLoopStepIds?.includes(target)) {
return {
updatedSourceStep: sourceStep,
shouldPersist: false,
};
}
return {
updatedSourceStep: {
...sourceStep,
settings: {
...sourceStep.settings,
input: {
...sourceStep.settings.input,
initialLoopStepIds: currentInitialLoopStepIds.filter(
(id) => id !== target,
),
},
},
},
shouldPersist: true,
};
}
return {
updatedSourceStep: {
...sourceStep,
nextStepIds: sourceStep.nextStepIds?.filter((id) => id !== target),
},
shouldPersist: true,
};
default:
return {
updatedSourceStep: {
...sourceStep,
nextStepIds: sourceStep.nextStepIds?.filter((id) => id !== target),
},
shouldPersist: true,
};
}
}
private findFilterBetweenNodes({
steps,
sourceNextStepIds,
@@ -3,7 +3,7 @@ import { type WorkflowActionType } from 'src/modules/workflow/workflow-executor/
type WorkflowIteratorStepConnectionOptions = {
connectedStepType: WorkflowActionType.ITERATOR;
settings: {
shouldInsertToLoop: boolean;
isConnectedToLoop: boolean;
};
};
@@ -178,7 +178,7 @@ describe('insertStep', () => {
});
});
it('should add step to iterator initialLoopStepIds when shouldInsertToLoop is true', () => {
it('should add step to iterator initialLoopStepIds when isConnectedToLoop is true', () => {
const existingTrigger = createMockTrigger(['1']);
const newStep = createMockAction('new');
@@ -190,7 +190,7 @@ describe('insertStep', () => {
parentStepConnectionOptions: {
connectedStepType: WorkflowActionType.ITERATOR,
settings: {
shouldInsertToLoop: true,
isConnectedToLoop: true,
},
},
});
@@ -204,7 +204,7 @@ describe('insertStep', () => {
]);
});
it('should not add step to iterator initialLoopStepIds when shouldInsertToLoop is false', () => {
it('should not add step to iterator initialLoopStepIds when isConnectedToLoop is false', () => {
const existingTrigger = createMockTrigger(['1']);
const newStep = createMockAction('new');
@@ -216,7 +216,7 @@ describe('insertStep', () => {
parentStepConnectionOptions: {
connectedStepType: WorkflowActionType.ITERATOR,
settings: {
shouldInsertToLoop: false,
isConnectedToLoop: false,
},
},
});
@@ -173,7 +173,7 @@ const updateStepsWithOptions = ({
switch (parentStepConnectionOptions.connectedStepType) {
case WorkflowActionType.ITERATOR:
if (!parentStepConnectionOptions.settings.shouldInsertToLoop) {
if (!parentStepConnectionOptions.settings.isConnectedToLoop) {
break;
}