[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).
This commit is contained in:
@ -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,
|
||||
|
||||
@ -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(
|
||||
|
||||
@ -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(
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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],
|
||||
|
||||
@ -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(
|
||||
|
||||
@ -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(
|
||||
|
||||
@ -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 } },
|
||||
]);
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user