From 47be2ad7230966f9ce0f7afe5c845bf2abde5071 Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Sun, 7 Jan 2024 13:58:30 +0100 Subject: [PATCH] fix(medusa): Order by using joins and pagination (#5990) --- .changeset/sour-birds-build.md | 5 ++ .../api/__tests__/admin/sales-channels.js | 2 +- packages/medusa/src/repositories/product.ts | 2 +- packages/medusa/src/utils/repository.ts | 82 +++++++++++++++++-- 4 files changed, 80 insertions(+), 11 deletions(-) create mode 100644 .changeset/sour-birds-build.md diff --git a/.changeset/sour-birds-build.md b/.changeset/sour-birds-build.md new file mode 100644 index 0000000000..a5fdda1107 --- /dev/null +++ b/.changeset/sour-birds-build.md @@ -0,0 +1,5 @@ +--- +"@medusajs/medusa": patch +--- + +fix(medusa): Ordering management using joins and pagination diff --git a/integration-tests/api/__tests__/admin/sales-channels.js b/integration-tests/api/__tests__/admin/sales-channels.js index 9340c65f9d..15cd0d357e 100644 --- a/integration-tests/api/__tests__/admin/sales-channels.js +++ b/integration-tests/api/__tests__/admin/sales-channels.js @@ -23,7 +23,7 @@ const adminReqConfig = { }, } -jest.setTimeout(50000) +jest.setTimeout(60000) describe("sales channels", () => { let medusaProcess diff --git a/packages/medusa/src/repositories/product.ts b/packages/medusa/src/repositories/product.ts index 37252b7e11..11751b86f7 100644 --- a/packages/medusa/src/repositories/product.ts +++ b/packages/medusa/src/repositories/product.ts @@ -74,7 +74,7 @@ export const ProductRepository = dataSource.getRepository(Product).extend({ optionsWithoutRelations?.where?.discount_condition_id delete optionsWithoutRelations?.where?.discount_condition_id - return queryEntityWithoutRelations({ + return await queryEntityWithoutRelations({ repository: this, optionsWithoutRelations, shouldCount, diff --git a/packages/medusa/src/utils/repository.ts b/packages/medusa/src/utils/repository.ts index f7a4c8b70a..f07fb8e53f 100644 --- a/packages/medusa/src/utils/repository.ts +++ b/packages/medusa/src/utils/repository.ts @@ -150,11 +150,7 @@ export async function queryEntityWithoutRelations({ }): Promise<[T[], number]> { const alias = repository.metadata.name.toLowerCase() - const qb = repository - .createQueryBuilder(alias) - .select([`${alias}.id`]) - .skip(optionsWithoutRelations.skip) - .take(optionsWithoutRelations.take) + const qb = repository.createQueryBuilder(alias).select([`${alias}.id`]) if (optionsWithoutRelations.where) { qb.where(optionsWithoutRelations.where) @@ -189,14 +185,82 @@ export async function queryEntityWithoutRelations({ qb.withDeleted() } + /* + * Deduplicate tuples for join + ordering (e.g. variants.prices.amount) since typeorm doesnt + * know how to manage it by itself + */ + const expressionMapAllOrderBys = qb.expressionMap.allOrderBys + if ( + expressionMapAllOrderBys && + Object.keys(expressionMapAllOrderBys).length + ) { + const orderBysString = Object.keys(expressionMapAllOrderBys) + .map((column) => { + return `${column} ${expressionMapAllOrderBys[column]}` + }) + .join(", ") + + qb.addSelect( + `row_number() OVER (PARTITION BY ${alias}.id ORDER BY ${orderBysString}) AS rownum` + ) + } else { + qb.addSelect(`1 AS rownum`) + } + + /* + * In typeorm SelectQueryBuilder, the orderBy is removed from the original query when there is pagination + * and join involved together. + * + * This workaround allows us to include the order as part of the original query (including joins) before + * selecting the distinct ids of the main alias entity. The distinct ids deduplication + * is managed by the rownum column added to the select below. + * + * see: node_modules/typeorm/query-builder/SelectQueryBuilder.js(1973) + */ + const outerQb = new SelectQueryBuilder(qb.connection, (qb as any).queryRunner) + .select(`${qb.escape(`${alias}_id`)}`) + .from(`(${qb.getQuery()})`, alias) + .where(`${alias}.rownum = 1`) + .setParameters(qb.getParameters()) + .setNativeParameters(qb.expressionMap.nativeParameters) + .offset(optionsWithoutRelations.skip) + .limit(optionsWithoutRelations.take) + + const mapToEntities = (array: any) => { + return array.map((rawProduct) => ({ + id: rawProduct[`${alias}_id`], + })) as unknown as T[] + } + let entities: T[] let count = 0 if (shouldCount) { - const result = await promiseAll([qb.getMany(), qb.getCount()]) - entities = result[0] - count = result[1] + const outerQbCount = new SelectQueryBuilder( + qb.connection, + (qb as any).queryRunner + ) + .select(`COUNT(1)`, `count`) + .from(`(${qb.getQuery()})`, alias) + .where(`${alias}.rownum = 1`) + .setParameters(qb.getParameters()) + .setNativeParameters(qb.expressionMap.nativeParameters) + .orderBy() + .groupBy() + .offset(undefined) + .limit(undefined) + .skip(undefined) + .take(undefined) + + const result = await promiseAll([ + outerQb.getRawMany(), + outerQbCount.getRawOne(), + ]) + + entities = mapToEntities(result[0]) + count = Number(result[1].count) } else { - entities = await qb.getMany() + const result = await outerQb.getRawMany() + entities = mapToEntities(result) } return [entities, count]