From d9c8608e51d21563f0fa3ba382a06e5d5f33ae27 Mon Sep 17 00:00:00 2001 From: Pedro Guzman Date: Wed, 7 May 2025 01:57:49 +0200 Subject: [PATCH] fix: use service methods instead of entity manager for product updates --- .../product-module-service/products.spec.ts | 38 ++++++- .../src/services/product-module-service.ts | 106 ++++++++++-------- 2 files changed, 95 insertions(+), 49 deletions(-) diff --git a/packages/modules/product/integration-tests/__tests__/product-module-service/products.spec.ts b/packages/modules/product/integration-tests/__tests__/product-module-service/products.spec.ts index 1a2f68b16a..72acb05824 100644 --- a/packages/modules/product/integration-tests/__tests__/product-module-service/products.spec.ts +++ b/packages/modules/product/integration-tests/__tests__/product-module-service/products.spec.ts @@ -180,6 +180,22 @@ moduleIntegrationTestRunner({ productTwo = res[1] }) + it("should update multiple products", async () => { + await service.upsertProducts([ + { id: productOne.id, title: "updated title 1" }, + { id: productTwo.id, title: "updated title 2" }, + ]) + + const products = await service.listProducts( + { id: [productOne.id, productTwo.id] }, + { relations: ["*"] } + ) + + expect(products).toHaveLength(2) + expect(products[0].title).toEqual("updated title 1") + expect(products[1].title).toEqual("updated title 2") + }) + it("should update a product and upsert relations that are not created yet", async () => { const tags = await service.createProductTags([{ value: "tag-1" }]) const data = buildProductAndRelationsData({ @@ -839,13 +855,25 @@ moduleIntegrationTestRunner({ expect(product.options).toHaveLength(1) expect(product.options[0].title).toEqual("material") - expect(product.options[0].values).toHaveLength(2) - expect(product.options[0].values[0].value).toEqual("cotton") - expect(product.options[0].values[1].value).toEqual("silk") + expect(product.options[0].values).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + value: "cotton", + }), + expect.objectContaining({ + value: "silk", + }), + ]) + ) expect(product.variants).toHaveLength(1) - expect(product.variants[0].options).toHaveLength(1) - expect(product.variants[0].options[0].value).toEqual("cotton") + expect(product.variants[0].options).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + value: "cotton", + }), + ]) + ) }) it("should throw an error when some tag id does not exist", async () => { diff --git a/packages/modules/product/src/services/product-module-service.ts b/packages/modules/product/src/services/product-module-service.ts index 857c928371..f6ecc90c80 100644 --- a/packages/modules/product/src/services/product-module-service.ts +++ b/packages/modules/product/src/services/product-module-service.ts @@ -42,7 +42,6 @@ import { removeUndefined, toHandle, } from "@medusajs/framework/utils" -import { wrap } from "@mikro-orm/core" import { UpdateCategoryInput, UpdateCollectionInput, @@ -1663,59 +1662,78 @@ export default class ProductModuleService const products = await this.productService_.list( { id: normalizedProducts.map((p) => p.id) }, - { relations: ["*"] }, + { + relations: [ + "images", + "variants", + "variants.options", + "options", + "options.values", + ], + }, // loading all relations is the only way for productService_.update to update deep relations, otherwise it triggers INSERTS for all relations sharedContext ) const productsMap = new Map(products.map((p) => [p.id, p])) - for (const normalizedProduct of normalizedProducts) { - const update = normalizedProduct as any - const product = productsMap.get(update.id)! + await this.productService_.update( + normalizedProducts.map((normalizedProduct: any) => { + const update = { ...normalizedProduct } + if (update.tags) { + update.tags = update.tags.map((t: { id: string }) => t.id) + } + if (update.categories) { + update.categories = update.categories.map((c: { id: string }) => c.id) + } + if (update.images) { + update.images = update.images.map((image: any, index: number) => ({ + ...image, + rank: index, + })) + } - // Assign the options first, so they'll be available for the variants loop below - if (update.options) { - wrap(product).assign({ options: update.options }) - delete update.options // already assigned above, so no longer necessary - } + // There's an integration test that checks that metadata updates are all-or-nothing + // but productService_.update merges instead, so we update the field directly + productsMap.get(normalizedProduct.id)!.metadata = update.metadata + delete update.metadata - if (update.variants) { - ProductModuleService.validateVariantOptions( - update.variants, - product.options - ) + delete update.variants // variants are updated in the next step - update.variants.forEach((variant: any) => { - if (variant.options) { - variant.options = Object.entries(variant.options).map( - ([key, value]) => { - const productOption = product.options.find( - (option) => option.title === key - )! - const productOptionValue = productOption.values?.find( - (optionValue) => optionValue.value === value - )! - return productOptionValue.id - } - ) + return update + }), + sharedContext + ) + + // We update variants in a second step because we need the options to be updated first + await this.productService_.update( + normalizedProducts + .filter((p) => p.variants) + .map((normalizedProduct: any) => { + const update = { + id: normalizedProduct.id, + variants: normalizedProduct.variants, } - }) - } - if (update.tags) { - update.tags = update.tags.map((t: { id: string }) => t.id) - } - if (update.categories) { - update.categories = update.categories.map((c: { id: string }) => c.id) - } - if (update.images) { - update.images = update.images.map((image: any, index: number) => ({ - ...image, - rank: index, - })) - } + const options = productsMap.get(normalizedProduct.id)!.options + ProductModuleService.validateVariantOptions(update.variants, options) - wrap(product!).assign(update) - } + update.variants.forEach((variant: any) => { + if (variant.options) { + variant.options = Object.entries(variant.options).map( + ([key, value]) => { + const option = options.find((option) => option.title === key)! + const optionValue = option.values?.find( + (optionValue) => optionValue.value === value + )! + return optionValue.id + } + ) + } + }) + + return update + }), + sharedContext + ) return products }