From 07f1ae2df8976a387e2c38fb75820b63eb01105d Mon Sep 17 00:00:00 2001 From: Stefan Hauke Date: Tue, 23 Aug 2022 14:21:58 +0200 Subject: [PATCH 1/2] Revert "fix: allow same product list call after a specified time (#1140)" This reverts commit c4bfe88d92e326ea6cbae69f8a29ddba82fcff12. --- .../product-listing.effects.ts | 8 +++-- src/app/core/utils/operators.ts | 36 +------------------ 2 files changed, 6 insertions(+), 38 deletions(-) diff --git a/src/app/core/store/shopping/product-listing/product-listing.effects.ts b/src/app/core/store/shopping/product-listing/product-listing.effects.ts index 0c27a2ee9e..95c3ee92b1 100644 --- a/src/app/core/store/shopping/product-listing/product-listing.effects.ts +++ b/src/app/core/store/shopping/product-listing/product-listing.effects.ts @@ -19,7 +19,7 @@ import { } from 'ish-core/store/shopping/filter'; import { loadProductsForCategory, loadProductsForMaster } from 'ish-core/store/shopping/products'; import { searchProducts } from 'ish-core/store/shopping/search'; -import { mapToPayload, throttleOrChanged, whenFalsy, whenTruthy } from 'ish-core/utils/operators'; +import { mapToPayload, whenFalsy, whenTruthy } from 'ish-core/utils/operators'; import { stringToFormParams } from 'ish-core/utils/url-form-params'; import { @@ -106,7 +106,7 @@ export class ProductListingEffects { }) ); }), - throttleOrChanged(5000), + distinctUntilChanged(isEqual), map(({ id, filters, sorting, page }) => loadMoreProductsForParams({ id, filters, sorting, page })) ) ); @@ -115,6 +115,7 @@ export class ProductListingEffects { this.actions$.pipe( ofType(loadMoreProductsForParams), mapToPayload(), + distinctUntilChanged(isEqual), map(({ id, sorting, page, filters }) => { if (filters) { const searchParameter = filters; @@ -132,7 +133,8 @@ export class ProductListingEffects { } } }), - whenTruthy() + whenTruthy(), + distinctUntilChanged(isEqual) ) ); diff --git a/src/app/core/utils/operators.ts b/src/app/core/utils/operators.ts index 34f3381a4c..d410b3cbd2 100644 --- a/src/app/core/utils/operators.ts +++ b/src/app/core/utils/operators.ts @@ -1,6 +1,5 @@ import { ofType } from '@ngrx/effects'; import { Action, ActionCreator } from '@ngrx/store'; -import { isEqual } from 'lodash-es'; import { MonoTypeOperatorFunction, NEVER, @@ -12,17 +11,7 @@ import { throwError, connect, } from 'rxjs'; -import { - buffer, - catchError, - distinctUntilChanged, - filter, - map, - mergeAll, - scan, - take, - withLatestFrom, -} from 'rxjs/operators'; +import { buffer, catchError, distinctUntilChanged, filter, map, mergeAll, take, withLatestFrom } from 'rxjs/operators'; import { HttpError } from 'ish-core/models/http-error/http-error.model'; @@ -106,26 +95,3 @@ export function delayUntil(notifier: Observable): OperatorFunction concat(concat(connected, NEVER).pipe(buffer(notifier), take(1), mergeAll()), connected)) ); } - -/** - * Throttle observable emissions for specified duration or until a new value was emitted - * - * Taken from https://stackoverflow.com/questions/53623221/rxjs-throttle-same-value-but-let-new-values-through - * - * @param duration specified time where observable emissions are ignored - * @param equals callback function to check if value changes changed - * @returns - */ -export function throttleOrChanged(duration: number, equals: (a: T, b: T) => boolean = isEqual) { - return (source: Observable) => - source.pipe( - map(x => ({ val: x, time: Date.now(), keep: true })), - scan((acc, cur) => { - const diff = cur.time - acc.time; - const isSame = equals(acc.val, cur.val); - return diff > duration || (diff < duration && !isSame) ? { ...cur, keep: true } : { ...acc, keep: false }; - }), - filter(x => x.keep), - map(x => x.val) - ); -} From e669196e7c90a7dc5e216d472d3b44e2f02ac3b9 Mon Sep 17 00:00:00 2001 From: Stefan Hauke Date: Tue, 23 Aug 2022 17:43:26 +0200 Subject: [PATCH 2/2] fix: do not override product information in the state with information with lower completeness level (#1256) * overriding product detail response information with product information from a products list call removed product images, description etc. * now only the availability information from product information with a lower completeness level is merged into the existing product information --- .../models/product/product.helper.spec.ts | 103 ++++++++++++++++++ src/app/core/models/product/product.helper.ts | 32 ++++++ .../shopping/products/products.reducer.ts | 11 +- .../products/products.selectors.spec.ts | 49 +++++---- 4 files changed, 166 insertions(+), 29 deletions(-) diff --git a/src/app/core/models/product/product.helper.spec.ts b/src/app/core/models/product/product.helper.spec.ts index 7d9c4286f7..ae7bcc48da 100644 --- a/src/app/core/models/product/product.helper.spec.ts +++ b/src/app/core/models/product/product.helper.spec.ts @@ -379,4 +379,107 @@ describe('Product Helper', () => { }); }); }); + + describe('updateProductInformation()', () => { + let detailProduct: Product; + let listProduct: Product; + let stubProduct: Product; + let stubProduct2: Product; + + beforeEach(() => { + detailProduct = { + sku: '110', + completenessLevel: ProductCompletenessLevel.Detail, + name: 'Detail Product', + manufacturer: 'Detail Manufacturer', + shortDescription: 'The best product', + available: true, + } as Product; + listProduct = { + sku: '110', + completenessLevel: ProductCompletenessLevel.List, + name: 'List Product', + manufacturer: 'List Manufacturer', + available: false, + } as Product; + stubProduct = { + sku: '110', + completenessLevel: 0, + name: 'Stub Product', + available: false, + } as Product; + stubProduct2 = { + sku: '110', + completenessLevel: 0, + name: 'Stub Product 2', + available: true, + } as Product; + }); + + it('should return current product information if no new product information is provided', () => { + expect(ProductHelper.updateProductInformation(detailProduct, undefined)).toMatchInlineSnapshot(` + Object { + "available": true, + "completenessLevel": 3, + "manufacturer": "Detail Manufacturer", + "name": "Detail Product", + "shortDescription": "The best product", + "sku": "110", + } + `); + }); + + it('should return new product information if no product information exists', () => { + expect(ProductHelper.updateProductInformation(undefined, stubProduct)).toMatchInlineSnapshot(` + Object { + "available": false, + "completenessLevel": 0, + "name": "Stub Product", + "sku": "110", + } + `); + }); + + it('should return new product information if completeness level ist higher', () => { + expect(ProductHelper.updateProductInformation(listProduct, detailProduct)).toMatchInlineSnapshot(` + Object { + "available": true, + "completenessLevel": 3, + "manufacturer": "Detail Manufacturer", + "name": "Detail Product", + "shortDescription": "The best product", + "sku": "110", + } + `); + }); + + it('should return new product information if completeness level ist equal', () => { + expect(ProductHelper.updateProductInformation(stubProduct, stubProduct2)).toMatchInlineSnapshot(` + Object { + "available": true, + "completenessLevel": 0, + "name": "Stub Product 2", + "sku": "110", + } + `); + }); + + it('should return updated current product information if completeness level ist lower', () => { + expect(ProductHelper.updateProductInformation(detailProduct, stubProduct)).toMatchInlineSnapshot(` + Object { + "available": false, + "availableStock": undefined, + "completenessLevel": 3, + "manufacturer": "Detail Manufacturer", + "name": "Detail Product", + "shortDescription": "The best product", + "sku": "110", + } + `); + }); + + it('should return undefined if no current or new product information is provided', () => { + expect(ProductHelper.updateProductInformation(undefined, undefined)).toMatchInlineSnapshot(`undefined`); + }); + }); }); diff --git a/src/app/core/models/product/product.helper.ts b/src/app/core/models/product/product.helper.ts index b76de0869a..faf698eb06 100644 --- a/src/app/core/models/product/product.helper.ts +++ b/src/app/core/models/product/product.helper.ts @@ -196,4 +196,36 @@ export class ProductHelper { const attributes = product.attributes?.filter(att => !common.includes(att.name)); return { ...product, attributes }; } + + /** + * Updates current product information with new product information considering completeness levels and dynamic product information + * + * @param currentProduct The already available product information + * @param newProduct The new product information to update the existing information with + * @returns The updated product information + */ + static updateProductInformation( + currentProduct: Partial, + newProduct: Partial + ): AllProductTypes { + // if there is no new product information return the current product information + if (!newProduct) { + return currentProduct as AllProductTypes; + } + // set the current product information as base or construct an empty product stub + let product = currentProduct || { completenessLevel: 0 }; + // update the complete product information if the newProduct information + // has a higher or equal completeness level than the product information + if (!product.completenessLevel || newProduct.completenessLevel >= product.completenessLevel) { + return newProduct as AllProductTypes; + } + // always update dynamic product information with the new product information (e.g. availability) + product = { + ...product, + // list of product properties that should be updated + available: newProduct.available ?? product.available, + availableStock: newProduct.availableStock ?? product.availableStock, + }; + return product as AllProductTypes; + } } diff --git a/src/app/core/store/shopping/products/products.reducer.ts b/src/app/core/store/shopping/products/products.reducer.ts index a7a01ea714..f59e28d59b 100644 --- a/src/app/core/store/shopping/products/products.reducer.ts +++ b/src/app/core/store/shopping/products/products.reducer.ts @@ -2,7 +2,7 @@ import { EntityState, createEntityAdapter } from '@ngrx/entity'; import { createReducer, on } from '@ngrx/store'; import { ProductLinksDictionary } from 'ish-core/models/product-links/product-links.model'; -import { AllProductTypes, SkuQuantityType } from 'ish-core/models/product/product.model'; +import { AllProductTypes, ProductHelper, SkuQuantityType } from 'ish-core/models/product/product.model'; import { loadProductFail, @@ -50,14 +50,7 @@ export const productsReducer = createReducer( })), on(loadProductSuccess, (state, action) => { const product = action.payload.product; - const oldProduct = state.entities[product.sku] || { completenessLevel: 0 }; - - const newProduct = { ...product }; - if (product.completenessLevel || oldProduct?.completenessLevel) { - newProduct.completenessLevel = Math.max(product.completenessLevel, oldProduct.completenessLevel); - } - - return productAdapter.upsertOne(newProduct as AllProductTypes, { + return productAdapter.upsertOne(ProductHelper.updateProductInformation(state.entities[product.sku], product), { ...state, failed: removeFailed(state.failed, product.sku), }); diff --git a/src/app/core/store/shopping/products/products.selectors.spec.ts b/src/app/core/store/shopping/products/products.selectors.spec.ts index d65c265d83..6454e115fd 100644 --- a/src/app/core/store/shopping/products/products.selectors.spec.ts +++ b/src/app/core/store/shopping/products/products.selectors.spec.ts @@ -76,26 +76,35 @@ describe('Products Selectors', () => { expect(getProductEntities(store$.state)).toEqual({ [prod.sku]: prod }); }); - it.each([ - [ProductCompletenessLevel.Detail, ProductCompletenessLevel.Detail - 1], - [ProductCompletenessLevel.Detail + 1, ProductCompletenessLevel.Detail + 1], - ])( - 'should merge product updates with "%s" completeness when new info with "%s" completeness is available', - (resultCompletenessLevel, completenessLevel) => { - const newProd = { - ...prod, - completenessLevel, - manufacturer: 'Microsoft', - name: 'Updated product', - available: false, - } as Product; - store$.dispatch(loadProductSuccess({ product: newProd })); - - expect(getProductEntities(store$.state)).toEqual({ - [prod.sku]: { ...newProd, completenessLevel: resultCompletenessLevel }, - }); - } - ); + it('should update product information with the same completeness level', () => { + const newProd = { + ...prod, + completenessLevel: ProductCompletenessLevel.Detail, + manufacturer: 'Microsoft', + name: 'Updated product', + available: false, + } as Product; + store$.dispatch(loadProductSuccess({ product: newProd })); + + expect(getProductEntities(store$.state)).toEqual({ + [prod.sku]: { ...newProd }, + }); + }); + + it('should only partially update product information with a lower completeness level', () => { + const newProd = { + ...prod, + completenessLevel: ProductCompletenessLevel.List, + manufacturer: 'Microsoft', + name: 'Updated product', + available: false, + } as Product; + store$.dispatch(loadProductSuccess({ product: newProd })); + + expect(getProductEntities(store$.state)).toEqual({ + [prod.sku]: { ...prod, available: false, availableStock: undefined }, + }); + }); }); describe('and reporting failure', () => {