fix(core): Fix regression with custom field relations & product by slug

Fixes #1723

packages/core/e2e/custom-field-relations.e2e-spec.ts

@@ -29,7 +29,7 @@ import path from 'path';
 import { Entity, JoinColumn, OneToOne } from 'typeorm';
 
 import { initialData } from '../../../e2e-common/e2e-initial-data';
-import { TEST_SETUP_TIMEOUT_MS, testConfig } from '../../../e2e-common/test-config';
+import { testConfig, TEST_SETUP_TIMEOUT_MS } from '../../../e2e-common/test-config';
 
 import { testSuccessfulPaymentMethod } from './fixtures/test-payment-methods';
 import { AddItemToOrder } from './graphql/generated-e2e-shop-types';
@@ -800,7 +800,7 @@ describe('Custom field relations', () => {
             });
 
             // https://github.com/vendure-ecommerce/vendure/issues/1664
-            it('successfully gets product with eager-loading custom field relation', async () => {
+            it('successfully gets product by id with eager-loading custom field relation', async () => {
                 const { product } = await shopClient.query(gql`
                     query {
                         product(id: "T_1") {
@@ -818,6 +818,25 @@ describe('Custom field relations', () => {
 
                 expect(product).toBeDefined();
             });
+
+            it('successfully gets product by slug with eager-loading custom field relation', async () => {
+                const { product } = await shopClient.query(gql`
+                    query {
+                        product(slug: "laptop") {
+                            id
+                            customFields {
+                                cfVendor {
+                                    featuredProduct {
+                                        id
+                                    }
+                                }
+                            }
+                        }
+                    }
+                `);
+
+                expect(product).toBeDefined();
+            });
         });
 
         describe('ProductOptionGroup, ProductOption entity', () => {

packages/core/src/connection/remove-custom-fields-with-eager-relations.ts

@@ -0,0 +1,63 @@
+import { SelectQueryBuilder } from 'typeorm';
+
+import { Logger } from '../config/index';
+
+/**
+ * This is a work-around for this issue: https://github.com/vendure-ecommerce/vendure/issues/1664
+ *
+ * Explanation:
+ * When calling `FindOptionsUtils.joinEagerRelations()`, there appears to be a bug in TypeORM whereby
+ * it will throw the following error *if* the `options.relations` array contains any customField relations
+ * where the related entity itself has eagerly-loaded relations.
+ *
+ * For example, let's say we define a custom field on the Product entity like this:
+ * ```
+ * Product: [{
+ *   name: 'featuredFacet',
+ *   type: 'relation',
+ *   entity: Facet,
+ * }],
+ * ```
+ * and then we pass into `TransactionalConnection.findOneInChannel()` an options array of:
+ *
+ * ```
+ * { relations: ['customFields.featuredFacet'] }
+ * ```
+ * it will throw an error because the `Facet` entity itself has eager relations (namely the `translations` property).
+ * This will cause TypeORM to throw the error:
+ * ```
+ * TypeORMError: "entity__customFields" alias was not found. Maybe you forgot to join it?
+ * ```
+ *
+ * So this method introspects the QueryBuilder metadata and checks for any custom field relations which
+ * themselves have eager relations. If found, it removes those items from the `options.relations` array.
+ *
+ * TODO: Ideally create a minimal reproduction case and report in the TypeORM repo for an upstream fix.
+ */
+
+export function removeCustomFieldsWithEagerRelations(
+    qb: SelectQueryBuilder<any>,
+    relations: string[] = [],
+): string[] {
+    let resultingRelations = relations;
+    const mainAlias = qb.expressionMap.mainAlias;
+    const customFieldsMetadata = mainAlias?.metadata.embeddeds.find(
+        metadata => metadata.propertyName === 'customFields',
+    );
+    if (customFieldsMetadata) {
+        const customFieldRelationsWithEagerRelations = customFieldsMetadata.relations.filter(
+            relation => !!relation.inverseEntityMetadata.ownRelations.find(or => or.isEager === true),
+        );
+        for (const relation of customFieldRelationsWithEagerRelations) {
+            const propertyName = relation.propertyName;
+            const relationsToRemove = relations.filter(r => r.startsWith(`customFields.${propertyName}`));
+            if (relationsToRemove.length) {
+                Logger.debug(
+                    `TransactionalConnection.findOneInChannel cannot automatically join relation [${mainAlias?.metadata.name}.customFields.${propertyName}]`,
+                );
+                resultingRelations = relations.filter(r => !r.startsWith(`customFields.${propertyName}`));
+            }
+        }
+    }
+    return resultingRelations;
+}

packages/core/src/connection/transactional-connection.ts

@@ -19,6 +19,7 @@ import { ChannelAware, SoftDeletable } from '../common/types/common-types';
 import { Logger } from '../config/index';
 import { VendureEntity } from '../entity/base/base.entity';
 
+import { removeCustomFieldsWithEagerRelations } from './remove-custom-fields-with-eager-relations';
 import { TransactionWrapper } from './transaction-wrapper';
 import { GetEntityOrThrowOptions } from './types';
 
@@ -56,7 +57,7 @@ export class TransactionalConnection {
      * Returns a TypeORM repository. Note that when no RequestContext is supplied, the repository will not
      * be aware of any existing transaction. Therefore calling this method without supplying a RequestContext
      * is discouraged without a deliberate reason.
-     * 
+     *
      * @deprecated since 1.7.0: Use {@link TransactionalConnection.rawConnection rawConnection.getRepository()} function instead.
      */
     getRepository<Entity>(target: ObjectType<Entity> | EntitySchema<Entity> | string): Repository<Entity>;
@@ -264,7 +265,7 @@ export class TransactionalConnection {
         options: FindOneOptions = {},
     ) {
         const qb = this.getRepository(ctx, entity).createQueryBuilder('entity');
-        options.relations = this.removeCustomFieldsWithEagerRelations(qb, options.relations);
+        options.relations = removeCustomFieldsWithEagerRelations(qb, options.relations);
         FindOptionsUtils.applyFindManyOptionsOrConditionsToQueryBuilder(qb, options);
         if (options.loadEagerRelations !== false) {
             // tslint:disable-next-line:no-non-null-assertion
@@ -277,65 +278,6 @@ export class TransactionalConnection {
             .getOne();
     }
 
-    /**
-     * This is a work-around for this issue: https://github.com/vendure-ecommerce/vendure/issues/1664
-     *
-     * Explanation:
-     * When calling `FindOptionsUtils.joinEagerRelations()`, there appears to be a bug in TypeORM whereby
-     * it will throw the following error *if* the `options.relations` array contains any customField relations
-     * where the related entity itself has eagerly-loaded relations.
-     *
-     * For example, let's say we define a custom field on the Product entity like this:
-     * ```
-     * Product: [{
-     *   name: 'featuredFacet',
-     *   type: 'relation',
-     *   entity: Facet,
-     * }],
-     * ```
-     * and then we pass into `TransactionalConnection.findOneInChannel()` an options array of:
-     *
-     * ```
-     * { relations: ['customFields.featuredFacet'] }
-     * ```
-     * it will throw an error because the `Facet` entity itself has eager relations (namely the `translations` property).
-     * This will cause TypeORM to throw the error:
-     * ```
-     * TypeORMError: "entity__customFields" alias was not found. Maybe you forgot to join it?
-     * ```
-     *
-     * So this method introspects the QueryBuilder metadata and checks for any custom field relations which
-     * themselves have eager relations. If found, it removes those items from the `options.relations` array.
-     *
-     * TODO: Ideally create a minimal reproduction case and report in the TypeORM repo for an upstream fix.
-     */
-    private removeCustomFieldsWithEagerRelations(
-        qb: SelectQueryBuilder<any>,
-        relations: string[] = [],
-    ): string[] {
-        let resultingRelations = relations;
-        const mainAlias = qb.expressionMap.mainAlias;
-        const customFieldsMetadata = mainAlias?.metadata.embeddeds.find(
-            metadata => metadata.propertyName === 'customFields',
-        );
-        if (customFieldsMetadata) {
-            const customFieldRelationsWithEagerRelations = customFieldsMetadata.relations.filter(
-                relation => !!relation.inverseEntityMetadata.ownRelations.find(or => or.isEager === true),
-            );
-            for (const relation of customFieldRelationsWithEagerRelations) {
-                const propertyName = relation.propertyName;
-                const relationsToRemove = relations.filter(r => r.startsWith(`customFields.${propertyName}`));
-                if (relationsToRemove.length) {
-                    Logger.debug(
-                        `TransactionalConnection.findOneInChannel cannot automatically join relation [${mainAlias?.metadata.name}.customFields.${propertyName}]`,
-                    );
-                    resultingRelations = relations.filter(r => !r.startsWith(`customFields.${propertyName}`));
-                }
-            }
-        }
-        return resultingRelations;
-    }
-
     /**
      * @description
      * Like the TypeOrm `Repository.findByIds()` method, but limits the results to

packages/core/src/service/services/product.service.ts

@@ -20,6 +20,7 @@ import { ProductOptionInUseError } from '../../common/error/generated-graphql-ad
 import { ListQueryOptions } from '../../common/types/common-types';
 import { Translated } from '../../common/types/locale-types';
 import { assertFound, idsAreEqual } from '../../common/utils';
+import { removeCustomFieldsWithEagerRelations } from '../../connection/remove-custom-fields-with-eager-relations';
 import { TransactionalConnection } from '../../connection/transactional-connection';
 import { Channel } from '../../entity/channel/channel.entity';
 import { FacetValue } from '../../entity/facet-value/facet-value.entity';
@@ -173,8 +174,12 @@ export class ProductService {
         relations?: RelationPaths<Product>,
     ): Promise<Translated<Product> | undefined> {
         const qb = this.connection.getRepository(ctx, Product).createQueryBuilder('product');
+        const effectiveRelations = removeCustomFieldsWithEagerRelations(
+            qb,
+            relations ? [...new Set(this.relations.concat(relations))] : this.relations,
+        );
         FindOptionsUtils.applyFindManyOptionsOrConditionsToQueryBuilder(qb, {
-            relations: relations ? [...new Set(this.relations.concat(relations))] : this.relations,
+            relations: effectiveRelations,
         });
         // tslint:disable-next-line:no-non-null-assertion
         FindOptionsUtils.joinEagerRelations(qb, qb.alias, qb.expressionMap.mainAlias!.metadata);