fix(admin-ui): Fix various issues with product variant management view

Relates to #602

packages/admin-ui/src/lib/catalog/src/components/product-variants-editor/product-variants-editor.component.html

@@ -13,12 +13,7 @@
 <div *ngFor="let group of optionGroups" class="option-groups">
     <div class="name">
         <label>{{ 'catalog.option' | translate }}</label>
-        <input
-            clrInput
-            [(ngModel)]="group.name"
-            name="name"
-            [readonly]="!group.isNew"
-        />
+        <input clrInput [(ngModel)]="group.name" name="name" [readonly]="!group.isNew" />
     </div>
     <div class="values">
         <label>{{ 'catalog.option-values' | translate }}</label>
@@ -31,7 +26,11 @@
         ></vdr-option-value-input>
     </div>
 </div>
-<button class="btn btn-primary-outline btn-sm" (click)="addOption()" *ngIf="product.variants.length === 1">
+<button
+    class="btn btn-primary-outline btn-sm"
+    (click)="addOption()"
+    *ngIf="product?.variants.length === 1 && product?.optionGroups.length === 0"
+>
     <clr-icon shape="plus"></clr-icon>
     {{ 'catalog.add-option' | translate }}
 </button>
@@ -39,74 +38,71 @@
 <div class="variants-preview">
     <table class="table">
         <thead>
-        <tr>
-            <th>{{ 'common.create' | translate }}</th>
-            <th>{{ 'catalog.variant' | translate }}</th>
-            <th>{{ 'catalog.sku' | translate }}</th>
-            <th>{{ 'catalog.price' | translate }}</th>
-            <th>{{ 'catalog.stock-on-hand' | translate }}</th>
-            <th></th>
-        </tr>
+            <tr>
+                <th>{{ 'common.create' | translate }}</th>
+                <th>{{ 'catalog.variant' | translate }}</th>
+                <th>{{ 'catalog.sku' | translate }}</th>
+                <th>{{ 'catalog.price' | translate }}</th>
+                <th>{{ 'catalog.stock-on-hand' | translate }}</th>
+                <th></th>
+            </tr>
         </thead>
-        <tr
-            *ngFor="let variant of variants"
-            [class.disabled]="!variantFormValues[variant.id].enabled || variantFormValues[variant.id].existing"
-        >
+        <tr *ngFor="let variant of generatedVariants" [class.disabled]="!variant.enabled || variant.existing">
             <td>
                 <input
                     type="checkbox"
-                    *ngIf="!variantFormValues[variant.id].existing"
-                    [(ngModel)]="variantFormValues[variant.id].enabled"
+                    *ngIf="!variant.existing"
+                    [(ngModel)]="variant.enabled"
                     name="enabled"
                     clrCheckbox
                     (ngModelChange)="formValueChanged = true"
                 />
             </td>
             <td>
-                {{ getVariantName(variant) }}
+                {{ getVariantName(variant) | translate }}
             </td>
             <td>
-                <clr-input-container *ngIf="!variantFormValues[variant.id].existing">
+                <clr-input-container *ngIf="!variant.existing">
                     <input
                         clrInput
                         type="text"
-                        [(ngModel)]="variantFormValues[variant.id].sku"
+                        [(ngModel)]="variant.sku"
                         [placeholder]="'catalog.sku' | translate"
                         name="sku"
                         required
-                        (ngModelChange)="onFormChanged(variantFormValues[variant.id])"
+                        (ngModelChange)="onFormChanged(variant)"
                     />
                 </clr-input-container>
-                <span *ngIf="variantFormValues[variant.id].existing">{{ variantFormValues[variant.id].sku }}</span>
+                <span *ngIf="variant.existing">{{ variant.sku }}</span>
             </td>
             <td>
-                <clr-input-container *ngIf="!variantFormValues[variant.id].existing">
+                <clr-input-container *ngIf="!variant.existing">
                     <vdr-currency-input
                         clrInput
-                        [(ngModel)]="variantFormValues[variant.id].price"
+                        [(ngModel)]="variant.price"
                         name="price"
                         [currencyCode]="currencyCode"
-                        (ngModelChange)="onFormChanged(variantFormValues[variant.id])"
+                        (ngModelChange)="onFormChanged(variant)"
                     ></vdr-currency-input>
                 </clr-input-container>
-                <span *ngIf="variantFormValues[variant.id].existing">{{ variantFormValues[variant.id].price | localeCurrency: currencyCode }}</span>
+                <span *ngIf="variant.existing">{{ variant.price | localeCurrency: currencyCode }}</span>
             </td>
             <td>
-                <clr-input-container *ngIf="!variantFormValues[variant.id].existing">
+                <clr-input-container *ngIf="!variant.existing">
                     <input
                         clrInput
                         type="number"
-                        [(ngModel)]="variantFormValues[variant.id].stock"
+                        [(ngModel)]="variant.stock"
                         name="stock"
                         min="0"
                         step="1"
-                        (ngModelChange)="onFormChanged(variantFormValues[variant.id])"
+                        (ngModelChange)="onFormChanged(variant)"
                     />
                 </clr-input-container>
-                <span *ngIf="variantFormValues[variant.id].existing">{{ variantFormValues[variant.id].stock }}</span>
+                <span *ngIf="variant.existing">{{ variant.stock }}</span>
             </td>
             <td>
-                <vdr-dropdown *ngIf="variantFormValues[variant.id].productVariantId as productVariantId">
+                <vdr-dropdown *ngIf="variant.productVariantId as productVariantId">
                     <button class="icon-button" vdrDropdownTrigger>
                         <clr-icon shape="ellipsis-vertical"></clr-icon>
                     </button>
@@ -121,7 +117,6 @@
                             {{ 'common.delete' | translate }}
                         </button>
                     </vdr-dropdown-menu>
-
                 </vdr-dropdown>
             </td>
         </tr>

packages/admin-ui/src/lib/catalog/src/components/product-variants-editor/product-variants-editor.component.ts

@@ -15,30 +15,30 @@ import {
     ProductOptionGroupWithOptionsFragment,
 } from '@vendure/admin-ui/core';
 import { normalizeString } from '@vendure/common/lib/normalize-string';
+import { pick } from '@vendure/common/lib/pick';
 import { generateAllCombinations, notNullOrUndefined } from '@vendure/common/lib/shared-utils';
 import { EMPTY, forkJoin, Observable, of } from 'rxjs';
-import { filter, map, mergeMap, switchMap, take } from 'rxjs/operators';
+import { filter, map, mergeMap, switchMap } from 'rxjs/operators';
 
 import { ProductDetailService } from '../../providers/product-detail/product-detail.service';
 
-export interface VariantInfo {
+export class GeneratedVariant {
+    isDefault: boolean;
+    options: Array<{ name: string; id?: string }>;
     productVariantId?: string;
     enabled: boolean;
     existing: boolean;
-    options: string[];
     sku: string;
     price: number;
     stock: number;
-}
 
-export interface GeneratedVariant {
-    isDefault: boolean;
-    id: string;
-    options: Array<{ name: string; id?: string }>;
+    constructor(config: Partial<GeneratedVariant>) {
+        for (const key of Object.keys(config)) {
+            this[key] = config[key];
+        }
+    }
 }
 
-const DEFAULT_VARIANT_CODE = '__DEFAULT_VARIANT__';
-
 @Component({
     selector: 'vdr-product-variants-editor',
     templateUrl: './product-variants-editor.component.html',
@@ -47,7 +47,7 @@ const DEFAULT_VARIANT_CODE = '__DEFAULT_VARIANT__';
 })
 export class ProductVariantsEditorComponent implements OnInit, DeactivateAware {
     formValueChanged = false;
-    variants: GeneratedVariant[] = [];
+    generatedVariants: GeneratedVariant[] = [];
     optionGroups: Array<{
         id?: string;
         isNew: boolean;
@@ -58,7 +58,6 @@ export class ProductVariantsEditorComponent implements OnInit, DeactivateAware {
             locked: boolean;
         }>;
     }>;
-    variantFormValues: { [id: string]: VariantInfo } = {};
     product: GetProductVariantOptions.Product;
     currencyCode: CurrencyCode;
     private languageCode: LanguageCode;
@@ -80,7 +79,7 @@ export class ProductVariantsEditorComponent implements OnInit, DeactivateAware {
         });
     }
 
-    onFormChanged(variantInfo: VariantInfo) {
+    onFormChanged(variantInfo: GeneratedVariant) {
         this.formValueChanged = true;
         variantInfo.enabled = true;
     }
@@ -90,7 +89,7 @@ export class ProductVariantsEditorComponent implements OnInit, DeactivateAware {
     }
 
     getVariantsToAdd() {
-        return Object.values(this.variantFormValues).filter(v => !v.existing && v.enabled);
+        return this.generatedVariants.filter(v => !v.existing && v.enabled);
     }
 
     getVariantName(variant: GeneratedVariant) {
@@ -109,28 +108,32 @@ export class ProductVariantsEditorComponent implements OnInit, DeactivateAware {
 
     generateVariants() {
         const groups = this.optionGroups.map(g => g.values);
-        const previousVariants = this.variants;
-        this.variants = groups.length
-            ? generateAllCombinations(groups).map((options, i) => ({
-                  isDefault: this.product.variants.length === 1 && i === 0,
-                  id: this.generateOptionsId(options),
-                  options,
-              }))
-            : [{ isDefault: true, id: DEFAULT_VARIANT_CODE, options: [] }];
-
-        this.variants.forEach(variant => {
-            if (!this.variantFormValues[variant.id]) {
-                const prototype = this.getVariantPrototype(variant, previousVariants);
-                this.variantFormValues[variant.id] = {
-                    enabled: false,
-                    existing: false,
-                    options: variant.options.map(o => o.name),
-                    price: prototype.price,
-                    sku: prototype.sku,
-                    stock: prototype.stock,
-                };
-            }
-        });
+        const previousVariants = this.generatedVariants;
+        const generatedVariantFactory = (
+            isDefault: boolean,
+            options: GeneratedVariant['options'],
+            existingVariant?: GetProductVariantOptions.Variants,
+        ): GeneratedVariant => {
+            const prototype = this.getVariantPrototype(options, previousVariants);
+            return new GeneratedVariant({
+                enabled: false,
+                existing: !!existingVariant,
+                productVariantId: existingVariant?.id,
+                isDefault,
+                options,
+                price: existingVariant?.price ?? prototype.price,
+                sku: existingVariant?.sku ?? prototype.sku,
+                stock: existingVariant?.stockOnHand ?? prototype.stock,
+            });
+        };
+        this.generatedVariants = groups.length
+            ? generateAllCombinations(groups).map(options => {
+                  const existingVariant = this.product.variants.find(v =>
+                      this.optionsAreEqual(v.options, options),
+                  );
+                  return generatedVariantFactory(false, options, existingVariant);
+              })
+            : [generatedVariantFactory(true, [], this.product.variants[0])];
     }
 
     /**
@@ -138,17 +141,14 @@ export class ProductVariantsEditorComponent implements OnInit, DeactivateAware {
      * details off.
      */
     private getVariantPrototype(
-        variant: GeneratedVariant,
+        options: GeneratedVariant['options'],
         previousVariants: GeneratedVariant[],
-    ): Pick<VariantInfo, 'sku' | 'price' | 'stock'> {
-        if (variant.isDefault) {
-            return this.variantFormValues[DEFAULT_VARIANT_CODE];
-        }
+    ): Pick<GeneratedVariant, 'sku' | 'price' | 'stock'> {
         const variantsWithSimilarOptions = previousVariants.filter(v =>
-            variant.options.map(o => o.name).filter(name => v.options.map(o => o.name).includes(name)),
+            options.map(o => o.name).filter(name => v.options.map(o => o.name).includes(name)),
         );
         if (variantsWithSimilarOptions.length) {
-            return this.variantFormValues[this.generateOptionsId(variantsWithSimilarOptions[0].options)];
+            return pick(previousVariants[0], ['sku', 'price', 'stock']);
         }
         return {
             sku: '',
@@ -219,7 +219,7 @@ export class ProductVariantsEditorComponent implements OnInit, DeactivateAware {
     }
 
     private confirmDeletionOfDefault(): Observable<boolean> {
-        if (this.product.variants.length === 1) {
+        if (this.hasOnlyDefaultVariant(this.product)) {
             return this.modalService
                 .dialog({
                     title: _('catalog.confirm-adding-options-delete-default-title'),
@@ -239,6 +239,10 @@ export class ProductVariantsEditorComponent implements OnInit, DeactivateAware {
         }
     }
 
+    private hasOnlyDefaultVariant(product: GetProductVariantOptions.Product): boolean {
+        return product.variants.length === 1 && product.optionGroups.length === 0;
+    }
+
     private addOptionGroupsToProduct(
         createdOptionGroups: CreateProductOptionGroup.CreateProductOptionGroup[],
     ): Observable<CreateProductOptionGroup.CreateProductOptionGroup[]> {
@@ -306,14 +310,14 @@ export class ProductVariantsEditorComponent implements OnInit, DeactivateAware {
             .filter(notNullOrUndefined)
             .map(og => og.options)
             .reduce((flat, o) => [...flat, ...o], []);
-        const variants = Object.values(this.variantFormValues)
+        const variants = this.generatedVariants
             .filter(v => v.enabled && !v.existing)
             .map(v => ({
                 price: v.price,
                 sku: v.sku,
                 stock: v.stock,
                 optionIds: v.options
-                    .map(name => options.find(o => o.name === name))
+                    .map(name => options.find(o => o.name === name.name))
                     .filter(notNullOrUndefined)
                     .map(o => o.id),
             }));
@@ -326,7 +330,7 @@ export class ProductVariantsEditorComponent implements OnInit, DeactivateAware {
     }
 
     private deleteDefaultVariant<T>(input: T): Observable<T> {
-        if (this.product.variants.length === 1) {
+        if (this.hasOnlyDefaultVariant(this.product)) {
             // If the default single product variant has been replaced by multiple variants,
             // delete the original default variant.
             return this.dataService.product
@@ -347,15 +351,15 @@ export class ProductVariantsEditorComponent implements OnInit, DeactivateAware {
         }
     }
 
-    private initOptionsAndVariants() {
-        this.route.data
-            .pipe(
-                switchMap(data => data.entity as Observable<GetProductVariantOptions.Product>),
-                take(1),
-            )
-            .subscribe(product => {
-                this.product = product;
-                this.optionGroups = product.optionGroups.map(og => {
+    initOptionsAndVariants() {
+        this.dataService.product
+            // tslint:disable-next-line:no-non-null-assertion
+            .getProductVariantsOptions(this.route.snapshot.paramMap.get('id')!)
+            // tslint:disable-next-line:no-non-null-assertion
+            .mapSingle(({ product }) => product!)
+            .subscribe(p => {
+                this.product = p;
+                this.optionGroups = p.optionGroups.map(og => {
                     return {
                         id: og.id,
                         isNew: false,
@@ -367,35 +371,18 @@ export class ProductVariantsEditorComponent implements OnInit, DeactivateAware {
                         })),
                     };
                 });
-                this.variantFormValues = this.getExistingVariants(product.variants);
                 this.generateVariants();
             });
     }
 
-    private getExistingVariants(
-        variants: GetProductVariantOptions.Variants[],
-    ): { [id: string]: VariantInfo } {
-        return variants.reduce((all, v) => {
-            const id = v.options.length ? this.generateOptionsId(v.options) : DEFAULT_VARIANT_CODE;
-            return {
-                ...all,
-                [id]: {
-                    productVariantId: v.id,
-                    enabled: true,
-                    existing: true,
-                    options: v.options.map(o => o.name),
-                    sku: v.sku,
-                    price: v.price,
-                    stock: v.stockOnHand,
-                },
-            };
-        }, {});
-    }
+    private optionsAreEqual(a: Array<{ name: string }>, b: Array<{ name: string }>): boolean {
+        function toOptionString(o: Array<{ name: string }>) {
+            return o
+                .map(x => x.name)
+                .sort()
+                .join('|');
+        }
 
-    private generateOptionsId(options: GeneratedVariant['options']): string {
-        return options
-            .map(o => o.name)
-            .sort()
-            .join('|');
+        return toOptionString(a) === toOptionString(b);
     }
 }