Skip to content

Commit

Permalink
feat(core): Always pass current Order to TaxZoneStrategy calls
Browse files Browse the repository at this point in the history
Relates to #1048. This commit also introduces the use of RequestContextCacheService to optimize the
number of calls made to the `determineTaxZone()` method, as well as allowing async return values.
  • Loading branch information
michaelbromley committed Aug 30, 2021
1 parent 068cefd commit 7b76a7c
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 12 deletions.
17 changes: 16 additions & 1 deletion packages/core/src/config/tax/tax-zone-strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,23 @@ import { Channel, Order, Zone } from '../../entity';
* @description
* Defines how the active {@link Zone} is determined for the purposes of calculating taxes.
*
* This strategy is used in 2 scenarios:
*
* 1. To determine the applicable Zone when calculating the taxRate to apply when displaying ProductVariants. In this case the
* `order` argument will be undefined, as the request is not related to a specific Order.
* 2. To determine the applicable Zone when calculating the taxRate on the contents of a specific Order. In this case the
* `order` argument _will_ be defined, and can be used in the logic. For example, the shipping address can be taken into account.
*
* Note that this method is called very often in a typical user session, so any work it performs should be designed with as little
* performance impact as possible.
*
* @docsCategory tax
*/
export interface TaxZoneStrategy extends InjectableStrategy {
determineTaxZone(ctx: RequestContext, zones: Zone[], channel: Channel, order?: Order): Zone | undefined;
determineTaxZone(
ctx: RequestContext,
zones: Zone[],
channel: Channel,
order?: Order,
): Zone | Promise<Zone> | undefined;
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { AdjustmentType, LanguageCode, TaxLine } from '@vendure/common/lib/gener
import { summate } from '@vendure/common/lib/shared-utils';

import { RequestContext } from '../../../api/common/request-context';
import { RequestContextCacheService } from '../../../cache/request-context-cache.service';
import { PromotionItemAction, PromotionOrderAction, PromotionShippingAction } from '../../../config';
import { ConfigService } from '../../../config/config.service';
import { MockConfigService } from '../../../config/config.service.mock';
Expand Down Expand Up @@ -1537,6 +1538,7 @@ function createTestModule() {
return Test.createTestingModule({
providers: [
OrderCalculator,
RequestContextCacheService,
{ provide: TaxRateService, useClass: MockTaxRateService },
{ provide: ShippingCalculator, useValue: { getEligibleShippingMethods: () => [] } },
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { filterAsync } from '@vendure/common/lib/filter-async';
import { AdjustmentType } from '@vendure/common/lib/generated-types';

import { RequestContext } from '../../../api/common/request-context';
import { RequestContextCacheService } from '../../../cache/request-context-cache.service';
import { InternalServerError } from '../../../common/error/errors';
import { netPriceOf } from '../../../common/tax-utils';
import { idsAreEqual } from '../../../common/utils';
Expand All @@ -27,6 +28,7 @@ export class OrderCalculator {
private taxRateService: TaxRateService,
private shippingMethodService: ShippingMethodService,
private shippingCalculator: ShippingCalculator,
private requestContextCache: RequestContextCacheService,
) {}

/**
Expand All @@ -43,7 +45,10 @@ export class OrderCalculator {
): Promise<OrderItem[]> {
const { taxZoneStrategy } = this.configService.taxOptions;
const zones = this.zoneService.findAll(ctx);
const activeTaxZone = taxZoneStrategy.determineTaxZone(ctx, zones, ctx.channel, order);
const activeTaxZone = await this.requestContextCache.get(ctx, 'activeTaxZone', () =>
taxZoneStrategy.determineTaxZone(ctx, zones, ctx.channel, order),
);

let taxZoneChanged = false;
if (!activeTaxZone) {
throw new InternalServerError(`error.no-active-tax-zone`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,11 @@ export class OrderModifier {
],
});
lineWithRelations.productVariant = translateDeep(
await this.productVariantService.applyChannelPriceAndTax(lineWithRelations.productVariant, ctx),
await this.productVariantService.applyChannelPriceAndTax(
lineWithRelations.productVariant,
ctx,
order,
),
ctx.languageCode,
);
order.lines.push(lineWithRelations);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export class OrderTestingService {
line.productVariantId,
{ relations: ['taxCategory'] },
);
await this.productVariantService.applyChannelPriceAndTax(productVariant, ctx);
await this.productVariantService.applyChannelPriceAndTax(productVariant, ctx, mockOrder);
const orderLine = new OrderLine({
productVariant,
items: [],
Expand Down
8 changes: 3 additions & 5 deletions packages/core/src/service/services/order.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ export class OrderService {
if (order) {
for (const line of order.lines) {
line.productVariant = translateDeep(
await this.productVariantService.applyChannelPriceAndTax(line.productVariant, ctx),
await this.productVariantService.applyChannelPriceAndTax(line.productVariant, ctx, order),
ctx.languageCode,
);
}
Expand Down Expand Up @@ -1348,10 +1348,8 @@ export class OrderService {
updatedOrderLine?: OrderLine,
): Promise<Order> {
if (updatedOrderLine) {
const {
orderItemPriceCalculationStrategy,
changedPriceHandlingStrategy,
} = this.configService.orderOptions;
const { orderItemPriceCalculationStrategy, changedPriceHandlingStrategy } =
this.configService.orderOptions;
let priceResult = await orderItemPriceCalculationStrategy.calculateUnitPrice(
ctx,
updatedOrderLine.productVariant,
Expand Down
19 changes: 16 additions & 3 deletions packages/core/src/service/services/product-variant.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,14 @@ import { ListQueryOptions } from '../../common/types/common-types';
import { Translated } from '../../common/types/locale-types';
import { idsAreEqual } from '../../common/utils';
import { ConfigService } from '../../config/config.service';
import { Channel, OrderLine, ProductOptionGroup, ProductVariantPrice, TaxCategory } from '../../entity';
import {
Channel,
Order,
OrderLine,
ProductOptionGroup,
ProductVariantPrice,
TaxCategory,
} from '../../entity';
import { FacetValue } from '../../entity/facet-value/facet-value.entity';
import { ProductOption } from '../../entity/product-option/product-option.entity';
import { ProductVariantTranslation } from '../../entity/product-variant/product-variant-translation.entity';
Expand Down Expand Up @@ -551,7 +558,11 @@ export class ProductVariantService {
/**
* Populates the `price` field with the price for the specified channel.
*/
async applyChannelPriceAndTax(variant: ProductVariant, ctx: RequestContext): Promise<ProductVariant> {
async applyChannelPriceAndTax(
variant: ProductVariant,
ctx: RequestContext,
order?: Order,
): Promise<ProductVariant> {
const channelPrice = variant.productVariantPrices.find(p => idsAreEqual(p.channelId, ctx.channelId));
if (!channelPrice) {
throw new InternalServerError(`error.no-price-found-for-channel`, {
Expand All @@ -561,7 +572,9 @@ export class ProductVariantService {
}
const { taxZoneStrategy } = this.configService.taxOptions;
const zones = this.zoneService.findAll(ctx);
const activeTaxZone = taxZoneStrategy.determineTaxZone(ctx, zones, ctx.channel);
const activeTaxZone = await this.requestCache.get(ctx, 'activeTaxZone', () =>
taxZoneStrategy.determineTaxZone(ctx, zones, ctx.channel, order),
);
if (!activeTaxZone) {
throw new InternalServerError(`error.no-active-tax-zone`);
}
Expand Down

0 comments on commit 7b76a7c

Please sign in to comment.