From 1abe1e7654ed7ee6a064659132e66e08905c379c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Magrin?= Date: Thu, 21 Mar 2024 16:09:06 +0100 Subject: [PATCH] feat: wip health check fix default value --- .../dtos/default-value.input.ts | 6 +- .../utils/is-function-default-value.util.ts | 10 +- .../fixer/workspace-default-value.fixer.ts | 126 +++++++++++++++++- .../services/database-structure.service.ts | 48 +++++-- .../services/field-metadata-health.service.ts | 60 ++++----- .../services/workspace-fix.service.ts | 10 ++ .../calendar-channel.object-metadata.ts | 2 +- ...calendar-event-attendee.object-metadata.ts | 4 +- 8 files changed, 203 insertions(+), 63 deletions(-) diff --git a/packages/twenty-server/src/engine/metadata-modules/field-metadata/dtos/default-value.input.ts b/packages/twenty-server/src/engine/metadata-modules/field-metadata/dtos/default-value.input.ts index c0cb5e413e884..d538160ea7894 100644 --- a/packages/twenty-server/src/engine/metadata-modules/field-metadata/dtos/default-value.input.ts +++ b/packages/twenty-server/src/engine/metadata-modules/field-metadata/dtos/default-value.input.ts @@ -6,7 +6,6 @@ import { IsNotEmpty, IsNumber, IsNumberString, - IsString, Matches, } from 'class-validator'; @@ -17,6 +16,9 @@ export const fieldMetadataDefaultValueFunctionName = { NOW: 'now', } as const; +export type FieldMetadataDefaultValueFunctionNames = + (typeof fieldMetadataDefaultValueFunctionName)[keyof typeof fieldMetadataDefaultValueFunctionName]; + export class FieldMetadataDefaultValueString { @IsQuotedString() value: string | null; @@ -75,13 +77,11 @@ export class FieldMetadataDefaultValueFullName { export class FieldMetadataDefaultValueUuidFunction { @Matches(fieldMetadataDefaultValueFunctionName.UUID) @IsNotEmpty() - @IsString() value: typeof fieldMetadataDefaultValueFunctionName.UUID; } export class FieldMetadataDefaultValueNowFunction { @Matches(fieldMetadataDefaultValueFunctionName.NOW) @IsNotEmpty() - @IsString() value: typeof fieldMetadataDefaultValueFunctionName.NOW; } diff --git a/packages/twenty-server/src/engine/metadata-modules/field-metadata/utils/is-function-default-value.util.ts b/packages/twenty-server/src/engine/metadata-modules/field-metadata/utils/is-function-default-value.util.ts index 50ed3f1614934..78065b5386581 100644 --- a/packages/twenty-server/src/engine/metadata-modules/field-metadata/utils/is-function-default-value.util.ts +++ b/packages/twenty-server/src/engine/metadata-modules/field-metadata/utils/is-function-default-value.util.ts @@ -3,10 +3,10 @@ import { FieldMetadataFunctionDefaultValue, } from 'src/engine/metadata-modules/field-metadata/interfaces/field-metadata-default-value.interface'; -import { fieldMetadataDefaultValueFunctionName } from 'src/engine/metadata-modules/field-metadata/dtos/default-value.input'; - -type FieldMetadataDefaultValueFunctionNameUnion = - (typeof fieldMetadataDefaultValueFunctionName)[keyof typeof fieldMetadataDefaultValueFunctionName]; +import { + FieldMetadataDefaultValueFunctionNames, + fieldMetadataDefaultValueFunctionName, +} from 'src/engine/metadata-modules/field-metadata/dtos/default-value.input'; export const isFunctionDefaultValue = ( defaultValue: FieldMetadataDefaultSerializableValue, @@ -15,7 +15,7 @@ export const isFunctionDefaultValue = ( typeof defaultValue === 'string' && !defaultValue.startsWith("'") && Object.values(fieldMetadataDefaultValueFunctionName).includes( - defaultValue as FieldMetadataDefaultValueFunctionNameUnion, + defaultValue as FieldMetadataDefaultValueFunctionNames, ) ); }; diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-health/fixer/workspace-default-value.fixer.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-health/fixer/workspace-default-value.fixer.ts index 9beb6df32adb6..1950a61773a77 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-health/fixer/workspace-default-value.fixer.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-health/fixer/workspace-default-value.fixer.ts @@ -12,30 +12,72 @@ import { FieldMetadataDefaultValue } from 'src/engine/metadata-modules/field-met import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; import { WorkspaceMigrationEntity } from 'src/engine/metadata-modules/workspace-migration/workspace-migration.entity'; import { WorkspaceMigrationFieldFactory } from 'src/engine/workspace-manager/workspace-migration-builder/factories/workspace-migration-field.factory'; +import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; +import { + FieldMetadataDefaultValueFunctionNames, + fieldMetadataDefaultValueFunctionName, +} from 'src/engine/metadata-modules/field-metadata/dtos/default-value.input'; + +import { + AbstractWorkspaceFixer, + CompareEntity, +} from './abstract-workspace.fixer'; -import { AbstractWorkspaceFixer } from './abstract-workspace.fixer'; +type WorkspaceDefaultValueFixerType = + | WorkspaceHealthIssueType.COLUMN_DEFAULT_VALUE_CONFLICT + | WorkspaceHealthIssueType.COLUMN_DEFAULT_VALUE_NOT_VALID; @Injectable() -export class WorkspaceDefaultValueFixer extends AbstractWorkspaceFixer { +export class WorkspaceDefaultValueFixer extends AbstractWorkspaceFixer { constructor( private readonly workspaceMigrationFieldFactory: WorkspaceMigrationFieldFactory, ) { - super(WorkspaceHealthIssueType.COLUMN_DEFAULT_VALUE_CONFLICT); + super( + WorkspaceHealthIssueType.COLUMN_DEFAULT_VALUE_CONFLICT, + WorkspaceHealthIssueType.COLUMN_DEFAULT_VALUE_NOT_VALID, + ); } async createWorkspaceMigrations( manager: EntityManager, objectMetadataCollection: ObjectMetadataEntity[], - issues: WorkspaceHealthColumnIssue[], + issues: WorkspaceHealthColumnIssue[], ): Promise[]> { if (issues.length <= 0) { return []; } + const splittedIssues = this.splitIssuesByType(issues); + const issueNeedingMigration = + splittedIssues[WorkspaceHealthIssueType.COLUMN_DEFAULT_VALUE_CONFLICT] ?? + []; + + return this.fixColumnDefaultValueConflictIssues( + objectMetadataCollection, + issueNeedingMigration as WorkspaceHealthColumnIssue[], + ); + } + + async createMetadataUpdates( + manager: EntityManager, + objectMetadataCollection: ObjectMetadataEntity[], + issues: WorkspaceHealthColumnIssue[], + ): Promise[]> { + if (issues.length <= 0) { + return []; + } - return this.fixColumnDefaultValueIssues(objectMetadataCollection, issues); + const splittedIssues = this.splitIssuesByType(issues); + const issueNeedingMetadataUpdate = + splittedIssues[WorkspaceHealthIssueType.COLUMN_DEFAULT_VALUE_NOT_VALID] ?? + []; + + return this.fixColumnDefaultValueNotValidIssues( + manager, + issueNeedingMetadataUpdate as WorkspaceHealthColumnIssue[], + ); } - private async fixColumnDefaultValueIssues( + private async fixColumnDefaultValueConflictIssues( objectMetadataCollection: ObjectMetadataEntity[], issues: WorkspaceHealthColumnIssue[], ): Promise[]> { @@ -61,6 +103,78 @@ export class WorkspaceDefaultValueFixer extends AbstractWorkspaceFixer[], + ): Promise[]> { + const fieldMetadataRepository = manager.getRepository(FieldMetadataEntity); + const updatedEntities: CompareEntity[] = []; + + for (const issue of issues) { + const currentDefaultValue: + | FieldMetadataDefaultValue<'default'> + // Old format for default values + // TODO: Remove this after all workspaces are migrated + | { type: FieldMetadataDefaultValueFunctionNames } + | null = issue.fieldMetadata.defaultValue; + let alteredDefaultValue: FieldMetadataDefaultValue<'default'> | null = + null; + + // Check if it's an old function default value + if (currentDefaultValue && 'type' in currentDefaultValue) { + alteredDefaultValue = { + value: + currentDefaultValue.type as FieldMetadataDefaultValueFunctionNames, + }; + } + + // Check if it's an old string default value + if (currentDefaultValue) { + for (const key of Object.keys(currentDefaultValue)) { + if (key === 'type') { + continue; + } + + const value = currentDefaultValue[key]; + + if ( + typeof value === 'string' && + !value.startsWith("'") && + !fieldMetadataDefaultValueFunctionName[value] + ) { + // This error is expcted because we're creating a default value in a loop and key is typed a string + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-expect-error + alteredDefaultValue = { + ...alteredDefaultValue, + [key]: `'${value}'`, + }; + } + } + } + + if (alteredDefaultValue === null) { + continue; + } + + await fieldMetadataRepository.update(issue.fieldMetadata.id, { + defaultValue: alteredDefaultValue, + }); + const alteredEntity = await fieldMetadataRepository.findOne({ + where: { + id: issue.fieldMetadata.id, + }, + }); + + updatedEntities.push({ + current: issue.fieldMetadata, + altered: alteredEntity as FieldMetadataEntity | null, + }); + } + + return updatedEntities; + } + private computeFieldMetadataDefaultValueFromColumnDefault( columnDefault: string | undefined, ): FieldMetadataDefaultValue<'default'> { diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-health/services/database-structure.service.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-health/services/database-structure.service.ts index 89eec407dbd72..51b30dbf1c403 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-health/services/database-structure.service.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-health/services/database-structure.service.ts @@ -7,7 +7,10 @@ import { WorkspaceTableStructure, WorkspaceTableStructureResult, } from 'src/engine/workspace-manager/workspace-health/interfaces/workspace-table-definition.interface'; -import { FieldMetadataDefaultValue } from 'src/engine/metadata-modules/field-metadata/interfaces/field-metadata-default-value.interface'; +import { + FieldMetadataDefaultValue, + FieldMetadataFunctionDefaultValue, +} from 'src/engine/metadata-modules/field-metadata/interfaces/field-metadata-default-value.interface'; import { TypeORMService } from 'src/database/typeorm/typeorm.service'; import { @@ -19,6 +22,7 @@ import { serializeFunctionDefaultValue } from 'src/engine/metadata-modules/field import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util'; import { isRelationFieldMetadataType } from 'src/engine/utils/is-relation-field-metadata-type.util'; import { isFunctionDefaultValue } from 'src/engine/metadata-modules/field-metadata/utils/is-function-default-value.util'; +import { FieldMetadataDefaultValueFunctionNames } from 'src/engine/metadata-modules/field-metadata/dtos/default-value.input'; @Injectable() export class DatabaseStructureService { @@ -205,30 +209,39 @@ export class DatabaseStructureService { getPostgresDefault( fieldMetadataType: FieldMetadataType, - defaultValue: FieldMetadataDefaultValue | null, + defaultValue: + | FieldMetadataDefaultValue + // Old format for default values + // TODO: Should be removed once all default values are migrated + | { type: FieldMetadataDefaultValueFunctionNames } + | null, ): string | null | undefined { const typeORMType = fieldMetadataTypeToColumnType( fieldMetadataType, ) as ColumnType; const mainDataSource = this.typeORMService.getMainDataSource(); - const value = + let value = defaultValue && 'value' in defaultValue ? defaultValue.value : null; - if (isFunctionDefaultValue(value)) { - const serializedDefaultValue = serializeFunctionDefaultValue(value); - - // Special case for uuid_generate_v4() default value - if (serializedDefaultValue === 'public.uuid_generate_v4()') { - return 'uuid_generate_v4()'; - } + // Old format for default values + // TODO: Should be removed once all default values are migrated + if (defaultValue && 'type' in defaultValue) { + return this.computeFunctionDefaultValue(defaultValue.type); + } - return serializedDefaultValue; + if (isFunctionDefaultValue(value)) { + return this.computeFunctionDefaultValue(value); } if (typeof value === 'number') { return value.toString(); } + // Remove leading and trailing single quotes for string default values as it's already handled by TypeORM + if (typeof value === 'string' && value.match(/^'.*'$/)) { + value = value.replace(/^'/, '').replace(/'$/, ''); + } + return mainDataSource.driver.normalizeDefault({ type: typeORMType, default: value, @@ -236,4 +249,17 @@ export class DatabaseStructureService { // Workaround to use normalizeDefault without a complete ColumnMetadata object } as ColumnMetadata); } + + private computeFunctionDefaultValue( + value: FieldMetadataFunctionDefaultValue['value'], + ) { + const serializedDefaultValue = serializeFunctionDefaultValue(value); + + // Special case for uuid_generate_v4() default value + if (serializedDefaultValue === 'public.uuid_generate_v4()') { + return 'uuid_generate_v4()'; + } + + return serializedDefaultValue; + } } diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-health/services/field-metadata-health.service.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-health/services/field-metadata-health.service.ts index a8dc1595f8c55..09b487a5e5e56 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-health/services/field-metadata-health.service.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-health/services/field-metadata-health.service.ts @@ -1,4 +1,4 @@ -import { Injectable, InternalServerErrorException } from '@nestjs/common'; +import { Injectable } from '@nestjs/common'; import isEqual from 'lodash.isequal'; @@ -67,18 +67,30 @@ export class FieldMetadataHealthService { issues.push(...defaultValueIssues); } - for (const compositeFieldMetadata of compositeFieldMetadataCollection) { - const compositeFieldIssues = await this.healthCheckField( + // Only check structure on nested composite fields + if (options.mode === 'structure' || options.mode === 'all') { + for (const compositeFieldMetadata of compositeFieldMetadataCollection) { + const compositeFieldStructureIssues = this.structureFieldCheck( + tableName, + workspaceTableColumns, + computeCompositeFieldMetadata( + compositeFieldMetadata, + fieldMetadata, + ), + ); + + issues.push(...compositeFieldStructureIssues); + } + } + + // Only check metadata on the parent composite field + if (options.mode === 'metadata' || options.mode === 'all') { + const compositeFieldMetadataIssues = this.metadataFieldCheck( tableName, - workspaceTableColumns, - computeCompositeFieldMetadata( - compositeFieldMetadata, - fieldMetadata, - ), - options, + fieldMetadata, ); - issues.push(...compositeFieldIssues); + issues.push(...compositeFieldMetadataIssues); } } else { const fieldIssues = await this.healthCheckField( @@ -137,6 +149,7 @@ export class FieldMetadataHealthService { fieldMetadata.type, fieldMetadata.defaultValue, ); + // Check if column exist in database const columnStructure = workspaceTableColumns.find( (tableDefinition) => tableDefinition.columnName === columnName, @@ -178,7 +191,7 @@ export class FieldMetadataHealthService { if (columnDefaultValue && isEnumFieldMetadataType(fieldMetadata.type)) { const enumValues = fieldMetadata.options?.map((option) => - serializeDefaultValue(option.value), + serializeDefaultValue(`'${option.value}'`), ); if (!enumValues.includes(columnDefaultValue)) { @@ -341,29 +354,4 @@ export class FieldMetadataHealthService { return issues; } - - private isCompositeObjectWellStructured( - fieldMetadataType: FieldMetadataType, - object: any, - ): boolean { - const subFields = compositeDefinitions.get(fieldMetadataType)?.() ?? []; - - if (!object) { - return true; - } - - if (subFields.length === 0) { - throw new InternalServerErrorException( - `The composite field type ${fieldMetadataType} doesn't have any sub fields, it seems this one is not implemented in the composite definitions map`, - ); - } - - for (const subField of subFields) { - if (!object[subField.name]) { - return false; - } - } - - return true; - } } diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-health/services/workspace-fix.service.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-health/services/workspace-fix.service.ts index 523ba45630681..dae84af7a49b2 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-health/services/workspace-fix.service.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-health/services/workspace-fix.service.ts @@ -90,6 +90,16 @@ export class WorkspaceFixService { filteredIssues, ); } + case WorkspaceHealthFixKind.DefaultValue: { + const filteredIssues = + this.workspaceDefaultValueFixer.filterIssues(issues); + + return this.workspaceDefaultValueFixer.createMetadataUpdates( + manager, + objectMetadataCollection, + filteredIssues, + ); + } default: { return []; } diff --git a/packages/twenty-server/src/modules/calendar/standard-objects/calendar-channel.object-metadata.ts b/packages/twenty-server/src/modules/calendar/standard-objects/calendar-channel.object-metadata.ts index 8acb2b869113f..be0e48c48ff21 100644 --- a/packages/twenty-server/src/modules/calendar/standard-objects/calendar-channel.object-metadata.ts +++ b/packages/twenty-server/src/modules/calendar/standard-objects/calendar-channel.object-metadata.ts @@ -72,7 +72,7 @@ export class CalendarChannelObjectMetadata extends BaseObjectMetadata { color: 'orange', }, ], - defaultValue: { value: CalendarChannelVisibility.SHARE_EVERYTHING }, + defaultValue: { value: `'${CalendarChannelVisibility.SHARE_EVERYTHING}'` }, }) visibility: string; diff --git a/packages/twenty-server/src/modules/calendar/standard-objects/calendar-event-attendee.object-metadata.ts b/packages/twenty-server/src/modules/calendar/standard-objects/calendar-event-attendee.object-metadata.ts index f346d8eb06db9..0a3a7038d66af 100644 --- a/packages/twenty-server/src/modules/calendar/standard-objects/calendar-event-attendee.object-metadata.ts +++ b/packages/twenty-server/src/modules/calendar/standard-objects/calendar-event-attendee.object-metadata.ts @@ -100,7 +100,9 @@ export class CalendarEventAttendeeObjectMetadata extends BaseObjectMetadata { color: 'green', }, ], - defaultValue: { value: CalendarEventAttendeeResponseStatus.NEEDS_ACTION }, + defaultValue: { + value: `'${CalendarEventAttendeeResponseStatus.NEEDS_ACTION}'`, + }, }) responseStatus: string;