[Fix] Prevent fields name conflicts with composite subfields names (#…

…6713) At field creation we are checking the availability of the name by comparing it to the other fields' names' on the object; but for composite fields the fields' names' as indicated in the repository do not exactly match the column names' on the tables (e.g "createdBy" field is actually represented by columns createdByName, createdBySource etc.). In this PR we prevent the conflict with the standard composite fields' names. There is still room for errors with the custom composite fields: for example a custom composite field "address" of type address on a custom object "listing" will introduce the columns addressAddressStreet1, addressAddressStreet2 etc. while we won't prevent the user from later creating a custom field named "addressAddressStreet1". For now I decided not to tackle this as this seem extremely edgy + would impact performance on creation of all fields while never actually useful (I think).
twentyhq · Aug 23, 2024 · ee6180a · ee6180a
1 parent 981f311
commit ee6180a
Show file tree

Hide file tree

Showing 27 changed files with 275 additions and 166 deletions.
diff --git a/packages/twenty-server/src/engine/api/__mocks__/object-metadata-item.mock.ts b/packages/twenty-server/src/engine/api/__mocks__/object-metadata-item.mock.ts
@@ -2,6 +2,12 @@ import { FieldActorSource } from 'src/engine/metadata-modules/field-metadata/com
 import { FieldMetadataType } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity';
 import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity';
 
+export const FIELD_LINKS_MOCK_NAME = 'fieldLinks';
+export const FIELD_CURRENCY_MOCK_NAME = 'fieldCurrency';
+export const FIELD_ADDRESS_MOCK_NAME = 'fieldAddress';
+export const FIELD_ACTOR_MOCK_NAME = 'fieldActor';
+export const FIELD_FULL_NAME_MOCK_NAME = 'fieldFullName';
+
 export const fieldNumberMock = {
   name: 'fieldNumber',
   type: FieldMetadataType.NUMBER,
@@ -16,15 +22,8 @@ export const fieldTextMock = {
   defaultValue: null,
 };
 
-export const fieldLinkMock = {
-  name: 'fieldLink',
-  type: FieldMetadataType.LINK,
-  isNullable: false,
-  defaultValue: { label: '', url: '' },
-};
-
 export const fieldCurrencyMock = {
-  name: 'fieldCurrency',
+  name: FIELD_CURRENCY_MOCK_NAME,
   type: FieldMetadataType.CURRENCY,
   isNullable: true,
   defaultValue: { amountMicros: null, currencyCode: "''" },
@@ -89,7 +88,7 @@ export const fieldRelationMock = {
 };
 
 const fieldLinksMock = {
-  name: 'fieldLinks',
+  name: FIELD_LINKS_MOCK_NAME,
   type: FieldMetadataType.LINKS,
   isNullable: false,
   defaultValue: [
@@ -147,7 +146,7 @@ const fieldNumericMock = {
 };
 
 const fieldFullNameMock = {
-  name: 'fieldFullName',
+  name: FIELD_FULL_NAME_MOCK_NAME,
   type: FieldMetadataType.FULL_NAME,
   isNullable: true,
   defaultValue: { firstName: '', lastName: '' },
@@ -168,7 +167,7 @@ const fieldPositionMock = {
 };
 
 const fieldAddressMock = {
-  name: 'fieldAddress',
+  name: FIELD_ADDRESS_MOCK_NAME,
   type: FieldMetadataType.ADDRESS,
   isNullable: true,
   defaultValue: {
@@ -198,7 +197,7 @@ const fieldRichTextMock = {
 };
 
 const fieldActorMock = {
-  name: 'fieldActor',
+  name: FIELD_ACTOR_MOCK_NAME,
   type: FieldMetadataType.ACTOR,
   isNullable: true,
   defaultValue: {
@@ -217,7 +216,6 @@ export const fields = [
   fieldBooleanMock,
   fieldNumberMock,
   fieldNumericMock,
-  fieldLinkMock,
   fieldLinksMock,
   fieldCurrencyMock,
   fieldFullNameMock,

diff --git a/...nty-server/src/engine/api/graphql/workspace-query-builder/factories/args-alias.factory.ts b/...nty-server/src/engine/api/graphql/workspace-query-builder/factories/args-alias.factory.ts
@@ -2,7 +2,7 @@ import { Injectable, Logger } from '@nestjs/common';
 
 import { FieldMetadataInterface } from 'src/engine/metadata-modules/field-metadata/interfaces/field-metadata.interface';
 
-import { compositeTypeDefintions } from 'src/engine/metadata-modules/field-metadata/composite-types';
+import { compositeTypeDefinitions } from 'src/engine/metadata-modules/field-metadata/composite-types';
 import { computeCompositeColumnName } from 'src/engine/metadata-modules/field-metadata/utils/compute-column-name.util';
 import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util';
 
@@ -52,7 +52,7 @@ export class ArgsAliasFactory {
         isCompositeFieldMetadataType(fieldMetadata.type)
       ) {
         // Get composite type definition
-        const compositeType = compositeTypeDefintions.get(fieldMetadata.type);
+        const compositeType = compositeTypeDefinitions.get(fieldMetadata.type);
 
         if (!compositeType) {
           this.logger.error(

diff --git a/...ty-server/src/engine/api/graphql/workspace-query-builder/factories/field-alias.factory.ts b/...ty-server/src/engine/api/graphql/workspace-query-builder/factories/field-alias.factory.ts
@@ -2,13 +2,13 @@ import { Injectable, Logger } from '@nestjs/common';
 
 import { FieldMetadataInterface } from 'src/engine/metadata-modules/field-metadata/interfaces/field-metadata.interface';
 
-import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util';
-import { compositeTypeDefintions } from 'src/engine/metadata-modules/field-metadata/composite-types';
 import { createCompositeFieldKey } from 'src/engine/api/graphql/workspace-query-builder/utils/composite-field-metadata.util';
+import { compositeTypeDefinitions } from 'src/engine/metadata-modules/field-metadata/composite-types';
 import {
   computeColumnName,
   computeCompositeColumnName,
 } from 'src/engine/metadata-modules/field-metadata/utils/compute-column-name.util';
+import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util';
 
 @Injectable()
 export class FieldAliasFactory {
@@ -23,7 +23,7 @@ export class FieldAliasFactory {
     }
 
     // If it's a composite field, we need to get the definition
-    const compositeType = compositeTypeDefintions.get(fieldMetadata.type);
+    const compositeType = compositeTypeDefinitions.get(fieldMetadata.type);
 
     if (!compositeType) {
       this.logger.error(

diff --git a/...enty-server/src/engine/api/graphql/workspace-schema-builder/type-definitions.generator.ts b/...enty-server/src/engine/api/graphql/workspace-schema-builder/type-definitions.generator.ts
@@ -1,27 +1,27 @@
 import { Injectable, Logger } from '@nestjs/common';
 
-import { ObjectMetadataInterface } from 'src/engine/metadata-modules/field-metadata/interfaces/object-metadata.interface';
 import { CompositeType } from 'src/engine/metadata-modules/field-metadata/interfaces/composite-type.interface';
+import { ObjectMetadataInterface } from 'src/engine/metadata-modules/field-metadata/interfaces/object-metadata.interface';
 
-import { EnumTypeDefinitionFactory } from 'src/engine/api/graphql/workspace-schema-builder/factories/enum-type-definition.factory';
-import { compositeTypeDefintions } from 'src/engine/metadata-modules/field-metadata/composite-types';
-import { CompositeObjectTypeDefinitionFactory } from 'src/engine/api/graphql/workspace-schema-builder/factories/composite-object-type-definition.factory';
-import { CompositeInputTypeDefinitionFactory } from 'src/engine/api/graphql/workspace-schema-builder/factories/composite-input-type-definition.factory';
 import { CompositeEnumTypeDefinitionFactory } from 'src/engine/api/graphql/workspace-schema-builder/factories/composite-enum-type-definition.factory';
+import { CompositeInputTypeDefinitionFactory } from 'src/engine/api/graphql/workspace-schema-builder/factories/composite-input-type-definition.factory';
+import { CompositeObjectTypeDefinitionFactory } from 'src/engine/api/graphql/workspace-schema-builder/factories/composite-object-type-definition.factory';
+import { EnumTypeDefinitionFactory } from 'src/engine/api/graphql/workspace-schema-builder/factories/enum-type-definition.factory';
+import { compositeTypeDefinitions } from 'src/engine/metadata-modules/field-metadata/composite-types';
 
-import { TypeDefinitionsStorage } from './storages/type-definitions.storage';
-import {
-  ObjectTypeDefinitionFactory,
-  ObjectTypeDefinitionKind,
-} from './factories/object-type-definition.factory';
+import { ConnectionTypeDefinitionFactory } from './factories/connection-type-definition.factory';
+import { EdgeTypeDefinitionFactory } from './factories/edge-type-definition.factory';
+import { ExtendObjectTypeDefinitionFactory } from './factories/extend-object-type-definition.factory';
 import {
   InputTypeDefinitionFactory,
   InputTypeDefinitionKind,
 } from './factories/input-type-definition.factory';
+import {
+  ObjectTypeDefinitionFactory,
+  ObjectTypeDefinitionKind,
+} from './factories/object-type-definition.factory';
 import { WorkspaceBuildSchemaOptions } from './interfaces/workspace-build-schema-optionts.interface';
-import { ConnectionTypeDefinitionFactory } from './factories/connection-type-definition.factory';
-import { EdgeTypeDefinitionFactory } from './factories/edge-type-definition.factory';
-import { ExtendObjectTypeDefinitionFactory } from './factories/extend-object-type-definition.factory';
+import { TypeDefinitionsStorage } from './storages/type-definitions.storage';
 import { objectContainsRelationField } from './utils/object-contains-relation-field';
 
 @Injectable()
@@ -55,7 +55,7 @@ export class TypeDefinitionsGenerator {
    * GENERATE COMPOSITE TYPE OBJECTS
    */
   private generateCompositeTypeDefs(options: WorkspaceBuildSchemaOptions) {
-    const compositeTypeCollection = [...compositeTypeDefintions.values()];
+    const compositeTypeCollection = [...compositeTypeDefinitions.values()];
 
     this.logger.log(
       `Generating composite type objects: [${compositeTypeCollection

diff --git a/...rest/core/query-builder/utils/__tests__/map-field-metadata-to-graphql-query.utils.spec.ts b/...rest/core/query-builder/utils/__tests__/map-field-metadata-to-graphql-query.utils.spec.ts
@@ -1,6 +1,5 @@
 import {
   fieldCurrencyMock,
-  fieldLinkMock,
   fieldNumberMock,
   fieldTextMock,
   objectMetadataItemMock,
@@ -17,15 +16,6 @@ describe('mapFieldMetadataToGraphqlQuery', () => {
     expect(
       mapFieldMetadataToGraphqlQuery([objectMetadataItemMock], fieldTextMock),
     ).toEqual('fieldText');
-    expect(
-      mapFieldMetadataToGraphqlQuery([objectMetadataItemMock], fieldLinkMock),
-    ).toEqual(`
-      fieldLink
-      {
-        label
-        url
-      }
-    `);
     expect(
       mapFieldMetadataToGraphqlQuery(
         [objectMetadataItemMock],

diff --git a/packages/twenty-server/src/engine/api/rest/core/query-builder/utils/check-fields.utils.ts b/packages/twenty-server/src/engine/api/rest/core/query-builder/utils/check-fields.utils.ts
@@ -2,7 +2,7 @@ import { BadRequestException } from '@nestjs/common';
 
 import { ObjectMetadataInterface } from 'src/engine/metadata-modules/field-metadata/interfaces/object-metadata.interface';
 
-import { compositeTypeDefintions } from 'src/engine/metadata-modules/field-metadata/composite-types';
+import { compositeTypeDefinitions } from 'src/engine/metadata-modules/field-metadata/composite-types';
 import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util';
 import { computeObjectTargetTable } from 'src/engine/utils/compute-object-target-table.util';
 
@@ -13,7 +13,7 @@ export const checkFields = (
   const fieldMetadataNames = objectMetadata.fields
     .map((field) => {
       if (isCompositeFieldMetadataType(field.type)) {
-        const compositeType = compositeTypeDefintions.get(field.type);
+        const compositeType = compositeTypeDefinitions.get(field.type);
 
         if (!compositeType) {
           throw new BadRequestException(

diff --git a/packages/twenty-server/src/engine/api/rest/core/query-builder/utils/check-order-by.utils.ts b/packages/twenty-server/src/engine/api/rest/core/query-builder/utils/check-order-by.utils.ts
@@ -1,9 +1,9 @@
 import { BadRequestException } from '@nestjs/common';
 
-import { ObjectMetadataInterface } from 'src/engine/metadata-modules/field-metadata/interfaces/object-metadata.interface';
 import { Record } from 'src/engine/api/graphql/workspace-query-builder/interfaces/record.interface';
+import { ObjectMetadataInterface } from 'src/engine/metadata-modules/field-metadata/interfaces/object-metadata.interface';
 
-import { compositeTypeDefintions } from 'src/engine/metadata-modules/field-metadata/composite-types';
+import { compositeTypeDefinitions } from 'src/engine/metadata-modules/field-metadata/composite-types';
 import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util';
 import { computeObjectTargetTable } from 'src/engine/utils/compute-object-target-table.util';
 
@@ -14,7 +14,7 @@ export const checkArrayFields = (
   const fieldMetadataNames = objectMetadata.fields
     .map((field) => {
       if (isCompositeFieldMetadataType(field.type)) {
-        const compositeType = compositeTypeDefintions.get(field.type);
+        const compositeType = compositeTypeDefinitions.get(field.type);
 
         if (!compositeType) {
           throw new BadRequestException(

diff --git a/...wenty-server/src/engine/api/rest/input-factories/__tests__/order-by-input.factory.spec.ts b/...wenty-server/src/engine/api/rest/input-factories/__tests__/order-by-input.factory.spec.ts
@@ -54,7 +54,7 @@ describe('OrderByInputFactory', () => {
       ]);
     });
 
-    it('should handler complex fields', () => {
+    it('should handle complex fields', () => {
       const request: any = {
         query: {
           order_by: 'fieldCurrency.amountMicros',
@@ -66,7 +66,7 @@ describe('OrderByInputFactory', () => {
       ]);
     });
 
-    it('should handler complex fields with direction', () => {
+    it('should handle complex fields with direction', () => {
       const request: any = {
         query: {
           order_by: 'fieldCurrency.amountMicros[DescNullsLast]',
@@ -78,17 +78,17 @@ describe('OrderByInputFactory', () => {
       ]);
     });
 
-    it('should handler multiple complex fields with direction', () => {
+    it('should handle multiple complex fields with direction', () => {
       const request: any = {
         query: {
           order_by:
-            'fieldCurrency.amountMicros[DescNullsLast],fieldLink.label[AscNullsLast]',
+            'fieldCurrency.amountMicros[DescNullsLast],fieldText.label[AscNullsLast]',
         },
       };
 
       expect(service.create(request, objectMetadata)).toEqual([
         { fieldCurrency: { amountMicros: OrderByDirection.DescNullsLast } },
-        { fieldLink: { label: OrderByDirection.AscNullsLast } },
+        { fieldText: { label: OrderByDirection.AscNullsLast } },
       ]);
     });
 

diff --git a/...s/twenty-server/src/engine/core-modules/open-api/utils/__tests__/components.utils.spec.ts b/...s/twenty-server/src/engine/core-modules/open-api/utils/__tests__/components.utils.spec.ts
@@ -7,9 +7,11 @@ import { FieldMetadataType } from 'src/engine/metadata-modules/field-metadata/fi
 import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity';
 
 describe('computeSchemaComponents', () => {
-  it('should test all field types', () => {
+  it('should test all non-deprecated field types', () => {
     expect(fields.map((field) => field.type)).toEqual(
-      Object.keys(FieldMetadataType),
+      Object.keys(FieldMetadataType).filter(
+        (key) => key !== FieldMetadataType.LINK,
+      ),
     );
   });
   it('should compute schema components', () => {
@@ -55,13 +57,6 @@ describe('computeSchemaComponents', () => {
           fieldNumeric: {
             type: 'number',
           },
-          fieldLink: {
-            properties: {
-              label: { type: 'string' },
-              url: { type: 'string' },
-            },
-            type: 'object',
-          },
           fieldLinks: {
             properties: {
               primaryLinkLabel: { type: 'string' },

diff --git a/packages/twenty-server/src/engine/core-modules/open-api/utils/components.utils.ts b/packages/twenty-server/src/engine/core-modules/open-api/utils/components.utils.ts
@@ -9,7 +9,7 @@ import {
   computeOrderByParameters,
   computeStartingAfterParameters,
 } from 'src/engine/core-modules/open-api/utils/parameters.utils';
-import { compositeTypeDefintions } from 'src/engine/metadata-modules/field-metadata/composite-types';
+import { compositeTypeDefinitions } from 'src/engine/metadata-modules/field-metadata/composite-types';
 import { FieldMetadataType } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity';
 import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity';
 import { capitalize } from 'src/utils/capitalize';
@@ -74,7 +74,7 @@ const getSchemaComponentsProperties = (
       case FieldMetadataType.ACTOR:
         itemProperty = {
           type: 'object',
-          properties: compositeTypeDefintions
+          properties: compositeTypeDefinitions
             .get(field.type)
             ?.properties?.reduce((properties, property) => {
               properties[property.name] = getFieldProperties(property.type);

diff --git a/packages/twenty-server/src/engine/metadata-modules/field-metadata/composite-types/index.ts b/packages/twenty-server/src/engine/metadata-modules/field-metadata/composite-types/index.ts
@@ -13,7 +13,7 @@ export type CompositeFieldsDefinitionFunction = (
   fieldMetadata?: FieldMetadataInterface,
 ) => FieldMetadataInterface[];
 
-export const compositeTypeDefintions = new Map<
+export const compositeTypeDefinitions = new Map<
   FieldMetadataType,
   CompositeType
 >([