Created BrowseByPageComponent that uses the new refactored BrowseBySw…

…itcherComponent extending AbstractComponentLoaderComponent

- Added the context to the rendersBrowseBy decorator
- Created AbstractBrowseByTypeComponent that is extended by all the browse type sections
- Fixed a bug in BrowseService where findListByHref was called with null instead of undefined, which prevented its default values from being used

src/app/browse-by/abstract-browse-by-type.component.ts

@@ -0,0 +1,32 @@
+import { Component, Input, OnDestroy } from '@angular/core';
+import { BrowseByDataType } from './browse-by-switcher/browse-by-decorator';
+import { Context } from '../core/shared/context.model';
+import { Subscription } from 'rxjs';
+import { hasValue } from '../shared/empty.util';
+
+@Component({
+  selector: 'ds-abstract-browse-by-type',
+  template: '',
+})
+export abstract class AbstractBrowseByTypeComponent implements OnDestroy {
+
+  /**
+   * The optional context
+   */
+  @Input() context: Context;
+
+  /**
+   * The {@link BrowseByDataType} of this Component
+   */
+  @Input() browseByType: BrowseByDataType;
+
+  /**
+   * List of subscriptions
+   */
+  subs: Subscription[] = [];
+
+  ngOnDestroy(): void {
+    this.subs.filter((sub: Subscription) => hasValue(sub)).forEach((sub: Subscription) => sub.unsubscribe());
+  }
+
+}

src/app/browse-by/browse-by-metadata-page/browse-by-metadata-page.component.ts

@@ -1,4 +1,4 @@
-import { combineLatest as observableCombineLatest, Observable, Subscription } from 'rxjs';
+import { combineLatest as observableCombineLatest, Observable } from 'rxjs';
 import { Component, Inject, OnInit, OnDestroy } from '@angular/core';
 import { RemoteData } from '../../core/data/remote-data';
 import { PaginatedList } from '../../core/data/paginated-list.model';
@@ -23,6 +23,7 @@ import { Community } from '../../core/shared/community.model';
 import { APP_CONFIG, AppConfig } from '../../../config/app-config.interface';
 import { DSONameService } from '../../core/breadcrumbs/dso-name.service';
 import { rendersBrowseBy, BrowseByDataType } from '../browse-by-switcher/browse-by-decorator';
+import { AbstractBrowseByTypeComponent } from '../abstract-browse-by-type.component';
 
 export const BBM_PAGINATION_ID = 'bbm';
 
@@ -38,7 +39,7 @@ export const BBM_PAGINATION_ID = 'bbm';
  * 'dc.contributor.*'
  */
 @rendersBrowseBy(BrowseByDataType.Metadata)
-export class BrowseByMetadataPageComponent implements OnInit, OnDestroy {
+export class BrowseByMetadataPageComponent extends AbstractBrowseByTypeComponent implements OnInit, OnDestroy {
 
   /**
    * The list of browse-entries to display
@@ -75,11 +76,6 @@ export class BrowseByMetadataPageComponent implements OnInit, OnDestroy {
    */
   currentSort$: Observable<SortOptions>;
 
-  /**
-   * List of subscriptions
-   */
-  subs: Subscription[] = [];
-
   /**
    * The default browse id to resort to when none is provided
    */
@@ -132,7 +128,7 @@ export class BrowseByMetadataPageComponent implements OnInit, OnDestroy {
                      @Inject(APP_CONFIG) public appConfig: AppConfig,
                      public dsoNameService: DSONameService,
   ) {
-
+    super();
     this.fetchThumbnails = this.appConfig.browseBy.showThumbnails;
     this.paginationConfig = Object.assign(new PaginationComponentOptions(), {
         id: BBM_PAGINATION_ID,
@@ -276,7 +272,7 @@ export class BrowseByMetadataPageComponent implements OnInit, OnDestroy {
   }
 
   ngOnDestroy(): void {
-    this.subs.filter((sub) => hasValue(sub)).forEach((sub) => sub.unsubscribe());
+    super.ngOnDestroy();
     this.paginationService.clearPagination(this.paginationConfig.id);
   }
 

src/app/browse-by/browse-by-page.module.ts

@@ -5,21 +5,29 @@ import { ItemDataService } from '../core/data/item-data.service';
 import { BrowseService } from '../core/browse/browse.service';
 import { BrowseByGuard } from './browse-by-guard';
 import { SharedBrowseByModule } from '../shared/browse-by/shared-browse-by.module';
+import { BrowseByPageComponent } from './browse-by-page/browse-by-page.component';
+
+const DECLARATIONS = [
+  BrowseByPageComponent,
+];
 
 @NgModule({
   imports: [
     SharedBrowseByModule,
     BrowseByRoutingModule,
-    BrowseByModule.withEntryComponents(),
+    BrowseByModule,
   ],
   providers: [
     ItemDataService,
     BrowseService,
     BrowseByGuard,
   ],
   declarations: [
-
-  ]
+    ...DECLARATIONS,
+  ],
+  exports: [
+    ...DECLARATIONS,
+  ],
 })
 export class BrowseByPageModule {
 

src/app/browse-by/browse-by-page/browse-by-page.component.html

@@ -0,0 +1,2 @@
+<ds-browse-by-switcher [browseByType]="browseByType$ | async">
+</ds-browse-by-switcher>

src/app/browse-by/browse-by-page/browse-by-page.component.spec.ts

@@ -0,0 +1,24 @@
+import { ComponentFixture, TestBed } from '@angular/core/testing';
+import { BrowseByPageComponent } from './browse-by-page.component';
+
+// TODO port old logic from BrowseBySwitcherComponent
+fdescribe('BrowseByPageComponent', () => {
+  let component: BrowseByPageComponent;
+  let fixture: ComponentFixture<BrowseByPageComponent>;
+
+  beforeEach(async () => {
+    await TestBed.configureTestingModule({
+      declarations: [
+        BrowseByPageComponent,
+      ],
+    }).compileComponents();
+
+    fixture = TestBed.createComponent(BrowseByPageComponent);
+    component = fixture.componentInstance;
+    fixture.detectChanges();
+  });
+
+  it('should create', () => {
+    expect(component).toBeTruthy();
+  });
+});

src/app/browse-by/browse-by-page/browse-by-page.component.ts

@@ -0,0 +1,31 @@
+import { Component, OnInit } from '@angular/core';
+import { map } from 'rxjs/operators';
+import { BrowseDefinition } from '../../core/shared/browse-definition.model';
+import { ActivatedRoute } from '@angular/router';
+import { Observable } from 'rxjs';
+import { BrowseByDataType } from '../browse-by-switcher/browse-by-decorator';
+
+@Component({
+  selector: 'ds-browse-by-page',
+  templateUrl: './browse-by-page.component.html',
+  styleUrls: ['./browse-by-page.component.scss'],
+})
+export class BrowseByPageComponent implements OnInit {
+
+  browseByType$: Observable<BrowseByDataType>;
+
+  constructor(
+    protected route: ActivatedRoute,
+  ) {
+  }
+
+  /**
+   * Fetch the correct browse-by component by using the relevant config from the route data
+   */
+  ngOnInit(): void {
+    this.browseByType$ = this.route.data.pipe(
+      map((data: { browseDefinition: BrowseDefinition }) => data.browseDefinition.getRenderType()),
+    );
+  }
+
+}

src/app/browse-by/browse-by-routing.module.ts

@@ -3,7 +3,7 @@ import { NgModule } from '@angular/core';
 import { BrowseByGuard } from './browse-by-guard';
 import { BrowseByDSOBreadcrumbResolver } from './browse-by-dso-breadcrumb.resolver';
 import { BrowseByI18nBreadcrumbResolver } from './browse-by-i18n-breadcrumb.resolver';
-import { ThemedBrowseBySwitcherComponent } from './browse-by-switcher/themed-browse-by-switcher.component';
+import { BrowseByPageComponent } from './browse-by-page/browse-by-page.component';
 import { DSOEditMenuResolver } from '../shared/dso-page/dso-edit-menu.resolver';
 
 @NgModule({
@@ -18,7 +18,7 @@ import { DSOEditMenuResolver } from '../shared/dso-page/dso-edit-menu.resolver';
         children: [
           {
             path: ':id',
-            component: ThemedBrowseBySwitcherComponent,
+            component: BrowseByPageComponent,
             canActivate: [BrowseByGuard],
             resolve: { breadcrumb: BrowseByI18nBreadcrumbResolver },
             data: { title: 'browse.title.page', breadcrumbKey: 'browse.metadata' }

src/app/browse-by/browse-by-switcher/browse-by-decorator.ts

@@ -1,10 +1,9 @@
 import { hasNoValue } from '../../shared/empty.util';
 import { InjectionToken } from '@angular/core';
+import { DEFAULT_THEME, resolveTheme } from '../../shared/object-collection/shared/listable-object/listable-object.decorator';
+import { AbstractBrowseByTypeComponent } from '../abstract-browse-by-type.component';
+import { Context } from '../../core/shared/context.model';
 import { GenericConstructor } from '../../core/shared/generic-constructor';
-import {
-  DEFAULT_THEME,
-  resolveTheme
-} from '../../shared/object-collection/shared/listable-object/listable-object.decorator';
 
 export enum BrowseByDataType {
   Title = 'title',
@@ -14,41 +13,54 @@ export enum BrowseByDataType {
 }
 
 export const DEFAULT_BROWSE_BY_TYPE = BrowseByDataType.Metadata;
+export const DEFAULT_BROWSE_BY_CONTEXT = Context.Any;
 
-export const BROWSE_BY_COMPONENT_FACTORY = new InjectionToken<(browseByType, theme) => GenericConstructor<any>>('getComponentByBrowseByType', {
+export const BROWSE_BY_COMPONENT_FACTORY = new InjectionToken<(browseByType: BrowseByDataType, context: Context, theme: string) => GenericConstructor<AbstractBrowseByTypeComponent>>('getComponentByBrowseByType', {
   providedIn: 'root',
   factory: () => getComponentByBrowseByType
 });
 
-const map = new Map();
+const map: Map<BrowseByDataType, Map<Context, Map<string, GenericConstructor<AbstractBrowseByTypeComponent>>>> = new Map();
 
 /**
  * Decorator used for rendering Browse-By pages by type
  * @param browseByType  The type of page
+ * @param context The optional context for the component
  * @param theme The optional theme for the component
  */
-export function rendersBrowseBy(browseByType: string, theme = DEFAULT_THEME) {
+export function rendersBrowseBy(browseByType: BrowseByDataType, context = DEFAULT_BROWSE_BY_CONTEXT, theme = DEFAULT_THEME) {
   return function decorator(component: any) {
+    if (hasNoValue(browseByType)) {
+      return;
+    }
     if (hasNoValue(map.get(browseByType))) {
       map.set(browseByType, new Map());
     }
-    if (hasNoValue(map.get(browseByType).get(theme))) {
-      map.get(browseByType).set(theme, component);
+    if (hasNoValue(map.get(browseByType).get(context))) {
+      map.get(browseByType).set(context, new Map());
+    }
+    if (hasNoValue(map.get(browseByType).get(context).get(theme))) {
+      map.get(browseByType).get(context).set(theme, component);
     } else {
-      throw new Error(`There can't be more than one component to render Browse-By of type "${browseByType}" and theme "${theme}"`);
+      throw new Error(`There can't be more than one component to render Browse-By of type "${browseByType}", context "${context}" and theme "${theme}"`);
     }
   };
 }
 
 /**
  * Get the component used for rendering a Browse-By page by type
  * @param browseByType  The type of page
+ * @param context The context to match
  * @param theme the theme to match
  */
-export function getComponentByBrowseByType(browseByType, theme) {
-  let themeMap = map.get(browseByType);
+export function getComponentByBrowseByType(browseByType: BrowseByDataType, context: Context, theme: string): GenericConstructor<AbstractBrowseByTypeComponent> {
+  let contextMap: Map<Context, Map<string, GenericConstructor<AbstractBrowseByTypeComponent>>> = map.get(browseByType);
+  if (hasNoValue(contextMap)) {
+    contextMap = map.get(DEFAULT_BROWSE_BY_TYPE);
+  }
+  let themeMap: Map<string, GenericConstructor<AbstractBrowseByTypeComponent>> = contextMap.get(context);
   if (hasNoValue(themeMap)) {
-    themeMap = map.get(DEFAULT_BROWSE_BY_TYPE);
+    themeMap = contextMap.get(DEFAULT_BROWSE_BY_CONTEXT);
   }
   const comp = resolveTheme(themeMap, theme);
   if (hasNoValue(comp)) {

src/app/browse-by/browse-by-switcher/browse-by-switcher.component.spec.ts

@@ -1,13 +1,24 @@
 import { BrowseBySwitcherComponent } from './browse-by-switcher.component';
 import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
-import { NO_ERRORS_SCHEMA } from '@angular/core';
-import { ActivatedRoute } from '@angular/router';
-import { BROWSE_BY_COMPONENT_FACTORY, BrowseByDataType } from './browse-by-decorator';
-import { BehaviorSubject } from 'rxjs';
+import { SimpleChange, Component } from '@angular/core';
+import { BrowseByDataType, rendersBrowseBy } from './browse-by-decorator';
 import { ThemeService } from '../../shared/theme-support/theme.service';
 import { FlatBrowseDefinition } from '../../core/shared/flat-browse-definition.model';
 import { ValueListBrowseDefinition } from '../../core/shared/value-list-browse-definition.model';
 import { NonHierarchicalBrowseDefinition } from '../../core/shared/non-hierarchical-browse-definition';
+import { getMockThemeService } from '../../shared/mocks/theme-service.mock';
+import { DynamicComponentLoaderDirective } from '../../shared/abstract-component-loader/dynamic-component-loader.directive';
+import { RouterTestingModule } from '@angular/router/testing';
+import { AbstractBrowseByTypeComponent } from '../abstract-browse-by-type.component';
+
+@rendersBrowseBy('test' as BrowseByDataType)
+@Component({
+  // eslint-disable-next-line @angular-eslint/component-selector
+  selector: '',
+  template: '<span id="BrowseByTestComponent"></span>',
+})
+class BrowseByTestComponent extends AbstractBrowseByTypeComponent {
+}
 
 describe('BrowseBySwitcherComponent', () => {
   let comp: BrowseBySwitcherComponent;
@@ -41,46 +52,48 @@ describe('BrowseBySwitcherComponent', () => {
     ),
   ];
 
-  const data = new BehaviorSubject(createDataWithBrowseDefinition(new FlatBrowseDefinition()));
-
-  const activatedRouteStub = {
-    data
-  };
-
   let themeService: ThemeService;
-  let themeName: string;
+  const themeName = 'dspace';
 
   beforeEach(waitForAsync(() => {
-    themeName = 'dspace';
-    themeService = jasmine.createSpyObj('themeService', {
-      getThemeName: themeName,
-    });
+    themeService = getMockThemeService(themeName);
 
-    TestBed.configureTestingModule({
-      declarations: [BrowseBySwitcherComponent],
+    void TestBed.configureTestingModule({
+      imports: [
+        RouterTestingModule,
+      ],
+      declarations: [
+        BrowseBySwitcherComponent,
+        DynamicComponentLoaderDirective,
+      ],
       providers: [
-        { provide: ActivatedRoute, useValue: activatedRouteStub },
+        BrowseByTestComponent,
         { provide: ThemeService, useValue: themeService },
-        { provide: BROWSE_BY_COMPONENT_FACTORY, useValue: jasmine.createSpy('getComponentByBrowseByType').and.returnValue(null) }
       ],
-      schemas: [NO_ERRORS_SCHEMA]
     }).compileComponents();
   }));
 
   beforeEach(waitForAsync(() => {
     fixture = TestBed.createComponent(BrowseBySwitcherComponent);
     comp = fixture.componentInstance;
+    spyOn(comp, 'getComponent').and.returnValue(BrowseByTestComponent);
+    spyOn(comp, 'connectInputsAndOutputs').and.callThrough();
   }));
 
   types.forEach((type: NonHierarchicalBrowseDefinition) => {
     describe(`when switching to a browse-by page for "${type.id}"`, () => {
-      beforeEach(() => {
-        data.next(createDataWithBrowseDefinition(type));
+      beforeEach(async () => {
+        comp.browseByType = type.dataType;
+        comp.ngOnChanges({
+          browseByType: new SimpleChange(undefined, type.dataType, true),
+        });
         fixture.detectChanges();
+        await fixture.whenStable();
       });
 
-      it(`should call getComponentByBrowseByType with type "${type.dataType}"`, () => {
-        expect((comp as any).getComponentByBrowseByType).toHaveBeenCalledWith(type.dataType, themeName);
+      it(`should call getComponent with type "${type.dataType}"`, () => {
+        expect(comp.getComponent).toHaveBeenCalled();
+        expect(comp.connectInputsAndOutputs).toHaveBeenCalled();
       });
     });
   });