fix(medusa): category list api bug where limit skews results (#3914)

* chore: fix category list api bug where limit skews results

* chore: add limits to integration test

.changeset/popular-apricots-sip.md

@@ -0,0 +1,5 @@
+---
+"@medusajs/medusa": patch
+---
+
+fix(medusa): fix category list api bug where limit skews results

integration-tests/api/__tests__/admin/product-category.ts

@@ -179,14 +179,14 @@ describe("/admin/product-categories", () => {
       const api = useApi()
 
       const response = await api.get(
-        `/admin/product-categories`,
+        `/admin/product-categories?limit=7`,
         adminHeaders
       )
 
       expect(response.status).toEqual(200)
       expect(response.data.count).toEqual(7)
       expect(response.data.offset).toEqual(0)
-      expect(response.data.limit).toEqual(100)
+      expect(response.data.limit).toEqual(7)
 
       expect(response.data.product_categories).toEqual(
         [
@@ -296,7 +296,7 @@ describe("/admin/product-categories", () => {
       const api = useApi()
 
       const response = await api.get(
-        `/admin/product-categories?is_internal=true`,
+        `/admin/product-categories?is_internal=true&limit=7`,
         adminHeaders,
       )
 
@@ -309,7 +309,7 @@ describe("/admin/product-categories", () => {
       const api = useApi()
 
       const response = await api.get(
-        `/admin/product-categories?handle=${productCategory.handle}`,
+        `/admin/product-categories?handle=${productCategory.handle}&limit=2`,
         adminHeaders,
       )
 
@@ -322,7 +322,7 @@ describe("/admin/product-categories", () => {
       const api = useApi()
 
       const response = await api.get(
-        `/admin/product-categories?q=men`,
+        `/admin/product-categories?q=men&limit=1`,
         adminHeaders,
       )
 
@@ -335,7 +335,7 @@ describe("/admin/product-categories", () => {
       const api = useApi()
 
       const response = await api.get(
-        `/admin/product-categories?parent_category_id=${productCategoryParent.id}`,
+        `/admin/product-categories?parent_category_id=${productCategoryParent.id}&limit=7`,
         adminHeaders,
       )
 
@@ -357,7 +357,7 @@ describe("/admin/product-categories", () => {
       const api = useApi()
 
       const response = await api.get(
-        `/admin/product-categories?parent_category_id=null&include_descendants_tree=true`,
+        `/admin/product-categories?parent_category_id=null&include_descendants_tree=true&limit=7`,
         adminHeaders
       )
 

integration-tests/api/__tests__/store/product-category.ts

@@ -178,13 +178,13 @@ describe("/store/product-categories", () => {
       const api = useApi()
 
       const response = await api.get(
-        `/store/product-categories`,
+        `/store/product-categories?limit=10`,
       )
 
       expect(response.status).toEqual(200)
       expect(response.data.count).toEqual(4)
       expect(response.data.offset).toEqual(0)
-      expect(response.data.limit).toEqual(100)
+      expect(response.data.limit).toEqual(10)
 
       expect(response.data.product_categories).toEqual(
         [
@@ -239,7 +239,7 @@ describe("/store/product-categories", () => {
       const api = useApi()
 
       const response = await api.get(
-        `/store/product-categories?parent_category_id=null&include_descendants_tree=true`,
+        `/store/product-categories?parent_category_id=null&include_descendants_tree=true&limit=10`,
       )
 
       expect(response.status).toEqual(200)
@@ -280,7 +280,7 @@ describe("/store/product-categories", () => {
       const api = useApi()
 
       const error = await api.get(
-        `/store/product-categories?is_internal=true`,
+        `/store/product-categories?is_internal=true&limit=10`,
       ).catch(e => e)
 
       expect(error.response.status).toEqual(400)
@@ -292,7 +292,7 @@ describe("/store/product-categories", () => {
       const api = useApi()
 
       const response = await api.get(
-        `/store/product-categories?q=category-parent`,
+        `/store/product-categories?q=category-parent&limit=10`,
       )
 
       expect(response.status).toEqual(200)
@@ -304,7 +304,7 @@ describe("/store/product-categories", () => {
       const api = useApi()
 
       const response = await api.get(
-        `/store/product-categories?handle=${productCategory.handle}`,
+        `/store/product-categories?handle=${productCategory.handle}&limit=10`,
       )
 
       expect(response.status).toEqual(200)
@@ -316,7 +316,7 @@ describe("/store/product-categories", () => {
       const api = useApi()
 
       const response = await api.get(
-        `/store/product-categories?parent_category_id=${productCategory.id}`,
+        `/store/product-categories?parent_category_id=${productCategory.id}&limit=10`,
       )
 
       expect(response.status).toEqual(200)

packages/medusa/src/repositories/product-category.ts

@@ -51,7 +51,9 @@ export const ProductCategoryRepository = dataSource
         const modelColumns = this.metadata.ownColumns.map(
           (column) => column.propertyName
         )
-        const selectColumns = columnsSelected.length ? columnsSelected : modelColumns
+        const selectColumns = columnsSelected.length
+          ? columnsSelected
+          : modelColumns
 
         return selectColumns.map((column) => {
           return `${relationName}.${column}`
@@ -69,13 +71,16 @@ export const ProductCategoryRepository = dataSource
         delete options_.where?.name
         delete options_.where?.handle
 
-        options_.where = [{
-          ...options_.where,
-          name: ILike(`%${q}%`)
-        }, {
-          ...options_.where,
-          handle: ILike(`%${q}%`)
-        }]
+        options_.where = [
+          {
+            ...options_.where,
+            name: ILike(`%${q}%`),
+          },
+          {
+            ...options_.where,
+            handle: ILike(`%${q}%`),
+          },
+        ]
       }
 
       queryBuilder.where(options_.where)
@@ -97,8 +102,6 @@ export const ProductCategoryRepository = dataSource
             treeScope
           )
           .addSelect(fetchSelectColumns(treeRelation))
-          .addOrderBy(`${treeRelation}.rank`, "ASC")
-          .addOrderBy(`${treeRelation}.handle`, "ASC")
       })
 
       const nonTreeRelations: string[] = relationsSelected.filter(
@@ -111,15 +114,15 @@ export const ProductCategoryRepository = dataSource
 
       let [categories, count] = await queryBuilder.getManyAndCount()
 
-      if (includeTree) {
-        categories = await Promise.all(
-          categories.map(async (productCategory) => {
+      categories = await Promise.all(
+        categories.map(async (productCategory) => {
+          if (includeTree) {
             productCategory = await this.findDescendantsTree(productCategory)
+          }
 
-            return sortChildren(productCategory, treeScope)
-          })
-        )
-      }
+          return sortChildren(productCategory, treeScope)
+        })
+      )
 
       return [categories, count]
     },
@@ -166,13 +169,15 @@ const scopeChildren = (
     return category
   }
 
-  category.category_children = category.category_children.filter(
-    (categoryChild) => {
-      return !Object.entries(treeScope).some(
-        ([attribute, value]) => categoryChild[attribute] !== value
-      )
-    }
-  )
+  if (category.category_children) {
+    category.category_children = category.category_children.filter(
+      (categoryChild) => {
+        return !Object.entries(treeScope).some(
+          ([attribute, value]) => categoryChild[attribute] !== value
+        )
+      }
+    )
+  }
 
   return category
 }
@@ -182,13 +187,23 @@ const sortChildren = (
   treeScope: QuerySelector<ProductCategory> = {}
 ): ProductCategory => {
   if (category.category_children) {
-    category.category_children = category?.category_children
+    category.category_children = category.category_children
       .map(
         // Before we sort the children, we need scope the children
         // to conform to treeScope conditions
         (child) => sortChildren(scopeChildren(child, treeScope), treeScope)
       )
-      .sort((a, b) => a.rank - b.rank)
+      .sort((a, b) => {
+        // Sort by rank first
+        const rankDiff = a.rank - b.rank
+
+        // If the ranks are the same, sort by handle in ascending order
+        if (rankDiff === 0) {
+          return a.handle.localeCompare(b.handle)
+        }
+
+        return rankDiff
+      })
   }
 
   return category