Add workflow email action #7279

Devessier · 2024-09-27T08:32:18Z

Add the SAVE_EMAIL action. This action requires more setting parameters than the Serverless Function action.
Changed the way we computed the workflow diagram. It now preserves some properties, like the selected property. That's necessary to not close the right drawer when the workflow back-end data change.
Added the possibility to set a label to a TextArea. This uses a <label> HTML element and the useId() hook to create an id linking the label with the input.

…ve properties like selected

greptile-apps

PR Summary

This pull request introduces significant enhancements to the workflow system, particularly focusing on adding a new 'SEND_EMAIL' action type. Here's a concise summary of the major changes:

Implemented a new 'SEND_EMAIL' action type with dedicated components for editing email actions
Enhanced the TextArea component with label support, improving accessibility
Refactored the workflow diagram generation and merging process to preserve UI state
Updated the WorkflowDiagramStepNode to visually distinguish between different action types
Introduced a reusable WorkflowEditActionFormBase component for consistent action form layouts
Added unit tests for the new mergeWorkflowDiagrams function to ensure correct functionality

These changes significantly expand the capabilities of the workflow system while improving code modularity and maintainability.

_{17 file(s) reviewed, 7 comment(s)}
_{Edit PR Review Bot Settings}

greptile-apps · 2024-09-27T16:06:30Z

packages/twenty-front/src/modules/ui/input/components/TextArea.tsx

+        value={value}
+        onChange={(event) =>
+          onChange?.(turnIntoEmptyStringIfWhitespacesOnly(event.target.value))
+        }


style: Consider memoizing this onChange callback to prevent unnecessary re-renders

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

greptile-apps · 2024-09-27T16:08:38Z

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

+              onChange={(email) => {
+                field.onChange(email);
+
+                handleSave();
+              }}


style: onChange prop receives 'email' but it's actually the subject. Rename for clarity

greptile-apps · 2024-09-27T16:08:39Z

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

+              onChange={(email) => {
+                field.onChange(email);
+
+                handleSave();
+              }}


style: onChange prop receives 'email' but it's actually the body. Rename for clarity

greptile-apps · 2024-09-27T16:12:15Z

packages/twenty-front/src/modules/workflow/utils/getStepDefaultDefinition.ts

+          subject: 'hello',
+          title: 'hello',
+          template: '{{title}}',


style: Consider using more descriptive placeholder values for subject, title, and template

greptile-apps · 2024-09-27T16:12:16Z

packages/twenty-front/src/modules/workflow/utils/getStepDefaultDefinition.ts

+        settings: {
+          subject: 'hello',
+          title: 'hello',
+          template: '{{title}}',
+          errorHandlingOptions: {
+            continueOnFailure: {
+              value: false,
+            },
+            retryOnFailure: {
+              value: false,
+            },
+          },
+        },


logic: Email recipient information is missing from the default settings

greptile-apps · 2024-09-27T16:12:38Z

packages/twenty-front/src/modules/workflow/utils/mergeWorkflowDiagrams.ts

+    const nodeWithPreservedProperties = nodePropertiesToPreserve.reduce(
+      (nodeToSet, propertyToPreserve) => {
+        return Object.assign(nodeToSet, {
+          [propertyToPreserve]: previousNode?.[propertyToPreserve],


logic: This may overwrite nextNode properties if they exist in nodePropertiesToPreserve. Ensure this is the intended behavior

thomtrp

Great work 🔥

Just a couple of comments. Also, to avoid spreading switch everywhere, could you put those into utils. So you can put all utils into the workflow folder

thomtrp · 2024-09-30T16:18:39Z

packages/twenty-front/src/modules/ui/input/components/TextArea.tsx

@@ -57,6 +73,8 @@ export const TextArea = ({
 }: TextAreaProps) => {
  const computedMinRows = Math.min(minRows, MAX_ROWS);

+  const inputId = useId();


we usually use v4(). Do we need an id that is not an uuid maybe?

I documented this change in my issue about using real <label> HTML elements for inputs. (#7281)

I saw that a UUID v4 was generated in a render function to serve as an input id:

twenty/packages/twenty-front/src/modules/ui/input/components/Checkbox.tsx

Line 168 in 2f41e14

const checkboxId = 'checkbox' + v4();

UUIDs are expensive to compute as cryptographically secure identifiers with low collision probabilities. However, these properties aren't necessary for an input's ID.

It's also better to have a consistent identifier that doesn't change every time. Assistive technologies like screen readers will behave better with a consistent identifier.

Furthermore, generating a UUID in a render function will re-compute it every time the component is rendered. As generating a UUID comes with a cost, it's better to either memoize it or not use an UUID.

The new useId() React hook returns a short and memoized ID with no collision probability. It also works well with server-side rendering, as the ID will be the same whether the component is rendered client-side or server-side.

It's important to mention that I only recommend using the useId() hook to generate DOM identifiers, like an input id. The useId() hook is not meant to generate other kinds of identifiers best served with a UUID v4.

thomtrp · 2024-09-30T16:36:44Z

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

-    />
-  );
+  if (stepDefinition.type === 'action') {
+    if (stepDefinition.definition.type === 'CODE') {


Let's use a switch, we will add more types soon

can you use the type from packages/twenty-front/src/modules/workflow/constants/Actions.ts

I will work on that tomorrow. Thanks.

… right drawer

thomtrp · 2024-10-01T10:18:52Z

packages/twenty-front/src/modules/workflow/utils/getStepDefaultDefinition.ts

@@ -26,6 +26,27 @@ export const getStepDefaultDefinition = (
        },
      };
    }
+    case 'SEND_EMAIL': {


packages/twenty-front/src/modules/workflow/utils/getStepDefaultDefinition.ts should be suffixed with .util.ts

thomtrp · 2024-10-01T10:19:27Z

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

@@ -33,11 +33,25 @@ export const WorkflowDiagramStepNode = ({
        );
      }
      case 'action': {


to be moved into an util next to the others

thomtrp · 2024-10-01T10:19:34Z

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

-        onTriggerUpdate={updateTrigger}
-      />
-    );
+  switch (stepDefinition.type) {


… type or an action type

Devessier · 2024-10-01T11:48:00Z

packages/twenty-front/src/modules/workflow/utils/assertUnreachable.ts

@@ -0,0 +1,3 @@
+export const assertUnreachable = (x: never, errorMessage?: string): never => {


The function is inspired by https://stackoverflow.com/questions/39419170/how-do-i-check-that-a-switch-block-is-exhaustive-in-typescript.

github-actions · 2024-10-01T12:23:20Z

Thanks @Devessier for your contribution!
This marks your 10th PR on the repo. You're top 3% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter

- Add the SAVE_EMAIL action. This action requires more setting parameters than the Serverless Function action. - Changed the way we computed the workflow diagram. It now preserves some properties, like the `selected` property. That's necessary to not close the right drawer when the workflow back-end data change. - Added the possibility to set a label to a TextArea. This uses a `<label>` HTML element and the `useId()` hook to create an id linking the label with the input.

Devessier added 10 commits September 27, 2024 17:26

refactor: prepare the codebase to receive multiple action types

6572f0c

feat: add label property to textarea component

4cd8220

feat: implement send email action

c748929

fix: temporary disable closing the drawer when a node is unselected

229db53

feat: merge the previous workflow diagram with the next one to preser…

59eb7a5

…ve properties like selected

test: write test for the mergeWorkflowDiagrams

ecb90cd

feat: use a label html component for TextArea

f7ac0c2

refactor: throw when encountering invalid step definition

21b76a8

fix: preserve only properties we are interested in to prevent bugs

eeda2d6

feat: be sure the settings are saved when exiting the drawer

b0b39f5

Devessier force-pushed the add-workflow-email-action branch from efb505d to b0b39f5 Compare September 27, 2024 15:26

Devessier marked this pull request as ready for review September 27, 2024 16:06

greptile-apps bot reviewed Sep 27, 2024

View reviewed changes

thomtrp reviewed Sep 30, 2024

View reviewed changes

refactor: use switch statement to decide which form to display on the…

3c91a83

… right drawer

thomtrp reviewed Oct 1, 2024

View reviewed changes

refactor: ensure there is a compilation-time error when we add a step…

7249349

… type or an action type

Devessier commented Oct 1, 2024

View reviewed changes

thomtrp approved these changes Oct 1, 2024

View reviewed changes

test: fix failing workflow test

c1ec445

Devessier merged commit cde255a into main Oct 1, 2024
7 of 13 checks passed

Devessier deleted the add-workflow-email-action branch October 1, 2024 12:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add workflow email action #7279

Add workflow email action #7279

Devessier commented Sep 27, 2024 •

edited

Loading

greptile-apps bot left a comment

greptile-apps bot Sep 27, 2024

greptile-apps bot Sep 27, 2024

greptile-apps bot Sep 27, 2024

greptile-apps bot Sep 27, 2024

greptile-apps bot Sep 27, 2024

greptile-apps bot Sep 27, 2024

thomtrp left a comment

thomtrp Sep 30, 2024

Devessier Sep 30, 2024 •

edited

Loading

thomtrp Sep 30, 2024

thomtrp Sep 30, 2024

Devessier Sep 30, 2024

Devessier Oct 1, 2024

thomtrp Oct 1, 2024

thomtrp Oct 1, 2024

thomtrp Oct 1, 2024

Devessier Oct 1, 2024

github-actions bot commented Oct 1, 2024

		@@ -0,0 +1,3 @@
		export const assertUnreachable = (x: never, errorMessage?: string): never => {

Add workflow email action #7279

Add workflow email action #7279

Conversation

Devessier commented Sep 27, 2024 • edited Loading

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Sep 27, 2024

Choose a reason for hiding this comment

greptile-apps bot Sep 27, 2024

Choose a reason for hiding this comment

greptile-apps bot Sep 27, 2024

Choose a reason for hiding this comment

greptile-apps bot Sep 27, 2024

Choose a reason for hiding this comment

greptile-apps bot Sep 27, 2024

Choose a reason for hiding this comment

greptile-apps bot Sep 27, 2024

Choose a reason for hiding this comment

thomtrp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Devessier Sep 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 1, 2024

Devessier commented Sep 27, 2024 •

edited

Loading

Devessier Sep 30, 2024 •

edited

Loading