From dd1a8e60166dc1cfebafcad1e4c1e646e61a22c5 Mon Sep 17 00:00:00 2001 From: Stevche Radevski Date: Thu, 13 Jun 2024 16:35:50 +0200 Subject: [PATCH] fix: Sort product categories based on rank (#7702) --- .../admin/product-category.spec.ts | 188 +++++++++++++++++- .../__tests__/product-category.ts | 52 ++--- .../src/repositories/product-category.ts | 129 ++++++++---- 3 files changed, 301 insertions(+), 68 deletions(-) diff --git a/integration-tests/http/__tests__/product-category/admin/product-category.spec.ts b/integration-tests/http/__tests__/product-category/admin/product-category.spec.ts index 11e5decdb1..27ed0e0e4f 100644 --- a/integration-tests/http/__tests__/product-category/admin/product-category.spec.ts +++ b/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`, diff --git a/packages/modules/product/integration-tests/__tests__/product-category.ts b/packages/modules/product/integration-tests/__tests__/product-category.ts index 1dfa9a8ec8..95aa671336 100644 --- a/packages/modules/product/integration-tests/__tests__/product-category.ts +++ b/packages/modules/product/integration-tests/__tests__/product-category.ts @@ -297,39 +297,39 @@ moduleIntegrationTestRunner({ 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([ - { - id: "4k-gaming", - handle: "4k-gaming-laptops", - mpath: - "electronics.computers.laptops.gaming-laptops.high-performance.4k-gaming", - parent_category_id: "high-performance", - category_children: [], - }, - { + expect.objectContaining({ 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({ ) 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", - }, + }), ], - }, + }), ]) }) diff --git a/packages/modules/product/src/repositories/product-category.ts b/packages/modules/product/src/repositories/product-category.ts index 43c51ea50d..9ac171d828 100644 --- a/packages/modules/product/src/repositories/product-category.ts +++ b/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 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 [ - await this.buildProductCategoriesWithTree( - { - descendants: transformOptions.includeDescendantsTree, - ancestors: transformOptions.includeAncestorsTree, - }, - productCategories, - findOptions_ - ), - count, - ] + const categoriesTree = await this.buildProductCategoriesWithTree( + { + descendants: transformOptions.includeDescendantsTree, + ancestors: transformOptions.includeAncestorsTree, + }, + productCategories, + findOptions_ + ) + + return [this.sortCategoriesByRank(categoriesTree), count] } async delete(ids: string[], context: Context = {}): Promise { @@ -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 + if (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,45 +393,76 @@ 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 && - categoryData.parent_category_id !== productCategory.parent_category_id + !isDefined(categoryData.parent_category_id) && + !isDefined(categoryData.rank) ) { - const newParentCategory = await manager.findOne( - ProductCategory, - categoryData.parent_category_id - ) - if (!newParentCategory) { - throw new MedusaError( - MedusaError.Types.INVALID_ARGUMENT, - `Parent category with id: '${categoryData.parent_category_id}' does not exist` - ) + for (const key in categoryData) { + if (isDefined(categoryData[key])) { + productCategory[key] = categoryData[key] + } } - categoryData.mpath = `${newParentCategory.mpath}.${productCategory.id}` + 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 + ) + if (!newParentCategory) { + throw new MedusaError( + MedusaError.Types.INVALID_ARGUMENT, + `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 - } else { - if (categoryData.rank > siblingsCount + i) { - categoryData.rank = siblingsCount + i - } + categoryData.rank ??= siblingsCount + i + 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) - } - // 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)) { + + for (const key in categoryData) { + if (isDefined(categoryData[key])) { + productCategory[key] = categoryData[key] + } + } + + 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 }