diff --git a/src/app/+collection-page/collection-page.component.ts b/src/app/+collection-page/collection-page.component.ts index b820fea354..164208566d 100644 --- a/src/app/+collection-page/collection-page.component.ts +++ b/src/app/+collection-page/collection-page.component.ts @@ -1,6 +1,6 @@ import { ChangeDetectionStrategy, Component, OnDestroy, OnInit } from '@angular/core'; import { ActivatedRoute, Router } from '@angular/router'; -import { Observable, Subscription } from 'rxjs'; +import { Observable, Subscription } from 'rxjs'; import { SortDirection, SortOptions } from '../core/cache/models/sort-options.model'; import { CollectionDataService } from '../core/data/collection-data.service'; import { ItemDataService } from '../core/data/item-data.service'; @@ -16,11 +16,8 @@ import { Item } from '../core/shared/item.model'; import { fadeIn, fadeInOut } from '../shared/animations/fade'; import { hasValue, isNotEmpty } from '../shared/empty.util'; import { PaginationComponentOptions } from '../shared/pagination/pagination-component-options.model'; -import { combineLatest, filter, first, flatMap, map } from 'rxjs/operators'; -import { SearchService } from '../+search-page/search-service/search.service'; -import { PaginatedSearchOptions } from '../+search-page/paginated-search-options.model'; -import { renderPageNotFoundOn404, toDSpaceObjectListRD } from '../core/shared/operators'; -import { DSpaceObjectType } from '../core/shared/dspace-object-type.model'; +import { filter, first, flatMap, map } from 'rxjs/operators'; +import { redirectToPageNotFoundOn404 } from '../core/shared/operators'; @Component({ selector: 'ds-collection-page', @@ -58,7 +55,7 @@ export class CollectionPageComponent implements OnInit, OnDestroy { ngOnInit(): void { this.collectionRD$ = this.route.data.pipe( map((data) => data.collection as RemoteData), - renderPageNotFoundOn404(this.router), + redirectToPageNotFoundOn404(this.router), first() ); this.logoRD$ = this.collectionRD$.pipe( diff --git a/src/app/+collection-page/collection-page.resolver.ts b/src/app/+collection-page/collection-page.resolver.ts index d1c52e3383..8c6e3ad8a6 100644 --- a/src/app/+collection-page/collection-page.resolver.ts +++ b/src/app/+collection-page/collection-page.resolver.ts @@ -19,7 +19,8 @@ export class CollectionPageResolver implements Resolve> { * Method for resolving a collection based on the parameters in the current route * @param {ActivatedRouteSnapshot} route The current ActivatedRouteSnapshot * @param {RouterStateSnapshot} state The current RouterStateSnapshot - * @returns Observable<> Emits the found collection based on the parameters in the current route + * @returns Observable<> Emits the found collection based on the parameters in the current route, + * or an error if something went wrong */ resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable> { return this.collectionService.findById(route.params.id).pipe( diff --git a/src/app/+community-page/community-page.component.ts b/src/app/+community-page/community-page.component.ts index 4eb59a8b9e..f337d70250 100644 --- a/src/app/+community-page/community-page.component.ts +++ b/src/app/+community-page/community-page.component.ts @@ -13,7 +13,7 @@ import { MetadataService } from '../core/metadata/metadata.service'; import { fadeInOut } from '../shared/animations/fade'; import { hasValue } from '../shared/empty.util'; -import { renderPageNotFoundOn404 } from '../core/shared/operators'; +import { redirectToPageNotFoundOn404 } from '../core/shared/operators'; @Component({ selector: 'ds-community-page', @@ -47,7 +47,7 @@ export class CommunityPageComponent implements OnInit { ngOnInit(): void { this.communityRD$ = this.route.data.pipe( map((data) => data.community as RemoteData), - renderPageNotFoundOn404(this.router) + redirectToPageNotFoundOn404(this.router) ); this.logoRD$ = this.communityRD$.pipe( map((rd: RemoteData) => rd.payload), diff --git a/src/app/+community-page/community-page.resolver.ts b/src/app/+community-page/community-page.resolver.ts index d8957b740b..ffa66fa123 100644 --- a/src/app/+community-page/community-page.resolver.ts +++ b/src/app/+community-page/community-page.resolver.ts @@ -19,11 +19,12 @@ export class CommunityPageResolver implements Resolve> { * Method for resolving a community based on the parameters in the current route * @param {ActivatedRouteSnapshot} route The current ActivatedRouteSnapshot * @param {RouterStateSnapshot} state The current RouterStateSnapshot - * @returns Observable<> Emits the found community based on the parameters in the current route + * @returns Observable<> Emits the found community based on the parameters in the current route, + * or an error if something went wrong */ resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable> { return this.communityService.findById(route.params.id).pipe( - find((RD) => hasValue(RD.error) || RD.hasSucceeded), + find((RD) => hasValue(RD.error) || RD.hasSucceeded) ); } } diff --git a/src/app/+item-page/item-page.resolver.ts b/src/app/+item-page/item-page.resolver.ts index 52c301e034..4b7ef23b69 100644 --- a/src/app/+item-page/item-page.resolver.ts +++ b/src/app/+item-page/item-page.resolver.ts @@ -1,11 +1,11 @@ import { Injectable } from '@angular/core'; -import { ActivatedRouteSnapshot, Resolve, Router, RouterStateSnapshot } from '@angular/router'; +import { ActivatedRouteSnapshot, Resolve, RouterStateSnapshot } from '@angular/router'; import { Observable } from 'rxjs'; import { RemoteData } from '../core/data/remote-data'; import { ItemDataService } from '../core/data/item-data.service'; import { Item } from '../core/shared/item.model'; import { hasValue } from '../shared/empty.util'; -import { find, map } from 'rxjs/operators'; +import { find } from 'rxjs/operators'; /** * This class represents a resolver that requests a specific item before the route is activated @@ -19,7 +19,8 @@ export class ItemPageResolver implements Resolve> { * Method for resolving an item based on the parameters in the current route * @param {ActivatedRouteSnapshot} route The current ActivatedRouteSnapshot * @param {RouterStateSnapshot} state The current RouterStateSnapshot - * @returns Observable<> Emits the found item based on the parameters in the current route + * @returns Observable<> Emits the found item based on the parameters in the current route, + * or an error if something went wrong */ resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable> { return this.itemService.findById(route.params.id) diff --git a/src/app/+item-page/simple/item-page.component.ts b/src/app/+item-page/simple/item-page.component.ts index 3b24dfa174..e4d96e3bc4 100644 --- a/src/app/+item-page/simple/item-page.component.ts +++ b/src/app/+item-page/simple/item-page.component.ts @@ -14,8 +14,8 @@ import { MetadataService } from '../../core/metadata/metadata.service'; import { fadeInOut } from '../../shared/animations/fade'; import { hasValue } from '../../shared/empty.util'; +import { redirectToPageNotFoundOn404 } from '../../core/shared/operators'; import { ItemViewMode } from '../../shared/items/item-type-decorator'; -import { renderPageNotFoundOn404 } from '../../core/shared/operators'; /** * This component renders a simple item page. @@ -61,7 +61,7 @@ export class ItemPageComponent implements OnInit { ngOnInit(): void { this.itemRD$ = this.route.data.pipe( map((data) => data.item as RemoteData), - renderPageNotFoundOn404(this.router) + redirectToPageNotFoundOn404(this.router) ); this.metadataService.processRemoteData(this.itemRD$); this.thumbnail$ = this.itemRD$.pipe( diff --git a/src/app/core/shared/operators.spec.ts b/src/app/core/shared/operators.spec.ts index 2eb47507b2..564b0ff319 100644 --- a/src/app/core/shared/operators.spec.ts +++ b/src/app/core/shared/operators.spec.ts @@ -13,9 +13,11 @@ import { getRequestFromRequestUUID, getResourceLinksFromResponse, getResponseFromEntry, - getSucceededRemoteData + getSucceededRemoteData, redirectToPageNotFoundOn404 } from './operators'; import { RemoteData } from '../data/remote-data'; +import { RemoteDataError } from '../data/remote-data-error'; +import { of as observableOf } from 'rxjs'; describe('Core Module - RxJS Operators', () => { let scheduler: TestScheduler; @@ -193,6 +195,34 @@ describe('Core Module - RxJS Operators', () => { }); }); + describe('redirectToPageNotFoundOn404', () => { + let router; + beforeEach(() => { + router = jasmine.createSpyObj('router', ['navigateByUrl']); + }); + + it('should call navigateByUrl to a 404 page, when the remote data contains a 404 error', () => { + const testRD = new RemoteData(false, false, false, new RemoteDataError(404, 'Not Found', 'Object was not found'), undefined); + + observableOf(testRD).pipe(redirectToPageNotFoundOn404(router)).subscribe(); + expect(router.navigateByUrl).toHaveBeenCalledWith('/404', { skipLocationChange: true }); + }); + + it('should not call navigateByUrl to a 404 page, when the remote data contains another error than a 404', () => { + const testRD = new RemoteData(false, false, false, new RemoteDataError(500, 'Server Error', 'Something went wrong'), undefined); + + observableOf(testRD).pipe(redirectToPageNotFoundOn404(router)).subscribe(); + expect(router.navigateByUrl).not.toHaveBeenCalled(); + }); + + it('should not call navigateByUrl to a 404 page, when the remote data contains no error', () => { + const testRD = new RemoteData(false, false, true, null, undefined); + + observableOf(testRD).pipe(redirectToPageNotFoundOn404(router)).subscribe(); + expect(router.navigateByUrl).not.toHaveBeenCalled(); + }); + }); + describe('getResponseFromEntry', () => { it('should return the response for all not empty request entries, when they have a value', () => { const source = hot('abcdefg', testRCEs); diff --git a/src/app/core/shared/operators.ts b/src/app/core/shared/operators.ts index 8b85cc6458..ae46691e39 100644 --- a/src/app/core/shared/operators.ts +++ b/src/app/core/shared/operators.ts @@ -64,7 +64,12 @@ export const getSucceededRemoteData = () => (source: Observable>): Observable> => source.pipe(find((rd: RemoteData) => rd.hasSucceeded)); -export const renderPageNotFoundOn404 = (router: Router) => +/** + * Operator that checks if a remote data object contains a page not found error + * When it does contain such an error, it will redirect the user to a page not found, without altering the current URL + * @param router The router used to navigate to a new page + */ +export const redirectToPageNotFoundOn404 = (router: Router) => (source: Observable>): Observable> => source.pipe( tap((rd: RemoteData) => { diff --git a/src/app/header/header.component.html b/src/app/header/header.component.html index 0c0e07de50..a03fd01c53 100644 --- a/src/app/header/header.component.html +++ b/src/app/header/header.component.html @@ -5,7 +5,7 @@