From 7b94af852b5adb933cd5514c3cec0e37d1d6171e Mon Sep 17 00:00:00 2001 From: Danilo Hoffmann Date: Mon, 17 Feb 2020 13:15:17 +0100 Subject: [PATCH] fixup! feat: SEO friendly localized URLs for product detail pages (#110) --- .../shopping/products/products.effects.spec.ts | 15 +++++++-------- .../store/shopping/products/products.effects.ts | 16 ++++++++++++++-- .../shopping/products/products.selectors.ts | 4 +--- .../shopping/recently/recently.effects.spec.ts | 10 +++++++++- .../store/shopping/recently/recently.effects.ts | 1 + src/app/extensions/seo/store/seo/seo.effects.ts | 3 ++- 6 files changed, 34 insertions(+), 15 deletions(-) diff --git a/src/app/core/store/shopping/products/products.effects.spec.ts b/src/app/core/store/shopping/products/products.effects.spec.ts index dd9619be15a..b55e7cbb208 100644 --- a/src/app/core/store/shopping/products/products.effects.spec.ts +++ b/src/app/core/store/shopping/products/products.effects.spec.ts @@ -347,11 +347,13 @@ describe('Products Effects', () => { }); describe('redirectIfErrorInProducts$', () => { - it('should redirect if triggered on product detail page', fakeAsync(() => { - when(router.url).thenReturn('/category/A/product/SKU'); - - const action = new fromActions.LoadProductFail({ sku: 'SKU', error: { status: 404 } as HttpError }); + beforeEach(() => { + store$.dispatch(new fromActions.LoadProductFail({ sku: 'SKU', error: { status: 404 } as HttpError })); + store$.dispatch(new fromActions.SelectProduct({ sku: 'SKU' })); + }); + it('should redirect if triggered on product detail page', fakeAsync(() => { + const action = new RouteNavigation({ path: 'pr', params: { sku: 'SKU' } }); actions$ = of(action); effects.redirectIfErrorInProducts$.subscribe(noop, fail, noop); @@ -362,10 +364,7 @@ describe('Products Effects', () => { })); it('should not redirect if triggered on page other than product detail page', done => { - when(router.url).thenReturn('/search/term'); - - const action = new fromActions.LoadProductFail({ sku: 'SKU', error: { status: 404 } as HttpError }); - + const action = new RouteNavigation({ path: 'any' }); actions$ = of(action); effects.redirectIfErrorInProducts$.subscribe(fail, fail, done); diff --git a/src/app/core/store/shopping/products/products.effects.ts b/src/app/core/store/shopping/products/products.effects.ts index f115787a7e6..d35056248e6 100644 --- a/src/app/core/store/shopping/products/products.effects.ts +++ b/src/app/core/store/shopping/products/products.effects.ts @@ -8,11 +8,14 @@ import { concatMap, distinct, distinctUntilChanged, + distinctUntilKeyChanged, filter, groupBy, map, mergeMap, switchMap, + switchMapTo, + take, takeUntil, tap, throttleTime, @@ -223,6 +226,7 @@ export class ProductsEffects { this.store.pipe( select(productsSelectors.getSelectedProduct), whenTruthy(), + filter(p => !ProductHelper.isFailedLoading(p)), filter(product => !product.defaultCategory()), mapToProperty('defaultCategoryId'), whenTruthy(), @@ -268,8 +272,16 @@ export class ProductsEffects { @Effect({ dispatch: false }) redirectIfErrorInProducts$ = this.actions$.pipe( - ofType(productsActions.ProductsActionTypes.LoadProductFail), - filter(() => this.router.url.includes('/product/')), + ofProductRoute(), + switchMapTo( + this.store.pipe( + select(productsSelectors.getSelectedProduct), + whenTruthy(), + distinctUntilKeyChanged('sku'), + filter(ProductHelper.isFailedLoading), + take(1) + ) + ), tap(() => this.httpStatusCodeService.setStatusAndRedirect(404)) ); diff --git a/src/app/core/store/shopping/products/products.selectors.ts b/src/app/core/store/shopping/products/products.selectors.ts index 86acc520906..9ba4831b536 100644 --- a/src/app/core/store/shopping/products/products.selectors.ts +++ b/src/app/core/store/shopping/products/products.selectors.ts @@ -109,9 +109,7 @@ export const getProducts = createSelector( export const getSelectedProduct = createSelector( state => state, getSelectedProductId, - getFailed, - (state, sku, failed): ProductView | VariationProductView | VariationProductMasterView => - failed.includes(sku) ? undefined : getProduct(state, { sku }) + (state, sku): ProductView | VariationProductView | VariationProductMasterView => getProduct(state, { sku }) ); export const getProductVariationOptions = createSelector( diff --git a/src/app/core/store/shopping/recently/recently.effects.spec.ts b/src/app/core/store/shopping/recently/recently.effects.spec.ts index de040566fbd..0d44242a999 100644 --- a/src/app/core/store/shopping/recently/recently.effects.spec.ts +++ b/src/app/core/store/shopping/recently/recently.effects.spec.ts @@ -5,10 +5,11 @@ import { cold, hot } from 'jest-marbles'; import { Observable } from 'rxjs'; import { FeatureToggleModule } from 'ish-core/feature-toggle.module'; +import { HttpError } from 'ish-core/models/http-error/http-error.model'; import { Product } from 'ish-core/models/product/product.model'; import { ApplyConfiguration } from 'ish-core/store/configuration'; import { configurationReducer } from 'ish-core/store/configuration/configuration.reducer'; -import { LoadProductSuccess, SelectProduct } from 'ish-core/store/shopping/products'; +import { LoadProductFail, LoadProductSuccess, SelectProduct } from 'ish-core/store/shopping/products'; import { shoppingReducers } from 'ish-core/store/shopping/shopping-store.module'; import { ngrxTesting } from 'ish-core/utils/dev/ngrx-testing'; @@ -54,6 +55,13 @@ describe('Recently Effects', () => { expect(effects.viewedProduct$).toBeObservable(cold('a', { a: new AddToRecently({ sku: 'A' }) })); }); + it('should not fire when product failed loading', () => { + store$.dispatch(new LoadProductFail({ error: {} as HttpError, sku: 'A' })); + store$.dispatch(new SelectProduct({ sku: 'A' })); + + expect(effects.viewedProduct$).toBeObservable(cold('------')); + }); + it('should not fire when product is deselected', () => { store$.dispatch(new SelectProduct({ sku: undefined })); diff --git a/src/app/core/store/shopping/recently/recently.effects.ts b/src/app/core/store/shopping/recently/recently.effects.ts index 4fcd8f412a6..1385124cc56 100644 --- a/src/app/core/store/shopping/recently/recently.effects.ts +++ b/src/app/core/store/shopping/recently/recently.effects.ts @@ -18,6 +18,7 @@ export class RecentlyEffects { viewedProduct$ = this.store.pipe( select(getSelectedProduct), whenTruthy(), + filter(p => !ProductHelper.isFailedLoading(p)), distinctUntilKeyChanged('sku'), filter( product => diff --git a/src/app/extensions/seo/store/seo/seo.effects.ts b/src/app/extensions/seo/store/seo/seo.effects.ts index 736ad279a0f..54e7a2be689 100644 --- a/src/app/extensions/seo/store/seo/seo.effects.ts +++ b/src/app/extensions/seo/store/seo/seo.effects.ts @@ -5,7 +5,7 @@ import { Store, select } from '@ngrx/store'; import { MetaService } from '@ngx-meta/core'; import { TranslateService } from '@ngx-translate/core'; import { mapToParam, ofRoute } from 'ngrx-router'; -import { debounce, distinctUntilKeyChanged, first, map, switchMap, tap } from 'rxjs/operators'; +import { debounce, distinctUntilKeyChanged, filter, first, map, switchMap, tap } from 'rxjs/operators'; import { ProductHelper } from 'ish-core/models/product/product.helper'; import { SeoAttributes } from 'ish-core/models/seo-attribute/seo-attribute.model'; @@ -83,6 +83,7 @@ export class SeoEffects { this.store.pipe( select(getSelectedProduct), whenTruthy(), + filter(p => !ProductHelper.isFailedLoading(p)), map(p => (ProductHelper.isVariationProduct(p) && p.productMaster()) || p), distinctUntilKeyChanged('sku'), whenTruthy(),