From 13b0ae08f9cd426ece9d5eb14231964584a123fa Mon Sep 17 00:00:00 2001 From: Sebastian Rindom Date: Sat, 28 Sep 2024 18:13:53 +0200 Subject: [PATCH 1/3] fix: when creating updating shipping option prices validate regions exist --- .../workflows/update-shipping-options.ts | 127 ++++++++++++++++++ .../steps/validate-shipping-option-prices.ts | 51 +++++++ .../workflows/update-shipping-options.ts | 7 +- 3 files changed, 184 insertions(+), 1 deletion(-) create mode 100644 packages/core/core-flows/src/fulfillment/steps/validate-shipping-option-prices.ts diff --git a/integration-tests/modules/__tests__/shipping-options/workflows/update-shipping-options.ts b/integration-tests/modules/__tests__/shipping-options/workflows/update-shipping-options.ts index 6c89aa60d63de..c5719f5ef3af8 100644 --- a/integration-tests/modules/__tests__/shipping-options/workflows/update-shipping-options.ts +++ b/integration-tests/modules/__tests__/shipping-options/workflows/update-shipping-options.ts @@ -257,6 +257,133 @@ medusaIntegrationTestRunner({ ) }) + it("should fail on non-existent region update shipping options", async () => { + const regionService = container.resolve( + Modules.REGION + ) as IRegionModuleService + + const [region] = await regionService.createRegions([ + { + name: "Test region", + currency_code: "eur", + countries: ["fr"], + }, + ]) + + const shippingOptionData: FulfillmentWorkflow.CreateShippingOptionsWorkflowInput = + { + name: "Test shipping option", + price_type: "flat", + service_zone_id: serviceZone.id, + shipping_profile_id: shippingProfile.id, + provider_id, + type: { + code: "manual-type", + label: "Manual Type", + description: "Manual Type Description", + }, + prices: [ + { + currency_code: "usd", + amount: 10, + }, + { + region_id: region.id, + amount: 100, + }, + { + currency_code: "dkk", + amount: 1000, + }, + ], + rules: [ + { + attribute: "total", + operator: RuleOperator.EQ, + value: "100", + }, + ], + } + + const { result } = await createShippingOptionsWorkflow(container).run({ + input: [shippingOptionData], + }) + + const remoteQuery = container.resolve( + ContainerRegistrationKeys.REMOTE_QUERY + ) + + let remoteQueryObject = remoteQueryObjectFromString({ + entryPoint: "shipping_option", + variables: { + id: result[0].id, + }, + fields: [ + "id", + "name", + "price_type", + "service_zone_id", + "shipping_profile_id", + "provider_id", + "data", + "metadata", + "type.*", + "created_at", + "updated_at", + "deleted_at", + "shipping_option_type_id", + "prices.*", + ], + }) + + const [createdShippingOption] = await remoteQuery(remoteQueryObject) + + const usdPrice = createdShippingOption.prices.find((price) => { + return price.currency_code === "usd" + }) + + const dkkPrice = createdShippingOption.prices.find((price) => { + return price.currency_code === "dkk" + }) + + const updateData: UpdateShippingOptionsWorkflowInput = { + id: createdShippingOption.id, + name: "Test shipping option", + price_type: "flat", + type: { + code: "manual-type", + label: "Manual Type", + description: "Manual Type Description", + }, + prices: [ + // We keep the usd price as is + // update the dkk price to 100 + // delete the third price eur + // create a new eur one instead + usdPrice, + { + ...dkkPrice, + amount: 100, + }, + { + region_id: region.id, + amount: 1000, + }, + ], + } + + await regionService.softDeleteRegions([region.id]) + + const { errors } = await updateShippingOptionsWorkflow(container).run({ + input: [updateData], + throwOnError: false, + }) + + expect(errors[0].error.message).toEqual( + `Cannot create prices for non-existent regions. Region with ids [${region.id}] were not found.` + ) + }) + it("should revert the shipping options", async () => { const regionService = container.resolve( Modules.REGION diff --git a/packages/core/core-flows/src/fulfillment/steps/validate-shipping-option-prices.ts b/packages/core/core-flows/src/fulfillment/steps/validate-shipping-option-prices.ts new file mode 100644 index 0000000000000..efb7c8a268f42 --- /dev/null +++ b/packages/core/core-flows/src/fulfillment/steps/validate-shipping-option-prices.ts @@ -0,0 +1,51 @@ +import { FulfillmentWorkflow } from "@medusajs/framework/types" +import { MedusaError, Modules } from "@medusajs/framework/utils" +import { StepResponse, createStep } from "@medusajs/framework/workflows-sdk" + +export const validateShippingOptionPricesStepId = + "validate-shipping-option-prices" + +/** + * Validate that regions exist for the shipping option prices. + */ +export const validateShippingOptionPricesStep = createStep( + validateShippingOptionPricesStepId, + async ( + input: FulfillmentWorkflow.UpdateShippingOptionsWorkflowInput[], + { container } + ) => { + const allPrices = input.flatMap((option) => { + if (!option.prices) { + return [] + } + return option.prices + }) + + const regionIdSet = new Set() + + allPrices.forEach((p) => { + if ("region_id" in p && p.region_id) { + regionIdSet.add(p.region_id) + } + }) + + const regionService = container.resolve(Modules.REGION) + const regionList = await regionService.listRegions({ + id: Array.from(regionIdSet), + }) + + if (regionList.length !== regionIdSet.size) { + const missingRegions = Array.from(regionIdSet).filter( + (id) => !regionList.some((region) => region.id === id) + ) + throw new MedusaError( + MedusaError.Types.INVALID_DATA, + `Cannot create prices for non-existent regions. Region with ids [${missingRegions.join( + ", " + )}] were not found.` + ) + } + + return new StepResponse(void 0) + } +) diff --git a/packages/core/core-flows/src/fulfillment/workflows/update-shipping-options.ts b/packages/core/core-flows/src/fulfillment/workflows/update-shipping-options.ts index 47c435d5a59a5..0c2a624b0645e 100644 --- a/packages/core/core-flows/src/fulfillment/workflows/update-shipping-options.ts +++ b/packages/core/core-flows/src/fulfillment/workflows/update-shipping-options.ts @@ -1,6 +1,7 @@ import { FulfillmentWorkflow } from "@medusajs/framework/types" import { createWorkflow, + parallelize, transform, WorkflowData, WorkflowResponse, @@ -10,6 +11,7 @@ import { upsertShippingOptionsStep, } from "../steps" import { validateFulfillmentProvidersStep } from "../steps/validate-fulfillment-providers" +import { validateShippingOptionPricesStep } from "../steps/validate-shipping-option-prices" export const updateShippingOptionsWorkflowId = "update-shipping-options-workflow" @@ -23,7 +25,10 @@ export const updateShippingOptionsWorkflow = createWorkflow( FulfillmentWorkflow.UpdateShippingOptionsWorkflowInput[] > ): WorkflowResponse => { - validateFulfillmentProvidersStep(input) + parallelize( + validateFulfillmentProvidersStep(input), + validateShippingOptionPricesStep(input) + ) const data = transform(input, (data) => { const shippingOptionsIndexToPrices = data.map((option, index) => { From a1fdea0e0c5d26b496462f58927b94640e8d024e Mon Sep 17 00:00:00 2001 From: Sebastian Rindom Date: Sat, 28 Sep 2024 18:45:38 +0200 Subject: [PATCH 2/3] fix: validate shipping option prices on creation --- .../workflows/create-shipping-options.ts | 55 +++++++++++++++++++ .../steps/validate-shipping-option-prices.ts | 6 +- .../workflows/create-shipping-options.ts | 7 ++- 3 files changed, 65 insertions(+), 3 deletions(-) diff --git a/integration-tests/modules/__tests__/shipping-options/workflows/create-shipping-options.ts b/integration-tests/modules/__tests__/shipping-options/workflows/create-shipping-options.ts index 01089e9dec1ba..403da1c36b88a 100644 --- a/integration-tests/modules/__tests__/shipping-options/workflows/create-shipping-options.ts +++ b/integration-tests/modules/__tests__/shipping-options/workflows/create-shipping-options.ts @@ -204,6 +204,61 @@ medusaIntegrationTestRunner({ ) }) + it("should fail with invalid region price", async () => { + const regionService = container.resolve( + Modules.REGION + ) as IRegionModuleService + + const [region] = await regionService.createRegions([ + { + name: "Test region", + currency_code: "eur", + countries: ["fr"], + }, + ]) + await regionService.softDeleteRegions([region.id]) + + const shippingOptionData: FulfillmentWorkflow.CreateShippingOptionsWorkflowInput = + { + name: "Test shipping option", + price_type: "flat", + service_zone_id: serviceZone.id, + shipping_profile_id: shippingProfile.id, + provider_id, + type: { + code: "manual-type", + label: "Manual Type", + description: "Manual Type Description", + }, + prices: [ + { + currency_code: "usd", + amount: 10, + }, + { + region_id: region.id, + amount: 100, + }, + ], + rules: [ + { + attribute: "total", + operator: RuleOperator.EQ, + value: "100", + }, + ], + } + + const { errors } = await createShippingOptionsWorkflow(container).run({ + input: [shippingOptionData], + throwOnError: false, + }) + + expect(errors[0].error.message).toEqual( + `Cannot create prices for non-existent regions. Region with ids [${region.id}] were not found.` + ) + }) + it("should revert the shipping options and prices", async () => { const regionService = container.resolve( Modules.REGION diff --git a/packages/core/core-flows/src/fulfillment/steps/validate-shipping-option-prices.ts b/packages/core/core-flows/src/fulfillment/steps/validate-shipping-option-prices.ts index efb7c8a268f42..ef9d209f59a0c 100644 --- a/packages/core/core-flows/src/fulfillment/steps/validate-shipping-option-prices.ts +++ b/packages/core/core-flows/src/fulfillment/steps/validate-shipping-option-prices.ts @@ -11,10 +11,12 @@ export const validateShippingOptionPricesStepId = export const validateShippingOptionPricesStep = createStep( validateShippingOptionPricesStepId, async ( - input: FulfillmentWorkflow.UpdateShippingOptionsWorkflowInput[], + options: { + prices?: FulfillmentWorkflow.UpdateShippingOptionsWorkflowInput["prices"] + }[], { container } ) => { - const allPrices = input.flatMap((option) => { + const allPrices = options.flatMap((option) => { if (!option.prices) { return [] } diff --git a/packages/core/core-flows/src/fulfillment/workflows/create-shipping-options.ts b/packages/core/core-flows/src/fulfillment/workflows/create-shipping-options.ts index b309ba14099ea..dda9dbef0d213 100644 --- a/packages/core/core-flows/src/fulfillment/workflows/create-shipping-options.ts +++ b/packages/core/core-flows/src/fulfillment/workflows/create-shipping-options.ts @@ -1,6 +1,7 @@ import { FulfillmentWorkflow } from "@medusajs/framework/types" import { createWorkflow, + parallelize, transform, WorkflowData, WorkflowResponse, @@ -11,6 +12,7 @@ import { } from "../steps" import { setShippingOptionsPriceSetsStep } from "../steps/set-shipping-options-price-sets" import { validateFulfillmentProvidersStep } from "../steps/validate-fulfillment-providers" +import { validateShippingOptionPricesStep } from "../steps/validate-shipping-option-prices" export const createShippingOptionsWorkflowId = "create-shipping-options-workflow" @@ -24,7 +26,10 @@ export const createShippingOptionsWorkflow = createWorkflow( FulfillmentWorkflow.CreateShippingOptionsWorkflowInput[] > ): WorkflowResponse => { - validateFulfillmentProvidersStep(input) + parallelize( + validateFulfillmentProvidersStep(input), + validateShippingOptionPricesStep(input) + ) const data = transform(input, (data) => { const shippingOptionsIndexToPrices = data.map((option, index) => { From 9d29290da03684617ef799757d5a996f52a8e6e7 Mon Sep 17 00:00:00 2001 From: Sebastian Rindom Date: Sun, 29 Sep 2024 11:37:04 +0200 Subject: [PATCH 3/3] fix: pr comments --- .../steps/validate-shipping-option-prices.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/packages/core/core-flows/src/fulfillment/steps/validate-shipping-option-prices.ts b/packages/core/core-flows/src/fulfillment/steps/validate-shipping-option-prices.ts index ef9d209f59a0c..ab07f9280693c 100644 --- a/packages/core/core-flows/src/fulfillment/steps/validate-shipping-option-prices.ts +++ b/packages/core/core-flows/src/fulfillment/steps/validate-shipping-option-prices.ts @@ -16,21 +16,20 @@ export const validateShippingOptionPricesStep = createStep( }[], { container } ) => { - const allPrices = options.flatMap((option) => { - if (!option.prices) { - return [] - } - return option.prices - }) + const allPrices = options.flatMap((option) => option.prices ?? []) const regionIdSet = new Set() - allPrices.forEach((p) => { - if ("region_id" in p && p.region_id) { - regionIdSet.add(p.region_id) + allPrices.forEach((price) => { + if ("region_id" in price && price.region_id) { + regionIdSet.add(price.region_id) } }) + if (regionIdSet.size === 0) { + return new StepResponse(void 0) + } + const regionService = container.resolve(Modules.REGION) const regionList = await regionService.listRegions({ id: Array.from(regionIdSet),