Merge branch 'refactor-comcol-pages_contribute-7.6' into refactor-comcol-pages_contribute-main

Author: alexandrevryghem

cypress/e2e/browse-by-author.cy.ts

@@ -5,9 +5,9 @@ describe('Browse By Author', () => {
         cy.visit('/browse/author');
 
         // Wait for <ds-browse-by-metadata-page> to be visible
-        cy.get('ds-browse-by-metadata-page').should('be.visible');
+        cy.get('ds-browse-by-metadata').should('be.visible');
 
         // Analyze <ds-browse-by-metadata-page> for accessibility
-        testA11y('ds-browse-by-metadata-page');
+        testA11y('ds-browse-by-metadata');
     });
 });

cypress/e2e/browse-by-dateissued.cy.ts

@@ -5,9 +5,9 @@ describe('Browse By Date Issued', () => {
         cy.visit('/browse/dateissued');
 
         // Wait for <ds-browse-by-date-page> to be visible
-        cy.get('ds-browse-by-date-page').should('be.visible');
+        cy.get('ds-browse-by-date').should('be.visible');
 
         // Analyze <ds-browse-by-date-page> for accessibility
-        testA11y('ds-browse-by-date-page');
+        testA11y('ds-browse-by-date');
     });
 });

cypress/e2e/browse-by-subject.cy.ts

@@ -5,9 +5,9 @@ describe('Browse By Subject', () => {
         cy.visit('/browse/subject');
 
         // Wait for <ds-browse-by-metadata-page> to be visible
-        cy.get('ds-browse-by-metadata-page').should('be.visible');
+        cy.get('ds-browse-by-metadata').should('be.visible');
 
         // Analyze <ds-browse-by-metadata-page> for accessibility
-        testA11y('ds-browse-by-metadata-page');
+        testA11y('ds-browse-by-metadata');
     });
 });

cypress/e2e/browse-by-title.cy.ts

@@ -5,9 +5,9 @@ describe('Browse By Title', () => {
         cy.visit('/browse/title');
 
         // Wait for <ds-browse-by-title-page> to be visible
-        cy.get('ds-browse-by-title-page').should('be.visible');
+        cy.get('ds-browse-by-title').should('be.visible');
 
         // Analyze <ds-browse-by-title-page> for accessibility
-        testA11y('ds-browse-by-title-page');
+        testA11y('ds-browse-by-title');
     });
 });

…ge/browse-by-date-page.component.spec.ts …by-date/browse-by-date.component.spec.tssrc/app/browse-by/browse-by-date-page/browse-by-date-page.component.spec.ts renamed to src/app/browse-by/browse-by-date/browse-by-date.component.spec.ts src/app/browse-by/browse-by-date-page/browse-by-date-page.component.spec.ts renamed to src/app/browse-by/browse-by-date/browse-by-date.component.spec.ts

@@ -1,4 +1,4 @@
-import { BrowseByDatePageComponent } from './browse-by-date-page.component';
+import { BrowseByDateComponent } from './browse-by-date.component';
 import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
 import { CommonModule } from '@angular/common';
 import { RouterTestingModule } from '@angular/router/testing';
@@ -15,18 +15,19 @@ import { ActivatedRouteStub } from '../../shared/testing/active-router.stub';
 import { Community } from '../../core/shared/community.model';
 import { Item } from '../../core/shared/item.model';
 import { BrowseEntrySearchOptions } from '../../core/browse/browse-entry-search-options.model';
-import { toRemoteData } from '../browse-by-metadata-page/browse-by-metadata-page.component.spec';
+import { toRemoteData } from '../browse-by-metadata/browse-by-metadata.component.spec';
 import { VarDirective } from '../../shared/utils/var.directive';
 import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils';
 import { PaginationService } from '../../core/pagination/pagination.service';
 import { PaginationServiceStub } from '../../shared/testing/pagination-service.stub';
 import { APP_CONFIG } from '../../../config/app-config.interface';
 import { environment } from '../../../environments/environment';
 import { SortDirection } from '../../core/cache/models/sort-options.model';
+import { cold } from 'jasmine-marbles';
 
-describe('BrowseByDatePageComponent', () => {
-  let comp: BrowseByDatePageComponent;
-  let fixture: ComponentFixture<BrowseByDatePageComponent>;
+describe('BrowseByDateComponent', () => {
+  let comp: BrowseByDateComponent;
+  let fixture: ComponentFixture<BrowseByDateComponent>;
   let route: ActivatedRoute;
   let paginationService;
 
@@ -86,7 +87,7 @@ describe('BrowseByDatePageComponent', () => {
   beforeEach(waitForAsync(() => {
     TestBed.configureTestingModule({
       imports: [CommonModule, RouterTestingModule.withRoutes([]), TranslateModule.forRoot(), NgbModule],
-      declarations: [BrowseByDatePageComponent, EnumKeysPipe, VarDirective],
+      declarations: [BrowseByDateComponent, EnumKeysPipe, VarDirective],
       providers: [
         { provide: ActivatedRoute, useValue: activatedRouteStub },
         { provide: BrowseService, useValue: mockBrowseService },
@@ -101,7 +102,7 @@ describe('BrowseByDatePageComponent', () => {
   }));
 
   beforeEach(() => {
-    fixture = TestBed.createComponent(BrowseByDatePageComponent);
+    fixture = TestBed.createComponent(BrowseByDateComponent);
     const browseService = fixture.debugElement.injector.get(BrowseService);
     spyOn(browseService, 'getFirstItemFor')
       // ok to expect the default browse as first param since we just need the mock items obtained via sort direction.
@@ -112,9 +113,13 @@ describe('BrowseByDatePageComponent', () => {
     fixture.detectChanges();
   });
 
-  it('should initialize the list of items', () => {
+  it('should initialize the list of items', (done: DoneFn) => {
+    expect(comp.loading$).toBeObservable(cold('(a|)', {
+      a: false,
+    }));
     comp.items$.subscribe((result) => {
       expect(result.payload.page).toEqual([firstItem]);
+      done();
     });
   });
 

…te-page/browse-by-date-page.component.ts …owse-by-date/browse-by-date.component.tssrc/app/browse-by/browse-by-date-page/browse-by-date-page.component.ts renamed to src/app/browse-by/browse-by-date/browse-by-date.component.ts src/app/browse-by/browse-by-date-page/browse-by-date-page.component.ts renamed to src/app/browse-by/browse-by-date/browse-by-date.component.ts

@@ -1,10 +1,6 @@
 import { ChangeDetectorRef, Component, Inject, OnInit } from '@angular/core';
-import {
-  BrowseByMetadataPageComponent,
-  browseParamsToOptions,
-  getBrowseSearchOptions
-} from '../browse-by-metadata-page/browse-by-metadata-page.component';
-import { combineLatest as observableCombineLatest } from 'rxjs';
+import { BrowseByMetadataComponent, browseParamsToOptions, getBrowseSearchOptions } from '../browse-by-metadata/browse-by-metadata.component';
+import { combineLatest as observableCombineLatest, Observable } from 'rxjs';
 import { hasValue, isNotEmpty } from '../../shared/empty.util';
 import { ActivatedRoute, Params, Router } from '@angular/router';
 import { BrowseService } from '../../core/browse/browse.service';
@@ -23,31 +19,32 @@ import { rendersBrowseBy } from '../browse-by-switcher/browse-by-decorator';
 import { BrowseByDataType } from '../browse-by-switcher/browse-by-data-type';
 
 @Component({
-  selector: 'ds-browse-by-date-page',
-  styleUrls: ['../browse-by-metadata-page/browse-by-metadata-page.component.scss'],
-  templateUrl: '../browse-by-metadata-page/browse-by-metadata-page.component.html'
+  selector: 'ds-browse-by-date',
+  styleUrls: ['../browse-by-metadata/browse-by-metadata.component.scss'],
+  templateUrl: '../browse-by-metadata/browse-by-metadata.component.html',
 })
 /**
  * Component for browsing items by metadata definition of type 'date'
  * A metadata definition (a.k.a. browse id) is a short term used to describe one or multiple metadata fields.
  * An example would be 'dateissued' for 'dc.date.issued'
  */
 @rendersBrowseBy(BrowseByDataType.Date)
-export class BrowseByDatePageComponent extends BrowseByMetadataPageComponent implements OnInit {
+export class BrowseByDateComponent extends BrowseByMetadataComponent implements OnInit {
 
   /**
    * The default metadata keys to use for determining the lower limit of the StartsWith dropdown options
    */
   defaultMetadataKeys = ['dc.date.issued'];
 
-  public constructor(protected route: ActivatedRoute,
-                     protected browseService: BrowseService,
-                     protected dsoService: DSpaceObjectDataService,
-                     protected router: Router,
-                     protected paginationService: PaginationService,
-                     protected cdRef: ChangeDetectorRef,
-                     @Inject(APP_CONFIG) public appConfig: AppConfig,
-                     public dsoNameService: DSONameService,
+  public constructor(
+    protected route: ActivatedRoute,
+    protected browseService: BrowseService,
+    protected dsoService: DSpaceObjectDataService,
+    protected paginationService: PaginationService,
+    protected router: Router,
+    @Inject(APP_CONFIG) public appConfig: AppConfig,
+    public dsoNameService: DSONameService,
+    protected cdRef: ChangeDetectorRef,
   ) {
     super(route, browseService, dsoService, paginationService, router, appConfig, dsoNameService);
   }
@@ -60,19 +57,17 @@ export class BrowseByDatePageComponent extends BrowseByMetadataPageComponent imp
     this.currentPagination$ = this.paginationService.getCurrentPagination(this.paginationConfig.id, this.paginationConfig);
     this.currentSort$ = this.paginationService.getCurrentSort(this.paginationConfig.id, sortConfig);
     this.subs.push(
-      observableCombineLatest([this.route.params, this.route.queryParams, this.route.data,
+      observableCombineLatest([this.route.params, this.route.queryParams, this.scope$, this.route.data,
         this.currentPagination$, this.currentSort$]).pipe(
-        map(([routeParams, queryParams, data, currentPage, currentSort]) => {
-          return [Object.assign({}, routeParams, queryParams, data), currentPage, currentSort];
+        map(([routeParams, queryParams, scope, data, currentPage, currentSort]) => {
+          return [Object.assign({}, routeParams, queryParams, data), scope, currentPage, currentSort];
         })
-      ).subscribe(([params, currentPage, currentSort]: [Params, PaginationComponentOptions, SortOptions]) => {
+      ).subscribe(([params, scope, currentPage, currentSort]: [Params, string, PaginationComponentOptions, SortOptions]) => {
         const metadataKeys = params.browseDefinition ? params.browseDefinition.metadataKeys : this.defaultMetadataKeys;
         this.browseId = params.id || this.defaultBrowseId;
         this.startsWith = +params.startsWith || params.startsWith;
-        const searchOptions = browseParamsToOptions(params, currentPage, currentSort, this.browseId, this.fetchThumbnails);
+        const searchOptions = browseParamsToOptions(params, scope, currentPage, currentSort, this.browseId, this.fetchThumbnails);
         this.updatePageWithItems(searchOptions, this.value, undefined);
-        this.updateParent(params.scope);
-        this.updateLogo();
         this.updateStartsWithOptions(this.browseId, metadataKeys, params.scope);
       }));
   }
@@ -88,12 +83,21 @@ export class BrowseByDatePageComponent extends BrowseByMetadataPageComponent imp
    * @param scope           The scope under which to fetch the earliest item for
    */
   updateStartsWithOptions(definition: string, metadataKeys: string[], scope?: string) {
-    const firstItemRD = this.browseService.getFirstItemFor(definition, scope, SortDirection.ASC);
-    const lastItemRD = this.browseService.getFirstItemFor(definition, scope, SortDirection.DESC);
+    const firstItemRD$: Observable<RemoteData<Item>> = this.browseService.getFirstItemFor(definition, scope, SortDirection.ASC);
+    const lastItemRD$: Observable<RemoteData<Item>> = this.browseService.getFirstItemFor(definition, scope, SortDirection.DESC);
+    this.loading$ = observableCombineLatest([
+      firstItemRD$,
+      lastItemRD$,
+    ]).pipe(
+      map(([firstItemRD, lastItemRD]: [RemoteData<Item>, RemoteData<Item>]) => firstItemRD.isLoading || lastItemRD.isLoading)
+    );
     this.subs.push(
-      observableCombineLatest([firstItemRD, lastItemRD]).subscribe(([firstItem, lastItem]) => {
-        let lowerLimit: number = this.getLimit(firstItem, metadataKeys, this.appConfig.browseBy.defaultLowerLimit);
-        let upperLimit: number = this.getLimit(lastItem, metadataKeys, new Date().getUTCFullYear());
+      observableCombineLatest([
+        firstItemRD$,
+        lastItemRD$,
+      ]).subscribe(([firstItemRD, lastItemRD]: [RemoteData<Item>, RemoteData<Item>]) => {
+        let lowerLimit: number = this.getLimit(firstItemRD, metadataKeys, this.appConfig.browseBy.defaultLowerLimit);
+        let upperLimit: number = this.getLimit(lastItemRD, metadataKeys, new Date().getUTCFullYear());
         const options: number[] = [];
         const oneYearBreak: number = Math.floor((upperLimit - this.appConfig.browseBy.oneYearLimit) / 5) * 5;
         const fiveYearBreak: number = Math.floor((upperLimit - this.appConfig.browseBy.fiveYearLimit) / 10) * 10;

src/app/browse-by/browse-by-guard.spec.ts

@@ -1,17 +1,13 @@
 import { first } from 'rxjs/operators';
 import { BrowseByGuard } from './browse-by-guard';
-import { of as observableOf } from 'rxjs';
 import { createFailedRemoteDataObject$, createSuccessfulRemoteDataObject$ } from '../shared/remote-data.utils';
 import { BrowseByDataType } from './browse-by-switcher/browse-by-data-type';
 import { ValueListBrowseDefinition } from '../core/shared/value-list-browse-definition.model';
-import { DSONameServiceMock } from '../shared/mocks/dso-name.service.mock';
-import { DSONameService } from '../core/breadcrumbs/dso-name.service';
 import { RouterStub } from '../shared/testing/router.stub';
 
 describe('BrowseByGuard', () => {
   describe('canActivate', () => {
     let guard: BrowseByGuard;
-    let dsoService: any;
     let translateService: any;
     let browseDefinitionService: any;
     let router: any;
@@ -25,10 +21,6 @@ describe('BrowseByGuard', () => {
     const browseDefinition = Object.assign(new ValueListBrowseDefinition(), { type: BrowseByDataType.Metadata, metadataKeys: ['dc.contributor'] });
 
     beforeEach(() => {
-      dsoService = {
-        findById: (dsoId: string) => observableOf({ payload: { name: name }, hasSucceeded: true })
-      };
-
       translateService = {
         instant: () => field
       };
@@ -39,7 +31,7 @@ describe('BrowseByGuard', () => {
 
       router = new RouterStub() as any;
 
-      guard = new BrowseByGuard(dsoService, translateService, browseDefinitionService, new DSONameServiceMock() as DSONameService, router);
+      guard = new BrowseByGuard(translateService, browseDefinitionService, router);
     });
 
     it('should return true, and sets up the data correctly, with a scope and value', () => {
@@ -48,6 +40,7 @@ describe('BrowseByGuard', () => {
           title: field,
           browseDefinition,
         },
+        parent: null,
         params: {
           id,
         },
@@ -64,7 +57,7 @@ describe('BrowseByGuard', () => {
               title,
               id,
               browseDefinition,
-              collection: name,
+              scope,
               field,
               value: '"' + value + '"'
             };
@@ -97,7 +90,7 @@ describe('BrowseByGuard', () => {
               title,
               id,
               browseDefinition,
-              collection: name,
+              scope,
               field,
               value: ''
             };
@@ -108,12 +101,48 @@ describe('BrowseByGuard', () => {
         );
     });
 
+    it('should return true, and sets up the data correctly using the community/collection page id, with a scope and without value', () => {
+      const scopedNoValueRoute = {
+        data: {
+          title: field,
+          browseDefinition,
+        },
+        parent: {
+          params: {
+            id: scope,
+          },
+        },
+        params: {
+          id,
+        },
+        queryParams: {
+        },
+      };
+
+      guard.canActivate(scopedNoValueRoute as any, undefined).pipe(
+        first(),
+      ).subscribe((canActivate) => {
+        const result = {
+          title,
+          id,
+          browseDefinition,
+          scope,
+          field,
+          value: '',
+        };
+        expect(scopedNoValueRoute.data).toEqual(result);
+        expect(router.navigate).not.toHaveBeenCalled();
+        expect(canActivate).toEqual(true);
+      });
+    });
+
     it('should return true, and sets up the data correctly, without a scope and with a value', () => {
       const route = {
         data: {
           title: field,
           browseDefinition,
         },
+        parent: null,
         params: {
           id,
         },
@@ -129,7 +158,7 @@ describe('BrowseByGuard', () => {
               title,
               id,
               browseDefinition,
-              collection: '',
+              scope: undefined,
               field,
               value: '"' + value + '"'
             };
@@ -147,6 +176,7 @@ describe('BrowseByGuard', () => {
         data: {
           title: field,
         },
+        parent: null,
         params: {
           id,
         },