Remove filters following deleted trigger/steps (#13697)

https://github.com/user-attachments/assets/e6ec591a-2064-4dfe-9048-2f234bef5c79
This commit is contained in:
Thomas Trompette
2025-08-07 09:40:26 +02:00
committed by GitHub
parent fecf8ca913
commit 8c41c956d5
14 changed files with 248 additions and 118 deletions
@@ -1,6 +1,6 @@
import { computeWorkflowVersionStepChanges } from 'src/modules/workflow/workflow-builder/utils/compute-workflow-version-step-updates.util';
import { WorkflowTrigger } from 'src/modules/workflow/workflow-trigger/types/workflow-trigger.type';
import { WorkflowAction } from 'src/modules/workflow/workflow-executor/workflow-actions/types/workflow-action.type';
import { WorkflowTrigger } from 'src/modules/workflow/workflow-trigger/types/workflow-trigger.type';
describe('computeWorkflowVersionStepChanges', () => {
it('should compute next step ids', () => {
@@ -10,13 +10,13 @@ describe('computeWorkflowVersionStepChanges', () => {
{ id: '1', nextStepIds: ['3'] },
{ id: '2', nextStepIds: ['3'] },
] as WorkflowAction[],
deletedStepId: '5',
deletedStepIds: ['5'],
};
const expectedResult = {
triggerNextStepIds: ['1', '2'],
stepsNextStepIds: { '1': ['3'], '2': ['3'] },
deletedStepId: '5',
deletedStepIds: ['5'],
};
expect(computeWorkflowVersionStepChanges(input)).toEqual(expectedResult);
@@ -1,17 +1,17 @@
import { WorkflowVersionStepChangesDTO } from 'src/engine/core-modules/workflow/dtos/workflow-version-step-changes.dto';
import { WorkflowTrigger } from 'src/modules/workflow/workflow-trigger/types/workflow-trigger.type';
import { WorkflowAction } from 'src/modules/workflow/workflow-executor/workflow-actions/types/workflow-action.type';
import { WorkflowTrigger } from 'src/modules/workflow/workflow-trigger/types/workflow-trigger.type';
export const computeWorkflowVersionStepChanges = ({
trigger,
steps,
createdStep,
deletedStepId,
deletedStepIds,
}: {
trigger: WorkflowTrigger | null;
steps: WorkflowAction[] | null;
createdStep?: WorkflowAction;
deletedStepId?: string;
deletedStepIds?: string[];
}): WorkflowVersionStepChangesDTO => {
return {
triggerNextStepIds: trigger?.nextStepIds,
@@ -19,6 +19,6 @@ export const computeWorkflowVersionStepChanges = ({
(steps || []).map((step) => [step.id, step.nextStepIds]),
),
createdStep,
deletedStepId,
deletedStepIds,
};
};
@@ -1,22 +1,22 @@
import { Test, TestingModule } from '@nestjs/testing';
import { getRepositoryToken } from '@nestjs/typeorm';
import { WorkflowVersionStepWorkspaceService } from 'src/modules/workflow/workflow-builder/workflow-version-step/workflow-version-step.workspace-service';
import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager';
import { WorkflowSchemaWorkspaceService } from 'src/modules/workflow/workflow-builder/workflow-schema/workflow-schema.workspace-service';
import { ServerlessFunctionService } from 'src/engine/metadata-modules/serverless-function/serverless-function.service';
import { AgentService } from 'src/engine/metadata-modules/agent/agent.service';
import { WorkflowRunWorkspaceService } from 'src/modules/workflow/workflow-runner/workflow-run/workflow-run.workspace-service';
import { WorkflowRunnerWorkspaceService } from 'src/modules/workflow/workflow-runner/workspace-services/workflow-runner.workspace-service';
import { WorkflowCommonWorkspaceService } from 'src/modules/workflow/common/workspace-services/workflow-common.workspace-service';
import { ScopedWorkspaceContextFactory } from 'src/engine/twenty-orm/factories/scoped-workspace-context.factory';
import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity';
import { WorkflowVersionWorkspaceEntity } from 'src/modules/workflow/common/standard-objects/workflow-version.workspace-entity';
import { ServerlessFunctionService } from 'src/engine/metadata-modules/serverless-function/serverless-function.service';
import { ScopedWorkspaceContextFactory } from 'src/engine/twenty-orm/factories/scoped-workspace-context.factory';
import { WorkspaceRepository } from 'src/engine/twenty-orm/repository/workspace.repository';
import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager';
import { WorkflowVersionWorkspaceEntity } from 'src/modules/workflow/common/standard-objects/workflow-version.workspace-entity';
import { WorkflowCommonWorkspaceService } from 'src/modules/workflow/common/workspace-services/workflow-common.workspace-service';
import { WorkflowSchemaWorkspaceService } from 'src/modules/workflow/workflow-builder/workflow-schema/workflow-schema.workspace-service';
import { WorkflowVersionStepWorkspaceService } from 'src/modules/workflow/workflow-builder/workflow-version-step/workflow-version-step.workspace-service';
import {
WorkflowAction,
WorkflowActionType,
} from 'src/modules/workflow/workflow-executor/workflow-actions/types/workflow-action.type';
import { WorkflowRunWorkspaceService } from 'src/modules/workflow/workflow-runner/workflow-run/workflow-run.workspace-service';
import { WorkflowRunnerWorkspaceService } from 'src/modules/workflow/workflow-runner/workspace-services/workflow-runner.workspace-service';
import { WorkflowTriggerType } from 'src/modules/workflow/workflow-trigger/types/workflow-trigger.type';
type MockWorkspaceRepository = Partial<
@@ -253,7 +253,7 @@ describe('WorkflowVersionStepWorkspaceService', () => {
'step-2': [],
'step-3': [],
},
deletedStepId: 'step-1',
deletedStepIds: ['step-1'],
});
});
@@ -268,6 +268,7 @@ describe('WorkflowVersionStepWorkspaceService', () => {
mockWorkflowVersionWorkspaceRepository.update,
).toHaveBeenCalledWith(mockWorkflowVersionId, {
trigger: null,
steps: mockSteps,
});
expect(result).toEqual({
@@ -276,7 +277,7 @@ describe('WorkflowVersionStepWorkspaceService', () => {
'step-2': [],
'step-3': [],
},
deletedStepId: 'trigger',
deletedStepIds: ['trigger'],
});
});
});
@@ -50,8 +50,8 @@ describe('removeStep', () => {
stepIdToDelete: '2',
});
expect(result.steps).toEqual([step1, step3]);
expect(result.trigger).toEqual(mockTrigger);
expect(result.updatedSteps).toEqual([step1, step3]);
expect(result.updatedTrigger).toEqual(mockTrigger);
});
it('should handle removing a step that has no next steps', () => {
@@ -65,8 +65,8 @@ describe('removeStep', () => {
stepIdToDelete: '2',
});
expect(result.steps).toEqual([{ ...step1, nextStepIds: [] }, step3]);
expect(result.trigger).toEqual(mockTrigger);
expect(result.updatedSteps).toEqual([{ ...step1, nextStepIds: [] }, step3]);
expect(result.updatedTrigger).toEqual(mockTrigger);
});
it('should update nextStepIds of parent steps to include children of removed step', () => {
@@ -81,8 +81,11 @@ describe('removeStep', () => {
stepToDeleteChildrenIds: ['3'],
});
expect(result.steps).toEqual([{ ...step1, nextStepIds: ['3'] }, step3]);
expect(result.trigger).toEqual(mockTrigger);
expect(result.updatedSteps).toEqual([
{ ...step1, nextStepIds: ['3'] },
step3,
]);
expect(result.updatedTrigger).toEqual(mockTrigger);
});
it('should handle multiple parent steps pointing to the same step', () => {
@@ -98,12 +101,12 @@ describe('removeStep', () => {
stepToDeleteChildrenIds: ['4'],
});
expect(result.steps).toEqual([
expect(result.updatedSteps).toEqual([
{ ...step1, nextStepIds: ['4'] },
{ ...step2, nextStepIds: ['4'] },
step4,
]);
expect(result.trigger).toEqual(mockTrigger);
expect(result.updatedTrigger).toEqual(mockTrigger);
});
it('should handle removing a step with multiple children', () => {
@@ -119,12 +122,12 @@ describe('removeStep', () => {
stepToDeleteChildrenIds: ['3', '4'],
});
expect(result.steps).toEqual([
expect(result.updatedSteps).toEqual([
{ ...step1, nextStepIds: ['3', '4'] },
step3,
step4,
]);
expect(result.trigger).toEqual(mockTrigger);
expect(result.updatedTrigger).toEqual(mockTrigger);
});
it('should handle removing a step linked to trigger', () => {
@@ -139,7 +142,56 @@ describe('removeStep', () => {
stepToDeleteChildrenIds: ['2'],
});
expect(result.steps).toEqual([step2, step3]);
expect(result.trigger).toEqual({ ...mockTrigger, nextStepIds: ['2'] });
expect(result.updatedSteps).toEqual([step2, step3]);
expect(result.updatedTrigger).toEqual({
...mockTrigger,
nextStepIds: ['2'],
});
});
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',
stepToDeleteChildrenIds: ['1'],
});
expect(result.updatedTrigger).toEqual(null);
expect(result.updatedSteps).toEqual([step2, step3]);
});
});
@@ -1,6 +1,9 @@
import { isDefined } from 'twenty-shared/utils';
import { WorkflowAction } from 'src/modules/workflow/workflow-executor/workflow-actions/types/workflow-action.type';
import {
WorkflowAction,
WorkflowActionType,
} from 'src/modules/workflow/workflow-executor/workflow-actions/types/workflow-action.type';
import { WorkflowTrigger } from 'src/modules/workflow/workflow-trigger/types/workflow-trigger.type';
const computeUpdatedNextStepIds = ({
@@ -25,7 +28,7 @@ const computeUpdatedNextStepIds = ({
];
};
export const removeStep = ({
const removeRegularStep = ({
existingTrigger,
existingSteps,
stepIdToDelete,
@@ -35,9 +38,30 @@ export const removeStep = ({
existingSteps: WorkflowAction[];
stepIdToDelete: string;
stepToDeleteChildrenIds?: string[];
}): { steps: WorkflowAction[]; trigger: WorkflowTrigger | null } => {
}): {
updatedSteps: WorkflowAction[];
updatedTrigger: WorkflowTrigger | null;
removedStepIds: string[];
} => {
const stepIdsToRemove = [stepIdToDelete];
const stepIdsToRemoveChildrenIds =
stepToDeleteChildrenIds
?.map((id) => {
const step = existingSteps.find((step) => step.id === id);
if (step?.type === WorkflowActionType.FILTER) {
stepIdsToRemove.push(step.id);
return step.nextStepIds;
}
return id;
})
.filter(isDefined)
.flat() ?? [];
const updatedSteps = existingSteps
.filter((step) => step.id !== stepIdToDelete)
.filter((step) => !stepIdsToRemove.includes(step.id))
.map((step) => {
if (step.nextStepIds?.includes(stepIdToDelete)) {
return {
@@ -45,7 +69,7 @@ export const removeStep = ({
nextStepIds: computeUpdatedNextStepIds({
existingNextStepIds: step.nextStepIds,
stepIdToDelete,
stepToDeleteChildrenIds,
stepToDeleteChildrenIds: stepIdsToRemoveChildrenIds,
}),
};
}
@@ -62,11 +86,66 @@ export const removeStep = ({
nextStepIds: computeUpdatedNextStepIds({
existingNextStepIds: existingTrigger.nextStepIds,
stepIdToDelete,
stepToDeleteChildrenIds,
stepToDeleteChildrenIds: stepIdsToRemoveChildrenIds,
}),
};
}
}
return { trigger: updatedTrigger, steps: updatedSteps };
return {
updatedTrigger,
updatedSteps,
removedStepIds: stepIdsToRemove,
};
};
const removeTrigger = ({
existingSteps,
triggerChildrenIds,
}: {
existingSteps: WorkflowAction[];
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', ...stepIdsToRemove],
};
};
export const removeStep = ({
existingTrigger,
existingSteps,
stepIdToDelete,
stepToDeleteChildrenIds,
}: {
existingTrigger: WorkflowTrigger | null;
existingSteps: WorkflowAction[];
stepIdToDelete: string;
stepToDeleteChildrenIds?: string[];
}) => {
if (stepIdToDelete === 'trigger') {
return removeTrigger({
existingSteps,
triggerChildrenIds: stepToDeleteChildrenIds,
});
} else {
return removeRegularStep({
existingTrigger,
existingSteps,
stepIdToDelete,
stepToDeleteChildrenIds,
});
}
};
@@ -10,6 +10,8 @@ import { v4 } from 'uuid';
import { BASE_TYPESCRIPT_PROJECT_INPUT_SCHEMA } from 'src/engine/core-modules/serverless/drivers/constants/base-typescript-project-input-schema';
import { CreateWorkflowVersionStepInput } from 'src/engine/core-modules/workflow/dtos/create-workflow-version-step-input.dto';
import { WorkflowStepPositionInput } from 'src/engine/core-modules/workflow/dtos/update-workflow-step-position-input.dto';
import { WorkflowVersionStepChangesDTO } from 'src/engine/core-modules/workflow/dtos/workflow-version-step-changes.dto';
import { AgentService } from 'src/engine/metadata-modules/agent/agent.service';
import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity';
import { ServerlessFunctionService } from 'src/engine/metadata-modules/serverless-function/serverless-function.service';
@@ -21,6 +23,7 @@ import {
import { WorkflowVersionWorkspaceEntity } from 'src/modules/workflow/common/standard-objects/workflow-version.workspace-entity';
import { assertWorkflowVersionIsDraft } from 'src/modules/workflow/common/utils/assert-workflow-version-is-draft.util';
import { WorkflowCommonWorkspaceService } from 'src/modules/workflow/common/workspace-services/workflow-common.workspace-service';
import { computeWorkflowVersionStepChanges } from 'src/modules/workflow/workflow-builder/utils/compute-workflow-version-step-updates.util';
import { WorkflowSchemaWorkspaceService } from 'src/modules/workflow/workflow-builder/workflow-schema/workflow-schema.workspace-service';
import { insertStep } from 'src/modules/workflow/workflow-builder/workflow-version-step/utils/insert-step';
import { removeStep } from 'src/modules/workflow/workflow-builder/workflow-version-step/utils/remove-step';
@@ -32,9 +35,6 @@ import {
} from 'src/modules/workflow/workflow-executor/workflow-actions/types/workflow-action.type';
import { WorkflowRunWorkspaceService } from 'src/modules/workflow/workflow-runner/workflow-run/workflow-run.workspace-service';
import { WorkflowRunnerWorkspaceService } from 'src/modules/workflow/workflow-runner/workspace-services/workflow-runner.workspace-service';
import { WorkflowStepPositionInput } from 'src/engine/core-modules/workflow/dtos/update-workflow-step-position-input.dto';
import { WorkflowVersionStepChangesDTO } from 'src/engine/core-modules/workflow/dtos/workflow-version-step-changes.dto';
import { computeWorkflowVersionStepChanges } from 'src/modules/workflow/workflow-builder/utils/compute-workflow-version-step-updates.util';
const BASE_STEP_DEFINITION: BaseWorkflowActionSettings = {
outputSchema: {},
@@ -225,51 +225,54 @@ export class WorkflowVersionStepWorkspaceService {
);
}
if (stepIdToDelete === 'trigger') {
await workflowVersionRepository.update(workflowVersion.id, {
trigger: null,
});
return computeWorkflowVersionStepChanges({
trigger: null,
steps: workflowVersion?.steps,
deletedStepId: stepIdToDelete,
});
}
const existingTrigger = workflowVersion.trigger;
const isDeletingTrigger =
stepIdToDelete === 'trigger' && isDefined(existingTrigger);
const stepToDelete = workflowVersion.steps.find(
(step) => step.id === stepIdToDelete,
);
if (!isDefined(stepToDelete)) {
if (!isDefined(stepToDelete) && !isDeletingTrigger) {
throw new WorkflowVersionStepException(
"Can't delete not existing step",
WorkflowVersionStepExceptionCode.NOT_FOUND,
);
}
const workflowVersionUpdates = removeStep({
const stepToDeleteChildrenIds = isDeletingTrigger
? (existingTrigger?.nextStepIds ?? [])
: (stepToDelete?.nextStepIds ?? []);
const { updatedSteps, updatedTrigger, removedStepIds } = removeStep({
existingTrigger,
existingSteps: workflowVersion.steps,
stepIdToDelete,
stepToDeleteChildrenIds: stepToDelete.nextStepIds,
stepToDeleteChildrenIds,
});
await workflowVersionRepository.update(
workflowVersion.id,
workflowVersionUpdates,
await workflowVersionRepository.update(workflowVersion.id, {
steps: updatedSteps,
trigger: updatedTrigger,
});
const removedSteps = workflowVersion.steps.filter((step) =>
removedStepIds.includes(step.id),
);
await this.runWorkflowVersionStepDeletionSideEffects({
step: stepToDelete,
workspaceId,
});
await Promise.all(
removedSteps.map((step) =>
this.runWorkflowVersionStepDeletionSideEffects({
step,
workspaceId,
}),
),
);
return computeWorkflowVersionStepChanges({
...workflowVersionUpdates,
deletedStepId: stepIdToDelete,
steps: updatedSteps,
trigger: updatedTrigger,
deletedStepIds: removedStepIds,
});
}