feat(core): Simplify TaxCalculationStrategy API

Relates to #307

BREAKING CHANGE: The TaxCalculationStrategy return value has been simplified - it now only need
return the `price` and `priceIncludesTax` properties. The `ProductVariant` entity has also been
refactored to bring it into line with the corrected tax handling of the OrderItem entity. This
will require a DB migration. See release blog post for details.

packages/asset-server-plugin/e2e/graphql/generated-e2e-asset-server-plugin-types.ts

@@ -1111,18 +1111,6 @@ export type UpdateFacetValueInput = {
     customFields?: Maybe<Scalars['JSON']>;
 };
 
-export type Fulfillment = Node & {
-    nextStates: Array<Scalars['String']>;
-    id: Scalars['ID'];
-    createdAt: Scalars['DateTime'];
-    updatedAt: Scalars['DateTime'];
-    orderItems: Array<OrderItem>;
-    state: Scalars['String'];
-    method: Scalars['String'];
-    trackingCode?: Maybe<Scalars['String']>;
-    customFields?: Maybe<Scalars['JSON']>;
-};
-
 export type UpdateGlobalSettingsInput = {
     availableLanguages?: Maybe<Array<LanguageCode>>;
     trackInventory?: Maybe<Scalars['Boolean']>;
@@ -1286,6 +1274,18 @@ export type OrderHistoryArgs = {
     options?: Maybe<HistoryEntryListOptions>;
 };
 
+export type Fulfillment = Node & {
+    nextStates: Array<Scalars['String']>;
+    id: Scalars['ID'];
+    createdAt: Scalars['DateTime'];
+    updatedAt: Scalars['DateTime'];
+    orderItems: Array<OrderItem>;
+    state: Scalars['String'];
+    method: Scalars['String'];
+    trackingCode?: Maybe<Scalars['String']>;
+    customFields?: Maybe<Scalars['JSON']>;
+};
+
 export type UpdateOrderInput = {
     id: Scalars['ID'];
     customFields?: Maybe<Scalars['JSON']>;
@@ -1553,6 +1553,7 @@ export type ProductVariant = Node & {
     assets: Array<Asset>;
     price: Scalars['Int'];
     currencyCode: CurrencyCode;
+    /** @deprecated price is now always exluding tax */
     priceIncludesTax: Scalars['Boolean'];
     priceWithTax: Scalars['Int'];
     taxRateApplied: TaxRate;

packages/common/src/generated-shop-types.ts

@@ -2123,6 +2123,7 @@ export type ProductVariant = Node & {
     assets: Array<Asset>;
     price: Scalars['Int'];
     currencyCode: CurrencyCode;
+    /** @deprecated price is now always exluding tax */
     priceIncludesTax: Scalars['Boolean'];
     priceWithTax: Scalars['Int'];
     taxRateApplied: TaxRate;

packages/common/src/generated-types.ts

@@ -1256,19 +1256,6 @@ export type UpdateFacetValueInput = {
   customFields?: Maybe<Scalars['JSON']>;
 };
 
-export type Fulfillment = Node & {
-  __typename?: 'Fulfillment';
-  nextStates: Array<Scalars['String']>;
-  id: Scalars['ID'];
-  createdAt: Scalars['DateTime'];
-  updatedAt: Scalars['DateTime'];
-  orderItems: Array<OrderItem>;
-  state: Scalars['String'];
-  method: Scalars['String'];
-  trackingCode?: Maybe<Scalars['String']>;
-  customFields?: Maybe<Scalars['JSON']>;
-};
-
 export type UpdateGlobalSettingsInput = {
   availableLanguages?: Maybe<Array<LanguageCode>>;
   trackInventory?: Maybe<Scalars['Boolean']>;
@@ -1444,6 +1431,19 @@ export type OrderHistoryArgs = {
   options?: Maybe<HistoryEntryListOptions>;
 };
 
+export type Fulfillment = Node & {
+  __typename?: 'Fulfillment';
+  nextStates: Array<Scalars['String']>;
+  id: Scalars['ID'];
+  createdAt: Scalars['DateTime'];
+  updatedAt: Scalars['DateTime'];
+  orderItems: Array<OrderItem>;
+  state: Scalars['String'];
+  method: Scalars['String'];
+  trackingCode?: Maybe<Scalars['String']>;
+  customFields?: Maybe<Scalars['JSON']>;
+};
+
 export type UpdateOrderInput = {
   id: Scalars['ID'];
   customFields?: Maybe<Scalars['JSON']>;
@@ -1705,6 +1705,7 @@ export type ProductVariant = Node & {
   assets: Array<Asset>;
   price: Scalars['Int'];
   currencyCode: CurrencyCode;
+  /** @deprecated price is now always exluding tax */
   priceIncludesTax: Scalars['Boolean'];
   priceWithTax: Scalars['Int'];
   taxRateApplied: TaxRate;

packages/core/e2e/graphql/fragments.ts

@@ -38,9 +38,8 @@ export const PRODUCT_VARIANT_FRAGMENT = gql`
         enabled
         languageCode
         name
-        price
         currencyCode
-        priceIncludesTax
+        price
         priceWithTax
         stockOnHand
         trackInventory

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

@@ -1111,18 +1111,6 @@ export type UpdateFacetValueInput = {
     customFields?: Maybe<Scalars['JSON']>;
 };
 
-export type Fulfillment = Node & {
-    nextStates: Array<Scalars['String']>;
-    id: Scalars['ID'];
-    createdAt: Scalars['DateTime'];
-    updatedAt: Scalars['DateTime'];
-    orderItems: Array<OrderItem>;
-    state: Scalars['String'];
-    method: Scalars['String'];
-    trackingCode?: Maybe<Scalars['String']>;
-    customFields?: Maybe<Scalars['JSON']>;
-};
-
 export type UpdateGlobalSettingsInput = {
     availableLanguages?: Maybe<Array<LanguageCode>>;
     trackInventory?: Maybe<Scalars['Boolean']>;
@@ -1286,6 +1274,18 @@ export type OrderHistoryArgs = {
     options?: Maybe<HistoryEntryListOptions>;
 };
 
+export type Fulfillment = Node & {
+    nextStates: Array<Scalars['String']>;
+    id: Scalars['ID'];
+    createdAt: Scalars['DateTime'];
+    updatedAt: Scalars['DateTime'];
+    orderItems: Array<OrderItem>;
+    state: Scalars['String'];
+    method: Scalars['String'];
+    trackingCode?: Maybe<Scalars['String']>;
+    customFields?: Maybe<Scalars['JSON']>;
+};
+
 export type UpdateOrderInput = {
     id: Scalars['ID'];
     customFields?: Maybe<Scalars['JSON']>;
@@ -1553,6 +1553,7 @@ export type ProductVariant = Node & {
     assets: Array<Asset>;
     price: Scalars['Int'];
     currencyCode: CurrencyCode;
+    /** @deprecated price is now always exluding tax */
     priceIncludesTax: Scalars['Boolean'];
     priceWithTax: Scalars['Int'];
     taxRateApplied: TaxRate;
@@ -4669,9 +4670,8 @@ export type ProductVariantFragment = Pick<
     | 'enabled'
     | 'languageCode'
     | 'name'
-    | 'price'
     | 'currencyCode'
-    | 'priceIncludesTax'
+    | 'price'
     | 'priceWithTax'
     | 'stockOnHand'
     | 'trackInventory'

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

@@ -2047,6 +2047,7 @@ export type ProductVariant = Node & {
     assets: Array<Asset>;
     price: Scalars['Int'];
     currencyCode: CurrencyCode;
+    /** @deprecated price is now always exluding tax */
     priceIncludesTax: Scalars['Boolean'];
     priceWithTax: Scalars['Int'];
     taxRateApplied: TaxRate;

packages/core/e2e/product-channel.e2e-spec.ts

@@ -138,6 +138,7 @@ describe('ChannelAware Products and ProductVariants', () => {
         );
 
         it('assigns Product to Channel and applies price factor', async () => {
+            adminClient.setChannelToken(E2E_DEFAULT_CHANNEL_TOKEN);
             const PRICE_FACTOR = 0.5;
             await adminClient.asSuperAdmin();
             const { assignProductsToChannel } = await adminClient.query<
@@ -161,11 +162,11 @@ describe('ChannelAware Products and ProductVariants', () => {
             });
 
             expect(product!.variants.map(v => v.price)).toEqual(
-                product1.variants.map(v => v.price * PRICE_FACTOR),
+                product1.variants.map(v => Math.round((v.price * PRICE_FACTOR) / 1.2)),
             );
             // Second Channel is configured to include taxes in price, so they should be the same.
             expect(product!.variants.map(v => v.priceWithTax)).toEqual(
-                product1.variants.map(v => v.price * PRICE_FACTOR),
+                product1.variants.map(v => Math.round((v.priceWithTax * PRICE_FACTOR) / 1.2)),
             );
         });
 
@@ -295,7 +296,9 @@ describe('ChannelAware Products and ProductVariants', () => {
                 id: product1.id,
             });
             expect(product!.channels.map(c => c.id).sort()).toEqual(['T_1', 'T_3']);
-            expect(product!.variants.map(v => v.price)).toEqual([product1.variants[0].price * PRICE_FACTOR]);
+            expect(product!.variants.map(v => v.price)).toEqual([
+                Math.round((product1.variants[0].price * PRICE_FACTOR) / 1.2),
+            ]);
             // Third Channel is configured to include taxes in price, so they should be the same.
             expect(product!.variants.map(v => v.priceWithTax)).toEqual([
                 product1.variants[0].price * PRICE_FACTOR,

packages/core/src/api/schema/common/product.type.graphql

@@ -43,7 +43,7 @@ type ProductVariant implements Node {
     assets: [Asset!]!
     price: Int!
     currencyCode: CurrencyCode!
-    priceIncludesTax: Boolean!
+    priceIncludesTax: Boolean! @deprecated(reason: "price now always excludes tax")
     priceWithTax: Int!
     taxRateApplied: TaxRate!
     taxCategory: TaxCategory!

packages/core/src/config/order/default-price-calculation-strategy.ts

@@ -15,6 +15,9 @@ export class DefaultPriceCalculationStrategy implements PriceCalculationStrategy
         ctx: RequestContext,
         productVariant: ProductVariant,
     ): CalculatedPrice | Promise<CalculatedPrice> {
-        return productVariant;
+        return {
+            price: productVariant.listPrice,
+            priceIncludesTax: productVariant.listPriceIncludesTax,
+        };
     }
 }

packages/core/src/config/tax/default-tax-calculation-strategy.ts

@@ -16,8 +16,6 @@ export class DefaultTaxCalculationStrategy implements TaxCalculationStrategy {
     calculate(args: TaxCalculationArgs): TaxCalculationResult {
         const { inputPrice, activeTaxZone, ctx, taxCategory, taxRateService } = args;
         let price = 0;
-        let priceWithTax = 0;
-        let priceWithoutTax = 0;
         let priceIncludesTax = false;
         const taxRate = taxRateService.getApplicableTaxRate(activeTaxZone, taxCategory);
 
@@ -27,28 +25,20 @@ export class DefaultTaxCalculationStrategy implements TaxCalculationStrategy {
                 ctx.channel.defaultTaxZone,
                 taxCategory,
             );
-            priceWithoutTax = taxRateForDefaultZone.netPriceOf(inputPrice);
 
             if (isDefaultZone) {
                 priceIncludesTax = true;
                 price = inputPrice;
-                priceWithTax = inputPrice;
             } else {
-                price = priceWithoutTax;
-                priceWithTax = taxRate.grossPriceOf(priceWithoutTax);
+                price = taxRateForDefaultZone.netPriceOf(inputPrice);
             }
         } else {
-            const netPrice = inputPrice;
-            price = netPrice;
-            priceWithTax = netPrice + taxRate.taxPayableOn(netPrice);
-            priceWithoutTax = netPrice;
+            price = inputPrice;
         }
 
         return {
             price,
             priceIncludesTax,
-            priceWithTax,
-            priceWithoutTax,
         };
     }
 }

packages/core/src/data-import/providers/importer/fast-importer.service.ts

@@ -113,9 +113,14 @@ export class FastImporterService {
             input.price = 0;
         }
 
+        const inputWithoutPrice = {
+            ...input,
+        };
+        delete inputWithoutPrice.price;
+
         const createdVariant = await this.translatableSaver.create({
             ctx: RequestContext.empty(),
-            input,
+            input: inputWithoutPrice,
             entityType: ProductVariant,
             translationType: ProductVariantTranslation,
             beforeSave: async variant => {
@@ -154,7 +159,7 @@ export class FastImporterService {
             );
         }
         const variantPrice = new ProductVariantPrice({
-            price: createdVariant.price,
+            price: input.price,
             channelId: this.defaultChannel.id,
         });
         variantPrice.variant = createdVariant;

packages/core/src/entity/product-variant/product-variant.entity.ts

@@ -2,6 +2,7 @@ import { CurrencyCode, GlobalFlag } from '@vendure/common/lib/generated-types';
 import { DeepPartial, ID } from '@vendure/common/lib/shared-types';
 import { Column, Entity, JoinTable, ManyToMany, ManyToOne, OneToMany } from 'typeorm';
 
+import { Calculated } from '../../common/calculated-decorator';
 import { ChannelAware, SoftDeletable } from '../../common/types/common-types';
 import { LocaleString, Translatable, Translation } from '../../common/types/locale-types';
 import { HasCustomFields } from '../../config/custom-field/custom-field-types';
@@ -50,30 +51,39 @@ export class ProductVariant
     @Column()
     sku: string;
 
-    /**
-     * A synthetic property which is populated with data from a ProductVariantPrice entity.
-     * It is marked as a @Column() so that changes to it will trigger the afterUpdate subscriber.
-     */
-    @Column({
-        name: 'lastPriceValue',
-        comment: 'Not used - actual price is stored in product_variant_price table',
-    })
-    price: number;
+    // TODO: Remove as deprecated
+    priceIncludesTax = false;
 
     /**
      * Calculated at run-time
      */
-    currencyCode: CurrencyCode;
+    listPrice: number;
 
     /**
      * Calculated at run-time
      */
-    priceIncludesTax: boolean;
+    listPriceIncludesTax: boolean;
 
     /**
      * Calculated at run-time
      */
-    priceWithTax: number;
+    currencyCode: CurrencyCode;
+
+    @Calculated()
+    get price(): number {
+        if (this.listPrice == null) {
+            return 0;
+        }
+        return this.listPriceIncludesTax ? this.taxRateApplied.netPriceOf(this.listPrice) : this.listPrice;
+    }
+
+    @Calculated()
+    get priceWithTax(): number {
+        if (this.listPrice == null) {
+            return 0;
+        }
+        return this.listPriceIncludesTax ? this.listPrice : this.taxRateApplied.grossPriceOf(this.listPrice);
+    }
 
     /**
      * Calculated at run-time

packages/core/src/service/helpers/tax-calculator/tax-calculator.ts

@@ -19,8 +19,6 @@ import { TaxRateService } from '../../services/tax-rate.service';
 export interface TaxCalculationResult {
     price: number;
     priceIncludesTax: boolean;
-    priceWithoutTax: number;
-    priceWithTax: number;
 }
 
 @Injectable()

packages/core/src/service/services/product-variant.service.ts

@@ -286,10 +286,13 @@ export class ProductVariantService {
             input.price = 0;
         }
         input.taxCategoryId = (await this.getTaxCategoryForNewVariant(ctx, input.taxCategoryId)).id;
-
+        const inputWithoutPrice = {
+            ...input,
+        };
+        delete inputWithoutPrice.price;
         const createdVariant = await this.translatableSaver.create({
             ctx,
-            input,
+            input: inputWithoutPrice,
             entityType: ProductVariant,
             translationType: ProductVariantTranslation,
             beforeSave: async variant => {
@@ -323,7 +326,7 @@ export class ProductVariantService {
             );
         }
 
-        await this.createProductVariantPrice(ctx, createdVariant.id, createdVariant.price, ctx.channelId);
+        await this.createProductVariantPrice(ctx, createdVariant.id, input.price, ctx.channelId);
         return createdVariant.id;
     }
 
@@ -334,9 +337,13 @@ export class ProductVariantService {
         if (input.stockOnHand && input.stockOnHand < 0) {
             throw new UserInputError('error.stockonhand-cannot-be-negative');
         }
+        const inputWithoutPrice = {
+            ...input,
+        };
+        delete inputWithoutPrice.price;
         await this.translatableSaver.update({
             ctx,
-            input,
+            input: inputWithoutPrice,
             entityType: ProductVariant,
             translationType: ProductVariantTranslation,
             beforeSave: async updatedVariant => {
@@ -431,16 +438,15 @@ export class ProductVariantService {
             variant.taxCategory,
         );
 
-        const { price, priceIncludesTax, priceWithTax, priceWithoutTax } = this.taxCalculator.calculate(
+        const { price, priceIncludesTax } = this.taxCalculator.calculate(
             channelPrice.price,
             variant.taxCategory,
             activeTaxZone,
             ctx,
         );
 
-        variant.price = price;
-        variant.priceIncludesTax = priceIncludesTax;
-        variant.priceWithTax = priceWithTax;
+        variant.listPrice = price;
+        variant.listPriceIncludesTax = priceIncludesTax;
         variant.taxRateApplied = applicableTaxRate;
         variant.currencyCode = ctx.channel.currencyCode;
         return variant;
@@ -460,9 +466,10 @@ export class ProductVariantService {
         }
         const variants = await this.connection
             .getRepository(ctx, ProductVariant)
-            .findByIds(input.productVariantIds);
+            .findByIds(input.productVariantIds, { relations: ['taxCategory'] });
         const priceFactor = input.priceFactor != null ? input.priceFactor : 1;
         for (const variant of variants) {
+            this.applyChannelPriceAndTax(variant, ctx);
             await this.channelService.assignToChannels(ctx, Product, variant.productId, [input.channelId]);
             await this.channelService.assignToChannels(ctx, ProductVariant, variant.id, [input.channelId]);
             await this.createProductVariantPrice(

packages/elasticsearch-plugin/e2e/graphql/generated-e2e-elasticsearch-plugin-types.ts

@@ -1111,18 +1111,6 @@ export type UpdateFacetValueInput = {
     customFields?: Maybe<Scalars['JSON']>;
 };
 
-export type Fulfillment = Node & {
-    nextStates: Array<Scalars['String']>;
-    id: Scalars['ID'];
-    createdAt: Scalars['DateTime'];
-    updatedAt: Scalars['DateTime'];
-    orderItems: Array<OrderItem>;
-    state: Scalars['String'];
-    method: Scalars['String'];
-    trackingCode?: Maybe<Scalars['String']>;
-    customFields?: Maybe<Scalars['JSON']>;
-};
-
 export type UpdateGlobalSettingsInput = {
     availableLanguages?: Maybe<Array<LanguageCode>>;
     trackInventory?: Maybe<Scalars['Boolean']>;
@@ -1286,6 +1274,18 @@ export type OrderHistoryArgs = {
     options?: Maybe<HistoryEntryListOptions>;
 };
 
+export type Fulfillment = Node & {
+    nextStates: Array<Scalars['String']>;
+    id: Scalars['ID'];
+    createdAt: Scalars['DateTime'];
+    updatedAt: Scalars['DateTime'];
+    orderItems: Array<OrderItem>;
+    state: Scalars['String'];
+    method: Scalars['String'];
+    trackingCode?: Maybe<Scalars['String']>;
+    customFields?: Maybe<Scalars['JSON']>;
+};
+
 export type UpdateOrderInput = {
     id: Scalars['ID'];
     customFields?: Maybe<Scalars['JSON']>;
@@ -1553,6 +1553,7 @@ export type ProductVariant = Node & {
     assets: Array<Asset>;
     price: Scalars['Int'];
     currencyCode: CurrencyCode;
+    /** @deprecated price is now always exluding tax */
     priceIncludesTax: Scalars['Boolean'];
     priceWithTax: Scalars['Int'];
     taxRateApplied: TaxRate;