fix(core): Ignore deleted variants when validating options

Fixes #412

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

@@ -22,6 +22,7 @@ import {
     GetProductVariant,
     GetProductWithVariants,
     LanguageCode,
+    ProductVariantFragment,
     ProductWithVariants,
     RemoveOptionGroupFromProduct,
     SortOrder,
@@ -1008,6 +1009,8 @@ describe('Product resolver', () => {
                 ),
             );
 
+            let deletedVariant: ProductVariantFragment;
+
             it('deleteProductVariant', async () => {
                 const result1 = await adminClient.query<
                     GetProductWithVariants.Query,
@@ -1034,6 +1037,30 @@ describe('Product resolver', () => {
                     id: newProduct.id,
                 });
                 expect(result2.product!.variants.map(v => v.id).sort()).toEqual(['T_36', 'T_37']);
+
+                deletedVariant = result1.product?.variants.find(v => v.id === 'T_35')!;
+            });
+
+            /** Testing https://github.com/vendure-ecommerce/vendure/issues/412 **/
+            it('createProductVariants ignores deleted variants when checking for existing combinations', async () => {
+                const { createProductVariants } = await adminClient.query<
+                    CreateProductVariants.Mutation,
+                    CreateProductVariants.Variables
+                >(CREATE_PRODUCT_VARIANTS, {
+                    input: [
+                        {
+                            productId: newProduct.id,
+                            sku: 'RE1',
+                            optionIds: [deletedVariant.options[0].id, deletedVariant.options[1].id],
+                            translations: [{ languageCode: LanguageCode.en, name: 'Re-created Variant' }],
+                        },
+                    ],
+                });
+
+                expect(createProductVariants.length).toBe(1);
+                expect(createProductVariants[0]!.options.map(o => o.code)).toEqual(
+                    deletedVariant.options.map(o => o.code),
+                );
             });
         });
     });

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

@@ -61,7 +61,7 @@ export class ProductVariantService {
         return this.connection
             .getRepository(ProductVariant)
             .findOne(productVariantId, { relations })
-            .then((result) => {
+            .then(result => {
                 if (result) {
                     return translateDeep(this.applyChannelPriceAndTax(result, ctx), ctx.languageCode, [
                         'product',
@@ -83,8 +83,8 @@ export class ProductVariantService {
                     'featuredAsset',
                 ],
             })
-            .then((variants) => {
-                return variants.map((variant) =>
+            .then(variants => {
+                return variants.map(variant =>
                     translateDeep(this.applyChannelPriceAndTax(variant, ctx), ctx.languageCode, [
                         'options',
                         'facetValues',
@@ -114,8 +114,8 @@ export class ProductVariantService {
                     id: 'ASC',
                 },
             })
-            .then((variants) =>
-                variants.map((variant) => {
+            .then(variants =>
+                variants.map(variant => {
                     const variantWithPrices = this.applyChannelPriceAndTax(variant, ctx);
                     return translateDeep(variantWithPrices, ctx.languageCode, [
                         'options',
@@ -146,7 +146,7 @@ export class ProductVariantService {
         }
 
         return qb.getManyAndCount().then(async ([variants, totalItems]) => {
-            const items = variants.map((variant) => {
+            const items = variants.map(variant => {
                 const variantWithPrices = this.applyChannelPriceAndTax(variant, ctx);
                 return translateDeep(variantWithPrices, ctx.languageCode);
             });
@@ -168,17 +168,15 @@ export class ProductVariantService {
         return this.connection
             .getRepository(ProductVariant)
             .findOne(variantId, { relations: ['options'] })
-            .then((variant) =>
-                !variant ? [] : variant.options.map((o) => translateDeep(o, ctx.languageCode)),
-            );
+            .then(variant => (!variant ? [] : variant.options.map(o => translateDeep(o, ctx.languageCode))));
     }
 
     getFacetValuesForVariant(ctx: RequestContext, variantId: ID): Promise<Array<Translated<FacetValue>>> {
         return this.connection
             .getRepository(ProductVariant)
             .findOne(variantId, { relations: ['facetValues', 'facetValues.facet'] })
-            .then((variant) =>
-                !variant ? [] : variant.facetValues.map((o) => translateDeep(o, ctx.languageCode, ['facet'])),
+            .then(variant =>
+                !variant ? [] : variant.facetValues.map(o => translateDeep(o, ctx.languageCode, ['facet'])),
             );
     }
 
@@ -215,7 +213,7 @@ export class ProductVariantService {
         }
         const updatedVariants = await this.findByIds(
             ctx,
-            input.map((i) => i.id),
+            input.map(i => i.id),
         );
         this.eventBus.publish(new ProductVariantEvent(ctx, updatedVariants, 'updated'));
         return updatedVariants;
@@ -235,7 +233,7 @@ export class ProductVariantService {
             input,
             entityType: ProductVariant,
             translationType: ProductVariantTranslation,
-            beforeSave: async (variant) => {
+            beforeSave: async variant => {
                 const { optionIds } = input;
                 if (optionIds && optionIds.length) {
                     const selectedOptions = await this.connection
@@ -280,7 +278,7 @@ export class ProductVariantService {
             input,
             entityType: ProductVariant,
             translationType: ProductVariantTranslation,
-            beforeSave: async (updatedVariant) => {
+            beforeSave: async updatedVariant => {
                 if (input.taxCategoryId) {
                     const taxCategory = await this.taxCategoryService.findOne(input.taxCategoryId);
                     if (taxCategory) {
@@ -352,9 +350,7 @@ export class ProductVariantService {
      * Populates the `price` field with the price for the specified channel.
      */
     applyChannelPriceAndTax(variant: ProductVariant, ctx: RequestContext): ProductVariant {
-        const channelPrice = variant.productVariantPrices.find((p) =>
-            idsAreEqual(p.channelId, ctx.channelId),
-        );
+        const channelPrice = variant.productVariantPrices.find(p => idsAreEqual(p.channelId, ctx.channelId));
         if (!channelPrice) {
             throw new InternalServerError(`error.no-price-found-for-channel`);
         }
@@ -408,7 +404,7 @@ export class ProductVariantService {
         if (
             !samplesEach(
                 optionIds,
-                optionGroups.map((g) => g.options.map((o) => o.id)),
+                optionGroups.map(g => g.options.map(o => o.id)),
             )
         ) {
             this.throwIncompatibleOptionsError(optionGroups);
@@ -427,14 +423,16 @@ export class ProductVariantService {
 
         const inputOptionIds = this.sortJoin(optionIds, ',');
 
-        product.variants.forEach((variant) => {
-            const variantOptionIds = this.sortJoin(variant.options, ',', 'id');
-            if (variantOptionIds === inputOptionIds) {
-                throw new UserInputError('error.product-variant-options-combination-already-exists', {
-                    optionNames: this.sortJoin(variant.options, ', ', 'code'),
-                });
-            }
-        });
+        product.variants
+            .filter(v => !v.deletedAt)
+            .forEach(variant => {
+                const variantOptionIds = this.sortJoin(variant.options, ',', 'id');
+                if (variantOptionIds === inputOptionIds) {
+                    throw new UserInputError('error.product-variant-options-combination-already-exists', {
+                        optionNames: this.sortJoin(variant.options, ', ', 'code'),
+                    });
+                }
+            });
     }
 
     private throwIncompatibleOptionsError(optionGroups: ProductOptionGroup[]) {
@@ -445,7 +443,7 @@ export class ProductVariantService {
 
     private sortJoin<T>(arr: T[], glue: string, prop?: keyof T): string {
         return arr
-            .map((x) => (prop ? x[prop] : x))
+            .map(x => (prop ? x[prop] : x))
             .sort()
             .join(glue);
     }