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

fix: refactor batch product update

Author: peterlgh7

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,
-              title: "new variant 2",
-              options: { size: "l", color: "green" }, // just preserve old one
+              id: existingVariant2.id, // 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 { DALUtils } from "@medusajs/framework/utils"
-import { SqlEntityManager } from "@mikro-orm/postgresql"
+import { Context, DAL, InferEntityType } from "@medusajs/framework/types"
+import { buildQuery, DALUtils } from "@medusajs/framework/utils"
+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