From c133129eb09918185f748cbdcc06552933e63906 Mon Sep 17 00:00:00 2001
From: Baptiste Devessier <baptiste@devessier.fr>
Date: Wed, 20 Nov 2024 10:15:55 +0100
Subject: [PATCH] Fix state mutation serverless action (#8580)

In this PR:

- Ensure the `setNestedValue` does a deep copy of the provided object
and doesn't mutate it directly.
- Store the form's state in a `useState` instead of relying entirely on
the backend's state. This makes the form more resilient to slow network
connections.
- Ensure the input settings are reset when selecting another function.
- The Inner component now expects the serverless functions data to be
resolved before being mounted, so I split it.

Closes https://github.com/twentyhq/twenty/issues/8523
---
 .../hooks/useGetManyServerlessFunctions.ts    |  10 +-
 ...rkflowEditActionFormServerlessFunction.tsx | 205 +--------------
 ...wEditActionFormServerlessFunctionInner.tsx | 239 ++++++++++++++++++
 .../utils/__tests__/setNestedValue.test.ts    |  15 ++
 .../modules/workflow/utils/setNestedValue.ts  |   2 +-
 5 files changed, 272 insertions(+), 199 deletions(-)
 create mode 100644 packages/twenty-front/src/modules/workflow/components/WorkflowEditActionFormServerlessFunctionInner.tsx

diff --git a/packages/twenty-front/src/modules/settings/serverless-functions/hooks/useGetManyServerlessFunctions.ts b/packages/twenty-front/src/modules/settings/serverless-functions/hooks/useGetManyServerlessFunctions.ts
index 4023745907..46226987f0 100644
--- a/packages/twenty-front/src/modules/settings/serverless-functions/hooks/useGetManyServerlessFunctions.ts
+++ b/packages/twenty-front/src/modules/settings/serverless-functions/hooks/useGetManyServerlessFunctions.ts
@@ -1,6 +1,6 @@
-import { useQuery } from '@apollo/client';
-import { FIND_MANY_SERVERLESS_FUNCTIONS } from '@/settings/serverless-functions/graphql/queries/findManyServerlessFunctions';
 import { useApolloMetadataClient } from '@/object-metadata/hooks/useApolloMetadataClient';
+import { FIND_MANY_SERVERLESS_FUNCTIONS } from '@/settings/serverless-functions/graphql/queries/findManyServerlessFunctions';
+import { useQuery } from '@apollo/client';
 import {
   GetManyServerlessFunctionsQuery,
   GetManyServerlessFunctionsQueryVariables,
@@ -8,13 +8,17 @@ import {
 
 export const useGetManyServerlessFunctions = () => {
   const apolloMetadataClient = useApolloMetadataClient();
-  const { data } = useQuery<
+
+  const { data, loading, error } = useQuery<
     GetManyServerlessFunctionsQuery,
     GetManyServerlessFunctionsQueryVariables
   >(FIND_MANY_SERVERLESS_FUNCTIONS, {
     client: apolloMetadataClient ?? undefined,
   });
+
   return {
     serverlessFunctions: data?.findManyServerlessFunctions || [],
+    loading,
+    error,
   };
 };
diff --git a/packages/twenty-front/src/modules/workflow/components/WorkflowEditActionFormServerlessFunction.tsx b/packages/twenty-front/src/modules/workflow/components/WorkflowEditActionFormServerlessFunction.tsx
index ea6ef3281c..c1b17b8100 100644
--- a/packages/twenty-front/src/modules/workflow/components/WorkflowEditActionFormServerlessFunction.tsx
+++ b/packages/twenty-front/src/modules/workflow/components/WorkflowEditActionFormServerlessFunction.tsx
@@ -1,41 +1,6 @@
-import { ReactNode, Fragment } from 'react';
-import styled from '@emotion/styled';
 import { useGetManyServerlessFunctions } from '@/settings/serverless-functions/hooks/useGetManyServerlessFunctions';
-import { Select, SelectOption } from '@/ui/input/components/Select';
-import { WorkflowEditGenericFormBase } from '@/workflow/components/WorkflowEditGenericFormBase';
-import VariableTagInput from '@/workflow/search-variables/components/VariableTagInput';
-import { FunctionInput } from '@/workflow/types/FunctionInput';
+import { WorkflowEditActionFormServerlessFunctionInner } from '@/workflow/components/WorkflowEditActionFormServerlessFunctionInner';
 import { WorkflowCodeAction } from '@/workflow/types/Workflow';
-import { getDefaultFunctionInputFromInputSchema } from '@/workflow/utils/getDefaultFunctionInputFromInputSchema';
-import { mergeDefaultFunctionInputAndFunctionInput } from '@/workflow/utils/mergeDefaultFunctionInputAndFunctionInput';
-import { setNestedValue } from '@/workflow/utils/setNestedValue';
-import { useTheme } from '@emotion/react';
-import { HorizontalSeparator, IconCode, isDefined } from 'twenty-ui';
-import { useDebouncedCallback } from 'use-debounce';
-
-const StyledContainer = styled.div`
-  display: inline-flex;
-  flex-direction: column;
-`;
-
-const StyledLabel = styled.div`
-  color: ${({ theme }) => theme.font.color.light};
-  font-size: ${({ theme }) => theme.font.size.md};
-  font-weight: ${({ theme }) => theme.font.weight.semiBold};
-  margin-top: ${({ theme }) => theme.spacing(2)};
-  margin-bottom: ${({ theme }) => theme.spacing(2)};
-`;
-
-const StyledInputContainer = styled.div`
-  background: ${({ theme }) => theme.background.secondary};
-  border: 1px solid ${({ theme }) => theme.border.color.medium};
-  border-radius: ${({ theme }) => theme.border.radius.md};
-  display: flex;
-  flex-direction: column;
-  gap: ${({ theme }) => theme.spacing(2)};
-  padding: ${({ theme }) => theme.spacing(2)};
-  position: relative;
-`;
 
 type WorkflowEditActionFormServerlessFunctionProps = {
   action: WorkflowCodeAction;
@@ -53,167 +18,17 @@ export const WorkflowEditActionFormServerlessFunction = ({
   action,
   actionOptions,
 }: WorkflowEditActionFormServerlessFunctionProps) => {
-  const theme = useTheme();
-  const { serverlessFunctions } = useGetManyServerlessFunctions();
+  const { loading: isLoadingServerlessFunctions } =
+    useGetManyServerlessFunctions();
 
-  const getFunctionInput = (serverlessFunctionId: string) => {
-    if (!serverlessFunctionId) {
-      return {};
-    }
-
-    const serverlessFunction = serverlessFunctions.find(
-      (f) => f.id === serverlessFunctionId,
-    );
-    const inputSchema = serverlessFunction?.latestVersionInputSchema;
-    const defaultFunctionInput =
-      getDefaultFunctionInputFromInputSchema(inputSchema);
-
-    const existingFunctionInput = action.settings.input.serverlessFunctionInput;
-
-    return mergeDefaultFunctionInputAndFunctionInput({
-      defaultFunctionInput,
-      functionInput: existingFunctionInput,
-    });
-  };
-
-  const functionInput = getFunctionInput(
-    action.settings.input.serverlessFunctionId,
-  );
-
-  const updateFunctionInput = useDebouncedCallback(
-    async (newFunctionInput: object) => {
-      if (actionOptions.readonly === true) {
-        return;
-      }
-
-      actionOptions.onActionUpdate({
-        ...action,
-        settings: {
-          ...action.settings,
-          input: {
-            ...action.settings.input,
-            serverlessFunctionInput: newFunctionInput,
-          },
-        },
-      });
-    },
-    1_000,
-  );
-
-  const handleInputChange = (value: any, path: string[]) => {
-    updateFunctionInput(setNestedValue(functionInput, path, value));
-  };
-
-  const availableFunctions: Array<SelectOption<string>> = [
-    ...serverlessFunctions
-      .filter((serverlessFunction) =>
-        isDefined(serverlessFunction.latestVersion),
-      )
-      .map((serverlessFunction) => ({
-        label: serverlessFunction.name,
-        value: serverlessFunction.id,
-        latestVersionInputSchema: serverlessFunction.latestVersionInputSchema,
-      })),
-  ];
-
-  const handleFunctionChange = (newServerlessFunctionId: string) => {
-    if (actionOptions.readonly === true) {
-      return;
-    }
-
-    const serverlessFunction = serverlessFunctions.find(
-      (f) => f.id === newServerlessFunctionId,
-    );
-
-    const newProps = {
-      ...action,
-      settings: {
-        ...action.settings,
-        input: {
-          serverlessFunctionId: newServerlessFunctionId,
-          serverlessFunctionVersion:
-            serverlessFunction?.latestVersion || 'latest',
-          serverlessFunctionInput: getFunctionInput(newServerlessFunctionId),
-        },
-      },
-    };
-
-    actionOptions.onActionUpdate(newProps);
-  };
-
-  const renderFields = (
-    functionInput: FunctionInput,
-    path: string[] = [],
-    isRoot = true,
-  ): ReactNode[] => {
-    const displaySeparator = (functionInput: FunctionInput) => {
-      const keys = Object.keys(functionInput);
-      if (keys.length > 1) {
-        return true;
-      }
-      if (keys.length === 1) {
-        const subKeys = Object.keys(functionInput[keys[0]]);
-        return subKeys.length > 0;
-      }
-      return false;
-    };
-
-    return Object.entries(functionInput).map(([inputKey, inputValue]) => {
-      const currentPath = [...path, inputKey];
-      const pathKey = currentPath.join('.');
-
-      if (inputValue !== null && typeof inputValue === 'object') {
-        if (isRoot) {
-          return (
-            <Fragment key={pathKey}>
-              {displaySeparator(functionInput) && (
-                <HorizontalSeparator noMargin />
-              )}
-              {renderFields(inputValue, currentPath, false)}
-            </Fragment>
-          );
-        }
-        return (
-          <StyledContainer key={pathKey}>
-            <StyledLabel>{inputKey}</StyledLabel>
-            <StyledInputContainer>
-              {renderFields(inputValue, currentPath, false)}
-            </StyledInputContainer>
-          </StyledContainer>
-        );
-      } else {
-        return (
-          <VariableTagInput
-            key={pathKey}
-            inputId={`input-${inputKey}`}
-            label={inputKey}
-            placeholder="Enter value"
-            value={`${inputValue || ''}`}
-            onChange={(value) => handleInputChange(value, currentPath)}
-            readonly={actionOptions.readonly}
-          />
-        );
-      }
-    });
-  };
+  if (isLoadingServerlessFunctions) {
+    return null;
+  }
 
   return (
-    <WorkflowEditGenericFormBase
-      HeaderIcon={<IconCode color={theme.color.orange} />}
-      headerTitle="Code - Serverless Function"
-      headerType="Code"
-    >
-      <Select
-        dropdownId="select-serverless-function-id"
-        label="Function"
-        fullWidth
-        value={action.settings.input.serverlessFunctionId}
-        options={availableFunctions}
-        emptyOption={{ label: 'None', value: '' }}
-        disabled={actionOptions.readonly}
-        onChange={handleFunctionChange}
-      />
-      {renderFields(functionInput)}
-    </WorkflowEditGenericFormBase>
+    <WorkflowEditActionFormServerlessFunctionInner
+      action={action}
+      actionOptions={actionOptions}
+    />
   );
 };
diff --git a/packages/twenty-front/src/modules/workflow/components/WorkflowEditActionFormServerlessFunctionInner.tsx b/packages/twenty-front/src/modules/workflow/components/WorkflowEditActionFormServerlessFunctionInner.tsx
new file mode 100644
index 0000000000..1779f15932
--- /dev/null
+++ b/packages/twenty-front/src/modules/workflow/components/WorkflowEditActionFormServerlessFunctionInner.tsx
@@ -0,0 +1,239 @@
+import { useGetManyServerlessFunctions } from '@/settings/serverless-functions/hooks/useGetManyServerlessFunctions';
+import { Select, SelectOption } from '@/ui/input/components/Select';
+import { WorkflowEditGenericFormBase } from '@/workflow/components/WorkflowEditGenericFormBase';
+import VariableTagInput from '@/workflow/search-variables/components/VariableTagInput';
+import { FunctionInput } from '@/workflow/types/FunctionInput';
+import { WorkflowCodeAction } from '@/workflow/types/Workflow';
+import { getDefaultFunctionInputFromInputSchema } from '@/workflow/utils/getDefaultFunctionInputFromInputSchema';
+import { mergeDefaultFunctionInputAndFunctionInput } from '@/workflow/utils/mergeDefaultFunctionInputAndFunctionInput';
+import { setNestedValue } from '@/workflow/utils/setNestedValue';
+import { useTheme } from '@emotion/react';
+import styled from '@emotion/styled';
+import { Fragment, ReactNode, useState } from 'react';
+import { HorizontalSeparator, IconCode, isDefined } from 'twenty-ui';
+import { useDebouncedCallback } from 'use-debounce';
+
+const StyledContainer = styled.div`
+  display: inline-flex;
+  flex-direction: column;
+`;
+
+const StyledLabel = styled.div`
+  color: ${({ theme }) => theme.font.color.light};
+  font-size: ${({ theme }) => theme.font.size.md};
+  font-weight: ${({ theme }) => theme.font.weight.semiBold};
+  margin-top: ${({ theme }) => theme.spacing(2)};
+  margin-bottom: ${({ theme }) => theme.spacing(2)};
+`;
+
+const StyledInputContainer = styled.div`
+  background: ${({ theme }) => theme.background.secondary};
+  border: 1px solid ${({ theme }) => theme.border.color.medium};
+  border-radius: ${({ theme }) => theme.border.radius.md};
+  display: flex;
+  flex-direction: column;
+  gap: ${({ theme }) => theme.spacing(2)};
+  padding: ${({ theme }) => theme.spacing(2)};
+  position: relative;
+`;
+
+type WorkflowEditActionFormServerlessFunctionInnerProps = {
+  action: WorkflowCodeAction;
+  actionOptions:
+    | {
+        readonly: true;
+      }
+    | {
+        readonly?: false;
+        onActionUpdate: (action: WorkflowCodeAction) => void;
+      };
+};
+
+type ServerlessFunctionInputFormData = {
+  [field: string]: string | ServerlessFunctionInputFormData;
+};
+
+export const WorkflowEditActionFormServerlessFunctionInner = ({
+  action,
+  actionOptions,
+}: WorkflowEditActionFormServerlessFunctionInnerProps) => {
+  const theme = useTheme();
+
+  const { serverlessFunctions } = useGetManyServerlessFunctions();
+
+  const getFunctionInput = (serverlessFunctionId: string) => {
+    if (!serverlessFunctionId) {
+      return {};
+    }
+
+    const serverlessFunction = serverlessFunctions.find(
+      (f) => f.id === serverlessFunctionId,
+    );
+    const inputSchema = serverlessFunction?.latestVersionInputSchema;
+    const defaultFunctionInput =
+      getDefaultFunctionInputFromInputSchema(inputSchema);
+
+    return defaultFunctionInput;
+  };
+
+  const [selectedFunctionId, setSelectedFunctionId] = useState(
+    action.settings.input.serverlessFunctionId,
+  );
+
+  const [functionInput, setFunctionInput] =
+    useState<ServerlessFunctionInputFormData>(
+      mergeDefaultFunctionInputAndFunctionInput({
+        defaultFunctionInput: getFunctionInput(selectedFunctionId),
+        functionInput: action.settings.input.serverlessFunctionInput,
+      }),
+    );
+
+  const updateFunctionInput = useDebouncedCallback(
+    async (newFunctionInput: object) => {
+      if (actionOptions.readonly === true) {
+        return;
+      }
+
+      actionOptions.onActionUpdate({
+        ...action,
+        settings: {
+          ...action.settings,
+          input: {
+            ...action.settings.input,
+            serverlessFunctionInput: newFunctionInput,
+          },
+        },
+      });
+    },
+    1_000,
+  );
+
+  const handleInputChange = (value: any, path: string[]) => {
+    const updatedFunctionInput = setNestedValue(functionInput, path, value);
+
+    setFunctionInput(updatedFunctionInput);
+
+    updateFunctionInput(updatedFunctionInput);
+  };
+
+  const availableFunctions: Array<SelectOption<string>> = [
+    ...serverlessFunctions
+      .filter((serverlessFunction) =>
+        isDefined(serverlessFunction.latestVersion),
+      )
+      .map((serverlessFunction) => ({
+        label: serverlessFunction.name,
+        value: serverlessFunction.id,
+        latestVersionInputSchema: serverlessFunction.latestVersionInputSchema,
+      })),
+  ];
+
+  const handleFunctionChange = (newServerlessFunctionId: string) => {
+    if (actionOptions.readonly === true) {
+      return;
+    }
+
+    updateFunctionInput.cancel();
+
+    setSelectedFunctionId(newServerlessFunctionId);
+
+    const serverlessFunction = serverlessFunctions.find(
+      (f) => f.id === newServerlessFunctionId,
+    );
+
+    const newFunctionInput = getFunctionInput(newServerlessFunctionId);
+
+    const newProps = {
+      ...action,
+      settings: {
+        ...action.settings,
+        input: {
+          serverlessFunctionId: newServerlessFunctionId,
+          serverlessFunctionVersion:
+            serverlessFunction?.latestVersion || 'latest',
+          serverlessFunctionInput: newFunctionInput,
+        },
+      },
+    };
+
+    setFunctionInput(newFunctionInput);
+
+    actionOptions.onActionUpdate(newProps);
+  };
+
+  const renderFields = (
+    functionInput: FunctionInput,
+    path: string[] = [],
+    isRoot = true,
+  ): ReactNode[] => {
+    const displaySeparator = (functionInput: FunctionInput) => {
+      const keys = Object.keys(functionInput);
+      if (keys.length > 1) {
+        return true;
+      }
+      if (keys.length === 1) {
+        const subKeys = Object.keys(functionInput[keys[0]]);
+        return subKeys.length > 0;
+      }
+      return false;
+    };
+
+    return Object.entries(functionInput).map(([inputKey, inputValue]) => {
+      const currentPath = [...path, inputKey];
+      const pathKey = currentPath.join('.');
+
+      if (inputValue !== null && typeof inputValue === 'object') {
+        if (isRoot) {
+          return (
+            <Fragment key={pathKey}>
+              {displaySeparator(functionInput) && (
+                <HorizontalSeparator noMargin />
+              )}
+              {renderFields(inputValue, currentPath, false)}
+            </Fragment>
+          );
+        }
+        return (
+          <StyledContainer key={pathKey}>
+            <StyledLabel>{inputKey}</StyledLabel>
+            <StyledInputContainer>
+              {renderFields(inputValue, currentPath, false)}
+            </StyledInputContainer>
+          </StyledContainer>
+        );
+      } else {
+        return (
+          <VariableTagInput
+            key={pathKey}
+            inputId={`input-${inputKey}`}
+            label={inputKey}
+            placeholder="Enter value"
+            readonly={actionOptions.readonly}
+            value={`${inputValue || ''}`}
+            onChange={(value) => handleInputChange(value, currentPath)}
+          />
+        );
+      }
+    });
+  };
+
+  return (
+    <WorkflowEditGenericFormBase
+      HeaderIcon={<IconCode color={theme.color.orange} />}
+      headerTitle="Code - Serverless Function"
+      headerType="Code"
+    >
+      <Select
+        dropdownId="select-serverless-function-id"
+        label="Function"
+        fullWidth
+        value={selectedFunctionId}
+        options={availableFunctions}
+        emptyOption={{ label: 'None', value: '' }}
+        disabled={actionOptions.readonly}
+        onChange={handleFunctionChange}
+      />
+      {renderFields(functionInput)}
+    </WorkflowEditGenericFormBase>
+  );
+};
diff --git a/packages/twenty-front/src/modules/workflow/utils/__tests__/setNestedValue.test.ts b/packages/twenty-front/src/modules/workflow/utils/__tests__/setNestedValue.test.ts
index 1c2bf307fc..87386c793c 100644
--- a/packages/twenty-front/src/modules/workflow/utils/__tests__/setNestedValue.test.ts
+++ b/packages/twenty-front/src/modules/workflow/utils/__tests__/setNestedValue.test.ts
@@ -8,4 +8,19 @@ describe('setNestedValue', () => {
     const expectedResult = { a: { b: newValue } };
     expect(setNestedValue(obj, path, newValue)).toEqual(expectedResult);
   });
+
+  it('should not mutate the initial object', () => {
+    const expectedObject = { a: { b: 'b' } };
+
+    const initialObject = structuredClone(expectedObject);
+    const path = ['a', 'b'];
+    const newValue = 'bb';
+
+    const updatedObject = setNestedValue(initialObject, path, newValue);
+
+    expect(initialObject).toEqual(expectedObject);
+
+    expect(updatedObject).not.toBe(initialObject);
+    expect(updatedObject.a).not.toBe(initialObject.a);
+  });
 });
diff --git a/packages/twenty-front/src/modules/workflow/utils/setNestedValue.ts b/packages/twenty-front/src/modules/workflow/utils/setNestedValue.ts
index a3b351cfbc..340439eb4d 100644
--- a/packages/twenty-front/src/modules/workflow/utils/setNestedValue.ts
+++ b/packages/twenty-front/src/modules/workflow/utils/setNestedValue.ts
@@ -1,5 +1,5 @@
 export const setNestedValue = (obj: any, path: string[], value: any) => {
-  const newObj = { ...obj };
+  const newObj = structuredClone(obj);
   path.reduce((o, key, index) => {
     if (index === path.length - 1) {
       o[key] = value;