diff --git a/packages/core/e2e/fixtures/test-shipping-eligibility-checkers.ts b/packages/core/e2e/fixtures/test-shipping-eligibility-checkers.ts index 79680d0872..d06510119c 100644 --- a/packages/core/e2e/fixtures/test-shipping-eligibility-checkers.ts +++ b/packages/core/e2e/fixtures/test-shipping-eligibility-checkers.ts @@ -1,5 +1,5 @@ import { LanguageCode } from '@vendure/common/lib/generated-types'; -import { ShippingEligibilityChecker } from '@vendure/core'; +import { EntityHydrator, ShippingEligibilityChecker } from '@vendure/core'; export const countryCodeShippingEligibilityChecker = new ShippingEligibilityChecker({ code: 'country-code-shipping-eligibility-checker', @@ -13,3 +13,28 @@ export const countryCodeShippingEligibilityChecker = new ShippingEligibilityChec return order.shippingAddress?.countryCode === args.countryCode; }, }); + +let entityHydrator: EntityHydrator; + +/** + * @description + * This checker does nothing except for hydrating the Order in order to test + * an edge-case which would cause inconsistencies when modifying the Order in which + * the OrderService would e.g. remove an OrderLine, but then this step would re-add it + * because the removal had not yet been persisted by the time the `applyPriceAdjustments()` + * step was run (during which this checker will run). + * + * See https://github.com/vendure-ecommerce/vendure/issues/2548 + */ +export const hydratingShippingEligibilityChecker = new ShippingEligibilityChecker({ + code: 'hydrating-shipping-eligibility-checker', + description: [{ languageCode: LanguageCode.en, value: 'Hydrating Shipping Eligibility Checker' }], + args: {}, + init(injector) { + entityHydrator = injector.get(EntityHydrator); + }, + check: async (ctx, order) => { + await entityHydrator.hydrate(ctx, order, { relations: ['lines.sellerChannel'] }); + return true; + }, +}); diff --git a/packages/core/e2e/shop-order.e2e-spec.ts b/packages/core/e2e/shop-order.e2e-spec.ts index 0ad60c7740..78a3a94d52 100644 --- a/packages/core/e2e/shop-order.e2e-spec.ts +++ b/packages/core/e2e/shop-order.e2e-spec.ts @@ -21,14 +21,18 @@ import { testFailingPaymentMethod, testSuccessfulPaymentMethod, } from './fixtures/test-payment-methods'; -import { countryCodeShippingEligibilityChecker } from './fixtures/test-shipping-eligibility-checkers'; +import { + countryCodeShippingEligibilityChecker, + hydratingShippingEligibilityChecker, +} from './fixtures/test-shipping-eligibility-checkers'; import { CreateAddressInput, + CreateShippingMethodDocument, CreateShippingMethodInput, LanguageCode, } from './graphql/generated-e2e-admin-types'; import * as Codegen from './graphql/generated-e2e-admin-types'; -import { ErrorCode } from './graphql/generated-e2e-shop-types'; +import { ErrorCode, RemoveItemFromOrderDocument } from './graphql/generated-e2e-shop-types'; import * as CodegenShop from './graphql/generated-e2e-shop-types'; import { ATTEMPT_LOGIN, @@ -83,6 +87,7 @@ describe('Shop orders', () => { shippingEligibilityCheckers: [ defaultShippingEligibilityChecker, countryCodeShippingEligibilityChecker, + hydratingShippingEligibilityChecker, ], }, customFields: { @@ -2226,12 +2231,12 @@ describe('Shop orders', () => { id: minPriceShippingMethodId, }, ); - const result1 = await shopClient.query(GET_ACTIVE_ORDER); + const result1 = await shopClient.query(GET_ACTIVE_ORDER); expect(result1.activeOrder?.shippingLines[0].shippingMethod.id).toBe(minPriceShippingMethodId); const { removeAllOrderLines } = await shopClient.query< - RemoveAllOrderLines.Mutation, - RemoveAllOrderLines.Variables + CodegenShop.RemoveAllOrderLinesMutation, + CodegenShop.RemoveAllOrderLinesMutationVariables >(REMOVE_ALL_ORDER_LINES); orderResultGuard.assertSuccess(removeAllOrderLines); expect(removeAllOrderLines.shippingLines.length).toBe(0); @@ -2306,6 +2311,77 @@ describe('Shop orders', () => { expect(activeOrder.shippingAddress).toEqual(shippingAddress); expect(activeOrder.billingAddress).toEqual(billingAddress); }); + + // https://github.com/vendure-ecommerce/vendure/issues/2548 + it('hydrating Order in the ShippingEligibilityChecker does not break order modification', async () => { + // First we'll create a ShippingMethod that uses the hydrating checker + const { createShippingMethod } = await adminClient.query(CreateShippingMethodDocument, { + input: { + code: 'hydrating-checker', + translations: [ + { languageCode: LanguageCode.en, name: 'hydrating checker', description: '' }, + ], + fulfillmentHandler: manualFulfillmentHandler.code, + checker: { + code: hydratingShippingEligibilityChecker.code, + arguments: [], + }, + calculator: { + code: defaultShippingCalculator.code, + arguments: [ + { name: 'rate', value: '1000' }, + { name: 'taxRate', value: '0' }, + { name: 'includesTax', value: 'auto' }, + ], + }, + }, + }); + + await shopClient.asAnonymousUser(); + await shopClient.query< + CodegenShop.AddItemToOrderMutation, + CodegenShop.AddItemToOrderMutationVariables + >(ADD_ITEM_TO_ORDER, { + productVariantId: 'T_1', + quantity: 1, + }); + await shopClient.query< + CodegenShop.AddItemToOrderMutation, + CodegenShop.AddItemToOrderMutationVariables + >(ADD_ITEM_TO_ORDER, { + productVariantId: 'T_2', + quantity: 3, + }); + + const result1 = await shopClient.query(GET_ACTIVE_ORDER); + + expect(result1.activeOrder?.lines.map(l => l.linePriceWithTax).sort()).toEqual([155880, 503640]); + expect(result1.activeOrder?.subTotalWithTax).toBe(659520); + + // set the shipping method that uses the hydrating checker + const { eligibleShippingMethods } = await shopClient.query( + GET_ELIGIBLE_SHIPPING_METHODS, + ); + const { setOrderShippingMethod } = await shopClient.query< + CodegenShop.SetShippingMethodMutation, + CodegenShop.SetShippingMethodMutationVariables + >(SET_SHIPPING_METHOD, { + id: eligibleShippingMethods.find(m => m.code === 'hydrating-checker')!.id, + }); + orderResultGuard.assertSuccess(setOrderShippingMethod); + + // Remove an item from the order + const { removeOrderLine } = await shopClient.query(RemoveItemFromOrderDocument, { + orderLineId: result1.activeOrder!.lines[0].id, + }); + orderResultGuard.assertSuccess(removeOrderLine); + expect(removeOrderLine.lines.length).toBe(1); + + const result2 = await shopClient.query(GET_ACTIVE_ORDER); + + expect(result2.activeOrder?.lines.map(l => l.linePriceWithTax).sort()).toEqual([503640]); + expect(result2.activeOrder?.subTotalWithTax).toBe(503640); + }); }); }); diff --git a/packages/core/src/service/services/order.service.ts b/packages/core/src/service/services/order.service.ts index 0604c0feda..27781de8dd 100644 --- a/packages/core/src/service/services/order.service.ts +++ b/packages/core/src/service/services/order.service.ts @@ -620,6 +620,11 @@ export class OrderService { } const orderLine = this.getOrderLineOrThrow(order, orderLineId); order.lines = order.lines.filter(line => !idsAreEqual(line.id, orderLineId)); + // Persist the orderLine removal before applying price adjustments + // so that any hydration of the Order entity during the course of the + // `applyPriceAdjustments()` (e.g. in a ShippingEligibilityChecker etc) + // will not re-add the OrderLine. + await this.connection.getRepository(ctx, Order).save(order, { reload: false }); const updatedOrder = await this.applyPriceAdjustments(ctx, order); const deletedOrderLine = new OrderLine(orderLine); await this.connection.getRepository(ctx, OrderLine).remove(orderLine);