fix: refactor batch product update

packages/modules/product/integration-tests/__tests__/product-module-service/products.spec.ts

@@ -12,10 +12,10 @@ import {
   ProductStatus,
 } from "@medusajs/framework/utils"
 import {
-  ProductImage,
   Product,
   ProductCategory,
   ProductCollection,
+  ProductImage,
   ProductType,
 } from "@models"
 
@@ -400,9 +400,7 @@ moduleIntegrationTestRunner<IProductModuleService>({
               options: { size: "x", color: "red" }, // update options
             },
             {
-              id: existingVariant2.id,
+              id: existingVariant2.id, // just preserve old one
-              title: "new variant 2",
-              options: { size: "l", color: "green" }, // just preserve old one
             },
             {
               product_id: product.id,
@@ -722,30 +720,6 @@ moduleIntegrationTestRunner<IProductModuleService>({
           expect(error).toEqual(`Product with id: does-not-exist was not found`)
         })
 
-        it("should throw because variant doesn't have all options set", async () => {
-          const error = await service
-            .createProducts([
-              {
-                title: "Product with variants and options",
-                options: [
-                  { title: "opt1", values: ["1", "2"] },
-                  { title: "opt2", values: ["3", "4"] },
-                ],
-                variants: [
-                  {
-                    title: "missing option",
-                    options: { opt1: "1" },
-                  },
-                ],
-              },
-            ])
-            .catch((e) => e)
-
-          expect(error.message).toEqual(
-            `Product "Product with variants and options" has variants with missing options: [missing option]`
-          )
-        })
-
         it("should update, create and delete variants", async () => {
           const updateData = {
             id: productTwo.id,
@@ -849,6 +823,136 @@ moduleIntegrationTestRunner<IProductModuleService>({
             ])
           )
         })
+
+        it("should simultaneously update options and variants", async () => {
+          const updateData = {
+            id: productTwo.id,
+            options: [{ title: "material", values: ["cotton", "silk"] }],
+            variants: [{ title: "variant 1", options: { material: "cotton" } }],
+          }
+
+          await service.upsertProducts([updateData])
+
+          const product = await service.retrieveProduct(productTwo.id, {
+            relations: ["*"],
+          })
+
+          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.variants).toHaveLength(1)
+          expect(product.variants[0].options).toHaveLength(1)
+          expect(product.variants[0].options[0].value).toEqual("cotton")
+        })
+
+        it("should throw an error when some tag id does not exist", async () => {
+          const error = await service
+            .updateProducts(productOne.id, {
+              tag_ids: ["does-not-exist"],
+            })
+            .catch((e) => e)
+
+          expect(error?.message).toEqual(
+            `You tried to set relationship product_tag_id: does-not-exist, but such entity does not exist`
+          )
+        })
+
+        it("should throw an error when some category id does not exist", async () => {
+          const error = await service
+            .updateProducts(productOne.id, {
+              category_ids: ["does-not-exist"],
+            })
+            .catch((e) => e)
+
+          expect(error?.message).toEqual(
+            `You tried to set relationship product_category_id: does-not-exist, but such entity does not exist`
+          )
+        })
+
+        it("should throw an error when collection id does not exist", async () => {
+          const error = await service
+            .updateProducts(productOne.id, {
+              collection_id: "does-not-exist",
+            })
+            .catch((e) => e)
+
+          expect(error?.message).toEqual(
+            `You tried to set relationship collection_id: does-not-exist, but such entity does not exist`
+          )
+        })
+
+        it("should throw an error when type id does not exist", async () => {
+          const error = await service
+            .updateProducts(productOne.id, {
+              type_id: "does-not-exist",
+            })
+            .catch((e) => e)
+
+          expect(error?.message).toEqual(
+            `You tried to set relationship type_id: does-not-exist, but such entity does not exist`
+          )
+        })
+
+        it("should throw if two variants have the same options combination", async () => {
+          const error = await service
+            .updateProducts(productTwo.id, {
+              variants: [
+                {
+                  title: "variant 1",
+                  options: { size: "small", color: "blue" },
+                },
+                {
+                  title: "variant 2",
+                  options: { size: "small", color: "blue" },
+                },
+              ],
+            })
+            .catch((e) => e)
+
+          expect(error?.message).toEqual(
+            `Variant "variant 1" has same combination of option values as "variant 2".`
+          )
+        })
+
+        it("should throw if a variant doesn't have all options set", async () => {
+          const error = await service
+            .updateProducts(productTwo.id, {
+              variants: [
+                {
+                  title: "variant 1",
+                  options: { size: "small" },
+                },
+              ],
+            })
+            .catch((e) => e)
+
+          expect(error?.message).toEqual(
+            `Product has 2 option values but there were 1 provided option values for the variant: variant 1.`
+          )
+        })
+
+        it("should throw if a variant uses a non-existing option", async () => {
+          const error = await service
+            .updateProducts(productTwo.id, {
+              variants: [
+                {
+                  title: "variant 1",
+                  options: {
+                    size: "small",
+                    non_existing_option: "non_existing_value",
+                  },
+                },
+              ],
+            })
+            .catch((e) => e)
+
+          expect(error?.message).toEqual(
+            `Option value non_existing_value does not exist for option non_existing_option`
+          )
+        })
       })
 
       describe("create", function () {
@@ -963,6 +1067,30 @@ moduleIntegrationTestRunner<IProductModuleService>({
             }
           )
         })
+
+        it("should throw because variant doesn't have all options set", async () => {
+          const error = await service
+            .createProducts([
+              {
+                title: "Product with variants and options",
+                options: [
+                  { title: "opt1", values: ["1", "2"] },
+                  { title: "opt2", values: ["3", "4"] },
+                ],
+                variants: [
+                  {
+                    title: "missing option",
+                    options: { opt1: "1" },
+                  },
+                ],
+              },
+            ])
+            .catch((e) => e)
+
+          expect(error.message).toEqual(
+            `Product "Product with variants and options" has variants with missing options: [missing option]`
+          )
+        })
       })
 
       describe("softDelete", function () {
@@ -1408,6 +1536,28 @@ moduleIntegrationTestRunner<IProductModuleService>({
           ])
         })
 
+        it("should delete images if empty array is passed on update", async () => {
+          const images = [
+            { url: "image-1" },
+            { url: "image-2" },
+            { url: "image-3" },
+          ]
+
+          const [product] = await service.createProducts([
+            buildProductAndRelationsData({ images }),
+          ])
+
+          await service.updateProducts(product.id, {
+            images: [],
+          })
+
+          const productAfterUpdate = await service.retrieveProduct(product.id, {
+            relations: ["*"],
+          })
+
+          expect(productAfterUpdate.images).toHaveLength(0)
+        })
+
         it("should retrieve images in the correct order consistently", async () => {
           const images = Array.from({ length: 1000 }, (_, i) => ({
             url: `image-${i + 1}`,

packages/modules/product/src/services/product-module-service.ts

@@ -39,10 +39,10 @@ import {
   MedusaService,
   Modules,
   ProductStatus,
-  promiseAll,
   removeUndefined,
   toHandle,
 } from "@medusajs/framework/utils"
+import { wrap } from "@mikro-orm/core"
 import {
   UpdateCategoryInput,
   UpdateCollectionInput,
@@ -1661,124 +1661,63 @@ export default class ProductModuleService
       this.validateProductUpdatePayload(product)
     }
 
-    const { entities: productData } =
+    const products = await this.productService_.list(
-      await this.productService_.upsertWithReplace(
+      { id: normalizedProducts.map((p) => p.id) },
-        normalizedProducts,
+      { relations: ["*"] },
-        {
+      sharedContext
-          relations: ["tags", "categories"],
-        },
-        sharedContext
-      )
-
-    // There is more than 1-level depth of relations here, so we need to handle the options and variants manually
-    await promiseAll(
-      // Note: It's safe to rely on the order here as `upsertWithReplace` preserves the order of the input
-      normalizedProducts.map(async (product, i) => {
-        const upsertedProduct: any = productData[i]
-        let allOptions: any[] = []
-
-        if (product.options?.length) {
-          const { entities: productOptions } =
-            await this.productOptionService_.upsertWithReplace(
-              product.options?.map((option) => ({
-                ...option,
-                product_id: upsertedProduct.id,
-              })) ?? [],
-              { relations: ["values"] },
-              sharedContext
-            )
-          upsertedProduct.options = productOptions
-
-          // Since we handle the options and variants outside of the product upsert, we need to clean up manually
-          await this.productOptionService_.delete(
-            {
-              product_id: upsertedProduct.id,
-              id: {
-                $nin: upsertedProduct.options.map(({ id }) => id),
-              },
-            },
-            sharedContext
-          )
-          allOptions = upsertedProduct.options
-        } else {
-          // If the options weren't affected, but the user is changing the variants, make sure we have all options available locally
-          if (product.variants?.length) {
-            allOptions = await this.productOptionService_.list(
-              { product_id: upsertedProduct.id },
-              { relations: ["values"] },
-              sharedContext
-            )
-          }
-        }
-
-        if (product.variants?.length) {
-          const productVariantsWithOptions =
-            ProductModuleService.assignOptionsToVariants(
-              product.variants.map((v) => ({
-                ...v,
-                product_id: upsertedProduct.id,
-              })) ?? [],
-              allOptions
-            )
-
-          ProductModuleService.checkIfVariantsHaveUniqueOptionsCombinations(
-            productVariantsWithOptions as any
-          )
-
-          const { entities: productVariants } =
-            await this.productVariantService_.upsertWithReplace(
-              productVariantsWithOptions,
-              { relations: ["options"] },
-              sharedContext
-            )
-
-          upsertedProduct.variants = productVariants
-
-          await this.productVariantService_.delete(
-            {
-              product_id: upsertedProduct.id,
-              id: {
-                $nin: upsertedProduct.variants.map(({ id }) => id),
-              },
-            },
-            sharedContext
-          )
-        }
-
-        if (Array.isArray(product.images)) {
-          if (product.images.length) {
-            const { entities: productImages } =
-              await this.productImageService_.upsertWithReplace(
-                product.images.map((image, rank) => ({
-                  ...image,
-                  product_id: upsertedProduct.id,
-                  rank,
-                })),
-                {},
-                sharedContext
-              )
-            upsertedProduct.images = productImages
-
-            await this.productImageService_.delete(
-              {
-                product_id: upsertedProduct.id,
-                id: {
-                  $nin: productImages.map(({ id }) => id),
-                },
-              },
-              sharedContext
-            )
-          } else {
-            await this.productImageService_.delete(
-              { product_id: upsertedProduct.id },
-              sharedContext
-            )
-          }
-        }
-      })
     )
+    const productsMap = new Map(products.map((p) => [p.id, p]))
 
-    return productData
+    for (const normalizedProduct of normalizedProducts) {
+      const update = normalizedProduct as any
+      const product = productsMap.get(update.id)!
+
+      // 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
+      }
+
+      if (update.variants) {
+        ProductModuleService.validateVariantOptions(
+          update.variants,
+          product.options
+        )
+
+        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
+              }
+            )
+          }
+        })
+      }
+
+      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,
+        }))
+      }
+
+      wrap(product!).assign(update)
+    }
+
+    return products
   }
 
   // @ts-expect-error
@@ -2052,6 +1991,26 @@ export default class ProductModuleService
     return collectionData
   }
 
+  protected static validateVariantOptions(
+    variants:
+      | ProductTypes.CreateProductVariantDTO[]
+      | ProductTypes.UpdateProductVariantDTO[],
+    options: InferEntityType<typeof ProductOption>[]
+  ) {
+    const variantsWithOptions = ProductModuleService.assignOptionsToVariants(
+      variants.map((v) => ({
+        ...v,
+        // adding product_id to the variant to make it valid for the assignOptionsToVariants function
+        ...(options.length ? { product_id: options[0].product_id } : {}),
+      })),
+      options
+    )
+
+    ProductModuleService.checkIfVariantsHaveUniqueOptionsCombinations(
+      variantsWithOptions as any
+    )
+  }
+
   protected static assignOptionsToVariants(
     variants:
       | ProductTypes.CreateProductVariantDTO[]
@@ -2063,7 +2022,6 @@ export default class ProductModuleService
     if (!variants.length) {
       return variants
     }
-
     const variantsWithOptions = variants.map((variant: any) => {
       const numOfProvidedVariantOptionValues = Object.keys(
         variant.options || {}
@@ -2079,7 +2037,7 @@ export default class ProductModuleService
       ) {
         throw new MedusaError(
           MedusaError.Types.INVALID_DATA,
-          `Product has ${productsOptions.length} but there were ${numOfProvidedVariantOptionValues} provided option values for the variant: ${variant.title}.`
+          `Product has ${productsOptions.length} option values but there were ${numOfProvidedVariantOptionValues} provided option values for the variant: ${variant.title}.`
         )
       }