From f4b2d4153c799f28e0b95662c3fcde40f5a78d79 Mon Sep 17 00:00:00 2001 From: Danilo Hoffmann Date: Mon, 29 Apr 2024 15:17:21 +0200 Subject: [PATCH] refactor: disconnect master product from product view model (#1644) BREAKING CHANGE: The productMaster property on the product view model has been removed. The master product should be individually retrieved. --- docs/guides/migrations.md | 4 +++- .../core/facades/product-context.facade.ts | 9 ++++++++ .../product-variation.helper.spec.ts | 12 +++++----- .../product-variation.helper.ts | 22 +++++++++++++------ .../models/product-view/product-view.model.ts | 3 --- .../shopping/products/products.selectors.ts | 21 ++---------------- ...product-variation-select.component.spec.ts | 4 ++++ .../product-variation-select.component.ts | 13 ++++++++--- 8 files changed, 49 insertions(+), 39 deletions(-) diff --git a/docs/guides/migrations.md b/docs/guides/migrations.md index 42c03c40e1..ac804ebc73 100644 --- a/docs/guides/migrations.md +++ b/docs/guides/migrations.md @@ -43,7 +43,9 @@ Product variations were eagerly loaded via effects. In projects with a lot of Variations, this can lead to performance issues, especially if the variations data is not needed for the current views. For that reason product variations are now loaded lazily through the following changes that might need adaptions with project customizations. -- The variations property on the product view interface was removed. Variations can now be retrieved via the product context facade or the shopping facade. +- The `variations` property on the product view interface was removed. Variations can now be retrieved via the product context facade or the shopping facade. + +* The `productMaster` property on the product view model has been removed. The master product should be individually retrieved. ## From 5.0 to 5.1 diff --git a/src/app/core/facades/product-context.facade.ts b/src/app/core/facades/product-context.facade.ts index fd4b528376..8b239f4a56 100644 --- a/src/app/core/facades/product-context.facade.ts +++ b/src/app/core/facades/product-context.facade.ts @@ -28,6 +28,7 @@ import { ProductHelper, SkuQuantityType, VariationProduct, + VariationProductMaster, } from 'ish-core/models/product/product.model'; import { Promotion } from 'ish-core/models/promotion/promotion.model'; import { generateProductUrl } from 'ish-core/routing/product/product.route'; @@ -109,6 +110,7 @@ export interface ProductContext { parts: SkuQuantityType[]; variations: VariationProduct[]; variationCount: number; + productMaster: VariationProductMaster; // quantity quantity: number; @@ -453,6 +455,13 @@ export class ProductContextFacade extends RxState implements OnD case 'variationCount': wrap('variationCount', this.shoppingFacade.productVariationCount$(this.validProductSKU$)); break; + case 'productMaster': + wrap( + 'productMaster', + this.shoppingFacade + .product$(this.masterProductSKU$, ProductCompletenessLevel.List) + .pipe(filter(ProductHelper.isMasterProduct)) + ); case 'links': wrap('links', this.shoppingFacade.productLinks$(this.validProductSKU$)); break; diff --git a/src/app/core/models/product-variation/product-variation.helper.spec.ts b/src/app/core/models/product-variation/product-variation.helper.spec.ts index 0496e75e8e..57ad4d937c 100644 --- a/src/app/core/models/product-variation/product-variation.helper.spec.ts +++ b/src/app/core/models/product-variation/product-variation.helper.spec.ts @@ -1,5 +1,4 @@ import { FilterNavigation } from 'ish-core/models/filter-navigation/filter-navigation.model'; -import { ProductView } from 'ish-core/models/product-view/product-view.model'; import { VariationProduct, VariationProductMaster } from 'ish-core/models/product/product.model'; import { ProductVariationHelper } from './product-variation.helper'; @@ -67,15 +66,16 @@ const productMaster = { ], } as VariationProductMaster; -const variationProduct = { - ...productVariations[0], - productMaster, -} as ProductView; +const variationProduct = productVariations[0]; describe('Product Variation Helper', () => { describe('buildVariationOptionGroups', () => { it('should build variation option groups for variation product', () => { - const result = ProductVariationHelper.buildVariationOptionGroups(variationProduct, productVariations); + const result = ProductVariationHelper.buildVariationOptionGroups( + variationProduct, + productMaster, + productVariations + ); expect(result).toMatchInlineSnapshot(` [ { diff --git a/src/app/core/models/product-variation/product-variation.helper.ts b/src/app/core/models/product-variation/product-variation.helper.ts index 50b8bbb067..f91bfdaa2f 100644 --- a/src/app/core/models/product-variation/product-variation.helper.ts +++ b/src/app/core/models/product-variation/product-variation.helper.ts @@ -2,7 +2,7 @@ import { groupBy } from 'lodash-es'; import { FilterNavigation } from 'ish-core/models/filter-navigation/filter-navigation.model'; import { ProductView } from 'ish-core/models/product-view/product-view.model'; -import { VariationProduct } from 'ish-core/models/product/product.model'; +import { VariationProduct, VariationProductMaster } from 'ish-core/models/product/product.model'; import { omit } from 'ish-core/utils/functions'; import { VariationAttribute } from './variation-attribute.model'; @@ -13,13 +13,17 @@ export class ProductVariationHelper { /** * Build select value structure */ - static buildVariationOptionGroups(product: ProductView, variations: VariationProduct[]): VariationOptionGroup[] { - if (!product?.variableVariationAttributes?.length) { + static buildVariationOptionGroups( + variationProduct: VariationProduct, + productMaster: VariationProductMaster, + variations: VariationProduct[] + ): VariationOptionGroup[] { + if (!variationProduct?.variableVariationAttributes?.length) { return []; } // transform currently selected variation attribute list to object with the attributeId as key - const currentSettings = product.variableVariationAttributes.reduce<{ [id: string]: VariationAttribute }>( + const currentSettings = variationProduct.variableVariationAttributes.reduce<{ [id: string]: VariationAttribute }>( (acc, attr) => ({ ...acc, [attr.variationAttributeId]: attr, @@ -29,7 +33,7 @@ export class ProductVariationHelper { // transform all variation attribute values to selectOptions // each with information about alternative combinations and active status (active status comes from currently selected variation) - const options: VariationSelectOption[] = (product.productMaster?.variationAttributeValues || []) + const options: VariationSelectOption[] = (productMaster?.variationAttributeValues || []) .map(attr => ({ label: ProductVariationHelper.toDisplayValue(attr.value), value: ProductVariationHelper.toValue(attr.value)?.toString(), @@ -39,7 +43,11 @@ export class ProductVariationHelper { })) .map(option => ({ ...option, - alternativeCombination: ProductVariationHelper.alternativeCombinationCheck(option, product, variations), + alternativeCombination: ProductVariationHelper.alternativeCombinationCheck( + option, + variationProduct, + variations + ), })); // group options list by attributeId @@ -48,7 +56,7 @@ export class ProductVariationHelper { // go through those groups and transform them to more complex objects return Object.keys(groupedOptions).map(attrId => { // we need to get one of the original attributes again here, because we lost the attribute name - const attribute = product.productMaster.variationAttributeValues.find(a => a.variationAttributeId === attrId); + const attribute = productMaster.variationAttributeValues.find(a => a.variationAttributeId === attrId); return { id: attribute.variationAttributeId, label: attribute.name, diff --git a/src/app/core/models/product-view/product-view.model.ts b/src/app/core/models/product-view/product-view.model.ts index 5a26025e51..2899559099 100644 --- a/src/app/core/models/product-view/product-view.model.ts +++ b/src/app/core/models/product-view/product-view.model.ts @@ -11,7 +11,6 @@ interface SimpleProductView extends Product { interface VariationProductView extends VariationProduct, SimpleProductView { type: VariationProduct['type']; - productMaster: VariationProductMaster; } interface VariationProductMasterView extends VariationProductMaster, SimpleProductView { @@ -44,14 +43,12 @@ export function createVariationProductMasterView( export function createVariationProductView( product: VariationProduct, - productMaster: VariationProductMaster, defaultCategory?: CategoryView ): VariationProductView { return ( product && { ...createProductView(product, defaultCategory), type: 'VariationProduct', - productMaster, } ); } diff --git a/src/app/core/store/shopping/products/products.selectors.ts b/src/app/core/store/shopping/products/products.selectors.ts index ed2dfa52d0..3430a12db5 100644 --- a/src/app/core/store/shopping/products/products.selectors.ts +++ b/src/app/core/store/shopping/products/products.selectors.ts @@ -17,7 +17,6 @@ import { ProductCompletenessLevel, ProductHelper, VariationProduct, - VariationProductMaster, } from 'ish-core/models/product/product.model'; import { generateCategoryUrl } from 'ish-core/routing/category/category.route'; import { selectRouteParam } from 'ish-core/store/core/router'; @@ -73,33 +72,17 @@ export const getProductVariations = (sku: string) => variations?.map(variationSku => productOrFailedStub(state, variationSku)) || [] ); -const internalProductMasterSKU = (sku: string) => - /* memoization automatically by output: string identity */ - createSelector( - internalRawProduct(sku), - product => ProductHelper.isVariationProduct(product) && product.productMasterSKU - ); - -const internalProductMaster = (sku: string) => - /* memoization manually by output: master SKU doesn't vary, but reference to state changes often */ - createSelectorFactory(projector => resultMemoize(projector, isEqual))( - getProductsState, - internalProductMasterSKU(sku), - productOrFailedStub - ); - export const getProduct = (sku: string) => /* memoization automatically by inputs: as long as all dependant selectors are properly memoized */ createSelector( internalRawProduct(sku), internalProductDefaultCategory(sku), internalProductDefaultVariationSKU(sku), - internalProductMaster(sku), - (product, defaultCategory, defaultVariationSKU, productMaster): ProductView => + (product, defaultCategory, defaultVariationSKU): ProductView => ProductHelper.isMasterProduct(product) ? createVariationProductMasterView(product, defaultVariationSKU, defaultCategory) : ProductHelper.isVariationProduct(product) - ? createVariationProductView(product, productMaster, defaultCategory) + ? createVariationProductView(product, defaultCategory) : createProductView(product, defaultCategory) ); diff --git a/src/app/shared/components/product/product-variation-select/product-variation-select.component.spec.ts b/src/app/shared/components/product/product-variation-select/product-variation-select.component.spec.ts index bbeed085bb..d0d80deaaf 100644 --- a/src/app/shared/components/product/product-variation-select/product-variation-select.component.spec.ts +++ b/src/app/shared/components/product/product-variation-select/product-variation-select.component.spec.ts @@ -20,6 +20,7 @@ describe('Product Variation Select Component', () => { let context: ProductContextFacade; const productMaster = { + type: 'VariationProductMaster', variationAttributeValues: [ { variationAttributeId: 'a1', value: 'A', attributeType: 'colorCode' }, { variationAttributeId: 'a1', value: 'B', attributeType: 'colorCode' }, @@ -35,6 +36,7 @@ describe('Product Variation Select Component', () => { } as VariationProductMaster; const variationProduct = { + type: 'VariationProduct', variableVariationAttributes: [ { variationAttributeId: 'a1', value: 'B', attributeType: 'colorCode' }, { variationAttributeId: 'a2', value: 'D', attributeType: 'defaultAndColorCode' }, @@ -51,6 +53,7 @@ describe('Product Variation Select Component', () => { beforeEach(async () => { context = mock(ProductContextFacade); + await TestBed.configureTestingModule({ declarations: [ MockComponent(ProductVariationSelectDefaultComponent), @@ -70,6 +73,7 @@ describe('Product Variation Select Component', () => { when(context.select('product')).thenReturn(of(variationProductView)); when(context.select('variations')).thenReturn(of([variationProduct])); when(context.select('displayProperties', 'variations')).thenReturn(of(true)); + when(context.select('productMaster')).thenReturn(of(productMaster)); }); it('should be created', () => { diff --git a/src/app/shared/components/product/product-variation-select/product-variation-select.component.ts b/src/app/shared/components/product/product-variation-select/product-variation-select.component.ts index c3a296fe13..ddb560601b 100644 --- a/src/app/shared/components/product/product-variation-select/product-variation-select.component.ts +++ b/src/app/shared/components/product/product-variation-select/product-variation-select.component.ts @@ -1,11 +1,12 @@ import { ChangeDetectionStrategy, Component, OnInit } from '@angular/core'; import { Observable, combineLatest } from 'rxjs'; -import { map } from 'rxjs/operators'; +import { filter, map } from 'rxjs/operators'; import { v4 as uuid } from 'uuid'; import { ProductContextFacade } from 'ish-core/facades/product-context.facade'; import { ProductVariationHelper } from 'ish-core/models/product-variation/product-variation.helper'; import { VariationOptionGroup } from 'ish-core/models/product-variation/variation-option-group.model'; +import { ProductHelper } from 'ish-core/models/product/product.helper'; @Component({ selector: 'ish-product-variation-select', @@ -20,8 +21,14 @@ export class ProductVariationSelectComponent implements OnInit { constructor(private context: ProductContextFacade) {} ngOnInit() { - this.variationOptions$ = combineLatest([this.context.select('product'), this.context.select('variations')]).pipe( - map(([product, variations]) => ProductVariationHelper.buildVariationOptionGroups(product, variations)) + this.variationOptions$ = combineLatest([ + this.context.select('product').pipe(filter(ProductHelper.isVariationProduct)), + this.context.select('variations'), + this.context.select('productMaster'), + ]).pipe( + map(([product, variations, masterProduct]) => + ProductVariationHelper.buildVariationOptionGroups(product, masterProduct, variations) + ) ); this.visible$ = this.context.select('displayProperties', 'variations'); }