From e1eead56c6d398395e73174f716e0f9e21f8c28c Mon Sep 17 00:00:00 2001 From: Thomas Trompette Date: Wed, 15 May 2024 17:05:30 +0200 Subject: [PATCH] Alter comment on foreign key deletion (#5406) We do not update the comment on the local table when a foreign table key is deleted. This was not breaking, which is why we did not see it. But comments should be kept up to date. --------- Co-authored-by: Thomas Trompette --- .../object-metadata.service.ts | 20 +++- ...oreign-key-comment-alteration.util.spec.ts | 76 +++++++++++++++ ...for-foreign-key-comment-alteration.util.ts | 97 +++++++++++++++++++ ...tions-for-custom-object-relations.util.ts} | 2 +- ...tions-for-remote-object-relations.util.ts} | 2 +- 5 files changed, 191 insertions(+), 6 deletions(-) create mode 100644 packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/__tests__/create-migration-for-foreign-key-comment-alteration.util.spec.ts create mode 100644 packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/create-migration-for-foreign-key-comment-alteration.util.ts rename packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/{create-workspace-migrations-for-custom-object.util.ts => create-migrations-for-custom-object-relations.util.ts} (98%) rename packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/{create-workspace-migrations-for-remote-object.util.ts => create-workspace-migrations-for-remote-object-relations.util.ts} (98%) diff --git a/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.service.ts b/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.service.ts index 93d00531c2f..b6b72e49b44 100644 --- a/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.service.ts @@ -54,14 +54,15 @@ import { createForeignKeyDeterministicUuid, createRelationDeterministicUuid, } from 'src/engine/workspace-manager/workspace-sync-metadata/utils/create-deterministic-uuid.util'; -import { createWorkspaceMigrationsForCustomObject } from 'src/engine/metadata-modules/object-metadata/utils/create-workspace-migrations-for-custom-object.util'; -import { createWorkspaceMigrationsForRemoteObject } from 'src/engine/metadata-modules/object-metadata/utils/create-workspace-migrations-for-remote-object.util'; +import { createWorkspaceMigrationsForCustomObjectRelations } from 'src/engine/metadata-modules/object-metadata/utils/create-migrations-for-custom-object-relations.util'; +import { createWorkspaceMigrationsForRemoteObjectRelations } from 'src/engine/metadata-modules/object-metadata/utils/create-workspace-migrations-for-remote-object-relations.util'; import { computeColumnName } from 'src/engine/metadata-modules/field-metadata/utils/compute-column-name.util'; import { DataSourceEntity } from 'src/engine/metadata-modules/data-source/data-source.entity'; import { validateObjectMetadataInput } from 'src/engine/metadata-modules/object-metadata/utils/validate-object-metadata-input.util'; import { mapUdtNameToFieldType } from 'src/engine/metadata-modules/remote-server/remote-table/utils/udt-name-mapper.util'; import { WorkspaceCacheVersionService } from 'src/engine/metadata-modules/workspace-cache-version/workspace-cache-version.service'; import { UpdateOneObjectInput } from 'src/engine/metadata-modules/object-metadata/dtos/update-object.input'; +import { createMigrationToAlterCommentOnForeignKeyDeletion } from 'src/engine/metadata-modules/object-metadata/utils/create-migration-for-foreign-key-comment-alteration.util'; import { ObjectMetadataEntity } from './object-metadata.entity'; @@ -205,6 +206,17 @@ export class ObjectMetadataService extends TypeOrmQueryService { + const localObjectMetadataName = 'favorite'; + const remoteObjectMetadataName = 'blog'; + const schema = 'schema'; + const workspaceDataSource = { + query: jest.fn(), + }; + + it('should return null if no comment ', async () => { + workspaceDataSource.query.mockResolvedValueOnce([]); + + const result = await buildAlteredCommentOnForeignKeyDeletion( + localObjectMetadataName, + remoteObjectMetadataName, + schema, + workspaceDataSource as any, + ); + + expect(result).toBeNull(); + }); + + it('should return null if the existing comment does not contain foreign keys', async () => { + workspaceDataSource.query.mockResolvedValueOnce([ + { col_description: '@graphql({"totalCount":{"enabled":true}})' }, + ]); + + const result = await buildAlteredCommentOnForeignKeyDeletion( + localObjectMetadataName, + remoteObjectMetadataName, + schema, + workspaceDataSource as any, + ); + + expect(result).toBeNull(); + }); + + it('should return altered comment without foreign key', async () => { + const existingComment = { + col_description: `@graphql({"totalCount":{"enabled":true},"foreign_keys":[{"local_name":"favoriteCollection","local_columns":["${remoteObjectMetadataName}Id"],"foreign_name":"${remoteObjectMetadataName}","foreign_schema":"schema","foreign_table":"${remoteObjectMetadataName}","foreign_columns":["id"]}]})`, + }; + + workspaceDataSource.query.mockResolvedValueOnce([existingComment]); + + const result = await buildAlteredCommentOnForeignKeyDeletion( + localObjectMetadataName, + remoteObjectMetadataName, + schema, + workspaceDataSource as any, + ); + + expect(result).toBe( + '@graphql({"totalCount":{"enabled":true},"foreign_keys":[]})', + ); + }); + + it('should return altered comment without the input foreign key', async () => { + const existingComment = { + col_description: `@graphql({"totalCount":{"enabled":true},"foreign_keys":[{"local_name":"favoriteCollection","local_columns":["${remoteObjectMetadataName}Id"],"foreign_name":"${remoteObjectMetadataName}","foreign_schema":"schema","foreign_table":"${remoteObjectMetadataName}","foreign_columns":["id"]}, {"local_name":"favoriteCollection","local_columns":["testId"],"foreign_name":"test","foreign_schema":"schema","foreign_table":"test","foreign_columns":["id"]}]})`, + }; + + workspaceDataSource.query.mockResolvedValueOnce([existingComment]); + + const result = await buildAlteredCommentOnForeignKeyDeletion( + localObjectMetadataName, + remoteObjectMetadataName, + schema, + workspaceDataSource as any, + ); + + expect(result).toBe( + '@graphql({"totalCount":{"enabled":true},"foreign_keys":[{"local_name":"favoriteCollection","local_columns":["testId"],"foreign_name":"test","foreign_schema":"schema","foreign_table":"test","foreign_columns":["id"]}]})', + ); + }); +}); diff --git a/packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/create-migration-for-foreign-key-comment-alteration.util.ts b/packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/create-migration-for-foreign-key-comment-alteration.util.ts new file mode 100644 index 00000000000..f42dcbff672 --- /dev/null +++ b/packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/create-migration-for-foreign-key-comment-alteration.util.ts @@ -0,0 +1,97 @@ +import { DataSource } from 'typeorm'; + +import { TypeORMService } from 'src/database/typeorm/typeorm.service'; +import { DataSourceService } from 'src/engine/metadata-modules/data-source/data-source.service'; +import { RelationToDelete } from 'src/engine/metadata-modules/relation-metadata/types/relation-to-delete'; +import { generateMigrationName } from 'src/engine/metadata-modules/workspace-migration/utils/generate-migration-name.util'; +import { + WorkspaceMigrationTableActionType, + WorkspaceMigrationColumnActionType, + WorkspaceMigrationCreateComment, +} from 'src/engine/metadata-modules/workspace-migration/workspace-migration.entity'; +import { WorkspaceMigrationService } from 'src/engine/metadata-modules/workspace-migration/workspace-migration.service'; +import { computeTableName } from 'src/engine/utils/compute-table-name.util'; + +export const buildAlteredCommentOnForeignKeyDeletion = async ( + localObjectMetadataName: string, + remoteObjectMetadataName: string, + schema: string, + workspaceDataSource: DataSource | undefined, +): Promise => { + const existingComment = await workspaceDataSource?.query( + `SELECT col_description('${schema}."${localObjectMetadataName}"'::regclass, 0)`, + ); + + if (!existingComment[0]?.col_description) { + return null; + } + + const commentWithoutGraphQL = existingComment[0].col_description + .replace('@graphql(', '') + .replace(')', ''); + + const parsedComment = JSON.parse(commentWithoutGraphQL); + + const currentForeignKeys = parsedComment.foreign_keys; + + if (!currentForeignKeys) { + return null; + } + + const updatedForeignKeys = currentForeignKeys.filter( + (foreignKey: any) => + foreignKey.foreign_name !== remoteObjectMetadataName && + foreignKey.foreign_table !== remoteObjectMetadataName, + ); + + parsedComment.foreign_keys = updatedForeignKeys; + + return `@graphql(${JSON.stringify(parsedComment)})`; +}; + +export const createMigrationToAlterCommentOnForeignKeyDeletion = async ( + dataSourceService: DataSourceService, + typeORMService: TypeORMService, + workspaceMigrationService: WorkspaceMigrationService, + workspaceId: string, + relationToDelete: RelationToDelete, +) => { + const dataSourceMetadata = + await dataSourceService.getLastDataSourceMetadataFromWorkspaceIdOrFail( + workspaceId, + ); + + const workspaceDataSource = + await typeORMService.connectToDataSource(dataSourceMetadata); + + const alteredComment = await buildAlteredCommentOnForeignKeyDeletion( + relationToDelete.toObjectName, + relationToDelete.fromObjectName, + dataSourceMetadata.schema, + workspaceDataSource, + ); + + if (alteredComment) { + await workspaceMigrationService.createCustomMigration( + generateMigrationName( + `alter-comment-${relationToDelete.fromObjectName}-${relationToDelete.toObjectName}`, + ), + workspaceId, + [ + { + name: computeTableName( + relationToDelete.toObjectName, + relationToDelete.toObjectMetadataIsCustom, + ), + action: WorkspaceMigrationTableActionType.ALTER, + columns: [ + { + action: WorkspaceMigrationColumnActionType.CREATE_COMMENT, + comment: alteredComment, + } satisfies WorkspaceMigrationCreateComment, + ], + }, + ], + ); + } +}; diff --git a/packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/create-workspace-migrations-for-custom-object.util.ts b/packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/create-migrations-for-custom-object-relations.util.ts similarity index 98% rename from packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/create-workspace-migrations-for-custom-object.util.ts rename to packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/create-migrations-for-custom-object-relations.util.ts index 180cac0e1ff..eb2b71c5bf0 100644 --- a/packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/create-workspace-migrations-for-custom-object.util.ts +++ b/packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/create-migrations-for-custom-object-relations.util.ts @@ -9,7 +9,7 @@ import { } from 'src/engine/metadata-modules/workspace-migration/workspace-migration.entity'; import { computeObjectTargetTable } from 'src/engine/utils/compute-object-target-table.util'; -export const createWorkspaceMigrationsForCustomObject = ( +export const createWorkspaceMigrationsForCustomObjectRelations = ( createdObjectMetadata: ObjectMetadataEntity, activityTargetObjectMetadata: ObjectMetadataEntity, attachmentObjectMetadata: ObjectMetadataEntity, diff --git a/packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/create-workspace-migrations-for-remote-object.util.ts b/packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/create-workspace-migrations-for-remote-object-relations.util.ts similarity index 98% rename from packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/create-workspace-migrations-for-remote-object.util.ts rename to packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/create-workspace-migrations-for-remote-object-relations.util.ts index bf5a6ee02c2..7daf6728ab8 100644 --- a/packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/create-workspace-migrations-for-remote-object.util.ts +++ b/packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/create-workspace-migrations-for-remote-object-relations.util.ts @@ -47,7 +47,7 @@ const buildCommentForRemoteObjectForeignKey = async ( return `@graphql(${JSON.stringify(parsedComment)})`; }; -export const createWorkspaceMigrationsForRemoteObject = async ( +export const createWorkspaceMigrationsForRemoteObjectRelations = async ( createdObjectMetadata: ObjectMetadataEntity, activityTargetObjectMetadata: ObjectMetadataEntity, attachmentObjectMetadata: ObjectMetadataEntity,