fix: move product update logic to repository

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

@@ -42,6 +42,7 @@ import {
   removeUndefined,
   toHandle,
 } from "@medusajs/framework/utils"
+import { ProductRepository } from "../repositories"
 import {
   UpdateCategoryInput,
   UpdateCollectionInput,
@@ -56,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>
@@ -112,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>
   >
@@ -142,6 +145,7 @@ export default class ProductModuleService
   constructor(
     {
       baseRepository,
+      productRepository,
       productService,
       productVariantService,
       productTagService,
@@ -160,6 +164,7 @@ export default class ProductModuleService
     super(...arguments)
 
     this.baseRepository_ = baseRepository
+    this.productRepository_ = productRepository
     this.productService_ = productService
     this.productVariantService_ = productVariantService
     this.productTagService_ = productTagService
@@ -1660,82 +1665,11 @@ export default class ProductModuleService
       this.validateProductUpdatePayload(product)
     }
 
-    const products = await this.productService_.list(
+    return this.productRepository_.deepUpdate(
-      { id: normalizedProducts.map((p) => p.id) },
+      normalizedProducts,
-      {
+      ProductModuleService.validateVariantOptions,
-        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]))
-
-    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,
-          }))
-        }
-
-        // 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
-
-        delete update.variants // variants are updated in the next step
-
-        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,
-          }
-
-          const options = productsMap.get(normalizedProduct.id)!.options
-          ProductModuleService.validateVariantOptions(update.variants, options)
-
-          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
   }
 
   // @ts-expect-error