[CST-24913] Refactor collection page component to use CollectionDataService for fetching collection data

Author: atarix83

src/app/collection-page/collection-page.component.spec.ts

@@ -18,6 +18,7 @@ import {
 import { TranslateModule } from '@ngx-translate/core';
 import { cold } from 'jasmine-marbles';
 import { of } from 'rxjs';
+import { CollectionDataService } from 'src/app/core/data/collection-data.service';
 import { environment } from 'src/environments/environment.test';
 
 import { APP_CONFIG } from '../../config/app-config.interface';
@@ -53,6 +54,7 @@ describe('CollectionPageComponent', () => {
   let authServiceSpy: jasmine.SpyObj<AuthService>;
   let authorizationDataServiceSpy: jasmine.SpyObj<AuthorizationDataService>;
   let dsoNameServiceSpy: jasmine.SpyObj<DSONameService>;
+  let collectionService: jasmine.SpyObj<CollectionDataService>;
   const aroute: ActivatedRouteStub = new ActivatedRouteStub();
   const router = new RouterStub();
 
@@ -75,6 +77,7 @@ describe('CollectionPageComponent', () => {
     authServiceSpy = jasmine.createSpyObj('AuthService', ['isAuthenticated']);
     authorizationDataServiceSpy = jasmine.createSpyObj('AuthorizationDataService', ['isAuthorized']);
     dsoNameServiceSpy = jasmine.createSpyObj('DSONameService', ['getName']);
+    collectionService = jasmine.createSpyObj('CollectionService', ['findById']);
 
     await TestBed.configureTestingModule({
       imports: [FormsModule, TranslateModule.forRoot(), BrowserAnimationsModule, CollectionPageComponent, VarDirective],
@@ -85,6 +88,7 @@ describe('CollectionPageComponent', () => {
         { provide: AuthService, useValue: authServiceSpy },
         { provide: AuthorizationDataService, useValue: authorizationDataServiceSpy },
         { provide: DSONameService, useValue: dsoNameServiceSpy },
+        { provide: CollectionDataService, useValue: collectionService },
         { provide: APP_CONFIG, useValue: environment },
         { provide: PLATFORM_ID, useValue: 'browser' },
       ],
@@ -102,9 +106,10 @@ describe('CollectionPageComponent', () => {
 
   describe('when collection has no logo', () => {
     beforeEach(() => {
-      aroute.testData = {
-        dso: collectionWithoutLogoRD,
+      aroute.testParams = {
+        id: 'id',
       };
+      collectionService.findById.and.returnValue(of(collectionWithoutLogoRD));
       fixture.detectChanges();
     });
 
@@ -113,7 +118,7 @@ describe('CollectionPageComponent', () => {
     });
 
     it('should initialize the component', () => {
-      expect(component.collectionRD$).toBeObservable(cold('(a|)', {
+      expect(component.collectionRD$).toBeObservable(cold('(a)', {
         a: collectionWithoutLogoRD,
       }));
       expect(component.logoRD$).toBeObservable(cold('(a|)', {
@@ -133,9 +138,10 @@ describe('CollectionPageComponent', () => {
 
   describe('when collection has logo', () => {
     beforeEach(() => {
-      aroute.testData = {
-        dso: collectionWithLogoRD,
+      aroute.testParams = {
+        id: 'id',
       };
+      collectionService.findById.and.returnValue(of(collectionWithLogoRD));
       fixture.detectChanges();
     });
 
@@ -144,7 +150,7 @@ describe('CollectionPageComponent', () => {
     });
 
     it('should initialize the component', () => {
-      expect(component.collectionRD$).toBeObservable(cold('(a|)', {
+      expect(component.collectionRD$).toBeObservable(cold('(a)', {
         a: collectionWithLogoRD,
       }));
       expect(component.logoRD$).toBeObservable(cold('(a|)', {

src/app/collection-page/collection-page.component.ts

@@ -16,13 +16,19 @@ import {
   RouterOutlet,
 } from '@angular/router';
 import { TranslateModule } from '@ngx-translate/core';
-import { Observable } from 'rxjs';
 import {
+  BehaviorSubject,
+  Observable,
+} from 'rxjs';
+import {
+  distinctUntilChanged,
   filter,
   map,
   mergeMap,
+  switchMap,
   take,
 } from 'rxjs/operators';
+import { CollectionDataService } from 'src/app/core/data/collection-data.service';
 
 import {
   APP_CONFIG,
@@ -37,7 +43,10 @@ import { RemoteData } from '../core/data/remote-data';
 import { redirectOn4xx } from '../core/shared/authorized.operators';
 import { Bitstream } from '../core/shared/bitstream.model';
 import { Collection } from '../core/shared/collection.model';
-import { getAllSucceededRemoteDataPayload } from '../core/shared/operators';
+import {
+  getAllSucceededRemoteDataPayload,
+  getFirstCompletedRemoteData,
+} from '../core/shared/operators';
 import {
   fadeIn,
   fadeInOut,
@@ -48,14 +57,12 @@ import { ThemedComcolPageHandleComponent } from '../shared/comcol/comcol-page-ha
 import { ComcolPageHeaderComponent } from '../shared/comcol/comcol-page-header/comcol-page-header.component';
 import { ComcolPageLogoComponent } from '../shared/comcol/comcol-page-logo/comcol-page-logo.component';
 import { ContextMenuComponent } from '../shared/context-menu/context-menu.component';
-import { DsoEditMenuComponent } from '../shared/dso-page/dso-edit-menu/dso-edit-menu.component';
 import {
   hasValue,
   isNotEmpty,
 } from '../shared/empty.util';
 import { ErrorComponent } from '../shared/error/error.component';
 import { ThemedLoadingComponent } from '../shared/loading/themed-loading.component';
-import { ObjectCollectionComponent } from '../shared/object-collection/object-collection.component';
 import { PaginationComponentOptions } from '../shared/pagination/pagination-component-options.model';
 import { VarDirective } from '../shared/utils/var.directive';
 import { getCollectionPageRoute } from './collection-page-routing-paths';
@@ -80,16 +87,14 @@ import { getCollectionPageRoute } from './collection-page-routing-paths';
     ComcolPageHeaderComponent,
     ComcolPageLogoComponent,
     ThemedComcolPageHandleComponent,
-    DsoEditMenuComponent,
     ThemedComcolPageBrowseByComponent,
-    ObjectCollectionComponent,
     RouterOutlet,
     ContextMenuComponent,
   ],
   standalone: true,
 })
 export class CollectionPageComponent implements OnInit {
-  collectionRD$: Observable<RemoteData<Collection>>;
+  collectionRD$: BehaviorSubject<RemoteData<Collection>> = new BehaviorSubject(null);
   logoRD$: Observable<RemoteData<Bitstream>>;
   paginationConfig: PaginationComponentOptions;
   sortConfig: SortOptions;
@@ -111,6 +116,7 @@ export class CollectionPageComponent implements OnInit {
     protected router: Router,
     protected authService: AuthService,
     protected authorizationDataService: AuthorizationDataService,
+    protected collectionService: CollectionDataService,
     public dsoNameService: DSONameService,
   ) {
   }
@@ -120,12 +126,18 @@ export class CollectionPageComponent implements OnInit {
       return;
     }
 
-    this.collectionRD$ = this.route.data.pipe(
-      map((data) => data.dso as RemoteData<Collection>),
-      redirectOn4xx(this.router, this.authService),
-      take(1),
-    );
+    this.route.params.pipe(
+      map(params => params.id),
+      distinctUntilChanged(),
+      switchMap((id) => this.collectionService.findById(id).pipe(
+        getFirstCompletedRemoteData(),
+        redirectOn4xx(this.router, this.authService),
+        take(1),
+      )),
+    ).subscribe(this.collectionRD$);
+
     this.logoRD$ = this.collectionRD$.pipe(
+      getFirstCompletedRemoteData(),
       map((rd: RemoteData<Collection>) => rd.payload),
       filter((collection: Collection) => hasValue(collection)),
       mergeMap((collection: Collection) => collection.logo),

src/app/core/auth/auth.actions.ts

@@ -35,6 +35,7 @@ export const AuthActionTypes = {
   LOG_OUT_ERROR: type('dspace/auth/LOG_OUT_ERROR'),
   LOG_OUT_SUCCESS: type('dspace/auth/LOG_OUT_SUCCESS'),
   SET_REDIRECT_URL: type('dspace/auth/SET_REDIRECT_URL'),
+  SET_REDIRECT_URL_AND_NAVIGATE: type('dspace/auth/SET_REDIRECT_URL_AND_NAVIGATE'),
   RETRIEVE_AUTHENTICATED_EPERSON: type('dspace/auth/RETRIEVE_AUTHENTICATED_EPERSON'),
   RETRIEVE_AUTHENTICATED_EPERSON_SUCCESS: type('dspace/auth/RETRIEVE_AUTHENTICATED_EPERSON_SUCCESS'),
   RETRIEVE_AUTHENTICATED_EPERSON_ERROR: type('dspace/auth/RETRIEVE_AUTHENTICATED_EPERSON_ERROR'),
@@ -360,6 +361,23 @@ export class SetRedirectUrlAction implements Action {
   }
 }
 
+/**
+ * Change the redirect url.
+ * @class SetRedirectUrlAction
+ * @implements {Action}
+ */
+export class SetRedirectUrlAdnNavigateAction implements Action {
+  public type: string = AuthActionTypes.SET_REDIRECT_URL_AND_NAVIGATE;
+  payload: {
+    redirectUrl: string;
+    navigateUrl: string;
+  };
+
+  constructor(redirectUrl: string, navigateUrl: string) {
+    this.payload = { redirectUrl, navigateUrl };
+  }
+}
+
 /**
  * Start loading for a hard redirect
  * @class StartHardRedirectLoadingAction

src/app/core/auth/auth.effects.ts

@@ -79,6 +79,7 @@ import {
   RetrieveAuthMethodsErrorAction,
   RetrieveAuthMethodsSuccessAction,
   RetrieveTokenAction,
+  SetRedirectUrlAdnNavigateAction,
   SetUserAsIdleAction,
 } from './auth.actions';
 // import services
@@ -347,6 +348,13 @@ export class AuthEffects {
   { dispatch: false },
   );
 
+  public redirectAndNavigate$: Observable<Action> = createEffect(() => this.actions$
+    .pipe(ofType(AuthActionTypes.SET_REDIRECT_URL_AND_NAVIGATE),
+      tap((action: SetRedirectUrlAdnNavigateAction) => this.router.navigate([decodeURIComponent(action.payload.navigateUrl)])),
+    ),
+  { dispatch: false },
+  );
+
   /**
    * For any action that is not in {@link IDLE_TIMER_IGNORE_TYPES} that comes in => Start the idleness timer
    * If the idleness timer runs out (so no un-ignored action come through for that amount of time)

src/app/core/auth/auth.reducer.ts

@@ -17,6 +17,7 @@ import {
   RetrieveAuthMethodsSuccessAction,
   SetAuthCookieStatus,
   SetRedirectUrlAction,
+  SetRedirectUrlAdnNavigateAction,
 } from './auth.actions';
 import { AuthMethod } from './models/auth.method';
 import { AuthMethodType } from './models/auth.method-type';
@@ -267,6 +268,11 @@ export function authReducer(state: any = initialState, action: AuthActions): Aut
         redirectUrl: (action as SetRedirectUrlAction).payload,
       });
 
+    case AuthActionTypes.SET_REDIRECT_URL_AND_NAVIGATE:
+      return Object.assign({}, state, {
+        redirectUrl: (action as SetRedirectUrlAdnNavigateAction).payload.redirectUrl,
+      });
+
     case AuthActionTypes.REDIRECT_AFTER_LOGIN_SUCCESS:
       return Object.assign({}, state, {
         loading: true,

src/app/core/auth/auth.service.ts

@@ -64,6 +64,7 @@ import {
   ResetAuthenticationMessagesAction,
   SetAuthCookieStatus,
   SetRedirectUrlAction,
+  SetRedirectUrlAdnNavigateAction,
   SetUserAsIdleAction,
   UnsetUserAsIdleAction,
 } from './auth.actions';
@@ -556,15 +557,19 @@ export class AuthService {
   /**
    * Set redirect url
    */
-  setRedirectUrl(url: string) {
+  setRedirectUrl(redirectUrl: string, navigateUrl?: string) {
     // Add 1 hour to the current date
     const expireDate = Date.now() + (1000 * 60 * 60);
 
     // Set the cookie expire date
     const expires = new Date(expireDate);
     const options: CookieAttributes = { expires: expires };
-    this.storage.set(REDIRECT_COOKIE, url, options);
-    this.store.dispatch(new SetRedirectUrlAction(isNotUndefined(url) ? url : ''));
+    this.storage.set(REDIRECT_COOKIE, redirectUrl, options);
+    if (hasValue(navigateUrl)) {
+      this.store.dispatch(new SetRedirectUrlAdnNavigateAction(isNotUndefined(redirectUrl) ? redirectUrl : '', navigateUrl));
+    } else {
+      this.store.dispatch(new SetRedirectUrlAction(isNotUndefined(redirectUrl) ? redirectUrl : ''));
+    }
   }
 
   /**

src/app/core/shared/authorized.operators.ts

@@ -47,8 +47,7 @@ export const redirectOn4xx = <T>(router: Router, authService: AuthService) =>
               router.navigateByUrl(getForbiddenRoute(), { skipLocationChange: true });
               return false;
             } else {
-              authService.setRedirectUrl(router.url);
-              router.navigateByUrl('login');
+              authService.setRedirectUrl(router.url, 'login');
               return false;
             }
           }

src/app/core/shared/operators.spec.ts

@@ -277,7 +277,6 @@ describe('Core Module - RxJS Operators', () => {
           expectObservable(source.pipe(redirectOn4xx(router, authService))).toBe(expected, values);
           flush();
           expect(authService.setRedirectUrl).toHaveBeenCalled();
-          expect(router.navigateByUrl).toHaveBeenCalledWith('login');
         });
       });
 
@@ -291,7 +290,6 @@ describe('Core Module - RxJS Operators', () => {
           expectObservable(source.pipe(redirectOn4xx(router, authService))).toBe(expected, values);
           flush();
           expect(authService.setRedirectUrl).toHaveBeenCalled();
-          expect(router.navigateByUrl).toHaveBeenCalledWith('login');
         });
       });
     });

src/app/statistics/statistics.service.ts

@@ -44,11 +44,13 @@ export class StatisticsService {
     dso: DSpaceObject,
     referrer: string,
   ) {
-    this.sendEvent('/statistics/viewevents', {
-      targetId: dso.uuid,
-      targetType: (dso as any).type,
-      referrer,
-    });
+    if (dso) {
+      this.sendEvent('/statistics/viewevents', {
+        targetId: dso.uuid,
+        targetType: (dso as any).type,
+        referrer,
+      });
+    }
   }
 
   /**