Refactor triggerUpdateRelationsOptimisticEffect to compute relationship from Metadatas

packages/twenty-front/src/modules/apollo/optimistic-effect/utils/triggerDestroyRecordsOptimisticEffect.ts

@@ -4,7 +4,7 @@ import { triggerUpdateRelationsOptimisticEffect } from '@/apollo/optimistic-effe
 import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem';
 import { RecordGqlRefEdge } from '@/object-record/cache/types/RecordGqlRefEdge';
 import { isObjectRecordConnectionWithRefs } from '@/object-record/cache/utils/isObjectRecordConnectionWithRefs';
-import { RecordGqlNode } from '@/object-record/graphql/types/RecordGqlNode';
+import { ObjectRecord } from '@/object-record/types/ObjectRecord';
 import { isDefined } from '~/utils/isDefined';
 
 export const triggerDestroyRecordsOptimisticEffect = ({
@@ -15,7 +15,8 @@ export const triggerDestroyRecordsOptimisticEffect = ({
 }: {
   cache: ApolloCache<unknown>;
   objectMetadataItem: ObjectMetadataItem;
-  recordsToDestroy: RecordGqlNode[];
+  // TODO refactor this should consume RecordObject[]
+  recordsToDestroy: ObjectRecord[];
   objectMetadataItems: ObjectMetadataItem[];
 }) => {
   cache.modify<StoreObject>({

packages/twenty-front/src/modules/apollo/optimistic-effect/utils/triggerUpdateRelationsOptimisticEffect.ts

@@ -1,32 +1,36 @@
-import { ApolloCache } from '@apollo/client';
 
 import { triggerAttachRelationOptimisticEffect } from '@/apollo/optimistic-effect/utils/triggerAttachRelationOptimisticEffect';
 import { triggerDestroyRecordsOptimisticEffect } from '@/apollo/optimistic-effect/utils/triggerDestroyRecordsOptimisticEffect';
 import { triggerDetachRelationOptimisticEffect } from '@/apollo/optimistic-effect/utils/triggerDetachRelationOptimisticEffect';
 import { CORE_OBJECT_NAMES_TO_DELETE_ON_TRIGGER_RELATION_DETACH } from '@/apollo/types/coreObjectNamesToDeleteOnRelationDetach';
 import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular';
 import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem';
+import { getRecordFromRecordNode } from '@/object-record/cache/utils/getRecordFromRecordNode';
+import { getRecordsFromRecordConnection } from '@/object-record/cache/utils/getRecordsFromRecordConnection';
 import { isObjectRecordConnection } from '@/object-record/cache/utils/isObjectRecordConnection';
 import { RecordGqlConnection } from '@/object-record/graphql/types/RecordGqlConnection';
 import { RecordGqlNode } from '@/object-record/graphql/types/RecordGqlNode';
 import { ObjectRecord } from '@/object-record/types/ObjectRecord';
+import { ApolloCache } from '@apollo/client';
+import { isArray } from '@sniptt/guards';
 import { FieldMetadataType } from '~/generated-metadata/graphql';
 import { isDeeplyEqual } from '~/utils/isDeeplyEqual';
 import { isDefined } from '~/utils/isDefined';
 
+type triggerUpdateRelationsOptimisticEffectArgs = {
+  cache: ApolloCache<unknown>;
+  sourceObjectMetadataItem: ObjectMetadataItem;
+  currentSourceRecord: ObjectRecord | null;
+  updatedSourceRecord: ObjectRecord | null;
+  objectMetadataItems: ObjectMetadataItem[];
+};
 export const triggerUpdateRelationsOptimisticEffect = ({
   cache,
   sourceObjectMetadataItem,
   currentSourceRecord,
   updatedSourceRecord,
   objectMetadataItems,
-}: {
-  cache: ApolloCache<unknown>;
-  sourceObjectMetadataItem: ObjectMetadataItem;
-  currentSourceRecord: ObjectRecord | null;
-  updatedSourceRecord: ObjectRecord | null;
-  objectMetadataItems: ObjectMetadataItem[];
-}) => {
+}: triggerUpdateRelationsOptimisticEffectArgs) => {
   return sourceObjectMetadataItem.fields.forEach(
     (fieldMetadataItemOnSourceRecord) => {
       const notARelationField =
@@ -81,79 +85,65 @@ export const triggerUpdateRelationsOptimisticEffect = ({
       ) {
         return;
       }
+      const extractTargetRecordsFromRelation = (
+        value: RecordGqlConnection | RecordGqlNode | null,
+      ): ObjectRecord[] => {
+        // TODO investigate on the root cause of empty array injection here, should never occurs
+        // Cache might be corrupted somewhere
+        if (!isDefined(value) || isArray(value)) {
+          return [];
+        }
 
-      // TODO: replace this by a relation type check, if it's one to many,
-      //   it's an object record connection (we can still check it though as a safeguard)
-      const currentFieldValueOnSourceRecordIsARecordConnection =
-        isObjectRecordConnection(
-          targetObjectMetadata.nameSingular,
-          currentFieldValueOnSourceRecord,
-        );
+        if (isObjectRecordConnection(relationDefinition, value)) {
+          return getRecordsFromRecordConnection({recordConnection: value})
+        }
 
-      const targetRecordsToDetachFrom =
-        currentFieldValueOnSourceRecordIsARecordConnection
-          ? currentFieldValueOnSourceRecord.edges.map(
-              ({ node }) => node as RecordGqlNode,
-            )
-          : [currentFieldValueOnSourceRecord].filter(isDefined);
+        return [getRecordFromRecordNode({recordNode: value})];
+      };
+      const targetRecordsToDetachFrom = extractTargetRecordsFromRelation(
+        currentFieldValueOnSourceRecord,
+      );
+      const targetRecordsToAttachTo = extractTargetRecordsFromRelation(
+        updatedFieldValueOnSourceRecord,
+      );
 
-      const updatedFieldValueOnSourceRecordIsARecordConnection =
-        isObjectRecordConnection(
-          targetObjectMetadata.nameSingular,
-          updatedFieldValueOnSourceRecord,
+      // TODO: see if we can de-hardcode this, put cascade delete in relation metadata item
+      //   Instead of hardcoding it here
+      const shouldCascadeDeleteTargetRecords =
+        CORE_OBJECT_NAMES_TO_DELETE_ON_TRIGGER_RELATION_DETACH.includes(
+          targetObjectMetadata.nameSingular as CoreObjectNameSingular,
         );
-
-      const targetRecordsToAttachTo =
-        updatedFieldValueOnSourceRecordIsARecordConnection
-          ? updatedFieldValueOnSourceRecord.edges.map(
-              ({ node }) => node as RecordGqlNode,
-            )
-          : [updatedFieldValueOnSourceRecord].filter(isDefined);
-
-      const shouldDetachSourceFromAllTargets =
-        isDefined(currentSourceRecord) && targetRecordsToDetachFrom.length > 0;
-
-      if (shouldDetachSourceFromAllTargets) {
-        // TODO: see if we can de-hardcode this, put cascade delete in relation metadata item
-        //   Instead of hardcoding it here
-        const shouldCascadeDeleteTargetRecords =
-          CORE_OBJECT_NAMES_TO_DELETE_ON_TRIGGER_RELATION_DETACH.includes(
-            targetObjectMetadata.nameSingular as CoreObjectNameSingular,
-          );
-
-        if (shouldCascadeDeleteTargetRecords) {
-          triggerDestroyRecordsOptimisticEffect({
+      if (shouldCascadeDeleteTargetRecords) {
+        triggerDestroyRecordsOptimisticEffect({
+          cache,
+          objectMetadataItem: fullTargetObjectMetadataItem,
+          recordsToDestroy: targetRecordsToDetachFrom,
+          objectMetadataItems,
+        });
+        // Could it be an invariant ?
+      } else if (isDefined(currentSourceRecord)) {
+        targetRecordsToDetachFrom.forEach((targetRecordToDetachFrom) => {
+          triggerDetachRelationOptimisticEffect({
             cache,
-            objectMetadataItem: fullTargetObjectMetadataItem,
-            recordsToDestroy: targetRecordsToDetachFrom,
-            objectMetadataItems,
-          });
-        } else {
-          targetRecordsToDetachFrom.forEach((targetRecordToDetachFrom) => {
-            triggerDetachRelationOptimisticEffect({
-              cache,
-              sourceObjectNameSingular: sourceObjectMetadataItem.nameSingular,
-              sourceRecordId: currentSourceRecord.id,
-              fieldNameOnTargetRecord: targetFieldMetadata.name,
-              targetObjectNameSingular: targetObjectMetadata.nameSingular,
-              targetRecordId: targetRecordToDetachFrom.id,
-            });
+            sourceObjectNameSingular: sourceObjectMetadataItem.nameSingular,
+            sourceRecordId: currentSourceRecord.id,
+            fieldNameOnTargetRecord: targetFieldMetadata.name,
+            targetObjectNameSingular: targetObjectMetadata.nameSingular,
+            targetRecordId: targetRecordToDetachFrom.id,
           });
-        }
+        });
       }
 
-      const shouldAttachSourceToAllTargets =
-        isDefined(updatedSourceRecord) && targetRecordsToAttachTo.length > 0;
-
-      if (shouldAttachSourceToAllTargets) {
-        targetRecordsToAttachTo.forEach((targetRecordToAttachTo) =>
+      // Could it be an invariant ?
+      if (isDefined(updatedSourceRecord)) {
+        targetRecordsToAttachTo.forEach((targetRecordsToAttachTo) =>
           triggerAttachRelationOptimisticEffect({
             cache,
             sourceObjectNameSingular: sourceObjectMetadataItem.nameSingular,
             sourceRecordId: updatedSourceRecord.id,
             fieldNameOnTargetRecord: targetFieldMetadata.name,
             targetObjectNameSingular: targetObjectMetadata.nameSingular,
-            targetRecordId: targetRecordToAttachTo.id,
+            targetRecordId: targetRecordsToAttachTo.id,
           }),
         );
       }

packages/twenty-front/src/modules/object-record/cache/utils/__tests__/isObjectRecordConnection.test.ts

@@ -2,12 +2,13 @@ import { peopleQueryResult } from '~/testing/mock-data/people';
 
 import { isObjectRecordConnection } from '@/object-record/cache/utils/isObjectRecordConnection';
 
+// TODO refactor consuming relation from mockss
 describe('isObjectRecordConnection', () => {
   it('should work with query result', () => {
     const validQueryResult = peopleQueryResult.people;
 
     const isValidQueryResult = isObjectRecordConnection(
-      'person',
+      {},
       validQueryResult,
     );
 

packages/twenty-front/src/modules/object-record/cache/utils/getRecordsFromRecordConnection.ts

@@ -7,6 +7,7 @@ export const getRecordsFromRecordConnection = <T extends ObjectRecord>({
 }: {
   recordConnection: RecordGqlConnection;
 }): T[] => {
+  // This can return undefined
   return recordConnection?.edges?.map((edge) =>
     getRecordFromRecordNode<T>({ recordNode: edge.node }),
   );

packages/twenty-front/src/modules/object-record/cache/utils/isObjectRecordConnection.ts

@@ -1,30 +1,24 @@
-import { z } from 'zod';
 
+import { FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem';
 import { RecordGqlConnection } from '@/object-record/graphql/types/RecordGqlConnection';
-import { capitalize } from 'twenty-shared';
+import { assertUnreachable } from '@/workflow/utils/assertUnreachable';
+import { RelationDefinitionType } from '~/generated-metadata/graphql';
 
 export const isObjectRecordConnection = (
-  objectNameSingular: string,
+  relationDefinition: NonNullable<FieldMetadataItem['relationDefinition']>,
   value: unknown,
 ): value is RecordGqlConnection => {
-  const objectConnectionTypeName = `${capitalize(
-    objectNameSingular,
-  )}Connection`;
-  const objectEdgeTypeName = `${capitalize(objectNameSingular)}Edge`;
-
-  const objectConnectionSchema = z.object({
-    __typename: z.literal(objectConnectionTypeName).optional(),
-    edges: z.array(
-      z.object({
-        __typename: z.literal(objectEdgeTypeName).optional(),
-        node: z.object({
-          id: z.string().uuid(),
-        }),
-      }),
-    ),
-  });
-
-  const connectionValidation = objectConnectionSchema.safeParse(value);
-
-  return connectionValidation.success;
-};
+  switch (relationDefinition.direction) {
+    case RelationDefinitionType.MANY_TO_MANY:
+    case RelationDefinitionType.ONE_TO_MANY: {
+      return true;
+    }
+    case RelationDefinitionType.MANY_TO_ONE:
+    case RelationDefinitionType.ONE_TO_ONE: {
+      return false;
+    }
+    default: {
+      return assertUnreachable(relationDefinition.direction);
+    }
+  }
+}