115427: Fixed edit item pages not redirecting to 404 with invalid id

Author: alexandrevryghem

src/app/item-page/full/full-item-page.component.spec.ts

@@ -15,7 +15,6 @@ import { BehaviorSubject, of as observableOf } from 'rxjs';
 import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
 import { By } from '@angular/platform-browser';
 import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils';
-import { AuthService } from '../../core/auth/auth.service';
 import { createPaginatedList } from '../../shared/testing/utils.test';
 import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
 import { createRelationshipsObservable } from '../simple/item-types/shared/item.component.spec';
@@ -54,7 +53,6 @@ describe('FullItemPageComponent', () => {
   let comp: FullItemPageComponent;
   let fixture: ComponentFixture<FullItemPageComponent>;
 
-  let authService: AuthService;
   let routeStub: ActivatedRouteStub;
   let routeData;
   let authorizationDataService: AuthorizationDataService;
@@ -75,11 +73,6 @@ describe('FullItemPageComponent', () => {
   };
 
   beforeEach(waitForAsync(() => {
-    authService = jasmine.createSpyObj('authService', {
-      isAuthenticated: observableOf(true),
-      setRedirectUrl: {}
-    });
-
     routeData = {
       dso: createSuccessfulRemoteDataObject(mockItem),
     };
@@ -117,7 +110,6 @@ describe('FullItemPageComponent', () => {
         { provide: ActivatedRoute, useValue: routeStub },
         { provide: ItemDataService, useValue: {} },
         { provide: MetadataService, useValue: metadataServiceStub },
-        { provide: AuthService, useValue: authService },
         { provide: AuthorizationDataService, useValue: authorizationDataService },
         { provide: ServerResponseService, useValue: serverResponseService },
         { provide: SignpostingDataService, useValue: signpostingDataService },

src/app/item-page/full/full-item-page.component.ts

@@ -13,7 +13,6 @@ import { Item } from '../../core/shared/item.model';
 
 import { fadeInOut } from '../../shared/animations/fade';
 import { hasValue } from '../../shared/empty.util';
-import { AuthService } from '../../core/auth/auth.service';
 import { Location } from '@angular/common';
 import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
 import { ServerResponseService } from '../../core/services/server-response.service';
@@ -49,15 +48,14 @@ export class FullItemPageComponent extends ItemPageComponent implements OnInit,
     protected route: ActivatedRoute,
     protected router: Router,
     protected items: ItemDataService,
-    protected authService: AuthService,
     protected authorizationService: AuthorizationDataService,
     protected _location: Location,
     protected responseService: ServerResponseService,
     protected signpostingDataService: SignpostingDataService,
     protected linkHeadService: LinkHeadService,
     @Inject(PLATFORM_ID) protected platformId: string,
   ) {
-    super(route, router, items, authService, authorizationService, responseService, signpostingDataService, linkHeadService, platformId);
+    super(route, router, items, authorizationService, responseService, signpostingDataService, linkHeadService, platformId);
   }
 
   /*** AoT inheritance fix, will hopefully be resolved in the near future **/

src/app/item-page/item-page.resolver.spec.ts

@@ -3,14 +3,15 @@ import { createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils';
 import { DSpaceObject } from '../core/shared/dspace-object.model';
 import { MetadataValueFilter } from '../core/shared/metadata.models';
 import { first } from 'rxjs/operators';
-import { Router } from '@angular/router';
+import { Router, RouterModule } from '@angular/router';
 import { TestBed } from '@angular/core/testing';
-import { RouterTestingModule } from '@angular/router/testing';
+import { AuthServiceStub } from '../shared/testing/auth-service.stub';
+import { AuthService } from '../core/auth/auth.service';
 
 describe('ItemPageResolver', () => {
   beforeEach(() => {
     TestBed.configureTestingModule({
-      imports: [RouterTestingModule.withRoutes([{
+      imports: [RouterModule.forRoot([{
         path: 'entities/:entity-type/:id',
         component: {} as any
       }])]
@@ -21,7 +22,8 @@ describe('ItemPageResolver', () => {
     let resolver: ItemPageResolver;
     let itemService: any;
     let store: any;
-    let router: any;
+    let router: Router;
+    let authService: AuthServiceStub;
 
     const uuid = '1234-65487-12354-1235';
     let item: DSpaceObject;
@@ -41,7 +43,8 @@ describe('ItemPageResolver', () => {
         store = jasmine.createSpyObj('store', {
           dispatch: {},
         });
-        resolver = new ItemPageResolver(itemService, store, router);
+        authService = new AuthServiceStub();
+        resolver = new ItemPageResolver(itemService, store, router, authService as unknown as AuthService);
       });
 
       it('should redirect to the correct route for the entity type', (done) => {

src/app/item-page/item-page.resolver.ts

@@ -9,6 +9,8 @@ import { map } from 'rxjs/operators';
 import { hasValue } from '../shared/empty.util';
 import { getItemPageRoute } from './item-page-routing-paths';
 import { ItemResolver } from './item.resolver';
+import { redirectOn4xx } from '../core/shared/authorized.operators';
+import { AuthService } from '../core/auth/auth.service';
 
 /**
  * This class represents a resolver that requests a specific item before the route is activated and will redirect to the
@@ -19,7 +21,8 @@ export class ItemPageResolver extends ItemResolver {
   constructor(
     protected itemService: ItemDataService,
     protected store: Store<any>,
-    protected router: Router
+    protected router: Router,
+    protected authService: AuthService,
   ) {
     super(itemService, store, router);
   }
@@ -33,6 +36,7 @@ export class ItemPageResolver extends ItemResolver {
    */
   resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<RemoteData<Item>> {
     return super.resolve(route, state).pipe(
+      redirectOn4xx(this.router, this.authService),
       map((rd: RemoteData<Item>) => {
         if (rd.hasSucceeded && hasValue(rd.payload)) {
           const thisRoute = state.url;

src/app/item-page/simple/item-page.component.spec.ts

@@ -19,7 +19,6 @@ import {
   createSuccessfulRemoteDataObject,
   createSuccessfulRemoteDataObject$
 } from '../../shared/remote-data.utils';
-import { AuthService } from '../../core/auth/auth.service';
 import { createPaginatedList } from '../../shared/testing/utils.test';
 import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
 import { ServerResponseService } from '../../core/services/server-response.service';
@@ -57,7 +56,6 @@ const mockSignpostingLinks: SignpostingLink[] = [mocklink, mocklink2];
 describe('ItemPageComponent', () => {
   let comp: ItemPageComponent;
   let fixture: ComponentFixture<ItemPageComponent>;
-  let authService: AuthService;
   let authorizationDataService: AuthorizationDataService;
   let serverResponseService: jasmine.SpyObj<ServerResponseService>;
   let signpostingDataService: jasmine.SpyObj<SignpostingDataService>;
@@ -74,10 +72,6 @@ describe('ItemPageComponent', () => {
   });
 
   beforeEach(waitForAsync(() => {
-    authService = jasmine.createSpyObj('authService', {
-      isAuthenticated: observableOf(true),
-      setRedirectUrl: {}
-    });
     authorizationDataService = jasmine.createSpyObj('authorizationDataService', {
       isAuthorized: observableOf(false),
     });
@@ -107,7 +101,6 @@ describe('ItemPageComponent', () => {
         { provide: ItemDataService, useValue: {} },
         { provide: MetadataService, useValue: mockMetadataService },
         { provide: Router, useValue: {} },
-        { provide: AuthService, useValue: authService },
         { provide: AuthorizationDataService, useValue: authorizationDataService },
         { provide: ServerResponseService, useValue: serverResponseService },
         { provide: SignpostingDataService, useValue: signpostingDataService },

src/app/item-page/simple/item-page.component.ts

@@ -11,9 +11,7 @@ import { Item } from '../../core/shared/item.model';
 import { fadeInOut } from '../../shared/animations/fade';
 import { getAllSucceededRemoteDataPayload } from '../../core/shared/operators';
 import { ViewMode } from '../../core/shared/view-mode.model';
-import { AuthService } from '../../core/auth/auth.service';
 import { getItemPageRoute } from '../item-page-routing-paths';
-import { redirectOn4xx } from '../../core/shared/authorized.operators';
 import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
 import { FeatureID } from '../../core/data/feature-authorization/feature-id';
 import { ServerResponseService } from '../../core/services/server-response.service';
@@ -72,7 +70,6 @@ export class ItemPageComponent implements OnInit, OnDestroy {
     protected route: ActivatedRoute,
     protected router: Router,
     protected items: ItemDataService,
-    protected authService: AuthService,
     protected authorizationService: AuthorizationDataService,
     protected responseService: ServerResponseService,
     protected signpostingDataService: SignpostingDataService,
@@ -88,7 +85,6 @@ export class ItemPageComponent implements OnInit, OnDestroy {
   ngOnInit(): void {
     this.itemRD$ = this.route.data.pipe(
       map((data) => data.dso as RemoteData<Item>),
-      redirectOn4xx(this.router, this.authService)
     );
     this.itemPageRoute$ = this.itemRD$.pipe(
       getAllSucceededRemoteDataPayload(),