fix(core): Fix ListQueryBuilder language handling logic

Fixes #1631, fixes #1611. This commit also should theoretically improve performance of list queries
where there is no sorting on a translatable property, as in this case it omits an entire complex
nested subquery.

packages/core/e2e/list-query-builder.e2e-spec.ts

@@ -964,7 +964,7 @@ describe('ListQueryBuilder', () => {
     });
 
     // https://github.com/vendure-ecommerce/vendure/issues/1611
-    xdescribe('translations handling', () => {
+    describe('translations handling', () => {
         const allTranslations = [
             [
                 { languageCode: LanguageCode.en, name: 'apple' },

packages/core/e2e/product.e2e-spec.ts

@@ -11,6 +11,7 @@ import { testConfig, TEST_SETUP_TIMEOUT_MS } from '../../../e2e-common/test-conf
 import { PRODUCT_VARIANT_FRAGMENT, PRODUCT_WITH_OPTIONS_FRAGMENT } from './graphql/fragments';
 import {
     AddOptionGroupToProduct,
+    ChannelFragment,
     CreateProduct,
     CreateProductVariants,
     DeleteProduct,
@@ -27,10 +28,13 @@ import {
     GetProductWithVariants,
     LanguageCode,
     ProductVariantFragment,
+    ProductVariantListOptions,
     ProductWithOptionsFragment,
     ProductWithVariants,
     RemoveOptionGroupFromProduct,
     SortOrder,
+    UpdateChannel,
+    UpdateGlobalSettings,
     UpdateProduct,
     UpdateProductVariants,
 } from './graphql/generated-e2e-admin-types';
@@ -44,6 +48,8 @@ import {
     GET_PRODUCT_LIST,
     GET_PRODUCT_SIMPLE,
     GET_PRODUCT_WITH_VARIANTS,
+    UPDATE_CHANNEL,
+    UPDATE_GLOBAL_SETTINGS,
     UPDATE_PRODUCT,
     UPDATE_PRODUCT_VARIANTS,
 } from './graphql/shared-definitions';
@@ -52,12 +58,16 @@ import { assertThrowsWithMessage } from './utils/assert-throws-with-message';
 // tslint:disable:no-non-null-assertion
 
 describe('Product resolver', () => {
-    const { server, adminClient, shopClient } = createTestEnvironment(testConfig());
+    const { server, adminClient, shopClient } = createTestEnvironment({
+        ...testConfig(),
+    });
 
     const removeOptionGuard: ErrorResultGuard<ProductWithOptionsFragment> = createErrorResultGuard(
         input => !!input.optionGroups,
     );
 
+    const updateChannelGuard: ErrorResultGuard<ChannelFragment> = createErrorResultGuard(input => !!input.id);
+
     beforeAll(async () => {
         await server.init({
             initialData,
@@ -1724,6 +1734,132 @@ describe('Product resolver', () => {
 
                 expect(product?.variants.length).toBe(1);
             });
+
+            // https://github.com/vendure-ecommerce/vendure/issues/1631
+            describe('changing the Channel default language', () => {
+                let productId: string;
+
+                function getProductWithVariantsInLanguage(
+                    id: string,
+                    languageCode: LanguageCode,
+                    variantListOptions?: ProductVariantListOptions,
+                ) {
+                    return adminClient.query<
+                        GetProductWithVariantList.Query,
+                        GetProductWithVariantList.Variables
+                    >(GET_PRODUCT_WITH_VARIANT_LIST, { id, variantListOptions }, { languageCode });
+                }
+
+                beforeAll(async () => {
+                    await adminClient.query<UpdateGlobalSettings.Mutation, UpdateGlobalSettings.Variables>(
+                        UPDATE_GLOBAL_SETTINGS,
+                        {
+                            input: {
+                                availableLanguages: [LanguageCode.en, LanguageCode.de],
+                            },
+                        },
+                    );
+                    const { createProduct } = await adminClient.query<
+                        CreateProduct.Mutation,
+                        CreateProduct.Variables
+                    >(CREATE_PRODUCT, {
+                        input: {
+                            translations: [
+                                {
+                                    languageCode: LanguageCode.en,
+                                    name: 'Bottle',
+                                    slug: 'bottle',
+                                    description: 'A container for liquids',
+                                },
+                            ],
+                        },
+                    });
+
+                    productId = createProduct.id;
+                    await adminClient.query<CreateProductVariants.Mutation, CreateProductVariants.Variables>(
+                        CREATE_PRODUCT_VARIANTS,
+                        {
+                            input: [
+                                {
+                                    productId,
+                                    sku: 'BOTTLE111',
+                                    optionIds: [],
+                                    translations: [{ languageCode: LanguageCode.en, name: 'Bottle' }],
+                                },
+                            ],
+                        },
+                    );
+                });
+
+                afterAll(async () => {
+                    // Restore the default language to English for the subsequent tests
+                    await adminClient.query<UpdateChannel.Mutation, UpdateChannel.Variables>(UPDATE_CHANNEL, {
+                        input: {
+                            id: 'T_1',
+                            defaultLanguageCode: LanguageCode.en,
+                        },
+                    });
+                });
+
+                it('returns all variants', async () => {
+                    const { product: product1 } = await adminClient.query<
+                        GetProductWithVariants.Query,
+                        GetProductWithVariants.Variables
+                    >(
+                        GET_PRODUCT_WITH_VARIANTS,
+                        {
+                            id: productId,
+                        },
+                        { languageCode: LanguageCode.en },
+                    );
+                    expect(product1?.variants.length).toBe(1);
+
+                    // Change the default language of the channel to "de"
+                    const { updateChannel } = await adminClient.query<
+                        UpdateChannel.Mutation,
+                        UpdateChannel.Variables
+                    >(UPDATE_CHANNEL, {
+                        input: {
+                            id: 'T_1',
+                            defaultLanguageCode: LanguageCode.de,
+                        },
+                    });
+                    updateChannelGuard.assertSuccess(updateChannel);
+                    expect(updateChannel.defaultLanguageCode).toBe(LanguageCode.de);
+
+                    // Fetch the product in en, it should still return 1 variant
+                    const { product: product2 } = await getProductWithVariantsInLanguage(
+                        productId,
+                        LanguageCode.en,
+                    );
+                    expect(product2?.variantList.items.length).toBe(1);
+
+                    // Fetch the product in de, it should still return 1 variant
+                    const { product: product3 } = await getProductWithVariantsInLanguage(
+                        productId,
+                        LanguageCode.de,
+                    );
+                    expect(product3?.variantList.items.length).toBe(1);
+                });
+
+                it('returns all variants when sorting on variant name', async () => {
+                    // Fetch the product in en, it should still return 1 variant
+                    const { product: product1 } = await getProductWithVariantsInLanguage(
+                        productId,
+                        LanguageCode.en,
+                        { sort: { name: SortOrder.ASC } },
+                    );
+                    expect(product1?.variantList.items.length).toBe(1);
+
+                    // Fetch the product in de, it should still return 1 variant
+                    const { product: product2 } = await getProductWithVariantsInLanguage(
+                        productId,
+                        LanguageCode.de,
+                        { sort: { name: SortOrder.ASC } },
+                    );
+                    expect(product2?.variantList.items.length).toBe(1);
+                });
+            });
         });
     });
 

packages/core/src/service/helpers/list-query-builder/connection-utils.ts

@@ -2,7 +2,8 @@ import { Type } from '@vendure/common/lib/shared-types';
 import { Connection } from 'typeorm';
 import { ColumnMetadata } from 'typeorm/metadata/ColumnMetadata';
 
-import { CalculatedColumnDefinition, CALCULATED_PROPERTIES } from '../../../common/calculated-decorator';
+import { Translation } from '../../../common/index';
+import { VendureEntity } from '../../../entity/index';
 
 /**
  * @description
@@ -16,9 +17,17 @@ export function getColumnMetadata<T>(connection: Connection, entity: Type<T>) {
 
     const translationRelation = relations.find(r => r.propertyName === 'translations');
     if (translationRelation) {
+        const commonFields: Array<keyof (Translation<T> & VendureEntity)> = [
+            'id',
+            'createdAt',
+            'updatedAt',
+            'languageCode',
+        ];
         const translationMetadata = connection.getMetadata(translationRelation.type);
         translationColumns = translationColumns.concat(
-            translationMetadata.columns.filter(c => !c.relationMetadata),
+            translationMetadata.columns.filter(
+                c => !c.relationMetadata && !commonFields.includes(c.propertyName as any),
+            ),
         );
     }
     const alias = metadata.name.toLowerCase();

packages/core/src/service/helpers/list-query-builder/list-query-builder.ts

@@ -17,7 +17,7 @@ import { FindOptionsUtils } from 'typeorm/find-options/FindOptionsUtils';
 import { ApiType } from '../../../api/common/get-api-type';
 import { RequestContext } from '../../../api/common/request-context';
 import { UserInputError } from '../../../common/error/errors';
-import { FilterParameter, ListQueryOptions, SortParameter } from '../../../common/types/common-types';
+import { ListQueryOptions, NullOptionals, SortParameter } from '../../../common/types/common-types';
 import { ConfigService } from '../../../config/config.service';
 import { CustomFields } from '../../../config/index';
 import { Logger } from '../../../config/logger/vendure-logger';
@@ -212,8 +212,6 @@ export class ListQueryBuilder implements OnApplicationBootstrap {
         // tslint:disable-next-line:no-non-null-assertion
         FindOptionsUtils.joinEagerRelations(qb, qb.alias, qb.expressionMap.mainAlias!.metadata);
 
-        this.applyTranslationConditions(qb, entity, extendedOptions.ctx, extendedOptions.entityAlias);
-
         // join the tables required by calculated columns
         this.joinCalculatedColumnRelations(qb, entity, options);
 
@@ -222,10 +220,18 @@ export class ListQueryBuilder implements OnApplicationBootstrap {
             this.normalizeCustomPropertyMap(customPropertyMap, options, qb);
         }
         const customFieldsForType = this.configService.customFields[entity.name as keyof CustomFields];
+        const sortParams = Object.assign({}, options.sort, extendedOptions.orderBy);
+        this.applyTranslationConditions(
+            qb,
+            entity,
+            sortParams,
+            extendedOptions.ctx,
+            extendedOptions.entityAlias,
+        );
         const sort = parseSortParams(
             rawConnection,
             entity,
-            Object.assign({}, options.sort, extendedOptions.orderBy),
+            sortParams,
             customPropertyMap,
             entityAlias,
             customFieldsForType,
@@ -513,13 +519,15 @@ export class ListQueryBuilder implements OnApplicationBootstrap {
     }
 
     /**
-     * If this entity is Translatable, then we need to apply appropriate WHERE clauses to limit
-     * the joined translation relations. This method applies a simple "WHERE" on the languageCode
-     * in the case of the default language, otherwise we use a more complex.
+     * @description
+     * If this entity is Translatable, and we are sorting on one of the translatable fields,
+     * then we need to apply appropriate WHERE clauses to limit
+     * the joined translation relations.
      */
     private applyTranslationConditions<T extends VendureEntity>(
         qb: SelectQueryBuilder<any>,
         entity: Type<T>,
+        sortParams: NullOptionals<SortParameter<T>> & FindOneOptions<T>['order'],
         ctx?: RequestContext,
         entityAlias?: string,
     ) {
@@ -532,7 +540,15 @@ export class ListQueryBuilder implements OnApplicationBootstrap {
         } = getColumnMetadata(this.connection.rawConnection, entity);
         const alias = entityAlias ?? defaultAlias;
 
-        if (translationColumns.length) {
+        const sortKeys = Object.keys(sortParams);
+        let sortingOnTranslatableKey = false;
+        for (const translationColumn of translationColumns) {
+            if (sortKeys.includes(translationColumn.propertyName)) {
+                sortingOnTranslatableKey = true;
+            }
+        }
+
+        if (translationColumns.length && sortingOnTranslatableKey) {
             const translationsAlias = qb.connection.namingStrategy.eagerJoinRelationAlias(
                 alias,
                 'translations',
@@ -584,7 +600,7 @@ export class ListQueryBuilder implements OnApplicationBootstrap {
                     }
                     qb.setParameters({
                         nonDefaultLanguageCode: languageCode,
-                        defaultLanguageCode: this.configService.defaultLanguageCode,
+                        defaultLanguageCode,
                     });
                 }),
             );