fix: Fixes to product module and improving tests (#6898)

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

@@ -11,7 +11,6 @@ const {
 const { PriceListStatus, PriceListType } = require("@medusajs/types")
 const { ContainerRegistrationKeys } = require("@medusajs/utils")
 
-let priceListSeeder = undefined
 let {
   ProductOptionValue,
   MoneyAmount,
@@ -96,7 +95,6 @@ medusaIntegrationTestRunner({
 
     beforeAll(() => {
       // Note: We have to lazily load everything because there are weird ordering issues when doing `require` of `@medusajs/medusa`
-      priceListSeeder = require("../../../helpers/price-list-seeder")
       ;({
         ProductOptionValue,
         MoneyAmount,
@@ -1958,10 +1956,9 @@ medusaIntegrationTestRunner({
         })
       })
 
-      // TODO: Enable variant price updates
+      describe("updates a variant's default prices (ignores prices associated with a Price List)", () => {
-      describe.skip("updates a variant's default prices (ignores prices associated with a Price List)", () => {
         beforeEach(async () => {
-          await priceListSeeder(dbConnection)
+          // await priceListSeeder(dbConnection)
           await simpleSalesChannelFactory(dbConnection, {
             name: "Default channel",
             id: "default-channel",
@@ -1979,23 +1976,24 @@ medusaIntegrationTestRunner({
             ],
           }
 
-          const response = await api
+          const response = await api.post(
-            .post(
+            `/admin/products/${baseProduct.id}/variants/${baseProduct.variants[0].id}`,
-              "/admin/products/test-product/variants/test-variant",
             data,
             adminHeaders
           )
-            .catch((err) => {
-              console.log(err)
-            })
 
+          expect(
+            baseProduct.variants[0].prices.find(
+              (p) => p.currency_code === "usd"
+            ).amount
+          ).toEqual(100)
           expect(response.status).toEqual(200)
           expect(response.data).toEqual({
             product: expect.objectContaining({
-              id: "test-product",
+              id: baseProduct.id,
               variants: expect.arrayContaining([
                 expect.objectContaining({
-                  id: "test-variant",
+                  id: baseProduct.variants[0].id,
                   prices: expect.arrayContaining([
                     expect.objectContaining({
                       amount: 1500,
@@ -2008,7 +2006,8 @@ medusaIntegrationTestRunner({
           })
         })
 
-        it("successfully updates a variant's price by changing an existing price (given a region_id)", async () => {
+        // TODO: Do we want to add support for region prices through the product APIs?
+        it.skip("successfully updates a variant's price by changing an existing price (given a region_id)", async () => {
           const data = {
             prices: [
               {
@@ -2049,14 +2048,16 @@ medusaIntegrationTestRunner({
         })
 
         it("successfully updates a variant's prices by adding a new price", async () => {
+          const usdPrice = baseProduct.variants[0].prices.find(
+            (p) => p.currency_code === "usd"
+          )
           const data = {
             title: "Test variant prices",
             prices: [
-              // usd price coming from the product seeder
               {
-                id: "test-price",
+                id: usdPrice.id,
-                amount: 100,
                 currency_code: "usd",
+                amount: 100,
               },
               {
                 currency_code: "eur",
@@ -2065,30 +2066,26 @@ medusaIntegrationTestRunner({
             ],
           }
 
-          const response = await api
+          const response = await api.post(
-            .post(
+            `/admin/products/${baseProduct.id}/variants/${baseProduct.variants[0].id}`,
-              "/admin/products/test-product/variants/test-variant",
             data,
             adminHeaders
           )
-            .catch((err) => {
-              console.log(err)
-            })
 
           expect(response.status).toEqual(200)
 
           expect(response.data).toEqual(
             expect.objectContaining({
               product: expect.objectContaining({
-                id: "test-product",
+                id: baseProduct.id,
                 variants: expect.arrayContaining([
                   expect.objectContaining({
-                    id: "test-variant",
+                    id: baseProduct.variants[0].id,
                     prices: expect.arrayContaining([
                       expect.objectContaining({
                         amount: 100,
                         currency_code: "usd",
-                        id: "test-price",
+                        id: usdPrice.id,
                       }),
                       expect.objectContaining({
                         amount: 4500,
@@ -2102,42 +2099,6 @@ medusaIntegrationTestRunner({
           )
         })
 
-        it("successfully updates a variant's prices by replacing a price", async () => {
-          const variantId = "test-variant"
-          const data = {
-            prices: [
-              {
-                currency_code: "usd",
-                amount: 4500,
-              },
-            ],
-          }
-
-          const response = await api
-            .post(
-              `/admin/products/test-product/variants/${variantId}`,
-              data,
-              adminHeaders
-            )
-            .catch((err) => {
-              console.log(err)
-            })
-
-          expect(response.status).toEqual(200)
-          const variant = response.data.product.variants.find(
-            (v) => v.id === variantId
-          )
-          expect(variant.prices.length).toEqual(1)
-          expect(variant.prices).toEqual(
-            expect.arrayContaining([
-              expect.objectContaining({
-                amount: 4500,
-                currency_code: "usd",
-              }),
-            ])
-          )
-        })
-
         it("successfully updates a variant's prices by deleting a price and adding another price", async () => {
           const data = {
             prices: [
@@ -2152,10 +2113,9 @@ medusaIntegrationTestRunner({
             ],
           }
 
-          const variantId = "test-variant"
           const response = await api
             .post(
-              `/admin/products/test-product/variants/${variantId}`,
+              `/admin/products/${baseProduct.id}/variants/${baseProduct.variants[0].id}`,
               data,
               adminHeaders
             )
@@ -2165,9 +2125,7 @@ medusaIntegrationTestRunner({
 
           expect(response.status).toEqual(200)
 
-          const variant = response.data.product.variants.find(
+          const variant = response.data.product.variants[0]
-            (v) => v.id === variantId
-          )
           expect(variant.prices.length).toEqual(2)
 
           expect(variant.prices).toEqual(
@@ -2184,7 +2142,8 @@ medusaIntegrationTestRunner({
           )
         })
 
-        it("successfully updates a variant's prices by updating an existing price (using region_id) and adding another price", async () => {
+        // TODO: Similarly we need to decide how to handle regions
+        it.skip("successfully updates a variant's prices by updating an existing price (using region_id) and adding another price", async () => {
           const data = {
             prices: [
               {
@@ -2231,7 +2190,8 @@ medusaIntegrationTestRunner({
           )
         })
 
-        it("successfully deletes a region price", async () => {
+        // TODO: Similarly we need to decide how to handle regions
+        it.skip("successfully deletes a region price", async () => {
           const createRegionPricePayload = {
             prices: [
               {
@@ -2322,7 +2282,8 @@ medusaIntegrationTestRunner({
           )
         })
 
-        it("successfully updates a variants prices by deleting both a currency and region price", async () => {
+        // TODO: Similarly we need to decide how to handle regions
+        it.skip("successfully updates a variants prices by deleting both a currency and region price", async () => {
           // await Promise.all(
           //   ["reg_1", "reg_2", "reg_3"].map(async (regionId) => {
           //     return await simpleRegionFactory(dbConnection, {
@@ -2585,102 +2546,6 @@ medusaIntegrationTestRunner({
           expect(variantPost).not.toBeTruthy()
         })
 
-        // TODO: This one is a bit more complex, leaving for later
-        it.skip("successfully deletes a product variant and its associated option values", async () => {
-          // Validate that the option value exists
-          const optValPre = await dbConnection.manager.findOne(
-            ProductOptionValue,
-            {
-              where: { variant_id: "test-variant_2" },
-            }
-          )
-
-          expect(optValPre).toBeTruthy()
-
-          // Soft delete the variant
-          const response = await api.delete(
-            "/admin/products/test-product/variants/test-variant_2",
-            adminHeaders
-          )
-
-          expect(response.status).toEqual(200)
-
-          // Validate that the option value was deleted
-          const optValPost = await dbConnection.manager.findOne(
-            ProductOptionValue,
-            { where: { variant_id: "test-variant_2" } }
-          )
-
-          expect(optValPost).not.toBeTruthy()
-
-          // Validate that the option still exists in the DB with deleted_at
-          const optValDeleted = await dbConnection.manager.findOne(
-            ProductOptionValue,
-            {
-              where: {
-                variant_id: "test-variant_2",
-              },
-              withDeleted: true,
-            }
-          )
-
-          expect(optValDeleted).toEqual(
-            expect.objectContaining({
-              deleted_at: expect.any(Date),
-              variant_id: "test-variant_2",
-            })
-          )
-        })
-
-        // TODO: This will need a bit more rework
-        it.skip("successfully deletes a product and any option value associated with one of its variants", async () => {
-          // Validate that the option value exists
-          const optValPre = await dbConnection.manager.findOne(
-            ProductOptionValue,
-            {
-              where: { variant_id: "test-variant_2" },
-            }
-          )
-
-          expect(optValPre).toBeTruthy()
-
-          // Soft delete the product
-          const response = await api.delete(
-            "/admin/products/test-product",
-            adminHeaders
-          )
-
-          expect(response.status).toEqual(200)
-
-          // Validate that the option value has been deleted
-          const optValPost = await dbConnection.manager.findOne(
-            ProductOptionValue,
-            {
-              where: { variant_id: "test-variant_2" },
-            }
-          )
-
-          expect(optValPost).not.toBeTruthy()
-
-          // Validate that the option still exists in the DB with deleted_at
-          const optValDeleted = await dbConnection.manager.findOne(
-            ProductOptionValue,
-            {
-              where: {
-                variant_id: "test-variant_2",
-              },
-              withDeleted: true,
-            }
-          )
-
-          expect(optValDeleted).toEqual(
-            expect.objectContaining({
-              deleted_at: expect.any(Date),
-              variant_id: "test-variant_2",
-            })
-          )
-        })
-
         it.skip("successfully deletes a product variant and its associated prices", async () => {
           // Validate that the price exists
           const pricePre = await dbConnection.manager.findOne(MoneyAmount, {
@@ -2769,25 +2634,18 @@ medusaIntegrationTestRunner({
         it.skip("successfully creates product with soft-deleted product handle and deletes it again", async () => {
           // First we soft-delete the product
           const response = await api
-            .delete("/admin/products/test-product", adminHeaders)
+            .delete(`/admin/products/${baseProduct.id}`, adminHeaders)
             .catch((err) => {
               console.log(err)
             })
 
           expect(response.status).toEqual(200)
-          expect(response.data.id).toEqual("test-product")
+          expect(response.data.id).toEqual(baseProduct.id)
 
           // Lets try to create a product with same handle as deleted one
           const payload = {
-            title: "Test product",
+            title: baseProduct.title,
-            handle: "test-product",
+            handle: baseProduct.handle,
-            description: "test-product-description",
-            images: breaking(
-              () => ["test-image.png", "test-image-2.png"],
-              () => [{ url: "test-image.png" }, { url: "test-image-2.png" }]
-            ),
-            collection_id: "test-collection",
-            tags: [{ value: "123" }, { value: "456" }],
             variants: [
               {
                 title: "Test variant",
@@ -2800,31 +2658,26 @@ medusaIntegrationTestRunner({
           const res = await api.post("/admin/products", payload, adminHeaders)
 
           expect(res.status).toEqual(200)
-          expect(res.data.product.handle).toEqual("test-product")
+          expect(res.data.product.handle).toEqual(baseProduct.handle)
 
           // Delete product again to ensure uniqueness is enforced in all cases
           const response2 = await api
-            .delete("/admin/products/test-product", adminHeaders)
+            .delete(`/admin/products/${res.data.product.id}`, adminHeaders)
             .catch((err) => {
               console.log(err)
             })
 
           expect(response2.status).toEqual(200)
-          expect(response2.data.id).toEqual("test-product")
+          expect(response2.data.id).toEqual(res.data.product.id)
         })
 
+        // TODO: We just need to return the correct error message
         it.skip("should fail when creating a product with a handle that already exists", async () => {
           // Lets try to create a product with same handle as deleted one
           const payload = {
-            title: "Test product",
+            title: baseProduct.title,
-            handle: "test-product",
+            handle: baseProduct.handle,
             description: "test-product-description",
-            images: breaking(
-              () => ["test-image.png", "test-image-2.png"],
-              () => [{ url: "test-image.png" }, { url: "test-image-2.png" }]
-            ),
-            collection_id: "test-collection",
-            tags: [{ value: "123" }, { value: "456" }],
             variants: [
               {
                 title: "Test variant",
@@ -2857,19 +2710,17 @@ medusaIntegrationTestRunner({
 
         // TODO: This needs to be fixed, it returns 422 now.
         it.skip("successfully creates soft-deleted product collection", async () => {
-          const response = await api
+          const response = await api.delete(
-            .delete("/admin/collections/test-collection", adminHeaders)
+            `/admin/collections/${baseCollection.id}`,
-            .catch((err) => {
+            adminHeaders
-              console.log(err)
+          )
-            })
-
           expect(response.status).toEqual(200)
-          expect(response.data.id).toEqual("test-collection")
+          expect(response.data.id).toEqual(baseCollection.id)
 
           // Lets try to create a product collection with same handle as deleted one
           const payload = {
             title: "Another test collection",
-            handle: "test-collection",
+            handle: baseCollection.handle,
           }
 
           const res = await api.post(
@@ -2879,7 +2730,7 @@ medusaIntegrationTestRunner({
           )
 
           expect(res.status).toEqual(200)
-          expect(res.data.collection.handle).toEqual("test-collection")
+          expect(res.data.collection.handle).toEqual(baseCollection.handle)
         })
 
         it("should fail when creating a collection with a handle that already exists", async () => {
@@ -2900,20 +2751,11 @@ medusaIntegrationTestRunner({
 
         // TODO: This needs to be fixed
         it.skip("successfully creates soft-deleted product variant", async () => {
-          await api
+          const variant = baseProduct.variants[0]
-            .get("/admin/products/test-product", adminHeaders)
+          const response = await api.delete(
-            .catch((err) => {
+            `/admin/products/${baseProduct.id}/variants/${variant.id}`,
-              console.log(err)
-            })
-
-          const response = await api
-            .delete(
-              "/admin/products/test-product/variants/test-variant",
             adminHeaders
           )
-            .catch((err) => {
-              console.log(err)
-            })
 
           expect(response.status).toEqual(200)
           expect(
@@ -2921,14 +2763,14 @@ medusaIntegrationTestRunner({
               () => response.data.variant_id,
               () => response.data.id
             )
-          ).toEqual("test-variant")
+          ).toEqual(baseProduct.variants[0].id)
 
           const payload = {
             title: "Second variant",
-            sku: "test-sku",
+            sku: variant.sku,
-            ean: "test-ean",
+            ean: variant.ean,
-            upc: "test-upc",
+            upc: variant.upc,
-            barcode: "test-barcode",
+            barcode: variant.barcode,
             prices: [
               {
                 currency_code: "usd",
@@ -2937,23 +2779,21 @@ medusaIntegrationTestRunner({
             ],
           }
 
-          const res = await api
+          const res = await api.post(
-            .post(
+            `/admin/products/${baseProduct.id}/variants`,
-              "/admin/products/test-product/variants",
             payload,
             adminHeaders
           )
-            .catch((err) => console.log(err))
 
           expect(res.status).toEqual(200)
           expect(res.data.product.variants).toEqual(
             expect.arrayContaining([
               expect.objectContaining({
                 title: "Second variant",
-                sku: "test-sku",
+                sku: variant.sku,
-                ean: "test-ean",
+                ean: variant.ean,
-                upc: "test-upc",
+                upc: variant.upc,
-                barcode: "test-barcode",
+                barcode: variant.barcode,
               }),
             ])
           )

packages/core-flows/src/product/workflows/update-product-variants.ts

@@ -58,7 +58,7 @@ export const updateProductVariantsWorkflow = createWorkflow(
 
         return {
           selector: {
-            ids: data.variantPriceSetLinks.map((link) => link.price_set_id),
+            id: data.variantPriceSetLinks.map((link) => link.price_set_id),
           } as PricingTypes.FilterablePriceSetProps,
           update: {
             prices: data.input.update.prices,
@@ -78,11 +78,11 @@ export const updateProductVariantsWorkflow = createWorkflow(
       },
       (data) => {
         return data.updatedVariants.map((variant, i) => {
-          const linkForVariant = data.variantPriceSetLinks.find(
+          const linkForVariant = data.variantPriceSetLinks?.find(
             (link) => link.variant_id === variant.id
           )
 
-          const priceSetForVariant = data.updatedPriceSets.find(
+          const priceSetForVariant = data.updatedPriceSets?.find(
             (priceSet) => priceSet.id === linkForVariant?.price_set_id
           )
 

packages/core-flows/src/product/workflows/update-products.ts

@@ -15,6 +15,7 @@ export const updateProductsWorkflow = createWorkflow(
   (
     input: WorkflowData<WorkflowInput>
   ): WorkflowData<ProductTypes.ProductDTO[]> => {
+    // TODO: Delete price sets for removed variants
     return updateProductsStep(input)
   }
 )

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

@@ -19,35 +19,38 @@ moduleIntegrationTestRunner({
       let productTwo: Product
 
       beforeEach(async () => {
-        const testManager = await MikroOrmWrapper.forkManager()
+        productOne = await service.create({
-
-        productOne = testManager.create(Product, {
           id: "product-1",
           title: "product 1",
           status: ProductTypes.ProductStatus.PUBLISHED,
+          options: [
+            {
+              title: "size",
+              values: ["large"],
+            },
+          ],
         })
 
-        productTwo = testManager.create(Product, {
+        productTwo = await service.create({
           id: "product-2",
           title: "product 2",
           status: ProductTypes.ProductStatus.PUBLISHED,
         })
 
-        variantOne = testManager.create(ProductVariant, {
+        variantOne = await service.createVariants({
           id: "test-1",
           title: "variant 1",
           inventory_quantity: 10,
-          product: productOne,
+          product_id: productOne.id,
+          options: { size: "large" },
         })
 
-        variantTwo = testManager.create(ProductVariant, {
+        variantTwo = await service.createVariants({
           id: "test-2",
           title: "variant",
           inventory_quantity: 10,
-          product: productTwo,
+          product_id: productTwo.id,
         })
-
-        await testManager.persistAndFlush([variantOne, variantTwo])
       })
 
       describe("listAndCountVariants", () => {
@@ -165,6 +168,35 @@ moduleIntegrationTestRunner({
           )
         })
       })
+
+      describe("softDelete variant", () => {
+        it("should soft delete a variant and its relations", async () => {
+          const beforeDeletedVariants = await service.listVariants(
+            { id: variantOne.id },
+            {
+              relations: ["options", "options.option_value", "options.variant"],
+            }
+          )
+
+          await service.softDeleteVariants([variantOne.id])
+          const deletedVariants = await service.listVariants(
+            { id: variantOne.id },
+            {
+              relations: ["options", "options.option_value", "options.variant"],
+              withDeleted: true,
+            }
+          )
+
+          expect(deletedVariants).toHaveLength(1)
+          expect(deletedVariants[0].deleted_at).not.toBeNull()
+
+          for (const variantOption of deletedVariants[0].options) {
+            expect(variantOption.deleted_at).not.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

@@ -738,7 +738,6 @@ moduleIntegrationTestRunner({
               relations: [
                 "variants",
                 "variants.options",
-                "variants.options",
                 "options",
                 "options.values",
               ],

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

@@ -246,6 +246,7 @@ export default class ProductModuleService<
       },
       {
         take: null,
+        relations: ["values"],
       },
       sharedContext
     )

packages/types/src/product/common.ts

@@ -579,6 +579,18 @@ export interface ProductVariantOptionDTO {
    * 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
 }
 
 /**

packages/types/src/product/service.ts

@@ -259,10 +259,7 @@ export interface IProductModuleService extends IModuleService {
    *   title: "Shirt",
    * })
    */
-  upsert(
+  upsert(data: UpsertProductDTO, sharedContext?: Context): Promise<ProductDTO>
-    data: UpsertProductDTO[],
-    sharedContext?: Context
-  ): Promise<ProductDTO[]>
 
   /**
    * This method is used to update a product.