From 6e0c1b4c73105f159fe43ec5e38f7a53798b2cd8 Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Sat, 3 Aug 2024 17:53:01 +0200 Subject: [PATCH] Fallback to default value when migrating value from enum (#6517) When migrating the option values of a select type, if the field is non nullable (for now, only available for opportunity's "stage" standard field), we fallback to the (potentially updated) default value instead of nullifying the value to avoid getting a database error. --------- Co-authored-by: Charles Bochet --- .../record-field/types/FieldMetadata.ts | 5 +++ .../types/guards/isFieldPosition.ts | 9 ++++ .../types/guards/isFieldPositionValue.ts | 8 ++++ .../record-field/utils/isFieldValueEmpty.ts | 4 +- .../utils/getCurrencyFieldPreviewValue.ts | 2 +- .../utils/unserialize-default-value.ts | 42 ++++++++++++++++++ ...ations-for-custom-object-relations.util.ts | 8 ++++ .../relation-metadata.service.ts | 35 ++++++++------- .../foreign-table/foreign-table.service.ts | 6 ++- ...s-to-create-remote-table-relations.util.ts | 3 +- .../remote-table-schema-update.service.ts | 6 ++- .../factories/basic-column-action.factory.ts | 16 +++---- .../factories/enum-column-action.factory.ts | 16 +++---- .../workspace-migration.entity.ts | 4 +- .../crons/clean-inactive-workspace.job.ts | 3 +- .../workspace-migration-enum.service.ts | 44 +++++++++++++------ 16 files changed, 154 insertions(+), 57 deletions(-) create mode 100644 packages/twenty-front/src/modules/object-record/record-field/types/guards/isFieldPosition.ts create mode 100644 packages/twenty-front/src/modules/object-record/record-field/types/guards/isFieldPositionValue.ts create mode 100644 packages/twenty-server/src/engine/metadata-modules/field-metadata/utils/unserialize-default-value.ts diff --git a/packages/twenty-front/src/modules/object-record/record-field/types/FieldMetadata.ts b/packages/twenty-front/src/modules/object-record/record-field/types/FieldMetadata.ts index 04d6f6571..7777c10c0 100644 --- a/packages/twenty-front/src/modules/object-record/record-field/types/FieldMetadata.ts +++ b/packages/twenty-front/src/modules/object-record/record-field/types/FieldMetadata.ts @@ -100,6 +100,11 @@ export type FieldRichTextMetadata = { fieldName: string; }; +export type FieldPositionMetadata = { + objectMetadataNameSingular?: string; + fieldName: string; +}; + export type FieldDefinitionRelationType = | 'FROM_MANY_OBJECTS' | 'FROM_ONE_OBJECT' diff --git a/packages/twenty-front/src/modules/object-record/record-field/types/guards/isFieldPosition.ts b/packages/twenty-front/src/modules/object-record/record-field/types/guards/isFieldPosition.ts new file mode 100644 index 000000000..b94970d69 --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-field/types/guards/isFieldPosition.ts @@ -0,0 +1,9 @@ +import { FieldMetadataType } from '~/generated-metadata/graphql'; + +import { FieldDefinition } from '../FieldDefinition'; +import { FieldMetadata, FieldPositionMetadata } from '../FieldMetadata'; + +export const isFieldPosition = ( + field: Pick, 'type'>, +): field is FieldDefinition => + field.type === FieldMetadataType.Position; diff --git a/packages/twenty-front/src/modules/object-record/record-field/types/guards/isFieldPositionValue.ts b/packages/twenty-front/src/modules/object-record/record-field/types/guards/isFieldPositionValue.ts new file mode 100644 index 000000000..a0fd82944 --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/record-field/types/guards/isFieldPositionValue.ts @@ -0,0 +1,8 @@ +import { isNumber } from '@sniptt/guards'; +import { FieldPositionMetadata } from '../FieldMetadata'; + +// TODO: add zod +export const isFieldPhoneValue = ( + fieldValue: unknown, +): fieldValue is FieldPositionMetadata => + isNumber(fieldValue) || fieldValue === 'first' || fieldValue === 'last'; diff --git a/packages/twenty-front/src/modules/object-record/record-field/utils/isFieldValueEmpty.ts b/packages/twenty-front/src/modules/object-record/record-field/utils/isFieldValueEmpty.ts index 1519a2fd1..1cb5296c4 100644 --- a/packages/twenty-front/src/modules/object-record/record-field/utils/isFieldValueEmpty.ts +++ b/packages/twenty-front/src/modules/object-record/record-field/utils/isFieldValueEmpty.ts @@ -22,6 +22,7 @@ import { isFieldMultiSelect } from '@/object-record/record-field/types/guards/is import { isFieldMultiSelectValue } from '@/object-record/record-field/types/guards/isFieldMultiSelectValue'; import { isFieldNumber } from '@/object-record/record-field/types/guards/isFieldNumber'; import { isFieldPhone } from '@/object-record/record-field/types/guards/isFieldPhone'; +import { isFieldPosition } from '@/object-record/record-field/types/guards/isFieldPosition'; import { isFieldRating } from '@/object-record/record-field/types/guards/isFieldRating'; import { isFieldRawJson } from '@/object-record/record-field/types/guards/isFieldRawJson'; import { isFieldRelation } from '@/object-record/record-field/types/guards/isFieldRelation'; @@ -58,7 +59,8 @@ export const isFieldValueEmpty = ({ isFieldRelation(fieldDefinition) || isFieldRawJson(fieldDefinition) || isFieldRichText(fieldDefinition) || - isFieldPhone(fieldDefinition) + isFieldPhone(fieldDefinition) || + isFieldPosition(fieldDefinition) ) { return isValueEmpty(fieldValue); } diff --git a/packages/twenty-front/src/modules/settings/data-model/fields/preview/utils/getCurrencyFieldPreviewValue.ts b/packages/twenty-front/src/modules/settings/data-model/fields/preview/utils/getCurrencyFieldPreviewValue.ts index 408f46505..fc139304b 100644 --- a/packages/twenty-front/src/modules/settings/data-model/fields/preview/utils/getCurrencyFieldPreviewValue.ts +++ b/packages/twenty-front/src/modules/settings/data-model/fields/preview/utils/getCurrencyFieldPreviewValue.ts @@ -18,7 +18,7 @@ export const getCurrencyFieldPreviewValue = ({ const placeholderDefaultValue = getSettingsFieldTypeConfig( FieldMetadataType.Currency, - ).defaultValue; + ).exampleValue; return currencyFieldDefaultValueSchema .transform((value) => ({ diff --git a/packages/twenty-server/src/engine/metadata-modules/field-metadata/utils/unserialize-default-value.ts b/packages/twenty-server/src/engine/metadata-modules/field-metadata/utils/unserialize-default-value.ts new file mode 100644 index 000000000..cfe9a5984 --- /dev/null +++ b/packages/twenty-server/src/engine/metadata-modules/field-metadata/utils/unserialize-default-value.ts @@ -0,0 +1,42 @@ +import { FieldMetadataDefaultSerializableValue } from 'src/engine/metadata-modules/field-metadata/interfaces/field-metadata-default-value.interface'; + +export const unserializeDefaultValue = ( + serializedDefaultValue: FieldMetadataDefaultSerializableValue, +): any => { + if (serializedDefaultValue === null) { + return null; + } + + if (typeof serializedDefaultValue === 'number') { + return serializedDefaultValue; + } + + if (typeof serializedDefaultValue === 'boolean') { + return serializedDefaultValue; + } + + if (typeof serializedDefaultValue === 'string') { + return serializedDefaultValue.replace(/'/g, ''); + } + + if (Array.isArray(serializedDefaultValue)) { + return serializedDefaultValue.map((value) => + unserializeDefaultValue(value), + ); + } + + if (typeof serializedDefaultValue === 'object') { + return Object.entries(serializedDefaultValue).reduce( + (acc, [key, value]) => { + acc[key] = unserializeDefaultValue(value); + + return acc; + }, + {}, + ); + } + + throw new Error( + `Invalid serialized default value "${serializedDefaultValue}"`, + ); +}; diff --git a/packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/build-migrations-for-custom-object-relations.util.ts b/packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/build-migrations-for-custom-object-relations.util.ts index 16a44b460..690e793d3 100644 --- a/packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/build-migrations-for-custom-object-relations.util.ts +++ b/packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/build-migrations-for-custom-object-relations.util.ts @@ -34,6 +34,7 @@ export const buildMigrationsForCustomObjectRelations = ( }), columnType: 'uuid', isNullable: true, + defaultValue: null, } satisfies WorkspaceMigrationColumnCreate, ], }, @@ -64,6 +65,7 @@ export const buildMigrationsForCustomObjectRelations = ( }), columnType: 'uuid', isNullable: true, + defaultValue: null, } satisfies WorkspaceMigrationColumnCreate, ], }, @@ -94,6 +96,7 @@ export const buildMigrationsForCustomObjectRelations = ( }), columnType: 'uuid', isNullable: true, + defaultValue: null, } satisfies WorkspaceMigrationColumnCreate, ], }, @@ -124,6 +127,7 @@ export const buildMigrationsForCustomObjectRelations = ( }), columnType: 'uuid', isNullable: true, + defaultValue: null, } satisfies WorkspaceMigrationColumnCreate, ], }, @@ -154,6 +158,7 @@ export const buildMigrationsForCustomObjectRelations = ( }), columnType: 'uuid', isNullable: true, + defaultValue: null, } satisfies WorkspaceMigrationColumnCreate, ], }, @@ -184,6 +189,7 @@ export const buildMigrationsForCustomObjectRelations = ( }), columnType: 'uuid', isNullable: true, + defaultValue: null, } satisfies WorkspaceMigrationColumnCreate, ], }, @@ -211,6 +217,7 @@ export const buildMigrationsForCustomObjectRelations = ( columnName: 'position', columnType: 'float', isNullable: true, + defaultValue: null, } satisfies WorkspaceMigrationColumnCreate, ], } satisfies WorkspaceMigrationTableAction, @@ -224,6 +231,7 @@ export const buildMigrationsForCustomObjectRelations = ( columnName: 'name', columnType: 'text', defaultValue: "'Untitled'", + isNullable: false, } satisfies WorkspaceMigrationColumnCreate, ], } satisfies WorkspaceMigrationTableAction, diff --git a/packages/twenty-server/src/engine/metadata-modules/relation-metadata/relation-metadata.service.ts b/packages/twenty-server/src/engine/metadata-modules/relation-metadata/relation-metadata.service.ts index b287f9e6f..3f4526870 100644 --- a/packages/twenty-server/src/engine/metadata-modules/relation-metadata/relation-metadata.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/relation-metadata/relation-metadata.service.ts @@ -2,36 +2,36 @@ import { Injectable, NotFoundException } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; import { TypeOrmQueryService } from '@ptc-org/nestjs-query-typeorm'; -import { FindOneOptions, In, Repository } from 'typeorm'; import camelCase from 'lodash.camelcase'; +import { FindOneOptions, In, Repository } from 'typeorm'; import { v4 as uuidV4 } from 'uuid'; -import { ObjectMetadataService } from 'src/engine/metadata-modules/object-metadata/object-metadata.service'; -import { FieldMetadataService } from 'src/engine/metadata-modules/field-metadata/field-metadata.service'; -import { CreateRelationInput } from 'src/engine/metadata-modules/relation-metadata/dtos/create-relation.input'; -import { WorkspaceMigrationRunnerService } from 'src/engine/workspace-manager/workspace-migration-runner/workspace-migration-runner.service'; -import { WorkspaceMigrationService } from 'src/engine/metadata-modules/workspace-migration/workspace-migration.service'; import { FieldMetadataEntity, FieldMetadataType, } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; +import { FieldMetadataService } from 'src/engine/metadata-modules/field-metadata/field-metadata.service'; +import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; +import { ObjectMetadataService } from 'src/engine/metadata-modules/object-metadata/object-metadata.service'; +import { CreateRelationInput } from 'src/engine/metadata-modules/relation-metadata/dtos/create-relation.input'; +import { + RelationMetadataException, + RelationMetadataExceptionCode, +} from 'src/engine/metadata-modules/relation-metadata/relation-metadata.exception'; +import { + InvalidStringException, + validateMetadataNameOrThrow, +} from 'src/engine/metadata-modules/utils/validate-metadata-name.utils'; +import { WorkspaceCacheVersionService } from 'src/engine/metadata-modules/workspace-cache-version/workspace-cache-version.service'; +import { generateMigrationName } from 'src/engine/metadata-modules/workspace-migration/utils/generate-migration-name.util'; import { WorkspaceMigrationColumnActionType, WorkspaceMigrationColumnDrop, WorkspaceMigrationTableActionType, } from 'src/engine/metadata-modules/workspace-migration/workspace-migration.entity'; -import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; +import { WorkspaceMigrationService } from 'src/engine/metadata-modules/workspace-migration/workspace-migration.service'; import { computeObjectTargetTable } from 'src/engine/utils/compute-object-target-table.util'; -import { generateMigrationName } from 'src/engine/metadata-modules/workspace-migration/utils/generate-migration-name.util'; -import { - validateMetadataNameOrThrow, - InvalidStringException, -} from 'src/engine/metadata-modules/utils/validate-metadata-name.utils'; -import { WorkspaceCacheVersionService } from 'src/engine/metadata-modules/workspace-cache-version/workspace-cache-version.service'; -import { - RelationMetadataException, - RelationMetadataExceptionCode, -} from 'src/engine/metadata-modules/relation-metadata/relation-metadata.exception'; +import { WorkspaceMigrationRunnerService } from 'src/engine/workspace-manager/workspace-migration-runner/workspace-migration-runner.service'; import { RelationMetadataEntity, @@ -213,6 +213,7 @@ export class RelationMetadataService extends TypeOrmQueryService option.value)] : undefined, isArray: currentFieldMetadata.type === FieldMetadataType.MULTI_SELECT, - isNullable: currentFieldMetadata.isNullable, + isNullable: currentFieldMetadata.isNullable ?? true, defaultValue: serializeDefaultValue( currentFieldMetadata.defaultValue, ), @@ -112,7 +112,7 @@ export class EnumColumnActionFactory extends ColumnActionAbstractFactory, private readonly dataSourceService: DataSourceService, private readonly objectMetadataService: ObjectMetadataService, diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-migration-runner/services/workspace-migration-enum.service.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-migration-runner/services/workspace-migration-enum.service.ts index 03dd078fa..afd381cb1 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-migration-runner/services/workspace-migration-enum.service.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-migration-runner/services/workspace-migration-enum.service.ts @@ -1,14 +1,15 @@ import { Injectable } from '@nestjs/common'; +import { isDefined } from 'class-validator'; import { QueryRunner, TableColumn } from 'typeorm'; import { v4 } from 'uuid'; -import { isDefined } from 'class-validator'; +import { serializeDefaultValue } from 'src/engine/metadata-modules/field-metadata/utils/serialize-default-value'; +import { unserializeDefaultValue } from 'src/engine/metadata-modules/field-metadata/utils/unserialize-default-value'; import { WorkspaceMigrationColumnAlter, WorkspaceMigrationRenamedEnum, } from 'src/engine/metadata-modules/workspace-migration/workspace-migration.entity'; -import { serializeDefaultValue } from 'src/engine/metadata-modules/field-metadata/utils/serialize-default-value'; @Injectable() export class WorkspaceMigrationEnumService { @@ -111,11 +112,17 @@ export class WorkspaceMigrationEnumService { `); } - private migrateEnumValue( - value: string, - renamedEnumValues?: WorkspaceMigrationRenamedEnum[], - allEnumValues?: string[], - ) { + private migrateEnumValue({ + value, + renamedEnumValues, + allEnumValues, + defaultValueFallback, + }: { + value: string; + renamedEnumValues?: WorkspaceMigrationRenamedEnum[]; + allEnumValues?: string[]; + defaultValueFallback?: string; + }) { if (renamedEnumValues?.find((enumVal) => enumVal?.from === value)?.to) { return renamedEnumValues?.find((enumVal) => enumVal?.from === value)?.to; } @@ -124,6 +131,10 @@ export class WorkspaceMigrationEnumService { return value; } + if (isDefined(defaultValueFallback)) { + return defaultValueFallback; + } + return null; } @@ -152,16 +163,23 @@ export class WorkspaceMigrationEnumService { .split(',') .map((v: string) => v.trim()) .map((v: string) => - this.migrateEnumValue(v, renamedEnumValues, enumValues), + this.migrateEnumValue({ + value: v, + renamedEnumValues: renamedEnumValues, + allEnumValues: enumValues, + }), ) .filter((v: string | null) => isDefined(v)), ); } else if (typeof val === 'string') { - const migratedValue = this.migrateEnumValue( - val, - renamedEnumValues, - enumValues, - ); + const migratedValue = this.migrateEnumValue({ + value: val, + renamedEnumValues: renamedEnumValues, + allEnumValues: enumValues, + defaultValueFallback: columnDefinition.isNullable + ? null + : unserializeDefaultValue(columnDefinition.defaultValue), + }); val = isDefined(migratedValue) ? `'${migratedValue}'` : null; }