fix(medusa): Ensure no orphan product option values (#2408)

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

@@ -16,6 +16,12 @@ const { simpleProductFactory } = require("../../factories")
 
 jest.setTimeout(50000)
 
+const adminHeaders = {
+  headers: {
+    Authorization: "Bearer test_token",
+  },
+}
+
 describe("/admin/products", () => {
   let medusaProcess
   let dbConnection
@@ -48,11 +54,7 @@ describe("/admin/products", () => {
       const api = useApi()
 
       const res = await api
-        .get("/admin/products", {
+        .get("/admin/products", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -76,11 +78,7 @@ describe("/admin/products", () => {
       const api = useApi()
 
       const res = await api
-        .get("/admin/products?q=", {
+        .get("/admin/products?q=", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -109,21 +107,13 @@ describe("/admin/products", () => {
 
       // update test-product status to proposed
       await api
-        .post("/admin/products/test-product", payload, {
+        .post("/admin/products/test-product", payload, adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
 
       const response = await api
-        .get("/admin/products?status[]=proposed", {
+        .get("/admin/products?status[]=proposed", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -151,11 +141,7 @@ describe("/admin/products", () => {
       ]
 
       const response = await api
-        .get("/admin/products?status[]=published,proposed", {
+        .get("/admin/products?status[]=published,proposed", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -193,11 +179,10 @@ describe("/admin/products", () => {
       ]
 
       const response = await api
-        .get("/admin/products?status[]=published,proposed&expand=tags", {
+        .get(
-          headers: {
+          "/admin/products?status[]=published,proposed&expand=tags",
-            Authorization: "Bearer test_token",
+          adminHeaders
-          },
+        )
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -228,11 +213,7 @@ describe("/admin/products", () => {
       const api = useApi()
 
       const response = await api
-        .get("/admin/products?deleted_at[gt]=01-26-1990&q=test", {
+        .get("/admin/products?deleted_at[gt]=01-26-1990&q=test", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -253,11 +234,7 @@ describe("/admin/products", () => {
       const api = useApi()
 
       const response = await api
-        .get("/admin/products?q=t&limit=2", {
+        .get("/admin/products?q=t&limit=2", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -270,11 +247,7 @@ describe("/admin/products", () => {
       const api = useApi()
 
       const response = await api
-        .get("/admin/products?q=test+product1", {
+        .get("/admin/products?q=test+product1", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -300,11 +273,7 @@ describe("/admin/products", () => {
       const api = useApi()
 
       const response = await api
-        .get("/admin/products?q=t&offset=1", {
+        .get("/admin/products?q=t&offset=1", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -317,11 +286,7 @@ describe("/admin/products", () => {
       const api = useApi()
 
       const response = await api
-        .get("/admin/products?deleted_at[gt]=01-26-1990", {
+        .get("/admin/products?deleted_at[gt]=01-26-1990", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -346,11 +311,7 @@ describe("/admin/products", () => {
       ]
 
       const response = await api
-        .get("/admin/products?collection_id[]=test-collection1", {
+        .get("/admin/products?collection_id[]=test-collection1", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -387,11 +348,7 @@ describe("/admin/products", () => {
       ]
 
       const response = await api
-        .get("/admin/products?tags[]=tag3", {
+        .get("/admin/products?tags[]=tag3", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -432,11 +389,10 @@ describe("/admin/products", () => {
       ]
 
       const response = await api
-        .get("/admin/products?collection_id[]=test-collection1&tags[]=tag4", {
+        .get(
-          headers: {
+          "/admin/products?collection_id[]=test-collection1&tags[]=tag4",
-            Authorization: "Bearer test_token",
+          adminHeaders
-          },
+        )
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -483,22 +439,12 @@ describe("/admin/products", () => {
         ],
       }
 
-      await api
+      await api.post("/admin/products", payload, adminHeaders).catch((err) => {
-        .post("/admin/products", payload, {
+        console.log(err)
-          headers: {
+      })
-            Authorization: "Bearer test_token",
-          },
-        })
-        .catch((err) => {
-          console.log(err)
-        })
 
       const response = await api
-        .get("/admin/products?is_giftcard=true", {
+        .get("/admin/products?is_giftcard=true", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -578,22 +524,12 @@ describe("/admin/products", () => {
         ],
       }
 
-      await api
+      await api.post("/admin/products", payload, adminHeaders).catch((err) => {
-        .post("/admin/products", payload, {
+        console.log(err)
-          headers: {
+      })
-            Authorization: "Bearer test_token",
-          },
-        })
-        .catch((err) => {
-          console.log(err)
-        })
 
       const response = await api
-        .get("/admin/products?is_giftcard=false", {
+        .get("/admin/products?is_giftcard=false", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -609,11 +545,7 @@ describe("/admin/products", () => {
       const api = useApi()
 
       const response = await api
-        .get("/admin/products", {
+        .get("/admin/products", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -910,11 +842,7 @@ describe("/admin/products", () => {
       }
 
       const response = await api
-        .post("/admin/products", payload, {
+        .post("/admin/products", payload, adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -1065,11 +993,7 @@ describe("/admin/products", () => {
       }
 
       const response = await api
-        .post("/admin/products", payload, {
+        .post("/admin/products", payload, adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -1110,11 +1034,7 @@ describe("/admin/products", () => {
       }
 
       const creationResponse = await api
-        .post("/admin/products", payload, {
+        .post("/admin/products", payload, adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -1124,11 +1044,7 @@ describe("/admin/products", () => {
       const productId = creationResponse.data.product.id
 
       const response = await api
-        .get(`/admin/products/${productId}`, {
+        .get(`/admin/products/${productId}`, adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -1166,11 +1082,7 @@ describe("/admin/products", () => {
       }
 
       const response = await api
-        .post("/admin/products", payload, {
+        .post("/admin/products", payload, adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -1208,11 +1120,7 @@ describe("/admin/products", () => {
       }
 
       const response = await api
-        .post("/admin/products/test-product", payload, {
+        .post("/admin/products/test-product", payload, adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -1315,11 +1223,7 @@ describe("/admin/products", () => {
       }
 
       const response = await api
-        .post("/admin/products/test-product", payload, {
+        .post("/admin/products/test-product", payload, adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -1337,11 +1241,7 @@ describe("/admin/products", () => {
       }
 
       try {
-        await api.post("/admin/products/test-product", payload, {
+        await api.post("/admin/products/test-product", payload, adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
       } catch (e) {
         expect(e.response.status).toEqual(400)
         expect(e.response.data.type).toEqual("invalid_data")
@@ -1368,11 +1268,7 @@ describe("/admin/products", () => {
       }
 
       const response = await api
-        .post("/admin/products/test-product", payload, {
+        .post("/admin/products/test-product", payload, adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -1410,11 +1306,7 @@ describe("/admin/products", () => {
       }
 
       const response = await api
-        .post("/admin/products/test-product/options", payload, {
+        .post("/admin/products/test-product/options", payload, adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -1436,22 +1328,32 @@ describe("/admin/products", () => {
 
   describe("DELETE /admin/products/:id/options/:option_id", () => {
     beforeEach(async () => {
-      try {
+      await simpleProductFactory(dbConnection, {
-        await simpleProductFactory(dbConnection, {
+        id: "test-product-without-variants",
-          id: "test-product-without-variants",
+        variants: [],
-          variants: [],
+        options: [
-          options: [
+          {
-            {
+            id: "test-product-option",
-              id: "test-product-option",
+            title: "Test option",
-              title: "Test option",
+          },
-            },
+        ],
-          ],
+      })
-        })
+      await simpleProductFactory(dbConnection, {
-        await adminSeeder(dbConnection)
+        id: "test-product-with-variant",
-      } catch (err) {
+        variants: [
-        console.log(err)
+          {
-        throw err
+            product_id: "test-product-with-variant",
-      }
+            options: [{ option_id: "test-product-option-1", value: "test" }],
+          },
+        ],
+        options: [
+          {
+            id: "test-product-option-1",
+            title: "Test option 1",
+          },
+        ],
+      })
+      await adminSeeder(dbConnection)
     })
 
     afterEach(async () => {
@@ -1465,11 +1367,7 @@ describe("/admin/products", () => {
       const response = await api
         .delete(
           "/admin/products/test-product-without-variants/options/test-product-option",
-          {
+          adminHeaders
-            headers: {
-              Authorization: "Bearer test_token",
-            },
-          }
         )
         .catch((err) => {
           console.log(err)
@@ -1484,6 +1382,24 @@ describe("/admin/products", () => {
         })
       )
     })
+
+    it("deletes a values associated with deleted option", async () => {
+      const api = useApi()
+
+      const response = await api.delete(
+        "/admin/products/test-product-with-variant/options/test-product-option-1",
+        adminHeaders
+      )
+
+      const values = await dbConnection.manager.find(ProductOptionValue, {
+        where: { option_id: "test-product-option-1" },
+        withDeleted: true,
+      })
+
+      expect(values).toEqual([
+        expect.objectContaining({ deleted_at: expect.any(Date) }),
+      ])
+    })
   })
 
   describe("GET /admin/products/:id/variants", () => {
@@ -1501,11 +1417,7 @@ describe("/admin/products", () => {
       const api = useApi()
 
       const res = await api
-        .get("/admin/products/test-product/variants", {
+        .get("/admin/products/test-product/variants", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -1559,11 +1471,11 @@ describe("/admin/products", () => {
       }
 
       const response = await api
-        .post("/admin/products/test-product/variants/test-variant", data, {
+        .post(
-          headers: {
+          "/admin/products/test-product/variants/test-variant",
-            Authorization: "Bearer test_token",
+          data,
-          },
+          adminHeaders
-        })
+        )
         .catch((err) => {
           console.log(err)
         })
@@ -1599,11 +1511,11 @@ describe("/admin/products", () => {
       }
 
       const response = await api
-        .post("/admin/products/test-product1/variants/test-variant_3", data, {
+        .post(
-          headers: {
+          "/admin/products/test-product1/variants/test-variant_3",
-            Authorization: "Bearer test_token",
+          data,
-          },
+          adminHeaders
-        })
+        )
         .catch((err) => {
           console.log(err)
         })
@@ -1647,11 +1559,11 @@ describe("/admin/products", () => {
       }
 
       const response = await api
-        .post("/admin/products/test-product/variants/test-variant", data, {
+        .post(
-          headers: {
+          "/admin/products/test-product/variants/test-variant",
-            Authorization: "Bearer test_token",
+          data,
-          },
+          adminHeaders
-        })
+        )
         .catch((err) => {
           console.log(err)
         })
@@ -1694,11 +1606,11 @@ describe("/admin/products", () => {
       }
 
       const response = await api
-        .post("/admin/products/test-product/variants/test-variant", data, {
+        .post(
-          headers: {
+          "/admin/products/test-product/variants/test-variant",
-            Authorization: "Bearer test_token",
+          data,
-          },
+          adminHeaders
-        })
+        )
         .catch((err) => {
           console.log(err)
         })
@@ -1732,11 +1644,11 @@ describe("/admin/products", () => {
       }
 
       const response = await api
-        .post("/admin/products/test-product/variants/test-variant", data, {
+        .post(
-          headers: {
+          "/admin/products/test-product/variants/test-variant",
-            Authorization: "Bearer test_token",
+          data,
-          },
+          adminHeaders
-        })
+        )
         .catch((err) => {
           console.log(err)
         })
@@ -1775,11 +1687,11 @@ describe("/admin/products", () => {
       }
 
       const response = await api
-        .post("/admin/products/test-product1/variants/test-variant_3", data, {
+        .post(
-          headers: {
+          "/admin/products/test-product1/variants/test-variant_3",
-            Authorization: "Bearer test_token",
+          data,
-          },
+          adminHeaders
-        })
+        )
         .catch((err) => {
           console.log(err)
         })
@@ -1845,11 +1757,7 @@ describe("/admin/products", () => {
       }
 
       const res = await api
-        .post("/admin/products/test-product/variants", payload, {
+        .post("/admin/products/test-product/variants", payload, adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => console.log(err))
 
       const insertedVariant = res.data.product.variants.find(
@@ -1895,11 +1803,7 @@ describe("/admin/products", () => {
       const api = useApi()
 
       const response = await api
-        .delete("/admin/products/test-product", {
+        .delete("/admin/products/test-product", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -1924,11 +1828,7 @@ describe("/admin/products", () => {
       expect(variantPre).not.toEqual(undefined)
 
       const response = await api
-        .delete("/admin/products/test-product", {
+        .delete("/admin/products/test-product", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -1962,11 +1862,7 @@ describe("/admin/products", () => {
       // Soft delete the variant
       const response = await api.delete(
         "/admin/products/test-product/variants/test-variant_2",
-        {
+        adminHeaders
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        }
       )
 
       expect(response.status).toEqual(200)
@@ -2011,11 +1907,10 @@ describe("/admin/products", () => {
       expect(optValPre).not.toEqual(undefined)
 
       // Soft delete the product
-      const response = await api.delete("/admin/products/test-product", {
+      const response = await api.delete(
-        headers: {
+        "/admin/products/test-product",
-          Authorization: "Bearer test_token",
+        adminHeaders
-        },
+      )
-      })
 
       expect(response.status).toEqual(200)
 
@@ -2061,11 +1956,7 @@ describe("/admin/products", () => {
       // Soft delete the variant
       const response = await api.delete(
         "/admin/products/test-product/variants/test-variant",
-        {
+        adminHeaders
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        }
       )
 
       expect(response.status).toEqual(200)
@@ -2107,11 +1998,10 @@ describe("/admin/products", () => {
       expect(pricePre).not.toEqual(undefined)
 
       // Soft delete the product
-      const response = await api.delete("/admin/products/test-product", {
+      const response = await api.delete(
-        headers: {
+        "/admin/products/test-product",
-          Authorization: "Bearer test_token",
+        adminHeaders
-        },
+      )
-      })
 
       expect(response.status).toEqual(200)
 
@@ -2146,11 +2036,7 @@ describe("/admin/products", () => {
 
       // First we soft-delete the product
       const response = await api
-        .delete("/admin/products/test-product", {
+        .delete("/admin/products/test-product", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -2178,22 +2064,14 @@ describe("/admin/products", () => {
         ],
       }
 
-      const res = await api.post("/admin/products", payload, {
+      const res = await api.post("/admin/products", payload, adminHeaders)
-        headers: {
-          Authorization: "Bearer test_token",
-        },
-      })
 
       expect(res.status).toEqual(200)
       expect(res.data.product.handle).toEqual("test-product")
 
       // Delete product again to ensure uniqueness is enforced in all cases
       const response2 = await api
-        .delete("/admin/products/test-product", {
+        .delete("/admin/products/test-product", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -2226,11 +2104,7 @@ describe("/admin/products", () => {
       }
 
       try {
-        await api.post("/admin/products", payload, {
+        await api.post("/admin/products", payload, adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
       } catch (error) {
         expect(error.response.data.message).toMatch(
           "Product with handle test-product already exists."
@@ -2243,11 +2117,7 @@ describe("/admin/products", () => {
 
       // First we soft-delete the product collection
       const response = await api
-        .delete("/admin/collections/test-collection", {
+        .delete("/admin/collections/test-collection", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -2260,11 +2130,7 @@ describe("/admin/products", () => {
       const api = useApi()
 
       const response = await api
-        .delete("/admin/collections/test-collection", {
+        .delete("/admin/collections/test-collection", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -2278,11 +2144,7 @@ describe("/admin/products", () => {
         handle: "test-collection",
       }
 
-      const res = await api.post("/admin/collections", payload, {
+      const res = await api.post("/admin/collections", payload, adminHeaders)
-        headers: {
-          Authorization: "Bearer test_token",
-        },
-      })
 
       expect(res.status).toEqual(200)
       expect(res.data.collection.handle).toEqual("test-collection")
@@ -2298,11 +2160,7 @@ describe("/admin/products", () => {
       }
 
       try {
-        await api.post("/admin/collections", payload, {
+        await api.post("/admin/collections", payload, adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
       } catch (error) {
         expect(error.response.data.message).toMatch(
           "Product_collection with handle test-collection already exists."
@@ -2314,21 +2172,16 @@ describe("/admin/products", () => {
       const api = useApi()
 
       await api
-        .get("/admin/products/test-product", {
+        .get("/admin/products/test-product", adminHeaders)
-          headers: {
-            Authorization: "bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })
 
       const response = await api
-        .delete("/admin/products/test-product/variants/test-variant", {
+        .delete(
-          headers: {
+          "/admin/products/test-product/variants/test-variant",
-            Authorization: "Bearer test_token",
+          adminHeaders
-          },
+        )
-        })
         .catch((err) => {
           console.log(err)
         })
@@ -2352,11 +2205,7 @@ describe("/admin/products", () => {
       }
 
       const res = await api
-        .post("/admin/products/test-product/variants", payload, {
+        .post("/admin/products/test-product/variants", payload, adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => console.log(err))
 
       expect(res.status).toEqual(200)
@@ -2402,11 +2251,7 @@ describe("/admin/products", () => {
           {
             inventory_quantity: 10,
           },
-          {
+          adminHeaders
-            headers: {
-              Authorization: "Bearer test_token",
-            },
-          }
         )
         .catch((err) => {
           console.log(err)
@@ -2431,11 +2276,7 @@ describe("/admin/products", () => {
       const api = useApi()
 
       const res = await api
-        .get("/admin/products/tag-usage", {
+        .get("/admin/products/tag-usage", adminHeaders)
-          headers: {
-            Authorization: "Bearer test_token",
-          },
-        })
         .catch((err) => {
           console.log(err)
         })

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

@@ -18,7 +18,9 @@ export class ProductOption extends SoftDeletableEntity {
   @Column()
   title: string
 
-  @OneToMany(() => ProductOptionValue, (value) => value.option)
+  @OneToMany(() => ProductOptionValue, (value) => value.option, {
+    cascade: ["soft-remove", "remove"],
+  })
   values: ProductOptionValue[]
 
   @Column()

packages/medusa/src/services/product.ts

@@ -812,6 +812,7 @@ class ProductService extends TransactionBaseService {
 
       const productOption = await productOptionRepo.findOne({
         where: { id: optionId, product_id: productId },
+        relations: ["values"],
       })
 
       if (!productOption) {