fix(core): Fix sorting by translatable fields in list queries

Fixes #689

packages/core/e2e/fixtures/test-plugins/list-query-plugin.ts

@@ -1,10 +1,17 @@
 import { Args, Query, Resolver } from '@nestjs/graphql';
-import { ID } from '@vendure/common/lib/shared-types';
+import { LanguageCode } from '@vendure/common/lib/generated-types';
+import { DeepPartial, ID } from '@vendure/common/lib/shared-types';
 import {
+    Ctx,
     ListQueryBuilder,
+    LocaleString,
     OnVendureBootstrap,
     PluginCommonModule,
+    RequestContext,
     TransactionalConnection,
+    Translatable,
+    translateDeep,
+    Translation,
     VendureEntity,
     VendurePlugin,
 } from '@vendure/core';
@@ -15,13 +22,15 @@ import { Calculated } from '../../../src/common/calculated-decorator';
 import { EntityId } from '../../../src/entity/entity-id.decorator';
 
 @Entity()
-export class TestEntity extends VendureEntity {
+export class TestEntity extends VendureEntity implements Translatable {
     constructor(input: Partial<TestEntity>) {
         super(input);
     }
     @Column()
     label: string;
 
+    name: LocaleString;
+
     @Column()
     description: string;
 
@@ -52,6 +61,23 @@ export class TestEntity extends VendureEntity {
 
     @OneToMany(type => TestEntityPrice, price => price.parent)
     prices: TestEntityPrice[];
+
+    @OneToMany(type => TestEntityTranslation, translation => translation.base, { eager: true })
+    translations: Array<Translation<TestEntity>>;
+}
+
+@Entity()
+export class TestEntityTranslation extends VendureEntity implements Translation<TestEntity> {
+    constructor(input?: DeepPartial<Translation<TestEntity>>) {
+        super(input);
+    }
+
+    @Column('varchar') languageCode: LanguageCode;
+
+    @Column() name: string;
+
+    @ManyToOne(type => TestEntity, base => base.translations)
+    base: TestEntity;
 }
 
 @Entity()
@@ -74,9 +100,9 @@ export class ListQueryResolver {
     constructor(private listQueryBuilder: ListQueryBuilder) {}
 
     @Query()
-    testEntities(@Args() args: any) {
+    testEntities(@Ctx() ctx: RequestContext, @Args() args: any) {
         return this.listQueryBuilder
-            .build(TestEntity, args.options)
+            .build(TestEntity, args.options, { ctx })
             .getManyAndCount()
             .then(([items, totalItems]) => {
                 for (const item of items) {
@@ -85,7 +111,7 @@ export class ListQueryResolver {
                     }
                 }
                 return {
-                    items,
+                    items: items.map(i => translateDeep(i, ctx.languageCode)),
                     totalItems,
                 };
             });
@@ -98,6 +124,7 @@ const adminApiExtensions = gql`
         createdAt: DateTime!
         updatedAt: DateTime!
         label: String!
+        name: String!
         description: String!
         active: Boolean!
         order: Int!
@@ -120,7 +147,7 @@ const adminApiExtensions = gql`
 
 @VendurePlugin({
     imports: [PluginCommonModule],
-    entities: [TestEntity, TestEntityPrice],
+    entities: [TestEntity, TestEntityPrice, TestEntityTranslation],
     adminApiExtensions: {
         schema: adminApiExtensions,
         resolvers: [ListQueryResolver],
@@ -169,6 +196,15 @@ export class ListQueryPlugin implements OnVendureBootstrap {
                     order: 4,
                 }),
             ]);
+
+            const translations: any = {
+                A: { [LanguageCode.en]: 'apple', [LanguageCode.de]: 'apfel' },
+                B: { [LanguageCode.en]: 'bike', [LanguageCode.de]: 'fahrrad' },
+                C: { [LanguageCode.en]: 'cake', [LanguageCode.de]: 'kuchen' },
+                D: { [LanguageCode.en]: 'dog', [LanguageCode.de]: 'hund' },
+                E: { [LanguageCode.en]: 'egg' },
+            };
+
             for (const testEntity of testEntities) {
                 await this.connection.getRepository(TestEntityPrice).save([
                     new TestEntityPrice({
@@ -182,6 +218,19 @@ export class ListQueryPlugin implements OnVendureBootstrap {
                         parent: testEntity,
                     }),
                 ]);
+
+                for (const code of [LanguageCode.en, LanguageCode.de]) {
+                    const translation = translations[testEntity.label][code];
+                    if (translation) {
+                        await this.connection.getRepository(TestEntityTranslation).save(
+                            new TestEntityTranslation({
+                                name: translation,
+                                base: testEntity,
+                                languageCode: code,
+                            }),
+                        );
+                    }
+                }
             }
         }
     }

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

@@ -7,7 +7,7 @@ import { initialData } from '../../../e2e-common/e2e-initial-data';
 import { testConfig, TEST_SETUP_TIMEOUT_MS } from '../../../e2e-common/test-config';
 
 import { ListQueryPlugin } from './fixtures/test-plugins/list-query-plugin';
-import { SortOrder } from './graphql/generated-e2e-admin-types';
+import { LanguageCode, SortOrder } from './graphql/generated-e2e-admin-types';
 import { fixPostgresTimezone } from './utils/fix-pg-timezone';
 
 fixPostgresTimezone();
@@ -36,6 +36,103 @@ describe('ListQueryBuilder', () => {
         return items.map((x: any) => x.label).sort();
     }
 
+    describe('pagination', () => {
+        it('all en', async () => {
+            const { testEntities } = await adminClient.query(
+                GET_LIST,
+                {
+                    options: {},
+                },
+                { languageCode: LanguageCode.en },
+            );
+
+            expect(testEntities.totalItems).toBe(5);
+            expect(getItemLabels(testEntities.items)).toEqual(['A', 'B', 'C', 'D', 'E']);
+            expect(testEntities.items.map((i: any) => i.name)).toEqual([
+                'apple',
+                'bike',
+                'cake',
+                'dog',
+                'egg',
+            ]);
+        });
+
+        it('all de', async () => {
+            const { testEntities } = await adminClient.query(
+                GET_LIST,
+                {
+                    options: {},
+                },
+                { languageCode: LanguageCode.de },
+            );
+
+            expect(testEntities.totalItems).toBe(5);
+            expect(getItemLabels(testEntities.items)).toEqual(['A', 'B', 'C', 'D', 'E']);
+            expect(testEntities.items.map((i: any) => i.name)).toEqual([
+                'apfel',
+                'fahrrad',
+                'kuchen',
+                'hund',
+                'egg', // falls back to en translation when de doesn't exist
+            ]);
+        });
+
+        it('take', async () => {
+            const { testEntities } = await adminClient.query(GET_LIST, {
+                options: {
+                    take: 2,
+                },
+            });
+
+            expect(testEntities.totalItems).toBe(5);
+            expect(getItemLabels(testEntities.items)).toEqual(['A', 'B']);
+        });
+
+        it('skip', async () => {
+            const { testEntities } = await adminClient.query(GET_LIST, {
+                options: {
+                    skip: 2,
+                },
+            });
+
+            expect(testEntities.totalItems).toBe(5);
+            expect(getItemLabels(testEntities.items)).toEqual(['C', 'D', 'E']);
+        });
+
+        it('skip negative is ignored', async () => {
+            const { testEntities } = await adminClient.query(GET_LIST, {
+                options: {
+                    skip: -1,
+                },
+            });
+
+            expect(testEntities.totalItems).toBe(5);
+            expect(testEntities.items.length).toBe(5);
+        });
+
+        it('take zero is ignored', async () => {
+            const { testEntities } = await adminClient.query(GET_LIST, {
+                options: {
+                    take: 0,
+                },
+            });
+
+            expect(testEntities.totalItems).toBe(5);
+            expect(testEntities.items.length).toBe(5);
+        });
+
+        it('take negative is ignored', async () => {
+            const { testEntities } = await adminClient.query(GET_LIST, {
+                options: {
+                    take: -1,
+                },
+            });
+
+            expect(testEntities.totalItems).toBe(5);
+            expect(testEntities.items.length).toBe(5);
+        });
+    });
+
     describe('string filtering', () => {
         it('eq', async () => {
             const { testEntities } = await adminClient.query(GET_LIST, {
@@ -419,6 +516,72 @@ describe('ListQueryBuilder', () => {
             });
             expect(testEntities.items.map((x: any) => x.label)).toEqual(['E', 'D', 'C', 'B', 'A']);
         });
+
+        it('sort by translated field en', async () => {
+            const { testEntities } = await adminClient.query(GET_LIST, {
+                options: {
+                    sort: {
+                        name: SortOrder.ASC,
+                    },
+                },
+            });
+            expect(testEntities.items.map((x: any) => x.name)).toEqual([
+                'apple',
+                'bike',
+                'cake',
+                'dog',
+                'egg',
+            ]);
+        });
+
+        it('sort by translated field de', async () => {
+            const { testEntities } = await adminClient.query(
+                GET_LIST,
+                {
+                    options: {
+                        sort: {
+                            name: SortOrder.ASC,
+                        },
+                    },
+                },
+                { languageCode: LanguageCode.de },
+            );
+            expect(testEntities.items.map((x: any) => x.name)).toEqual([
+                'apfel',
+                'egg',
+                'fahrrad',
+                'hund',
+                'kuchen',
+            ]);
+        });
+
+        it('sort by translated field en with take', async () => {
+            const { testEntities } = await adminClient.query(GET_LIST, {
+                options: {
+                    sort: {
+                        name: SortOrder.ASC,
+                    },
+                    take: 3,
+                },
+            });
+            expect(testEntities.items.map((x: any) => x.name)).toEqual(['apple', 'bike', 'cake']);
+        });
+
+        it('sort by translated field de with take', async () => {
+            const { testEntities } = await adminClient.query(
+                GET_LIST,
+                {
+                    options: {
+                        sort: {
+                            name: SortOrder.ASC,
+                        },
+                        take: 3,
+                    },
+                },
+                { languageCode: LanguageCode.de },
+            );
+            expect(testEntities.items.map((x: any) => x.name)).toEqual(['apfel', 'egg', 'fahrrad']);
+        });
     });
 
     describe('calculated fields', () => {
@@ -479,6 +642,7 @@ const GET_LIST = gql`
             items {
                 id
                 label
+                name
                 date
             }
         }

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

@@ -17,7 +17,9 @@ export function getColumnMetadata<T>(connection: Connection, entity: Type<T>) {
     const translationRelation = relations.find(r => r.propertyName === 'translations');
     if (translationRelation) {
         const translationMetadata = connection.getMetadata(translationRelation.type);
-        translationColumns = columns.concat(translationMetadata.columns.filter(c => !c.relationMetadata));
+        translationColumns = translationColumns.concat(
+            translationMetadata.columns.filter(c => !c.relationMetadata),
+        );
     }
     const alias = metadata.name.toLowerCase();
     return { columns, translationColumns, alias };

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

@@ -1,13 +1,14 @@
 import { Injectable, OnApplicationBootstrap } from '@nestjs/common';
 import { ID, Type } from '@vendure/common/lib/shared-types';
 import { unique } from '@vendure/common/lib/unique';
-import { FindConditions, FindManyOptions, FindOneOptions, SelectQueryBuilder } from 'typeorm';
+import { Brackets, FindConditions, FindManyOptions, FindOneOptions, SelectQueryBuilder } from 'typeorm';
 import { BetterSqlite3Driver } from 'typeorm/driver/better-sqlite3/BetterSqlite3Driver';
 import { SqljsDriver } from 'typeorm/driver/sqljs/SqljsDriver';
 import { FindOptionsUtils } from 'typeorm/find-options/FindOptionsUtils';
 
 import { RequestContext } from '../../../api/common/request-context';
 import { ListQueryOptions } from '../../../common/types/common-types';
+import { ConfigService } from '../../../config/config.service';
 import { VendureEntity } from '../../../entity/base/base.entity';
 import { TransactionalConnection } from '../../transaction/transactional-connection';
 
@@ -31,7 +32,7 @@ export type ExtendedListQueryOptions<T extends VendureEntity> = {
 
 @Injectable()
 export class ListQueryBuilder implements OnApplicationBootstrap {
-    constructor(private connection: TransactionalConnection) {}
+    constructor(private connection: TransactionalConnection, private configService: ConfigService) {}
 
     onApplicationBootstrap(): any {
         this.registerSQLiteRegexpFunction();
@@ -71,6 +72,8 @@ 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);
+
         // join the tables required by calculated columns
         this.joinCalculatedColumnRelations(qb, entity, options);
 
@@ -123,6 +126,60 @@ 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.
+     */
+    private applyTranslationConditions<T extends VendureEntity>(
+        qb: SelectQueryBuilder<any>,
+        entity: Type<T>,
+        ctx?: RequestContext,
+    ) {
+        const languageCode = ctx?.languageCode || this.configService.defaultLanguageCode;
+
+        const { columns, translationColumns, alias } = getColumnMetadata(
+            this.connection.rawConnection,
+            entity,
+        );
+
+        if (translationColumns.length) {
+            const translationsAlias = qb.connection.namingStrategy.eagerJoinRelationAlias(
+                alias,
+                'translations',
+            );
+
+            qb.andWhere(`${translationsAlias}.languageCode = :languageCode`, { languageCode });
+
+            if (languageCode !== this.configService.defaultLanguageCode) {
+                // If the current languageCode is not the default, then we create a more
+                // complex WHERE clause to allow us to use the non-default translations and
+                // fall back to the default language if no translation exists.
+                qb.orWhere(
+                    new Brackets(qb1 => {
+                        const translationEntity = translationColumns[0].entityMetadata.target;
+                        const subQb1 = this.connection.rawConnection
+                            .createQueryBuilder(translationEntity, 'translation')
+                            .where(`translation.base = ${alias}.id`)
+                            .andWhere('translation.languageCode = :defaultLanguageCode');
+                        const subQb2 = this.connection.rawConnection
+                            .createQueryBuilder(translationEntity, 'translation')
+                            .where(`translation.base = ${alias}.id`)
+                            .andWhere('translation.languageCode = :nonDefaultLanguageCode');
+
+                        qb1.where(`EXISTS (${subQb1.getQuery()})`).andWhere(
+                            `NOT EXISTS (${subQb2.getQuery()})`,
+                        );
+                    }),
+                );
+                qb.setParameters({
+                    nonDefaultLanguageCode: languageCode,
+                    defaultLanguageCode: this.configService.defaultLanguageCode,
+                });
+            }
+        }
+    }
+
     /**
      * Registers a user-defined function (for flavors of SQLite driver that support it)
      * so that we can run regex filters on string fields.

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

@@ -48,7 +48,11 @@ export function parseFilterParams<T extends VendureEntity>(
                 if (columns.find(c => c.propertyName === key)) {
                     fieldName = `${alias}.${key}`;
                 } else if (translationColumns.find(c => c.propertyName === key)) {
-                    fieldName = `${alias}_translations.${key}`;
+                    const translationsAlias = connection.namingStrategy.eagerJoinRelationAlias(
+                        alias,
+                        'translations',
+                    );
+                    fieldName = `${translationsAlias}.${key}`;
                 } else if (calculatedColumnExpression) {
                     fieldName = escapeCalculatedColumnExpression(connection, calculatedColumnExpression);
                 } else {

packages/core/src/service/helpers/list-query-builder/parse-sort-params.spec.ts

@@ -1,4 +1,5 @@
 import { Type } from '@vendure/common/lib/shared-types';
+import { DefaultNamingStrategy } from 'typeorm';
 import { ColumnMetadata } from 'typeorm/metadata/ColumnMetadata';
 import { RelationMetadata } from 'typeorm/metadata/RelationMetadata';
 
@@ -146,6 +147,7 @@ export class MockConnection {
             relations: this.relationsMap.get(entity) || [],
         };
     };
+    namingStrategy = new DefaultNamingStrategy();
     readonly options = {
         type: 'sqljs',
     };

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

@@ -33,7 +33,8 @@ export function parseSortParams<T extends VendureEntity>(
         if (columns.find(c => c.propertyName === key)) {
             output[`${alias}.${key}`] = order as any;
         } else if (translationColumns.find(c => c.propertyName === key)) {
-            output[`${alias}_translations.${key}`] = order as any;
+            const translationsAlias = connection.namingStrategy.eagerJoinRelationAlias(alias, 'translations');
+            output[`${translationsAlias}.${key}`] = order as any;
         } else if (calculatedColumnDef) {
             const instruction = calculatedColumnDef.listQuery;
             if (instruction) {