Move the workflow draft version overriding to the backend #9328

Devessier · 2025-01-02T17:34:27Z

In the formatFieldMetadataValue function, allow people to call TypeORM's save() method with unserialized JSON data.
Create an overrideWorkflowDraftVersion mutation that takes a workflow id and the id of the workflow version to use as the new draft
- If no draft exists yet, create one
- If a draft already exists, deactivate its serverless functions
- Duplicate every step. For serverless function steps, it includes duplicating the functions
- Save the data of the step in DB
Call the overrideWorkflowDraftVersion mutation in the old workflow header and in the new Cmd+K actions
- I chose to not update the Apollo cache manually as the information of the new draft are going to be automatically fetched once the user lands on the workflow's show page. Note that we redirect the user to this page after overriding the draft version.

packages/twenty-server/src/engine/core-modules/workflow/dtos/override-workflow-draft-version.ts

...er/src/modules/workflow/common/workspace-services/workflow-version-step.workspace-service.ts

...twenty-server/src/engine/metadata-modules/serverless-function/serverless-function.service.ts

…ting steps and trigger

…low id instead of the current draft workflow version id

Devessier · 2025-01-03T16:55:55Z

packages/twenty-server/src/engine/twenty-orm/utils/format-data.util.ts

+  if (
+    fieldMetadata.type === FieldMetadataType.RAW_JSON &&
+    typeof value === 'string'
+  ) {
    return JSON.parse(value as string);
  }


I discussed this change with @Weiko

Devessier · 2025-01-03T17:13:47Z

Hi @martmull! I would be glad if you could look at my PR when you return to work. It's related to serverless functions, and I want to confirm everything is okay for you. Thanks!

greptile-apps

PR Summary

This PR moves workflow draft version overriding functionality from frontend to backend, improving architecture and data handling.

Added new overrideWorkflowDraftVersion mutation in /packages/twenty-server/src/engine/core-modules/workflow/resolvers/workflow-version-step.resolver.ts to handle draft version management
Added copyOneServerlessFunction method in serverless-function.service.ts to properly duplicate serverless functions when copying workflow steps
Modified formatFieldMetadataValue in /packages/twenty-server/src/engine/twenty-orm/utils/format-data.util.ts to handle unserialized JSON data for TypeORM's save() method
Added type safety with assertWorkflowVersionHasSteps utility in /packages/twenty-server/src/modules/workflow/common/utils/assert-workflow-version-has-steps.ts
Updated frontend components to use new backend mutation instead of handling draft version creation client-side

_{13 file(s) reviewed, 18 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

...gle-record/workflow-version-actions/hooks/useUseAsDraftWorkflowVersionSingleRecordAction.tsx

greptile-apps · 2025-01-03T17:15:05Z

packages/twenty-front/src/modules/workflow/components/RecordShowPageWorkflowVersionHeader.tsx

+              await overrideWorkflowDraftVersion({
                workflowId: workflowVersion.workflow.id,
-                name: `v${workflowVersion.workflow.versions.length + 1}`,
-                status: 'DRAFT',
-                trigger: workflowVersion.trigger,
-                steps: workflowVersion.steps,
+                workflowVersionIdToCopy: workflowVersion.id,
              });


style: No error handling for failed override operation. Consider wrapping in try/catch.

greptile-apps · 2025-01-03T17:15:38Z

packages/twenty-front/src/modules/workflow/graphql/mutations/overrideWorkflowDraftVersion.ts

+  mutation OverrideWorkflowDraftVersion(
+    $input: OverrideWorkflowDraftVersionInput!
+  ) {
+    overrideWorkflowDraftVersion(input: $input)


style: mutation response type is not specified - should define exact return type for better type safety

greptile-apps · 2025-01-03T17:15:42Z

packages/twenty-server/src/engine/core-modules/serverless/utils/serverless-get-folder.utils.ts

@@ -9,7 +9,7 @@ export const getServerlessFolder = ({
  version,
 }: {
  serverlessFunction: ServerlessFunctionEntity;
-  version?: string;
+  version?: 'draft' | 'latest' | (string & NonNullable<unknown>);


style: The type string & NonNullable<unknown> is redundant since string is already non-nullable. Consider using just string or creating a specific type/enum for versions.

not sure this change helps a lot

Maybe we can use constants instead of hard-coded strings?

we can't because version can be '1', '2', etc...

greptile-apps · 2025-01-03T17:15:46Z

...twenty-server/src/engine/core-modules/workflow/dtos/override-workflow-draft-version-input.ts

+    description: 'Workflow version ID',
+    nullable: false,
+  })
+  workflowVersionIdToCopy: string;


style: field name 'workflowVersionIdToCopy' could be more concise, e.g. 'sourceVersionId'

greptile-apps · 2025-01-03T17:16:38Z

packages/twenty-server/src/modules/workflow/common/utils/assert-workflow-version-has-steps.ts

+): asserts workflowVersion is WorkflowVersionWorkspaceEntity & {
+  steps: WorkflowAction[];
+} {


style: assertion type could be more specific by using Required<Pick<WorkflowVersionWorkspaceEntity, 'steps'>> to ensure steps property exists

greptile-apps · 2025-01-03T17:17:01Z

...er/src/modules/workflow/common/workspace-services/workflow-version-step.workspace-service.ts

+    if (Array.isArray(draftWorkflowVersion.steps)) {
+      await Promise.all(
+        draftWorkflowVersion.steps.map((step) =>
+          this.runWorkflowVersionStepDeletionSideEffects({
+            step,
+            workspaceId,
+          }),
+        ),
+      );
+    }


logic: potential race condition if multiple deletion operations fail - consider using Promise.allSettled and handling partial failures

...er/src/modules/workflow/common/workspace-services/workflow-version-step.workspace-service.ts

greptile-apps · 2025-01-03T17:17:02Z

...er/src/modules/workflow/common/workspace-services/workflow-version-step.workspace-service.ts

+    return stepToDelete;
+  }
+
+  async overrideWorkflowDraftVersion({


style: no return type specified for overrideWorkflowDraftVersion method

greptile-apps · 2025-01-03T17:17:02Z

...er/src/modules/workflow/common/workspace-services/workflow-version-step.workspace-service.ts

+    for (const step of workflowVersionToCopy.steps) {
+      const duplicatedStep = await this.duplicateStep({
+        step,
+        workspaceId,
+      });
+
+      newWorkflowVersionSteps.push(duplicatedStep);
+    }


style: duplicating steps sequentially could be slow for large workflows - consider using Promise.all for parallel execution

martmull

Very nice code, I left some minor comments but the overall logic looks good to me

...gle-record/workflow-version-actions/hooks/useUseAsDraftWorkflowVersionSingleRecordAction.tsx

martmull · 2025-01-06T08:58:56Z

packages/twenty-server/src/engine/core-modules/serverless/utils/serverless-get-folder.utils.ts

@@ -9,7 +9,7 @@ export const getServerlessFolder = ({
  version,
 }: {
  serverlessFunction: ServerlessFunctionEntity;
-  version?: string;
+  version?: 'draft' | 'latest' | (string & NonNullable<unknown>);


not sure this change helps a lot

...twenty-server/src/engine/metadata-modules/serverless-function/serverless-function.service.ts

...er/src/modules/workflow/common/workspace-services/workflow-version-step.workspace-service.ts

Devessier · 2025-01-06T10:36:25Z

We want to rename the overrideWorkflowDraftVersion to createDraftFromWorkflowVersion.

thomtrp

Great work 🔥

thomtrp · 2025-01-06T13:17:57Z

...er/src/modules/workflow/common/workspace-services/workflow-version-step.workspace-service.ts

+      });
+    }
+
+    assertWorkflowVersionIsDraft(draftWorkflowVersion);


can be in else {}

I think it's safer to run the assertion for both cases. I might have forgotten to create a draft version above. That way, we are sure we will deal with a draft version.

github-actions · 2025-01-06T13:57:02Z

	Fails
🚫	`node` failed.

Log

�[31mError: �[39m SyntaxError: Unexpected token C in JSON at position 0
    at JSON.parse (<anonymous>)
�[90m    at parseJSONFromBytes (node:internal/deps/undici/undici:5584:19)�[39m
�[90m    at successSteps (node:internal/deps/undici/undici:5555:27)�[39m
�[90m    at fullyReadBody (node:internal/deps/undici/undici:1665:9)�[39m
�[90m    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)�[39m
�[90m    at async specConsumeBody (node:internal/deps/undici/undici:5564:7)�[39m
danger-results://tmp/danger-results-7fc25f99.json

Generated by 🚫 dangerJS against 5ba02b6

thomtrp reviewed Jan 2, 2025

View reviewed changes

Devessier force-pushed the move-workflow-draft-version-override-to-backend branch from 6694098 to 70261ad Compare January 2, 2025 18:06

Devessier added 3 commits January 3, 2025 10:32

feat: wip

4ec67d0

chore: remove test

615512a

refactor: extract the code for step duplication into a separate function

9299f08

Devessier force-pushed the move-workflow-draft-version-override-to-backend branch from 70261ad to 9299f08 Compare January 3, 2025 12:04

Weiko added the -PR: wip label Jan 3, 2025

Devessier added 3 commits January 3, 2025 17:09

fix: allow the save method to be called on workflow versions when set…

53c1f2f

…ting steps and trigger

feat: create draft version if it doesn't exist yet and take the workf…

541f0ee

…low id instead of the current draft workflow version id

feat: call the override workflow draft version mutation on the frontend

4319ee8

Devessier commented Jan 3, 2025

View reviewed changes

fix: ensure we can only duplicate a version from the same workflow

4ebe413

Devessier marked this pull request as ready for review January 3, 2025 17:14

Devessier added -PR: awaiting review and removed -PR: wip labels Jan 3, 2025

greptile-apps bot reviewed Jan 3, 2025

View reviewed changes

martmull reviewed Jan 6, 2025

View reviewed changes

This comment was marked as duplicate.

Sign in to view

Devessier added -PR: awaiting author and removed -PR: awaiting review labels Jan 6, 2025

Devessier added 3 commits January 6, 2025 11:27

refactor: prefer throwing a custom error

604de97

refactor: remove a useless default empty value

fdfbdb4

refactor: add missing break statement

f23424a

Devessier added 3 commits January 6, 2025 11:36

refactor: rename variables

183f7f0

refactor: rename the mutation

3e63951

lint: fix space errors

5ba02b6

Devessier added -PR: awaiting review and removed -PR: awaiting author labels Jan 6, 2025

thomtrp approved these changes Jan 6, 2025

View reviewed changes

Devessier merged commit 17bf2b6 into main Jan 6, 2025
23 checks passed

Devessier deleted the move-workflow-draft-version-override-to-backend branch January 6, 2025 13:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move the workflow draft version overriding to the backend #9328

Move the workflow draft version overriding to the backend #9328

Devessier commented Jan 2, 2025 •

edited

Loading

Devessier Jan 3, 2025

Devessier commented Jan 3, 2025

greptile-apps bot left a comment

greptile-apps bot Jan 3, 2025

greptile-apps bot Jan 3, 2025

greptile-apps bot Jan 3, 2025

martmull Jan 6, 2025

Devessier Jan 6, 2025

martmull Jan 6, 2025

greptile-apps bot Jan 3, 2025

greptile-apps bot Jan 3, 2025

greptile-apps bot Jan 3, 2025

greptile-apps bot Jan 3, 2025

greptile-apps bot Jan 3, 2025

martmull left a comment

martmull Jan 6, 2025

This comment was marked as duplicate.

Devessier commented Jan 6, 2025

thomtrp left a comment

thomtrp Jan 6, 2025

Devessier Jan 6, 2025

github-actions bot commented Jan 6, 2025

Move the workflow draft version overriding to the backend #9328

Move the workflow draft version overriding to the backend #9328

Conversation

Devessier commented Jan 2, 2025 • edited Loading

Devessier Jan 3, 2025

Choose a reason for hiding this comment

Devessier commented Jan 3, 2025

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Jan 3, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 3, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 3, 2025

Choose a reason for hiding this comment

martmull Jan 6, 2025

Choose a reason for hiding this comment

Devessier Jan 6, 2025

Choose a reason for hiding this comment

martmull Jan 6, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 3, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 3, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 3, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 3, 2025

Choose a reason for hiding this comment

greptile-apps bot Jan 3, 2025

Choose a reason for hiding this comment

martmull left a comment

Choose a reason for hiding this comment

martmull Jan 6, 2025

Choose a reason for hiding this comment

This comment was marked as duplicate.

Devessier commented Jan 6, 2025

thomtrp left a comment

Choose a reason for hiding this comment

thomtrp Jan 6, 2025

Choose a reason for hiding this comment

Devessier Jan 6, 2025

Choose a reason for hiding this comment

github-actions bot commented Jan 6, 2025

Log

Devessier commented Jan 2, 2025 •

edited

Loading