Merge pull request #12367 from medusajs/fix/refactor-update-products

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"
 
@@ -180,6 +180,22 @@ moduleIntegrationTestRunner<IProductModuleService>({
           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({
@@ -400,9 +416,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 +736,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 +839,148 @@ 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).toEqual(
+            expect.arrayContaining([
+              expect.objectContaining({
+                value: "cotton",
+              }),
+              expect.objectContaining({
+                value: "silk",
+              }),
+            ])
+          )
+
+          expect(product.variants).toHaveLength(1)
+          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 () => {
+          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 +1095,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 +1564,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/repositories/product.ts

@@ -1,8 +1,8 @@
-import { Product } from "@models"
+import { Product, ProductOption } from "@models"
 
-import { Context, DAL } from "@medusajs/framework/types"
+import { Context, DAL, InferEntityType } from "@medusajs/framework/types"
-import { DALUtils } from "@medusajs/framework/utils"
+import { buildQuery, DALUtils } from "@medusajs/framework/utils"
-import { SqlEntityManager } from "@mikro-orm/postgresql"
+import { SqlEntityManager, wrap } from "@mikro-orm/postgresql"
 
 // eslint-disable-next-line max-len
 export class ProductRepository extends DALUtils.mikroOrmBaseRepositoryFactory(
@@ -13,6 +13,68 @@ export class ProductRepository extends DALUtils.mikroOrmBaseRepositoryFactory(
     super(...arguments)
   }
 
+  async deepUpdate(
+    updates: any[],
+    validateVariantOptions: (
+      variants: any[],
+      options: InferEntityType<typeof ProductOption>[]
+    ) => void,
+    context: Context = {}
+  ): Promise<InferEntityType<typeof Product>[]> {
+    const products = await this.find(
+      buildQuery({ id: updates.map((p) => p.id) }, { relations: ["*"] }),
+      context
+    )
+    const productsMap = new Map(products.map((p) => [p.id, p]))
+
+    for (const update of updates) {
+      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) {
+        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
+  }
+
   /**
    * In order to be able to have a strict not in categories, and prevent a product
    * to be return in the case it also belongs to other categories, we need to

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 { ProductRepository } from "../repositories"
 import {
   UpdateCategoryInput,
   UpdateCollectionInput,
@@ -57,6 +57,7 @@ import { joinerConfig } from "./../joiner-config"
 
 type InjectedDependencies = {
   baseRepository: DAL.RepositoryService
+  productRepository: ProductRepository
   productService: ModulesSdkTypes.IMedusaInternalService<any, any>
   productVariantService: ModulesSdkTypes.IMedusaInternalService<any, any>
   productTagService: ModulesSdkTypes.IMedusaInternalService<any>
@@ -113,6 +114,7 @@ export default class ProductModuleService
   implements ProductTypes.IProductModuleService
 {
   protected baseRepository_: DAL.RepositoryService
+  protected readonly productRepository_: ProductRepository
   protected readonly productService_: ModulesSdkTypes.IMedusaInternalService<
     InferEntityType<typeof Product>
   >
@@ -143,6 +145,7 @@ export default class ProductModuleService
   constructor(
     {
       baseRepository,
+      productRepository,
       productService,
       productVariantService,
       productTagService,
@@ -161,6 +164,7 @@ export default class ProductModuleService
     super(...arguments)
 
     this.baseRepository_ = baseRepository
+    this.productRepository_ = productRepository
     this.productService_ = productService
     this.productVariantService_ = productVariantService
     this.productTagService_ = productTagService
@@ -1661,124 +1665,11 @@ export default class ProductModuleService
       this.validateProductUpdatePayload(product)
     }
 
-    const { entities: productData } =
+    return this.productRepository_.deepUpdate(
-      await this.productService_.upsertWithReplace(
+      normalizedProducts,
-        normalizedProducts,
+      ProductModuleService.validateVariantOptions,
-        {
+      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
-            )
-          }
-        }
-      })
     )
-
-    return productData
   }
 
   // @ts-expect-error
@@ -2052,6 +1943,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 +1974,6 @@ export default class ProductModuleService
     if (!variants.length) {
       return variants
     }
-
     const variantsWithOptions = variants.map((variant: any) => {
       const numOfProvidedVariantOptionValues = Object.keys(
         variant.options || {}
@@ -2079,7 +1989,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}.`
         )
       }