From e28ba3def3652e70ef5aca1329f3105ee532b4d4 Mon Sep 17 00:00:00 2001 From: Michael Bromley Date: Tue, 5 Mar 2024 15:48:59 +0100 Subject: [PATCH] fix(core): Fix querying order variant after removal from channel Fixes #2716 --- packages/core/e2e/product-channel.e2e-spec.ts | 100 ++++++++++++++++++ .../product-price-applicator.ts | 11 +- .../services/product-variant.service.ts | 5 +- 3 files changed, 110 insertions(+), 6 deletions(-) diff --git a/packages/core/e2e/product-channel.e2e-spec.ts b/packages/core/e2e/product-channel.e2e-spec.ts index 2c083ce1ed..d60564fdef 100644 --- a/packages/core/e2e/product-channel.e2e-spec.ts +++ b/packages/core/e2e/product-channel.e2e-spec.ts @@ -5,6 +5,8 @@ import { E2E_DEFAULT_CHANNEL_TOKEN, ErrorResultGuard, } from '@vendure/testing'; +import { fail } from 'assert'; +import gql from 'graphql-tag'; import path from 'path'; import { afterAll, beforeAll, describe, expect, it } from 'vitest'; @@ -36,6 +38,8 @@ import { UpdateProductDocument, UpdateProductVariantsDocument, } from './graphql/generated-e2e-admin-types'; +import { AddItemToOrderMutation, AddItemToOrderMutationVariables } from './graphql/generated-e2e-shop-types'; +import { ADD_ITEM_TO_ORDER } from './graphql/shop-definitions'; import { assertThrowsWithMessage } from './utils/assert-throws-with-message'; describe('ChannelAware Products and ProductVariants', () => { @@ -43,6 +47,9 @@ describe('ChannelAware Products and ProductVariants', () => { const SECOND_CHANNEL_TOKEN = 'second_channel_token'; const THIRD_CHANNEL_TOKEN = 'third_channel_token'; let secondChannelAdminRole: CreateRoleMutation['createRole']; + const orderResultGuard: ErrorResultGuard<{ lines: Array<{ id: string }> }> = createErrorResultGuard( + input => !!input.lines, + ); beforeAll(async () => { await server.init({ @@ -216,6 +223,99 @@ describe('ChannelAware Products and ProductVariants', () => { expect(removeProductsFromChannel[0].channels.map(c => c.id)).toEqual(['T_1']); }); + + // https://github.com/vendure-ecommerce/vendure/issues/2716 + it('querying an Order with a variant that was since removed from the channel', async () => { + await adminClient.query(AssignProductsToChannelDocument, { + input: { + channelId: 'T_2', + productIds: [product1.id], + priceFactor: 1, + }, + }); + + // Create an order in the second channel with the variant just assigned + shopClient.setChannelToken(SECOND_CHANNEL_TOKEN); + const { addItemToOrder } = await shopClient.query< + AddItemToOrderMutation, + AddItemToOrderMutationVariables + >(ADD_ITEM_TO_ORDER, { + productVariantId: product1.variants[0].id, + quantity: 1, + }); + orderResultGuard.assertSuccess(addItemToOrder); + + // Now remove that variant from the second channel + await adminClient.query(RemoveProductsFromChannelDocument, { + input: { + productIds: [product1.id], + channelId: 'T_2', + }, + }); + + adminClient.setChannelToken(SECOND_CHANNEL_TOKEN); + + // If no price fields are requested on the ProductVariant, then the query will + // succeed even if the ProductVariant is no longer assigned to the channel. + const GET_ORDER_WITHOUT_VARIANT_PRICE = ` + query GetOrderWithoutVariantPrice($id: ID!) { + order(id: $id) { + id + lines { + id + linePrice + productVariant { + id + name + } + } + } + }`; + const { order } = await adminClient.query(gql(GET_ORDER_WITHOUT_VARIANT_PRICE), { + id: addItemToOrder.id, + }); + + expect(order).toEqual({ + id: 'T_1', + lines: [ + { + id: 'T_1', + linePrice: 129900, + productVariant: { + id: 'T_1', + name: 'Laptop 13 inch 8GB', + }, + }, + ], + }); + + try { + // The API will only throw if one of the price fields is requested in the query + const GET_ORDER_WITH_VARIANT_PRICE = ` + query GetOrderWithVariantPrice($id: ID!) { + order(id: $id) { + id + lines { + id + linePrice + productVariant { + id + name + price + } + } + } + }`; + await adminClient.query(gql(GET_ORDER_WITH_VARIANT_PRICE), { + id: addItemToOrder.id, + }); + fail(`Should have thrown`); + } catch (e: any) { + expect(e.message).toContain( + 'No price information was found for ProductVariant ID "1" in the Channel "second-channel"', + ); + } + }); }); describe('assigning ProductVariant to Channels', () => { diff --git a/packages/core/src/service/helpers/product-price-applicator/product-price-applicator.ts b/packages/core/src/service/helpers/product-price-applicator/product-price-applicator.ts index b32511e842..890d19cff8 100644 --- a/packages/core/src/service/helpers/product-price-applicator/product-price-applicator.ts +++ b/packages/core/src/service/helpers/product-price-applicator/product-price-applicator.ts @@ -3,7 +3,6 @@ import { Injectable } from '@nestjs/common'; import { RequestContext } from '../../../api/common/request-context'; import { RequestContextCacheService } from '../../../cache/request-context-cache.service'; import { InternalServerError } from '../../../common/error/errors'; -import { idsAreEqual } from '../../../common/utils'; import { ConfigService } from '../../../config/config.service'; import { Order } from '../../../entity/order/order.entity'; import { ProductVariant } from '../../../entity/product-variant/product-variant.entity'; @@ -51,11 +50,15 @@ export class ProductPriceApplicator { * @description * Populates the `price` field with the price for the specified channel. Make sure that * the ProductVariant being passed in has its `taxCategory` relation joined. + * + * If the `throwIfNoPriceFound` option is set to `true`, then an error will be thrown if no + * price is found for the given Channel. */ async applyChannelPriceAndTax( variant: ProductVariant, ctx: RequestContext, order?: Order, + throwIfNoPriceFound = false, ): Promise { const { productVariantPriceSelectionStrategy, productVariantPriceCalculationStrategy } = this.configService.catalogOptions; @@ -63,7 +66,7 @@ export class ProductPriceApplicator { ctx, variant.productVariantPrices, ); - if (!channelPrice) { + if (!channelPrice && throwIfNoPriceFound) { throw new InternalServerError('error.no-price-found-for-channel', { variantId: variant.id, channel: ctx.channel.code, @@ -86,7 +89,7 @@ export class ProductPriceApplicator { ); const { price, priceIncludesTax } = await productVariantPriceCalculationStrategy.calculate({ - inputPrice: channelPrice.price, + inputPrice: channelPrice?.price ?? 0, taxCategory: variant.taxCategory, productVariant: variant, activeTaxZone, @@ -96,7 +99,7 @@ export class ProductPriceApplicator { variant.listPrice = price; variant.listPriceIncludesTax = priceIncludesTax; variant.taxRateApplied = applicableTaxRate; - variant.currencyCode = channelPrice.currencyCode; + variant.currencyCode = channelPrice?.currencyCode ?? ctx.currencyCode; return variant; } } diff --git a/packages/core/src/service/services/product-variant.service.ts b/packages/core/src/service/services/product-variant.service.ts index 61ebf3d891..113755830a 100644 --- a/packages/core/src/service/services/product-variant.service.ts +++ b/packages/core/src/service/services/product-variant.service.ts @@ -651,7 +651,7 @@ export class ProductVariantService { ); variant.taxCategory = variantWithTaxCategory.taxCategory; } - resolve(await this.applyChannelPriceAndTax(variant, ctx)); + resolve(await this.applyChannelPriceAndTax(variant, ctx, undefined, true)); } catch (e: any) { reject(e); } @@ -691,8 +691,9 @@ export class ProductVariantService { variant: ProductVariant, ctx: RequestContext, order?: Order, + throwIfNoPriceFound = false, ): Promise { - return this.productPriceApplicator.applyChannelPriceAndTax(variant, ctx, order); + return this.productPriceApplicator.applyChannelPriceAndTax(variant, ctx, order, throwIfNoPriceFound); } /**