From d14a0398fb884a1cd472c147af8ff5fa6fdbe4cb Mon Sep 17 00:00:00 2001 From: Oliver Windall Juhl <59018053+olivermrbl@users.noreply.github.com> Date: Mon, 29 Aug 2022 20:51:45 +0200 Subject: [PATCH] fix(medusa): Delete ProductOption on Product without Variants (#1846) **What** Solves admin issue [166](https://github.com/medusajs/admin/issues/166) Deleting a product option on a product without variants currently throws, because we are cleaning up variant options as well. **How** Only do variant clean up, if product has variants --- .changeset/twelve-pots-kneel.md | 5 ++ .../api/__tests__/admin/product.js | 53 +++++++++++++++++++ packages/medusa/src/services/product.ts | 51 ++++++++++-------- 3 files changed, 86 insertions(+), 23 deletions(-) create mode 100644 .changeset/twelve-pots-kneel.md diff --git a/.changeset/twelve-pots-kneel.md b/.changeset/twelve-pots-kneel.md new file mode 100644 index 0000000000..2ebf66b16e --- /dev/null +++ b/.changeset/twelve-pots-kneel.md @@ -0,0 +1,5 @@ +--- +"@medusajs/medusa": patch +--- + +Fixes an error thrown when deleting product options on a product without variants diff --git a/integration-tests/api/__tests__/admin/product.js b/integration-tests/api/__tests__/admin/product.js index 6d74f5f65b..ecf115a234 100644 --- a/integration-tests/api/__tests__/admin/product.js +++ b/integration-tests/api/__tests__/admin/product.js @@ -12,6 +12,7 @@ const { MoneyAmount, } = require("@medusajs/medusa") const priceListSeeder = require("../../helpers/price-list-seeder") +const { simpleProductFactory } = require("../../factories") jest.setTimeout(50000) @@ -1433,6 +1434,58 @@ describe("/admin/products", () => { }) }) + describe("DELETE /admin/products/:id/options/:option_id", () => { + beforeEach(async () => { + try { + await simpleProductFactory(dbConnection, { + id: "test-product-without-variants", + variants: [], + options: [ + { + id: "test-product-option", + title: "Test option", + }, + ], + }) + await adminSeeder(dbConnection) + } catch (err) { + console.log(err) + throw err + } + }) + + afterEach(async () => { + const db = useDb() + await db.teardown() + }) + + it("deletes a product option", async () => { + const api = useApi() + + const response = await api + .delete( + "/admin/products/test-product-without-variants/options/test-product-option", + { + headers: { + Authorization: "Bearer test_token", + }, + } + ) + .catch((err) => { + console.log(err) + }) + + expect(response.status).toEqual(200) + expect(response.data.product).toEqual( + expect.objectContaining({ + options: [], + id: "test-product-without-variants", + variants: [], + }) + ) + }) + }) + describe("GET /admin/products/:id/variants", () => { beforeEach(async () => { await productSeeder(dbConnection) diff --git a/packages/medusa/src/services/product.ts b/packages/medusa/src/services/product.ts index aeb89fb2fc..e6249fa59b 100644 --- a/packages/medusa/src/services/product.ts +++ b/packages/medusa/src/services/product.ts @@ -818,32 +818,37 @@ class ProductService extends TransactionBaseService { return Promise.resolve() } - // For the option we want to delete, make sure that all variants have the - // same option values. The reason for doing is, that we want to avoid - // duplicate variants. For example, if we have a product with size and - // color options, that has four variants: (black, 1), (black, 2), - // (blue, 1), (blue, 2) and we delete the size option from the product, - // we would end up with four variants: (black), (black), (blue), (blue). - // We now have two duplicate variants. To ensure that this does not - // happen, we will force the user to select which variants to keep. - const firstVariant = product.variants[0] + // In case the product does not contain variants, we can safely delete the option + // If it does contain variants, we need to make sure no variant exist for the + // product option to delete + if (product?.variants?.length) { + // For the option we want to delete, make sure that all variants have the + // same option values. The reason for doing is, that we want to avoid + // duplicate variants. For example, if we have a product with size and + // color options, that has four variants: (black, 1), (black, 2), + // (blue, 1), (blue, 2) and we delete the size option from the product, + // we would end up with four variants: (black), (black), (blue), (blue). + // We now have two duplicate variants. To ensure that this does not + // happen, we will force the user to select which variants to keep. + const firstVariant = product.variants[0] - const valueToMatch = firstVariant.options.find( - (o) => o.option_id === optionId - )?.value + const valueToMatch = firstVariant.options.find( + (o) => o.option_id === optionId + )?.value - const equalsFirst = await Promise.all( - product.variants.map(async (v) => { - const option = v.options.find((o) => o.option_id === optionId) - return option?.value === valueToMatch - }) - ) - - if (!equalsFirst.every((v) => v)) { - throw new MedusaError( - MedusaError.Types.INVALID_DATA, - `To delete an option, first delete all variants, such that when an option is deleted, no duplicate variants will exist.` + const equalsFirst = await Promise.all( + product.variants.map(async (v) => { + const option = v.options.find((o) => o.option_id === optionId) + return option?.value === valueToMatch + }) ) + + if (!equalsFirst.every((v) => v)) { + throw new MedusaError( + MedusaError.Types.INVALID_DATA, + `To delete an option, first delete all variants, such that when an option is deleted, no duplicate variants will exist.` + ) + } } // If we reach this point, we can safely delete the product option