feat: Improve how options are defined and handled for product (#7007)

* feat: Improve how options are defined and handled for product

* fix: Updating option values supports passing without id

integration-tests/api/__tests__/admin/product.js

@@ -806,10 +806,8 @@ medusaIntegrationTestRunner({
                       () =>
                         expect.arrayContaining([
                           expect.objectContaining({
-                            id: expect.stringMatching(/^varopt_*/),
+                            id: expect.stringMatching(/^optval_*/),
-                            option_value: expect.objectContaining({
+                            value: "100",
-                              value: "100",
-                            }),
                           }),
                         ])
                     ),
@@ -910,10 +908,8 @@ medusaIntegrationTestRunner({
                       () =>
                         expect.arrayContaining([
                           expect.objectContaining({
-                            id: expect.stringMatching(/^varopt_*/),
+                            id: expect.stringMatching(/^optval_*/),
-                            option_value: expect.objectContaining({
+                            value: "large",
-                              value: "large",
-                            }),
                           }),
                         ])
                     ),
@@ -963,10 +959,8 @@ medusaIntegrationTestRunner({
                       () =>
                         expect.arrayContaining([
                           expect.objectContaining({
-                            id: expect.stringMatching(/^varopt_*/),
+                            id: expect.stringMatching(/^optval_*/),
-                            option_value: expect.objectContaining({
+                            value: "green",
-                              value: "green",
-                            }),
                           }),
                         ])
                     ),
@@ -1394,21 +1388,17 @@ medusaIntegrationTestRunner({
                     () =>
                       expect.arrayContaining([
                         expect.objectContaining({
-                          id: expect.stringMatching(/^varopt_*/),
+                          id: expect.stringMatching(/^optval_*/),
-                          option_value: expect.objectContaining({
+                          value: "large",
-                            value: "large",
+                          option: expect.objectContaining({
-                            option: expect.objectContaining({
+                            title: "size",
-                              title: "size",
-                            }),
                           }),
                         }),
                         expect.objectContaining({
-                          id: expect.stringMatching(/^varopt_*/),
+                          id: expect.stringMatching(/^optval_*/),
-                          option_value: expect.objectContaining({
+                          value: "green",
-                            value: "green",
+                          option: expect.objectContaining({
-                            option: expect.objectContaining({
+                            title: "color",
-                              title: "color",
-                            }),
                           }),
                         }),
                       ])
@@ -1661,12 +1651,10 @@ medusaIntegrationTestRunner({
                     () =>
                       expect.arrayContaining([
                         expect.objectContaining({
-                          id: expect.stringMatching(/^varopt_*/),
+                          id: expect.stringMatching(/^optval_*/),
-                          option_value: expect.objectContaining({
+                          value: "large",
-                            value: "large",
+                          option: expect.objectContaining({
-                            option: expect.objectContaining({
+                            title: "size",
-                              title: "size",
-                            }),
                           }),
                         }),
                       ])

packages/admin-next/dashboard/src/v2-routes/products/product-detail/components/product-variant-section/use-variant-table-columns.tsx

@@ -82,7 +82,7 @@ export const useProductVariantTableColumns = (product?: Product) => {
         ),
         cell: ({ row }) => {
           const variantOpt: any = row.original.options.find(
-            (opt: any) => opt.option_value.option_id === option.id
+            (opt: any) => opt.option_id === option.id
           )
           if (!variantOpt) {
             return <PlaceholderCell />
@@ -94,7 +94,7 @@ export const useProductVariantTableColumns = (product?: Product) => {
                 size="2xsmall"
                 className="flex min-w-[20px] items-center justify-center"
               >
-                {variantOpt.option_value.value}
+                {variantOpt.value}
               </Badge>
             </div>
           )

packages/admin-next/dashboard/src/v2-routes/products/product-edit-variant/components/product-edit-variant-form/product-edit-variant-form.tsx

@@ -53,10 +53,8 @@ export const ProductEditVariantForm = ({
   const { t } = useTranslation()
   const { handleSuccess } = useRouteModal()
   const defaultOptions = product.options.reduce((acc: any, option: any) => {
-    const varOpt = variant.options.find(
+    const varOpt = variant.options.find((o: any) => o.option_id === option.id)
-      (o: any) => o.option_value.option_id === option.id
+    acc[option.title] = varOpt?.value
-    )
-    acc[option.title] = varOpt?.option_value?.value
     return acc
   }, {})
 

packages/admin-next/dashboard/src/v2-routes/products/product-edit-variant/loader.ts

@@ -0,0 +1,34 @@
+import { LoaderFunctionArgs } from "react-router-dom"
+
+import { queryClient } from "../../../lib/medusa"
+import { productsQueryKeys } from "../../../hooks/api/products"
+import { client } from "../../../lib/client"
+
+const queryKey = (id: string) => {
+  return [productsQueryKeys.detail(id)]
+}
+
+const queryFn = async (id: string) => {
+  const productRes = await client.products.retrieve(id)
+  return {
+    initialData: productRes,
+    isStockAndInventoryEnabled: false,
+  }
+}
+
+const editProductVariantQuery = (id: string) => ({
+  queryKey: queryKey(id),
+  queryFn: async () => queryFn(id),
+})
+
+export const editProductVariantLoader = async ({
+  params,
+}: LoaderFunctionArgs) => {
+  const id = params.id
+  const query = editProductVariantQuery(id!)
+
+  return (
+    queryClient.getQueryData<ReturnType<typeof queryFn>>(query.queryKey) ??
+    (await queryClient.fetchQuery(query))
+  )
+}

packages/medusa/src/api-v2/admin/products/query-config.ts

@@ -24,7 +24,6 @@ export const defaultAdminProductsVariantFields = [
   "barcode",
   "*prices",
   "*options",
-  "*options.option_value",
 ]
 
 export const retrieveVariantConfig = {
@@ -85,7 +84,6 @@ export const defaultAdminProductFields = [
   "*variants",
   "*variants.prices",
   "*variants.options",
-  "*variants.options.option_value",
   "*sales_channels",
 ]
 

packages/product/integration-tests/__tests__/services/product-module-service/product-variants.spec.ts

@@ -28,6 +28,10 @@ moduleIntegrationTestRunner({
               title: "size",
               values: ["large", "small"],
             },
+            {
+              title: "color",
+              values: ["red", "blue"],
+            },
           ],
         })
 
@@ -182,7 +186,7 @@ moduleIntegrationTestRunner({
           expect(productVariant.title).toEqual("new test")
         })
 
-        it("should update the options of a variant successfully", async () => {
+        it("should upsert the options of a variant successfully", async () => {
           await service.upsertVariants([
             {
               id: variantOne.id,
@@ -191,12 +195,33 @@ moduleIntegrationTestRunner({
           ])
 
           const productVariant = await service.retrieveVariant(variantOne.id, {
-            relations: ["options", "options.option_value", "options.variant"],
+            relations: ["options"],
           })
           expect(productVariant.options).toEqual(
             expect.arrayContaining([
               expect.objectContaining({
-                option_value: expect.objectContaining({ value: "small" }),
+                value: "small",
+              }),
+            ])
+          )
+        })
+
+        it("should do a partial update on the options of a variant successfully", async () => {
+          await service.updateVariants(variantOne.id, {
+            options: { size: "small", color: "red" },
+          })
+
+          const productVariant = await service.retrieveVariant(variantOne.id, {
+            relations: ["options"],
+          })
+
+          expect(productVariant.options).toEqual(
+            expect.arrayContaining([
+              expect.objectContaining({
+                value: "small",
+              }),
+              expect.objectContaining({
+                value: "red",
               }),
             ])
           )
@@ -222,7 +247,7 @@ moduleIntegrationTestRunner({
           const beforeDeletedVariants = await service.listVariants(
             { id: variantOne.id },
             {
-              relations: ["options", "options.option_value", "options.variant"],
+              relations: ["options"],
             }
           )
 
@@ -230,7 +255,7 @@ moduleIntegrationTestRunner({
           const deletedVariants = await service.listVariants(
             { id: variantOne.id },
             {
-              relations: ["options", "options.option_value", "options.variant"],
+              relations: ["options"],
               withDeleted: true,
             }
           )
@@ -239,9 +264,7 @@ moduleIntegrationTestRunner({
           expect(deletedVariants[0].deleted_at).not.toBeNull()
 
           for (const variantOption of deletedVariants[0].options) {
-            expect(variantOption.deleted_at).not.toBeNull()
+            expect(variantOption?.deleted_at).toBeNull()
-            // The value itself should not be affected
-            expect(variantOption?.option_value?.deleted_at).toBeNull()
           }
         })
       })

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

@@ -54,7 +54,6 @@ moduleIntegrationTestRunner({
         let productTwo: Product
         let productCategoryOne: ProductCategory
         let productCategoryTwo: ProductCategory
-        let variantTwo: ProductVariant
         let productTypeOne: ProductType
         let productTypeTwo: ProductType
         let images = [{ url: "image-1" }]
@@ -112,43 +111,57 @@ moduleIntegrationTestRunner({
           productCategoryOne = categories[0]
           productCategoryTwo = categories[1]
 
-          productOne = testManager.create(Product, {
+          productOne = service.create({
             id: "product-1",
             title: "product 1",
             status: ProductTypes.ProductStatus.PUBLISHED,
+            variants: [
+              {
+                id: "variant-1",
+                title: "variant 1",
+                inventory_quantity: 10,
+              },
+            ],
           })
 
-          productTwo = testManager.create(Product, {
+          productTwo = service.create({
             id: "product-2",
             title: "product 2",
             status: ProductTypes.ProductStatus.PUBLISHED,
-            categories: [productCategoryOne],
+            categories: [{ id: productCategoryOne.id }],
             collection_id: productCollectionOne.id,
             tags: tagsData,
+            options: [
+              {
+                title: "size",
+                values: ["large", "small"],
+              },
+              {
+                title: "color",
+                values: ["red", "blue"],
+              },
+            ],
+            variants: [
+              {
+                id: "variant-2",
+                title: "variant 2",
+                inventory_quantity: 10,
+              },
+              {
+                id: "variant-3",
+                title: "variant 3",
+                inventory_quantity: 10,
+                options: {
+                  size: "small",
+                  color: "red",
+                },
+              },
+            ],
           })
 
-          testManager.create(ProductVariant, {
+          const res = await Promise.all([productOne, productTwo])
-            id: "variant-1",
+          productOne = res[0]
-            title: "variant 1",
+          productTwo = res[1]
-            inventory_quantity: 10,
-            product: productOne,
-          })
-
-          variantTwo = testManager.create(ProductVariant, {
-            id: "variant-2",
-            title: "variant 2",
-            inventory_quantity: 10,
-            product: productTwo,
-          })
-
-          testManager.create(ProductVariant, {
-            id: "variant-3",
-            title: "variant 3",
-            inventory_quantity: 10,
-            product: productTwo,
-          })
-
-          await testManager.persistAndFlush([productOne, productTwo])
         })
 
         it("should update a product and upsert relations that are not created yet", async () => {
@@ -250,9 +263,7 @@ moduleIntegrationTestRunner({
                   options: expect.arrayContaining([
                     expect.objectContaining({
                       id: expect.any(String),
-                      option_value: expect.objectContaining({
+                      value: data.options[0].values[0],
-                        value: data.options[0].values[0],
-                      }),
                     }),
                   ]),
                 }),
@@ -435,7 +446,7 @@ moduleIntegrationTestRunner({
             // Note: VariantThree is already assigned to productTwo, that should be deleted
             variants: [
               {
-                id: variantTwo.id,
+                id: productTwo.variants[0].id,
                 title: "updated-variant",
               },
               {
@@ -456,7 +467,7 @@ moduleIntegrationTestRunner({
               id: expect.any(String),
               variants: expect.arrayContaining([
                 expect.objectContaining({
-                  id: variantTwo.id,
+                  id: productTwo.variants[0].id,
                   title: "updated-variant",
                 }),
                 expect.objectContaining({
@@ -468,6 +479,32 @@ moduleIntegrationTestRunner({
           )
         })
 
+        it("should do a partial update on the options of a variant successfully", async () => {
+          await service.update(productTwo.id, {
+            variants: [
+              {
+                id: "variant-3",
+                options: { size: "small", color: "blue" },
+              },
+            ],
+          })
+
+          const fetchedProduct = await service.retrieve(productTwo.id, {
+            relations: ["variants", "variants.options"],
+          })
+
+          expect(fetchedProduct.variants[0].options).toEqual(
+            expect.arrayContaining([
+              expect.objectContaining({
+                value: "small",
+              }),
+              expect.objectContaining({
+                value: "blue",
+              }),
+            ])
+          )
+        })
+
         it("should createa variant with id that was passed if it does not exist", async () => {
           const updateData = {
             id: productTwo.id,
@@ -583,9 +620,7 @@ moduleIntegrationTestRunner({
                   options: expect.arrayContaining([
                     expect.objectContaining({
                       id: expect.any(String),
-                      option_value: expect.objectContaining({
+                      value: data.options[0].values[0],
-                        value: data.options[0].values[0],
-                      }),
                     }),
                   ]),
                 }),

packages/product/src/migrations/.snapshot-medusa-products.json

@@ -112,17 +112,27 @@
           "length": 6,
           "default": "now()",
           "mappedType": "datetime"
+        },
+        "deleted_at": {
+          "name": "deleted_at",
+          "type": "timestamptz",
+          "unsigned": false,
+          "autoincrement": false,
+          "primary": false,
+          "nullable": true,
+          "length": 6,
+          "mappedType": "datetime"
         }
       },
       "name": "product_category",
       "schema": "public",
       "indexes": [
         {
-          "keyName": "IDX_product_category_path",
           "columnNames": [
-            "mpath"
+            "deleted_at"
           ],
           "composite": false,
+          "keyName": "IDX_product_category_deleted_at",
           "primary": false,
           "unique": false
         },
@@ -314,15 +324,6 @@
       "name": "image",
       "schema": "public",
       "indexes": [
-        {
-          "columnNames": [
-            "url"
-          ],
-          "composite": false,
-          "keyName": "IDX_product_image_url",
-          "primary": false,
-          "unique": false
-        },
         {
           "columnNames": [
             "deleted_at"
@@ -754,15 +755,6 @@
       "name": "product",
       "schema": "public",
       "indexes": [
-        {
-          "columnNames": [
-            "type_id"
-          ],
-          "composite": false,
-          "keyName": "IDX_product_type_id",
-          "primary": false,
-          "unique": false
-        },
         {
           "columnNames": [
             "deleted_at"
@@ -996,15 +988,6 @@
       "name": "product_option_value",
       "schema": "public",
       "indexes": [
-        {
-          "columnNames": [
-            "option_id"
-          ],
-          "composite": false,
-          "keyName": "IDX_product_option_value_option_id",
-          "primary": false,
-          "unique": false
-        },
         {
           "columnNames": [
             "deleted_at"
@@ -1458,15 +1441,6 @@
       "name": "product_variant",
       "schema": "public",
       "indexes": [
-        {
-          "columnNames": [
-            "product_id"
-          ],
-          "composite": false,
-          "keyName": "IDX_product_variant_product_id",
-          "primary": false,
-          "unique": false
-        },
         {
           "columnNames": [
             "deleted_at"
@@ -1505,8 +1479,8 @@
     },
     {
       "columns": {
-        "id": {
+        "variant_id": {
-          "name": "id",
+          "name": "variant_id",
           "type": "text",
           "unsigned": false,
           "autoincrement": false,
@@ -1520,88 +1494,26 @@
           "unsigned": false,
           "autoincrement": false,
           "primary": false,
-          "nullable": true,
-          "mappedType": "text"
-        },
-        "variant_id": {
-          "name": "variant_id",
-          "type": "text",
-          "unsigned": false,
-          "autoincrement": false,
-          "primary": false,
-          "nullable": true,
-          "mappedType": "text"
-        },
-        "created_at": {
-          "name": "created_at",
-          "type": "timestamptz",
-          "unsigned": false,
-          "autoincrement": false,
-          "primary": false,
           "nullable": false,
-          "length": 6,
+          "mappedType": "text"
-          "default": "now()",
-          "mappedType": "datetime"
-        },
-        "updated_at": {
-          "name": "updated_at",
-          "type": "timestamptz",
-          "unsigned": false,
-          "autoincrement": false,
-          "primary": false,
-          "nullable": false,
-          "length": 6,
-          "default": "now()",
-          "mappedType": "datetime"
-        },
-        "deleted_at": {
-          "name": "deleted_at",
-          "type": "timestamptz",
-          "unsigned": false,
-          "autoincrement": false,
-          "primary": false,
-          "nullable": true,
-          "length": 6,
-          "mappedType": "datetime"
         }
       },
       "name": "product_variant_option",
       "schema": "public",
       "indexes": [
-        {
-          "columnNames": [
-            "deleted_at"
-          ],
-          "composite": false,
-          "keyName": "IDX_product_variant_option_deleted_at",
-          "primary": false,
-          "unique": false
-        },
         {
           "keyName": "product_variant_option_pkey",
           "columnNames": [
-            "id"
+            "variant_id",
+            "option_value_id"
           ],
-          "composite": false,
+          "composite": true,
           "primary": true,
           "unique": true
         }
       ],
       "checks": [],
       "foreignKeys": {
-        "product_variant_option_option_value_id_foreign": {
-          "constraintName": "product_variant_option_option_value_id_foreign",
-          "columnNames": [
-            "option_value_id"
-          ],
-          "localTableName": "public.product_variant_option",
-          "referencedColumnNames": [
-            "id"
-          ],
-          "referencedTableName": "public.product_option_value",
-          "deleteRule": "cascade",
-          "updateRule": "cascade"
-        },
         "product_variant_option_variant_id_foreign": {
           "constraintName": "product_variant_option_variant_id_foreign",
           "columnNames": [
@@ -1614,6 +1526,19 @@
           "referencedTableName": "public.product_variant",
           "deleteRule": "cascade",
           "updateRule": "cascade"
+        },
+        "product_variant_option_option_value_id_foreign": {
+          "constraintName": "product_variant_option_option_value_id_foreign",
+          "columnNames": [
+            "option_value_id"
+          ],
+          "localTableName": "public.product_variant_option",
+          "referencedColumnNames": [
+            "id"
+          ],
+          "referencedTableName": "public.product_option_value",
+          "deleteRule": "cascade",
+          "updateRule": "cascade"
         }
       }
     }

packages/product/src/migrations/InitialSetup20240401153642.ts

@@ -42,15 +42,12 @@ export class InitialSetup20240315083440 extends Migration {
     this.addSql('create unique index if not exists "IDX_option_value_option_id_unique" on "product_option_value" (option_id, value) where deleted_at is null;')
     this.addSql('create index if not exists "IDX_product_option_value_deleted_at" on "product_option_value" ("deleted_at");');
 
-    this.addSql('create table if not exists "product_variant_option" ("id" text not null, "option_value_id" text null, "variant_id" text null, "created_at" timestamptz not null default now(), "updated_at" timestamptz not null default now(), "deleted_at" timestamptz null, constraint "product_variant_option_pkey" primary key ("id"));');
-    this.addSql('create unique index if not exists "IDX_variant_option_option_value_unique" on "product_variant_option" (variant_id, option_value_id) where deleted_at is null;')
-    this.addSql('create index if not exists "IDX_product_variant_option_deleted_at" on "product_variant_option" ("deleted_at");');
-
     this.addSql('create table if not exists "image" ("id" text not null, "url" text not null, "metadata" jsonb null, "created_at" timestamptz not null default now(), "updated_at" timestamptz not null default now(), "deleted_at" timestamptz null, constraint "image_pkey" primary key ("id"));');
     this.addSql('create index if not exists "IDX_product_image_url" on "image" ("url") where deleted_at is null;');
     this.addSql('create index if not exists "IDX_product_image_deleted_at" on "image" ("deleted_at");');
 
     this.addSql('create table if not exists "product_tag" ("id" text not null, "value" text not null, "metadata" jsonb null, "created_at" timestamptz not null default now(), "updated_at" timestamptz not null default now(), "deleted_at" timestamptz null, constraint "product_tag_pkey" primary key ("id"));');
+
     // TODO: We need to modify upsertWithReplace to handle unique constraints
     // this.addSql('create unique index if not exists "IDX_tag_value_unique" on "product_tag" (value) where deleted_at is null;')
     this.addSql('create index if not exists "IDX_product_tag_deleted_at" on "product_tag" ("deleted_at");');
@@ -72,7 +69,7 @@ export class InitialSetup20240315083440 extends Migration {
     this.addSql('create table if not exists "product_tags" ("product_id" text not null, "product_tag_id" text not null, constraint "product_tags_pkey" primary key ("product_id", "product_tag_id"));');
     this.addSql('create table if not exists "product_images" ("product_id" text not null, "image_id" text not null, constraint "product_images_pkey" primary key ("product_id", "image_id"));');
     this.addSql('create table if not exists "product_category_product" ("product_id" text not null, "product_category_id" text not null, constraint "product_category_product_pkey" primary key ("product_id", "product_category_id"));');
-
+    this.addSql('create table if not exists "product_variant_option" ("variant_id" text not null, "option_value_id" text not null, constraint "product_variant_option_pkey" primary key ("variant_id", "option_value_id"));');
 
     /* --- FOREIGN KEYS AND CONSTRAINTS --- */
     this.addSql('alter table if exists "product" add constraint "product_collection_id_foreign" foreign key ("collection_id") references "product_collection" ("id") on update cascade on delete set null;');
@@ -84,8 +81,8 @@ export class InitialSetup20240315083440 extends Migration {
 
     this.addSql('alter table if exists "product_option_value" add constraint "product_option_value_option_id_foreign" foreign key ("option_id") references "product_option" ("id") on update cascade on delete cascade;');
 
-    this.addSql('alter table if exists "product_variant_option" add constraint "product_variant_option_option_value_id_foreign" foreign key ("option_value_id") references "product_option_value" ("id") on update cascade on delete cascade;');
     this.addSql('alter table if exists "product_variant_option" add constraint "product_variant_option_variant_id_foreign" foreign key ("variant_id") references "product_variant" ("id") on update cascade on delete cascade;');
+    this.addSql('alter table if exists "product_variant_option" add constraint "product_variant_option_option_value_id_foreign" foreign key ("option_value_id") references "product_option_value" ("id") on update cascade on delete cascade;');
 
     this.addSql('alter table if exists "product_images" add constraint "product_images_product_id_foreign" foreign key ("product_id") references "product" ("id") on update cascade on delete cascade;');
     this.addSql('alter table if exists "product_images" add constraint "product_images_image_id_foreign" foreign key ("image_id") references "image" ("id") on update cascade on delete cascade;');

packages/product/src/models/index.ts

@@ -6,5 +6,4 @@ export { default as ProductType } from "./product-type"
 export { default as ProductVariant } from "./product-variant"
 export { default as ProductOption } from "./product-option"
 export { default as ProductOptionValue } from "./product-option-value"
-export { default as ProductVariantOption } from "./product-variant-option"
 export { default as Image } from "./product-image"

packages/product/src/models/product-option-value.ts

@@ -9,13 +9,13 @@ import {
   Entity,
   Filter,
   Index,
+  ManyToMany,
   ManyToOne,
   OnInit,
-  OneToMany,
   PrimaryKey,
   Property,
 } from "@mikro-orm/core"
-import { ProductOption, ProductVariantOption } from "./index"
+import { ProductOption, ProductVariant } from "./index"
 
 const optionValueOptionIdIndexName = "IDX_option_value_option_id_unique"
 const optionValueOptionIdIndexStatement = createPsqlIndexStatementHelper({
@@ -51,8 +51,8 @@ class ProductOptionValue {
   })
   option: ProductOption | null
 
-  @OneToMany(() => ProductVariantOption, (value) => value.option_value, {})
+  @ManyToMany(() => ProductVariant, (variant) => variant.options)
-  variant_options = new Collection<ProductVariantOption>(this)
+  variants = new Collection<ProductVariant>(this)
 
   @Property({ columnType: "jsonb", nullable: true })
   metadata?: Record<string, unknown> | null

packages/product/src/models/product-variant-option.ts

@@ -1,92 +0,0 @@
-import {
-  DALUtils,
-  createPsqlIndexStatementHelper,
-  generateEntityId,
-} from "@medusajs/utils"
-import {
-  BeforeCreate,
-  Entity,
-  Filter,
-  Index,
-  ManyToOne,
-  OnInit,
-  PrimaryKey,
-  Property,
-} from "@mikro-orm/core"
-import { ProductOptionValue, ProductVariant } from "./index"
-
-const variantOptionValueIndexName = "IDX_variant_option_option_value_unique"
-const variantOptionValueIndexStatement = createPsqlIndexStatementHelper({
-  name: variantOptionValueIndexName,
-  tableName: "product_variant_option",
-  columns: ["variant_id", "option_value_id"],
-  unique: true,
-  where: "deleted_at IS NULL",
-})
-
-variantOptionValueIndexStatement.MikroORMIndex()
-@Entity({ tableName: "product_variant_option" })
-@Filter(DALUtils.mikroOrmSoftDeletableFilterOptions)
-class ProductVariantOption {
-  @PrimaryKey({ columnType: "text" })
-  id!: string
-
-  @ManyToOne(() => ProductOptionValue, {
-    columnType: "text",
-    nullable: true,
-    fieldName: "option_value_id",
-    mapToPk: true,
-    onDelete: "cascade",
-  })
-  option_value_id!: string
-
-  @ManyToOne(() => ProductOptionValue, {
-    persist: false,
-    nullable: true,
-  })
-  option_value!: ProductOptionValue
-
-  @ManyToOne(() => ProductVariant, {
-    columnType: "text",
-    nullable: true,
-    fieldName: "variant_id",
-    mapToPk: true,
-    onDelete: "cascade",
-  })
-  variant_id: string | null
-
-  @ManyToOne(() => ProductVariant, {
-    persist: false,
-    nullable: true,
-  })
-  variant!: ProductVariant
-
-  @Property({
-    onCreate: () => new Date(),
-    columnType: "timestamptz",
-    defaultRaw: "now()",
-  })
-  created_at: Date
-
-  @Property({
-    onCreate: () => new Date(),
-    onUpdate: () => new Date(),
-    columnType: "timestamptz",
-    defaultRaw: "now()",
-  })
-  updated_at: Date
-
-  @Index({ name: "IDX_product_variant_option_deleted_at" })
-  @Property({ columnType: "timestamptz", nullable: true })
-  deleted_at?: Date
-
-  @OnInit()
-  @BeforeCreate()
-  onInit() {
-    this.id = generateEntityId(this.id, "varopt")
-    this.variant_id ??= this.variant?.id ?? null
-    this.option_value_id ??= this.option_value?.id ?? null
-  }
-}
-
-export default ProductVariantOption

packages/product/src/models/product-variant.ts

@@ -12,14 +12,14 @@ import {
   Entity,
   Filter,
   Index,
+  ManyToMany,
   ManyToOne,
   OneToMany,
   OnInit,
   PrimaryKey,
   Property,
 } from "@mikro-orm/core"
-import { Product } from "@models"
+import { Product, ProductOptionValue } from "@models"
-import ProductVariantOption from "./product-variant-option"
 
 const variantSkuIndexName = "IDX_product_variant_sku_unique"
 const variantSkuIndexStatement = createPsqlIndexStatementHelper({
@@ -162,14 +162,13 @@ class ProductVariant {
   })
   product: Product | null
 
-  @OneToMany(
+  @ManyToMany(() => ProductOptionValue, "variants", {
-    () => ProductVariantOption,
+    owner: true,
-    (variantOption) => variantOption.variant,
+    pivotTable: "product_variant_option",
-    {
+    joinColumn: "variant_id",
-      cascade: [Cascade.PERSIST, "soft-remove" as any],
+    inverseJoinColumn: "option_value_id",
-    }
+  })
-  )
+  options = new Collection<ProductOptionValue>(this)
-  options = new Collection<ProductVariantOption>(this)
 
   @Property({
     onCreate: () => new Date(),

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

@@ -17,7 +17,6 @@ import {
   ProductTag,
   ProductType,
   ProductVariant,
-  ProductVariantOption,
 } from "@models"
 import {
   ProductCategoryService,
@@ -66,7 +65,6 @@ type InjectedDependencies = {
   productTypeService: ProductTypeService<any>
   productOptionService: ProductOptionService<any>
   productOptionValueService: ModulesSdkTypes.InternalModuleService<any>
-  productVariantOptionService: ModulesSdkTypes.InternalModuleService<any>
   eventBusModuleService?: IEventBusModuleService
 }
 
@@ -77,11 +75,6 @@ const generateMethodForModels = [
   { model: ProductTag, singular: "Tag", plural: "Tags" },
   { model: ProductType, singular: "Type", plural: "Types" },
   { model: ProductVariant, singular: "Variant", plural: "Variants" },
-  {
-    model: ProductVariantOption,
-    singular: "VariantOption",
-    plural: "VariantOptions",
-  },
 ]
 
 export default class ProductModuleService<
@@ -93,8 +86,7 @@ export default class ProductModuleService<
     TProductImage extends Image = Image,
     TProductType extends ProductType = ProductType,
     TProductOption extends ProductOption = ProductOption,
-    TProductOptionValue extends ProductOptionValue = ProductOptionValue,
+    TProductOptionValue extends ProductOptionValue = ProductOptionValue
-    TProductVariantOption extends ProductVariantOption = ProductVariantOption
   >
   extends ModulesSdkUtils.abstractModuleServiceFactory<
     InjectedDependencies,
@@ -130,11 +122,6 @@ export default class ProductModuleService<
         singular: "Variant"
         plural: "Variants"
       }
-      VariantOption: {
-        dto: ProductTypes.ProductVariantOptionDTO
-        singular: "VariantOption"
-        plural: "VariantOptions"
-      }
     }
   >(Product, generateMethodForModels, entityNameToLinkableKeysMap)
   implements ProductTypes.IProductModuleService
@@ -154,8 +141,6 @@ export default class ProductModuleService<
   protected readonly productTypeService_: ProductTypeService<TProductType>
   protected readonly productOptionService_: ProductOptionService<TProductOption>
   // eslint-disable-next-line max-len
-  protected readonly productVariantOptionService_: ModulesSdkTypes.InternalModuleService<TProductVariantOption>
-  // eslint-disable-next-line max-len
   protected readonly productOptionValueService_: ModulesSdkTypes.InternalModuleService<TProductOptionValue>
   protected readonly eventBusModuleService_?: IEventBusModuleService
 
@@ -171,7 +156,6 @@ export default class ProductModuleService<
       productTypeService,
       productOptionService,
       productOptionValueService,
-      productVariantOptionService,
       eventBusModuleService,
     }: InjectedDependencies,
     protected readonly moduleDeclaration: InternalModuleDeclaration
@@ -190,7 +174,6 @@ export default class ProductModuleService<
     this.productTypeService_ = productTypeService
     this.productOptionService_ = productOptionService
     this.productOptionValueService_ = productOptionValueService
-    this.productVariantOptionService_ = productVariantOptionService
     this.eventBusModuleService_ = eventBusModuleService
   }
 
@@ -357,7 +340,7 @@ export default class ProductModuleService<
     const variantIdsToUpdate = data.map(({ id }) => id)
     const variants = await this.productVariantService_.list(
       { id: variantIdsToUpdate },
-      { relations: ["options"], take: null },
+      { take: null },
       sharedContext
     )
     if (variants.length !== data.length) {
@@ -686,26 +669,63 @@ export default class ProductModuleService<
     @MedusaContext() sharedContext: Context = {}
   ): Promise<ProductOption[]> {
     // Validation step
-    const normalizedInput = data.map((opt) => {
+    if (data.some((option) => !option.id)) {
-      return {
-        ...opt,
-        ...(opt.values
-          ? {
-              values: opt.values.map((v) => {
-                return typeof v === "string" ? { value: v } : v
-              }),
-            }
-          : {}),
-      } as UpdateProductOptionInput
-    })
-
-    if (normalizedInput.some((option) => !option.id)) {
       throw new MedusaError(
         MedusaError.Types.INVALID_DATA,
         "Tried to update options without specifying an ID"
       )
     }
 
+    const dbOptions = await this.productOptionService_.list(
+      { id: data.map(({ id }) => id) },
+      { take: null, relations: ["values"] },
+      sharedContext
+    )
+
+    if (dbOptions.length !== data.length) {
+      throw new MedusaError(
+        MedusaError.Types.INVALID_DATA,
+        `Cannot update non-existing options with ids: ${arrayDifference(
+          data.map(({ id }) => id),
+          dbOptions.map(({ id }) => id)
+        ).join(", ")}`
+      )
+    }
+
+    // Data normalization
+    const normalizedInput = data.map((opt) => {
+      const dbValues = dbOptions.find(({ id }) => id === opt.id)?.values || []
+      const normalizedValues = opt.values?.map((v) => {
+        return typeof v === "string" ? { value: v } : v
+      })
+
+      return {
+        ...opt,
+        ...(normalizedValues
+          ? {
+              // Oftentimes the options are only passed by value without an id, even if they exist in the DB
+              values: normalizedValues.map((normVal) => {
+                if ("id" in normVal) {
+                  return normVal
+                }
+
+                const dbVal = dbValues.find(
+                  (dbVal) => dbVal.value === normVal.value
+                )
+                if (!dbVal) {
+                  return normVal
+                }
+
+                return {
+                  id: dbVal.id,
+                  value: normVal.value,
+                }
+              }),
+            }
+          : {}),
+      } as UpdateProductOptionInput
+    })
+
     return await this.productOptionService_.upsertWithReplace(
       normalizedInput,
       { relations: ["values"] },
@@ -1188,7 +1208,7 @@ export default class ProductModuleService<
               sharedContext
             )
 
-          // Since we handle the options and variants outside of the product upsert, we need to clean up manuallys
+          // 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,
@@ -1345,8 +1365,7 @@ export default class ProductModuleService<
           }
 
           return {
-            variant_id: variant.id,
+            id: optionValue.id,
-            option_value_id: optionValue.id,
           }
         }
       )

packages/types/src/product/common.ts

@@ -237,7 +237,7 @@ export interface ProductVariantDTO {
    *
    * @expandable
    */
-  options: ProductVariantOptionDTO[]
+  options: ProductOptionValueDTO[]
   /**
    * Holds custom data in key-value pairs.
    */
@@ -554,45 +554,6 @@ export interface ProductOptionDTO {
   deleted_at?: string | Date
 }
 
-export interface ProductVariantOptionDTO {
-  /**
-   * The ID of the product variant option.
-   */
-  id: string
-  /**
-   * The value of the product variant option.
-   *
-   * @expandable
-   */
-  option_value?: ProductOptionValueDTO | null
-  /**
-   * The value of the product variant option id.
-   */
-  option_value_id?: string | null
-  /**
-   * The associated product variant.
-   *
-   * @expandable
-   */
-  variant?: ProductVariantDTO | null
-  /**
-   * The associated product variant id.
-   */
-  variant_id?: string | null
-  /**
-   * When the product variant option was created.
-   */
-  created_at: string | Date
-  /**
-   * When the product variant option was updated.
-   */
-  updated_at: string | Date
-  /**
-   * When the product variant option was deleted.
-   */
-  deleted_at?: string | Date
-}
-
 /**
  * @interface
  *

packages/utils/src/dal/mikro-orm/db-error-mapper.ts

@@ -13,7 +13,10 @@ export const dbErrorMapper = (err: Error) => {
     throw new MedusaError(MedusaError.Types.NOT_FOUND, err.message)
   }
 
-  if (err instanceof UniqueConstraintViolationException) {
+  if (
+    err instanceof UniqueConstraintViolationException ||
+    (err as any).code === "23505"
+  ) {
     const info = getConstraintInfo(err)
     if (!info) {
       throw err
@@ -27,7 +30,10 @@ export const dbErrorMapper = (err: Error) => {
     )
   }
 
-  if (err instanceof NotNullConstraintViolationException) {
+  if (
+    err instanceof NotNullConstraintViolationException ||
+    (err as any).code === "23502"
+  ) {
     throw new MedusaError(
       MedusaError.Types.INVALID_DATA,
       `Cannot set field '${(err as any).column}' of ${upperCaseFirst(
@@ -36,7 +42,10 @@ export const dbErrorMapper = (err: Error) => {
     )
   }
 
-  if (err instanceof InvalidFieldNameException) {
+  if (
+    err instanceof InvalidFieldNameException ||
+    (err as any).code === "42703"
+  ) {
     const userFriendlyMessage = err.message.match(/(column.*)/)?.[0]
     throw new MedusaError(
       MedusaError.Types.INVALID_DATA,
@@ -44,7 +53,10 @@ export const dbErrorMapper = (err: Error) => {
     )
   }
 
-  if (err instanceof ForeignKeyConstraintViolationException) {
+  if (
+    err instanceof ForeignKeyConstraintViolationException ||
+    (err as any).code === "23503"
+  ) {
     const info = getConstraintInfo(err)
     throw new MedusaError(
       MedusaError.Types.NOT_FOUND,

packages/utils/src/dal/mikro-orm/integration-tests/__tests__/mikro-orm-repository.spec.ts

@@ -656,8 +656,7 @@ describe("mikroOrmRepository", () => {
       expect(listedEntities[0].entity3.getItems()).toEqual(
         expect.arrayContaining([
           expect.objectContaining({
-            // title: "en3-1",
+            title: "en3-1",
-            title: "newen3-1",
           }),
           expect.objectContaining({
             title: "en3-4",
@@ -744,8 +743,7 @@ describe("mikroOrmRepository", () => {
       expect(listedEntities[0].entity3.getItems()).toEqual(
         expect.arrayContaining([
           expect.objectContaining({
-            // title: "en3-1",
+            title: "en3-1",
-            title: "newen3-1",
           }),
           expect.objectContaining({
             title: "en3-4",
@@ -817,31 +815,34 @@ describe("mikroOrmRepository", () => {
       )
     })
 
-    // it("should correctly handle many-to-many upserts with a uniqueness constriant on a non-primary key", async () => {
+    it("should correctly handle many-to-many upserts with a uniqueness constriant on a non-primary key", async () => {
-    //   const entity1 = {
+      const entity1 = {
-    //     id: "1",
+        id: "1",
-    //     title: "en1",
+        title: "en1",
-    //     entity3: [{ title: "en3-1" }, { title: "en3-2" }] as any,
+        entity3: [
-    //   }
+          { id: "4", title: "en3-1" },
+          { id: "5", title: "en3-2" },
+        ] as any,
+      }
 
-    //   await manager1().upsertWithReplace([entity1], {
+      await manager1().upsertWithReplace([entity1], {
-    //     relations: ["entity3"],
+        relations: ["entity3"],
-    //   })
+      })
 
-    //   await manager1().upsertWithReplace([{ ...entity1, id: "2" }], {
+      await manager1().upsertWithReplace([{ ...entity1, id: "2" }], {
-    //     relations: ["entity3"],
+        relations: ["entity3"],
-    //   })
+      })
 
-    //   const listedEntities = await manager1().find({
+      const listedEntities = await manager1().find({
-    //     where: {},
+        where: {},
-    //     options: { populate: ["entity3"] },
+        options: { populate: ["entity3"] },
-    //   })
+      })
 
-    //   expect(listedEntities).toHaveLength(2)
+      expect(listedEntities).toHaveLength(2)
-    //   expect(listedEntities[0].entity3.getItems()).toEqual(
+      expect(listedEntities[0].entity3.getItems()).toEqual(
-    //     listedEntities[1].entity3.getItems()
+        listedEntities[1].entity3.getItems()
-    //   )
+      )
-    // })
+    })
   })
 
   describe("error mapping", () => {

packages/utils/src/dal/mikro-orm/mikro-orm-repository.ts

@@ -602,8 +602,7 @@ export function mikroOrmBaseRepositoryFactory<T extends object = object>(
           return normalizedData
         }
 
-        // TODO: Currently we will also do an update of the data on the other side of the many-to-many relationship. Reevaluate if we should avoid that.
+        await this.upsertMany_(manager, relation.type, normalizedData, true)
-        await this.upsertMany_(manager, relation.type, normalizedData)
 
         const pivotData = normalizedData.map((currModel) => {
           return {
@@ -721,39 +720,17 @@ export function mikroOrmBaseRepositoryFactory<T extends object = object>(
       return { id: (created as any).id, ...data }
     }
 
-    protected async upsertManyToOneRelations_(
-      manager: SqlEntityManager,
-      upsertsPerType: Record<string, any[]>
-    ) {
-      const typesToUpsert = Object.entries(upsertsPerType)
-      if (!typesToUpsert.length) {
-        return []
-      }
-
-      return (
-        await promiseAll(
-          typesToUpsert.map(([type, data]) => {
-            return this.upsertMany_(manager, type, data)
-          })
-        )
-      ).flat()
-    }
-
     protected async upsertMany_(
       manager: SqlEntityManager,
       entityName: string,
-      entries: any[]
+      entries: any[],
+      skipUpdate: boolean = false
     ) {
-      const existingEntities: any[] = await manager.find(
+      const selectQb = manager.qb(entityName)
-        entityName,
+      const existingEntities: any[] = await selectQb.select("*").where({
-        {
+        id: { $in: entries.map((d) => d.id) },
-          id: { $in: entries.map((d) => d.id) },
+      })
-        },
+
-        {
-          populate: [],
-          disableIdentityMap: true,
-        }
-      )
       const existingEntitiesMap = new Map(
         existingEntities.map((e) => [e.id, e])
       )
@@ -762,12 +739,21 @@ export function mikroOrmBaseRepositoryFactory<T extends object = object>(
 
       await promiseAll(
         entries.map(async (data) => {
-          orderedEntities.push(data)
           const existingEntity = existingEntitiesMap.get(data.id)
+          orderedEntities.push(data)
           if (existingEntity) {
+            if (skipUpdate) {
+              return
+            }
             await manager.nativeUpdate(entityName, { id: data.id }, data)
           } else {
-            await manager.insert(entityName, data)
+            const qb = manager.qb(entityName)
+            if (skipUpdate) {
+              await qb.insert(data).onConflict().ignore().execute()
+            } else {
+              await manager.insert(entityName, data)
+              // await manager.insert(entityName, data)
+            }
           }
         })
       )