Explorar o código

perf(core): Optimize invocation of ShippingEligibilityCheckers

Relates to #536. This commit implements the following changes:

1. The `check` function is only run on changes to the order _if_ a ShippingMethod has been assigned
 to the Order.
2. If the order has a ShippingMethod, then _only_ that method's `check` function is invoked on
changes.
Michael Bromley %!s(int64=5) %!d(string=hai) anos
pai
achega
11415e64b4

+ 59 - 0
packages/core/e2e/graphql/fragments.ts

@@ -540,3 +540,62 @@ export const GLOBAL_SETTINGS_FRAGMENT = gql`
         }
     }
 `;
+
+export const CUSTOMER_GROUP_FRAGMENT = gql`
+    fragment CustomerGroup on CustomerGroup {
+        id
+        name
+        customers {
+            items {
+                id
+            }
+            totalItems
+        }
+    }
+`;
+
+export const PRODUCT_OPTION_GROUP_FRAGMENT = gql`
+    fragment ProductOptionGroup on ProductOptionGroup {
+        id
+        code
+        name
+        options {
+            id
+            code
+            name
+        }
+        translations {
+            id
+            languageCode
+            name
+        }
+    }
+`;
+
+export const PRODUCT_WITH_OPTIONS_FRAGMENT = gql`
+    fragment ProductWithOptions on Product {
+        id
+        optionGroups {
+            id
+            code
+            options {
+                id
+                code
+            }
+        }
+    }
+`;
+
+export const SHIPPING_METHOD_FRAGMENT = gql`
+    fragment ShippingMethod on ShippingMethod {
+        id
+        code
+        description
+        calculator {
+            code
+        }
+        checker {
+            code
+        }
+    }
+`;

+ 62 - 62
packages/core/e2e/graphql/generated-e2e-admin-types.ts

@@ -4667,6 +4667,26 @@ export type GlobalSettingsFragment = Pick<
     };
 };
 
+export type CustomerGroupFragment = Pick<CustomerGroup, 'id' | 'name'> & {
+    customers: Pick<CustomerList, 'totalItems'> & { items: Array<Pick<Customer, 'id'>> };
+};
+
+export type ProductOptionGroupFragment = Pick<ProductOptionGroup, 'id' | 'code' | 'name'> & {
+    options: Array<Pick<ProductOption, 'id' | 'code' | 'name'>>;
+    translations: Array<Pick<ProductOptionGroupTranslation, 'id' | 'languageCode' | 'name'>>;
+};
+
+export type ProductWithOptionsFragment = Pick<Product, 'id'> & {
+    optionGroups: Array<
+        Pick<ProductOptionGroup, 'id' | 'code'> & { options: Array<Pick<ProductOption, 'id' | 'code'>> }
+    >;
+};
+
+export type ShippingMethodFragment = Pick<ShippingMethod, 'id' | 'code' | 'description'> & {
+    calculator: Pick<ConfigurableOperation, 'code'>;
+    checker: Pick<ConfigurableOperation, 'code'>;
+};
+
 export type CreateAdministratorMutationVariables = Exact<{
     input: CreateAdministratorInput;
 }>;
@@ -4946,10 +4966,6 @@ export type GetOrderQueryVariables = Exact<{
 
 export type GetOrderQuery = { order?: Maybe<OrderWithLinesFragment> };
 
-export type CustomerGroupFragment = Pick<CustomerGroup, 'id' | 'name'> & {
-    customers: Pick<CustomerList, 'totalItems'> & { items: Array<Pick<Customer, 'id'>> };
-};
-
 export type CreateCustomerGroupMutationVariables = Exact<{
     input: CreateCustomerGroupInput;
 }>;
@@ -5191,23 +5207,12 @@ export type GetProductsWithVariantPricesQuery = {
     };
 };
 
-export type ProductOptionGroupFragment = Pick<ProductOptionGroup, 'id' | 'code' | 'name'> & {
-    options: Array<Pick<ProductOption, 'id' | 'code' | 'name'>>;
-    translations: Array<Pick<ProductOptionGroupTranslation, 'id' | 'languageCode' | 'name'>>;
-};
-
 export type CreateProductOptionGroupMutationVariables = Exact<{
     input: CreateProductOptionGroupInput;
 }>;
 
 export type CreateProductOptionGroupMutation = { createProductOptionGroup: ProductOptionGroupFragment };
 
-export type ProductWithOptionsFragment = Pick<Product, 'id'> & {
-    optionGroups: Array<
-        Pick<ProductOptionGroup, 'id' | 'code'> & { options: Array<Pick<ProductOption, 'id' | 'code'>> }
-    >;
-};
-
 export type AddOptionGroupToProductMutationVariables = Exact<{
     productId: Scalars['ID'];
     optionGroupId: Scalars['ID'];
@@ -5215,6 +5220,12 @@ export type AddOptionGroupToProductMutationVariables = Exact<{
 
 export type AddOptionGroupToProductMutation = { addOptionGroupToProduct: ProductWithOptionsFragment };
 
+export type CreateShippingMethodMutationVariables = Exact<{
+    input: CreateShippingMethodInput;
+}>;
+
+export type CreateShippingMethodMutation = { createShippingMethod: ShippingMethodFragment };
+
 export type UpdateOptionGroupMutationVariables = Exact<{
     input: UpdateProductOptionGroupInput;
 }>;
@@ -5449,11 +5460,6 @@ export type LogoutMutationVariables = Exact<{ [key: string]: never }>;
 
 export type LogoutMutation = { logout: Pick<Success, 'success'> };
 
-export type ShippingMethodFragment = Pick<ShippingMethod, 'id' | 'code' | 'description'> & {
-    calculator: Pick<ConfigurableOperation, 'code'>;
-    checker: Pick<ConfigurableOperation, 'code'>;
-};
-
 export type GetShippingMethodListQueryVariables = Exact<{ [key: string]: never }>;
 
 export type GetShippingMethodListQuery = {
@@ -5466,12 +5472,6 @@ export type GetShippingMethodQueryVariables = Exact<{
 
 export type GetShippingMethodQuery = { shippingMethod?: Maybe<ShippingMethodFragment> };
 
-export type CreateShippingMethodMutationVariables = Exact<{
-    input: CreateShippingMethodInput;
-}>;
-
-export type CreateShippingMethodMutation = { createShippingMethod: ShippingMethodFragment };
-
 export type UpdateShippingMethodMutationVariables = Exact<{
     input: UpdateShippingMethodInput;
 }>;
@@ -6534,6 +6534,36 @@ export namespace GlobalSettings {
     >;
 }
 
+export namespace CustomerGroup {
+    export type Fragment = CustomerGroupFragment;
+    export type Customers = NonNullable<CustomerGroupFragment['customers']>;
+    export type Items = NonNullable<
+        NonNullable<NonNullable<CustomerGroupFragment['customers']>['items']>[number]
+    >;
+}
+
+export namespace ProductOptionGroup {
+    export type Fragment = ProductOptionGroupFragment;
+    export type Options = NonNullable<NonNullable<ProductOptionGroupFragment['options']>[number]>;
+    export type Translations = NonNullable<NonNullable<ProductOptionGroupFragment['translations']>[number]>;
+}
+
+export namespace ProductWithOptions {
+    export type Fragment = ProductWithOptionsFragment;
+    export type OptionGroups = NonNullable<NonNullable<ProductWithOptionsFragment['optionGroups']>[number]>;
+    export type Options = NonNullable<
+        NonNullable<
+            NonNullable<NonNullable<ProductWithOptionsFragment['optionGroups']>[number]>['options']
+        >[number]
+    >;
+}
+
+export namespace ShippingMethod {
+    export type Fragment = ShippingMethodFragment;
+    export type Calculator = NonNullable<ShippingMethodFragment['calculator']>;
+    export type Checker = NonNullable<ShippingMethodFragment['checker']>;
+}
+
 export namespace CreateAdministrator {
     export type Variables = CreateAdministratorMutationVariables;
     export type Mutation = CreateAdministratorMutation;
@@ -6812,14 +6842,6 @@ export namespace GetOrder {
     export type Order = NonNullable<GetOrderQuery['order']>;
 }
 
-export namespace CustomerGroup {
-    export type Fragment = CustomerGroupFragment;
-    export type Customers = NonNullable<CustomerGroupFragment['customers']>;
-    export type Items = NonNullable<
-        NonNullable<NonNullable<CustomerGroupFragment['customers']>['items']>[number]
-    >;
-}
-
 export namespace CreateCustomerGroup {
     export type Variables = CreateCustomerGroupMutationVariables;
     export type Mutation = CreateCustomerGroupMutation;
@@ -7045,12 +7067,6 @@ export namespace GetProductsWithVariantPrices {
     >;
 }
 
-export namespace ProductOptionGroup {
-    export type Fragment = ProductOptionGroupFragment;
-    export type Options = NonNullable<NonNullable<ProductOptionGroupFragment['options']>[number]>;
-    export type Translations = NonNullable<NonNullable<ProductOptionGroupFragment['translations']>[number]>;
-}
-
 export namespace CreateProductOptionGroup {
     export type Variables = CreateProductOptionGroupMutationVariables;
     export type Mutation = CreateProductOptionGroupMutation;
@@ -7059,16 +7075,6 @@ export namespace CreateProductOptionGroup {
     >;
 }
 
-export namespace ProductWithOptions {
-    export type Fragment = ProductWithOptionsFragment;
-    export type OptionGroups = NonNullable<NonNullable<ProductWithOptionsFragment['optionGroups']>[number]>;
-    export type Options = NonNullable<
-        NonNullable<
-            NonNullable<NonNullable<ProductWithOptionsFragment['optionGroups']>[number]>['options']
-        >[number]
-    >;
-}
-
 export namespace AddOptionGroupToProduct {
     export type Variables = AddOptionGroupToProductMutationVariables;
     export type Mutation = AddOptionGroupToProductMutation;
@@ -7077,6 +7083,12 @@ export namespace AddOptionGroupToProduct {
     >;
 }
 
+export namespace CreateShippingMethod {
+    export type Variables = CreateShippingMethodMutationVariables;
+    export type Mutation = CreateShippingMethodMutation;
+    export type CreateShippingMethod = NonNullable<CreateShippingMethodMutation['createShippingMethod']>;
+}
+
 export namespace UpdateOptionGroup {
     export type Variables = UpdateOptionGroupMutationVariables;
     export type Mutation = UpdateOptionGroupMutation;
@@ -7320,12 +7332,6 @@ export namespace Logout {
     export type Logout = NonNullable<LogoutMutation['logout']>;
 }
 
-export namespace ShippingMethod {
-    export type Fragment = ShippingMethodFragment;
-    export type Calculator = NonNullable<ShippingMethodFragment['calculator']>;
-    export type Checker = NonNullable<ShippingMethodFragment['checker']>;
-}
-
 export namespace GetShippingMethodList {
     export type Variables = GetShippingMethodListQueryVariables;
     export type Query = GetShippingMethodListQuery;
@@ -7341,12 +7347,6 @@ export namespace GetShippingMethod {
     export type ShippingMethod = NonNullable<GetShippingMethodQuery['shippingMethod']>;
 }
 
-export namespace CreateShippingMethod {
-    export type Variables = CreateShippingMethodMutationVariables;
-    export type Mutation = CreateShippingMethodMutation;
-    export type CreateShippingMethod = NonNullable<CreateShippingMethodMutation['createShippingMethod']>;
-}
-
 export namespace UpdateShippingMethod {
     export type Variables = UpdateShippingMethodMutationVariables;
     export type Mutation = UpdateShippingMethodMutation;

+ 12 - 2
packages/core/e2e/graphql/generated-e2e-shop-types.ts

@@ -449,6 +449,7 @@ export enum ErrorCode {
     ORDER_LIMIT_ERROR = 'ORDER_LIMIT_ERROR',
     NEGATIVE_QUANTITY_ERROR = 'NEGATIVE_QUANTITY_ERROR',
     INSUFFICIENT_STOCK_ERROR = 'INSUFFICIENT_STOCK_ERROR',
+    INELIGIBLE_SHIPPING_METHOD_ERROR = 'INELIGIBLE_SHIPPING_METHOD_ERROR',
     ORDER_PAYMENT_STATE_ERROR = 'ORDER_PAYMENT_STATE_ERROR',
     PAYMENT_FAILED_ERROR = 'PAYMENT_FAILED_ERROR',
     PAYMENT_DECLINED_ERROR = 'PAYMENT_DECLINED_ERROR',
@@ -1413,6 +1414,12 @@ export type InsufficientStockError = ErrorResult & {
     order: Order;
 };
 
+/** Returned when attempting to set a ShippingMethod for which the order is not eligible */
+export type IneligibleShippingMethodError = ErrorResult & {
+    errorCode: ErrorCode;
+    message: Scalars['String'];
+};
+
 /** Returned when attempting to add a Payment to an Order that is not in the `ArrangingPayment` state. */
 export type OrderPaymentStateError = ErrorResult & {
     errorCode: ErrorCode;
@@ -1545,7 +1552,7 @@ export type UpdateOrderItemsResult =
 
 export type RemoveOrderItemsResult = Order | OrderModificationError;
 
-export type SetOrderShippingMethodResult = Order | OrderModificationError;
+export type SetOrderShippingMethodResult = Order | OrderModificationError | IneligibleShippingMethodError;
 
 export type ApplyCouponCodeResult =
     | Order
@@ -2914,7 +2921,10 @@ export type SetShippingMethodMutationVariables = Exact<{
 }>;
 
 export type SetShippingMethodMutation = {
-    setOrderShippingMethod: TestOrderFragmentFragment | Pick<OrderModificationError, 'errorCode' | 'message'>;
+    setOrderShippingMethod:
+        | TestOrderFragmentFragment
+        | Pick<OrderModificationError, 'errorCode' | 'message'>
+        | Pick<IneligibleShippingMethodError, 'errorCode' | 'message'>;
 };
 
 export type ActiveOrderCustomerFragment = Pick<Order, 'id'> & {

+ 13 - 45
packages/core/e2e/graphql/shared-definitions.ts

@@ -8,15 +8,19 @@ import {
     COUNTRY_FRAGMENT,
     CURRENT_USER_FRAGMENT,
     CUSTOMER_FRAGMENT,
+    CUSTOMER_GROUP_FRAGMENT,
     FACET_WITH_VALUES_FRAGMENT,
     FULFILLMENT_FRAGMENT,
     GLOBAL_SETTINGS_FRAGMENT,
     ORDER_FRAGMENT,
     ORDER_WITH_LINES_FRAGMENT,
+    PRODUCT_OPTION_GROUP_FRAGMENT,
     PRODUCT_VARIANT_FRAGMENT,
+    PRODUCT_WITH_OPTIONS_FRAGMENT,
     PRODUCT_WITH_VARIANTS_FRAGMENT,
     PROMOTION_FRAGMENT,
     ROLE_FRAGMENT,
+    SHIPPING_METHOD_FRAGMENT,
     TAX_RATE_FRAGMENT,
     VARIANT_WITH_STOCK_FRAGMENT,
 } from './fragments';
@@ -413,19 +417,6 @@ export const GET_ORDER = gql`
     ${ORDER_WITH_LINES_FRAGMENT}
 `;
 
-export const CUSTOMER_GROUP_FRAGMENT = gql`
-    fragment CustomerGroup on CustomerGroup {
-        id
-        name
-        customers {
-            items {
-                id
-            }
-            totalItems
-        }
-    }
-`;
-
 export const CREATE_CUSTOMER_GROUP = gql`
     mutation CreateCustomerGroup($input: CreateCustomerGroupInput!) {
         createCustomerGroup(input: $input) {
@@ -736,24 +727,6 @@ export const GET_PRODUCTS_WITH_VARIANT_PRICES = gql`
     }
 `;
 
-export const PRODUCT_OPTION_GROUP_FRAGMENT = gql`
-    fragment ProductOptionGroup on ProductOptionGroup {
-        id
-        code
-        name
-        options {
-            id
-            code
-            name
-        }
-        translations {
-            id
-            languageCode
-            name
-        }
-    }
-`;
-
 export const CREATE_PRODUCT_OPTION_GROUP = gql`
     mutation CreateProductOptionGroup($input: CreateProductOptionGroupInput!) {
         createProductOptionGroup(input: $input) {
@@ -763,20 +736,6 @@ export const CREATE_PRODUCT_OPTION_GROUP = gql`
     ${PRODUCT_OPTION_GROUP_FRAGMENT}
 `;
 
-export const PRODUCT_WITH_OPTIONS_FRAGMENT = gql`
-    fragment ProductWithOptions on Product {
-        id
-        optionGroups {
-            id
-            code
-            options {
-                id
-                code
-            }
-        }
-    }
-`;
-
 export const ADD_OPTION_GROUP_TO_PRODUCT = gql`
     mutation AddOptionGroupToProduct($productId: ID!, $optionGroupId: ID!) {
         addOptionGroupToProduct(productId: $productId, optionGroupId: $optionGroupId) {
@@ -785,3 +744,12 @@ export const ADD_OPTION_GROUP_TO_PRODUCT = gql`
     }
     ${PRODUCT_WITH_OPTIONS_FRAGMENT}
 `;
+
+export const CREATE_SHIPPING_METHOD = gql`
+    mutation CreateShippingMethod($input: CreateShippingMethodInput!) {
+        createShippingMethod(input: $input) {
+            ...ShippingMethod
+        }
+    }
+    ${SHIPPING_METHOD_FRAGMENT}
+`;

+ 2 - 1
packages/core/e2e/product-option.e2e-spec.ts

@@ -6,6 +6,7 @@ import { initialData } from '../../../e2e-common/e2e-initial-data';
 import { testConfig, TEST_SETUP_TIMEOUT_MS } from '../../../e2e-common/test-config';
 import { omit } from '../../common/lib/omit';
 
+import { PRODUCT_OPTION_GROUP_FRAGMENT } from './graphql/fragments';
 import {
     CreateProductOption,
     CreateProductOptionGroup,
@@ -14,7 +15,7 @@ import {
     UpdateProductOption,
     UpdateProductOptionGroup,
 } from './graphql/generated-e2e-admin-types';
-import { CREATE_PRODUCT_OPTION_GROUP, PRODUCT_OPTION_GROUP_FRAGMENT } from './graphql/shared-definitions';
+import { CREATE_PRODUCT_OPTION_GROUP } from './graphql/shared-definitions';
 import { assertThrowsWithMessage } from './utils/assert-throws-with-message';
 
 // tslint:disable:no-non-null-assertion

+ 1 - 1
packages/core/e2e/product.e2e-spec.ts

@@ -8,6 +8,7 @@ import path from 'path';
 import { initialData } from '../../../e2e-common/e2e-initial-data';
 import { testConfig, TEST_SETUP_TIMEOUT_MS } from '../../../e2e-common/test-config';
 
+import { PRODUCT_WITH_OPTIONS_FRAGMENT } from './graphql/fragments';
 import {
     AddOptionGroupToProduct,
     CreateProduct,
@@ -41,7 +42,6 @@ import {
     GET_PRODUCT_LIST,
     GET_PRODUCT_SIMPLE,
     GET_PRODUCT_WITH_VARIANTS,
-    PRODUCT_WITH_OPTIONS_FRAGMENT,
     UPDATE_PRODUCT,
     UPDATE_PRODUCT_VARIANTS,
 } from './graphql/shared-definitions';

+ 292 - 0
packages/core/e2e/shipping-method-eligibility.e2e-spec.ts

@@ -0,0 +1,292 @@
+import {
+    defaultShippingCalculator,
+    defaultShippingEligibilityChecker,
+    ShippingCalculator,
+    ShippingEligibilityChecker,
+} from '@vendure/core';
+import { createErrorResultGuard, createTestEnvironment, ErrorResultGuard } from '@vendure/testing';
+import path from 'path';
+
+import { initialData } from '../../../e2e-common/e2e-initial-data';
+import { testConfig, TEST_SETUP_TIMEOUT_MS } from '../../../e2e-common/test-config';
+
+import { CreateShippingMethod, ShippingMethodFragment } from './graphql/generated-e2e-admin-types';
+import {
+    AddItemToOrder,
+    AdjustItemQuantity,
+    ErrorCode,
+    GetActiveOrder,
+    GetShippingMethods,
+    RemoveItemFromOrder,
+    SetCustomerForOrder,
+    SetShippingMethod,
+    TestOrderFragmentFragment,
+    UpdatedOrderFragment,
+} from './graphql/generated-e2e-shop-types';
+import { CREATE_SHIPPING_METHOD } from './graphql/shared-definitions';
+import {
+    ADD_ITEM_TO_ORDER,
+    ADJUST_ITEM_QUANTITY,
+    GET_ACTIVE_ORDER,
+    GET_ELIGIBLE_SHIPPING_METHODS,
+    REMOVE_ITEM_FROM_ORDER,
+    SET_CUSTOMER,
+    SET_SHIPPING_METHOD,
+} from './graphql/shop-definitions';
+
+const check1Spy = jest.fn();
+const checker1 = new ShippingEligibilityChecker({
+    code: 'checker1',
+    description: [],
+    args: {},
+    check: (ctx, order) => {
+        check1Spy();
+        return order.lines.length === 1;
+    },
+});
+
+const check2Spy = jest.fn();
+const checker2 = new ShippingEligibilityChecker({
+    code: 'checker2',
+    description: [],
+    args: {},
+    check: (ctx, order) => {
+        check2Spy();
+        return order.lines.length > 1;
+    },
+});
+
+const calculator = new ShippingCalculator({
+    code: 'calculator',
+    description: [],
+    args: {},
+    calculate: ctx => {
+        return {
+            price: 10,
+            priceWithTax: 12,
+        };
+    },
+});
+
+describe('ShippingMethod resolver', () => {
+    const { server, adminClient, shopClient } = createTestEnvironment({
+        ...testConfig,
+        shippingOptions: {
+            shippingEligibilityCheckers: [defaultShippingEligibilityChecker, checker1, checker2],
+            shippingCalculators: [defaultShippingCalculator, calculator],
+        },
+    });
+
+    const orderGuard: ErrorResultGuard<
+        UpdatedOrderFragment | TestOrderFragmentFragment
+    > = createErrorResultGuard<UpdatedOrderFragment>(input => !!input.lines);
+
+    let order: UpdatedOrderFragment;
+    let singleLineShippingMethod: ShippingMethodFragment;
+    let multiLineShippingMethod: ShippingMethodFragment;
+
+    beforeAll(async () => {
+        await server.init({
+            initialData: {
+                ...initialData,
+                shippingMethods: [],
+            },
+            productsCsvPath: path.join(__dirname, 'fixtures/e2e-products-full.csv'),
+            customerCount: 1,
+        });
+        await adminClient.asSuperAdmin();
+
+        const result1 = await adminClient.query<
+            CreateShippingMethod.Mutation,
+            CreateShippingMethod.Variables
+        >(CREATE_SHIPPING_METHOD, {
+            input: {
+                code: 'single-line',
+                description: 'For single-line orders',
+                checker: {
+                    code: checker1.code,
+                    arguments: [],
+                },
+                calculator: {
+                    code: calculator.code,
+                    arguments: [],
+                },
+            },
+        });
+        singleLineShippingMethod = result1.createShippingMethod;
+
+        const result2 = await adminClient.query<
+            CreateShippingMethod.Mutation,
+            CreateShippingMethod.Variables
+        >(CREATE_SHIPPING_METHOD, {
+            input: {
+                code: 'multi-line',
+                description: 'For multi-line orders',
+                checker: {
+                    code: checker2.code,
+                    arguments: [],
+                },
+                calculator: {
+                    code: calculator.code,
+                    arguments: [],
+                },
+            },
+        });
+        multiLineShippingMethod = result2.createShippingMethod;
+    }, TEST_SETUP_TIMEOUT_MS);
+
+    afterAll(async () => {
+        await server.destroy();
+    });
+
+    it('Does not run checkers before a ShippingMethod is assigned to Order', async () => {
+        check1Spy.mockClear();
+        check2Spy.mockClear();
+
+        await shopClient.asUserWithCredentials('hayden.zieme12@hotmail.com', 'test');
+
+        const { addItemToOrder } = await shopClient.query<AddItemToOrder.Mutation, AddItemToOrder.Variables>(
+            ADD_ITEM_TO_ORDER,
+            {
+                quantity: 1,
+                productVariantId: 'T_1',
+            },
+        );
+        orderGuard.assertSuccess(addItemToOrder);
+
+        expect(check1Spy).not.toHaveBeenCalled();
+        expect(check2Spy).not.toHaveBeenCalled();
+
+        await shopClient.query<AdjustItemQuantity.Mutation, AdjustItemQuantity.Variables>(
+            ADJUST_ITEM_QUANTITY,
+            {
+                quantity: 2,
+                orderLineId: addItemToOrder.lines[0].id,
+            },
+        );
+
+        expect(check1Spy).not.toHaveBeenCalled();
+        expect(check2Spy).not.toHaveBeenCalled();
+
+        order = addItemToOrder;
+    });
+
+    it('Runs checkers when querying for eligible ShippingMethods', async () => {
+        check1Spy.mockClear();
+        check2Spy.mockClear();
+
+        const { eligibleShippingMethods } = await shopClient.query<GetShippingMethods.Query>(
+            GET_ELIGIBLE_SHIPPING_METHODS,
+        );
+
+        expect(check1Spy).toHaveBeenCalledTimes(1);
+        expect(check2Spy).toHaveBeenCalledTimes(1);
+    });
+
+    it('Runs checker of assigned method only', async () => {
+        check1Spy.mockClear();
+        check2Spy.mockClear();
+
+        await shopClient.query<SetShippingMethod.Mutation, SetShippingMethod.Variables>(SET_SHIPPING_METHOD, {
+            id: singleLineShippingMethod.id,
+        });
+
+        // A check is done when assigning the method to ensure it
+        // is eligible, and again when calculating order adjustments
+        expect(check1Spy).toHaveBeenCalledTimes(2);
+        expect(check2Spy).not.toHaveBeenCalled();
+
+        await shopClient.query<AdjustItemQuantity.Mutation, AdjustItemQuantity.Variables>(
+            ADJUST_ITEM_QUANTITY,
+            {
+                quantity: 3,
+                orderLineId: order.lines[0].id,
+            },
+        );
+
+        expect(check1Spy).toHaveBeenCalledTimes(3);
+        expect(check2Spy).not.toHaveBeenCalled();
+
+        await shopClient.query<AdjustItemQuantity.Mutation, AdjustItemQuantity.Variables>(
+            ADJUST_ITEM_QUANTITY,
+            {
+                quantity: 4,
+                orderLineId: order.lines[0].id,
+            },
+        );
+
+        expect(check1Spy).toHaveBeenCalledTimes(4);
+        expect(check2Spy).not.toHaveBeenCalled();
+    });
+
+    it('Prevents ineligible method from being assigned', async () => {
+        const { setOrderShippingMethod } = await shopClient.query<
+            SetShippingMethod.Mutation,
+            SetShippingMethod.Variables
+        >(SET_SHIPPING_METHOD, {
+            id: multiLineShippingMethod.id,
+        });
+
+        orderGuard.assertErrorResult(setOrderShippingMethod);
+
+        expect(setOrderShippingMethod.errorCode).toBe(ErrorCode.INELIGIBLE_SHIPPING_METHOD_ERROR);
+        expect(setOrderShippingMethod.message).toBe(
+            'This Order is not eligible for the selected ShippingMethod',
+        );
+    });
+
+    it('Runs checks when assigned method becomes ineligible', async () => {
+        check1Spy.mockClear();
+        check2Spy.mockClear();
+
+        // Adding a second OrderLine will make the singleLineShippingMethod
+        // ineligible
+        const { addItemToOrder } = await shopClient.query<AddItemToOrder.Mutation, AddItemToOrder.Variables>(
+            ADD_ITEM_TO_ORDER,
+            {
+                quantity: 1,
+                productVariantId: 'T_2',
+            },
+        );
+        orderGuard.assertSuccess(addItemToOrder);
+
+        // Checked once to see if still eligible (no)
+        expect(check1Spy).toHaveBeenCalledTimes(1);
+        // Checked once when looking for a fallback
+        expect(check2Spy).toHaveBeenCalledTimes(1);
+
+        const { activeOrder } = await shopClient.query<GetActiveOrder.Query>(GET_ACTIVE_ORDER);
+        // multiLineShippingMethod assigned as a fallback
+        expect(activeOrder?.shippingMethod?.id).toBe(multiLineShippingMethod.id);
+
+        await shopClient.query<AdjustItemQuantity.Mutation, AdjustItemQuantity.Variables>(
+            ADJUST_ITEM_QUANTITY,
+            {
+                quantity: 2,
+                orderLineId: addItemToOrder.lines[1].id,
+            },
+        );
+
+        // No longer called as singleLineShippingMethod not assigned
+        expect(check1Spy).toHaveBeenCalledTimes(1);
+        // Called on changes since multiLineShippingMethod is assigned
+        expect(check2Spy).toHaveBeenCalledTimes(2);
+
+        // Remove the second OrderLine and make multiLineShippingMethod ineligible
+        const { removeOrderLine } = await shopClient.query<
+            RemoveItemFromOrder.Mutation,
+            RemoveItemFromOrder.Variables
+        >(REMOVE_ITEM_FROM_ORDER, {
+            orderLineId: addItemToOrder.lines[1].id,
+        });
+        orderGuard.assertSuccess(removeOrderLine);
+
+        // Called when looking for a fallback
+        expect(check1Spy).toHaveBeenCalledTimes(2);
+        // Called when checking if still eligibile (no)
+        expect(check2Spy).toHaveBeenCalledTimes(3);
+
+        // Falls back to the first eligible shipping method
+        expect(removeOrderLine.shippingMethod?.id).toBe(singleLineShippingMethod.id);
+    });
+});

+ 2 - 23
packages/core/e2e/shipping-method.e2e-spec.ts

@@ -11,6 +11,7 @@ import path from 'path';
 import { initialData } from '../../../e2e-common/e2e-initial-data';
 import { testConfig, TEST_SETUP_TIMEOUT_MS } from '../../../e2e-common/test-config';
 
+import { SHIPPING_METHOD_FRAGMENT } from './graphql/fragments';
 import {
     CreateShippingMethod,
     DeleteShippingMethod,
@@ -24,6 +25,7 @@ import {
     TestShippingMethod,
     UpdateShippingMethod,
 } from './graphql/generated-e2e-admin-types';
+import { CREATE_SHIPPING_METHOD } from './graphql/shared-definitions';
 
 const TEST_METADATA = {
     foo: 'bar',
@@ -294,20 +296,6 @@ describe('ShippingMethod resolver', () => {
     });
 });
 
-const SHIPPING_METHOD_FRAGMENT = gql`
-    fragment ShippingMethod on ShippingMethod {
-        id
-        code
-        description
-        calculator {
-            code
-        }
-        checker {
-            code
-        }
-    }
-`;
-
 const GET_SHIPPING_METHOD_LIST = gql`
     query GetShippingMethodList {
         shippingMethods {
@@ -329,15 +317,6 @@ const GET_SHIPPING_METHOD = gql`
     ${SHIPPING_METHOD_FRAGMENT}
 `;
 
-const CREATE_SHIPPING_METHOD = gql`
-    mutation CreateShippingMethod($input: CreateShippingMethodInput!) {
-        createShippingMethod(input: $input) {
-            ...ShippingMethod
-        }
-    }
-    ${SHIPPING_METHOD_FRAGMENT}
-`;
-
 const UPDATE_SHIPPING_METHOD = gql`
     mutation UpdateShippingMethod($input: UpdateShippingMethodInput!) {
         updateShippingMethod(input: $input) {

+ 2 - 0
packages/core/src/service/helpers/order-calculator/order-calculator.spec.ts

@@ -16,6 +16,7 @@ import { Order } from '../../../entity/order/order.entity';
 import { TaxCategory } from '../../../entity/tax-category/tax-category.entity';
 import { EventBus } from '../../../event-bus/event-bus';
 import { WorkerService } from '../../../worker/worker.service';
+import { ShippingMethodService } from '../../services/shipping-method.service';
 import { TaxRateService } from '../../services/tax-rate.service';
 import { ZoneService } from '../../services/zone.service';
 import { TransactionalConnection } from '../../transaction/transactional-connection';
@@ -40,6 +41,7 @@ describe('OrderCalculator', () => {
                 TaxCalculator,
                 TaxRateService,
                 { provide: ShippingCalculator, useValue: { getEligibleShippingMethods: () => [] } },
+                { provide: ShippingMethodService, useValue: { findOne: () => undefined } },
                 { provide: TransactionalConnection, useClass: MockConnection },
                 { provide: ListQueryBuilder, useValue: {} },
                 { provide: ConfigService, useClass: MockConfigService },

+ 23 - 9
packages/core/src/service/helpers/order-calculator/order-calculator.ts

@@ -12,6 +12,7 @@ import { Order } from '../../../entity/order/order.entity';
 import { Promotion } from '../../../entity/promotion/promotion.entity';
 import { ShippingMethod } from '../../../entity/shipping-method/shipping-method.entity';
 import { Zone } from '../../../entity/zone/zone.entity';
+import { ShippingMethodService } from '../../services/shipping-method.service';
 import { TaxRateService } from '../../services/tax-rate.service';
 import { ZoneService } from '../../services/zone.service';
 import { TransactionalConnection } from '../../transaction/transactional-connection';
@@ -26,6 +27,7 @@ export class OrderCalculator {
         private zoneService: ZoneService,
         private taxRateService: TaxRateService,
         private taxCalculator: TaxCalculator,
+        private shippingMethodService: ShippingMethodService,
         private shippingCalculator: ShippingCalculator,
     ) {}
 
@@ -263,16 +265,28 @@ export class OrderCalculator {
     }
 
     private async applyShipping(ctx: RequestContext, order: Order) {
-        const results = await this.shippingCalculator.getEligibleShippingMethods(ctx, order);
-        const currentShippingMethod = order.shippingMethod;
-        if (results && results.length && currentShippingMethod) {
-            let selected: { method: ShippingMethod; result: ShippingCalculationResult } | undefined;
-            selected = results.find(r => idsAreEqual(r.method.id, currentShippingMethod.id));
-            if (!selected) {
-                selected = results[0];
+        const currentShippingMethod =
+            order.shippingMethodId && (await this.shippingMethodService.findOne(ctx, order.shippingMethodId));
+        if (!currentShippingMethod) {
+            return;
+        }
+        const currentMethodStillEligible = await currentShippingMethod.test(ctx, order);
+        if (currentMethodStillEligible) {
+            const result = await currentShippingMethod.apply(ctx, order);
+            if (result) {
+                order.shipping = result.price;
+                order.shippingWithTax = result.priceWithTax;
             }
-            order.shipping = selected.result.price;
-            order.shippingWithTax = selected.result.priceWithTax;
+            return;
+        }
+        const results = await this.shippingCalculator.getEligibleShippingMethods(ctx, order, [
+            currentShippingMethod.id,
+        ]);
+        if (results && results.length) {
+            const cheapest = results[0];
+            order.shipping = cheapest.result.price;
+            order.shippingWithTax = cheapest.result.priceWithTax;
+            order.shippingMethod = cheapest.method;
         }
     }
 

+ 10 - 2
packages/core/src/service/helpers/shipping-calculator/shipping-calculator.ts

@@ -20,9 +20,17 @@ export class ShippingCalculator {
     /**
      * Returns an array of each eligible ShippingMethod for the given Order and sorts them by
      * price, with the cheapest first.
+     *
+     * The `skipIds` argument is used to skip ShippingMethods with those IDs from being checked and calculated.
      */
-    async getEligibleShippingMethods(ctx: RequestContext, order: Order): Promise<EligibleShippingMethod[]> {
-        const shippingMethods = this.shippingMethodService.getActiveShippingMethods(ctx.channel);
+    async getEligibleShippingMethods(
+        ctx: RequestContext,
+        order: Order,
+        skipIds: ID[] = [],
+    ): Promise<EligibleShippingMethod[]> {
+        const shippingMethods = this.shippingMethodService
+            .getActiveShippingMethods(ctx.channel)
+            .filter(method => !skipIds.includes(method.id));
 
         const checkEligibilityPromises = shippingMethods.map(method =>
             this.checkEligibilityByShippingMethod(ctx, order, method),