Make filters a regular step (#14586)

- Remove special design logic for filters and edge with filters
- Remove backend logic to delete filters along with parent steps
- Add filters to the node type picker



https://github.com/user-attachments/assets/5b6ceda4-d86f-48a3-8c93-37a885398a8d
This commit is contained in:
Thomas Trompette
2025-09-18 13:07:23 +00:00
committed by GitHub
parent 235eef32f6
commit 3fbc257c06
32 changed files with 88 additions and 2184 deletions
@@ -746,195 +746,5 @@ describe('WorkflowVersionEdgeWorkspaceService', () => {
});
});
});
describe('with filter steps', () => {
it('should delete the filter step when deleting edge from trigger to target through filter', async () => {
const mockStepsWithFilter = [
{
id: 'step-1',
type: WorkflowActionType.FORM,
settings: {
errorHandlingOptions: {
continueOnFailure: { value: false },
retryOnFailure: { value: false },
},
},
nextStepIds: ['step-2'],
},
{
id: 'step-2',
type: WorkflowActionType.SEND_EMAIL,
settings: {
errorHandlingOptions: {
continueOnFailure: { value: false },
retryOnFailure: { value: false },
},
},
nextStepIds: [],
},
{
id: 'filter-step',
type: WorkflowActionType.FILTER,
settings: {
errorHandlingOptions: {
continueOnFailure: { value: false },
retryOnFailure: { value: false },
},
},
nextStepIds: ['step-2'],
},
] as WorkflowAction[];
const mockTriggerWithFilter = {
type: WorkflowTriggerType.MANUAL,
settings: {},
nextStepIds: ['step-1', 'filter-step'],
};
const mockWorkflowVersionWithFilter = {
id: mockWorkflowVersionId,
trigger: mockTriggerWithFilter,
steps: mockStepsWithFilter,
status: 'DRAFT',
} as WorkflowVersionWorkspaceEntity;
workflowCommonWorkspaceService.getWorkflowVersionOrFail.mockResolvedValue(
mockWorkflowVersionWithFilter,
);
const result = await service.deleteWorkflowVersionEdge({
source: TRIGGER_STEP_ID,
target: 'step-2',
workflowVersionId: mockWorkflowVersionId,
workspaceId: mockWorkspaceId,
});
expect(
workflowCommonWorkspaceService.getWorkflowVersionOrFail,
).toHaveBeenCalledWith({
workflowVersionId: mockWorkflowVersionId,
workspaceId: mockWorkspaceId,
});
expect(
mockWorkflowVersionWorkspaceRepository.update,
).toHaveBeenCalledWith(mockWorkflowVersionId, {
trigger: {
...mockTriggerWithFilter,
nextStepIds: ['step-1'],
},
steps: mockStepsWithFilter.filter(
(step) => step.id !== 'filter-step',
),
});
expect(result).toEqual({
triggerNextStepIds: ['step-1'],
stepsNextStepIds: {
'step-1': ['step-2'],
'step-2': [],
},
});
});
it('should delete the filter step when deleting edge from step to target through filter', async () => {
const mockStepsWithFilter = [
{
id: 'step-1',
type: WorkflowActionType.FORM,
settings: {
errorHandlingOptions: {
continueOnFailure: { value: false },
retryOnFailure: { value: false },
},
},
nextStepIds: ['step-2', 'filter-step'],
},
{
id: 'step-2',
type: WorkflowActionType.SEND_EMAIL,
settings: {
errorHandlingOptions: {
continueOnFailure: { value: false },
retryOnFailure: { value: false },
},
},
nextStepIds: [],
},
{
id: 'step-3',
type: WorkflowActionType.SEND_EMAIL,
settings: {
errorHandlingOptions: {
continueOnFailure: { value: false },
retryOnFailure: { value: false },
},
},
nextStepIds: [],
},
{
id: 'filter-step',
type: WorkflowActionType.FILTER,
settings: {
errorHandlingOptions: {
continueOnFailure: { value: false },
retryOnFailure: { value: false },
},
},
nextStepIds: ['step-3'],
},
] as WorkflowAction[];
const mockWorkflowVersionWithFilter = {
id: mockWorkflowVersionId,
trigger: mockTrigger,
steps: mockStepsWithFilter,
status: 'DRAFT',
} as WorkflowVersionWorkspaceEntity;
workflowCommonWorkspaceService.getWorkflowVersionOrFail.mockResolvedValue(
mockWorkflowVersionWithFilter,
);
const result = await service.deleteWorkflowVersionEdge({
source: 'step-1',
target: 'step-3',
workflowVersionId: mockWorkflowVersionId,
workspaceId: mockWorkspaceId,
});
expect(
workflowCommonWorkspaceService.getWorkflowVersionOrFail,
).toHaveBeenCalledWith({
workflowVersionId: mockWorkflowVersionId,
workspaceId: mockWorkspaceId,
});
expect(
mockWorkflowVersionWorkspaceRepository.update,
).toHaveBeenCalledWith(mockWorkflowVersionId, {
steps: mockStepsWithFilter
.map((step) => {
if (step.id === 'step-1') {
return {
...step,
nextStepIds: ['step-2'],
};
}
return step;
})
.filter((step) => step.id !== 'filter-step'),
});
expect(result).toEqual({
triggerNextStepIds: ['step-1'],
stepsNextStepIds: {
'step-1': ['step-2'],
'step-2': [],
'step-3': [],
},
});
});
});
});
});
@@ -222,6 +222,16 @@ export class WorkflowVersionEdgeWorkspaceService {
);
}
if (
sourceStep.nextStepIds?.includes(target) &&
!isDefined(sourceConnectionOptions)
) {
return computeWorkflowVersionStepChanges({
trigger,
steps,
});
}
const { updatedSourceStep, shouldPersist } = isDefined(
sourceConnectionOptions,
)
@@ -364,12 +374,9 @@ export class WorkflowVersionEdgeWorkspaceService {
}
if (!trigger.nextStepIds?.includes(target)) {
return this.handleFilterBetweenTriggerAndTarget({
return computeWorkflowVersionStepChanges({
trigger,
steps,
target,
workflowVersionId: workflowVersion.id,
workflowVersionRepository,
});
}
@@ -416,24 +423,13 @@ export class WorkflowVersionEdgeWorkspaceService {
);
}
// 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
!isDefined(sourceConnectionOptions)
) {
return await this.handleFilterBetweenSourceAndTarget({
return computeWorkflowVersionStepChanges({
trigger,
steps,
sourceStep,
target,
workflowVersionId: workflowVersion.id,
workflowVersionRepository,
});
}
@@ -480,109 +476,6 @@ export class WorkflowVersionEdgeWorkspaceService {
});
}
private async handleFilterBetweenTriggerAndTarget({
trigger,
steps,
target,
workflowVersionId,
workflowVersionRepository,
}: {
trigger: WorkflowTrigger;
steps: WorkflowAction[];
target: string;
workflowVersionId: string;
workflowVersionRepository: WorkspaceRepository<WorkflowVersionWorkspaceEntity>;
}): Promise<WorkflowVersionStepChangesDTO> {
const filterBetweenTriggerAndTarget = this.findFilterBetweenNodes({
steps,
sourceNextStepIds: trigger.nextStepIds,
target,
});
if (!isDefined(filterBetweenTriggerAndTarget)) {
return computeWorkflowVersionStepChanges({
trigger,
steps,
});
}
const updatedTrigger = {
...trigger,
nextStepIds: trigger.nextStepIds?.filter(
(nextStepId: string) => nextStepId !== filterBetweenTriggerAndTarget.id,
),
};
const updatedSteps = steps.filter(
(step) => step.id !== filterBetweenTriggerAndTarget.id,
);
await workflowVersionRepository.update(workflowVersionId, {
trigger: updatedTrigger,
steps: updatedSteps,
});
return computeWorkflowVersionStepChanges({
trigger: updatedTrigger,
steps: updatedSteps,
});
}
private async handleFilterBetweenSourceAndTarget({
trigger,
steps,
sourceStep,
target,
workflowVersionRepository,
workflowVersionId,
}: {
trigger: WorkflowTrigger | null;
steps: WorkflowAction[];
sourceStep: WorkflowAction;
target: string;
workflowVersionRepository: WorkspaceRepository<WorkflowVersionWorkspaceEntity>;
workflowVersionId: string;
}): Promise<WorkflowVersionStepChangesDTO> {
const filterBetweenSourceAndTarget = this.findFilterBetweenNodes({
steps,
sourceNextStepIds: sourceStep.nextStepIds,
target,
});
if (!isDefined(filterBetweenSourceAndTarget)) {
return computeWorkflowVersionStepChanges({
trigger,
steps,
});
}
const updatedSourceStep = {
...sourceStep,
nextStepIds: sourceStep.nextStepIds?.filter(
(nextStepId: string) => nextStepId !== filterBetweenSourceAndTarget.id,
),
};
const updatedSteps = steps
.map((step) => {
if (step.id === sourceStep.id) {
return updatedSourceStep;
}
return step;
})
.filter((step) => step.id !== filterBetweenSourceAndTarget.id);
await workflowVersionRepository.update(workflowVersionId, {
steps: updatedSteps,
});
return computeWorkflowVersionStepChanges({
trigger,
steps: updatedSteps,
});
}
private buildUpdatedSourceStepWithOptions({
sourceStep,
target,
@@ -650,22 +543,4 @@ export class WorkflowVersionEdgeWorkspaceService {
};
}
}
private findFilterBetweenNodes({
steps,
sourceNextStepIds,
target,
}: {
steps: WorkflowAction[];
sourceNextStepIds: string[] | undefined;
target: string;
}) {
const nextStepFilters = steps.filter(
(step) =>
sourceNextStepIds?.includes(step.id) &&
step.type === WorkflowActionType.FILTER,
);
return nextStepFilters.find((step) => step.nextStepIds?.includes(target));
}
}
@@ -151,100 +151,6 @@ describe('removeStep', () => {
});
});
it('should remove step child that is a filter', () => {
const step1 = createMockAction('1', ['2']);
const step2 = createMockAction('2', ['3']);
const step3 = {
id: '3',
name: 'Step 3',
type: WorkflowActionType.FILTER,
nextStepIds: ['4'],
} as WorkflowAction;
const step4 = createMockAction('4');
const result = removeStep({
existingTrigger: mockTrigger,
existingSteps: [step1, step2, step3, step4],
stepIdToDelete: '2',
stepToDeleteChildrenIds: ['3'],
});
expect(result.updatedTrigger).toEqual(mockTrigger);
expect(result.updatedSteps).toEqual([
{ ...step1, nextStepIds: ['4'] },
step4,
]);
});
it('should remove trigger children that is a filter', () => {
const step1 = {
id: '1',
name: 'Step 1',
type: WorkflowActionType.FILTER,
nextStepIds: ['2'],
} as WorkflowAction;
const step2 = createMockAction('2', ['3']);
const step3 = createMockAction('3');
const result = removeStep({
existingTrigger: mockTrigger,
existingSteps: [step1, step2, step3],
stepIdToDelete: TRIGGER_STEP_ID,
stepToDeleteChildrenIds: ['1'],
});
expect(result.updatedTrigger).toEqual(null);
expect(result.updatedSteps).toEqual([step2, step3]);
});
it('should remove filter step if it has no children', () => {
const step1 = {
id: '1',
name: 'Step 1',
type: WorkflowActionType.FILTER,
nextStepIds: ['2'],
} as WorkflowAction;
const step2 = createMockAction('2', ['3']);
const step3 = {
id: '3',
name: 'Step 3',
type: WorkflowActionType.FILTER,
nextStepIds: ['4'],
} as WorkflowAction;
const step4 = createMockAction('4');
const result = removeStep({
existingTrigger: mockTrigger,
existingSteps: [step1, step2, step3, step4],
stepIdToDelete: '4',
});
expect(result.updatedTrigger).toEqual(mockTrigger);
expect(result.updatedSteps).toEqual([step1, { ...step2, nextStepIds: [] }]);
});
it('should remove filter step if it is the last step', () => {
const step1 = {
id: '1',
name: 'Step 1',
type: WorkflowActionType.FILTER,
nextStepIds: ['2'],
} as WorkflowAction;
const step2 = createMockAction('2');
const result = removeStep({
existingTrigger: mockTrigger,
existingSteps: [step1, step2],
stepIdToDelete: '2',
});
expect(result.updatedTrigger).toEqual({
...mockTrigger,
nextStepIds: [],
});
expect(result.updatedSteps).toEqual([]);
});
it('should remove trigger if steps are null', () => {
const result = removeStep({
existingTrigger: { ...mockTrigger, nextStepIds: [] },
@@ -253,7 +159,7 @@ describe('removeStep', () => {
});
expect(result.updatedTrigger).toEqual(null);
expect(result.updatedSteps).toEqual([]);
expect(result.updatedSteps).toEqual(null);
});
it('should handle removing a step that is part of iteratorLoopStepIds', () => {
@@ -29,7 +29,7 @@ const computeUpdatedNextStepIds = ({
];
};
const removeOneStep = ({
export const removeStep = ({
existingTrigger,
existingSteps,
stepIdToDelete,
@@ -39,11 +39,15 @@ const removeOneStep = ({
existingSteps: WorkflowAction[] | null;
stepIdToDelete: string;
stepToDeleteChildrenIds?: string[];
}): {
updatedSteps: WorkflowAction[];
updatedTrigger: WorkflowTrigger | null;
removedStepIds: string[];
} => {
}) => {
if (stepIdToDelete === TRIGGER_STEP_ID) {
return {
updatedSteps: existingSteps,
updatedTrigger: null,
removedStepIds: [TRIGGER_STEP_ID],
};
}
const updatedSteps =
existingSteps
?.filter((step) => step.id !== stepIdToDelete)
@@ -104,131 +108,3 @@ const removeOneStep = ({
removedStepIds: [stepIdToDelete],
};
};
const removeRegularStep = ({
existingTrigger,
existingSteps,
stepIdToDelete,
stepToDeleteChildrenIds,
}: {
existingTrigger: WorkflowTrigger | null;
existingSteps: WorkflowAction[] | null;
stepIdToDelete: string;
stepToDeleteChildrenIds?: string[];
}): {
updatedSteps: WorkflowAction[];
updatedTrigger: WorkflowTrigger | null;
removedStepIds: string[];
} => {
let { updatedSteps, updatedTrigger, removedStepIds } = removeOneStep({
existingTrigger,
existingSteps,
stepIdToDelete,
stepToDeleteChildrenIds,
});
for (const stepId of stepToDeleteChildrenIds ?? []) {
const step = existingSteps?.find((step) => step.id === stepId);
if (step?.type === WorkflowActionType.FILTER) {
const {
updatedSteps: stepsAfterRemovingChildFilter,
updatedTrigger: triggerAfterRemovingChildFilter,
removedStepIds: removedStepIdsAfterRemovingChildFilter,
} = removeOneStep({
existingTrigger: updatedTrigger,
existingSteps: updatedSteps,
stepIdToDelete: stepId,
stepToDeleteChildrenIds: step.nextStepIds,
});
updatedSteps = stepsAfterRemovingChildFilter;
updatedTrigger = triggerAfterRemovingChildFilter;
removedStepIds = [
...removedStepIds,
...removedStepIdsAfterRemovingChildFilter,
];
}
}
for (const step of updatedSteps) {
if (
step?.type === WorkflowActionType.FILTER &&
(!isDefined(step?.nextStepIds) || step.nextStepIds?.length === 0)
) {
const {
updatedSteps: stepsAfterRemovingFilterWithoutChildren,
updatedTrigger: triggerAfterRemovingFilterWithoutChildren,
removedStepIds: removedStepIdsAfterRemovingFilterWithoutChildren,
} = removeOneStep({
existingTrigger: updatedTrigger,
existingSteps: updatedSteps,
stepIdToDelete: step.id,
stepToDeleteChildrenIds: step.nextStepIds,
});
updatedSteps = stepsAfterRemovingFilterWithoutChildren;
updatedTrigger = triggerAfterRemovingFilterWithoutChildren;
removedStepIds = [
...removedStepIds,
...removedStepIdsAfterRemovingFilterWithoutChildren,
];
}
}
return {
updatedSteps,
updatedTrigger,
removedStepIds,
};
};
const removeTrigger = ({
existingSteps,
triggerChildrenIds,
}: {
existingSteps: WorkflowAction[] | null;
triggerChildrenIds?: string[];
}) => {
const stepIdsToRemove =
triggerChildrenIds?.filter((id) => {
const step = existingSteps?.find((step) => step.id === id);
return step?.type === WorkflowActionType.FILTER;
}) ?? [];
const updatedSteps =
existingSteps?.filter((step) => !stepIdsToRemove.includes(step.id)) ?? [];
return {
updatedSteps,
updatedTrigger: null,
removedStepIds: [TRIGGER_STEP_ID, ...stepIdsToRemove],
};
};
export const removeStep = ({
existingTrigger,
existingSteps,
stepIdToDelete,
stepToDeleteChildrenIds,
}: {
existingTrigger: WorkflowTrigger | null;
existingSteps: WorkflowAction[] | null;
stepIdToDelete: string;
stepToDeleteChildrenIds?: string[];
}) => {
if (stepIdToDelete === TRIGGER_STEP_ID) {
return removeTrigger({
existingSteps,
triggerChildrenIds: stepToDeleteChildrenIds,
});
} else {
return removeRegularStep({
existingTrigger,
existingSteps,
stepIdToDelete,
stepToDeleteChildrenIds,
});
}
};