From d46f22567a13cd107aa21fffe1cac1ccea9c4a9e Mon Sep 17 00:00:00 2001 From: adrien2p Date: Wed, 10 Jul 2024 15:45:53 +0200 Subject: [PATCH 1/4] fix: Investigate geo zones with address checks --- .../shipping-option.spec.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/packages/modules/fulfillment/integration-tests/__tests__/fulfillment-module-service/shipping-option.spec.ts b/packages/modules/fulfillment/integration-tests/__tests__/fulfillment-module-service/shipping-option.spec.ts index 27a2e3af0e1b1..3da15fda4fd65 100644 --- a/packages/modules/fulfillment/integration-tests/__tests__/fulfillment-module-service/shipping-option.spec.ts +++ b/packages/modules/fulfillment/integration-tests/__tests__/fulfillment-module-service/shipping-option.spec.ts @@ -38,6 +38,7 @@ const providerId = FulfillmentProviderService.getRegistrationIdentifier( moduleIntegrationTestRunner({ moduleName: Modules.FULFILLMENT, moduleOptions, + debug: true, testSuite: ({ service }) => { let eventBusEmitSpy @@ -226,6 +227,10 @@ moduleIntegrationTestRunner({ { name: "test", geo_zones: [ + { + type: GeoZoneType.COUNTRY, + country_code: "fr", + }, { type: GeoZoneType.ZIP, country_code: "fr", @@ -304,6 +309,33 @@ moduleIntegrationTestRunner({ country_code: "fr", province_code: "rhone", city: "paris", + }, + }) + + expect(shippingOptions).toHaveLength(3) + + shippingOptions = await service.listShippingOptionsForContext({ + address: { + country_code: "fr", + province_code: "rhone", + }, + }) + + expect(shippingOptions).toHaveLength(3) + + shippingOptions = await service.listShippingOptionsForContext({ + address: { + country_code: "fr", + }, + }) + + expect(shippingOptions).toHaveLength(3) + + shippingOptions = await service.listShippingOptionsForContext({ + address: { + country_code: "us", + province_code: "rhone", + city: "paris", postal_expression: "75001", }, }) From 3f2ef6a6746fb175bd853f9209ef16812849b774 Mon Sep 17 00:00:00 2001 From: adrien2p Date: Wed, 10 Jul 2024 15:50:41 +0200 Subject: [PATCH 2/4] rm debug --- .../__tests__/fulfillment-module-service/shipping-option.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/modules/fulfillment/integration-tests/__tests__/fulfillment-module-service/shipping-option.spec.ts b/packages/modules/fulfillment/integration-tests/__tests__/fulfillment-module-service/shipping-option.spec.ts index 3da15fda4fd65..6e32146d41828 100644 --- a/packages/modules/fulfillment/integration-tests/__tests__/fulfillment-module-service/shipping-option.spec.ts +++ b/packages/modules/fulfillment/integration-tests/__tests__/fulfillment-module-service/shipping-option.spec.ts @@ -38,7 +38,6 @@ const providerId = FulfillmentProviderService.getRegistrationIdentifier( moduleIntegrationTestRunner({ moduleName: Modules.FULFILLMENT, moduleOptions, - debug: true, testSuite: ({ service }) => { let eventBusEmitSpy From 51e1b8a89ff8767f32619198f9519cf53f4cc74b Mon Sep 17 00:00:00 2001 From: adrien2p Date: Wed, 10 Jul 2024 18:07:01 +0200 Subject: [PATCH 3/4] fix constraint building checks --- .../shipping-option.spec.ts | 9 ++++++ .../services/fulfillment-module-service.ts | 31 ++----------------- 2 files changed, 12 insertions(+), 28 deletions(-) diff --git a/packages/modules/fulfillment/integration-tests/__tests__/fulfillment-module-service/shipping-option.spec.ts b/packages/modules/fulfillment/integration-tests/__tests__/fulfillment-module-service/shipping-option.spec.ts index 6e32146d41828..812bdb3852149 100644 --- a/packages/modules/fulfillment/integration-tests/__tests__/fulfillment-module-service/shipping-option.spec.ts +++ b/packages/modules/fulfillment/integration-tests/__tests__/fulfillment-module-service/shipping-option.spec.ts @@ -330,6 +330,15 @@ moduleIntegrationTestRunner({ expect(shippingOptions).toHaveLength(3) + shippingOptions = await service.listShippingOptionsForContext({ + address: { + country_code: "fr", + postal_expression: "75006", + }, + }) + + expect(shippingOptions).toHaveLength(3) + shippingOptions = await service.listShippingOptionsForContext({ address: { country_code: "us", diff --git a/packages/modules/fulfillment/src/services/fulfillment-module-service.ts b/packages/modules/fulfillment/src/services/fulfillment-module-service.ts index 799a6cd671f9e..190ebacc3b1c9 100644 --- a/packages/modules/fulfillment/src/services/fulfillment-module-service.ts +++ b/packages/modules/fulfillment/src/services/fulfillment-module-service.ts @@ -2188,38 +2188,13 @@ export default class FulfillmentModuleService country_code: ["country_code"], } - /** - * Validate that the address has the required properties for the geo zones - * constraints to build after. We are going from the narrowest to the broadest - */ - Object.entries(geoZoneRequirePropertyHierarchy).forEach( - ([prop, requiredProps]) => { - if (address![prop]) { - for (const requiredProp of requiredProps) { - if (!address![requiredProp]) { - throw new MedusaError( - MedusaError.Types.INVALID_DATA, - `Missing required property ${requiredProp} for address when property ${prop} is set` - ) - } - } - } - } - ) - const geoZoneConstraints = Object.entries(geoZoneRequirePropertyHierarchy) .map(([prop, requiredProps]) => { if (address![prop]) { return requiredProps.reduce((geoZoneConstraint, prop) => { - geoZoneConstraint.type = - prop === "postal_expression" - ? "zip" - : prop === "city" - ? "city" - : prop === "province_code" - ? "province" - : "country" - geoZoneConstraint[prop] = address![prop] + if (isPresent(address![prop])) { + geoZoneConstraint[prop] = address![prop] + } return geoZoneConstraint }, {} as Record) } From 4a7e3635131fe2291df991170a1d5eed5d6d5761 Mon Sep 17 00:00:00 2001 From: adrien2p Date: Wed, 10 Jul 2024 18:12:08 +0200 Subject: [PATCH 4/4] add more explanation --- .../fulfillment/src/services/fulfillment-module-service.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/modules/fulfillment/src/services/fulfillment-module-service.ts b/packages/modules/fulfillment/src/services/fulfillment-module-service.ts index 190ebacc3b1c9..cf95dcadddd3f 100644 --- a/packages/modules/fulfillment/src/services/fulfillment-module-service.ts +++ b/packages/modules/fulfillment/src/services/fulfillment-module-service.ts @@ -2188,6 +2188,12 @@ export default class FulfillmentModuleService country_code: ["country_code"], } + /** + * The following changes assume that the lowest level check (e.g postal expression) can't exist multiple times in the higher level (e.g country) + * In case we encounter situations where it is possible to have multiple postal expressions for the same country we need to change the logic back + * to this pr https://github.com/medusajs/medusa/pull/8066 + */ + const geoZoneConstraints = Object.entries(geoZoneRequirePropertyHierarchy) .map(([prop, requiredProps]) => { if (address![prop]) {