Skip to content

Commit

Permalink
fix(core): Avoid variant options combination check on updateProductVa…
Browse files Browse the repository at this point in the history
…riant mutation (#3361)
  • Loading branch information
DeltaSAMP authored Feb 11, 2025
1 parent 0ea3441 commit c820f42
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 18 deletions.
38 changes: 22 additions & 16 deletions packages/core/e2e/product.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1736,22 +1736,28 @@ describe('Product resolver', () => {
}, 'ProductVariant optionIds must include one optionId from each of the groups: group-2, group-3'),
);

it(
'passing optionIds that match an existing variant throws',
assertThrowsWithMessage(async () => {
await adminClient.query<
Codegen.UpdateProductVariantsMutation,
Codegen.UpdateProductVariantsMutationVariables
>(UPDATE_PRODUCT_VARIANTS, {
input: [
{
id: variantToModify.id,
optionIds: variants[1]!.options.map(o => o.id),
},
],
});
}, 'A ProductVariant with the selected options already exists: Variant 3'),
);
it('passing optionIds that match an existing variant should not throw', async () => {
const { updateProductVariants } = await adminClient.query<
Codegen.UpdateProductVariantsMutation,
Codegen.UpdateProductVariantsMutationVariables
>(UPDATE_PRODUCT_VARIANTS, {
input: [
{
id: variantToModify.id,
optionIds: variantToModify!.options.map(o => o.id),
sku: 'ABC',
price: 432,
},
],
});
const updatedVariant = updateProductVariants[0];
if (!updatedVariant) {
fail('no updated variant returned.');
return;
}
expect(updatedVariant.sku).toBe('ABC');
expect(updatedVariant.price).toBe(432);
});

it('addOptionGroupToProduct and then update existing ProductVariant with a new option', async () => {
const optionGroup4 = await createOptionGroup('group-4', [
Expand Down
10 changes: 8 additions & 2 deletions packages/core/src/service/services/product-variant.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ export class ProductVariantService {
throw new UserInputError('error.stockonhand-cannot-be-negative');
}
if (input.optionIds) {
await this.validateVariantOptionIds(ctx, existingVariant.productId, input.optionIds);
await this.validateVariantOptionIds(ctx, existingVariant.productId, input.optionIds, true);
}
const inputWithoutPriceAndStockLevels = {
...input,
Expand Down Expand Up @@ -899,7 +899,12 @@ export class ProductVariantService {
return result;
}

private async validateVariantOptionIds(ctx: RequestContext, productId: ID, optionIds: ID[] = []) {
private async validateVariantOptionIds(
ctx: RequestContext,
productId: ID,
optionIds: ID[] = [],
isUpdateOperation?: boolean,
) {
// this could be done with fewer queries but depending on the data, node will crash
// https://github.com/vendure-ecommerce/vendure/issues/328
const optionGroups = (
Expand Down Expand Up @@ -936,6 +941,7 @@ export class ProductVariantService {
.filter(v => !v.deletedAt)
.forEach(variant => {
const variantOptionIds = this.sortJoin(variant.options, ',', 'id');
if (isUpdateOperation) return;
if (variantOptionIds === inputOptionIds) {
throw new UserInputError('error.product-variant-options-combination-already-exists', {
variantName: this.translator.translate(variant, ctx).name,
Expand Down

0 comments on commit c820f42

Please sign in to comment.