Fix CSV import upsert (#12048)

Fixes https://github.com/twentyhq/twenty/issues/11864 and
https://github.com/twentyhq/core-team-issues/issues/908

We should not send `createManyXXX` mutations with FE-forged ids in the
payload if we want to do an upsert, because that 1) prevents records
from being merged 2) triggers optimistic rendering while we can't know
before-hand which records will actually be created and which records
will only be updated

Also noticed createdBy was being overriden even for records we are
updating and not creating, which did not seem right, so fixed that too

packages/twenty-front/src/modules/object-record/hooks/__tests__/useCreateManyRecords.test.tsx

@@ -40,6 +40,20 @@ const mocks = [
       },
     })),
   },
+  {
+    request: {
+      query,
+      variables: {
+        data: input,
+        upsert: true,
+      },
+    },
+    result: jest.fn(() => ({
+      data: {
+        createPeople: response,
+      },
+    })),
+  },
 ];
 
 const Wrapper = getJestMetadataAndApolloMocksWrapper({
@@ -69,4 +83,28 @@ describe('useCreateManyRecords', () => {
     expect(mocks[0].result).toHaveBeenCalled();
     expect(mockRefetchAggregateQueries).toHaveBeenCalledTimes(1);
   });
+
+  it('does not indicate id in request variables when upsert is true because we cant know if it will be an insert or an update', async () => {
+    const { result } = renderHook(
+      () =>
+        useCreateManyRecords({
+          objectNameSingular: CoreObjectNameSingular.Person,
+        }),
+      {
+        wrapper: Wrapper,
+      },
+    );
+
+    await act(async () => {
+      const res = await result.current.createManyRecords(input, true);
+      expect(res).toEqual(response);
+    });
+
+    // Verify that the mutation was called with data without IDs
+    expect(mocks[1].request.variables.data).toEqual(input);
+    mocks[1].request.variables.data.forEach((record: any) => {
+      expect(record).not.toHaveProperty('id');
+    });
+    expect(mockRefetchAggregateQueries).toHaveBeenCalledTimes(1);
+  });
 });

packages/twenty-front/src/modules/object-record/hooks/useCreateManyRecords.ts

@@ -27,6 +27,10 @@ type PartialObjectRecordWithId = Partial<ObjectRecord> & {
   id: string;
 };
 
+type PartialObjectRecordWithOptionalId = Partial<ObjectRecord> & {
+  id?: string;
+};
+
 type useCreateManyRecordsProps = {
   objectNameSingular: string;
   recordGqlFields?: RecordGqlOperationGqlRecordFields;
@@ -75,16 +79,20 @@ export const useCreateManyRecords = <
     recordsToCreate: Partial<CreatedObjectRecord>[],
     upsert?: boolean,
   ) => {
-    const sanitizedCreateManyRecordsInput: PartialObjectRecordWithId[] = [];
+    const sanitizedCreateManyRecordsInput: PartialObjectRecordWithOptionalId[] =
+      [];
     const recordOptimisticRecordsInput: PartialObjectRecordWithId[] = [];
     recordsToCreate.forEach((recordToCreate) => {
-      const idForCreation = recordToCreate?.id ?? v4();
+      const shouldDoOptimisticEffect = upsert !== true;
+      const idForCreation = shouldDoOptimisticEffect
+        ? (recordToCreate?.id ?? v4())
+        : undefined;
       const sanitizedRecord = {
         ...sanitizeRecordInput({
           objectMetadataItem,
           recordInput: recordToCreate,
         }),
-        id: idForCreation,
+        ...(isDefined(idForCreation) ? { id: idForCreation } : {}),
       };
       const baseOptimisticRecordInputCreatedBy:
         | { createdBy: FieldActorForInputValue }
@@ -96,22 +104,25 @@ export const useCreateManyRecords = <
             },
           }
         : undefined;
-      const optimisticRecordInput = {
-        ...computeOptimisticRecordFromInput({
-          cache: apolloClient.cache,
-          objectMetadataItem,
-          objectMetadataItems,
-          currentWorkspaceMember: currentWorkspaceMember,
-          recordInput: {
-            ...baseOptimisticRecordInputCreatedBy,
-            ...recordToCreate,
-          },
-        }),
-        id: idForCreation,
-      };
 
       sanitizedCreateManyRecordsInput.push(sanitizedRecord);
-      recordOptimisticRecordsInput.push(optimisticRecordInput);
+
+      if (shouldDoOptimisticEffect) {
+        const optimisticRecordInput = {
+          ...computeOptimisticRecordFromInput({
+            cache: apolloClient.cache,
+            objectMetadataItem,
+            objectMetadataItems,
+            currentWorkspaceMember: currentWorkspaceMember,
+            recordInput: {
+              ...baseOptimisticRecordInputCreatedBy,
+              ...recordToCreate,
+            },
+          }),
+          id: idForCreation as string,
+        };
+        recordOptimisticRecordsInput.push(optimisticRecordInput);
+      }
     });
 
     const recordsCreatedInCache = recordOptimisticRecordsInput

packages/twenty-server/src/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-create-many-resolver.service.ts

@@ -266,15 +266,20 @@ export class GraphqlQueryCreateManyResolverService extends GraphqlQueryBaseResol
     for (const record of recordsToUpdate) {
       const recordId = record.id as string;
 
-      // TODO: we should align update and insert
-      // For insert, formating is done in the server
-      // While for update, formatting is done at the resolver level
-
-      const formattedRecord = formatData(
+      // we should not update an existing record's createdBy value
+      const recordWithoutCreatedByUpdate = this.getRecordWithoutCreatedBy(
         record,
         objectMetadataItemWithFieldMaps,
       );
 
+      const formattedRecord = formatData(
+        recordWithoutCreatedByUpdate,
+        objectMetadataItemWithFieldMaps,
+      );
+
+      // TODO: we should align update and insert
+      // For insert, formating is done in the server
+      // While for update, formatting is done at the resolver level
       await repository.update(recordId, formattedRecord);
       result.identifiers.push({ id: recordId });
       result.generatedMaps.push({ id: recordId });
@@ -362,6 +367,25 @@ export class GraphqlQueryCreateManyResolverService extends GraphqlQueryBaseResol
     );
   }
 
+  private getRecordWithoutCreatedBy(
+    record: Partial<ObjectRecord>,
+    objectMetadataItemWithFieldMaps: ObjectMetadataItemWithFieldMaps,
+  ) {
+    let recordWithoutCreatedByUpdate = record;
+
+    if (
+      'createdBy' in record &&
+      objectMetadataItemWithFieldMaps.fieldsByName['createdBy']?.isCustom ===
+        false
+    ) {
+      const { createdBy: _createdBy, ...recordWithoutCreatedBy } = record;
+
+      recordWithoutCreatedByUpdate = recordWithoutCreatedBy;
+    }
+
+    return recordWithoutCreatedByUpdate;
+  }
+
   async validate<T extends ObjectRecord>(
     args: CreateManyResolverArgs<Partial<T>>,
     options: WorkspaceQueryRunnerOptions,

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

@@ -1,5 +1,5 @@
-import { capitalize } from 'twenty-shared/utils';
 import { FieldMetadataType } from 'twenty-shared/types';
+import { capitalize } from 'twenty-shared/utils';
 
 import { FieldMetadataInterface } from 'src/engine/metadata-modules/field-metadata/interfaces/field-metadata.interface';