feat(core): Check for name conflict in custom fields, test sort/filter

Relates to #85

packages/core/e2e/custom-fields.e2e-spec.ts

@@ -378,4 +378,41 @@ describe('Custom fields', () => {
             expect(product.customFields.public).toBe('ho!');
         });
     });
+
+    describe('sort & filter', () => {
+
+        it('can sort by custom fields', async () => {
+            const { products } = await adminClient.query(gql`
+                query {
+                    products(options: {
+                        sort: {
+                            nullable: ASC
+                        }
+                    }) {
+                        totalItems
+                    }
+                }
+            `);
+
+            expect(products.totalItems).toBe(1);
+        });
+
+        it('can filter by custom fields', async () => {
+            const { products } = await adminClient.query(gql`
+                query {
+                    products(options: {
+                        filter: {
+                            stringWithDefault: {
+                                contains: "hello"
+                            }
+                        }
+                    }) {
+                        totalItems
+                    }
+                }
+            `);
+
+            expect(products.totalItems).toBe(1);
+        });
+    });
 });

packages/core/e2e/graphql/shared-definitions.ts

@@ -56,7 +56,6 @@ export const GET_PRODUCT_LIST = gql`
         products(languageCode: $languageCode, options: $options) {
             items {
                 id
-                enabled
                 languageCode
                 name
                 slug

packages/core/src/api/config/graphql-custom-fields.ts

@@ -221,9 +221,8 @@ type GraphQLFieldType = 'DateTime' | 'String' | 'Int' | 'Float' | 'Boolean' | 'I
 function mapToFields(
     fieldDefs: CustomFieldConfig[],
     typeFn: (fieldType: CustomFieldType) => string,
-    prefix: string = '',
 ): string {
-    return fieldDefs.map(field => `${prefix}${field.name}: ${typeFn(field.type)}`).join('\n');
+    return fieldDefs.map(field => `${field.name}: ${typeFn(field.type)}`).join('\n');
 }
 
 function getFilterOperator(type: CustomFieldType): string {

packages/core/src/bootstrap.ts

@@ -139,6 +139,7 @@ export async function preBootstrapConfig(
     let config = getConfig();
     const customFieldValidationResult = validateCustomFieldsConfig(config.customFields, entities);
     if (!customFieldValidationResult.valid) {
+        process.exitCode = 1;
         throw new Error(`CustomFields config error:\n- ` + customFieldValidationResult.errors.join('\n- '));
     }
     config = await runPluginConfigurations(config);

packages/core/src/entity/validate-custom-fields-config.spec.ts

@@ -1,11 +1,8 @@
 import { Type } from '@vendure/common/lib/shared-types';
 
-import { User } from '../../dist/entity/user/user.entity';
 import { CustomFields } from '../config/custom-field/custom-field-types';
 
 import { coreEntitiesMap } from './entities';
-import { ProductTranslation } from './product/product-translation.entity';
-import { Product } from './product/product.entity';
 import { validateCustomFieldsConfig } from './validate-custom-fields-config';
 
 describe('validateCustomFieldsConfig()', () => {
@@ -113,6 +110,34 @@ describe('validateCustomFieldsConfig()', () => {
         ]);
     });
 
+    it('name conflict with existing fields', () => {
+        const config: CustomFields = {
+            Product: [
+                { name: 'id', type: 'string' },
+            ],
+        };
+        const result = validateCustomFieldsConfig(config, allEntities);
+
+        expect(result.valid).toBe(false);
+        expect(result.errors).toEqual([
+            'Product entity already has a field named "id"',
+        ]);
+    });
+
+    it('name conflict with existing fields in translation', () => {
+        const config: CustomFields = {
+            Product: [
+                { name: 'name', type: 'string' },
+            ],
+        };
+        const result = validateCustomFieldsConfig(config, allEntities);
+
+        expect(result.valid).toBe(false);
+        expect(result.errors).toEqual([
+            'Product entity already has a field named "name"',
+        ]);
+    });
+
     it('non-nullable must have defaultValue', () => {
         const config: CustomFields = {
             Product: [

packages/core/src/entity/validate-custom-fields-config.ts

@@ -1,22 +1,19 @@
-import { Connection, getConnection, getMetadataArgsStorage } from 'typeorm';
+import { Type } from '@vendure/common/src/shared-types';
+import { getMetadataArgsStorage } from 'typeorm';
 
-import { Type } from '../../../common/lib/shared-types';
 import { CustomFieldConfig, CustomFields } from '../config/custom-field/custom-field-types';
-
-import { VendureEntity } from './base/base.entity';
+import { VendureEntity } from '../entity/base/base.entity';
 
 function validateCustomFieldsForEntity(
     entity: Type<VendureEntity>,
     customFields: CustomFieldConfig[],
 ): string[] {
-    const metadata = getMetadataArgsStorage();
-    const isTranslatable =
-        -1 < metadata.relations.findIndex(r => r.target === entity && r.propertyName === 'translations');
     return [
         ...assertValidFieldNames(entity.name, customFields),
-        ...assertNoNameConflicts(entity.name, customFields),
+        ...assertNoNameConflictsWithEntity(entity, customFields),
+        ...assertNoDuplicatedCustomFieldNames(entity.name, customFields),
         ...assetNonNullablesHaveDefaults(entity.name, customFields),
-        ...(isTranslatable ? [] : assertNoLocaleStringFields(entity.name, customFields)),
+        ...(isTranslatable(entity) ? [] : assertNoLocaleStringFields(entity.name, customFields)),
     ];
 }
 
@@ -34,10 +31,24 @@ function assertValidFieldNames(entityName: string, customFields: CustomFieldConf
     return errors;
 }
 
+function assertNoNameConflictsWithEntity(entity: Type<any>, customFields: CustomFieldConfig[]): string[] {
+    const errors: string[] = [];
+    for (const field of customFields) {
+        const conflicts = (e: Type<any>): boolean => {
+            return -1 < getAllColumnNames(e).findIndex(name => name === field.name);
+        };
+        const translation = getEntityTranslation(entity);
+        if (conflicts(entity) || (translation && conflicts(translation))) {
+            errors.push(`${entity.name} entity already has a field named "${field.name}"`);
+        }
+    }
+    return errors;
+}
+
 /**
  * Assert that none of the custom field names conflict with one another.
  */
-function assertNoNameConflicts(entityName: string, customFields: CustomFieldConfig[]): string[] {
+function assertNoDuplicatedCustomFieldNames(entityName: string, customFields: CustomFieldConfig[]): string[] {
     const nameCounts = customFields
         .map(f => f.name)
         .reduce(
@@ -80,6 +91,35 @@ function assetNonNullablesHaveDefaults(entityName: string, customFields: CustomF
     return errors;
 }
 
+function isTranslatable(entity: Type<any>): boolean {
+    return !!getEntityTranslation(entity);
+}
+
+function getEntityTranslation(entity: Type<any>): Type<any> | undefined {
+    const metadata = getMetadataArgsStorage();
+    const translation = metadata.filterRelations(entity).find(r => r.propertyName === 'translations');
+    if (translation) {
+        const type = translation.type;
+        if (typeof type === 'function') {
+            return type();
+        }
+    }
+}
+
+function getAllColumnNames(entity: Type<any>): string[] {
+    const metadata = getMetadataArgsStorage();
+    const ownColumns = metadata.filterColumns(entity);
+    const relationColumns = metadata.filterRelations(entity);
+    const embeddedColumns = metadata.filterEmbeddeds(entity);
+    const baseColumns = metadata.filterColumns(VendureEntity);
+    return [
+        ...ownColumns,
+        ...relationColumns,
+        ...embeddedColumns,
+        ...baseColumns,
+    ].map(c => c.propertyName);
+}
+
 /**
  * Validates the custom fields config, e.g. by ensuring that there are no naming conflicts with the built-in fields
  * of each entity.
@@ -89,11 +129,12 @@ export function validateCustomFieldsConfig(
     entities: Array<Type<any>>,
 ): { valid: boolean; errors: string[] } {
     let errors: string[] = [];
+    getMetadataArgsStorage();
     for (const key of Object.keys(customFieldConfig)) {
         const entityName = key as keyof CustomFields;
         const customEntityFields = customFieldConfig[entityName] || [];
         const entity = entities.find(e => e.name === entityName);
-        if (entity) {
+        if (entity && customEntityFields.length) {
             errors = errors.concat(validateCustomFieldsForEntity(entity, customEntityFields));
         }
     }