Merge pull request #891 from atmire/DSO-404-fix

Fix DSO 404 pages

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<Collection>),
-      redirectToPageNotFoundOn404(this.router),
+      redirectOn404Or401(this.router),
       take(1)
     );
     this.logoRD$ = this.collectionRD$.pipe(

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<Community>),
-      redirectToPageNotFoundOn404(this.router)
+      redirectOn404Or401(this.router)
     );
     this.logoRD$ = this.communityRD$.pipe(
       map((rd: RemoteData<Community>) => rd.payload),

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<Item>),
-      redirectToPageNotFoundOn404(this.router)
+      redirectOn404Or401(this.router)
     );
     this.metadataService.processRemoteData(this.itemRD$);
   }

src/app/app-routing-paths.ts

@@ -55,12 +55,18 @@ export function getDSORoute(dso: DSpaceObject): string {
   }
 }
 
-export const UNAUTHORIZED_PATH = 'unauthorized';
+export const UNAUTHORIZED_PATH = '401';
 
 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}`;

src/app/core/breadcrumbs/dso-breadcrumb.resolver.ts

@@ -3,12 +3,13 @@ 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 { getRemoteDataPayload } from '../shared/operators';
-import { map } from 'rxjs/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';
 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<T extends ChildHALResource & DSpaceO
   resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<BreadcrumbConfig<T>> {
     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;
+        if (hasValue(object)) {
-        const url = fullPath.substr(0, fullPath.indexOf(uuid)) + uuid;
+          const fullPath = state.url;
-        return { provider: this.breadcrumbService, key: object, url: url };
+          const url = fullPath.substr(0, fullPath.indexOf(uuid)) + uuid;
+          return {provider: this.breadcrumbService, key: object, url: url};
+        } else {
+          return undefined;
+        }
       })
     );
   }

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';
@@ -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();
     });
   });

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';
 
 /**
@@ -171,16 +171,20 @@ export const getAllSucceededRemoteListPayload = () =>
     );
 
 /**
- * Operator that checks if a remote data object contains a page not found error
+ * 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 a page not found, without altering the current URL
+ * 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) =>
   <T>(source: Observable<RemoteData<T>>): Observable<RemoteData<T>> =>
     source.pipe(
       tap((rd: RemoteData<T>) => {
-        if (rd.hasFailed && rd.error.statusCode === 404) {
+        if (rd.hasFailed) {
-          router.navigateByUrl('/404', { skipLocationChange: true });
+          if (rd.error.statusCode === 404) {
+            router.navigateByUrl(getPageNotFoundRoute(), {skipLocationChange: true});
+          } else if (rd.error.statusCode === 401) {
+            router.navigateByUrl(getUnauthorizedRoute(), {skipLocationChange: true});
+          }
         }
       }));
 

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<Process>),
-      redirectToPageNotFoundOn404(this.router)
+      redirectOn404Or401(this.router)
     );
 
     this.filesRD$ = this.processRD$.pipe(

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<T extends DSpaceObject> implements
   protected getScope$(): Observable<DSpaceObject> {
     return this.route.data.pipe(
       map((data) => data.scope as RemoteData<T>),
-      redirectToPageNotFoundOn404(this.router),
+      redirectOn404Or401(this.router),
       getSucceededRemoteData(),
       getRemoteDataPayload(),
     );