feat: new relation sync-metadata, twenty-orm, create/update #10217

magrinj · 2025-02-14T11:27:00Z

Fix twentyhq/core-team-issues#330 (comment) and twentyhq/core-team-issues#327 (comment)

What this PR does when isNewRelationEnabled is set to true:

Drop the creation of the foreign key as a FieldMetadata
Stop creating RelationMetadata
Properly fill FieldMetadata of type RELATION during the sync command
Use new relation settings in TwentyORM
Properly create FieldMetadata relations when we create a new object
Handle database:reset with new relations

…p-foreign-key

greptile-apps

PR Summary

This PR introduces a significant architectural change in how relations are handled in the Twenty system, controlled by the isNewRelationEnabled feature flag.

Key changes:

Drops creation of foreign key fields as FieldMetadata and stops creating RelationMetadata entities
Replaces RelationMetadataType with RelationType across all workspace entities
Introduces WorkspaceSyncFieldMetadataRelationService for handling field metadata relations
Adds feature flag checks in query resolvers and formatters to support both old and new relation systems
Implements new utilities like determineSchemaRelationDetails and converRelationTypeToTypeORMRelationType for relation handling

The changes appear well-structured with proper feature flag controls for backward compatibility, though careful testing of database migrations and existing relation data is recommended.

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

greptile-apps · 2025-03-12T10:55:04Z

packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.resolver.ts

+        // Temporary fix as we don't have relationId in the new relation
+        relationId: createDeterministicUuid([
+          relation.sourceFieldMetadata.id,
+          relation.targetFieldMetadata.id,
+        ]),


style: Using field IDs for relation UUID could cause issues if fields are recreated with same configuration but different IDs. Consider using more stable identifiers.

greptile-apps · 2025-03-12T10:55:05Z

packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.resolver.ts

+          relation.sourceFieldMetadata.id,
+          relation.targetFieldMetadata.id,
+        ]),
+        direction: relation.type as unknown as RelationDefinitionType,


logic: Unsafe type casting from relationType to RelationDefinitionType. Should validate the values match before casting.

greptile-apps · 2025-03-12T10:55:24Z

packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.service.ts

+      const isNewRelationEnabled =
+        await this.featureFlagService.isFeatureEnabled(
+          FeatureFlagKey.IsNewRelationEnabled,
          objectMetadataInput.workspaceId,
-          createdObjectMetadata,
-          {
-            primaryKeyFieldMetadataSettings:
-              objectMetadataInput.primaryKeyFieldMetadataSettings,
-            primaryKeyColumnType: objectMetadataInput.primaryKeyColumnType,
-          },
        );


logic: Feature flag check should be moved before table creation to ensure consistent state if creation fails

greptile-apps · 2025-03-12T10:55:25Z

packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.service.ts

    if (input.update.isActive !== undefined) {
-      await this.objectMetadataRelationService.updateObjectRelationshipsActivationStatus(
-        input.id,
-        input.update.isActive,
-      );
+      // For new relation system, the active status is stitched to the field metadata
+      if (!isNewRelationEnabled) {
+        await this.objectMetadataRelationService.updateObjectRelationshipsActivationStatus(
+          input.id,
+          input.update.isActive,
+        );
+      }
    }


style: Consider moving isActive handling to a separate method for better maintainability and reusability

greptile-apps · 2025-03-12T10:55:48Z

...er/src/engine/metadata-modules/object-metadata/services/object-metadata-migration.service.ts

+  public async updateRelationMigrations(
+    currentObjectMetadata: ObjectMetadataEntity,
+    alteredObjectMetadata: ObjectMetadataEntity,
+    relationMetadataCollection: {
+      targetObjectMetadata: ObjectMetadataEntity;
+      targetFieldMetadata: FieldMetadataEntity;
+      sourceFieldMetadata: FieldMetadataEntity;
+    }[],
+    workspaceId: string,
+  ) {
+    for (const { targetObjectMetadata } of relationMetadataCollection) {
+      const targetTableName = computeObjectTargetTable(targetObjectMetadata);
+      const columnName = `${currentObjectMetadata.nameSingular}Id`;
+
+      await this.workspaceMigrationService.createCustomMigration(
+        generateMigrationName(
+          `rename-${currentObjectMetadata.nameSingular}-to-${alteredObjectMetadata.nameSingular}-in-${targetObjectMetadata.nameSingular}`,
+        ),
+        workspaceId,
+        [
+          {
+            name: targetTableName,
+            action: WorkspaceMigrationTableActionType.ALTER,
+            columns: [
+              {
+                action: WorkspaceMigrationColumnActionType.ALTER,
+                currentColumnDefinition: {
+                  columnName,
+                  columnType: 'uuid',
+                  isNullable: true,
+                  defaultValue: null,
+                },
+                alteredColumnDefinition: {
+                  columnName: `${alteredObjectMetadata.nameSingular}Id`,
+                  columnType: 'uuid',
+                  isNullable: true,
+                  defaultValue: null,
+                },
+              },
+            ],
+          },
+        ],
+      );
+    }
+  }


style: Duplicates logic from createUpdateForeignKeysMigrations method - consider consolidating these methods to avoid maintenance issues

greptile-apps · 2025-03-12T11:01:36Z

...rver/src/engine/workspace-manager/workspace-sync-metadata/interfaces/comparator.interface.ts

+      Partial<ComputedPartialFieldMetadata<FieldMetadataType.RELATION>> & {
+        id: string;
+      }


logic: The id field is required in the create result but relations might not have an id at creation time. Consider making id optional for CREATE operations

greptile-apps · 2025-03-12T11:03:06Z

...erver/src/engine/workspace-manager/workspace-sync-metadata/storage/workspace-sync.storage.ts

+  private readonly _fieldRelationMetadataCreateCollection: (Partial<
+    ComputedPartialFieldMetadata<FieldMetadataType.RELATION>
+  > & {
+    id: string;
+  })[] = [];
+  private readonly _fieldRelationMetadataUpdateCollection: (Partial<
+    ComputedPartialFieldMetadata<FieldMetadataType.RELATION>
+  > & {
+    id: string;
+  })[] = [];
+  private readonly _fieldRelationMetadataDeleteCollection: FieldMetadataEntity<FieldMetadataType.RELATION>[] =
+    [];


logic: The field relation metadata collections require an id on creation, which differs from the regular field metadata collection that doesn't require an id. This inconsistency could lead to confusion and potential bugs.

greptile-apps · 2025-03-12T11:03:12Z

...r/src/engine/workspace-manager/workspace-sync-metadata/utils/compute-standard-fields.util.ts

+  const isNewRelationEnabled =
+    context.featureFlags[FeatureFlagKey.IsNewRelationEnabled];


logic: Check that context.featureFlags exists before accessing IsNewRelationEnabled to prevent runtime errors

Suggested change

const isNewRelationEnabled =

context.featureFlags[FeatureFlagKey.IsNewRelationEnabled];

const isNewRelationEnabled =

context.featureFlags?.[FeatureFlagKey.IsNewRelationEnabled] ?? false;

greptile-apps · 2025-03-12T11:04:43Z

packages/twenty-server/src/modules/favorite/standard-objects/favorite.workspace-entity.ts

    standardId: FAVORITE_STANDARD_FIELD_IDS.workflowVersion,
-    type: RelationMetadataType.MANY_TO_ONE,
+    type: RelationType.MANY_TO_ONE,
    label: msg`Workflow`,
    description: msg`Favorite workflow version`,


style: Label 'Workflow' is reused for three different relations (workflow, workflowVersion, workflowRun) which could cause confusion in the UI

greptile-apps · 2025-03-12T11:07:56Z

...ules/workflow/workflow-executor/workflow-actions/record-crud/find-records.workflow-action.ts

+    const isNewRelationEnabled = await this.featureFlagService.isFeatureEnabled(
+      FeatureFlagKey.IsNewRelationEnabled,
+      objectMetadataItemWithFieldsMaps.workspaceId,
+    );


style: Consider caching feature flag result at class level to avoid repeated DB calls for the same workspace

charlesBochet · 2025-03-27T07:31:50Z

...r/src/engine/metadata-modules/field-metadata/interfaces/field-metadata-settings.interface.ts

  relationType: RelationType;
  onDelete?: RelationOnDeleteAction;
-  joinColumnName?: string;
+  joinColumnName?: string | null;


can we avoid | null here and just rely on undefined?

Follow up from #10217 Adding onDelete action on Many_to_one sides

magrinj force-pushed the feat/new-relation-drop-foreign-key branch 3 times, most recently from 45c4850 to ed349c4 Compare February 25, 2025 16:42

magrinj added 3 commits February 27, 2025 18:04

feat: relation-metadata stop declaring foreign key manually

ca8980c

wip

759a160

WIP on feat/new-relation-drop-foreign-key

6f38f6b

magrinj force-pushed the feat/new-relation-drop-foreign-key branch from 146ae15 to 6f38f6b Compare February 27, 2025 17:04

magrinj added 10 commits February 28, 2025 12:27

wip: sync-metadata

1260478

wip

987c820

wip

d2affef

wip: invalid onDeleteAction

45ad306

wip: missing foreign key migration to be appied

ee4d08e

wip

10f7f7a

feat: database:reset working

f466fe5

fix

c7c2b67

Merge remote-tracking branch 'origin/main' into feat/new-relation-dro…

3df7448

…p-foreign-key

fix

25c4e4b

magrinj changed the title ~~Feat/new relation drop foreign key~~ feat: new relation sync-metadata, twenty-orm, create/update Mar 12, 2025

magrinj marked this pull request as ready for review March 12, 2025 10:51

magrinj requested review from Weiko and charlesBochet March 12, 2025 10:51

greptile-apps bot reviewed Mar 12, 2025

View reviewed changes

charlesBochet self-assigned this Mar 26, 2025

Merge branch 'main' into feat/new-relation-drop-foreign-key

5adfd6b

charlesBochet reviewed Mar 27, 2025

View reviewed changes

charlesBochet and others added 4 commits March 27, 2025 08:47

Fix

7b97706

Fix

68b426d

Merge branch 'main' into feat/new-relation-drop-foreign-key

957ade7

Fix

d59becf

charlesBochet force-pushed the main branch from 526341f to 981b1bd Compare April 15, 2025 21:41

FelixMalfait mentioned this pull request Apr 16, 2025

Errors like "Uncompiled message detected" fill server logs #11593

Open

Merge branch 'main' into feat/new-relation-drop-foreign-key

63cb8d6

charlesBochet changed the base branch from main to 0.50.1 April 22, 2025 05:31

charlesBochet changed the base branch from 0.50.1 to main April 22, 2025 05:31

charlesBochet added 9 commits April 22, 2025 08:07

Improve devXP

32b08b4

Re-add joinColumnName to available output and input fields

b76ad76

Complete backend

1c3e6fa

Fix

69679cc

Fixes

66bfa3a

Fixes

8f7daa6

Fix

4fcf459

Fix

b7dce3c

Fix

d8fa7de

Weiko approved these changes Apr 22, 2025

View reviewed changes

charlesBochet added 4 commits April 22, 2025 18:36

Fix

c234484

Fix lint

a5b3de3

Merge branch 'main' into feat/new-relation-drop-foreign-key

0901f0d

Fix front tests

2194b3d

charlesBochet merged commit cc29c25 into main Apr 22, 2025
5 checks passed

charlesBochet deleted the feat/new-relation-drop-foreign-key branch April 22, 2025 17:01

martmull mentioned this pull request Apr 24, 2025

Fix missing on delete cascade for relation #11725

Merged

martmull added a commit that referenced this pull request Apr 24, 2025

Fix missing on delete cascade for relation (#11725)

4d7dbb1

Follow up from #10217 Adding onDelete action on Many_to_one sides

ehconitin pushed a commit that referenced this pull request Apr 24, 2025

Fix missing on delete cascade for relation (#11725)

61d484c

Follow up from #10217 Adding onDelete action on Many_to_one sides

charlesBochet pushed a commit that referenced this pull request Apr 24, 2025

Fix missing on delete cascade for relation (#11725)

8d95166

Follow up from #10217 Adding onDelete action on Many_to_one sides

		const isNewRelationEnabled =
		context.featureFlags[FeatureFlagKey.IsNewRelationEnabled];

feat: new relation sync-metadata, twenty-orm, create/update #10217

feat: new relation sync-metadata, twenty-orm, create/update #10217

Uh oh!

Conversation

magrinj commented Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

Uh oh!

greptile-apps bot Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

charlesBochet Mar 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

magrinj commented Feb 14, 2025 •

edited

Loading