Allow to insert into loop step ids (#14425)

- Create empty node on iterator creation - Add options on step creation to insert in loop - Handle properly when a step is removed from loop - Remove loopNextStepIds from frontend action - Add a frontend skeleton for empty action https://github.com/user-attachments/assets/281a8c15-8062-4702-afb4-0d9a50902252
2025-09-12 09:30:28 +02:00
parent bdf60b1680
commit 231d1a98af
23 changed files with 538 additions and 92 deletions
@@ -0,0 +1,10 @@
+import { type WorkflowActionType } from 'src/modules/workflow/workflow-executor/workflow-actions/types/workflow-action.type';
+
+type WorkflowIteratorStepCreationOptions = {
+  parentStepType: WorkflowActionType.ITERATOR;
+  settings: {
+    shouldInsertToLoop: boolean;
+  };
+};
+
+export type WorkflowStepCreationOptions = WorkflowIteratorStepCreationOptions;
@@ -3,6 +3,7 @@ import { TRIGGER_STEP_ID } from 'twenty-shared/workflow';
 import { insertStep } from 'src/modules/workflow/workflow-builder/workflow-version-step/utils/insert-step';
 import {
  type WorkflowAction,
+  type WorkflowIteratorAction,
  WorkflowActionType,
 } from 'src/modules/workflow/workflow-executor/workflow-actions/types/workflow-action.type';
 import {
@@ -10,6 +11,24 @@ import {
  WorkflowTriggerType,
 } from 'src/modules/workflow/workflow-trigger/types/workflow-trigger.type';

+const mockIteratorStep: WorkflowIteratorAction = {
+  id: '1',
+  name: 'Iterator 1',
+  type: WorkflowActionType.ITERATOR,
+  settings: {
+    input: {
+      initialLoopStepIds: ['existing-loop-step'],
+      items: [],
+    },
+    outputSchema: {},
+    errorHandlingOptions: {
+      retryOnFailure: { value: false },
+      continueOnFailure: { value: false },
+    },
+  },
+  valid: true,
+};
+
 describe('insertStep', () => {
  const createMockAction = (
    id: string,
@@ -158,4 +177,55 @@ describe('insertStep', () => {
      nextStepIds: ['1', 'new'],
    });
  });
+
+  it('should add step to iterator initialLoopStepIds when shouldInsertToLoop is true', () => {
+    const existingTrigger = createMockTrigger(['1']);
+    const newStep = createMockAction('new');
+
+    const result = insertStep({
+      existingTrigger,
+      existingSteps: [mockIteratorStep],
+      insertedStep: newStep,
+      parentStepId: '1',
+      parentStepOptions: {
+        parentStepType: WorkflowActionType.ITERATOR,
+        settings: {
+          shouldInsertToLoop: true,
+        },
+      },
+    });
+
+    const updatedIteratorStep = result
+      .updatedSteps[0] as WorkflowIteratorAction;
+
+    expect(updatedIteratorStep.settings.input.initialLoopStepIds).toEqual([
+      'existing-loop-step',
+      'new',
+    ]);
+  });
+
+  it('should not add step to iterator initialLoopStepIds when shouldInsertToLoop is false', () => {
+    const existingTrigger = createMockTrigger(['1']);
+    const newStep = createMockAction('new');
+
+    const result = insertStep({
+      existingTrigger,
+      existingSteps: [mockIteratorStep],
+      insertedStep: newStep,
+      parentStepId: '1',
+      parentStepOptions: {
+        parentStepType: WorkflowActionType.ITERATOR,
+        settings: {
+          shouldInsertToLoop: false,
+        },
+      },
+    });
+
+    const updatedIteratorStep = result
+      .updatedSteps[0] as WorkflowIteratorAction;
+
+    expect(updatedIteratorStep.settings.input.initialLoopStepIds).toEqual([
+      'existing-loop-step',
+    ]);
+  });
 });
@@ -255,4 +255,54 @@ describe('removeStep', () => {
    expect(result.updatedTrigger).toEqual(null);
    expect(result.updatedSteps).toEqual([]);
  });
+
+  it('should handle removing a step that is part of iteratorLoopStepIds', () => {
+    const step1 = createMockAction('1', ['2']);
+    const iteratorStep = {
+      id: '2',
+      name: 'Iterator Step',
+      type: WorkflowActionType.ITERATOR,
+      settings: {
+        input: {
+          initialLoopStepIds: ['3'],
+          iterableValue: { value: [] },
+          iteratorKey: 'item',
+        },
+        outputSchema: {},
+        errorHandlingOptions: {
+          retryOnFailure: { value: false },
+          continueOnFailure: { value: false },
+        },
+      },
+      valid: true,
+      nextStepIds: ['4'],
+    } as WorkflowAction;
+    const step3 = createMockAction('3', ['5']);
+    const step4 = createMockAction('4');
+    const step5 = createMockAction('5');
+
+    const result = removeStep({
+      existingTrigger: mockTrigger,
+      existingSteps: [step1, iteratorStep, step3, step4, step5],
+      stepIdToDelete: '3',
+      stepToDeleteChildrenIds: ['5'],
+    });
+
+    expect(result.updatedSteps).toEqual([
+      step1,
+      {
+        ...iteratorStep,
+        settings: {
+          ...iteratorStep.settings,
+          input: {
+            ...iteratorStep.settings.input,
+            initialLoopStepIds: ['5'],
+          },
+        },
+      },
+      step4,
+      step5,
+    ]);
+    expect(result.updatedTrigger).toEqual(mockTrigger);
+  });
 });
@@ -1,62 +1,50 @@
+import { isDefined } from 'twenty-shared/utils';
 import { TRIGGER_STEP_ID } from 'twenty-shared/workflow';

-import { type WorkflowAction } from 'src/modules/workflow/workflow-executor/workflow-actions/types/workflow-action.type';
+import {
+  WorkflowVersionStepException,
+  WorkflowVersionStepExceptionCode,
+} from 'src/modules/workflow/common/exceptions/workflow-version-step.exception';
+import { type WorkflowStepCreationOptions } from 'src/modules/workflow/workflow-builder/workflow-version-step/types/WorkflowStepCreationOptions';
+import { type WorkflowIteratorActionSettings } from 'src/modules/workflow/workflow-executor/workflow-actions/iterator/types/workflow-iterator-action-settings.type';
+import {
+  WorkflowActionType,
+  type WorkflowAction,
+} 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';

 export const insertStep = ({
  existingSteps,
  existingTrigger,
  insertedStep,
-  parentStepId,
  nextStepId,
+  parentStepId,
+  parentStepOptions,
 }: {
  existingSteps: WorkflowAction[];
  existingTrigger: WorkflowTrigger | null;
  insertedStep: WorkflowAction;
-  parentStepId?: string;
  nextStepId?: string;
+  parentStepId?: string;
+  parentStepOptions?: WorkflowStepCreationOptions;
 }): {
  updatedSteps: WorkflowAction[];
  updatedInsertedStep: WorkflowAction;
  updatedTrigger: WorkflowTrigger | null;
 } => {
-  let updatedTrigger = existingTrigger;
-
-  let updatedExistingSteps = existingSteps;
-
-  if (parentStepId === TRIGGER_STEP_ID) {
-    if (!existingTrigger) {
-      throw new Error('Cannot insert step from undefined trigger');
-    }
-
-    updatedTrigger = {
-      ...existingTrigger,
-      nextStepIds: [
-        ...new Set([
-          ...(existingTrigger.nextStepIds?.filter((id) => id !== nextStepId) ||
-            []),
-          insertedStep.id,
-        ]),
-      ],
-    };
-  } else {
-    updatedExistingSteps = existingSteps.map((existingStep) => {
-      if (existingStep.id === parentStepId) {
-        return {
-          ...existingStep,
-          nextStepIds: [
-            ...new Set([
-              ...(existingStep.nextStepIds?.filter((id) => id !== nextStepId) ||
-                []),
-              insertedStep.id,
-            ]),
-          ],
-        };
-      }
-
-      return existingStep;
-    });
-  }
+  let { updatedSteps, updatedTrigger } = isDefined(parentStepId)
+    ? updateParentStep({
+        trigger: existingTrigger,
+        steps: existingSteps,
+        parentStepId,
+        insertedStepId: insertedStep.id,
+        nextStepId,
+        parentStepOptions,
+      })
+    : {
+        updatedSteps: existingSteps,
+        updatedTrigger: existingTrigger,
+      };

  const updatedInsertedStep = {
    ...insertedStep,
@@ -64,8 +52,165 @@ export const insertStep = ({
  };

  return {
-    updatedSteps: [...updatedExistingSteps, updatedInsertedStep],
+    updatedSteps: [...updatedSteps, updatedInsertedStep],
    updatedTrigger,
    updatedInsertedStep,
  };
 };
+
+const updateParentStep = ({
+  steps,
+  trigger,
+  parentStepId,
+  insertedStepId,
+  nextStepId,
+  parentStepOptions,
+}: {
+  steps: WorkflowAction[];
+  trigger: WorkflowTrigger | null;
+  parentStepId: string;
+  insertedStepId: string;
+  nextStepId?: string;
+  parentStepOptions?: WorkflowStepCreationOptions;
+}): {
+  updatedSteps: WorkflowAction[];
+  updatedTrigger: WorkflowTrigger | null;
+} => {
+  if (isDefined(parentStepOptions)) {
+    return updateStepsWithOptions({
+      steps,
+      parentStepId,
+      insertedStepId,
+      parentStepOptions,
+      trigger,
+    });
+  } else {
+    return updateParentStepNextStepIds({
+      steps,
+      trigger,
+      parentStepId,
+      insertedStepId,
+      nextStepId,
+    });
+  }
+};
+
+const updateParentStepNextStepIds = ({
+  steps,
+  trigger,
+  parentStepId,
+  insertedStepId,
+  nextStepId,
+}: {
+  steps: WorkflowAction[];
+  trigger: WorkflowTrigger | null;
+  parentStepId: string;
+  insertedStepId: string;
+  nextStepId?: string;
+}): {
+  updatedSteps: WorkflowAction[];
+  updatedTrigger: WorkflowTrigger | null;
+} => {
+  let updatedTrigger = trigger;
+
+  let updatedSteps = steps;
+
+  if (parentStepId === TRIGGER_STEP_ID) {
+    if (!trigger) {
+      throw new WorkflowVersionStepException(
+        'Cannot insert step from undefined trigger',
+        WorkflowVersionStepExceptionCode.INVALID_REQUEST,
+      );
+    }
+
+    updatedTrigger = {
+      ...trigger,
+      nextStepIds: [
+        ...new Set([
+          ...(trigger.nextStepIds?.filter((id) => id !== nextStepId) || []),
+          insertedStepId,
+        ]),
+      ],
+    };
+  } else {
+    updatedSteps = steps.map((step) => {
+      if (step.id === parentStepId) {
+        return {
+          ...step,
+          nextStepIds: [
+            ...new Set([
+              ...(step.nextStepIds?.filter((id) => id !== nextStepId) || []),
+              insertedStepId,
+            ]),
+          ],
+        };
+      }
+
+      return step;
+    });
+  }
+
+  return {
+    updatedSteps,
+    updatedTrigger,
+  };
+};
+
+const updateStepsWithOptions = ({
+  parentStepId,
+  insertedStepId,
+  steps,
+  parentStepOptions,
+  trigger,
+}: {
+  parentStepId: string;
+  insertedStepId: string;
+  steps: WorkflowAction[];
+  parentStepOptions: WorkflowStepCreationOptions;
+  trigger: WorkflowTrigger | null;
+}) => {
+  let updatedSteps = steps;
+
+  switch (parentStepOptions.parentStepType) {
+    case WorkflowActionType.ITERATOR:
+      if (!parentStepOptions.settings.shouldInsertToLoop) {
+        break;
+      }
+
+      updatedSteps = steps.map((step) => {
+        if (step.id === parentStepId) {
+          if (step.type !== WorkflowActionType.ITERATOR) {
+            throw new WorkflowVersionStepException(
+              `Step ${step.id} is not an iterator`,
+              WorkflowVersionStepExceptionCode.INVALID_REQUEST,
+            );
+          }
+
+          return {
+            ...step,
+            settings: {
+              ...step.settings,
+              input: {
+                ...step.settings.input,
+                initialLoopStepIds: [
+                  ...(step.settings.input.initialLoopStepIds || []),
+                  insertedStepId,
+                ],
+              },
+            } satisfies WorkflowIteratorActionSettings,
+          };
+        }
+
+        return step;
+      });
+
+      break;
+    default:
+      break;
+  }
+
+  return {
+    updatedSteps,
+    updatedTrigger: trigger,
+  };
+};
@@ -59,6 +59,27 @@ const removeOneStep = ({
          };
        }

+        if (
+          step.type === WorkflowActionType.ITERATOR &&
+          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: stepToDeleteChildrenIds,
+                }),
+              },
+            },
+          };
+        }
+
        return step;
      }) ?? [];

@@ -31,6 +31,7 @@ import { type BaseWorkflowActionSettings } from 'src/modules/workflow/workflow-e
 import {
  type WorkflowAction,
  WorkflowActionType,
+  WorkflowEmptyAction,
  type WorkflowFormAction,
 } 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';
@@ -72,13 +73,20 @@ export class WorkflowVersionStepWorkspaceService {
    workspaceId: string;
    input: CreateWorkflowVersionStepInput;
  }): Promise<WorkflowVersionStepChangesDTO> {
-    const { workflowVersionId, stepType, parentStepId, nextStepId, position } =
-      input;
+    const {
+      workflowVersionId,
+      stepType,
+      parentStepId,
+      nextStepId,
+      position,
+      parentStepOptions,
+    } = input;

-    const newStep = await this.getStepDefaultDefinition({
+    const newStep = await this.runStepCreationSideEffectsAndBuildStep({
      type: stepType,
      workspaceId,
      position,
+      workflowVersionId,
    });

    const enrichedNewStep = await this.enrichOutputSchema({
@@ -118,6 +126,7 @@ export class WorkflowVersionStepWorkspaceService {
      insertedStep: enrichedNewStep,
      parentStepId,
      nextStepId,
+      parentStepOptions,
    });

    await workflowVersionRepository.update(workflowVersion.id, {
@@ -513,14 +522,16 @@ export class WorkflowVersionStepWorkspaceService {
    }
  }

-  private async getStepDefaultDefinition({
+  private async runStepCreationSideEffectsAndBuildStep({
    type,
    workspaceId,
    position,
+    workflowVersionId,
  }: {
    type: WorkflowActionType;
    workspaceId: string;
    position?: WorkflowStepPositionInput;
+    workflowVersionId: string;
  }): Promise<WorkflowAction> {
    const newStepId = v4();

@@ -722,6 +733,12 @@ export class WorkflowVersionStepWorkspaceService {
        };
      }
      case WorkflowActionType.ITERATOR: {
+        const emptyNodeStep = await this.createEmptyNodeForIteratorStep({
+          iteratorStepId: baseStep.id,
+          workflowVersionId,
+          workspaceId,
+        });
+
        return {
          ...baseStep,
          name: 'Iterator',
@@ -730,7 +747,7 @@ export class WorkflowVersionStepWorkspaceService {
            ...BASE_STEP_DEFINITION,
            input: {
              items: [],
-              initialLoopStepIds: [],
+              initialLoopStepIds: [emptyNodeStep.id],
            },
          },
        };
@@ -861,4 +878,54 @@ export class WorkflowVersionStepWorkspaceService {
      }
    }
  }
+
+  private async createEmptyNodeForIteratorStep({
+    iteratorStepId,
+    workflowVersionId,
+    workspaceId,
+  }: {
+    iteratorStepId: string;
+    workflowVersionId: string;
+    workspaceId: string;
+  }): Promise<WorkflowAction> {
+    const workflowVersionRepository =
+      await this.twentyORMGlobalManager.getRepositoryForWorkspace<WorkflowVersionWorkspaceEntity>(
+        workspaceId,
+        'workflowVersion',
+        { shouldBypassPermissionChecks: true },
+      );
+
+    const workflowVersion = await workflowVersionRepository.findOne({
+      where: {
+        id: workflowVersionId,
+      },
+    });
+
+    if (!isDefined(workflowVersion)) {
+      throw new WorkflowVersionStepException(
+        'WorkflowVersion not found',
+        WorkflowVersionStepExceptionCode.NOT_FOUND,
+      );
+    }
+
+    const existingSteps = workflowVersion.steps ?? [];
+
+    const emptyNodeStep: WorkflowEmptyAction = {
+      id: v4(),
+      name: 'Empty Node',
+      type: WorkflowActionType.EMPTY,
+      valid: true,
+      nextStepIds: [iteratorStepId],
+      settings: {
+        ...BASE_STEP_DEFINITION,
+        input: {},
+      },
+    };
+
+    await workflowVersionRepository.update(workflowVersion.id, {
+      steps: [...existingSteps, emptyNodeStep],
+    });
+
+    return emptyNodeStep;
+  }
 }