fix: Sort product categories based on rank (#7702)

integration-tests/http/__tests__/product-category/admin/product-category.spec.ts

@@ -62,6 +62,19 @@ medusaIntegrationTestRunner({
           )
         ).data.product_category
 
+        productCategoryChild1 = (
+          await api.post(
+            "/admin/product-categories",
+            {
+              name: "category child 1",
+              parent_category_id: productCategoryChild.id,
+              rank: 1,
+              description: "category child 1",
+            },
+            adminHeaders
+          )
+        ).data.product_category
+
         productCategoryChild2 = (
           await api.post(
             "/admin/product-categories",
@@ -94,6 +107,12 @@ medusaIntegrationTestRunner({
                 name: productCategoryChild.name,
                 handle: productCategoryChild.handle,
                 category_children: [
+                  expect.objectContaining({
+                    id: productCategoryChild1.id,
+                    name: productCategoryChild1.name,
+                    handle: productCategoryChild1.handle,
+                    category_children: [],
+                  }),
                   expect.objectContaining({
                     id: productCategoryChild2.id,
                     name: productCategoryChild2.name,
@@ -108,11 +127,57 @@ medusaIntegrationTestRunner({
 
         expect(response.status).toEqual(200)
       })
+
+      it("gets a category with children sorted by rank", async () => {
+        const path = `/admin/product-categories/${productCategoryChild.id}?include_descendants_tree=true`
+        let response = await api.get(path, adminHeaders)
+
+        expect(response.status).toEqual(200)
+        expect(response.data.product_category).toEqual(
+          expect.objectContaining({
+            id: productCategoryChild.id,
+            parent_category_id: productCategory.id,
+            category_children: [
+              expect.objectContaining({
+                id: productCategoryChild1.id,
+                handle: productCategoryChild1.handle,
+              }),
+              expect.objectContaining({
+                id: productCategoryChild2.id,
+                handle: productCategoryChild2.handle,
+              }),
+            ],
+          })
+        )
+
+        await api.post(
+          `/admin/product-categories/${productCategoryChild2.id}`,
+          { rank: 0 },
+          adminHeaders
+        )
+
+        response = await api.get(path, adminHeaders)
+        expect(response.status).toEqual(200)
+        expect(response.data.product_category).toEqual(
+          expect.objectContaining({
+            id: productCategoryChild.id,
+            parent_category_id: productCategory.id,
+            category_children: [
+              expect.objectContaining({
+                id: productCategoryChild2.id,
+                handle: productCategoryChild2.handle,
+              }),
+              expect.objectContaining({
+                id: productCategoryChild1.id,
+                handle: productCategoryChild1.handle,
+              }),
+            ],
+          })
+        )
+      })
     })
 
     describe("GET /admin/product-categories", () => {
-      // TODO/BREAKING: We don't support rank reordering upon creation in V2
-      //   New categories with the same parent are always added at the end of the "list"
       beforeEach(async () => {
         productCategoryParent = (
           await api.post(
@@ -352,6 +417,125 @@ medusaIntegrationTestRunner({
         )
       })
 
+      it("gets list of product category with immediate children and parents", async () => {
+        const path = `/admin/product-categories?include_descendants_tree=true`
+        const newChild = (
+          await api.post(
+            "/admin/product-categories",
+            {
+              name: "cotton",
+              parent_category_id: productCategory.id,
+            },
+            adminHeaders
+          )
+        ).data.product_category
+
+        let response = await api.get(path, adminHeaders)
+
+        expect(response.status).toEqual(200)
+        expect(response.data.product_categories).toEqual(
+          expect.arrayContaining([
+            expect.objectContaining({
+              id: productCategoryParent.id,
+              category_children: [
+                expect.objectContaining({
+                  id: productCategory.id,
+                  handle: productCategory.handle,
+                  category_children: [
+                    expect.objectContaining({
+                      id: productCategoryChild.id,
+                      parent_category_id: productCategory.id,
+                      category_children: [
+                        expect.objectContaining({
+                          id: productCategoryChild0.id,
+                          handle: productCategoryChild0.handle,
+                        }),
+                        expect.objectContaining({
+                          id: productCategoryChild1.id,
+                          handle: productCategoryChild1.handle,
+                        }),
+                        expect.objectContaining({
+                          id: productCategoryChild2.id,
+                          handle: productCategoryChild2.handle,
+                        }),
+                        expect.objectContaining({
+                          id: productCategoryChild3.id,
+                          handle: productCategoryChild3.handle,
+                        }),
+                      ],
+                    }),
+                    expect.objectContaining({
+                      id: newChild.id,
+                      handle: newChild.handle,
+                      category_children: [],
+                    }),
+                  ],
+                }),
+              ],
+            }),
+          ])
+        )
+
+        await api.post(
+          `/admin/product-categories/${productCategoryChild2.id}`,
+          { rank: 0 },
+          adminHeaders
+        )
+
+        await api.post(
+          `/admin/product-categories/${newChild.id}`,
+          { rank: 0 },
+          adminHeaders
+        )
+
+        response = await api.get(path, adminHeaders)
+
+        expect(response.status).toEqual(200)
+        expect(response.data.product_categories).toEqual(
+          expect.arrayContaining([
+            expect.objectContaining({
+              id: productCategoryParent.id,
+              category_children: [
+                expect.objectContaining({
+                  id: productCategory.id,
+                  handle: productCategory.handle,
+                  category_children: [
+                    expect.objectContaining({
+                      id: newChild.id,
+                      handle: newChild.handle,
+                      category_children: [],
+                    }),
+                    expect.objectContaining({
+                      id: productCategoryChild.id,
+                      parent_category_id: productCategory.id,
+                      category_children: [
+                        expect.objectContaining({
+                          id: productCategoryChild2.id,
+                          handle: productCategoryChild2.handle,
+                        }),
+                        expect.objectContaining({
+                          id: productCategoryChild0.id,
+                          handle: productCategoryChild0.handle,
+                        }),
+                        expect.objectContaining({
+                          id: productCategoryChild1.id,
+                          handle: productCategoryChild1.handle,
+                        }),
+
+                        expect.objectContaining({
+                          id: productCategoryChild3.id,
+                          handle: productCategoryChild3.handle,
+                        }),
+                      ],
+                    }),
+                  ],
+                }),
+              ],
+            }),
+          ])
+        )
+      })
+
       it("filters based on whitelisted attributes of the data model", async () => {
         const response = await api.get(
           `/admin/product-categories?is_internal=true&limit=7`,

packages/modules/product/integration-tests/__tests__/product-category.ts

@@ -297,39 +297,39 @@ moduleIntegrationTestRunner<Service>({
               mpath: "electronics.computers.laptops.gaming-laptops",
               parent_category_id: "laptops",
               category_children: [
-                {
+                expect.objectContaining({
                   id: "budget-gaming",
                   handle: "budget-gaming-laptops",
                   mpath:
                     "electronics.computers.laptops.gaming-laptops.budget-gaming",
                   parent_category_id: "gaming-laptops",
                   category_children: [],
-                },
+                }),
-                {
+                expect.objectContaining({
                   id: "high-performance",
                   handle: "high-performance-gaming-laptops",
                   mpath:
                     "electronics.computers.laptops.gaming-laptops.high-performance",
                   parent_category_id: "gaming-laptops",
                   category_children: expect.arrayContaining([
-                    {
+                    expect.objectContaining({
-                      id: "4k-gaming",
-                      handle: "4k-gaming-laptops",
-                      mpath:
-                        "electronics.computers.laptops.gaming-laptops.high-performance.4k-gaming",
-                      parent_category_id: "high-performance",
-                      category_children: [],
-                    },
-                    {
                       id: "vr-ready",
                       handle: "vr-ready-high-performance-gaming-laptops",
                       mpath:
                         "electronics.computers.laptops.gaming-laptops.high-performance.vr-ready",
                       parent_category_id: "high-performance",
                       category_children: [],
-                    },
+                    }),
+                    expect.objectContaining({
+                      id: "4k-gaming",
+                      handle: "4k-gaming-laptops",
+                      mpath:
+                        "electronics.computers.laptops.gaming-laptops.high-performance.4k-gaming",
+                      parent_category_id: "high-performance",
+                      category_children: [],
+                    }),
                   ]),
-                },
+                }),
               ],
             },
           ])
@@ -356,47 +356,47 @@ moduleIntegrationTestRunner<Service>({
           )
 
           expect(serializedObject).toEqual([
-            {
+            expect.objectContaining({
               id: "gaming-laptops",
               handle: "gaming-laptops",
               mpath: "electronics.computers.laptops.gaming-laptops",
               parent_category_id: "laptops",
-              parent_category: {
+              parent_category: expect.objectContaining({
                 id: "laptops",
                 handle: "laptops",
                 mpath: "electronics.computers.laptops",
                 parent_category_id: "computers",
-                parent_category: {
+                parent_category: expect.objectContaining({
                   id: "computers",
                   handle: "computers-&-accessories",
                   mpath: "electronics.computers",
                   parent_category_id: "electronics",
-                  parent_category: {
+                  parent_category: expect.objectContaining({
                     id: "electronics",
                     handle: "electronics",
                     mpath: "electronics",
                     parent_category_id: null,
                     parent_category: null,
-                  },
+                  }),
-                },
+                }),
-              },
+              }),
               category_children: [
-                {
+                expect.objectContaining({
                   id: "budget-gaming",
                   handle: "budget-gaming-laptops",
                   mpath:
                     "electronics.computers.laptops.gaming-laptops.budget-gaming",
                   parent_category_id: "gaming-laptops",
-                },
+                }),
-                {
+                expect.objectContaining({
                   id: "high-performance",
                   handle: "high-performance-gaming-laptops",
                   mpath:
                     "electronics.computers.laptops.gaming-laptops.high-performance",
                   parent_category_id: "gaming-laptops",
-                },
+                }),
               ],
-            },
+            }),
           ])
         })
 

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

@@ -21,7 +21,9 @@ export class ProductCategoryRepository extends DALUtils.MikroOrmBaseTreeReposito
     familyOptions: ProductCategoryTransformOptions = {}
   ) {
     const findOptions_ = { ...findOptions }
-    findOptions_.options ??= {}
+    findOptions_.options ??= {
+      orderBy: { rank: "ASC" },
+    }
 
     const fields = (findOptions_.options.fields ??= [])
     const populate = (findOptions_.options.populate ??= [])
@@ -86,7 +88,7 @@ export class ProductCategoryRepository extends DALUtils.MikroOrmBaseTreeReposito
       return productCategories
     }
 
-    return await this.buildProductCategoriesWithTree(
+    const categoriesTree = await this.buildProductCategoriesWithTree(
       {
         descendants: transformOptions.includeDescendantsTree,
         ancestors: transformOptions.includeAncestorsTree,
@@ -94,6 +96,23 @@ export class ProductCategoryRepository extends DALUtils.MikroOrmBaseTreeReposito
       productCategories,
       findOptions_
     )
+
+    return this.sortCategoriesByRank(categoriesTree)
+  }
+
+  sortCategoriesByRank(categories: ProductCategory[]): ProductCategory[] {
+    const sortedCategories = categories.sort((a, b) => a.rank - b.rank)
+
+    for (const category of sortedCategories) {
+      if (category.category_children) {
+        // All data up to this point is manipulated as an array, but it is a Collection<ProductCategory> type under the hood, so we are casting to any here.
+        category.category_children = this.sortCategoriesByRank(
+          category.category_children as any
+        ) as any
+      }
+    }
+
+    return sortedCategories
   }
 
   async buildProductCategoriesWithTree(
@@ -228,17 +247,16 @@ export class ProductCategoryRepository extends DALUtils.MikroOrmBaseTreeReposito
       return [productCategories, count]
     }
 
-    return [
+    const categoriesTree = await this.buildProductCategoriesWithTree(
-      await this.buildProductCategoriesWithTree(
       {
         descendants: transformOptions.includeDescendantsTree,
         ancestors: transformOptions.includeAncestorsTree,
       },
       productCategories,
       findOptions_
-      ),
+    )
-      count,
+
-    ]
+    return [this.sortCategoriesByRank(categoriesTree), count]
   }
 
   async delete(ids: string[], context: Context = {}): Promise<void> {
@@ -325,13 +343,13 @@ export class ProductCategoryRepository extends DALUtils.MikroOrmBaseTreeReposito
           parent_category_id: categoryData?.parent_category_id || null,
         })
 
-        if (!isDefined(categoryData.rank)) {
+        categoryData.rank ??= siblingsCount + i
-          categoryData.rank = siblingsCount + i
-        } else {
         if (categoryData.rank > siblingsCount + i) {
           categoryData.rank = siblingsCount + i
         }
 
+        // There is no need to rerank if it is the last item in the list
+        if (categoryData.rank < siblingsCount + i) {
           await this.rerankSiblingsAfterCreation(manager, categoryData)
         }
 
@@ -375,10 +393,30 @@ export class ProductCategoryRepository extends DALUtils.MikroOrmBaseTreeReposito
           id: categoryData.id,
         })
 
+        // If the parent or rank are not changed, no need to reorder anything.
         if (
-          categoryData.parent_category_id &&
+          !isDefined(categoryData.parent_category_id) &&
+          !isDefined(categoryData.rank)
+        ) {
+          for (const key in categoryData) {
+            if (isDefined(categoryData[key])) {
+              productCategory[key] = categoryData[key]
+            }
+          }
+
+          manager.assign(productCategory, categoryData)
+          return productCategory
+        }
+
+        // If the parent is changed, we need to rerank the siblings of the old parent and the new parent.
+        if (
+          isDefined(categoryData.parent_category_id) &&
           categoryData.parent_category_id !== productCategory.parent_category_id
         ) {
+          // Calculate the new mpath
+          if (categoryData.parent_category_id === null) {
+            categoryData.mpath = ""
+          } else {
             const newParentCategory = await manager.findOne(
               ProductCategory,
               categoryData.parent_category_id
@@ -389,31 +427,42 @@ export class ProductCategoryRepository extends DALUtils.MikroOrmBaseTreeReposito
                 `Parent category with id: '${categoryData.parent_category_id}' does not exist`
               )
             }
-
             categoryData.mpath = `${newParentCategory.mpath}.${productCategory.id}`
+          }
 
+          // Rerank the siblings in the new parent
           const siblingsCount = await manager.count(ProductCategory, {
             parent_category_id: categoryData.parent_category_id,
           })
-          if (!isDefined(categoryData.rank)) {
+
-            categoryData.rank = siblingsCount + i
+          categoryData.rank ??= siblingsCount + i
-          } else {
           if (categoryData.rank > siblingsCount + i) {
             categoryData.rank = siblingsCount + i
           }
 
+          // There is no need to rerank if it is the last item in the list
+          if (categoryData.rank < siblingsCount + i) {
             await this.rerankSiblingsAfterCreation(manager, categoryData)
           }
 
+          // Rerank the old parent's siblings
           await this.rerankSiblingsAfterDeletion(manager, productCategory)
+
+          for (const key in categoryData) {
+            if (isDefined(categoryData[key])) {
+              productCategory[key] = categoryData[key]
             }
-        // In the case of the parent being updated, we do a delete/create reranking. If only the rank was updated, we need to shift all siblings
+          }
-        else if (isDefined(categoryData.rank)) {
+
+          manager.assign(productCategory, categoryData)
+          return productCategory
+          // If only the rank changed, we need to rerank all siblings.
+        } else if (isDefined(categoryData.rank)) {
           const siblingsCount = await manager.count(ProductCategory, {
             parent_category_id: productCategory.parent_category_id,
           })
 
-          // We don't cout the updated category itself.
+          // Subtracting 1 since we don't count the modified category itself
           if (categoryData.rank > siblingsCount - 1 + i) {
             categoryData.rank = siblingsCount - 1 + i
           }