fix(medusa): Issue when ordering with multiple columns (#3385)

**What**
No true fix due to the same issue as [here](https://github.com/typeorm/typeorm/issues/6294) but at least the pagination works again. The ordering can't be applied on multiple columns/relation as it produce the wrong SQL.

FIXES CORE-1193

.changeset/wicked-toes-fold.md

@@ -0,0 +1,5 @@
+---
+"@medusajs/medusa": patch
+---
+
+fix(medusa): Issue when ordering with multiple columns

integration-tests/api/__tests__/admin/product/without-ff-sales-channel.js

@@ -1,71 +0,0 @@
-const path = require("path")
-const setupServer = require("../../../../helpers/setup-server")
-const { useApi } = require("../../../../helpers/use-api")
-const { initDb, useDb } = require("../../../../helpers/use-db")
-const adminSeeder = require("../../../helpers/admin-seeder")
-
-const adminHeaders = {
-  headers: {
-    Authorization: "Bearer test_token",
-  },
-}
-
-describe("/admin/products", () => {
-  let medusaProcess
-  let dataSource
-
-  beforeAll(async () => {
-    const cwd = path.resolve(path.join(__dirname, "..", "..", ".."))
-
-    dataSource = await initDb({ cwd })
-    medusaProcess = await setupServer({
-      cwd,
-      env: {
-        MEDUSA_FF_SALES_CHANNELS: false,
-      }
-    })
-  })
-
-  afterAll(async () => {
-    const db = useDb()
-    await db.shutdown()
-
-    medusaProcess.kill()
-  })
-
-  describe("POST /admin/products", () => {
-    beforeEach(async () => {
-      await adminSeeder(dataSource)
-    })
-
-    afterEach(async () => {
-      const db = useDb()
-      await db.teardown()
-    })
-
-    it("creates a product successfully", async () => {
-      const api = useApi()
-
-      const payload = {
-        title: "Test",
-        description: "test-product-description",
-      }
-
-      const response = await api
-        .post("/admin/products", payload, adminHeaders)
-
-      expect(response.status).toEqual(200)
-      expect(response.data.product).toEqual(
-        expect.objectContaining({
-          id: expect.stringMatching(/^prod_*/),
-          title: "Test",
-          description: "test-product-description",
-          handle: "test",
-          status: "draft",
-          created_at: expect.any(String),
-          updated_at: expect.any(String),
-        })
-      )
-    })
-  })
-})

packages/medusa/src/repositories/order.ts

@@ -34,6 +34,7 @@ export const OrderRepository = dataSource.getRepository(Order).extend({
           relations: rels,
           withDeleted:
             topLevel === ITEMS_REL_NAME || topLevel === REGION_REL_NAME,
+          relationLoadStrategy: "join",
         })
       })
     ).then(flatten)

packages/medusa/src/repositories/product.ts

@@ -5,11 +5,11 @@ import {
   In,
   SelectQueryBuilder,
 } from "typeorm"
-import { Product, ProductCategory } from "../models"
+import { Product, ProductCategory, ProductVariant } from "../models"
 import { ExtendedFindConfig } from "../types/common"
 import { dataSource } from "../loaders/database"
-import { isObject } from "../utils"
 import { ProductFilterOptions } from "../types/product"
+import { buildLegacyFieldsListFrom, isObject } from "../utils"
 
 export const ProductRepository = dataSource.getRepository(Product).extend({
   async bulkAddToCollection(
@@ -58,7 +58,10 @@ export const ProductRepository = dataSource.getRepository(Product).extend({
     options: ExtendedFindConfig<Product & ProductFilterOptions>,
     q?: string
   ): Promise<[Product[], number]> {
-    const queryBuilder = await this.prepareQueryBuilder_(options, q)
+    const options_ = { ...options }
+    options_.relationLoadStrategy = "query"
+
+    const queryBuilder = await this.prepareQueryBuilder_(options_, q)
     return await queryBuilder.getManyAndCount()
   },
 
@@ -78,82 +81,108 @@ export const ProductRepository = dataSource.getRepository(Product).extend({
     const productAlias = "product"
     const queryBuilder = this.createQueryBuilder(productAlias)
 
-    // TODO: https://github.com/typeorm/typeorm/issues/9719 waiting an answer before being able to set it to `query`
+    // TODO: https://github.com/typeorm/typeorm/issues/9719
-    // Therefore use query when there is only an ordering by the product entity otherwise fallback to join.
+    // https://github.com/typeorm/typeorm/issues/6294
-    // In other word, if the order depth is more than 1 then use join otherwise use query
+    // Cleanup the repo and fix order/skip/take and relation load strategy when those issues are resolved
-    /* const orderFieldsCollectionPointSeparated = buildLegacyFieldsListFrom(
+
+    const orderFieldsCollectionPointSeparated = buildLegacyFieldsListFrom(
       options.order ?? {}
     )
+
     const isDepth1 = !orderFieldsCollectionPointSeparated.some(
       (field) => field.indexOf(".") !== -1
     )
-    options_.relationLoadStrategy = isDepth1 ? "query" : "join"*/
+    options_.relationLoadStrategy = isDepth1
-    options_.relationLoadStrategy = "join"
+      ? options_.relationLoadStrategy
+      : "join"
 
     options_.relations = options_.relations ?? {}
     options_.where = options_.where as FindOptionsWhere<Product>
 
-    // Add explicit ordering for variant ranking on the variants join directly
+    const priceListId = options_.where.price_list_id as FindOperator<string[]>
-    // The constraint if there is any will be applied by the options_
+    const tags = options_.where.tags as FindOperator<string[]>
-    if (options_.relations.variants && !isObject(options_.order?.variants)) {
+    const salesChannelId = options_.where.sales_channel_id as FindOperator<
-      options_.order = options_.order ?? {}
+      string[]
-      options_.order.variants = {
+    >
-        variant_rank: "ASC",
+    const categoryId = options_.where.category_id as FindOperator<string[]>
-      }
+    const discountConditionId = options_.where.discount_condition_id
-      /* // The query strategy, as explain at the top of the function, does not select the column from the separated query
+    const includeCategoryChildren =
-      // It is not possible to order with that strategy at the moment and, we are waiting for an answer from the typeorm team
+      options_.where.include_category_children ?? false
-      options_.relationLoadStrategy = "join"
-      queryBuilder.leftJoinAndSelect(`${productAlias}.variants`, "variants")
 
-      options_.order = options_.order ?? {}
-
-      if (!isObject(options_.order.variants)) {
-        options_.order.variants = {
-          variant_rank: "ASC",
-        }
-      }*/
-    }
-
-    if (options_.where.price_list_id) {
-      /* options_.relations.variants = {
-        ...(isObject(options_.relations.variants)
-          ? options_.relations.variants
-          : {}),
-        prices: true,
-      }
-
-      const priceListIds = (
-        options_.where.price_list_id as FindOperator<string[]>
-      ).value
     delete options_.where.price_list_id
+    delete options_.where.tags
+    delete options_.where.sales_channel_id
+    delete options_.where.category_id
+    delete options_.where.discount_condition_id
+    delete options_.where.include_category_children
 
-      options_.where.variants = {
+    // TODO: move back to the service layer
-        ...(isObject(options_.where.variants) ? options_.where.variants : {}),
+    if (q) {
-        prices: [
+      options_.relations = options_.relations ?? {}
+      options_.relations.variants = options_.relations.variants ?? true
+      options_.relations.collection = options_.relations.collection ?? true
+
+      options_.where = [
         {
-            price_list_id: In(priceListIds),
+          ...options_.where,
+          description: ILike(`%${q}%`),
         },
-        ],
+        {
-      }*/
+          ...options_.where,
-      const priceListIds = (
+          title: ILike(`%${q}%`),
-        options_.where.price_list_id as FindOperator<string[]>
+        },
-      ).value
+        {
-      delete options_.where.price_list_id
+          ...options_.where,
+          variants: {
+            title: ILike(`%${q}%`),
+          },
+        },
+        {
+          ...options_.where,
+          variants: {
+            sku: ILike(`%${q}%`),
+          },
+        },
+        {
+          ...options_.where,
+          collection: {
+            title: ILike(`%${q}%`),
+          },
+        },
+      ]
+    }
+
+    // Add explicit ordering for variant ranking on the variants join directly
+    // This constraint is applied if no other order is applied
+    if (options_.relations.variants && !isObject(options_.order?.variants)) {
+      queryBuilder.leftJoin(
+        (subQueryBuilder) => {
+          return subQueryBuilder
+            .from(ProductVariant, "v")
+            .orderBy("v.variant_rank", "ASC")
+        },
+        "variants",
+        "product.id = variants.product_id"
+      )
+    }
+
+    if (priceListId) {
+      const priceListIds = priceListId.value
 
       queryBuilder
-        .leftJoin(`${productAlias}.variants`, "variants")
+        .leftJoin(`${productAlias}.variants`, "variants_")
-        .leftJoin("variants.prices", "ma")
+        .leftJoin("variants_.prices", "ma")
         .andWhere("ma.price_list_id IN (:...price_list_ids)", {
           price_list_ids: priceListIds,
         })
     }
 
-    if (options_.where.tags) {
+    if (tags) {
       const joinMethod = options_.relations.tags
         ? queryBuilder.leftJoinAndSelect.bind(queryBuilder)
         : queryBuilder.leftJoin.bind(queryBuilder)
 
-      const tagIds = (options_.where.tags as FindOperator<string[]>).value
+      const tagIds = tags.value
 
       // For an unknown reason, the implementation of the SelectQueryBuilder.setFindOptions -> buildWhere
       // Only check if it is a find operator MoreThan or LessThan. Otherwise, it has to be a relation of
@@ -166,19 +195,14 @@ export const ProductRepository = dataSource.getRepository(Product).extend({
           tag_ids: tagIds,
         }
       )
-
-      delete options_.where.tags
     }
 
-    if (options_.where.sales_channel_id) {
+    if (salesChannelId) {
       const joinMethod = options_.relations.sales_channel_id
         ? queryBuilder.innerJoinAndSelect.bind(queryBuilder)
         : queryBuilder.innerJoin.bind(queryBuilder)
 
-      const scIds = (options_.where.sales_channel_id as FindOperator<string[]>)
+      const scIds = salesChannelId.value
-        .value
-
-      // Same comment as in the tags if block above + inner join is only doable using the query builder and not the options
 
       joinMethod(
         `${productAlias}.sales_channels`,
@@ -188,21 +212,15 @@ export const ProductRepository = dataSource.getRepository(Product).extend({
           sales_channels_ids: scIds,
         }
       )
-
-      delete options_.where.sales_channel_id
     }
 
-    if (options_.where.category_id) {
+    if (categoryId) {
-      const includeCategoryChildren =
-        options_.where.include_category_children || false
       const joinMethod = options_.relations.category_id
         ? queryBuilder.innerJoinAndSelect.bind(queryBuilder)
         : queryBuilder.innerJoin.bind(queryBuilder)
 
-      let categoryIds = (options_.where.category_id as FindOperator<string[]>)
+      let categoryIds = categoryId.value
-        .value
 
-      // Same comment as in the tags if block above + inner join is only doable using the query builder and not the options
       if (includeCategoryChildren) {
         const categoryRepository =
           this.manager.getTreeRepository(ProductCategory)
@@ -240,58 +258,15 @@ export const ProductRepository = dataSource.getRepository(Product).extend({
           categoryIds,
         }
       )
-
-      delete options_.where.category_id
     }
 
-    delete options_.where.include_category_children
+    if (discountConditionId) {
-
-    if (options_.where.discount_condition_id) {
-      // inner join is only doable using the query builder and not the options
-
       queryBuilder.innerJoin(
         "discount_condition_product",
         "dc_product",
         `dc_product.product_id = product.id AND dc_product.condition_id = :dcId`,
-        { dcId: options_.where.discount_condition_id }
+        { dcId: discountConditionId }
       )
-
-      delete options_.where.discount_condition_id
-    }
-    // TODO: move back to the service layer
-    if (q) {
-      options_.relations = options_.relations ?? {}
-      options_.relations.variants = options_.relations.variants ?? true
-      options_.relations.collection = options_.relations.collection ?? true
-
-      options_.where = [
-        {
-          ...options_.where,
-          description: ILike(`%${q}%`),
-        },
-        {
-          ...options_.where,
-          title: ILike(`%${q}%`),
-        },
-        {
-          ...options_.where,
-          variants: {
-            title: ILike(`%${q}%`),
-          },
-        },
-        {
-          ...options_.where,
-          variants: {
-            sku: ILike(`%${q}%`),
-          },
-        },
-        {
-          ...options_.where,
-          collection: {
-            title: ILike(`%${q}%`),
-          },
-        },
-      ]
     }
 
     if (options_.withDeleted) {
@@ -299,6 +274,7 @@ export const ProductRepository = dataSource.getRepository(Product).extend({
     }
 
     queryBuilder.setFindOptions(options_)
+
     return queryBuilder
   },