From 3188e703b302d5557d30c6489dc903432775696c Mon Sep 17 00:00:00 2001 From: Adrien de Peretti Date: Fri, 15 Mar 2024 14:25:51 +0100 Subject: [PATCH] feat(fulfillment): Separate list and context rules validation (#6674) **What** - Add method to validate fulfillment option from the provider - Separate list/list and count from context rules validation and add listShippingOptionsForContext FIXES CORE-1861 --- .../cart/store/cart.workflows.spec.ts | 4 +- .../list-shipping-options-for-cart.ts | 12 +- .../list-shipping-options-for-context.ts | 11 +- .../fulfillment-set.spec.ts | 324 ++++++++++------ .../geo-zone.spec.ts | 150 +++++--- .../service-zone.spec.ts | 168 +++++--- .../shipping-option.spec.ts | 225 ++++++++++- packages/fulfillment/package.json | 2 +- .../src/services/__tests__/noop.ts | 5 - .../services/fulfillment-module-service.ts | 361 ++++++++++++++---- .../src/services/fulfillment-provider.ts | 4 +- packages/fulfillment/src/utils/utils.ts | 2 +- .../src/fulfillment/common/service-zone.ts | 6 +- .../src/fulfillment/common/shipping-option.ts | 21 +- .../src/fulfillment/mutations/geo-zone.ts | 10 +- packages/types/src/fulfillment/service.ts | 32 +- 16 files changed, 1011 insertions(+), 326 deletions(-) delete mode 100644 packages/fulfillment/src/services/__tests__/noop.ts diff --git a/integration-tests/modules/__tests__/cart/store/cart.workflows.spec.ts b/integration-tests/modules/__tests__/cart/store/cart.workflows.spec.ts index 8a46ab89e1..98228cfeec 100644 --- a/integration-tests/modules/__tests__/cart/store/cart.workflows.spec.ts +++ b/integration-tests/modules/__tests__/cart/store/cart.workflows.spec.ts @@ -1413,7 +1413,7 @@ medusaIntegrationTestRunner({ geo_zones: [ { type: "country", - country_code: "us", + country_code: "dk", }, ], }, @@ -1637,7 +1637,7 @@ medusaIntegrationTestRunner({ geo_zones: [ { type: "country", - country_code: "us", + country_code: "dk", }, ], }, diff --git a/packages/core-flows/src/definition/cart/workflows/list-shipping-options-for-cart.ts b/packages/core-flows/src/definition/cart/workflows/list-shipping-options-for-cart.ts index 5961d91475..5f1d871f34 100644 --- a/packages/core-flows/src/definition/cart/workflows/list-shipping-options-for-cart.ts +++ b/packages/core-flows/src/definition/cart/workflows/list-shipping-options-for-cart.ts @@ -1,8 +1,8 @@ import { ListShippingOptionsForCartWorkflowInputDTO } from "@medusajs/types" import { - WorkflowData, createWorkflow, transform, + WorkflowData, } from "@medusajs/workflows-sdk" import { useRemoteQueryStep } from "../../../common/steps/use-remote-query" import { listShippingOptionsForContextStep } from "../../../shipping-options" @@ -33,12 +33,10 @@ export const listShippingOptionsForCartWorkflow = createWorkflow( return { context: { fulfillment_set_id: fulfillmentSetIds, - service_zone: { - geo_zones: { - city: data.input.shipping_address?.city, - country_code: data.input.shipping_address?.country_code, - province_code: data.input.shipping_address?.province, - }, + address: { + city: data.input.shipping_address?.city, + country_code: data.input.shipping_address?.country_code, + province_code: data.input.shipping_address?.province, }, }, config: { diff --git a/packages/core-flows/src/shipping-options/steps/list-shipping-options-for-context.ts b/packages/core-flows/src/shipping-options/steps/list-shipping-options-for-context.ts index e3e2e465b0..878bcea7f2 100644 --- a/packages/core-flows/src/shipping-options/steps/list-shipping-options-for-context.ts +++ b/packages/core-flows/src/shipping-options/steps/list-shipping-options-for-context.ts @@ -4,7 +4,7 @@ import { IFulfillmentModuleService, ShippingOptionDTO, } from "@medusajs/types" -import { StepResponse, createStep } from "@medusajs/workflows-sdk" +import { createStep, StepResponse } from "@medusajs/workflows-sdk" interface StepInput { context: Record @@ -20,10 +20,11 @@ export const listShippingOptionsForContextStep = createStep( ModuleRegistrationName.FULFILLMENT ) - const shippingOptions = await fulfillmentService.listShippingOptions( - data.context, - data.config - ) + const shippingOptions = + await fulfillmentService.listShippingOptionsForContext( + data.context, + data.config + ) return new StepResponse(shippingOptions) } diff --git a/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service/fulfillment-set.spec.ts b/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service/fulfillment-set.spec.ts index bf82435b62..68f743b11d 100644 --- a/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service/fulfillment-set.spec.ts +++ b/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service/fulfillment-set.spec.ts @@ -1,4 +1,4 @@ -import {Modules} from "@medusajs/modules-sdk" +import { Modules } from "@medusajs/modules-sdk" import { CreateFulfillmentSetDTO, CreateServiceZoneDTO, @@ -6,8 +6,8 @@ import { ServiceZoneDTO, UpdateFulfillmentSetDTO, } from "@medusajs/types" -import {GeoZoneType} from "@medusajs/utils" -import {moduleIntegrationTestRunner, SuiteOptions} from "medusa-test-utils" +import { GeoZoneType } from "@medusajs/utils" +import { moduleIntegrationTestRunner, SuiteOptions } from "medusa-test-utils" jest.setTimeout(100000) @@ -17,123 +17,123 @@ moduleIntegrationTestRunner({ describe("Fulfillment Module Service", () => { describe("read", () => { it("should list fulfillment sets with a filter", async function () { - const createdSet1 = await service.create({ - name: "test", - type: "test-type", - }) - const createdSet2 = await service.create({ - name: "test2", - type: "test-type", - service_zones: [ - { - name: "test", - geo_zones: [ - { - type: GeoZoneType.COUNTRY, - country_code: "fr", - }, - ], - }, - { - name: "test2", - geo_zones: [ - { - type: GeoZoneType.COUNTRY, - country_code: "fr", - }, - ], - }, - { - name: "_test", - geo_zones: [ - { - type: GeoZoneType.COUNTRY, - country_code: "fr", - }, - ], - }, - ], - }) - - let listedSets = await service.list( - { - type: createdSet1.type, - }, - { - relations: ["service_zones"], - } - ) - - const listedSets2 = await service.list( - { - type: createdSet1.type, - }, - { - relations: ["service_zones"], - } - ) - - expect(listedSets).toEqual( - expect.arrayContaining([ - expect.objectContaining({ id: createdSet1.id }), - expect.objectContaining({ id: createdSet2.id }), - ]) - ) - - // Respecting order id by default - expect(listedSets[1].service_zones).toEqual([ - expect.objectContaining({ name: "test" }), - expect.objectContaining({ name: "test2" }), - expect.objectContaining({ name: "_test" }), - ]) - - expect(listedSets2).toEqual(listedSets2) - - listedSets = await service.list({ - name: createdSet2.name, - }) - - expect(listedSets).toEqual( - expect.arrayContaining([ - expect.objectContaining({ id: createdSet2.id }), - ]) - ) - expect(listedSets).not.toEqual( - expect.arrayContaining([ - expect.objectContaining({ id: createdSet1.id }), - ]) - ) - - listedSets = await service.list({ - service_zones: { name: "test" }, - }) - - expect(listedSets).toEqual( - expect.arrayContaining([ - expect.objectContaining({ id: createdSet2.id }), - ]) - ) - expect(listedSets).not.toEqual( - expect.arrayContaining([ - expect.objectContaining({ id: createdSet1.id }), - ]) - ) - - listedSets = await service.list({ - service_zones: { geo_zones: { country_code: "fr" } }, - }) - - expect(listedSets).toEqual( - expect.arrayContaining([ - expect.objectContaining({ id: createdSet2.id }), - ]) - ) - expect(listedSets).not.toEqual( - expect.arrayContaining([ - expect.objectContaining({ id: createdSet1.id }), - ]) - ) + const createdSet1 = await service.create({ + name: "test", + type: "test-type", }) + const createdSet2 = await service.create({ + name: "test2", + type: "test-type", + service_zones: [ + { + name: "test", + geo_zones: [ + { + type: GeoZoneType.COUNTRY, + country_code: "fr", + }, + ], + }, + { + name: "test2", + geo_zones: [ + { + type: GeoZoneType.COUNTRY, + country_code: "fr", + }, + ], + }, + { + name: "_test", + geo_zones: [ + { + type: GeoZoneType.COUNTRY, + country_code: "fr", + }, + ], + }, + ], + }) + + let listedSets = await service.list( + { + type: createdSet1.type, + }, + { + relations: ["service_zones"], + } + ) + + const listedSets2 = await service.list( + { + type: createdSet1.type, + }, + { + relations: ["service_zones"], + } + ) + + expect(listedSets).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: createdSet1.id }), + expect.objectContaining({ id: createdSet2.id }), + ]) + ) + + // Respecting order id by default + expect(listedSets[1].service_zones).toEqual([ + expect.objectContaining({ name: "test" }), + expect.objectContaining({ name: "test2" }), + expect.objectContaining({ name: "_test" }), + ]) + + expect(listedSets2).toEqual(listedSets2) + + listedSets = await service.list({ + name: createdSet2.name, + }) + + expect(listedSets).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: createdSet2.id }), + ]) + ) + expect(listedSets).not.toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: createdSet1.id }), + ]) + ) + + listedSets = await service.list({ + service_zones: { name: "test" }, + }) + + expect(listedSets).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: createdSet2.id }), + ]) + ) + expect(listedSets).not.toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: createdSet1.id }), + ]) + ) + + listedSets = await service.list({ + service_zones: { geo_zones: { country_code: "fr" } }, + }) + + expect(listedSets).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: createdSet2.id }), + ]) + ) + expect(listedSets).not.toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: createdSet1.id }), + ]) + ) + }) }) describe("mutations", () => { @@ -349,6 +349,7 @@ moduleIntegrationTestRunner({ { type: GeoZoneType.CITY, country_code: "fr", + province_code: "test", city: "lyon", }, ], @@ -400,6 +401,91 @@ moduleIntegrationTestRunner({ expect(err).toBeDefined() expect(err.constraint).toBe("IDX_fulfillment_set_name_unique") }) + + it("should fail on creating a new fulfillment set with new service zones and new geo zones that are not valid", async function () { + let data: CreateFulfillmentSetDTO = { + name: "test", + type: "test-type", + service_zones: [ + { + name: "test", + geo_zones: [ + { + type: GeoZoneType.PROVINCE, + country_code: "fr", + } as any, + ], + }, + ], + } + + let err = await service.create(data).catch((e) => e) + expect(err.message).toBe( + "Missing required property province_code for geo zone type province" + ) + + data = { + name: "test", + type: "test-type", + service_zones: [ + { + name: "test", + geo_zones: [ + { + type: GeoZoneType.CITY, + country_code: "fr", + province_code: "test", + } as any, + ], + }, + ], + } + + err = await service.create(data).catch((e) => e) + expect(err.message).toBe( + "Missing required property city for geo zone type city" + ) + + data = { + name: "test", + type: "test-type", + service_zones: [ + { + name: "test", + geo_zones: [ + { + type: GeoZoneType.ZIP, + postal_expression: "test", + } as any, + ], + }, + ], + } + + err = await service.create(data).catch((e) => e) + expect(err.message).toBe( + "Missing required property country_code for geo zone type zip" + ) + + data = { + name: "test", + type: "test-type", + service_zones: [ + { + name: "test", + geo_zones: [ + { + type: "unknown", + postal_expression: "test", + } as any, + ], + }, + ], + } + + err = await service.create(data).catch((e) => e) + expect(err.message).toBe(`Invalid geo zone type: unknown`) + }) }) describe("on update", () => { diff --git a/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service/geo-zone.spec.ts b/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service/geo-zone.spec.ts index 84e6c2c103..7e29ac3140 100644 --- a/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service/geo-zone.spec.ts +++ b/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service/geo-zone.spec.ts @@ -1,11 +1,11 @@ -import {Modules} from "@medusajs/modules-sdk" +import { Modules } from "@medusajs/modules-sdk" import { CreateGeoZoneDTO, IFulfillmentModuleService, UpdateGeoZoneDTO, } from "@medusajs/types" -import {GeoZoneType} from "@medusajs/utils" -import {moduleIntegrationTestRunner, SuiteOptions} from "medusa-test-utils" +import { GeoZoneType } from "@medusajs/utils" +import { moduleIntegrationTestRunner, SuiteOptions } from "medusa-test-utils" jest.setTimeout(100000) @@ -15,52 +15,52 @@ moduleIntegrationTestRunner({ describe("Fulfillment Module Service", () => { describe("read", () => { it("should list geo zones with a filter", async function () { - const fulfillmentSet = await service.create({ - name: "test", - type: "test-type", - }) - const serviceZone = await service.createServiceZones({ - name: "test", - fulfillment_set_id: fulfillmentSet.id, - }) - - const createdZone1 = await service.createGeoZones({ - service_zone_id: serviceZone.id, - type: GeoZoneType.COUNTRY, - country_code: "fr", - }) - const createdZone2 = await service.createGeoZones({ - service_zone_id: serviceZone.id, - type: GeoZoneType.COUNTRY, - country_code: "us", - }) - - let listedZones = await service.listGeoZones({ - type: createdZone1.type, - }) - - expect(listedZones).toEqual( - expect.arrayContaining([ - expect.objectContaining({ id: createdZone1.id }), - expect.objectContaining({ id: createdZone2.id }), - ]) - ) - - listedZones = await service.listGeoZones({ - country_code: createdZone2.country_code, - }) - - expect(listedZones).toEqual( - expect.arrayContaining([ - expect.objectContaining({ id: createdZone2.id }), - ]) - ) - expect(listedZones).not.toEqual( - expect.arrayContaining([ - expect.objectContaining({ id: createdZone1.id }), - ]) - ) + const fulfillmentSet = await service.create({ + name: "test", + type: "test-type", }) + const serviceZone = await service.createServiceZones({ + name: "test", + fulfillment_set_id: fulfillmentSet.id, + }) + + const createdZone1 = await service.createGeoZones({ + service_zone_id: serviceZone.id, + type: GeoZoneType.COUNTRY, + country_code: "fr", + }) + const createdZone2 = await service.createGeoZones({ + service_zone_id: serviceZone.id, + type: GeoZoneType.COUNTRY, + country_code: "us", + }) + + let listedZones = await service.listGeoZones({ + type: createdZone1.type, + }) + + expect(listedZones).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: createdZone1.id }), + expect.objectContaining({ id: createdZone2.id }), + ]) + ) + + listedZones = await service.listGeoZones({ + country_code: createdZone2.country_code, + }) + + expect(listedZones).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: createdZone2.id }), + ]) + ) + expect(listedZones).not.toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: createdZone1.id }), + ]) + ) + }) }) describe("mutations", () => { @@ -131,6 +131,60 @@ moduleIntegrationTestRunner({ ++i } }) + + it("should fail to create new geo zones that are not valid", async function () { + const fulfillmentSet = await service.create({ + name: "test", + type: "test-type", + }) + const serviceZone = await service.createServiceZones({ + name: "test", + fulfillment_set_id: fulfillmentSet.id, + }) + + let data: CreateGeoZoneDTO = { + service_zone_id: serviceZone.id, + type: GeoZoneType.PROVINCE, + country_code: "fr", + } as any + + let err = await service.createGeoZones(data).catch((e) => e) + expect(err.message).toBe( + "Missing required property province_code for geo zone type province" + ) + + data = { + service_zone_id: serviceZone.id, + type: GeoZoneType.CITY, + country_code: "fr", + province_code: "test", + } as any + + err = await service.createGeoZones(data).catch((e) => e) + expect(err.message).toBe( + "Missing required property city for geo zone type city" + ) + + data = { + service_zone_id: serviceZone.id, + type: GeoZoneType.ZIP, + postal_expression: "test", + } as any + + err = await service.createGeoZones(data).catch((e) => e) + expect(err.message).toBe( + "Missing required property country_code for geo zone type zip" + ) + + data = { + service_zone_id: serviceZone.id, + type: "unknown", + postal_expression: "test", + } as any + + err = await service.createGeoZones(data).catch((e) => e) + expect(err.message).toBe(`Invalid geo zone type: unknown`) + }) }) describe("on update", () => { diff --git a/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service/service-zone.spec.ts b/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service/service-zone.spec.ts index df3e2dcde4..55a781cb9a 100644 --- a/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service/service-zone.spec.ts +++ b/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service/service-zone.spec.ts @@ -16,56 +16,56 @@ moduleIntegrationTestRunner({ describe("Fulfillment Module Service", () => { describe("read", () => { it("should list service zones with a filter", async function () { - const fulfillmentSet = await service.create({ - name: "test", - type: "test-type", - }) - - const createdZone1 = await service.createServiceZones({ - name: "test", - fulfillment_set_id: fulfillmentSet.id, - }) - const createdZone2 = await service.createServiceZones({ - name: "test2", - fulfillment_set_id: fulfillmentSet.id, - geo_zones: [ - { - type: GeoZoneType.COUNTRY, - country_code: "fr", - }, - ], - }) - - let listedZones = await service.listServiceZones({ - name: createdZone2.name, - }) - - expect(listedZones).toEqual( - expect.arrayContaining([ - expect.objectContaining({ id: createdZone2.id }), - ]) - ) - expect(listedZones).not.toEqual( - expect.arrayContaining([ - expect.objectContaining({ id: createdZone1.id }), - ]) - ) - - listedZones = await service.listServiceZones({ - geo_zones: { country_code: "fr" }, - }) - - expect(listedZones).toEqual( - expect.arrayContaining([ - expect.objectContaining({ id: createdZone2.id }), - ]) - ) - expect(listedZones).not.toEqual( - expect.arrayContaining([ - expect.objectContaining({ id: createdZone1.id }), - ]) - ) + const fulfillmentSet = await service.create({ + name: "test", + type: "test-type", }) + + const createdZone1 = await service.createServiceZones({ + name: "test", + fulfillment_set_id: fulfillmentSet.id, + }) + const createdZone2 = await service.createServiceZones({ + name: "test2", + fulfillment_set_id: fulfillmentSet.id, + geo_zones: [ + { + type: GeoZoneType.COUNTRY, + country_code: "fr", + }, + ], + }) + + let listedZones = await service.listServiceZones({ + name: createdZone2.name, + }) + + expect(listedZones).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: createdZone2.id }), + ]) + ) + expect(listedZones).not.toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: createdZone1.id }), + ]) + ) + + listedZones = await service.listServiceZones({ + geo_zones: { country_code: "fr" }, + }) + + expect(listedZones).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: createdZone2.id }), + ]) + ) + expect(listedZones).not.toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: createdZone1.id }), + ]) + ) + }) }) describe("mutations", () => { @@ -189,6 +189,76 @@ moduleIntegrationTestRunner({ expect(err).toBeDefined() expect(err.constraint).toBe("IDX_service_zone_name_unique") }) + + it("should fail on creating a service zone and new geo zones that are not valid", async function () { + const fulfillmentSet = await service.create({ + name: "test", + type: "test-type", + }) + + let data: CreateServiceZoneDTO = { + name: "test", + fulfillment_set_id: fulfillmentSet.id, + geo_zones: [ + { + type: GeoZoneType.PROVINCE, + country_code: "fr", + } as any, + ], + } + + let err = await service.createServiceZones(data).catch((e) => e) + expect(err.message).toBe( + "Missing required property province_code for geo zone type province" + ) + + data = { + name: "test", + fulfillment_set_id: fulfillmentSet.id, + geo_zones: [ + { + type: GeoZoneType.CITY, + country_code: "fr", + province_code: "test", + } as any, + ], + } + + err = await service.createServiceZones(data).catch((e) => e) + expect(err.message).toBe( + "Missing required property city for geo zone type city" + ) + + data = { + name: "test", + fulfillment_set_id: fulfillmentSet.id, + geo_zones: [ + { + type: GeoZoneType.ZIP, + postal_expression: "test", + } as any, + ], + } + + err = await service.createServiceZones(data).catch((e) => e) + expect(err.message).toBe( + "Missing required property country_code for geo zone type zip" + ) + + data = { + name: "test", + fulfillment_set_id: fulfillmentSet.id, + geo_zones: [ + { + type: "unknown", + postal_expression: "test", + } as any, + ], + } + + err = await service.createServiceZones(data).catch((e) => e) + expect(err.message).toBe(`Invalid geo zone type: unknown`) + }) }) describe("on update", () => { diff --git a/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service/shipping-option.spec.ts b/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service/shipping-option.spec.ts index 1eae51938e..d13144022b 100644 --- a/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service/shipping-option.spec.ts +++ b/packages/fulfillment/integration-tests/__tests__/fulfillment-module-service/shipping-option.spec.ts @@ -8,6 +8,7 @@ import { generateCreateShippingOptionsData } from "../../__fixtures__" import { resolve } from "path" import { FulfillmentProviderService } from "@services" import { FulfillmentProviderServiceFixtures } from "../../__fixtures__/providers" +import { GeoZoneType } from "@medusajs/utils" jest.setTimeout(100000) @@ -35,10 +36,7 @@ const providerId = FulfillmentProviderService.getRegistrationIdentifier( moduleIntegrationTestRunner({ moduleName: Modules.FULFILLMENT, moduleOptions, - testSuite: ({ - MikroOrmWrapper, - service, - }: SuiteOptions) => { + testSuite: ({ service }: SuiteOptions) => { describe("Fulfillment Module Service", () => { describe("read", () => { it("should list shipping options with a filter", async function () { @@ -158,7 +156,7 @@ moduleIntegrationTestRunner({ }), ]) - let listedOptions = await service.listShippingOptions({ + let listedOptions = await service.listShippingOptionsForContext({ context: { "test-attribute": "test", "test-attribute2": { @@ -175,8 +173,12 @@ moduleIntegrationTestRunner({ ]) ) - listedOptions = await service.listShippingOptions({ - fulfillment_set_id: { $ne: fulfillmentSet.id }, + listedOptions = await service.listShippingOptionsForContext({ + service_zone: { + fulfillment_set: { + id: { $ne: fulfillmentSet.id }, + }, + }, context: { "test-attribute": "test", "test-attribute2": { @@ -187,8 +189,12 @@ moduleIntegrationTestRunner({ expect(listedOptions).toHaveLength(0) - listedOptions = await service.listShippingOptions({ - fulfillment_set_type: "non-existing-type", + listedOptions = await service.listShippingOptionsForContext({ + service_zone: { + fulfillment_set: { + type: "non-existing-type", + }, + }, context: { "test-attribute": "test", "test-attribute2": { @@ -199,6 +205,207 @@ moduleIntegrationTestRunner({ expect(listedOptions).toHaveLength(0) }) + + it(`should list the shipping options for a context with a specific address`, async function () { + const fulfillmentSet = await service.create({ + name: "test", + type: "test-type", + service_zones: [ + { + name: "test", + geo_zones: [ + { + type: GeoZoneType.ZIP, + country_code: "fr", + province_code: "rhone", + city: "paris", + postal_expression: "75006", + }, + ], + }, + ], + }) + + const shippingProfile = await service.createShippingProfiles({ + name: "test", + type: "default", + }) + + const [shippingOption1, , shippingOption3] = + await service.createShippingOptions([ + generateCreateShippingOptionsData({ + service_zone_id: fulfillmentSet.service_zones[0].id, + shipping_profile_id: shippingProfile.id, + provider_id: providerId, + rules: [ + { + attribute: "test-attribute", + operator: "in", + value: ["test"], + }, + ], + }), + generateCreateShippingOptionsData({ + service_zone_id: fulfillmentSet.service_zones[0].id, + shipping_profile_id: shippingProfile.id, + provider_id: providerId, + rules: [ + { + attribute: "test-attribute", + operator: "in", + value: ["test-test"], + }, + ], + }), + generateCreateShippingOptionsData({ + service_zone_id: fulfillmentSet.service_zones[0].id, + shipping_profile_id: shippingProfile.id, + provider_id: providerId, + rules: [ + { + attribute: "test-attribute", + operator: "eq", + value: "test", + }, + { + attribute: "test-attribute2.options", + operator: "in", + value: ["test", "test2"], + }, + ], + }), + ]) + + let shippingOptions = await service.listShippingOptionsForContext({ + address: { + country_code: "fr", + province_code: "rhone", + city: "paris", + postal_expression: "75006", + }, + }) + + expect(shippingOptions).toHaveLength(3) + + shippingOptions = await service.listShippingOptionsForContext({ + address: { + country_code: "fr", + province_code: "rhone", + city: "paris", + postal_expression: "75001", + }, + }) + + expect(shippingOptions).toHaveLength(0) + + shippingOptions = await service.listShippingOptionsForContext({ + address: { + country_code: "fr", + province_code: "rhone", + city: "paris", + postal_expression: "75006", + }, + context: { + "test-attribute": "test", + "test-attribute2": { + options: "test2", + }, + }, + }) + + expect(shippingOptions).toHaveLength(2) + expect(shippingOptions).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: shippingOption1.id }), + expect.objectContaining({ id: shippingOption3.id }), + ]) + ) + }) + }) + + it("should validate if a shipping option is applicable to a context", async function () { + const fulfillmentSet = await service.create({ + name: "test", + type: "test-type", + service_zones: [ + { + name: "test", + }, + ], + }) + + const shippingProfile = await service.createShippingProfiles({ + name: "test", + type: "default", + }) + + const [shippingOption1, shippingOption2, shippingOption3] = + await service.createShippingOptions([ + generateCreateShippingOptionsData({ + service_zone_id: fulfillmentSet.service_zones[0].id, + shipping_profile_id: shippingProfile.id, + provider_id: providerId, + rules: [ + { + attribute: "test-attribute", + operator: "in", + value: ["test"], + }, + ], + }), + generateCreateShippingOptionsData({ + service_zone_id: fulfillmentSet.service_zones[0].id, + shipping_profile_id: shippingProfile.id, + provider_id: providerId, + rules: [ + { + attribute: "test-attribute", + operator: "in", + value: ["test-test"], + }, + ], + }), + generateCreateShippingOptionsData({ + service_zone_id: fulfillmentSet.service_zones[0].id, + shipping_profile_id: shippingProfile.id, + provider_id: providerId, + rules: [ + { + attribute: "test-attribute", + operator: "eq", + value: "test", + }, + { + attribute: "test-attribute2.options", + operator: "in", + value: ["test", "test2"], + }, + ], + }), + ]) + + let listedOptions = await service.listShippingOptions() + + expect(listedOptions).toHaveLength(3) + + const context = { + "test-attribute": "test", + "test-attribute2": { + options: "test2", + }, + } + + const isShippingOption1Applicable = + await service.validateShippingOption(shippingOption1.id, context) + expect(isShippingOption1Applicable).toBeTruthy() + + const isShippingOption2Applicable = + await service.validateShippingOption(shippingOption2.id, context) + expect(isShippingOption2Applicable).toBeFalsy() + + const isShippingOption3Applicable = + await service.validateShippingOption(shippingOption3.id, context) + expect(isShippingOption3Applicable).toBeTruthy() }) describe("mutations", () => { diff --git a/packages/fulfillment/package.json b/packages/fulfillment/package.json index b0dec0b81f..8f0f8f9fff 100644 --- a/packages/fulfillment/package.json +++ b/packages/fulfillment/package.json @@ -28,7 +28,7 @@ "watch:test": "tsc --build tsconfig.spec.json --watch", "prepublishOnly": "cross-env NODE_ENV=production tsc --build && tsc-alias -p tsconfig.json", "build": "rimraf dist && tsc --build && tsc-alias -p tsconfig.json", - "test": "jest --runInBand --bail --forceExit -- src/**/__tests__/**/*.ts", + "test": "jest --runInBand --bail --forceExit --passWithNoTests -- src/**/__tests__/**/*.ts", "test:integration": "jest --forceExit -- integration-tests/**/__tests__/**/*.spec.ts", "migration:generate": " MIKRO_ORM_CLI=./mikro-orm.config.dev.ts mikro-orm migration:generate", "migration:initial": " MIKRO_ORM_CLI=./mikro-orm.config.dev.ts mikro-orm migration:create --initial -n InitialSetupMigration", diff --git a/packages/fulfillment/src/services/__tests__/noop.ts b/packages/fulfillment/src/services/__tests__/noop.ts deleted file mode 100644 index 333c84c1dd..0000000000 --- a/packages/fulfillment/src/services/__tests__/noop.ts +++ /dev/null @@ -1,5 +0,0 @@ -describe("noop", function () { - it("should run", function () { - expect(true).toBe(true) - }) -}) diff --git a/packages/fulfillment/src/services/fulfillment-module-service.ts b/packages/fulfillment/src/services/fulfillment-module-service.ts index 68bbb2d301..22915ea28e 100644 --- a/packages/fulfillment/src/services/fulfillment-module-service.ts +++ b/packages/fulfillment/src/services/fulfillment-module-service.ts @@ -2,8 +2,6 @@ import { Context, DAL, FilterableFulfillmentSetProps, - FilterableShippingOptionProps, - FilterQuery, FindConfig, FulfillmentDTO, FulfillmentTypes, @@ -131,69 +129,17 @@ export default class FulfillmentModuleService< return joinerConfig } - protected static normalizeShippingOptionsListParams( - filters: FilterableShippingOptionProps = {}, - config: FindConfig = {} - ) { - let { fulfillment_set_id, fulfillment_set_type, context, ...where } = - filters - - const normalizedConfig = { ...config } - normalizedConfig.relations = [ - "rules", - "type", - "shipping_profile", - "provider", - ...(normalizedConfig.relations ?? []), - ] - // The assumption is that there won't be an infinite amount of shipping options. So if a context filtering needs to be applied we can retrieve them all. - normalizedConfig.take = - normalizedConfig.take ?? (context ? null : undefined) - - let normalizedFilters = { ...where } as FilterQuery - - if (fulfillment_set_id || fulfillment_set_type) { - const fulfillmentSetConstraints = {} - - if (fulfillment_set_id) { - fulfillmentSetConstraints["id"] = fulfillment_set_id - } - - if (fulfillment_set_type) { - fulfillmentSetConstraints["type"] = fulfillment_set_type - } - - normalizedFilters = { - ...normalizedFilters, - service_zone: { - fulfillment_set: fulfillmentSetConstraints, - }, - } - - normalizedConfig.relations.push("service_zone.fulfillment_set") - } - - normalizedConfig.relations = Array.from(new Set(normalizedConfig.relations)) - - return { - filters: normalizedFilters, - config: normalizedConfig, - context, - } - } - @InjectManager("baseRepository_") - // @ts-ignore - async listShippingOptions( - filters: FilterableShippingOptionProps = {}, + async listShippingOptionsForContext( + filters: FulfillmentTypes.FilterableShippingOptionForContextProps, config: FindConfig = {}, @MedusaContext() sharedContext: Context = {} ): Promise { const { - filters: normalizedFilters, - config: normalizedConfig, context, - } = FulfillmentModuleService.normalizeShippingOptionsListParams( + config: normalizedConfig, + filters: normalizedFilters, + } = FulfillmentModuleService.normalizeListShippingOptionsForContextParams( filters, config ) @@ -204,7 +150,6 @@ export default class FulfillmentModuleService< sharedContext ) - // Apply rules context filtering if (context) { shippingOptions = shippingOptions.filter((shippingOption) => { if (!shippingOption.rules?.length) { @@ -245,14 +190,6 @@ export default class FulfillmentModuleService< ) } - async retrieveFulfillmentOptions( - providerId: string - ): Promise[]> { - return await this.fulfillmentProviderService_.getFulfillmentOptions( - providerId - ) - } - @InjectManager("baseRepository_") async listFulfillments( filters: FulfillmentTypes.FilterableFulfillmentProps = {}, @@ -331,6 +268,20 @@ export default class FulfillmentModuleService< ): Promise { const data_ = Array.isArray(data) ? data : [data] + if (!data_.length) { + return [] + } + + for (const fulfillmentSet of data_) { + if (fulfillmentSet.service_zones?.length) { + for (const serviceZone of fulfillmentSet.service_zones) { + if (serviceZone.geo_zones?.length) { + FulfillmentModuleService.validateGeoZones(serviceZone.geo_zones) + } + } + } + } + const createdFulfillmentSets = await this.fulfillmentSetService_.create( data_, sharedContext @@ -384,6 +335,14 @@ export default class FulfillmentModuleService< return [] } + for (const serviceZone of data_) { + if (serviceZone.geo_zones?.length) { + if (serviceZone.geo_zones?.length) { + FulfillmentModuleService.validateGeoZones(serviceZone.geo_zones) + } + } + } + const createdServiceZones = await this.serviceZoneService_.create( data_, sharedContext @@ -520,13 +479,17 @@ export default class FulfillmentModuleService< | FulfillmentTypes.CreateGeoZoneDTO[], @MedusaContext() sharedContext: Context = {} ): Promise { + const data_ = Array.isArray(data) ? data : [data] + + FulfillmentModuleService.validateGeoZones(data_) + const createdGeoZones = await this.geoZoneService_.create( - data, + data_, sharedContext ) return await this.baseRepository_.serialize( - createdGeoZones, + Array.isArray(data) ? createdGeoZones : createdGeoZones[0], { populate: true, } @@ -777,6 +740,11 @@ export default class FulfillmentModuleService< fulfillmentSet.service_zones = fulfillmentSet.service_zones.map( (serviceZone) => { if (!("id" in serviceZone)) { + if (serviceZone.geo_zones?.length) { + FulfillmentModuleService.validateGeoZones( + serviceZone.geo_zones + ) + } return serviceZone } return serviceZonesMap.get(serviceZone.id)! @@ -940,6 +908,7 @@ export default class FulfillmentModuleService< serviceZone.geo_zones = serviceZone.geo_zones.map((geoZone) => { if (!("id" in geoZone)) { + FulfillmentModuleService.validateGeoZones([geoZone]) return geoZone } return geoZonesMap.get(geoZone.id)! @@ -1138,6 +1107,14 @@ export default class FulfillmentModuleService< | FulfillmentTypes.UpdateGeoZoneDTO[], @MedusaContext() sharedContext: Context = {} ): Promise { + const data_ = Array.isArray(data) ? data : [data] + + if (!data_.length) { + return [] + } + + FulfillmentModuleService.validateGeoZones(data_) + const updatedGeoZones = await this.geoZoneService_.update( data, sharedContext @@ -1271,6 +1248,41 @@ export default class FulfillmentModuleService< return Array.isArray(result) ? result[0] : result } + async retrieveFulfillmentOptions( + providerId: string + ): Promise[]> { + return await this.fulfillmentProviderService_.getFulfillmentOptions( + providerId + ) + } + + async validateFulfillmentOption( + providerId: string, + data: Record + ): Promise { + return await this.fulfillmentProviderService_.validateOption( + providerId, + data + ) + } + + @InjectManager("baseRepository_") + async validateShippingOption( + shippingOptionId: string, + context: Record = {}, + @MedusaContext() sharedContext: Context = {} + ) { + const shippingOptions = await this.listShippingOptionsForContext( + { id: shippingOptionId, context }, + { + relations: ["rules"], + }, + sharedContext + ) + + return !!shippingOptions.length + } + protected static canCancelFulfillmentOrThrow(fulfillment: Fulfillment) { if (fulfillment.shipped_at) { throw new MedusaError( @@ -1336,4 +1348,213 @@ export default class FulfillmentModuleService< ) } } + + protected static validateGeoZones( + geoZones: ( + | (Partial & { type: string }) + | (Partial & { type: string }) + )[] + ) { + const requirePropForType = { + country: ["country_code"], + province: ["country_code", "province_code"], + city: ["country_code", "province_code", "city"], + zip: ["country_code", "province_code", "city", "postal_expression"], + } + + for (const geoZone of geoZones) { + if (!requirePropForType[geoZone.type]) { + throw new MedusaError( + MedusaError.Types.INVALID_DATA, + `Invalid geo zone type: ${geoZone.type}` + ) + } + + for (const prop of requirePropForType[geoZone.type]) { + if (!geoZone[prop]) { + throw new MedusaError( + MedusaError.Types.INVALID_DATA, + `Missing required property ${prop} for geo zone type ${geoZone.type}` + ) + } + } + } + } + + protected static normalizeListShippingOptionsForContextParams( + filters: FulfillmentTypes.FilterableShippingOptionForContextProps, + config: FindConfig = {} + ) { + let { + fulfillment_set_id, + fulfillment_set_type, + address, + context, + ...where + } = filters + + const normalizedConfig = { ...config } + normalizedConfig.relations = [ + "rules", + "type", + "shipping_profile", + "provider", + ...(normalizedConfig.relations ?? []), + ] + + normalizedConfig.take = + normalizedConfig.take ?? (context ? null : undefined) + + let normalizedFilters = { ...where } + + if (fulfillment_set_id || fulfillment_set_type) { + const fulfillmentSetConstraints = {} + + if (fulfillment_set_id) { + fulfillmentSetConstraints["id"] = fulfillment_set_id + } + + if (fulfillment_set_type) { + fulfillmentSetConstraints["type"] = fulfillment_set_type + } + + normalizedFilters = { + ...normalizedFilters, + service_zone: { + ...(normalizedFilters.service_zone ?? {}), + fulfillment_set: { + ...(normalizedFilters.service_zone?.fulfillment_set ?? {}), + ...fulfillmentSetConstraints, + }, + }, + } + + normalizedConfig.relations.push("service_zone.fulfillment_set") + } + + if (address) { + const geoZoneConstraints = + FulfillmentModuleService.buildGeoZoneConstraintsFromAddress(address) + + normalizedFilters = { + ...normalizedFilters, + service_zone: { + ...(normalizedFilters.service_zone ?? {}), + geo_zones: { + $or: geoZoneConstraints.map((geoZoneConstraint) => ({ + // Apply eventually provided constraints on the geo zone along side the address constraints + ...(normalizedFilters.service_zone?.geo_zones ?? {}), + ...geoZoneConstraint, + })), + }, + }, + } + + normalizedConfig.relations.push("service_zone.geo_zones") + } + + normalizedConfig.relations = Array.from(new Set(normalizedConfig.relations)) + + return { + filters: normalizedFilters, + config: normalizedConfig, + context, + } + } + + /** + * Build the constraints for the geo zones based on the address properties + * available and the hierarchy of required properties. + * We build a OR constraint from the narrowest to the broadest + * e.g. if we have a postal expression we build a constraint for the postal expression require props of type zip + * and a constraint for the city required props of type city + * and a constraint for the province code required props of type province + * and a constraint for the country code required props of type country + * example: + * { + * $or: [ + * { + * type: "zip", + * country_code: "SE", + * province_code: "AB", + * city: "Stockholm", + * postal_expression: "12345" + * }, + * { + * type: "city", + * country_code: "SE", + * province_code: "AB", + * city: "Stockholm" + * }, + * { + * type: "province", + * country_code: "SE", + * province_code: "AB" + * }, + * { + * type: "country", + * country_code: "SE" + * } + * ] + * } + */ + private static buildGeoZoneConstraintsFromAddress( + address: FulfillmentTypes.FilterableShippingOptionForContextProps["address"] + ) { + /** + * Define the hierarchy of required properties for the geo zones. + */ + const geoZoneRequirePropertyHierarchy = { + postal_expression: [ + "country_code", + "province_code", + "city", + "postal_expression", + ], + city: ["country_code", "province_code", "city"], + province_code: ["country_code", "province_code"], + 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] + return geoZoneConstraint + }, {} as Record) + } + return null + }) + .filter((v): v is Record => !!v) + + return geoZoneConstraints + } } diff --git a/packages/fulfillment/src/services/fulfillment-provider.ts b/packages/fulfillment/src/services/fulfillment-provider.ts index 5b7e18521f..2fb1abe798 100644 --- a/packages/fulfillment/src/services/fulfillment-provider.ts +++ b/packages/fulfillment/src/services/fulfillment-provider.ts @@ -78,8 +78,8 @@ export default class FulfillmentProviderService extends ModulesSdkUtils.internal return await provider.validateFulfillmentData(optionData, data, context) } - async validateOption(data: any) { - const provider = this.retrieveProviderRegistration(data.provider_id) + async validateOption(providerId: string, data: Record) { + const provider = this.retrieveProviderRegistration(providerId) return await provider.validateOption(data) } diff --git a/packages/fulfillment/src/utils/utils.ts b/packages/fulfillment/src/utils/utils.ts index b362da6097..ad0fb1bbc8 100644 --- a/packages/fulfillment/src/utils/utils.ts +++ b/packages/fulfillment/src/utils/utils.ts @@ -73,7 +73,7 @@ export function isContextValid( } = { someAreValid: false, } -) { +): boolean { const { someAreValid } = options const loopComparator = someAreValid ? rules.some : rules.every diff --git a/packages/types/src/fulfillment/common/service-zone.ts b/packages/types/src/fulfillment/common/service-zone.ts index acb827500e..be9ba5a523 100644 --- a/packages/types/src/fulfillment/common/service-zone.ts +++ b/packages/types/src/fulfillment/common/service-zone.ts @@ -1,4 +1,7 @@ -import { FulfillmentSetDTO } from "./fulfillment-set" +import { + FilterableFulfillmentSetProps, + FulfillmentSetDTO, +} from "./fulfillment-set" import { FilterableGeoZoneProps, GeoZoneDTO } from "./geo-zone" import { ShippingOptionDTO } from "./shipping-option" import { BaseFilterable, OperatorMap } from "../../dal" @@ -20,5 +23,6 @@ export interface FilterableServiceZoneProps id?: string | string[] | OperatorMap name?: string | string[] | OperatorMap geo_zones?: FilterableGeoZoneProps + fulfillment_set?: FilterableFulfillmentSetProps shipping_options?: any // TODO } diff --git a/packages/types/src/fulfillment/common/shipping-option.ts b/packages/types/src/fulfillment/common/shipping-option.ts index 7e4bf76d4c..ce2e37cabc 100644 --- a/packages/types/src/fulfillment/common/shipping-option.ts +++ b/packages/types/src/fulfillment/common/shipping-option.ts @@ -39,9 +39,7 @@ export interface FilterableShippingOptionProps extends BaseFilterable { id?: string | string[] | OperatorMap name?: string | string[] | OperatorMap - fulfillment_set_id?: string | string[] | OperatorMap shipping_profile_id?: string | string[] | OperatorMap - fulfillment_set_type?: string | string[] | OperatorMap price_type?: | ShippingOptionPriceType | ShippingOptionPriceType[] @@ -49,5 +47,22 @@ export interface FilterableShippingOptionProps service_zone?: FilterableServiceZoneProps shipping_option_type?: FilterableShippingOptionTypeProps rules?: FilterableShippingOptionRuleProps - context?: Record +} + +export interface FilterableShippingOptionForContextProps + extends FilterableShippingOptionProps { + fulfillment_set_id?: string | string[] | OperatorMap + fulfillment_set_type?: string | string[] | OperatorMap + /** + * The address is a shortcut to filter through geo_zones + * and build opinionated validation and filtering around the geo_zones. + * For custom filtering you can go through the service_zone.geo_zones directly. + */ + address?: { + country_code?: string + province_code?: string + city?: string + postal_expression?: string + } + context?: Record } diff --git a/packages/types/src/fulfillment/mutations/geo-zone.ts b/packages/types/src/fulfillment/mutations/geo-zone.ts index df2518316d..4cb10c2c05 100644 --- a/packages/types/src/fulfillment/mutations/geo-zone.ts +++ b/packages/types/src/fulfillment/mutations/geo-zone.ts @@ -18,11 +18,14 @@ export interface CreateProvinceGeoZoneDTO extends CreateGeoZoneBaseDTO { export interface CreateCityGeoZoneDTO extends CreateGeoZoneBaseDTO { type: "city" + province_code: string city: string } export interface CreateZipGeoZoneDTO extends CreateGeoZoneBaseDTO { type: "zip" + province_code: string + city: string postal_expression: Record } @@ -47,12 +50,15 @@ export interface UpdateProvinceGeoZoneDTO extends UpdateGeoZoneBaseDTO { export interface UpdateCityGeoZoneDTO extends UpdateGeoZoneBaseDTO { type: "city" - city: string + province_code?: string + city?: string } export interface UpdateZipGeoZoneDTO extends UpdateGeoZoneBaseDTO { type: "zip" - postal_expression: Record + province_code?: string + city?: string + postal_expression?: Record } export type UpdateGeoZoneDTO = diff --git a/packages/types/src/fulfillment/service.ts b/packages/types/src/fulfillment/service.ts index 788c708d42..76ece35af1 100644 --- a/packages/types/src/fulfillment/service.ts +++ b/packages/types/src/fulfillment/service.ts @@ -3,6 +3,7 @@ import { FilterableFulfillmentSetProps, FilterableGeoZoneProps, FilterableServiceZoneProps, + FilterableShippingOptionForContextProps, FilterableShippingOptionProps, FilterableShippingOptionRuleProps, FilterableShippingOptionTypeProps, @@ -326,13 +327,24 @@ export interface IFulfillmentModuleService extends IModuleService { sharedContext?: Context ): Promise /** - * List and count shipping options + * List shipping options and eventually filter the result based on the context and their rules + * @param filters + * @param config + * @param sharedContext + */ + listShippingOptionsForContext( + filters: FilterableShippingOptionForContextProps, + config?: FindConfig, + sharedContext?: Context + ): Promise + /** + * List and count shipping options without taking into account the context * @param filters * @param config * @param sharedContext */ listAndCountShippingOptions( - filters?: FilterableShippingOptionProps, + filters?: Omit, config?: FindConfig, sharedContext?: Context ): Promise<[ShippingOptionDTO[], number]> @@ -664,4 +676,20 @@ export interface IFulfillmentModuleService extends IModuleService { retrieveFulfillmentOptions( providerId: string ): Promise[]> + + /** + * Validate the given shipping option fulfillment option from the provided data + */ + validateFulfillmentOption( + providerId: string, + data: Record + ): Promise + + /** + * Validate if the given shipping option is valid for a given context + */ + validateShippingOption( + shippingOptionId: string, + context: Record + ): Promise }