From 5e21bfa5ca2369b4659efeb6e34daa9d23fbf10a Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 13 Oct 2020 13:28:47 +0200 Subject: [PATCH 1/5] 74053: fix issue #865 --- .../core/breadcrumbs/dso-breadcrumb.resolver.ts | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts b/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts index 09292fec21..9d04083749 100644 --- a/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts +++ b/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts @@ -4,11 +4,12 @@ import { ActivatedRouteSnapshot, Resolve, RouterStateSnapshot } from '@angular/r import { DSOBreadcrumbsService } from './dso-breadcrumbs.service'; import { DataService } from '../data/data.service'; import { getRemoteDataPayload, getSucceededRemoteData } from '../shared/operators'; -import { map } from 'rxjs/operators'; +import { filter, find, map, take } from 'rxjs/operators'; import { Observable } from 'rxjs'; import { DSpaceObject } from '../shared/dspace-object.model'; import { ChildHALResource } from '../shared/child-hal-resource.model'; import { FollowLinkConfig } from '../../shared/utils/follow-link-config.model'; +import { hasValue } from '../../shared/empty.util'; /** * The class that resolves the BreadcrumbConfig object for a DSpaceObject @@ -29,12 +30,17 @@ export abstract class DSOBreadcrumbResolver> { const uuid = route.params.id; return this.dataService.findById(uuid, ...this.followLinks).pipe( - getSucceededRemoteData(), + filter((rd) => hasValue(rd.error) || hasValue(rd.payload)), + take(1), getRemoteDataPayload(), map((object: T) => { - const fullPath = state.url; - const url = fullPath.substr(0, fullPath.indexOf(uuid)) + uuid; - return { provider: this.breadcrumbService, key: object, url: url }; + if (hasValue(object)) { + const fullPath = state.url; + const url = fullPath.substr(0, fullPath.indexOf(uuid)) + uuid; + return {provider: this.breadcrumbService, key: object, url: url}; + } else { + return undefined; + } }) ); } From d4bd128b673a071c6ba27586627b1db3f76c3466 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 13 Oct 2020 14:19:56 +0200 Subject: [PATCH 2/5] 74053: Remove unused imports --- src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts b/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts index 9d04083749..03d4db3f5d 100644 --- a/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts +++ b/src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts @@ -3,8 +3,8 @@ import { Injectable } from '@angular/core'; import { ActivatedRouteSnapshot, Resolve, RouterStateSnapshot } from '@angular/router'; import { DSOBreadcrumbsService } from './dso-breadcrumbs.service'; import { DataService } from '../data/data.service'; -import { getRemoteDataPayload, getSucceededRemoteData } from '../shared/operators'; -import { filter, find, map, take } from 'rxjs/operators'; +import { getRemoteDataPayload } from '../shared/operators'; +import { filter, map, take } from 'rxjs/operators'; import { Observable } from 'rxjs'; import { DSpaceObject } from '../shared/dspace-object.model'; import { ChildHALResource } from '../shared/child-hal-resource.model'; From 736704aae8fc2ee22a2559625a57ca17e9ab0fcd Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 27 Oct 2020 11:25:12 +0100 Subject: [PATCH 3/5] 74053: redirectOn404Or401 --- .../collection-page.component.ts | 4 ++-- .../community-page.component.ts | 4 ++-- .../+item-page/simple/item-page.component.ts | 4 ++-- src/app/app-routing-paths.ts | 2 +- src/app/core/shared/operators.spec.ts | 24 ++++++++++++------- src/app/core/shared/operators.ts | 14 +++++++---- .../detail/process-detail.component.ts | 4 ++-- .../statistics-page.component.ts | 4 ++-- 8 files changed, 36 insertions(+), 24 deletions(-) diff --git a/src/app/+collection-page/collection-page.component.ts b/src/app/+collection-page/collection-page.component.ts index c7d287ed6a..a390bda4cb 100644 --- a/src/app/+collection-page/collection-page.component.ts +++ b/src/app/+collection-page/collection-page.component.ts @@ -17,7 +17,7 @@ import { DSpaceObjectType } from '../core/shared/dspace-object-type.model'; import { Item } from '../core/shared/item.model'; import { getSucceededRemoteData, - redirectToPageNotFoundOn404, + redirectOn404Or401, toDSpaceObjectListRD } from '../core/shared/operators'; @@ -63,7 +63,7 @@ export class CollectionPageComponent implements OnInit { ngOnInit(): void { this.collectionRD$ = this.route.data.pipe( map((data) => data.dso as RemoteData), - redirectToPageNotFoundOn404(this.router), + redirectOn404Or401(this.router), take(1) ); this.logoRD$ = this.collectionRD$.pipe( diff --git a/src/app/+community-page/community-page.component.ts b/src/app/+community-page/community-page.component.ts index 3621829927..7c93b1bf4e 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 { redirectToPageNotFoundOn404 } from '../core/shared/operators'; +import { redirectOn404Or401 } 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.dso as RemoteData), - redirectToPageNotFoundOn404(this.router) + redirectOn404Or401(this.router) ); this.logoRD$ = this.communityRD$.pipe( map((rd: RemoteData) => rd.payload), diff --git a/src/app/+item-page/simple/item-page.component.ts b/src/app/+item-page/simple/item-page.component.ts index 10deef23e4..87d2294ff9 100644 --- a/src/app/+item-page/simple/item-page.component.ts +++ b/src/app/+item-page/simple/item-page.component.ts @@ -11,7 +11,7 @@ import { Item } from '../../core/shared/item.model'; import { MetadataService } from '../../core/metadata/metadata.service'; import { fadeInOut } from '../../shared/animations/fade'; -import { redirectToPageNotFoundOn404 } from '../../core/shared/operators'; +import { redirectOn404Or401 } from '../../core/shared/operators'; import { ViewMode } from '../../core/shared/view-mode.model'; /** @@ -56,7 +56,7 @@ export class ItemPageComponent implements OnInit { ngOnInit(): void { this.itemRD$ = this.route.data.pipe( map((data) => data.item as RemoteData), - redirectToPageNotFoundOn404(this.router) + redirectOn404Or401(this.router) ); this.metadataService.processRemoteData(this.itemRD$); } diff --git a/src/app/app-routing-paths.ts b/src/app/app-routing-paths.ts index 4e64a4a552..f9fc448b99 100644 --- a/src/app/app-routing-paths.ts +++ b/src/app/app-routing-paths.ts @@ -55,7 +55,7 @@ export function getDSORoute(dso: DSpaceObject): string { } } -export const UNAUTHORIZED_PATH = 'unauthorized'; +export const UNAUTHORIZED_PATH = '401'; export function getUnauthorizedRoute() { return `/${UNAUTHORIZED_PATH}`; diff --git a/src/app/core/shared/operators.spec.ts b/src/app/core/shared/operators.spec.ts index a19419259d..401efae760 100644 --- a/src/app/core/shared/operators.spec.ts +++ b/src/app/core/shared/operators.spec.ts @@ -13,7 +13,8 @@ import { getRequestFromRequestUUID, getResourceLinksFromResponse, getResponseFromEntry, - getSucceededRemoteData, redirectToPageNotFoundOn404 + getSucceededRemoteData, + redirectOn404Or401 } from './operators'; import { RemoteData } from '../data/remote-data'; import { RemoteDataError } from '../data/remote-data-error'; @@ -23,7 +24,7 @@ import { createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils'; -describe('Core Module - RxJS Operators', () => { +fdescribe('Core Module - RxJS Operators', () => { let scheduler: TestScheduler; let requestService: RequestService; const testRequestHref = 'https://rest.api/'; @@ -199,7 +200,7 @@ describe('Core Module - RxJS Operators', () => { }); }); - describe('redirectToPageNotFoundOn404', () => { + describe('redirectOn404Or401', () => { let router; beforeEach(() => { router = jasmine.createSpyObj('router', ['navigateByUrl']); @@ -208,21 +209,28 @@ describe('Core Module - RxJS Operators', () => { it('should call navigateByUrl to a 404 page, when the remote data contains a 404 error', () => { const testRD = createFailedRemoteDataObject(undefined, new RemoteDataError(404, 'Not Found', 'Object was not found')); - observableOf(testRD).pipe(redirectToPageNotFoundOn404(router)).subscribe(); + observableOf(testRD).pipe(redirectOn404Or401(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', () => { + it('should call navigateByUrl to a 401 page, when the remote data contains a 401 error', () => { + const testRD = createFailedRemoteDataObject(undefined, new RemoteDataError(401, 'Unauthorized', 'The current user is unauthorized')); + + observableOf(testRD).pipe(redirectOn404Or401(router)).subscribe(); + expect(router.navigateByUrl).toHaveBeenCalledWith('/401', { skipLocationChange: true }); + }); + + it('should not call navigateByUrl to a 404 or 401 page, when the remote data contains another error than a 404 or 401', () => { const testRD = createFailedRemoteDataObject(undefined, new RemoteDataError(500, 'Server Error', 'Something went wrong')); - observableOf(testRD).pipe(redirectToPageNotFoundOn404(router)).subscribe(); + observableOf(testRD).pipe(redirectOn404Or401(router)).subscribe(); expect(router.navigateByUrl).not.toHaveBeenCalled(); }); - it('should not call navigateByUrl to a 404 page, when the remote data contains no error', () => { + it('should not call navigateByUrl to a 404 or 401 page, when the remote data contains no error', () => { const testRD = createSuccessfulRemoteDataObject(undefined); - observableOf(testRD).pipe(redirectToPageNotFoundOn404(router)).subscribe(); + observableOf(testRD).pipe(redirectOn404Or401(router)).subscribe(); expect(router.navigateByUrl).not.toHaveBeenCalled(); }); }); diff --git a/src/app/core/shared/operators.ts b/src/app/core/shared/operators.ts index ad2588f2b9..41e9c7e18a 100644 --- a/src/app/core/shared/operators.ts +++ b/src/app/core/shared/operators.ts @@ -171,16 +171,20 @@ export const getAllSucceededRemoteListPayload = () => ); /** - * 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 + * Operator that checks if a remote data object returned a 401 or 404 error + * When it does contain such an error, it will redirect the user to the related error page, without altering the current URL * @param router The router used to navigate to a new page */ -export const redirectToPageNotFoundOn404 = (router: Router) => +export const redirectOn404Or401 = (router: Router) => (source: Observable>): Observable> => source.pipe( tap((rd: RemoteData) => { - if (rd.hasFailed && rd.error.statusCode === 404) { - router.navigateByUrl('/404', { skipLocationChange: true }); + if (rd.hasFailed) { + if (rd.error.statusCode === 404) { + router.navigateByUrl('/404', {skipLocationChange: true}); + } else if (rd.error.statusCode === 401) { + router.navigateByUrl('/401', {skipLocationChange: true}); + } } })); diff --git a/src/app/process-page/detail/process-detail.component.ts b/src/app/process-page/detail/process-detail.component.ts index b0e2c7e378..c4610b70e9 100644 --- a/src/app/process-page/detail/process-detail.component.ts +++ b/src/app/process-page/detail/process-detail.component.ts @@ -4,7 +4,7 @@ import { Observable } from 'rxjs/internal/Observable'; import { RemoteData } from '../../core/data/remote-data'; import { Process } from '../processes/process.model'; import { map, switchMap } from 'rxjs/operators'; -import { getFirstSucceededRemoteDataPayload, redirectToPageNotFoundOn404 } from '../../core/shared/operators'; +import { getFirstSucceededRemoteDataPayload, redirectOn404Or401 } from '../../core/shared/operators'; import { AlertType } from '../../shared/alert/aletr-type'; import { ProcessDataService } from '../../core/data/processes/process-data.service'; import { PaginatedList } from '../../core/data/paginated-list'; @@ -49,7 +49,7 @@ export class ProcessDetailComponent implements OnInit { ngOnInit(): void { this.processRD$ = this.route.data.pipe( map((data) => data.process as RemoteData), - redirectToPageNotFoundOn404(this.router) + redirectOn404Or401(this.router) ); this.filesRD$ = this.processRD$.pipe( diff --git a/src/app/statistics-page/statistics-page/statistics-page.component.ts b/src/app/statistics-page/statistics-page/statistics-page.component.ts index e034a35dca..31cb74eb08 100644 --- a/src/app/statistics-page/statistics-page/statistics-page.component.ts +++ b/src/app/statistics-page/statistics-page/statistics-page.component.ts @@ -4,7 +4,7 @@ import { UsageReportService } from '../../core/statistics/usage-report-data.serv import { map, switchMap } from 'rxjs/operators'; import { UsageReport } from '../../core/statistics/models/usage-report.model'; import { RemoteData } from '../../core/data/remote-data'; -import { getRemoteDataPayload, getSucceededRemoteData, redirectToPageNotFoundOn404 } from '../../core/shared/operators'; +import { getRemoteDataPayload, getSucceededRemoteData, redirectOn404Or401 } from '../../core/shared/operators'; import { DSpaceObject } from '../../core/shared/dspace-object.model'; import { ActivatedRoute, Router } from '@angular/router'; import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; @@ -55,7 +55,7 @@ export abstract class StatisticsPageComponent implements protected getScope$(): Observable { return this.route.data.pipe( map((data) => data.scope as RemoteData), - redirectToPageNotFoundOn404(this.router), + redirectOn404Or401(this.router), getSucceededRemoteData(), getRemoteDataPayload(), ); From 2d67b0b13ef2bccd74987ee4a98d88a1a6ec85e1 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 27 Oct 2020 12:29:39 +0100 Subject: [PATCH 4/5] 74053: remove fdescribe + use consts for routing paths --- src/app/app-routing-paths.ts | 6 ++++++ src/app/core/shared/operators.spec.ts | 2 +- src/app/core/shared/operators.ts | 6 +++--- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/app/app-routing-paths.ts b/src/app/app-routing-paths.ts index f9fc448b99..2b57a1957c 100644 --- a/src/app/app-routing-paths.ts +++ b/src/app/app-routing-paths.ts @@ -61,6 +61,12 @@ export function getUnauthorizedRoute() { return `/${UNAUTHORIZED_PATH}`; } +export const PAGE_NOT_FOUND_PATH = '404'; + +export function getPageNotFoundRoute() { + return `/${PAGE_NOT_FOUND_PATH}`; +} + export const INFO_MODULE_PATH = 'info'; export function getInfoModulePath() { return `/${INFO_MODULE_PATH}`; diff --git a/src/app/core/shared/operators.spec.ts b/src/app/core/shared/operators.spec.ts index 401efae760..8acf5ea860 100644 --- a/src/app/core/shared/operators.spec.ts +++ b/src/app/core/shared/operators.spec.ts @@ -24,7 +24,7 @@ import { createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils'; -fdescribe('Core Module - RxJS Operators', () => { +describe('Core Module - RxJS Operators', () => { let scheduler: TestScheduler; let requestService: RequestService; const testRequestHref = 'https://rest.api/'; diff --git a/src/app/core/shared/operators.ts b/src/app/core/shared/operators.ts index 41e9c7e18a..ecc1f53933 100644 --- a/src/app/core/shared/operators.ts +++ b/src/app/core/shared/operators.ts @@ -13,7 +13,7 @@ import { MetadataField } from '../metadata/metadata-field.model'; import { MetadataSchema } from '../metadata/metadata-schema.model'; import { BrowseDefinition } from './browse-definition.model'; import { DSpaceObject } from './dspace-object.model'; -import { getUnauthorizedRoute } from '../../app-routing-paths'; +import { getPageNotFoundRoute, getUnauthorizedRoute } from '../../app-routing-paths'; import { getEndUserAgreementPath } from '../../info/info-routing-paths'; /** @@ -181,9 +181,9 @@ export const redirectOn404Or401 = (router: Router) => tap((rd: RemoteData) => { if (rd.hasFailed) { if (rd.error.statusCode === 404) { - router.navigateByUrl('/404', {skipLocationChange: true}); + router.navigateByUrl(`/${getPageNotFoundRoute()}`, {skipLocationChange: true}); } else if (rd.error.statusCode === 401) { - router.navigateByUrl('/401', {skipLocationChange: true}); + router.navigateByUrl(`/${getUnauthorizedRoute()}`, {skipLocationChange: true}); } } })); From d52da292a65e6add90c914e6e5d758d873643d69 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 27 Oct 2020 13:14:01 +0100 Subject: [PATCH 5/5] 74053: Fix double slash in route --- src/app/core/shared/operators.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app/core/shared/operators.ts b/src/app/core/shared/operators.ts index ecc1f53933..29e41907e1 100644 --- a/src/app/core/shared/operators.ts +++ b/src/app/core/shared/operators.ts @@ -181,9 +181,9 @@ export const redirectOn404Or401 = (router: Router) => tap((rd: RemoteData) => { if (rd.hasFailed) { if (rd.error.statusCode === 404) { - router.navigateByUrl(`/${getPageNotFoundRoute()}`, {skipLocationChange: true}); + router.navigateByUrl(getPageNotFoundRoute(), {skipLocationChange: true}); } else if (rd.error.statusCode === 401) { - router.navigateByUrl(`/${getUnauthorizedRoute()}`, {skipLocationChange: true}); + router.navigateByUrl(getUnauthorizedRoute(), {skipLocationChange: true}); } } }));