Refactor `triggerUpdateRelationsOptimisticEffect` to compute relationship from Metadatas #9815

prastoin · 2025-01-23T14:04:36Z

Introduction

At the moment the relationships are inferred from the record data structure instead of its metadatas
We should refactor the code that computes or not the necessity to detach a relation on a mutation

We've refactored the isObjectRecordConnection method to be consuming a relationDefintion instead of "typeChecking" at the runtime the data structure using zod validation schema

Related to #9580

github-actions · 2025-01-23T16:56:51Z

	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-0f4d90fd.json

Generated by 🚫 dangerJS against afb243d

…ordConnection

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

greptile-apps

PR Summary

This PR refactors the relationship handling in optimistic updates by switching from record data structure inference to metadata-based relationship computation, improving type safety and reliability.

Added new computeRecordsToAttachAndDetach function in /packages/twenty-front/src/modules/apollo/optimistic-effect/utils/triggerUpdateRelationsOptimisticEffect.ts to determine relationship changes based on metadata
Implemented proper relationship direction handling (ONE_TO_MANY, MANY_TO_ONE, etc.) using RelationDefinitionType enum
Enhanced type safety with explicit typing for function arguments and return values in relationship computation
Added support for handling both connection-based and direct reference relationships through metadata

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

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

prastoin · 2025-01-24T13:19:55Z

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

@@ -7,6 +7,7 @@ export const getRecordsFromRecordConnection = <T extends ObjectRecord>({
 }: {
  recordConnection: RecordGqlConnection;
 }): T[] => {


Remark: Unless I'm mistaken this function could return undefined

}: { recordConnection: RecordGqlConnection; }): T[] | undefined => {

prastoin · 2025-01-24T14:09:54Z

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

-      if (shouldAttachSourceToAllTargets) {
-        targetRecordsToAttachTo.forEach((targetRecordToAttachTo) =>
+      // Could it be an invariant ?
+      if (isDefined(updatedSourceRecord)) {


Remark: This could be an invariant, but implicitly quite though to tell TypeScript

prastoin · 2025-01-24T14:09:59Z

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

+          objectMetadataItems,
+        });
+        // Could it be an invariant ?
+      } else if (isDefined(currentSourceRecord)) {


Remark: This could be an invariant, but implicitly quite though to tell TypeScript

prastoin · 2025-01-24T14:36:19Z

About to create an issue regarding the possible cache corruption due to ObjectRecord and RecordGqlNode inclusion, after the related critical bug has been resolved

greptile-apps

PR Summary

(updates since last review)

This PR continues the refactoring of relationship handling by simplifying the logic and adding robust testing for relationship type determination.

Added comprehensive test coverage in isObjectRecordConnection.test.ts for all relation definition types (MANY_TO_MANY, ONE_TO_MANY, etc.)
Simplified isObjectRecordConnection to make decisions purely based on relation direction metadata
Added edge case handling for array injection in cache through extractTargetRecordsFromRelation function
Improved error handling with explicit checks for unknown relation directions

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

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

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

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

charlesBochet · 2025-01-24T16:55:43Z

LGTM too

chore: to drop

45bff52

prastoin self-assigned this Jan 23, 2025

prastoin added -PR: wip -PR: awaiting author labels Jan 23, 2025

prastoin added 2 commits January 23, 2025 18:10

refactor(front): consume metadatas to compute if current value is rec…

9451bea

…ordConnection

chore: remove logs

b09bdb1

prastoin force-pushed the 9580-2-refactor-trigger-update-relations-optimistic-effect branch from c80242c to b09bdb1 Compare January 23, 2025 17:11

chore(front): naming convention

cf640e6

prastoin force-pushed the 9580-2-refactor-trigger-update-relations-optimistic-effect branch from 4ac212c to cf640e6 Compare January 23, 2025 17:16

prastoin commented Jan 23, 2025

View reviewed changes

...y-front/src/modules/apollo/optimistic-effect/utils/triggerUpdateRelationsOptimisticEffect.ts Outdated Show resolved Hide resolved

prastoin marked this pull request as ready for review January 23, 2025 17:21

prastoin marked this pull request as draft January 23, 2025 17:21

greptile-apps bot reviewed Jan 23, 2025

View reviewed changes

...y-front/src/modules/apollo/optimistic-effect/utils/triggerUpdateRelationsOptimisticEffect.ts Outdated Show resolved Hide resolved

prastoin added 6 commits January 23, 2025 18:28

refactor(front): avoid AHA

bff83ee

chore(front): remove unecessary optional access

ceaf618

chore(front): todos wip

770c024

chore(front): relocate hasManyTargetRecords

7c6e995

refactor(front): remove redundant conditions

8d6f5aa

refactor(front): handle ObjectRecord and not grapql record or connection

fcf12a6

prastoin force-pushed the 9580-2-refactor-trigger-update-relations-optimistic-effect branch from bc81a18 to fcf12a6 Compare January 24, 2025 10:19

prastoin added 2 commits January 24, 2025 13:49

fix(front): tmp fix replace isEmpty by isArray

97cec31

chore(front): to review

398966d

prastoin commented Jan 24, 2025

View reviewed changes

prastoin added 2 commits January 24, 2025 14:21

chore(front): comment about RecordObject reflexion

7bc75ce

test(front): isObjectRecordConnection

6fc76fa

prastoin commented Jan 24, 2025

View reviewed changes

prastoin force-pushed the 9580-2-refactor-trigger-update-relations-optimistic-effect branch 2 times, most recently from 746ae2d to 8cf4115 Compare January 24, 2025 14:17

chore(front): remove comments and logs

cca0c89

prastoin force-pushed the 9580-2-refactor-trigger-update-relations-optimistic-effect branch from 8cf4115 to cca0c89 Compare January 24, 2025 14:32

chore(front): update comment

6e3e2ed

prastoin marked this pull request as ready for review January 24, 2025 14:35

prastoin requested a review from charlesBochet January 24, 2025 14:36

greptile-apps bot reviewed Jan 24, 2025

View reviewed changes

prastoin requested a review from lucasbordeau January 24, 2025 14:36

chore(front): typos and renaming

afb243d

prastoin added -PR: awaiting review and removed -PR: wip -PR: awaiting author labels Jan 24, 2025

lucasbordeau approved these changes Jan 24, 2025

View reviewed changes

prastoin merged commit 95c7726 into main Jan 24, 2025
50 checks passed

prastoin deleted the 9580-2-refactor-trigger-update-relations-optimistic-effect branch January 24, 2025 16:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor `triggerUpdateRelationsOptimisticEffect` to compute relationship from Metadatas #9815

Refactor `triggerUpdateRelationsOptimisticEffect` to compute relationship from Metadatas #9815

prastoin commented Jan 23, 2025 •

edited

Loading

github-actions bot commented Jan 23, 2025 •

edited

Loading

greptile-apps bot left a comment

prastoin Jan 24, 2025 •

edited

Loading

prastoin Jan 24, 2025

prastoin Jan 24, 2025

prastoin commented Jan 24, 2025 •

edited

Loading

greptile-apps bot left a comment

charlesBochet commented Jan 24, 2025

Refactor triggerUpdateRelationsOptimisticEffect to compute relationship from Metadatas #9815

Refactor triggerUpdateRelationsOptimisticEffect to compute relationship from Metadatas #9815

Conversation

prastoin commented Jan 23, 2025 • edited Loading

Introduction

github-actions bot commented Jan 23, 2025 • edited Loading

Log

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

prastoin Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

prastoin Jan 24, 2025

Choose a reason for hiding this comment

prastoin Jan 24, 2025

Choose a reason for hiding this comment

prastoin commented Jan 24, 2025 • edited Loading

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

charlesBochet commented Jan 24, 2025

Refactor `triggerUpdateRelationsOptimisticEffect` to compute relationship from Metadatas #9815

Refactor `triggerUpdateRelationsOptimisticEffect` to compute relationship from Metadatas #9815

prastoin commented Jan 23, 2025 •

edited

Loading

github-actions bot commented Jan 23, 2025 •

edited

Loading

prastoin Jan 24, 2025 •

edited

Loading

prastoin commented Jan 24, 2025 •

edited

Loading