From 8216800a4a3aefdfbb26d696056469b69e087d75 Mon Sep 17 00:00:00 2001 From: Charles Bochet Date: Fri, 9 May 2025 19:03:39 +0200 Subject: [PATCH] Fix workspace relation sync (#11963) ## Context While deploying the IS_NEW_RELATION_ENABLED (we don't compute relation based on relationMetadata anymore) to existing workspace, I've tested to run a sync-metadata post feature flag activation. This has raised two issues: - the workspaceMigration generator (which is over-complex and should be refactored later) for fieldMetadata of type RELATION was not handling settings update properly ; - we need to delete existing fieldMetadata corresponding to the UUID foreignKey as they are not needed anymore. This is handled as a 0.53 upgrade command as 0.53 will also come with the full removal of the old relation system --------- Co-authored-by: Etienne <45695613+etiennejouan@users.noreply.github.com> Co-authored-by: prastoin --- ...tion-foreign-key-field-metadata.command.ts | 77 +++++++++++++ .../0-53-upgrade-version-command.module.ts | 7 +- .../upgrade.command.ts | 2 +- .../determine-schema-relation-details.util.ts | 2 +- ...kspace-migration-field-relation.factory.ts | 103 ++++++++++-------- .../workspace-field-relation.comparator.ts | 12 +- .../workspace-metadata-updater.service.ts | 26 +++-- 7 files changed, 166 insertions(+), 63 deletions(-) create mode 100644 packages/twenty-server/src/database/commands/upgrade-version-command/0-53/0-53-remove-relation-foreign-key-field-metadata.command.ts diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/0-53/0-53-remove-relation-foreign-key-field-metadata.command.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/0-53/0-53-remove-relation-foreign-key-field-metadata.command.ts new file mode 100644 index 000000000..2e775f4a4 --- /dev/null +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/0-53/0-53-remove-relation-foreign-key-field-metadata.command.ts @@ -0,0 +1,77 @@ +import { InjectRepository } from '@nestjs/typeorm'; + +import chalk from 'chalk'; +import { Command } from 'nest-commander'; +import { FieldMetadataType } from 'twenty-shared/types'; +import { In, Like, Repository } from 'typeorm'; + +import { + ActiveOrSuspendedWorkspacesMigrationCommandRunner, + RunOnWorkspaceArgs, +} from 'src/database/commands/command-runners/active-or-suspended-workspaces-migration.command-runner'; +import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; +import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; +import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager'; + +@Command({ + name: 'upgrade:0-53:remove-relation-foreign-key-field-metadata', + description: 'Remove relation foreign key from field metadata', +}) +export class RemoveRelationForeignKeyFieldMetadataCommand extends ActiveOrSuspendedWorkspacesMigrationCommandRunner { + constructor( + @InjectRepository(Workspace, 'core') + protected readonly workspaceRepository: Repository, + @InjectRepository(FieldMetadataEntity, 'metadata') + private readonly fieldMetadataRepository: Repository, + protected readonly twentyORMGlobalManager: TwentyORMGlobalManager, + ) { + super(workspaceRepository, twentyORMGlobalManager); + } + + override async runOnWorkspace({ + index, + total, + workspaceId, + options, + }: RunOnWorkspaceArgs): Promise { + this.logger.log( + `Running command for workspace ${workspaceId} ${index + 1}/${total}`, + ); + + const fieldMetadataCollection = await this.fieldMetadataRepository.find({ + where: { + workspaceId, + type: In([FieldMetadataType.UUID]), + label: Like('%(foreign key)%'), + }, + }); + + if (!fieldMetadataCollection.length) { + this.logger.log( + chalk.yellow( + `No relation field metadata found for workspace ${workspaceId}.`, + ), + ); + + return; + } + + if (options.dryRun) { + this.logger.log( + chalk.yellow( + `Dry run, would delete ${fieldMetadataCollection.length} relation field metadata for workspace ${workspaceId}.`, + ), + ); + } else { + await this.fieldMetadataRepository.delete({ + id: In( + fieldMetadataCollection.map((fieldMetadata) => fieldMetadata.id), + ), + }); + } + + this.logger.log( + chalk.green(`Command completed for workspace ${workspaceId}.`), + ); + } +} diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/0-53/0-53-upgrade-version-command.module.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/0-53/0-53-upgrade-version-command.module.ts index c2bcac09f..8e34c992f 100644 --- a/packages/twenty-server/src/database/commands/upgrade-version-command/0-53/0-53-upgrade-version-command.module.ts +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/0-53/0-53-upgrade-version-command.module.ts @@ -4,21 +4,26 @@ import { TypeOrmModule } from '@nestjs/typeorm'; import { BackfillWorkflowNextStepIdsCommand } from 'src/database/commands/upgrade-version-command/0-53/0-53-backfill-workflow-next-step-ids.command'; import { CopyTypeormMigrationsCommand } from 'src/database/commands/upgrade-version-command/0-53/0-53-copy-typeorm-migrations.command'; import { MigrateWorkflowEventListenersToAutomatedTriggersCommand } from 'src/database/commands/upgrade-version-command/0-53/0-53-migrate-workflow-event-listeners-to-automated-triggers.command'; +import { RemoveRelationForeignKeyFieldMetadataCommand } from 'src/database/commands/upgrade-version-command/0-53/0-53-remove-relation-foreign-key-field-metadata.command'; import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; +import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; import { WorkspaceDataSourceModule } from 'src/engine/workspace-datasource/workspace-datasource.module'; @Module({ imports: [ TypeOrmModule.forFeature([Workspace], 'core'), + TypeOrmModule.forFeature([FieldMetadataEntity], 'metadata'), WorkspaceDataSourceModule, ], providers: [ MigrateWorkflowEventListenersToAutomatedTriggersCommand, - BackfillWorkflowNextStepIdsCommand, CopyTypeormMigrationsCommand, + BackfillWorkflowNextStepIdsCommand, + RemoveRelationForeignKeyFieldMetadataCommand, ], exports: [ MigrateWorkflowEventListenersToAutomatedTriggersCommand, + RemoveRelationForeignKeyFieldMetadataCommand, BackfillWorkflowNextStepIdsCommand, CopyTypeormMigrationsCommand, ], diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/upgrade.command.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/upgrade.command.ts index 0a08d5fa6..46999b504 100644 --- a/packages/twenty-server/src/database/commands/upgrade-version-command/upgrade.command.ts +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/upgrade.command.ts @@ -1,7 +1,7 @@ import { InjectRepository } from '@nestjs/typeorm'; -import { Repository } from 'typeorm'; import { Command } from 'nest-commander'; +import { Repository } from 'typeorm'; import { AllCommands, diff --git a/packages/twenty-server/src/engine/twenty-orm/utils/determine-schema-relation-details.util.ts b/packages/twenty-server/src/engine/twenty-orm/utils/determine-schema-relation-details.util.ts index 331c2a5b4..a7ed2cef7 100644 --- a/packages/twenty-server/src/engine/twenty-orm/utils/determine-schema-relation-details.util.ts +++ b/packages/twenty-server/src/engine/twenty-orm/utils/determine-schema-relation-details.util.ts @@ -35,7 +35,7 @@ export async function determineSchemaRelationDetails( objectMetadataMaps.byId[fieldMetadata.relationTargetObjectMetadataId]; if (!sourceObjectMetadata || !targetObjectMetadata) { - throw new Error('Object metadata not found'); + throw new Error(`Object metadata not found for field ${fieldMetadata.id}`); } if (!fieldMetadata.relationTargetFieldMetadataId) { diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-migration-builder/factories/workspace-migration-field-relation.factory.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-migration-builder/factories/workspace-migration-field-relation.factory.ts index 04a9cbad6..73e42c42e 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-migration-builder/factories/workspace-migration-field-relation.factory.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-migration-builder/factories/workspace-migration-field-relation.factory.ts @@ -18,7 +18,6 @@ import { WorkspaceMigrationFactory } from 'src/engine/metadata-modules/workspace import { computeObjectTargetTable } from 'src/engine/utils/compute-object-target-table.util'; import { isFieldMetadataEntityOfType } from 'src/engine/utils/is-field-metadata-of-type.util'; import { FieldMetadataUpdate } from 'src/engine/workspace-manager/workspace-migration-builder/factories/workspace-migration-field.factory'; -import { camelCase } from 'src/utils/camel-case'; @Injectable() export class WorkspaceMigrationFieldRelationFactory { constructor( @@ -101,6 +100,24 @@ export class WorkspaceMigrationFieldRelationFactory { ); } + if (!sourceFieldMetadata.settings) { + throw new Error( + `FieldMetadata for relation with id ${sourceFieldMetadata.id} has no settings`, + ); + } + + if ( + sourceFieldMetadata.settings.relationType !== RelationType.MANY_TO_ONE + ) { + continue; + } + + if (!sourceFieldMetadata.settings.joinColumnName) { + throw new Error( + `FieldMetadata with id ${sourceFieldMetadata.id} has no join column name in settings`, + ); + } + if (!targetObjectMetadata) { throw new Error( `ObjectMetadata with id ${sourceFieldMetadata.relationTargetObjectMetadataId} not found`, @@ -137,29 +154,27 @@ export class WorkspaceMigrationFieldRelationFactory { const migrations: WorkspaceMigrationTableAction[] = [ { - name: computeObjectTargetTable(targetObjectMetadata), + name: computeObjectTargetTable(sourceObjectMetadata), action: WorkspaceMigrationTableActionType.ALTER, columns: [ { action: WorkspaceMigrationColumnActionType.DROP_FOREIGN_KEY, - columnName: `${camelCase(targetFieldMetadata.name)}Id`, + columnName: sourceFieldMetadata.settings.joinColumnName, }, ], }, { - name: computeObjectTargetTable(targetObjectMetadata), + name: computeObjectTargetTable(sourceObjectMetadata), action: WorkspaceMigrationTableActionType.ALTER, columns: [ { action: WorkspaceMigrationColumnActionType.CREATE_FOREIGN_KEY, - columnName: `${camelCase(targetFieldMetadata.name)}Id`, + columnName: sourceFieldMetadata.settings.joinColumnName, referencedTableName: - computeObjectTargetTable(sourceObjectMetadata), + computeObjectTargetTable(targetObjectMetadata), referencedTableColumnName: 'id', - isUnique: - targetFieldMetadata.settings.relationType === - RelationType.ONE_TO_ONE, - onDelete: targetFieldMetadata.settings.onDelete, + isUnique: false, + onDelete: sourceFieldMetadata.settings.onDelete, }, ], }, @@ -192,22 +207,27 @@ export class WorkspaceMigrationFieldRelationFactory { sourceFieldMetadata.relationTargetObjectMetadataId ]; + if (!sourceObjectMetadata) { + throw new Error( + `ObjectMetadata with id ${sourceFieldMetadata.objectMetadataId} not found`, + ); + } + if (!sourceFieldMetadata.settings) { throw new Error( `FieldMetadata for relation with id ${sourceFieldMetadata.id} has no settings`, ); } - // We're creating it from `ONE_TO_MANY` with the join column so we don't need to create a migration for `MANY_TO_ONE` if ( - sourceFieldMetadata.settings.relationType === RelationType.MANY_TO_ONE + sourceFieldMetadata.settings.relationType !== RelationType.MANY_TO_ONE ) { continue; } - if (!sourceObjectMetadata) { + if (!sourceFieldMetadata.settings.joinColumnName) { throw new Error( - `ObjectMetadata with id ${sourceFieldMetadata.objectMetadataId} not found`, + `FieldMetadata with id ${sourceFieldMetadata.id} has no join column name in settings`, ); } @@ -239,43 +259,29 @@ export class WorkspaceMigrationFieldRelationFactory { ); } - if (!targetFieldMetadata.settings) { - throw new Error( - `FieldMetadata for relation with id ${sourceFieldMetadata.id} has no settings`, - ); - } - - if (!targetFieldMetadata.settings.joinColumnName) { - continue; - } - const migrations: WorkspaceMigrationTableAction[] = [ { - name: computeObjectTargetTable(targetObjectMetadata), + name: computeObjectTargetTable(sourceObjectMetadata), action: WorkspaceMigrationTableActionType.ALTER, columns: [ ...this.workspaceMigrationFactory.createColumnActions( WorkspaceMigrationColumnActionType.CREATE, - targetFieldMetadata, + sourceFieldMetadata, ), ], }, { - name: computeObjectTargetTable(targetObjectMetadata), + name: computeObjectTargetTable(sourceObjectMetadata), action: WorkspaceMigrationTableActionType.ALTER, columns: [ { action: WorkspaceMigrationColumnActionType.CREATE_FOREIGN_KEY, - columnName: - targetFieldMetadata.settings.joinColumnName ?? - `${camelCase(targetFieldMetadata.name)}Id`, + columnName: sourceFieldMetadata.settings.joinColumnName, referencedTableName: - computeObjectTargetTable(sourceObjectMetadata), + computeObjectTargetTable(targetObjectMetadata), referencedTableColumnName: 'id', - isUnique: - targetFieldMetadata.settings.relationType === - RelationType.ONE_TO_ONE, - onDelete: targetFieldMetadata.settings.onDelete, + isUnique: false, + onDelete: sourceFieldMetadata.settings.onDelete, }, ], }, @@ -314,6 +320,19 @@ export class WorkspaceMigrationFieldRelationFactory { ); } + if (!sourceFieldMetadata.settings) { + throw new Error( + `FieldMetadata for relation with id ${sourceFieldMetadata.id} has no settings`, + ); + } + + if ( + sourceFieldMetadata.settings.relationType !== RelationType.MANY_TO_ONE + ) { + // Only MANY_TO_ONE relations deletion have consequences on the database schema + continue; + } + if (!targetObjectMetadata) { throw new Error( `ObjectMetadata with id ${sourceFieldMetadata.relationTargetObjectMetadataId} not found`, @@ -342,32 +361,30 @@ export class WorkspaceMigrationFieldRelationFactory { ); } - if (!targetFieldMetadata.settings) { + if (!sourceFieldMetadata.settings.joinColumnName) { throw new Error( - `FieldMetadata for relation with id ${sourceFieldMetadata.id} has no settings`, + `FieldMetadata for relation with id ${sourceFieldMetadata.id} has no join column name, it should not generate a workspace migration`, ); } const migrations: WorkspaceMigrationTableAction[] = [ { - name: computeObjectTargetTable(targetObjectMetadata), + name: computeObjectTargetTable(sourceObjectMetadata), action: WorkspaceMigrationTableActionType.ALTER, columns: [ { action: WorkspaceMigrationColumnActionType.DROP_FOREIGN_KEY, - columnName: `${camelCase(targetFieldMetadata.name)}Id`, + columnName: sourceFieldMetadata.settings.joinColumnName, }, ], }, { - name: computeObjectTargetTable(targetObjectMetadata), + name: computeObjectTargetTable(sourceObjectMetadata), action: WorkspaceMigrationTableActionType.ALTER, columns: [ { action: WorkspaceMigrationColumnActionType.DROP, - columnName: - targetFieldMetadata.settings.joinColumnName ?? - `${camelCase(targetFieldMetadata.name)}Id`, + columnName: sourceFieldMetadata.settings.joinColumnName, }, ], }, diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/comparators/workspace-field-relation.comparator.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/comparators/workspace-field-relation.comparator.ts index 2277e6b08..9364eb143 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/comparators/workspace-field-relation.comparator.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/comparators/workspace-field-relation.comparator.ts @@ -130,10 +130,6 @@ export class WorkspaceFieldRelationComparator { throw new Error(`Field ${fieldId} not found in originalObjectMetadata`); } - if (!standardFieldMetadata) { - throw new Error(`Field ${fieldId} not found in standardObjectMetadata`); - } - for (const difference of differences) { const property = difference.path[difference.path.length - 1]; @@ -175,11 +171,11 @@ export class WorkspaceFieldRelationComparator { (fieldPropertiesToStringify as readonly string[]).includes(property) ) { const newValue = this.parseJSONOrString(difference.value); + const oldValue = this.parseJSONOrString(difference.oldValue); if (property === 'settings' && difference.oldValue) { const newSettings = newValue as FieldMetadataRelationSettings; - const oldSettings = - difference.oldValue as FieldMetadataRelationSettings; + const oldSettings = oldValue as FieldMetadataRelationSettings; // Check if the relation type has changed if (oldSettings.relationType !== newSettings.relationType) { @@ -193,6 +189,10 @@ export class WorkspaceFieldRelationComparator { } } + if (!standardFieldMetadata) { + throw new Error(`Field ${fieldId} not found in standardObjectMetadata`); + } + if (relationTypeChange) { result.push({ action: ComparatorAction.DELETE, diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/services/workspace-metadata-updater.service.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/services/workspace-metadata-updater.service.ts index 990b7e5c0..37425d524 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/services/workspace-metadata-updater.service.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/services/workspace-metadata-updater.service.ts @@ -221,8 +221,6 @@ export class WorkspaceMetadataUpdaterService { []; let updatedFieldRelationMetadataCollection: FieldMetadataUpdate[] = []; - let deletedFieldRelationMetadataCollection: FieldMetadataUpdate[] = - []; /** * Create field relation metadata @@ -253,20 +251,26 @@ export class WorkspaceMetadataUpdaterService { } if (!options || options.actions.includes('delete')) { - deletedFieldRelationMetadataCollection = await this.updateEntities< - FieldMetadataEntity - >( - manager, - FieldMetadataEntity, - storage.fieldRelationMetadataDeleteCollection, - ['objectMetadataId', 'workspaceId'], - ); + const fieldMetadataRepository = + manager.getRepository(FieldMetadataEntity); + + if (storage.fieldRelationMetadataDeleteCollection.length > 0) { + await fieldMetadataRepository.delete( + storage.fieldRelationMetadataDeleteCollection.map( + (field) => field.id, + ), + ); + } } return { createdFieldRelationMetadataCollection, updatedFieldRelationMetadataCollection, - deletedFieldRelationMetadataCollection, + deletedFieldRelationMetadataCollection: + storage.fieldRelationMetadataDeleteCollection.map((field) => ({ + current: field, + altered: field, + })), }; }