fix(core): Correctly set OrderItem prices on tax zone change

Fixes #1216

e2e-common/test-config.ts

@@ -42,6 +42,8 @@ if (process.env.E2E_DEBUG) {
     // tslint:disable-next-line:no-console
     console.log('E2E_DEBUG', process.env.E2E_DEBUG, ' - setting long timeout');
     jest.setTimeout(1800 * 1000);
+} else {
+    jest.setTimeout(15 * 1000);
 }
 /**
  * Increase default timeout in CI because occasionally valid tests fail due to
@@ -49,8 +51,6 @@ if (process.env.E2E_DEBUG) {
  */
 if (process.env.CI) {
     jest.setTimeout(30 * 1000);
-} else {
-    jest.setTimeout(15 * 1000);
 }
 
 export const testConfig = () => {

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

@@ -6405,6 +6405,21 @@ export type DeletePromotionAdHoc1MutationVariables = Exact<{ [key: string]: neve
 
 export type DeletePromotionAdHoc1Mutation = { deletePromotion: Pick<DeletionResponse, 'result'> };
 
+export type GetTaxRateListQueryVariables = Exact<{
+    options?: Maybe<TaxRateListOptions>;
+}>;
+
+export type GetTaxRateListQuery = {
+    taxRates: Pick<TaxRateList, 'totalItems'> & {
+        items: Array<
+            Pick<TaxRate, 'id' | 'name' | 'enabled' | 'value'> & {
+                category: Pick<TaxCategory, 'id' | 'name'>;
+                zone: Pick<Zone, 'id' | 'name'>;
+            }
+        >;
+    };
+};
+
 export type GetOrderListFulfillmentsQueryVariables = Exact<{ [key: string]: never }>;
 
 export type GetOrderListFulfillmentsQuery = {
@@ -8738,6 +8753,21 @@ export namespace DeletePromotionAdHoc1 {
     export type DeletePromotion = NonNullable<DeletePromotionAdHoc1Mutation['deletePromotion']>;
 }
 
+export namespace GetTaxRateList {
+    export type Variables = GetTaxRateListQueryVariables;
+    export type Query = GetTaxRateListQuery;
+    export type TaxRates = NonNullable<GetTaxRateListQuery['taxRates']>;
+    export type Items = NonNullable<
+        NonNullable<NonNullable<GetTaxRateListQuery['taxRates']>['items']>[number]
+    >;
+    export type Category = NonNullable<
+        NonNullable<NonNullable<NonNullable<GetTaxRateListQuery['taxRates']>['items']>[number]>['category']
+    >;
+    export type Zone = NonNullable<
+        NonNullable<NonNullable<NonNullable<GetTaxRateListQuery['taxRates']>['items']>[number]>['zone']
+    >;
+}
+
 export namespace GetOrderListFulfillments {
     export type Variables = GetOrderListFulfillmentsQueryVariables;
     export type Query = GetOrderListFulfillmentsQuery;

packages/core/e2e/order-taxes.e2e-spec.ts

@@ -1,26 +1,96 @@
 /* tslint:disable:no-non-null-assertion */
 import { summate } from '@vendure/common/lib/shared-utils';
+import {
+    Channel,
+    Injector,
+    Order,
+    RequestContext,
+    TaxZoneStrategy,
+    TransactionalConnection,
+    Zone,
+} from '@vendure/core';
 import { createErrorResultGuard, createTestEnvironment, ErrorResultGuard } from '@vendure/testing';
+import gql from 'graphql-tag';
 import path from 'path';
 
 import { initialData } from '../../../e2e-common/e2e-initial-data';
 import { testConfig, TEST_SETUP_TIMEOUT_MS } from '../../../e2e-common/test-config';
 
 import { testSuccessfulPaymentMethod } from './fixtures/test-payment-methods';
-import { GetProductsWithVariantPrices, UpdateChannel } from './graphql/generated-e2e-admin-types';
+import {
+    GetProductsWithVariantPrices,
+    GetTaxRateList,
+    UpdateChannel,
+    UpdateTaxRate,
+} from './graphql/generated-e2e-admin-types';
 import {
     AddItemToOrder,
     GetActiveOrderWithPriceData,
+    SetBillingAddress,
+    SetShippingAddress,
     TestOrderFragmentFragment,
     UpdatedOrderFragment,
 } from './graphql/generated-e2e-shop-types';
-import { GET_PRODUCTS_WITH_VARIANT_PRICES, UPDATE_CHANNEL } from './graphql/shared-definitions';
-import { ADD_ITEM_TO_ORDER, GET_ACTIVE_ORDER_WITH_PRICE_DATA } from './graphql/shop-definitions';
+import {
+    GET_PRODUCTS_WITH_VARIANT_PRICES,
+    UPDATE_CHANNEL,
+    UPDATE_TAX_RATE,
+} from './graphql/shared-definitions';
+import {
+    ADD_ITEM_TO_ORDER,
+    GET_ACTIVE_ORDER_WITH_PRICE_DATA,
+    SET_BILLING_ADDRESS,
+    SET_SHIPPING_ADDRESS,
+} from './graphql/shop-definitions';
 import { sortById } from './utils/test-order-utils';
 
+/**
+ * Determines active tax zone based on:
+ *
+ * 1. billingAddress country, if set
+ * 2. else shippingAddress country, is set
+ * 3. else channel default tax zone.
+ */
+class TestTaxZoneStrategy implements TaxZoneStrategy {
+    private connection: TransactionalConnection;
+
+    init(injector: Injector): void | Promise<void> {
+        this.connection = injector.get(TransactionalConnection);
+    }
+
+    async determineTaxZone(
+        ctx: RequestContext,
+        zones: Zone[],
+        channel: Channel,
+        order?: Order,
+    ): Promise<Zone> {
+        if (!order?.billingAddress?.countryCode && !order?.shippingAddress?.countryCode) {
+            return channel.defaultTaxZone;
+        }
+
+        const countryCode = order?.billingAddress?.countryCode || order?.shippingAddress?.countryCode;
+        const zoneForCountryCode = await this.getZoneForCountryCode(ctx, countryCode);
+        return zoneForCountryCode ?? channel.defaultTaxZone;
+    }
+
+    private getZoneForCountryCode(ctx: RequestContext, countryCode: string): Promise<Zone | undefined> {
+        return this.connection
+            .getRepository(ctx, Zone)
+            .createQueryBuilder('zone')
+            .leftJoin('zone.members', 'country')
+            .where('country.code = :countryCode', {
+                countryCode,
+            })
+            .getOne();
+    }
+}
+
 describe('Order taxes', () => {
     const { server, adminClient, shopClient } = createTestEnvironment({
         ...testConfig(),
+        taxOptions: {
+            taxZoneStrategy: new TestTaxZoneStrategy(),
+        },
         paymentOptions: {
             paymentMethodHandlers: [testSuccessfulPaymentMethod],
         },
@@ -137,6 +207,92 @@ describe('Order taxes', () => {
                 },
             ]);
         });
+
+        // https://github.com/vendure-ecommerce/vendure/issues/1216
+        it('re-calculates OrderItem prices when shippingAddress causes activeTaxZone change', async () => {
+            const { taxRates } = await adminClient.query<GetTaxRateList.Query>(GET_TAX_RATE_LIST);
+            // Set the TaxRates to Asia to 0%
+            const taxRatesAsia = taxRates.items.filter(tr => tr.name.includes('Asia'));
+            for (const taxRate of taxRatesAsia) {
+                await adminClient.query<UpdateTaxRate.Mutation, UpdateTaxRate.Variables>(UPDATE_TAX_RATE, {
+                    input: {
+                        id: taxRate.id,
+                        value: 0,
+                    },
+                });
+            }
+
+            await shopClient.query<SetShippingAddress.Mutation, SetShippingAddress.Variables>(
+                SET_SHIPPING_ADDRESS,
+                {
+                    input: {
+                        countryCode: 'CN',
+                        streetLine1: '123 Lugu St',
+                        city: 'Beijing',
+                        province: 'Beijing',
+                        postalCode: '12340',
+                    },
+                },
+            );
+
+            const { activeOrder } = await shopClient.query<GetActiveOrderWithPriceData.Query>(
+                GET_ACTIVE_ORDER_WITH_PRICE_DATA,
+            );
+            expect(activeOrder?.totalWithTax).toBe(166);
+            expect(activeOrder?.total).toBe(166);
+            expect(activeOrder?.lines[0].taxRate).toBe(0);
+            expect(activeOrder?.lines[0].linePrice).toBe(166);
+            expect(activeOrder?.lines[0].lineTax).toBe(0);
+            expect(activeOrder?.lines[0].linePriceWithTax).toBe(166);
+            expect(activeOrder?.lines[0].unitPrice).toBe(83);
+            expect(activeOrder?.lines[0].unitPriceWithTax).toBe(83);
+            expect(activeOrder?.lines[0].items[0].unitPrice).toBe(83);
+            expect(activeOrder?.lines[0].items[0].unitPriceWithTax).toBe(83);
+            expect(activeOrder?.lines[0].items[0].taxRate).toBe(0);
+            expect(activeOrder?.lines[0].taxLines).toEqual([
+                {
+                    description: 'Standard Tax Asia',
+                    taxRate: 0,
+                },
+            ]);
+        });
+
+        // https://github.com/vendure-ecommerce/vendure/issues/1216
+        it('re-calculates OrderItem prices when billingAddress causes activeTaxZone change', async () => {
+            await shopClient.query<SetBillingAddress.Mutation, SetBillingAddress.Variables>(
+                SET_BILLING_ADDRESS,
+                {
+                    input: {
+                        countryCode: 'US',
+                        streetLine1: '123 Chad Street',
+                        city: 'Houston',
+                        province: 'Texas',
+                        postalCode: '12345',
+                    },
+                },
+            );
+
+            const { activeOrder } = await shopClient.query<GetActiveOrderWithPriceData.Query>(
+                GET_ACTIVE_ORDER_WITH_PRICE_DATA,
+            );
+            expect(activeOrder?.totalWithTax).toBe(200);
+            expect(activeOrder?.total).toBe(166);
+            expect(activeOrder?.lines[0].taxRate).toBe(20);
+            expect(activeOrder?.lines[0].linePrice).toBe(166);
+            expect(activeOrder?.lines[0].lineTax).toBe(34);
+            expect(activeOrder?.lines[0].linePriceWithTax).toBe(200);
+            expect(activeOrder?.lines[0].unitPrice).toBe(83);
+            expect(activeOrder?.lines[0].unitPriceWithTax).toBe(100);
+            expect(activeOrder?.lines[0].items[0].unitPrice).toBe(83);
+            expect(activeOrder?.lines[0].items[0].unitPriceWithTax).toBe(100);
+            expect(activeOrder?.lines[0].items[0].taxRate).toBe(20);
+            expect(activeOrder?.lines[0].taxLines).toEqual([
+                {
+                    description: 'Standard Tax Americas',
+                    taxRate: 20,
+                },
+            ]);
+        });
     });
 
     it('taxSummary works', async () => {
@@ -193,3 +349,25 @@ describe('Order taxes', () => {
         expect(taxSummaryBaseTotal + taxSummaryTaxTotal).toBe(activeOrder?.totalWithTax);
     });
 });
+
+export const GET_TAX_RATE_LIST = gql`
+    query GetTaxRateList($options: TaxRateListOptions) {
+        taxRates(options: $options) {
+            items {
+                id
+                name
+                enabled
+                value
+                category {
+                    id
+                    name
+                }
+                zone {
+                    id
+                    name
+                }
+            }
+            totalItems
+        }
+    }
+`;

packages/core/e2e/utils/assert-throws-with-message.ts

@@ -1,3 +1,5 @@
+import { fail } from 'assert';
+
 /**
  * Helper method for creating tests which assert a given error message when the operation is attempted.
  */

packages/core/src/service/helpers/product-price-applicator/product-price-applicator.ts

@@ -42,7 +42,7 @@ export class ProductPriceApplicator {
         }
         const { taxZoneStrategy } = this.configService.taxOptions;
         const zones = await this.requestCache.get(ctx, 'allZones', () => this.zoneService.findAll(ctx));
-        const activeTaxZone = await this.requestCache.get(ctx, 'activeTaxZone', () =>
+        const activeTaxZone = await this.requestCache.get(ctx, `activeTaxZone`, () =>
             taxZoneStrategy.determineTaxZone(ctx, zones, ctx.channel, order),
         );
         if (!activeTaxZone) {

packages/core/src/service/services/order.service.ts

@@ -40,6 +40,7 @@ import { unique } from '@vendure/common/lib/unique';
 import { FindOptionsUtils } from 'typeorm/find-options/FindOptionsUtils';
 
 import { RequestContext } from '../../api/common/request-context';
+import { RequestContextCacheService } from '../../cache/request-context-cache.service';
 import { ErrorResultUnion, isGraphQlErrorResult } from '../../common/error/error-result';
 import { EntityNotFoundError, InternalServerError, UserInputError } from '../../common/error/errors';
 import {
@@ -154,6 +155,7 @@ export class OrderService {
         private channelService: ChannelService,
         private orderModifier: OrderModifier,
         private customFieldRelationService: CustomFieldRelationService,
+        private requestCache: RequestContextCacheService,
     ) {}
 
     /**
@@ -459,7 +461,7 @@ export class OrderService {
             await this.orderModifier.updateOrderLineQuantity(ctx, orderLine, correctedQuantity, order);
         }
         const quantityWasAdjustedDown = correctedQuantity < quantity;
-        const updatedOrder = await this.applyPriceAdjustments(ctx, order, orderLine);
+        const updatedOrder = await this.applyPriceAdjustments(ctx, order, [orderLine]);
         if (quantityWasAdjustedDown) {
             return new InsufficientStockError(correctedQuantity, updatedOrder);
         } else {
@@ -509,7 +511,7 @@ export class OrderService {
             await this.orderModifier.updateOrderLineQuantity(ctx, orderLine, correctedQuantity, order);
         }
         const quantityWasAdjustedDown = correctedQuantity < quantity;
-        const updatedOrder = await this.applyPriceAdjustments(ctx, order, orderLine);
+        const updatedOrder = await this.applyPriceAdjustments(ctx, order, [orderLine]);
         if (quantityWasAdjustedDown) {
             return new InsufficientStockError(correctedQuantity, updatedOrder);
         } else {
@@ -691,7 +693,12 @@ export class OrderService {
         const order = await this.getOrderOrThrow(ctx, orderId);
         const country = await this.countryService.findOneByCode(ctx, input.countryCode);
         order.shippingAddress = { ...input, countryCode: input.countryCode, country: country.name };
-        return this.connection.getRepository(ctx, Order).save(order);
+        await this.connection.getRepository(ctx, Order).save(order);
+        // Since a changed ShippingAddress could alter the activeTaxZone,
+        // we will remove any cached activeTaxZone so it can be re-calculated
+        // as needed.
+        this.requestCache.set(ctx, 'activeTaxZone', undefined);
+        return this.applyPriceAdjustments(ctx, order, order.lines);
     }
 
     /**
@@ -702,7 +709,12 @@ export class OrderService {
         const order = await this.getOrderOrThrow(ctx, orderId);
         const country = await this.countryService.findOneByCode(ctx, input.countryCode);
         order.billingAddress = { ...input, countryCode: input.countryCode, country: country.name };
-        return this.connection.getRepository(ctx, Order).save(order);
+        await this.connection.getRepository(ctx, Order).save(order);
+        // Since a changed ShippingAddress could alter the activeTaxZone,
+        // we will remove any cached activeTaxZone so it can be re-calculated
+        // as needed.
+        this.requestCache.set(ctx, 'activeTaxZone', undefined);
+        return this.applyPriceAdjustments(ctx, order, order.lines);
     }
 
     /**
@@ -1530,32 +1542,39 @@ export class OrderService {
     private async applyPriceAdjustments(
         ctx: RequestContext,
         order: Order,
-        updatedOrderLine?: OrderLine,
+        updatedOrderLines?: OrderLine[],
     ): Promise<Order> {
-        if (updatedOrderLine) {
+        if (updatedOrderLines?.length) {
             const { orderItemPriceCalculationStrategy, changedPriceHandlingStrategy } =
                 this.configService.orderOptions;
-            let priceResult = await orderItemPriceCalculationStrategy.calculateUnitPrice(
-                ctx,
-                updatedOrderLine.productVariant,
-                updatedOrderLine.customFields || {},
-            );
-            const initialListPrice =
-                updatedOrderLine.items.find(i => i.initialListPrice != null)?.initialListPrice ??
-                priceResult.price;
-            if (initialListPrice !== priceResult.price) {
-                priceResult = await changedPriceHandlingStrategy.handlePriceChange(
+            for (const updatedOrderLine of updatedOrderLines) {
+                const variant = await this.productVariantService.applyChannelPriceAndTax(
+                    updatedOrderLine.productVariant,
                     ctx,
-                    priceResult,
-                    updatedOrderLine.items,
+                    order,
                 );
-            }
-            for (const item of updatedOrderLine.items) {
-                if (item.initialListPrice == null) {
-                    item.initialListPrice = initialListPrice;
+                let priceResult = await orderItemPriceCalculationStrategy.calculateUnitPrice(
+                    ctx,
+                    variant,
+                    updatedOrderLine.customFields || {},
+                );
+                const initialListPrice =
+                    updatedOrderLine.items.find(i => i.initialListPrice != null)?.initialListPrice ??
+                    priceResult.price;
+                if (initialListPrice !== priceResult.price) {
+                    priceResult = await changedPriceHandlingStrategy.handlePriceChange(
+                        ctx,
+                        priceResult,
+                        updatedOrderLine.items,
+                    );
+                }
+                for (const item of updatedOrderLine.items) {
+                    if (item.initialListPrice == null) {
+                        item.initialListPrice = initialListPrice;
+                    }
+                    item.listPrice = priceResult.price;
+                    item.listPriceIncludesTax = priceResult.priceIncludesTax;
                 }
-                item.listPrice = priceResult.price;
-                item.listPriceIncludesTax = priceResult.priceIncludesTax;
             }
         }
         const { items: promotions } = await this.promotionService.findAll(ctx, {
@@ -1566,7 +1585,7 @@ export class OrderService {
             ctx,
             order,
             promotions,
-            updatedOrderLine ? [updatedOrderLine] : [],
+            updatedOrderLines ?? [],
         );
         const updateFields: Array<keyof OrderItem> = [
             'initialListPrice',