From b86ae8dd143d9970e126c2a6f5154a9c9c5ec74e Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Tue, 14 Sep 2021 14:16:04 -0500 Subject: [PATCH 01/25] Allow enter key to expand or contract menu based on current active status --- .../expandable-navbar-section.component.html | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html index cb96435b77..45edc12b12 100644 --- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html +++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html @@ -1,18 +1,20 @@ -
- - - - -
+ *ngComponentOutlet="(sectionMap$ | async).get(section.id).component; injector: (sectionMap$ | async).get(section.id).injector;"> + + + + From 8e2ab83d92741e78a8a10984f1cac673ac960af0 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Fri, 17 Sep 2021 11:15:36 -0500 Subject: [PATCH 02/25] Fix existing tests. Add new specs for enter key presses --- .../expandable-navbar-section.component.html | 37 ++++++++--------- ...xpandable-navbar-section.component.spec.ts | 41 ++++++++++++++++++- 2 files changed, 57 insertions(+), 21 deletions(-) diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html index 45edc12b12..04bc6067ac 100644 --- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html +++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html @@ -1,20 +1,19 @@ - -
- +
+ + + +
    + - -
      - - - -
    -
- + *ngComponentOutlet="(sectionMap$ | async).get(subSection.id).component; injector: (sectionMap$ | async).get(subSection.id).injector;"> + + +
diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.spec.ts b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.spec.ts index 8eea76e5c6..6b7c3d4bfc 100644 --- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.spec.ts +++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.spec.ts @@ -9,6 +9,7 @@ import { HostWindowService } from '../../shared/host-window.service'; import { MenuService } from '../../shared/menu/menu.service'; import { HostWindowServiceStub } from '../../shared/testing/host-window-service.stub'; import { NoopAnimationsModule } from '@angular/platform-browser/animations'; +import { VarDirective } from '../../shared/utils/var.directive'; describe('ExpandableNavbarSectionComponent', () => { let component: ExpandableNavbarSectionComponent; @@ -19,7 +20,7 @@ describe('ExpandableNavbarSectionComponent', () => { beforeEach(waitForAsync(() => { TestBed.configureTestingModule({ imports: [NoopAnimationsModule], - declarations: [ExpandableNavbarSectionComponent, TestComponent], + declarations: [ExpandableNavbarSectionComponent, TestComponent, VarDirective], providers: [ { provide: 'sectionDataProvider', useValue: {} }, { provide: MenuService, useValue: menuService }, @@ -76,6 +77,42 @@ describe('ExpandableNavbarSectionComponent', () => { }); }); + describe('when Enter key is pressed on section header (while inactive)', () => { + beforeEach(() => { + spyOn(menuService, 'activateSection'); + // Make sure section is 'inactive'. Requires calling ngOnInit() to update component 'active' property. + spyOn(menuService, 'isSectionActive').and.returnValue(observableOf(false)); + component.ngOnInit(); + fixture.detectChanges(); + + const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); + // dispatch the (keyup.enter) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: 'Enter' })); + }); + + it('should call activateSection on the menuService', () => { + expect(menuService.activateSection).toHaveBeenCalled(); + }); + }); + + describe('when Enter key is pressed on section header (while active)', () => { + beforeEach(() => { + spyOn(menuService, 'deactivateSection'); + // Make sure section is 'active'. Requires calling ngOnInit() to update component 'active' property. + spyOn(menuService, 'isSectionActive').and.returnValue(observableOf(true)); + component.ngOnInit(); + fixture.detectChanges(); + + const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); + // dispatch the (keyup.enter) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: 'Enter' })); + }); + + it('should call deactivateSection on the menuService', () => { + expect(menuService.deactivateSection).toHaveBeenCalled(); + }); + }); + describe('when a click occurs on the section header', () => { beforeEach(() => { spyOn(menuService, 'toggleActiveSection'); @@ -96,7 +133,7 @@ describe('ExpandableNavbarSectionComponent', () => { beforeEach(waitForAsync(() => { TestBed.configureTestingModule({ imports: [NoopAnimationsModule], - declarations: [ExpandableNavbarSectionComponent, TestComponent], + declarations: [ExpandableNavbarSectionComponent, TestComponent, VarDirective], providers: [ { provide: 'sectionDataProvider', useValue: {} }, { provide: MenuService, useValue: menuService }, From 523fca2177f79a42fea3293e7361ee0b183d3a8b Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Fri, 17 Sep 2021 11:16:00 -0500 Subject: [PATCH 03/25] Minor updates to README to provide more helpful info on unit tests --- README.md | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 0e98bc7cf9..69b6132478 100644 --- a/README.md +++ b/README.md @@ -212,13 +212,17 @@ Once you have tested the Pull Request, please add a comment and/or approval to t ### Unit Tests -Unit tests use Karma. You can find the configuration file at the same level of this README file:`./karma.conf.js` If you are going to use a remote test environment you need to edit the `./karma.conf.js`. Follow the instructions you will find inside it. To executing tests whenever any file changes you can modify the 'autoWatch' option to 'true' and 'singleRun' option to 'false'. A coverage report is also available at: http://localhost:9876/ after you run: `yarn run coverage`. +Unit tests use the [Jasmine test framework](https://jasmine.github.io/), and are run via [Karma](https://karma-runner.github.io/). + +You can find the Karma configuration file at the same level of this README file:`./karma.conf.js` If you are going to use a remote test environment you need to edit the `./karma.conf.js`. Follow the instructions you will find inside it. To executing tests whenever any file changes you can modify the 'autoWatch' option to 'true' and 'singleRun' option to 'false'. A coverage report is also available at: http://localhost:9876/ after you run: `yarn run coverage`. The default browser is Google Chrome. -Place your tests in the same location of the application source code files that they test. +Place your tests in the same location of the application source code files that they test, e.g. ending with `*.component.spec.ts` -and run: `yarn run test` +and run: `yarn test` + +If you run into odd test errors, see the Angular guide to debugging tests: https://angular.io/guide/test-debugging ### E2E Tests @@ -258,6 +262,10 @@ _Hint: Creating e2e tests is easiest in an IDE (like Visual Studio), as it can h More Information: [docs.cypress.io](https://docs.cypress.io/) has great guides & documentation helping you learn more about writing/debugging e2e tests in Cypress. +### Learning how to build tests + +See our [DSpace Code Testing Guide](https://wiki.lyrasis.org/display/DSPACE/Code+Testing+Guide) for more hints/tips. + Documentation -------------- From 0b62144d97d87d8d7fbfce4740ab95da9730f3b8 Mon Sep 17 00:00:00 2001 From: Yura Date: Fri, 17 Sep 2021 15:11:49 +0200 Subject: [PATCH 04/25] 83631: Add the ability to extend a theme --- .../metadata-representation.decorator.spec.ts | 60 +++++++ .../metadata-representation.decorator.ts | 6 +- src/app/shared/mocks/theme-service.mock.ts | 15 +- .../listable-object.decorator.spec.ts | 60 +++++++ .../listable-object.decorator.ts | 50 +++++- src/app/shared/theme-support/theme.service.ts | 15 +- .../theme-support/themed.component.spec.ts | 153 +++++++++++++++--- .../shared/theme-support/themed.component.ts | 76 ++++++--- src/config/theme.model.ts | 6 + 9 files changed, 381 insertions(+), 60 deletions(-) diff --git a/src/app/shared/metadata-representation/metadata-representation.decorator.spec.ts b/src/app/shared/metadata-representation/metadata-representation.decorator.spec.ts index 25c5be0129..a9988aad6f 100644 --- a/src/app/shared/metadata-representation/metadata-representation.decorator.spec.ts +++ b/src/app/shared/metadata-representation/metadata-representation.decorator.spec.ts @@ -7,12 +7,17 @@ import { import { MetadataRepresentationType } from '../../core/shared/metadata-representation/metadata-representation.model'; import { Context } from '../../core/shared/context.model'; import * as uuidv4 from 'uuid/v4'; +import { environment } from '../../../environments/environment'; + +let ogEnvironmentThemes; describe('MetadataRepresentation decorator function', () => { const type1 = 'TestType'; const type2 = 'TestType2'; const type3 = 'TestType3'; const type4 = 'RandomType'; + const typeHier1 = 'TestTypeHier1'; + const typeHier2 = 'TestTypeHier2'; let prefix; /* tslint:disable:max-classes-per-file */ @@ -31,6 +36,12 @@ describe('MetadataRepresentation decorator function', () => { class Test3ItemSubmission { } + class TestHier1Ancestor { + } + + class TestHier2Unthemed { + } + /* tslint:enable:max-classes-per-file */ beforeEach(() => { @@ -46,8 +57,17 @@ describe('MetadataRepresentation decorator function', () => { metadataRepresentationComponent(key + type2, MetadataRepresentationType.Item, Context.Workspace)(Test2ItemSubmission); metadataRepresentationComponent(key + type3, MetadataRepresentationType.Item, Context.Workspace)(Test3ItemSubmission); + + metadataRepresentationComponent(key + typeHier1, MetadataRepresentationType.Item, Context.Any, 'ancestor')(TestHier1Ancestor); + metadataRepresentationComponent(key + typeHier2, MetadataRepresentationType.Item, Context.Any)(TestHier2Unthemed); + + ogEnvironmentThemes = environment.themes; } + afterEach(() => { + environment.themes = ogEnvironmentThemes; + }); + describe('If there\'s an exact match', () => { it('should return the matching class', () => { const component = getMetadataRepresentationComponent(prefix + type3, MetadataRepresentationType.Item, Context.Workspace); @@ -76,4 +96,44 @@ describe('MetadataRepresentation decorator function', () => { }); }); }); + + describe('With theme extensions', () => { + describe('If requested theme has no match', () => { + beforeEach(() => { + environment.themes = [ + { name: 'requested', extends: 'intermediate' }, + { name: 'intermediate', extends: 'ancestor' }, + ]; + }); + + it('should return component from ancestor theme if it has a match', () => { + const component = getMetadataRepresentationComponent(prefix + typeHier1, MetadataRepresentationType.Item, Context.Any, 'requested'); + expect(component).toEqual(TestHier1Ancestor); + }); + + it('should return default component if ancestor theme has no match', () => { + const component = getMetadataRepresentationComponent(prefix + typeHier2, MetadataRepresentationType.Item, Context.Any, 'requested'); + expect(component).toEqual(TestHier2Unthemed); + }); + }); + + describe('If there is a theme extension cycle', () => { + beforeEach(() => { + environment.themes = [ + { name: 'extension-cycle', extends: 'broken1' }, + { name: 'broken1', extends: 'broken2' }, + { name: 'broken2', extends: 'broken3' }, + { name: 'broken3', extends: 'broken1' }, + ]; + }); + + it('should throw an error', () => { + expect(() => { + getMetadataRepresentationComponent(prefix + typeHier1, MetadataRepresentationType.Item, Context.Any, 'extension-cycle'); + }).toThrowError( + 'Theme extension cycle detected: extension-cycle -> broken1 -> broken2 -> broken3 -> broken1' + ); + }); + }); + }); }); diff --git a/src/app/shared/metadata-representation/metadata-representation.decorator.ts b/src/app/shared/metadata-representation/metadata-representation.decorator.ts index 0b5bea33d9..eca08c5c62 100644 --- a/src/app/shared/metadata-representation/metadata-representation.decorator.ts +++ b/src/app/shared/metadata-representation/metadata-representation.decorator.ts @@ -3,6 +3,7 @@ import { hasNoValue, hasValue } from '../empty.util'; import { Context } from '../../core/shared/context.model'; import { InjectionToken } from '@angular/core'; import { GenericConstructor } from '../../core/shared/generic-constructor'; +import { resolveTheme } from '../object-collection/shared/listable-object/listable-object.decorator'; export const METADATA_REPRESENTATION_COMPONENT_FACTORY = new InjectionToken<(entityType: string, mdRepresentationType: MetadataRepresentationType, context: Context, theme: string) => GenericConstructor>('getMetadataRepresentationComponent', { providedIn: 'root', @@ -57,8 +58,9 @@ export function getMetadataRepresentationComponent(entityType: string, mdReprese if (hasValue(entityAndMDRepMap)) { const contextMap = entityAndMDRepMap.get(context); if (hasValue(contextMap)) { - if (hasValue(contextMap.get(theme))) { - return contextMap.get(theme); + const match = resolveTheme(contextMap, theme); + if (hasValue(match)) { + return match; } if (hasValue(contextMap.get(DEFAULT_THEME))) { return contextMap.get(DEFAULT_THEME); diff --git a/src/app/shared/mocks/theme-service.mock.ts b/src/app/shared/mocks/theme-service.mock.ts index 3594270807..058ba993bc 100644 --- a/src/app/shared/mocks/theme-service.mock.ts +++ b/src/app/shared/mocks/theme-service.mock.ts @@ -1,9 +1,18 @@ import { ThemeService } from '../theme-support/theme.service'; import { of as observableOf } from 'rxjs'; +import { ThemeConfig } from '../../../config/theme.model'; +import { isNotEmpty } from '../empty.util'; -export function getMockThemeService(themeName = 'base'): ThemeService { - return jasmine.createSpyObj('themeService', { +export function getMockThemeService(themeName = 'base', themes?: ThemeConfig[]): ThemeService { + const spy = jasmine.createSpyObj('themeService', { getThemeName: themeName, - getThemeName$: observableOf(themeName) + getThemeName$: observableOf(themeName), + getThemeConfigFor: undefined, }); + + if (isNotEmpty(themes)) { + spy.getThemeConfigFor.and.callFake((name: string) => themes.find(theme => theme.name === name)); + } + + return spy; } diff --git a/src/app/shared/object-collection/shared/listable-object/listable-object.decorator.spec.ts b/src/app/shared/object-collection/shared/listable-object/listable-object.decorator.spec.ts index 19765f86b0..d3c7a5a1e1 100644 --- a/src/app/shared/object-collection/shared/listable-object/listable-object.decorator.spec.ts +++ b/src/app/shared/object-collection/shared/listable-object/listable-object.decorator.spec.ts @@ -2,11 +2,16 @@ import { Item } from '../../../../core/shared/item.model'; import { ViewMode } from '../../../../core/shared/view-mode.model'; import { getListableObjectComponent, listableObjectComponent } from './listable-object.decorator'; import { Context } from '../../../../core/shared/context.model'; +import { environment } from '../../../../../environments/environment'; + +let ogEnvironmentThemes; describe('ListableObject decorator function', () => { const type1 = 'TestType'; const type2 = 'TestType2'; const type3 = 'TestType3'; + const typeHier1 = 'TestTypeHier1'; + const typeHier2 = 'TestTypeHier2'; /* tslint:disable:max-classes-per-file */ class Test1List { @@ -27,6 +32,12 @@ describe('ListableObject decorator function', () => { class Test3DetailedSubmission { } + class TestHier1Ancestor { + } + + class TestHier2Unthemed { + } + /* tslint:enable:max-classes-per-file */ beforeEach(() => { @@ -38,6 +49,15 @@ describe('ListableObject decorator function', () => { listableObjectComponent(type3, ViewMode.ListElement)(Test3List); listableObjectComponent(type3, ViewMode.DetailedListElement, Context.Workspace)(Test3DetailedSubmission); + + listableObjectComponent(typeHier1, ViewMode.ListElement, Context.Any, 'ancestor')(TestHier1Ancestor); + listableObjectComponent(typeHier2, ViewMode.ListElement, Context.Any)(TestHier2Unthemed); + + ogEnvironmentThemes = environment.themes; + }); + + afterEach(() => { + environment.themes = ogEnvironmentThemes; }); const gridDecorator = listableObjectComponent('Item', ViewMode.GridElement); @@ -80,4 +100,44 @@ describe('ListableObject decorator function', () => { }); }); }); + + describe('With theme extensions', () => { + describe('If requested theme has no match', () => { + beforeEach(() => { + environment.themes = [ + { name: 'requested', extends: 'intermediate' }, + { name: 'intermediate', extends: 'ancestor' }, + ]; + }); + + it('should return component from ancestor theme if it has a match', () => { + const component = getListableObjectComponent([typeHier1], ViewMode.ListElement, Context.Any, 'requested'); + expect(component).toEqual(TestHier1Ancestor); + }); + + it('should return default component if ancestor theme has no match', () => { + const component = getListableObjectComponent([typeHier2], ViewMode.ListElement, Context.Any, 'requested'); + expect(component).toEqual(TestHier2Unthemed); + }); + }); + + describe('If there is a theme extension cycle', () => { + beforeEach(() => { + environment.themes = [ + { name: 'extension-cycle', extends: 'broken1' }, + { name: 'broken1', extends: 'broken2' }, + { name: 'broken2', extends: 'broken3' }, + { name: 'broken3', extends: 'broken1' }, + ]; + }); + + it('should throw an error', () => { + expect(() => { + getListableObjectComponent([typeHier1], ViewMode.ListElement, Context.Any, 'extension-cycle'); + }).toThrowError( + 'Theme extension cycle detected: extension-cycle -> broken1 -> broken2 -> broken3 -> broken1' + ); + }); + }); + }); }); diff --git a/src/app/shared/object-collection/shared/listable-object/listable-object.decorator.ts b/src/app/shared/object-collection/shared/listable-object/listable-object.decorator.ts index 91140f0ea1..5df2683bb9 100644 --- a/src/app/shared/object-collection/shared/listable-object/listable-object.decorator.ts +++ b/src/app/shared/object-collection/shared/listable-object/listable-object.decorator.ts @@ -1,15 +1,26 @@ import { ViewMode } from '../../../../core/shared/view-mode.model'; import { Context } from '../../../../core/shared/context.model'; -import { hasNoValue, hasValue } from '../../../empty.util'; +import { hasNoValue, hasValue, isNotEmpty } from '../../../empty.util'; import { DEFAULT_CONTEXT, DEFAULT_THEME } from '../../../metadata-representation/metadata-representation.decorator'; import { GenericConstructor } from '../../../../core/shared/generic-constructor'; import { ListableObject } from '../listable-object.model'; +import { environment } from '../../../../../environments/environment'; +import { ThemeConfig } from '../../../../../config/theme.model'; +import { InjectionToken } from '@angular/core'; export const DEFAULT_VIEW_MODE = ViewMode.ListElement; +/** + * Factory to allow us to inject getThemeConfigFor so we can mock it in tests + */ +export const GET_THEME_CONFIG_FOR_FACTORY = new InjectionToken<(str) => ThemeConfig>('getThemeConfigFor', { + providedIn: 'root', + factory: () => getThemeConfigFor +}); + const map = new Map(); /** @@ -54,8 +65,9 @@ export function getListableObjectComponent(types: (string | GenericConstructor { + return environment.themes.find(theme => theme.name === themeName); +}; + +/** + * Find a match in the given map for the given theme name, taking theme extension into account + * + * @param contextMap A map of theme names to components + * @param themeName The name of the theme to check + * @param checkedThemeNames The list of theme names that are already checked + */ +export const resolveTheme = (contextMap: Map, themeName: string, checkedThemeNames: string[] = []): any => { + const match = contextMap.get(themeName); + if (hasValue(match)) { + return match; + } else { + const cfg = getThemeConfigFor(themeName); + if (hasValue(cfg) && isNotEmpty(cfg.extends)) { + const nextTheme = cfg.extends; + const nextCheckedThemeNames = [...checkedThemeNames, themeName]; + if (checkedThemeNames.includes(nextTheme)) { + throw new Error('Theme extension cycle detected: ' + [...nextCheckedThemeNames, nextTheme].join(' -> ')); + } else { + return resolveTheme(contextMap, nextTheme, nextCheckedThemeNames); + } + } + } +}; diff --git a/src/app/shared/theme-support/theme.service.ts b/src/app/shared/theme-support/theme.service.ts index 7b0af93e04..7f000447e4 100644 --- a/src/app/shared/theme-support/theme.service.ts +++ b/src/app/shared/theme-support/theme.service.ts @@ -1,10 +1,16 @@ -import { Injectable } from '@angular/core'; +import { Injectable, Inject } from '@angular/core'; import { Store, createFeatureSelector, createSelector, select } from '@ngrx/store'; import { Observable } from 'rxjs/internal/Observable'; import { ThemeState } from './theme.reducer'; import { SetThemeAction } from './theme.actions'; import { take } from 'rxjs/operators'; import { hasValue } from '../empty.util'; +import { ThemeConfig } from '../../../config/theme.model'; +import { environment } from '../../../environments/environment'; +import { + GET_THEME_CONFIG_FOR_FACTORY, + getThemeConfigFor +} from '../object-collection/shared/listable-object/listable-object.decorator'; export const themeStateSelector = createFeatureSelector('theme'); @@ -19,6 +25,7 @@ export const currentThemeSelector = createSelector( export class ThemeService { constructor( private store: Store, + @Inject(GET_THEME_CONFIG_FOR_FACTORY) private gtcf: (str) => ThemeConfig ) { } @@ -43,4 +50,10 @@ export class ThemeService { ); } + /** + * Searches for a ThemeConfig by its name; + */ + getThemeConfigFor(themeName: string): ThemeConfig { + return this.gtcf(themeName); + } } diff --git a/src/app/shared/theme-support/themed.component.spec.ts b/src/app/shared/theme-support/themed.component.spec.ts index abaee28a29..1db6de072d 100644 --- a/src/app/shared/theme-support/themed.component.spec.ts +++ b/src/app/shared/theme-support/themed.component.spec.ts @@ -5,6 +5,7 @@ import { VarDirective } from '../utils/var.directive'; import { ThemeService } from './theme.service'; import { getMockThemeService } from '../mocks/theme-service.mock'; import { TestComponent } from './test/test.component.spec'; +import { ThemeConfig } from '../../../config/theme.model'; /* tslint:disable:max-classes-per-file */ @Component({ @@ -32,8 +33,8 @@ describe('ThemedComponent', () => { let fixture: ComponentFixture; let themeService: ThemeService; - function setupTestingModuleForTheme(theme: string) { - themeService = getMockThemeService(theme); + function setupTestingModuleForTheme(theme: string, themes?: ThemeConfig[]) { + themeService = getMockThemeService(theme, themes); TestBed.configureTestingModule({ imports: [], declarations: [TestThemedComponent, VarDirective], @@ -44,17 +45,20 @@ describe('ThemedComponent', () => { }).compileComponents(); } + function initComponent() { + fixture = TestBed.createComponent(TestThemedComponent); + component = fixture.componentInstance; + spyOn(component as any, 'importThemedComponent').and.callThrough(); + component.testInput = 'changed'; + fixture.detectChanges(); + } + describe('when the current theme matches a themed component', () => { beforeEach(waitForAsync(() => { setupTestingModuleForTheme('custom'); })); - beforeEach(() => { - fixture = TestBed.createComponent(TestThemedComponent); - component = fixture.componentInstance; - component.testInput = 'changed'; - fixture.detectChanges(); - }); + beforeEach(initComponent); it('should set compRef to the themed component', waitForAsync(() => { fixture.whenStable().then(() => { @@ -70,28 +74,127 @@ describe('ThemedComponent', () => { }); describe('when the current theme doesn\'t match a themed component', () => { - beforeEach(waitForAsync(() => { - setupTestingModuleForTheme('non-existing-theme'); - })); + describe('and it doesn\'t extend another theme', () => { + beforeEach(waitForAsync(() => { + setupTestingModuleForTheme('non-existing-theme'); + })); - beforeEach(() => { - fixture = TestBed.createComponent(TestThemedComponent); - component = fixture.componentInstance; - component.testInput = 'changed'; - fixture.detectChanges(); + beforeEach(initComponent); + + it('should set compRef to the default component', waitForAsync(() => { + fixture.whenStable().then(() => { + expect((component as any).compRef.instance.type).toEqual('default'); + }); + })); + + it('should sync up this component\'s input with the default component', waitForAsync(() => { + fixture.whenStable().then(() => { + expect((component as any).compRef.instance.testInput).toEqual('changed'); + }); + })); }); - it('should set compRef to the default component', waitForAsync(() => { - fixture.whenStable().then(() => { - expect((component as any).compRef.instance.type).toEqual('default'); - }); - })); + describe('and it extends another theme', () => { + describe('that doesn\'t match it either', () => { + beforeEach(waitForAsync(() => { + setupTestingModuleForTheme('current-theme', [ + { name: 'current-theme', extends: 'non-existing-theme' }, + ]); + })); - it('should sync up this component\'s input with the default component', waitForAsync(() => { - fixture.whenStable().then(() => { - expect((component as any).compRef.instance.testInput).toEqual('changed'); + beforeEach(initComponent); + + it('should set compRef to the default component', waitForAsync(() => { + fixture.whenStable().then(() => { + expect((component as any).importThemedComponent).toHaveBeenCalledWith('current-theme'); + expect((component as any).importThemedComponent).toHaveBeenCalledWith('non-existing-theme'); + expect((component as any).compRef.instance.type).toEqual('default'); + }); + })); + + it('should sync up this component\'s input with the default component', waitForAsync(() => { + fixture.whenStable().then(() => { + expect((component as any).compRef.instance.testInput).toEqual('changed'); + }); + })); }); - })); + + describe('that does match it', () => { + beforeEach(waitForAsync(() => { + setupTestingModuleForTheme('current-theme', [ + { name: 'current-theme', extends: 'custom' }, + ]); + })); + + beforeEach(initComponent); + + it('should set compRef to the themed component', waitForAsync(() => { + fixture.whenStable().then(() => { + expect((component as any).importThemedComponent).toHaveBeenCalledWith('current-theme'); + expect((component as any).importThemedComponent).toHaveBeenCalledWith('custom'); + expect((component as any).compRef.instance.type).toEqual('themed'); + }); + })); + + it('should sync up this component\'s input with the themed component', waitForAsync(() => { + fixture.whenStable().then(() => { + expect((component as any).compRef.instance.testInput).toEqual('changed'); + }); + })); + }); + + describe('that extends another theme that doesn\'t match it either', () => { + beforeEach(waitForAsync(() => { + setupTestingModuleForTheme('current-theme', [ + { name: 'current-theme', extends: 'parent-theme' }, + { name: 'parent-theme', extends: 'non-existing-theme' }, + ]); + })); + + beforeEach(initComponent); + + it('should set compRef to the default component', waitForAsync(() => { + fixture.whenStable().then(() => { + expect((component as any).importThemedComponent).toHaveBeenCalledWith('current-theme'); + expect((component as any).importThemedComponent).toHaveBeenCalledWith('parent-theme'); + expect((component as any).importThemedComponent).toHaveBeenCalledWith('non-existing-theme'); + expect((component as any).compRef.instance.type).toEqual('default'); + }); + })); + + it('should sync up this component\'s input with the default component', waitForAsync(() => { + fixture.whenStable().then(() => { + expect((component as any).compRef.instance.testInput).toEqual('changed'); + }); + })); + }); + + describe('that extends another theme that does match it', () => { + beforeEach(waitForAsync(() => { + setupTestingModuleForTheme('current-theme', [ + { name: 'current-theme', extends: 'parent-theme' }, + { name: 'parent-theme', extends: 'custom' }, + ]); + })); + + beforeEach(initComponent); + + it('should set compRef to the themed component', waitForAsync(() => { + fixture.whenStable().then(() => { + expect((component as any).importThemedComponent).toHaveBeenCalledWith('current-theme'); + expect((component as any).importThemedComponent).toHaveBeenCalledWith('parent-theme'); + expect((component as any).importThemedComponent).toHaveBeenCalledWith('custom'); + expect((component as any).compRef.instance.type).toEqual('themed'); + }); + })); + + it('should sync up this component\'s input with the themed component', waitForAsync(() => { + fixture.whenStable().then(() => { + expect((component as any).compRef.instance.testInput).toEqual('changed'); + }); + })); + }); + }); }); }); /* tslint:enable:max-classes-per-file */ diff --git a/src/app/shared/theme-support/themed.component.ts b/src/app/shared/theme-support/themed.component.ts index 1a41327209..6646c0aa30 100644 --- a/src/app/shared/theme-support/themed.component.ts +++ b/src/app/shared/theme-support/themed.component.ts @@ -11,7 +11,7 @@ import { OnChanges } from '@angular/core'; import { hasValue, isNotEmpty } from '../empty.util'; -import { Subscription } from 'rxjs'; +import { Observable, of as observableOf, Subscription } from 'rxjs'; import { ThemeService } from './theme.service'; import { fromPromise } from 'rxjs/internal-compatibility'; import { catchError, switchMap, map } from 'rxjs/operators'; @@ -69,31 +69,27 @@ export abstract class ThemedComponent implements OnInit, OnDestroy, OnChanges this.lazyLoadSub.unsubscribe(); } - this.lazyLoadSub = - fromPromise(this.importThemedComponent(this.themeService.getThemeName())).pipe( - // if there is no themed version of the component an exception is thrown, - // catch it and return null instead - catchError(() => [null]), - switchMap((themedFile: any) => { - if (hasValue(themedFile) && hasValue(themedFile[this.getComponentName()])) { - // if the file is not null, and exports a component with the specified name, - // return that component - return [themedFile[this.getComponentName()]]; - } else { - // otherwise import and return the default component - return fromPromise(this.importUnthemedComponent()).pipe( - map((unthemedFile: any) => { - return unthemedFile[this.getComponentName()]; - }) - ); - } - }), - ).subscribe((constructor: GenericConstructor) => { - const factory = this.resolver.resolveComponentFactory(constructor); - this.compRef = this.vcr.createComponent(factory); - this.connectInputsAndOutputs(); - this.cdr.markForCheck(); - }); + this.lazyLoadSub = this.resolveThemedComponent(this.themeService.getThemeName()).pipe( + switchMap((themedFile: any) => { + if (hasValue(themedFile) && hasValue(themedFile[this.getComponentName()])) { + // if the file is not null, and exports a component with the specified name, + // return that component + return [themedFile[this.getComponentName()]]; + } else { + // otherwise import and return the default component + return fromPromise(this.importUnthemedComponent()).pipe( + map((unthemedFile: any) => { + return unthemedFile[this.getComponentName()]; + }) + ); + } + }), + ).subscribe((constructor: GenericConstructor) => { + const factory = this.resolver.resolveComponentFactory(constructor); + this.compRef = this.vcr.createComponent(factory); + this.connectInputsAndOutputs(); + this.cdr.markForCheck(); + }); } protected destroyComponentInstance(): void { @@ -113,4 +109,32 @@ export abstract class ThemedComponent implements OnInit, OnDestroy, OnChanges }); } } + + /** + * Attempt to import this component from the current theme or a theme it {@link NamedThemeConfig.extends}. + * Recurse until we succeed or when until we run out of themes to fall back to. + * + * @param themeName The name of the theme to check + * @param checkedThemeNames The list of theme names that are already checked + * @private + */ + private resolveThemedComponent(themeName?: string, checkedThemeNames: string[] = []): Observable { + if (isNotEmpty(themeName)) { + return fromPromise(this.importThemedComponent(themeName)).pipe( + catchError(() => { + // Try the next ancestor theme instead + const nextTheme = this.themeService.getThemeConfigFor(themeName)?.extends; + const nextCheckedThemeNames = [...checkedThemeNames, themeName]; + if (checkedThemeNames.includes(nextTheme)) { + throw new Error('Theme extension cycle detected: ' + [...nextCheckedThemeNames, nextTheme].join(' -> ')); + } else { + return this.resolveThemedComponent(nextTheme, nextCheckedThemeNames); + } + }), + ); + } else { + // If we got here, we've failed to import this component from any ancestor theme → fall back to unthemed + return observableOf(null); + } + } } diff --git a/src/config/theme.model.ts b/src/config/theme.model.ts index 908589c71c..0130b5ffd8 100644 --- a/src/config/theme.model.ts +++ b/src/config/theme.model.ts @@ -6,6 +6,12 @@ import { getDSORoute } from '../app/app-routing-paths'; // tslint:disable:max-classes-per-file export interface NamedThemeConfig extends Config { name: string; + + /** + * Specify another theme to build upon: whenever a themed component is not found in the current theme, + * its ancestor theme(s) will be checked recursively before falling back to the default theme. + */ + extends?: string; } export interface RegExThemeConfig extends NamedThemeConfig { From 5a136f8865a99b9a5341f8a3c8bc237cae08afd5 Mon Sep 17 00:00:00 2001 From: Alessandro Martelli Date: Wed, 22 Sep 2021 11:35:59 +0200 Subject: [PATCH 05/25] [DSC-227] Reload of My Dspace results is broken --- ...stable-object-component-loader.component.spec.ts | 13 +++++++++---- .../listable-object-component-loader.component.ts | 2 +- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.spec.ts b/src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.spec.ts index 458272c606..f99ec550f1 100644 --- a/src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.spec.ts +++ b/src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.spec.ts @@ -48,6 +48,7 @@ describe('ListableObjectComponentLoaderComponent', () => { comp.viewMode = testViewMode; comp.context = testContext; spyOn(comp, 'getComponent').and.returnValue(ItemListElementComponent as any); + spyOn(comp as any, 'connectInputsAndOutputs').and.callThrough(); fixture.detectChanges(); })); @@ -56,6 +57,10 @@ describe('ListableObjectComponentLoaderComponent', () => { it('should call the getListableObjectComponent function with the right types, view mode and context', () => { expect(comp.getComponent).toHaveBeenCalledWith([testType], testViewMode, testContext); }); + + it('should connectInputsAndOutputs of loaded component', () => { + expect((comp as any).connectInputsAndOutputs).toHaveBeenCalled(); + }); }); describe('when the object is an item and viewMode is a list', () => { @@ -121,20 +126,20 @@ describe('ListableObjectComponentLoaderComponent', () => { let reloadedObject: any; beforeEach(() => { - spyOn((comp as any), 'connectInputsAndOutputs').and.returnValue(null); + spyOn((comp as any), 'instantiateComponent').and.returnValue(null); spyOn((comp as any).contentChange, 'emit').and.returnValue(null); listableComponent = fixture.debugElement.query(By.css('ds-item-list-element')).componentInstance; reloadedObject = 'object'; }); - it('should pass it on connectInputsAndOutputs', fakeAsync(() => { - expect((comp as any).connectInputsAndOutputs).not.toHaveBeenCalled(); + it('should re-instantiate the listable component', fakeAsync(() => { + expect((comp as any).instantiateComponent).not.toHaveBeenCalled(); (listableComponent as any).reloadedObject.emit(reloadedObject); tick(); - expect((comp as any).connectInputsAndOutputs).toHaveBeenCalled(); + expect((comp as any).instantiateComponent).toHaveBeenCalledWith(reloadedObject); })); it('should re-emit it as a contentChange', fakeAsync(() => { diff --git a/src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.ts b/src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.ts index 4c6206cb43..67778b0aa6 100644 --- a/src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.ts +++ b/src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.ts @@ -184,7 +184,7 @@ export class ListableObjectComponentLoaderComponent implements OnInit, OnChanges if (reloadedObject) { this.compRef.destroy(); this.object = reloadedObject; - this.connectInputsAndOutputs(); + this.instantiateComponent(reloadedObject); this.contentChange.emit(reloadedObject); } }); From 2d638a738e9951ade099e54d537a2e99e5989b79 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Wed, 22 Sep 2021 11:51:05 +0200 Subject: [PATCH 06/25] 83628: Dynamic theme fixes --- src/app/app.component.ts | 28 +- .../theme-support/theme.effects.spec.ts | 260 ------------------ src/app/shared/theme-support/theme.effects.ts | 152 +--------- src/app/shared/theme-support/theme.service.ts | 177 +++++++++++- 4 files changed, 200 insertions(+), 417 deletions(-) diff --git a/src/app/app.component.ts b/src/app/app.component.ts index 356025da9e..d3668fad5a 100644 --- a/src/app/app.component.ts +++ b/src/app/app.component.ts @@ -1,4 +1,4 @@ -import { delay, distinctUntilChanged, filter, take, withLatestFrom } from 'rxjs/operators'; +import { delay, distinctUntilChanged, filter, switchMap, take, withLatestFrom } from 'rxjs/operators'; import { AfterViewInit, ChangeDetectionStrategy, @@ -9,7 +9,14 @@ import { Optional, PLATFORM_ID, } from '@angular/core'; -import { NavigationCancel, NavigationEnd, NavigationStart, Router } from '@angular/router'; +import { + ActivatedRouteSnapshot, + NavigationCancel, + NavigationEnd, + NavigationStart, + Router, + RoutesRecognized +} from '@angular/router'; import { BehaviorSubject, Observable, of } from 'rxjs'; import { select, Store } from '@ngrx/store'; @@ -71,6 +78,7 @@ export class AppComponent implements OnInit, AfterViewInit { */ isThemeLoading$: BehaviorSubject = new BehaviorSubject(false); + isThemeCSSLoading$: BehaviorSubject = new BehaviorSubject(false); /** * Whether or not the idle modal is is currently open @@ -106,6 +114,7 @@ export class AppComponent implements OnInit, AfterViewInit { if (isPlatformBrowser(this.platformId)) { // the theme css will never download server side, so this should only happen on the browser this.isThemeLoading$.next(true); + this.isThemeCSSLoading$.next(true); } if (hasValue(themeName)) { this.setThemeCss(themeName); @@ -184,6 +193,19 @@ export class AppComponent implements OnInit, AfterViewInit { ).subscribe((event) => { if (event instanceof NavigationStart) { this.isRouteLoading$.next(true); + } else if (event instanceof RoutesRecognized) { + const activatedRouteSnapShot: ActivatedRouteSnapshot = event.state.root; + this.themeService.updateThemeOnRouteChange$(event.urlAfterRedirects, activatedRouteSnapShot).pipe( + switchMap((changed) => { + if (changed) { + return this.isThemeCSSLoading$; + } else { + return [false]; + } + }) + ).subscribe((changed) => { + this.isThemeLoading$.next(changed); + }); } else if ( event instanceof NavigationEnd || event instanceof NavigationCancel @@ -237,7 +259,7 @@ export class AppComponent implements OnInit, AfterViewInit { }); } // the fact that this callback is used, proves we're on the browser. - this.isThemeLoading$.next(false); + this.isThemeCSSLoading$.next(false); }; head.appendChild(link); } diff --git a/src/app/shared/theme-support/theme.effects.spec.ts b/src/app/shared/theme-support/theme.effects.spec.ts index 7a0e9c8f19..43727df8d6 100644 --- a/src/app/shared/theme-support/theme.effects.spec.ts +++ b/src/app/shared/theme-support/theme.effects.spec.ts @@ -1,75 +1,17 @@ import { ThemeEffects } from './theme.effects'; -import { of as observableOf } from 'rxjs'; import { TestBed } from '@angular/core/testing'; import { provideMockActions } from '@ngrx/effects/testing'; -import { LinkService } from '../../core/cache/builders/link.service'; import { cold, hot } from 'jasmine-marbles'; import { ROOT_EFFECTS_INIT } from '@ngrx/effects'; import { SetThemeAction } from './theme.actions'; -import { Theme } from '../../../config/theme.model'; import { provideMockStore } from '@ngrx/store/testing'; -import { ROUTER_NAVIGATED } from '@ngrx/router-store'; -import { ResolverActionTypes } from '../../core/resolving/resolver.actions'; -import { Community } from '../../core/shared/community.model'; -import { COMMUNITY } from '../../core/shared/community.resource-type'; -import { NoOpAction } from '../ngrx/no-op.action'; -import { ITEM } from '../../core/shared/item.resource-type'; -import { DSpaceObject } from '../../core/shared/dspace-object.model'; -import { Item } from '../../core/shared/item.model'; -import { Collection } from '../../core/shared/collection.model'; -import { COLLECTION } from '../../core/shared/collection.resource-type'; -import { - createNoContentRemoteDataObject$, - createSuccessfulRemoteDataObject$ -} from '../remote-data.utils'; import { BASE_THEME_NAME } from './theme.constants'; -/** - * LinkService able to mock recursively resolving DSO parent links - * Every time resolveLinkWithoutAttaching is called, it returns the next object in the array of ancestorDSOs until - * none are left, after which it returns a no-content remote-date - */ -class MockLinkService { - index = -1; - - constructor(private ancestorDSOs: DSpaceObject[]) { - } - - resolveLinkWithoutAttaching() { - if (this.index >= this.ancestorDSOs.length - 1) { - return createNoContentRemoteDataObject$(); - } else { - this.index++; - return createSuccessfulRemoteDataObject$(this.ancestorDSOs[this.index]); - } - } -} - describe('ThemeEffects', () => { let themeEffects: ThemeEffects; - let linkService: LinkService; let initialState; - let ancestorDSOs: DSpaceObject[]; - function init() { - ancestorDSOs = [ - Object.assign(new Collection(), { - type: COLLECTION.value, - uuid: 'collection-uuid', - _links: { owningCommunity: { href: 'owning-community-link' } } - }), - Object.assign(new Community(), { - type: COMMUNITY.value, - uuid: 'sub-community-uuid', - _links: { parentCommunity: { href: 'parent-community-link' } } - }), - Object.assign(new Community(), { - type: COMMUNITY.value, - uuid: 'top-community-uuid', - }), - ]; - linkService = new MockLinkService(ancestorDSOs) as any; initialState = { theme: { currentTheme: 'custom', @@ -82,7 +24,6 @@ describe('ThemeEffects', () => { TestBed.configureTestingModule({ providers: [ ThemeEffects, - { provide: LinkService, useValue: linkService }, provideMockStore({ initialState }), provideMockActions(() => mockActions) ] @@ -110,205 +51,4 @@ describe('ThemeEffects', () => { expect(themeEffects.initTheme$).toBeObservable(expected); }); }); - - describe('updateThemeOnRouteChange$', () => { - const url = '/test/route'; - const dso = Object.assign(new Community(), { - type: COMMUNITY.value, - uuid: '0958c910-2037-42a9-81c7-dca80e3892b4', - }); - - function spyOnPrivateMethods() { - spyOn((themeEffects as any), 'getAncestorDSOs').and.returnValue(() => observableOf([dso])); - spyOn((themeEffects as any), 'matchThemeToDSOs').and.returnValue(new Theme({ name: 'custom' })); - spyOn((themeEffects as any), 'getActionForMatch').and.returnValue(new SetThemeAction('custom')); - } - - describe('when a resolved action is present', () => { - beforeEach(() => { - setupEffectsWithActions( - hot('--ab-', { - a: { - type: ROUTER_NAVIGATED, - payload: { routerState: { url } }, - }, - b: { - type: ResolverActionTypes.RESOLVED, - payload: { url, dso }, - } - }) - ); - spyOnPrivateMethods(); - }); - - it('should set the theme it receives from the DSO', () => { - const expected = cold('--b-', { - b: new SetThemeAction('custom') - }); - - expect(themeEffects.updateThemeOnRouteChange$).toBeObservable(expected); - }); - }); - - describe('when no resolved action is present', () => { - beforeEach(() => { - setupEffectsWithActions( - hot('--a-', { - a: { - type: ROUTER_NAVIGATED, - payload: { routerState: { url } }, - }, - }) - ); - spyOnPrivateMethods(); - }); - - it('should set the theme it receives from the route url', () => { - const expected = cold('--b-', { - b: new SetThemeAction('custom') - }); - - expect(themeEffects.updateThemeOnRouteChange$).toBeObservable(expected); - }); - }); - - describe('when no themes are present', () => { - beforeEach(() => { - setupEffectsWithActions( - hot('--a-', { - a: { - type: ROUTER_NAVIGATED, - payload: { routerState: { url } }, - }, - }) - ); - (themeEffects as any).themes = []; - }); - - it('should return an empty action', () => { - const expected = cold('--b-', { - b: new NoOpAction() - }); - - expect(themeEffects.updateThemeOnRouteChange$).toBeObservable(expected); - }); - }); - }); - - describe('private functions', () => { - beforeEach(() => { - setupEffectsWithActions(hot('-', {})); - }); - - describe('getActionForMatch', () => { - it('should return a SET action if the new theme differs from the current theme', () => { - const theme = new Theme({ name: 'new-theme' }); - expect((themeEffects as any).getActionForMatch(theme, 'old-theme')).toEqual(new SetThemeAction('new-theme')); - }); - - it('should return an empty action if the new theme equals the current theme', () => { - const theme = new Theme({ name: 'old-theme' }); - expect((themeEffects as any).getActionForMatch(theme, 'old-theme')).toEqual(new NoOpAction()); - }); - }); - - describe('matchThemeToDSOs', () => { - let themes: Theme[]; - let nonMatchingTheme: Theme; - let itemMatchingTheme: Theme; - let communityMatchingTheme: Theme; - let dsos: DSpaceObject[]; - - beforeEach(() => { - nonMatchingTheme = Object.assign(new Theme({ name: 'non-matching-theme' }), { - matches: () => false - }); - itemMatchingTheme = Object.assign(new Theme({ name: 'item-matching-theme' }), { - matches: (url, dso) => (dso as any).type === ITEM.value - }); - communityMatchingTheme = Object.assign(new Theme({ name: 'community-matching-theme' }), { - matches: (url, dso) => (dso as any).type === COMMUNITY.value - }); - dsos = [ - Object.assign(new Item(), { - type: ITEM.value, - uuid: 'item-uuid', - }), - Object.assign(new Collection(), { - type: COLLECTION.value, - uuid: 'collection-uuid', - }), - Object.assign(new Community(), { - type: COMMUNITY.value, - uuid: 'community-uuid', - }), - ]; - }); - - describe('when no themes match any of the DSOs', () => { - beforeEach(() => { - themes = [ nonMatchingTheme ]; - themeEffects.themes = themes; - }); - - it('should return undefined', () => { - expect((themeEffects as any).matchThemeToDSOs(dsos, '')).toBeUndefined(); - }); - }); - - describe('when one of the themes match a DSOs', () => { - beforeEach(() => { - themes = [ nonMatchingTheme, itemMatchingTheme ]; - themeEffects.themes = themes; - }); - - it('should return the matching theme', () => { - expect((themeEffects as any).matchThemeToDSOs(dsos, '')).toEqual(itemMatchingTheme); - }); - }); - - describe('when multiple themes match some of the DSOs', () => { - it('should return the first matching theme', () => { - themes = [ nonMatchingTheme, itemMatchingTheme, communityMatchingTheme ]; - themeEffects.themes = themes; - expect((themeEffects as any).matchThemeToDSOs(dsos, '')).toEqual(itemMatchingTheme); - - themes = [ nonMatchingTheme, communityMatchingTheme, itemMatchingTheme ]; - themeEffects.themes = themes; - expect((themeEffects as any).matchThemeToDSOs(dsos, '')).toEqual(communityMatchingTheme); - }); - }); - }); - - describe('getAncestorDSOs', () => { - it('should return an array of the provided DSO and its ancestors', (done) => { - const dso = Object.assign(new Item(), { - type: ITEM.value, - uuid: 'item-uuid', - _links: { owningCollection: { href: 'owning-collection-link' } }, - }); - - observableOf(dso).pipe( - (themeEffects as any).getAncestorDSOs() - ).subscribe((result) => { - expect(result).toEqual([dso, ...ancestorDSOs]); - done(); - }); - }); - - it('should return an array of just the provided DSO if it doesn\'t have any parents', (done) => { - const dso = { - type: ITEM.value, - uuid: 'item-uuid', - }; - - observableOf(dso).pipe( - (themeEffects as any).getAncestorDSOs() - ).subscribe((result) => { - expect(result).toEqual([dso]); - done(); - }); - }); - }); - }); }); diff --git a/src/app/shared/theme-support/theme.effects.ts b/src/app/shared/theme-support/theme.effects.ts index 894cfeca75..e120257728 100644 --- a/src/app/shared/theme-support/theme.effects.ts +++ b/src/app/shared/theme-support/theme.effects.ts @@ -1,22 +1,9 @@ import { Injectable } from '@angular/core'; import { createEffect, Actions, ofType, ROOT_EFFECTS_INIT } from '@ngrx/effects'; -import { ROUTER_NAVIGATED, RouterNavigatedAction } from '@ngrx/router-store'; -import { map, withLatestFrom, expand, switchMap, toArray, startWith, filter } from 'rxjs/operators'; +import { map } from 'rxjs/operators'; import { SetThemeAction } from './theme.actions'; import { environment } from '../../../environments/environment'; -import { ThemeConfig, themeFactory, Theme, } from '../../../config/theme.model'; -import { hasValue, isNotEmpty, hasNoValue } from '../empty.util'; -import { NoOpAction } from '../ngrx/no-op.action'; -import { Store, select } from '@ngrx/store'; -import { ThemeState } from './theme.reducer'; -import { currentThemeSelector } from './theme.service'; -import { of as observableOf, EMPTY, Observable } from 'rxjs'; -import { ResolverActionTypes, ResolvedAction } from '../../core/resolving/resolver.actions'; -import { followLink } from '../utils/follow-link-config.model'; -import { RemoteData } from '../../core/data/remote-data'; -import { DSpaceObject } from '../../core/shared/dspace-object.model'; -import { getFirstCompletedRemoteData } from '../../core/shared/operators'; -import { LinkService } from '../../core/cache/builders/link.service'; +import { hasValue, hasNoValue } from '../empty.util'; import { BASE_THEME_NAME } from './theme.constants'; export const DEFAULT_THEME_CONFIG = environment.themes.find((themeConfig: any) => @@ -27,16 +14,6 @@ export const DEFAULT_THEME_CONFIG = environment.themes.find((themeConfig: any) = @Injectable() export class ThemeEffects { - /** - * The list of configured themes - */ - themes: Theme[]; - - /** - * True if at least one theme depends on the route - */ - hasDynamicTheme: boolean; - /** * Initialize with a theme that doesn't depend on the route. */ @@ -53,133 +30,8 @@ export class ThemeEffects { ) ); - /** - * An effect that fires when a route change completes, - * and determines whether or not the theme should change - */ - updateThemeOnRouteChange$ = createEffect(() => this.actions$.pipe( - // Listen for when a route change ends - ofType(ROUTER_NAVIGATED), - withLatestFrom( - // Pull in the latest resolved action, or undefined if none was dispatched yet - this.actions$.pipe(ofType(ResolverActionTypes.RESOLVED), startWith(undefined)), - // and the current theme from the store - this.store.pipe(select(currentThemeSelector)) - ), - switchMap(([navigatedAction, resolvedAction, currentTheme]: [RouterNavigatedAction, ResolvedAction, string]) => { - if (this.hasDynamicTheme === true && isNotEmpty(this.themes)) { - const currentRouteUrl = navigatedAction.payload.routerState.url; - // If resolvedAction exists, and deals with the current url - if (hasValue(resolvedAction) && resolvedAction.payload.url === currentRouteUrl) { - // Start with the resolved dso and go recursively through its parents until you reach the top-level community - return observableOf(resolvedAction.payload.dso).pipe( - this.getAncestorDSOs(), - map((dsos: DSpaceObject[]) => { - const dsoMatch = this.matchThemeToDSOs(dsos, currentRouteUrl); - return this.getActionForMatch(dsoMatch, currentTheme); - }) - ); - } - - // check whether the route itself matches - const routeMatch = this.themes.find((theme: Theme) => theme.matches(currentRouteUrl, undefined)); - - return [this.getActionForMatch(routeMatch, currentTheme)]; - } - - // If there are no themes configured, do nothing - return [new NoOpAction()]; - }) - ) - ); - - /** - * return the action to dispatch based on the given matching theme - * - * @param newTheme The theme to create an action for - * @param currentThemeName The name of the currently active theme - * @private - */ - private getActionForMatch(newTheme: Theme, currentThemeName: string): SetThemeAction | NoOpAction { - if (hasValue(newTheme) && newTheme.config.name !== currentThemeName) { - // If we have a match, and it isn't already the active theme, set it as the new theme - return new SetThemeAction(newTheme.config.name); - } else { - // Otherwise, do nothing - return new NoOpAction(); - } - } - - /** - * Check the given DSpaceObjects in order to see if they match the configured themes in order. - * If a match is found, the matching theme is returned - * - * @param dsos The DSpaceObjects to check - * @param currentRouteUrl The url for the current route - * @private - */ - private matchThemeToDSOs(dsos: DSpaceObject[], currentRouteUrl: string): Theme { - // iterate over the themes in order, and return the first one that matches - return this.themes.find((theme: Theme) => { - // iterate over the dsos's in order (most specific one first, so Item, Collection, - // Community), and return the first one that matches the current theme - const match = dsos.find((dso: DSpaceObject) => theme.matches(currentRouteUrl, dso)); - return hasValue(match); - }); - - } - - /** - * An rxjs operator that will return an array of all the ancestors of the DSpaceObject used as - * input. The initial DSpaceObject will be the first element of the output array, followed by - * its parent, its grandparent etc - * - * @private - */ - private getAncestorDSOs() { - return (source: Observable): Observable => - source.pipe( - expand((dso: DSpaceObject) => { - // Check if the dso exists and has a parent link - if (hasValue(dso) && typeof (dso as any).getParentLinkKey === 'function') { - const linkName = (dso as any).getParentLinkKey(); - // If it does, retrieve it. - return this.linkService.resolveLinkWithoutAttaching(dso, followLink(linkName)).pipe( - getFirstCompletedRemoteData(), - map((rd: RemoteData) => { - if (hasValue(rd.payload)) { - // If there's a parent, use it for the next iteration - return rd.payload; - } else { - // If there's no parent, or an error, return null, which will stop recursion - // in the next iteration - return null; - } - }), - ); - } - - // The current dso has no value, or no parent. Return EMPTY to stop recursion - return EMPTY; - }), - // only allow through DSOs that have a value - filter((dso: DSpaceObject) => hasValue(dso)), - // Wait for recursion to complete, and emit all results at once, in an array - toArray() - ); - } - constructor( private actions$: Actions, - private store: Store, - private linkService: LinkService, ) { - // Create objects from the theme configs in the environment file - this.themes = environment.themes.map((themeConfig: ThemeConfig) => themeFactory(themeConfig)); - this.hasDynamicTheme = environment.themes.some((themeConfig: any) => - hasValue(themeConfig.regex) || - hasValue(themeConfig.handle) || - hasValue(themeConfig.uuid) - ); } } diff --git a/src/app/shared/theme-support/theme.service.ts b/src/app/shared/theme-support/theme.service.ts index 7b0af93e04..f351f320d7 100644 --- a/src/app/shared/theme-support/theme.service.ts +++ b/src/app/shared/theme-support/theme.service.ts @@ -1,10 +1,27 @@ import { Injectable } from '@angular/core'; -import { Store, createFeatureSelector, createSelector, select } from '@ngrx/store'; +import { Store, createFeatureSelector, createSelector, select, Action } from '@ngrx/store'; import { Observable } from 'rxjs/internal/Observable'; import { ThemeState } from './theme.reducer'; -import { SetThemeAction } from './theme.actions'; -import { take } from 'rxjs/operators'; -import { hasValue } from '../empty.util'; +import { SetThemeAction, ThemeActionTypes } from './theme.actions'; +import { expand, filter, map, startWith, switchMap, take, toArray } from 'rxjs/operators'; +import { hasValue, isNotEmpty } from '../empty.util'; +import { Actions, ofType } from '@ngrx/effects'; +import { ResolvedAction, ResolverActionTypes } from '../../core/resolving/resolver.actions'; +import { RemoteData } from '../../core/data/remote-data'; +import { DSpaceObject } from '../../core/shared/dspace-object.model'; +import { + getFirstCompletedRemoteData, + getFirstSucceededRemoteData, + getRemoteDataPayload +} from '../../core/shared/operators'; +import { combineLatest as observableCombineLatest, EMPTY, of as observableOf } from 'rxjs'; +import { Theme, ThemeConfig, themeFactory } from '../../../config/theme.model'; +import { NO_OP_ACTION_TYPE, NoOpAction } from '../ngrx/no-op.action'; +import { followLink } from '../utils/follow-link-config.model'; +import { LinkService } from '../../core/cache/builders/link.service'; +import { environment } from '../../../environments/environment'; +import { DSpaceObjectDataService } from '../../core/data/dspace-object-data.service'; +import { ActivatedRouteSnapshot } from '@angular/router'; export const themeStateSelector = createFeatureSelector('theme'); @@ -17,9 +34,29 @@ export const currentThemeSelector = createSelector( providedIn: 'root' }) export class ThemeService { + /** + * The list of configured themes + */ + themes: Theme[]; + + /** + * True if at least one theme depends on the route + */ + hasDynamicTheme: boolean; + constructor( private store: Store, + private actions$: Actions, + private linkService: LinkService, + private dSpaceObjectDataService: DSpaceObjectDataService, ) { + // Create objects from the theme configs in the environment file + this.themes = environment.themes.map((themeConfig: ThemeConfig) => themeFactory(themeConfig)); + this.hasDynamicTheme = environment.themes.some((themeConfig: any) => + hasValue(themeConfig.regex) || + hasValue(themeConfig.handle) || + hasValue(themeConfig.uuid) + ); } setTheme(newName: string) { @@ -43,4 +80,136 @@ export class ThemeService { ); } + updateThemeOnRouteChange$(currentRouteUrl: string, activatedRouteSnapshot: ActivatedRouteSnapshot): Observable { + // and the current theme from the store + const currentTheme$: Observable = this.store.pipe(select(currentThemeSelector)); + + const action$ = currentTheme$.pipe( + switchMap((currentTheme: string) => { + if (this.hasDynamicTheme === true && isNotEmpty(this.themes)) { + if (hasValue(activatedRouteSnapshot) && hasValue(activatedRouteSnapshot.data) && hasValue(activatedRouteSnapshot.data.dso)) { + const dsoRD: RemoteData = activatedRouteSnapshot.data.dso; + if (dsoRD.hasSucceeded) { + // Start with the resolved dso and go recursively through its parents until you reach the top-level community + return observableOf(dsoRD.payload).pipe( + this.getAncestorDSOs(), + map((dsos: DSpaceObject[]) => { + const dsoMatch = this.matchThemeToDSOs(dsos, currentRouteUrl); + return this.getActionForMatch(dsoMatch, currentTheme); + }) + ); + } + } + if (hasValue(activatedRouteSnapshot.queryParams) && hasValue(activatedRouteSnapshot.queryParams.scope)) { + const dsoFromScope$: Observable> = this.dSpaceObjectDataService.findById(activatedRouteSnapshot.queryParams.scope); + // Start with the resolved dso and go recursively through its parents until you reach the top-level community + return dsoFromScope$.pipe( + getFirstSucceededRemoteData(), + getRemoteDataPayload(), + this.getAncestorDSOs(), + map((dsos: DSpaceObject[]) => { + const dsoMatch = this.matchThemeToDSOs(dsos, currentRouteUrl); + return this.getActionForMatch(dsoMatch, currentTheme); + }) + ); + } + + // check whether the route itself matches + const routeMatch = this.themes.find((theme: Theme) => theme.matches(currentRouteUrl, undefined)); + + return [this.getActionForMatch(routeMatch, currentTheme)]; + } + + // If there are no themes configured, do nothing + return [new NoOpAction()]; + }), + ); + + action$.pipe( + take(1), + filter((action) => action.type !== NO_OP_ACTION_TYPE), + ).subscribe((action) => { + this.store.dispatch(action); + }); + + return action$.pipe( + map((action) => action.type === ThemeActionTypes.SET), + ); + } + + /** + * An rxjs operator that will return an array of all the ancestors of the DSpaceObject used as + * input. The initial DSpaceObject will be the first element of the output array, followed by + * its parent, its grandparent etc + * + * @private + */ + private getAncestorDSOs() { + return (source: Observable): Observable => + source.pipe( + expand((dso: DSpaceObject) => { + // Check if the dso exists and has a parent link + if (hasValue(dso) && typeof (dso as any).getParentLinkKey === 'function') { + const linkName = (dso as any).getParentLinkKey(); + // If it does, retrieve it. + return this.linkService.resolveLinkWithoutAttaching(dso, followLink(linkName)).pipe( + getFirstCompletedRemoteData(), + map((rd: RemoteData) => { + if (hasValue(rd.payload)) { + // If there's a parent, use it for the next iteration + return rd.payload; + } else { + // If there's no parent, or an error, return null, which will stop recursion + // in the next iteration + return null; + } + }), + ); + } + + // The current dso has no value, or no parent. Return EMPTY to stop recursion + return EMPTY; + }), + // only allow through DSOs that have a value + filter((dso: DSpaceObject) => hasValue(dso)), + // Wait for recursion to complete, and emit all results at once, in an array + toArray() + ); + } + + /** + * return the action to dispatch based on the given matching theme + * + * @param newTheme The theme to create an action for + * @param currentThemeName The name of the currently active theme + * @private + */ + private getActionForMatch(newTheme: Theme, currentThemeName: string): SetThemeAction | NoOpAction { + if (hasValue(newTheme) && newTheme.config.name !== currentThemeName) { + // If we have a match, and it isn't already the active theme, set it as the new theme + return new SetThemeAction(newTheme.config.name); + } else { + // Otherwise, do nothing + return new NoOpAction(); + } + } + + /** + * Check the given DSpaceObjects in order to see if they match the configured themes in order. + * If a match is found, the matching theme is returned + * + * @param dsos The DSpaceObjects to check + * @param currentRouteUrl The url for the current route + * @private + */ + private matchThemeToDSOs(dsos: DSpaceObject[], currentRouteUrl: string): Theme { + // iterate over the themes in order, and return the first one that matches + return this.themes.find((theme: Theme) => { + // iterate over the dsos's in order (most specific one first, so Item, Collection, + // Community), and return the first one that matches the current theme + const match = dsos.find((dso: DSpaceObject) => theme.matches(currentRouteUrl, dso)); + return hasValue(match); + }); + } + } From d9db65685b4df4b14e0330d758f9761f4624a7b9 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Wed, 22 Sep 2021 17:46:55 +0200 Subject: [PATCH 07/25] 83628: Dynamic theme fixes --- src/app/app.component.ts | 21 ++++++++------- src/app/shared/theme-support/theme.service.ts | 27 +++++++++++++++---- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/app/app.component.ts b/src/app/app.component.ts index d3668fad5a..6f06a84144 100644 --- a/src/app/app.component.ts +++ b/src/app/app.component.ts @@ -1,4 +1,4 @@ -import { delay, distinctUntilChanged, filter, switchMap, take, withLatestFrom } from 'rxjs/operators'; +import { distinctUntilChanged, filter, switchMap, take, withLatestFrom } from 'rxjs/operators'; import { AfterViewInit, ChangeDetectionStrategy, @@ -13,9 +13,8 @@ import { ActivatedRouteSnapshot, NavigationCancel, NavigationEnd, - NavigationStart, + NavigationStart, ResolveEnd, Router, - RoutesRecognized } from '@angular/router'; import { BehaviorSubject, Observable, of } from 'rxjs'; @@ -113,7 +112,6 @@ export class AppComponent implements OnInit, AfterViewInit { this.themeService.getThemeName$().subscribe((themeName: string) => { if (isPlatformBrowser(this.platformId)) { // the theme css will never download server side, so this should only happen on the browser - this.isThemeLoading$.next(true); this.isThemeCSSLoading$.next(true); } if (hasValue(themeName)) { @@ -186,14 +184,14 @@ export class AppComponent implements OnInit, AfterViewInit { } ngAfterViewInit() { - this.router.events.pipe( - // This fixes an ExpressionChangedAfterItHasBeenCheckedError from being thrown while loading the component - // More information on this bug-fix: https://blog.angular-university.io/angular-debugging/ - delay(0) - ).subscribe((event) => { + let resolveEndFound = false; + this.router.events.subscribe((event) => { if (event instanceof NavigationStart) { + resolveEndFound = false; this.isRouteLoading$.next(true); - } else if (event instanceof RoutesRecognized) { + this.isThemeLoading$.next(true); + } else if (event instanceof ResolveEnd) { + resolveEndFound = true; const activatedRouteSnapShot: ActivatedRouteSnapshot = event.state.root; this.themeService.updateThemeOnRouteChange$(event.urlAfterRedirects, activatedRouteSnapShot).pipe( switchMap((changed) => { @@ -210,6 +208,9 @@ export class AppComponent implements OnInit, AfterViewInit { event instanceof NavigationEnd || event instanceof NavigationCancel ) { + if (!resolveEndFound) { + this.isThemeLoading$.next(false); + } this.isRouteLoading$.next(false); } }); diff --git a/src/app/shared/theme-support/theme.service.ts b/src/app/shared/theme-support/theme.service.ts index f351f320d7..77be1691bf 100644 --- a/src/app/shared/theme-support/theme.service.ts +++ b/src/app/shared/theme-support/theme.service.ts @@ -3,9 +3,9 @@ import { Store, createFeatureSelector, createSelector, select, Action } from '@n import { Observable } from 'rxjs/internal/Observable'; import { ThemeState } from './theme.reducer'; import { SetThemeAction, ThemeActionTypes } from './theme.actions'; -import { expand, filter, map, startWith, switchMap, take, toArray } from 'rxjs/operators'; +import { expand, filter, map, startWith, switchMap, take, tap, toArray } from 'rxjs/operators'; import { hasValue, isNotEmpty } from '../empty.util'; -import { Actions, ofType } from '@ngrx/effects'; +import { act, Actions, ofType } from '@ngrx/effects'; import { ResolvedAction, ResolverActionTypes } from '../../core/resolving/resolver.actions'; import { RemoteData } from '../../core/data/remote-data'; import { DSpaceObject } from '../../core/shared/dspace-object.model'; @@ -86,9 +86,10 @@ export class ThemeService { const action$ = currentTheme$.pipe( switchMap((currentTheme: string) => { + const snapshotWithData = this.findRouteData(activatedRouteSnapshot); if (this.hasDynamicTheme === true && isNotEmpty(this.themes)) { - if (hasValue(activatedRouteSnapshot) && hasValue(activatedRouteSnapshot.data) && hasValue(activatedRouteSnapshot.data.dso)) { - const dsoRD: RemoteData = activatedRouteSnapshot.data.dso; + if (hasValue(snapshotWithData) && hasValue(snapshotWithData.data) && hasValue(snapshotWithData.data.dso)) { + const dsoRD: RemoteData = snapshotWithData.data.dso; if (dsoRD.hasSucceeded) { // Start with the resolved dso and go recursively through its parents until you reach the top-level community return observableOf(dsoRD.payload).pipe( @@ -123,10 +124,10 @@ export class ThemeService { // If there are no themes configured, do nothing return [new NoOpAction()]; }), + take(1), ); action$.pipe( - take(1), filter((action) => action.type !== NO_OP_ACTION_TYPE), ).subscribe((action) => { this.store.dispatch(action); @@ -137,6 +138,22 @@ export class ThemeService { ); } + findRouteData(...routes: ActivatedRouteSnapshot[]) { + const result = routes.find((route) => hasValue(route.data.dso)); + if (hasValue(result)) { + return result; + } else { + const nextLevelRoutes = routes + .map((route: ActivatedRouteSnapshot) => route.children) + .reduce((combined: ActivatedRouteSnapshot[], current: ActivatedRouteSnapshot[]) => [...combined, ...current]); + if (isNotEmpty(nextLevelRoutes)) { + return this.findRouteData(...nextLevelRoutes); + } else { + return undefined; + } + } + } + /** * An rxjs operator that will return an array of all the ancestors of the DSpaceObject used as * input. The initial DSpaceObject will be the first element of the output array, followed by From e2c2423e53391773cc165c956118bb0b15bc54f4 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 23 Sep 2021 11:27:38 +0200 Subject: [PATCH 08/25] 83628: Dynamic theme fixes - test cases and JSDocs --- .../browse-by/browse-by.component.spec.ts | 9 +- ...-object-component-loader.component.spec.ts | 11 +- .../theme-support/theme.service.spec.ts | 370 ++++++++++++++++++ src/app/shared/theme-support/theme.service.ts | 17 +- 4 files changed, 404 insertions(+), 3 deletions(-) create mode 100644 src/app/shared/theme-support/theme.service.spec.ts diff --git a/src/app/shared/browse-by/browse-by.component.spec.ts b/src/app/shared/browse-by/browse-by.component.spec.ts index 806f4bdb6f..915a86e87a 100644 --- a/src/app/shared/browse-by/browse-by.component.spec.ts +++ b/src/app/shared/browse-by/browse-by.component.spec.ts @@ -21,11 +21,14 @@ import { storeModuleConfig } from '../../app.reducer'; import { FindListOptions } from '../../core/data/request.models'; import { PaginationService } from '../../core/pagination/pagination.service'; import { PaginationServiceStub } from '../testing/pagination-service.stub'; +import { ThemeService } from '../theme-support/theme.service'; describe('BrowseByComponent', () => { let comp: BrowseByComponent; let fixture: ComponentFixture; + let themeService: ThemeService; + const mockItems = [ Object.assign(new Item(), { id: 'fakeId-1', @@ -57,6 +60,9 @@ describe('BrowseByComponent', () => { const paginationService = new PaginationServiceStub(paginationConfig); beforeEach(waitForAsync(() => { + themeService = jasmine.createSpyObj('themeService', { + getThemeName: 'dspace', + }); TestBed.configureTestingModule({ imports: [ CommonModule, @@ -75,7 +81,8 @@ describe('BrowseByComponent', () => { ], declarations: [], providers: [ - {provide: PaginationService, useValue: paginationService} + {provide: PaginationService, useValue: paginationService}, + { provide: ThemeService, useValue: themeService }, ], schemas: [NO_ERRORS_SCHEMA] }).compileComponents(); diff --git a/src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.spec.ts b/src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.spec.ts index 458272c606..4e3ca42c45 100644 --- a/src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.spec.ts +++ b/src/app/shared/object-collection/shared/listable-object/listable-object-component-loader.component.spec.ts @@ -11,6 +11,7 @@ import { TranslateModule } from '@ngx-translate/core'; import { By } from '@angular/platform-browser'; import { Item } from '../../../../core/shared/item.model'; import { provideMockStore } from '@ngrx/store/testing'; +import { ThemeService } from '../../../theme-support/theme.service'; const testType = 'TestType'; const testContext = Context.Search; @@ -26,12 +27,20 @@ describe('ListableObjectComponentLoaderComponent', () => { let comp: ListableObjectComponentLoaderComponent; let fixture: ComponentFixture; + let themeService: ThemeService; + beforeEach(waitForAsync(() => { + themeService = jasmine.createSpyObj('themeService', { + getThemeName: 'dspace', + }); TestBed.configureTestingModule({ imports: [TranslateModule.forRoot()], declarations: [ListableObjectComponentLoaderComponent, ItemListElementComponent, ListableObjectDirective], schemas: [NO_ERRORS_SCHEMA], - providers: [provideMockStore({})] + providers: [ + provideMockStore({}), + { provide: ThemeService, useValue: themeService }, + ] }).overrideComponent(ListableObjectComponentLoaderComponent, { set: { changeDetection: ChangeDetectionStrategy.Default, diff --git a/src/app/shared/theme-support/theme.service.spec.ts b/src/app/shared/theme-support/theme.service.spec.ts new file mode 100644 index 0000000000..84043369c0 --- /dev/null +++ b/src/app/shared/theme-support/theme.service.spec.ts @@ -0,0 +1,370 @@ +import { of as observableOf } from 'rxjs'; +import { TestBed } from '@angular/core/testing'; +import { provideMockActions } from '@ngrx/effects/testing'; +import { LinkService } from '../../core/cache/builders/link.service'; +import { cold, hot } from 'jasmine-marbles'; +import { SetThemeAction } from './theme.actions'; +import { Theme } from '../../../config/theme.model'; +import { provideMockStore } from '@ngrx/store/testing'; +import { Community } from '../../core/shared/community.model'; +import { COMMUNITY } from '../../core/shared/community.resource-type'; +import { NoOpAction } from '../ngrx/no-op.action'; +import { ITEM } from '../../core/shared/item.resource-type'; +import { DSpaceObject } from '../../core/shared/dspace-object.model'; +import { Item } from '../../core/shared/item.model'; +import { Collection } from '../../core/shared/collection.model'; +import { COLLECTION } from '../../core/shared/collection.resource-type'; +import { + createNoContentRemoteDataObject$, createSuccessfulRemoteDataObject, + createSuccessfulRemoteDataObject$ +} from '../remote-data.utils'; +import { DSpaceObjectDataService } from '../../core/data/dspace-object-data.service'; +import { ThemeService } from './theme.service'; +import { ROUTER_NAVIGATED } from '@ngrx/router-store'; +import { ActivatedRouteSnapshot } from '@angular/router'; + +/** + * LinkService able to mock recursively resolving DSO parent links + * Every time resolveLinkWithoutAttaching is called, it returns the next object in the array of ancestorDSOs until + * none are left, after which it returns a no-content remote-date + */ +class MockLinkService { + index = -1; + + constructor(private ancestorDSOs: DSpaceObject[]) { + } + + resolveLinkWithoutAttaching() { + if (this.index >= this.ancestorDSOs.length - 1) { + return createNoContentRemoteDataObject$(); + } else { + this.index++; + return createSuccessfulRemoteDataObject$(this.ancestorDSOs[this.index]); + } + } +} + +describe('ThemeService', () => { + let themeService: ThemeService; + let linkService: LinkService; + let initialState; + + let ancestorDSOs: DSpaceObject[]; + + const mockCommunity = Object.assign(new Community(), { + type: COMMUNITY.value, + uuid: 'top-community-uuid', + }); + + function init() { + ancestorDSOs = [ + Object.assign(new Collection(), { + type: COLLECTION.value, + uuid: 'collection-uuid', + _links: { owningCommunity: { href: 'owning-community-link' } } + }), + Object.assign(new Community(), { + type: COMMUNITY.value, + uuid: 'sub-community-uuid', + _links: { parentCommunity: { href: 'parent-community-link' } } + }), + mockCommunity, + ]; + linkService = new MockLinkService(ancestorDSOs) as any; + initialState = { + theme: { + currentTheme: 'custom', + }, + }; + } + + function setupServiceWithActions(mockActions) { + init(); + const mockDsoService = { + findById: () => createSuccessfulRemoteDataObject$(mockCommunity) + }; + TestBed.configureTestingModule({ + providers: [ + ThemeService, + { provide: LinkService, useValue: linkService }, + provideMockStore({ initialState }), + provideMockActions(() => mockActions), + { provide: DSpaceObjectDataService, useValue: mockDsoService } + ] + }); + + themeService = TestBed.inject(ThemeService); + spyOn((themeService as any).store, 'dispatch').and.stub(); + } + + describe('updateThemeOnRouteChange$', () => { + const url = '/test/route'; + const dso = Object.assign(new Community(), { + type: COMMUNITY.value, + uuid: '0958c910-2037-42a9-81c7-dca80e3892b4', + }); + + function spyOnPrivateMethods() { + spyOn((themeService as any), 'getAncestorDSOs').and.returnValue(() => observableOf([dso])); + spyOn((themeService as any), 'matchThemeToDSOs').and.returnValue(new Theme({ name: 'custom' })); + spyOn((themeService as any), 'getActionForMatch').and.returnValue(new SetThemeAction('custom')); + } + + describe('when no resolved action is present', () => { + beforeEach(() => { + setupServiceWithActions( + hot('--a-', { + a: { + type: ROUTER_NAVIGATED, + payload: { routerState: { url } }, + }, + }) + ); + spyOnPrivateMethods(); + }); + + it('should set the theme it receives from the route url', (done) => { + themeService.updateThemeOnRouteChange$(url, {} as ActivatedRouteSnapshot).subscribe(() => { + expect((themeService as any).store.dispatch).toHaveBeenCalledWith(new SetThemeAction('custom') as any); + done(); + }); + }); + + it('should return true', (done) => { + themeService.updateThemeOnRouteChange$(url, {} as ActivatedRouteSnapshot).subscribe((result) => { + expect(result).toEqual(true); + done(); + }); + }); + }); + + describe('when no themes are present', () => { + beforeEach(() => { + setupServiceWithActions( + hot('--a-', { + a: { + type: ROUTER_NAVIGATED, + payload: { routerState: { url } }, + }, + }) + ); + (themeService as any).themes = []; + }); + + it('should not dispatch any action', (done) => { + themeService.updateThemeOnRouteChange$(url, {} as ActivatedRouteSnapshot).subscribe(() => { + expect((themeService as any).store.dispatch).not.toHaveBeenCalled(); + done(); + }); + }); + + it('should return false', (done) => { + themeService.updateThemeOnRouteChange$(url, {} as ActivatedRouteSnapshot).subscribe((result) => { + expect(result).toEqual(false); + done(); + }); + }); + }); + + describe('when a dso is present in the snapshot\'s data', () => { + let snapshot; + + beforeEach(() => { + setupServiceWithActions( + hot('--a-', { + a: { + type: ROUTER_NAVIGATED, + payload: { routerState: { url } }, + }, + }) + ); + spyOnPrivateMethods(); + snapshot = Object.assign({ + data: { + dso: createSuccessfulRemoteDataObject(dso) + } + }); + }); + + it('should match the theme to the dso', (done) => { + themeService.updateThemeOnRouteChange$(url, snapshot).subscribe(() => { + expect((themeService as any).matchThemeToDSOs).toHaveBeenCalled(); + done(); + }); + }); + + it('should set the theme it receives from the data dso', (done) => { + themeService.updateThemeOnRouteChange$(url, snapshot).subscribe(() => { + expect((themeService as any).store.dispatch).toHaveBeenCalledWith(new SetThemeAction('custom') as any); + done(); + }); + }); + + it('should return true', (done) => { + themeService.updateThemeOnRouteChange$(url, snapshot).subscribe((result) => { + expect(result).toEqual(true); + done(); + }); + }); + }); + + describe('when a scope is present in the snapshot\'s parameters', () => { + let snapshot; + + beforeEach(() => { + setupServiceWithActions( + hot('--a-', { + a: { + type: ROUTER_NAVIGATED, + payload: { routerState: { url } }, + }, + }) + ); + spyOnPrivateMethods(); + snapshot = Object.assign({ + queryParams: { + scope: mockCommunity.uuid + } + }); + }); + + it('should match the theme to the dso found through the scope', (done) => { + themeService.updateThemeOnRouteChange$(url, snapshot).subscribe(() => { + expect((themeService as any).matchThemeToDSOs).toHaveBeenCalled(); + done(); + }); + }); + + it('should set the theme it receives from the dso found through the scope', (done) => { + themeService.updateThemeOnRouteChange$(url, snapshot).subscribe(() => { + expect((themeService as any).store.dispatch).toHaveBeenCalledWith(new SetThemeAction('custom') as any); + done(); + }); + }); + + it('should return true', (done) => { + themeService.updateThemeOnRouteChange$(url, snapshot).subscribe((result) => { + expect(result).toEqual(true); + done(); + }); + }); + }); + }); + + describe('private functions', () => { + beforeEach(() => { + setupServiceWithActions(hot('-', {})); + }); + + describe('getActionForMatch', () => { + it('should return a SET action if the new theme differs from the current theme', () => { + const theme = new Theme({ name: 'new-theme' }); + expect((themeService as any).getActionForMatch(theme, 'old-theme')).toEqual(new SetThemeAction('new-theme')); + }); + + it('should return an empty action if the new theme equals the current theme', () => { + const theme = new Theme({ name: 'old-theme' }); + expect((themeService as any).getActionForMatch(theme, 'old-theme')).toEqual(new NoOpAction()); + }); + }); + + describe('matchThemeToDSOs', () => { + let themes: Theme[]; + let nonMatchingTheme: Theme; + let itemMatchingTheme: Theme; + let communityMatchingTheme: Theme; + let dsos: DSpaceObject[]; + + beforeEach(() => { + nonMatchingTheme = Object.assign(new Theme({ name: 'non-matching-theme' }), { + matches: () => false + }); + itemMatchingTheme = Object.assign(new Theme({ name: 'item-matching-theme' }), { + matches: (url, dso) => (dso as any).type === ITEM.value + }); + communityMatchingTheme = Object.assign(new Theme({ name: 'community-matching-theme' }), { + matches: (url, dso) => (dso as any).type === COMMUNITY.value + }); + dsos = [ + Object.assign(new Item(), { + type: ITEM.value, + uuid: 'item-uuid', + }), + Object.assign(new Collection(), { + type: COLLECTION.value, + uuid: 'collection-uuid', + }), + Object.assign(new Community(), { + type: COMMUNITY.value, + uuid: 'community-uuid', + }), + ]; + }); + + describe('when no themes match any of the DSOs', () => { + beforeEach(() => { + themes = [ nonMatchingTheme ]; + themeService.themes = themes; + }); + + it('should return undefined', () => { + expect((themeService as any).matchThemeToDSOs(dsos, '')).toBeUndefined(); + }); + }); + + describe('when one of the themes match a DSOs', () => { + beforeEach(() => { + themes = [ nonMatchingTheme, itemMatchingTheme ]; + themeService.themes = themes; + }); + + it('should return the matching theme', () => { + expect((themeService as any).matchThemeToDSOs(dsos, '')).toEqual(itemMatchingTheme); + }); + }); + + describe('when multiple themes match some of the DSOs', () => { + it('should return the first matching theme', () => { + themes = [ nonMatchingTheme, itemMatchingTheme, communityMatchingTheme ]; + themeService.themes = themes; + expect((themeService as any).matchThemeToDSOs(dsos, '')).toEqual(itemMatchingTheme); + + themes = [ nonMatchingTheme, communityMatchingTheme, itemMatchingTheme ]; + themeService.themes = themes; + expect((themeService as any).matchThemeToDSOs(dsos, '')).toEqual(communityMatchingTheme); + }); + }); + }); + + describe('getAncestorDSOs', () => { + it('should return an array of the provided DSO and its ancestors', (done) => { + const dso = Object.assign(new Item(), { + type: ITEM.value, + uuid: 'item-uuid', + _links: { owningCollection: { href: 'owning-collection-link' } }, + }); + + observableOf(dso).pipe( + (themeService as any).getAncestorDSOs() + ).subscribe((result) => { + expect(result).toEqual([dso, ...ancestorDSOs]); + done(); + }); + }); + + it('should return an array of just the provided DSO if it doesn\'t have any parents', (done) => { + const dso = { + type: ITEM.value, + uuid: 'item-uuid', + }; + + observableOf(dso).pipe( + (themeService as any).getAncestorDSOs() + ).subscribe((result) => { + expect(result).toEqual([dso]); + done(); + }); + }); + }); + }); +}); diff --git a/src/app/shared/theme-support/theme.service.ts b/src/app/shared/theme-support/theme.service.ts index 77be1691bf..c41b9d9fa7 100644 --- a/src/app/shared/theme-support/theme.service.ts +++ b/src/app/shared/theme-support/theme.service.ts @@ -80,6 +80,16 @@ export class ThemeService { ); } + /** + * Determine whether or not the theme needs to change depending on the current route's URL and snapshot data + * If the snapshot contains a dso, this will be used to match a theme + * If the snapshot contains a scope parameters, this will be used to match a theme + * Otherwise the URL is matched against + * If none of the above find a match, the theme doesn't change + * @param currentRouteUrl + * @param activatedRouteSnapshot + * @return Observable boolean emitting whether or not the theme has been changed + */ updateThemeOnRouteChange$(currentRouteUrl: string, activatedRouteSnapshot: ActivatedRouteSnapshot): Observable { // and the current theme from the store const currentTheme$: Observable = this.store.pipe(select(currentThemeSelector)); @@ -138,8 +148,13 @@ export class ThemeService { ); } + /** + * Find a DSpaceObject in one of the provided route snapshots their data + * Recursively looks for the dso in the routes their child routes until it reaches a dead end or finds one + * @param routes + */ findRouteData(...routes: ActivatedRouteSnapshot[]) { - const result = routes.find((route) => hasValue(route.data.dso)); + const result = routes.find((route) => hasValue(route.data) && hasValue(route.data.dso)); if (hasValue(result)) { return result; } else { From 573a1338152a8720cd14bb21f383c96c5bdb2843 Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Thu, 23 Sep 2021 13:30:40 +0200 Subject: [PATCH 09/25] fix ciruclar dependency --- .../metadata-representation.decorator.ts | 7 ++++--- .../shared/listable-object/listable-object.decorator.ts | 6 ++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/app/shared/metadata-representation/metadata-representation.decorator.ts b/src/app/shared/metadata-representation/metadata-representation.decorator.ts index eca08c5c62..ae601e480c 100644 --- a/src/app/shared/metadata-representation/metadata-representation.decorator.ts +++ b/src/app/shared/metadata-representation/metadata-representation.decorator.ts @@ -3,7 +3,10 @@ import { hasNoValue, hasValue } from '../empty.util'; import { Context } from '../../core/shared/context.model'; import { InjectionToken } from '@angular/core'; import { GenericConstructor } from '../../core/shared/generic-constructor'; -import { resolveTheme } from '../object-collection/shared/listable-object/listable-object.decorator'; +import { + resolveTheme, + DEFAULT_THEME, DEFAULT_CONTEXT +} from '../object-collection/shared/listable-object/listable-object.decorator'; export const METADATA_REPRESENTATION_COMPONENT_FACTORY = new InjectionToken<(entityType: string, mdRepresentationType: MetadataRepresentationType, context: Context, theme: string) => GenericConstructor>('getMetadataRepresentationComponent', { providedIn: 'root', @@ -14,8 +17,6 @@ export const map = new Map(); export const DEFAULT_ENTITY_TYPE = 'Publication'; export const DEFAULT_REPRESENTATION_TYPE = MetadataRepresentationType.PlainText; -export const DEFAULT_CONTEXT = Context.Any; -export const DEFAULT_THEME = '*'; /** * Decorator function to store metadata representation mapping diff --git a/src/app/shared/object-collection/shared/listable-object/listable-object.decorator.ts b/src/app/shared/object-collection/shared/listable-object/listable-object.decorator.ts index 5df2683bb9..b7f27d1553 100644 --- a/src/app/shared/object-collection/shared/listable-object/listable-object.decorator.ts +++ b/src/app/shared/object-collection/shared/listable-object/listable-object.decorator.ts @@ -1,10 +1,6 @@ import { ViewMode } from '../../../../core/shared/view-mode.model'; import { Context } from '../../../../core/shared/context.model'; import { hasNoValue, hasValue, isNotEmpty } from '../../../empty.util'; -import { - DEFAULT_CONTEXT, - DEFAULT_THEME -} from '../../../metadata-representation/metadata-representation.decorator'; import { GenericConstructor } from '../../../../core/shared/generic-constructor'; import { ListableObject } from '../listable-object.model'; import { environment } from '../../../../../environments/environment'; @@ -12,6 +8,8 @@ import { ThemeConfig } from '../../../../../config/theme.model'; import { InjectionToken } from '@angular/core'; export const DEFAULT_VIEW_MODE = ViewMode.ListElement; +export const DEFAULT_CONTEXT = Context.Any; +export const DEFAULT_THEME = '*'; /** * Factory to allow us to inject getThemeConfigFor so we can mock it in tests From 100166023a582b5626d728073e540d6f0ea23001 Mon Sep 17 00:00:00 2001 From: Yana De Pauw Date: Thu, 23 Sep 2021 15:47:43 +0200 Subject: [PATCH 10/25] 83707: Control collection harvest from the UI --- .../collection-source-controls.component.html | 35 ++++ .../collection-source-controls.component.ts | 150 ++++++++++++++++++ .../collection-source.component.html | 125 ++++++++------- .../collection-source.component.ts | 2 +- .../edit-collection-page.module.ts | 3 + src/app/core/data/collection-data.service.ts | 4 +- src/app/core/shared/content-source.model.ts | 9 ++ src/assets/i18n/en.json5 | 16 ++ 8 files changed, 287 insertions(+), 57 deletions(-) create mode 100644 src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.html create mode 100644 src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts diff --git a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.html b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.html new file mode 100644 index 0000000000..425d773e2d --- /dev/null +++ b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.html @@ -0,0 +1,35 @@ +
+
+

{{ 'collection.source.controls.head' | translate }}

+
+ {{'collection.source.controls.harvest.status' | translate}} + {{contentSource?.harvestStatus}} +
+
+ {{'collection.source.controls.harvest.start' | translate}} + {{contentSource?.harvestStartTime ? contentSource?.harvestStartTime : 'collection.source.controls.harvest.no-information'|translate }} +
+
+ {{'collection.source.controls.harvest.last' | translate}} + {{contentSource?.lastHarvested ? contentSource?.lastHarvested : 'collection.source.controls.harvest.no-information'|translate }} +
+ + + + + + +
+
\ No newline at end of file diff --git a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts new file mode 100644 index 0000000000..417627cf46 --- /dev/null +++ b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts @@ -0,0 +1,150 @@ +import { Component, Input, OnDestroy } from '@angular/core'; +import { ScriptDataService } from '../../../../core/data/processes/script-data.service'; +import { ContentSource } from '../../../../core/shared/content-source.model'; +import { ProcessDataService } from '../../../../core/data/processes/process-data.service'; +import { + getAllCompletedRemoteData, + getAllSucceededRemoteDataPayload, + getFirstCompletedRemoteData, + getFirstSucceededRemoteDataPayload +} from '../../../../core/shared/operators'; +import { filter, map, switchMap, tap } from 'rxjs/operators'; +import { hasValue, hasValueOperator } from '../../../../shared/empty.util'; +import { ProcessStatus } from '../../../../process-page/processes/process-status.model'; +import { Subscription } from 'rxjs/internal/Subscription'; +import { RequestService } from '../../../../core/data/request.service'; +import { NotificationsService } from '../../../../shared/notifications/notifications.service'; +import { Collection } from '../../../../core/shared/collection.model'; +import { CollectionDataService } from '../../../../core/data/collection-data.service'; +import { Observable } from 'rxjs/internal/Observable'; +import { Process } from '../../../../process-page/processes/process.model'; +import { TranslateService } from '@ngx-translate/core'; +import { HttpClient } from '@angular/common/http'; +import { BitstreamDataService } from '../../../../core/data/bitstream-data.service'; + +@Component({ + selector: 'ds-collection-source-controls', + templateUrl: './collection-source-controls.component.html', +}) +export class CollectionSourceControlsComponent implements OnDestroy { + + @Input() isEnabled: boolean; + @Input() collection: Collection; + @Input() shouldShow: boolean; + + contentSource$: Observable; + private subs: Subscription[] = []; + + constructor(private scriptDataService: ScriptDataService, + private processDataService: ProcessDataService, + private requestService: RequestService, + private notificationsService: NotificationsService, + private collectionService: CollectionDataService, + private translateService: TranslateService, + private httpClient: HttpClient, + private bitstreamService: BitstreamDataService + ) { + } + + ngOnInit() { + this.contentSource$ = this.collectionService.findByHref(this.collection._links.self.href, false).pipe( + getAllSucceededRemoteDataPayload(), + switchMap((collection) => this.collectionService.getContentSource(collection.uuid, false)), + getAllSucceededRemoteDataPayload() + ); + } + + testConfiguration(contentSource) { + this.subs.push(this.scriptDataService.invoke('harvest', [ + {name: '-g', value: null}, + {name: '-a', value: contentSource.oaiSource}, + {name: '-i', value: contentSource.oaiSetId}, + ], []).pipe( + getFirstCompletedRemoteData(), + tap((rd) => { + if (rd.hasFailed) { + this.notificationsService.error(this.translateService.get('collection.source.controls.test.submit.error')); + } + }), + filter((rd) => rd.hasSucceeded && hasValue(rd.payload)), + switchMap((rd) => this.processDataService.findById(rd.payload.processId, false)), + getAllCompletedRemoteData(), + filter((rd) => !rd.isStale && (rd.hasSucceeded || rd.hasFailed)), + map((rd) => rd.payload), + hasValueOperator(), + ).subscribe((process: Process) => { + if (process.processStatus.toString() !== ProcessStatus[ProcessStatus.COMPLETED].toString() && + process.processStatus.toString() !== ProcessStatus[ProcessStatus.FAILED].toString()) { + setTimeout(() => { + this.requestService.setStaleByHrefSubstring(process._links.self.href); + }, 5000); + } + if (process.processStatus.toString() === ProcessStatus[ProcessStatus.FAILED].toString()) { + this.notificationsService.error(this.translateService.get('collection.source.controls.test.failed')); + } + if (process.processStatus.toString() === ProcessStatus[ProcessStatus.COMPLETED].toString()) { + this.bitstreamService.findByHref(process._links.output.href, false).pipe(getFirstSucceededRemoteDataPayload()).subscribe((bitstream) => { + this.httpClient.get(bitstream._links.content.href, {responseType: 'text'}).subscribe((data: any) => { + const output = data.replaceAll(new RegExp('.*\\@(.*)', 'g'), '$1') + .replaceAll('The script has started', '') + .replaceAll('The script has completed', ''); + this.notificationsService.success(this.translateService.get('collection.source.controls.test.completed'), output); + }); + }); + } + } + )); + } + + importNow() { + this.subs.push(this.scriptDataService.invoke('harvest', [ + {name: '-r', value: null}, + {name: '-e', value: 'dspacedemo+admin@gmail.com'}, + {name: '-c', value: this.collection.uuid}, + ], []) + .pipe( + getFirstCompletedRemoteData(), + tap((rd) => { + if (rd.hasFailed) { + this.notificationsService.error(this.translateService.get('collection.source.controls.test.submit.error')); + } + }), + filter((rd) => rd.hasSucceeded && hasValue(rd.payload)), + switchMap((rd) => this.processDataService.findById(rd.payload.processId, false)), + getAllCompletedRemoteData(), + filter((rd) => !rd.isStale && (rd.hasSucceeded || rd.hasFailed)), + map((rd) => rd.payload), + hasValueOperator(), + ).subscribe((process) => { + if (process.processStatus.toString() !== ProcessStatus[ProcessStatus.COMPLETED].toString() && + process.processStatus.toString() !== ProcessStatus[ProcessStatus.FAILED].toString()) { + setTimeout(() => { + this.requestService.setStaleByHrefSubstring(process._links.self.href); + this.requestService.setStaleByHrefSubstring(this.collection._links.self.href); + }, 5000); + } + if (process.processStatus.toString() === ProcessStatus[ProcessStatus.FAILED].toString()) { + this.notificationsService.error(this.translateService.get('collection.source.controls.import.failed')); + } + if (process.processStatus.toString() === ProcessStatus[ProcessStatus.COMPLETED].toString()) { + this.notificationsService.success(this.translateService.get('collection.source.controls.import.completed')); + setTimeout(() => { + this.requestService.setStaleByHrefSubstring(this.collection._links.self.href); + }, 5000); + } + } + )); + } + + resetAndReimport() { + // TODO implement when a single option is present + } + + ngOnDestroy(): void { + this.subs.forEach((sub) => { + if (hasValue(sub)) { + sub.unsubscribe(); + } + }); + } +} diff --git a/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.html b/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.html index de7f0b4708..b67ee9a1bd 100644 --- a/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.html +++ b/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.html @@ -1,57 +1,74 @@
-
- - - -
-

{{ 'collection.edit.tabs.source.head' | translate }}

-
- - -
- -

{{ 'collection.edit.tabs.source.form.head' | translate }}

+
+ + + +
+

{{ 'collection.edit.tabs.source.head' | translate }}

+
+ + +
+ +

{{ 'collection.edit.tabs.source.form.head' | translate }}

- -
-
- - - -
+
+
+
+
+
+
+ + + +
+
+
+
+ + + diff --git a/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.ts b/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.ts index c4b42d028d..ae48b9309e 100644 --- a/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.ts +++ b/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.ts @@ -380,7 +380,7 @@ export class CollectionSourceComponent extends AbstractTrackableComponent implem switchMap((uuid) => this.collectionService.getHarvesterEndpoint(uuid)), take(1) ).subscribe((endpoint) => this.requestService.removeByHrefSubstring(endpoint)); - + this.requestService.setStaleByHrefSubstring(this.contentSource._links.self.href); // Update harvester this.collectionRD$.pipe( getFirstSucceededRemoteData(), diff --git a/src/app/collection-page/edit-collection-page/edit-collection-page.module.ts b/src/app/collection-page/edit-collection-page/edit-collection-page.module.ts index b743032c8c..0b09542fa0 100644 --- a/src/app/collection-page/edit-collection-page/edit-collection-page.module.ts +++ b/src/app/collection-page/edit-collection-page/edit-collection-page.module.ts @@ -9,6 +9,7 @@ import { CollectionCurateComponent } from './collection-curate/collection-curate import { CollectionSourceComponent } from './collection-source/collection-source.component'; import { CollectionAuthorizationsComponent } from './collection-authorizations/collection-authorizations.component'; import { CollectionFormModule } from '../collection-form/collection-form.module'; +import { CollectionSourceControlsComponent } from './collection-source/collection-source-controls/collection-source-controls.component'; /** * Module that contains all components related to the Edit Collection page administrator functionality @@ -26,6 +27,8 @@ import { CollectionFormModule } from '../collection-form/collection-form.module' CollectionRolesComponent, CollectionCurateComponent, CollectionSourceComponent, + + CollectionSourceControlsComponent, CollectionAuthorizationsComponent ] }) diff --git a/src/app/core/data/collection-data.service.ts b/src/app/core/data/collection-data.service.ts index f58f36450f..334e56e371 100644 --- a/src/app/core/data/collection-data.service.ts +++ b/src/app/core/data/collection-data.service.ts @@ -138,7 +138,7 @@ export class CollectionDataService extends ComColDataService { * Get the collection's content harvester * @param collectionId */ - getContentSource(collectionId: string): Observable> { + getContentSource(collectionId: string, useCachedVersionIfAvailable = true): Observable> { const href$ = this.getHarvesterEndpoint(collectionId).pipe( isNotEmptyOperator(), take(1) @@ -146,7 +146,7 @@ export class CollectionDataService extends ComColDataService { href$.subscribe((href: string) => { const request = new ContentSourceRequest(this.requestService.generateRequestId(), href); - this.requestService.send(request, true); + this.requestService.send(request, useCachedVersionIfAvailable); }); return this.rdbService.buildSingle(href$); diff --git a/src/app/core/shared/content-source.model.ts b/src/app/core/shared/content-source.model.ts index 326407822f..c98901219d 100644 --- a/src/app/core/shared/content-source.model.ts +++ b/src/app/core/shared/content-source.model.ts @@ -70,6 +70,15 @@ export class ContentSource extends CacheableObject { */ metadataConfigs: MetadataConfig[]; + @autoserializeAs('harvest_status') + harvestStatus: string; + + @autoserializeAs('harvest_start_time') + harvestStartTime: string; + + @autoserializeAs('last_harvested') + lastHarvested: string; + /** * The {@link HALLink}s for this ContentSource */ diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 8908ca42f6..8d89561d68 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -881,6 +881,22 @@ "collection.select.table.title": "Title", + "collection.source.controls.head": "Harvest Controls", + "collection.source.controls.test.submit.error": "Something went wrong with initiating the testing of the settings", + "collection.source.controls.test.failed": "The script to test the settings has failed", + "collection.source.controls.test.completed": "The script to test the settings has successfully finished", + "collection.source.controls.test.submit": "Test configuration", + "collection.source.controls.import.submit.success": "The import has been successfully initiated", + "collection.source.controls.import.submit.error": "Something went wrong with initiating the import", + "collection.source.controls.import.submit": "Import now", + "collection.source.controls.import.failed": "An error occurred during the import", + "collection.source.controls.import.completed": "The import completed", + "collection.source.controls.reset.submit": "Reset and reimport", + "collection.source.controls.harvest.status": "Harvest status:", + "collection.source.controls.harvest.start": "Harvest start time:", + "collection.source.controls.harvest.last": "Last time harvested:", + "collection.source.controls.harvest.no-information": "N/A", + "collection.source.update.notifications.error.content": "The provided settings have been tested and didn't work.", From 4e9bd86012b006ab930b3430854ee7c52f97c94c Mon Sep 17 00:00:00 2001 From: Yana De Pauw Date: Mon, 27 Sep 2021 11:11:54 +0200 Subject: [PATCH 11/25] 83707: Implement tests --- ...llection-source-controls.component.spec.ts | 219 ++++++++++++++++++ .../collection-source-controls.component.ts | 35 ++- .../collection-source.component.spec.ts | 5 +- 3 files changed, 253 insertions(+), 6 deletions(-) create mode 100644 src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.spec.ts diff --git a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.spec.ts b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.spec.ts new file mode 100644 index 0000000000..3307b8fc79 --- /dev/null +++ b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.spec.ts @@ -0,0 +1,219 @@ +import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; +import { ContentSource } from '../../../../core/shared/content-source.model'; +import { Collection } from '../../../../core/shared/collection.model'; +import { createSuccessfulRemoteDataObject$ } from '../../../../shared/remote-data.utils'; +import { TranslateModule } from '@ngx-translate/core'; +import { RouterTestingModule } from '@angular/router/testing'; +import { NotificationsService } from '../../../../shared/notifications/notifications.service'; +import { CollectionDataService } from '../../../../core/data/collection-data.service'; +import { RequestService } from '../../../../core/data/request.service'; +import { NO_ERRORS_SCHEMA } from '@angular/core'; +import { ProcessDataService } from '../../../../core/data/processes/process-data.service'; +import { ScriptDataService } from '../../../../core/data/processes/script-data.service'; +import { HttpClient } from '@angular/common/http'; +import { BitstreamDataService } from '../../../../core/data/bitstream-data.service'; +import { NotificationsServiceStub } from '../../../../shared/testing/notifications-service.stub'; +import { Process } from '../../../../process-page/processes/process.model'; +import { of as observableOf } from 'rxjs'; +import { CollectionSourceControlsComponent } from './collection-source-controls.component'; +import { Bitstream } from '../../../../core/shared/bitstream.model'; +import { getTestScheduler } from 'jasmine-marbles'; +import { TestScheduler } from 'rxjs/testing'; +import { By } from '@angular/platform-browser'; +import { VarDirective } from '../../../../shared/utils/var.directive'; + +describe('CollectionSourceControlsComponent', () => { + let comp: CollectionSourceControlsComponent; + let fixture: ComponentFixture; + + const uuid = '29481ed7-ae6b-409a-8c51-34dd347a0ce4'; + let contentSource: ContentSource; + let collection: Collection; + let process: Process; + let bitstream: Bitstream; + + let scriptDataService: ScriptDataService; + let processDataService: ProcessDataService; + let requestService: RequestService; + let notificationsService; + let collectionService: CollectionDataService; + let httpClient: HttpClient; + let bitstreamService: BitstreamDataService; + let scheduler: TestScheduler; + + + beforeEach(waitForAsync(() => { + scheduler = getTestScheduler(); + contentSource = Object.assign(new ContentSource(), { + uuid: uuid, + metadataConfigs: [ + { + id: 'dc', + label: 'Simple Dublin Core', + nameSpace: 'http://www.openarchives.org/OAI/2.0/oai_dc/' + }, + { + id: 'qdc', + label: 'Qualified Dublin Core', + nameSpace: 'http://purl.org/dc/terms/' + }, + { + id: 'dim', + label: 'DSpace Intermediate Metadata', + nameSpace: 'http://www.dspace.org/xmlns/dspace/dim' + } + ], + oaiSource: 'oai-harvest-source', + oaiSetId: 'oai-set-id', + _links: {self: {href: 'contentsource-selflink'}} + }); + process = Object.assign(new Process(), { + processId: 'process-id', processStatus: 'COMPLETED', + _links: {output: {href: 'output-href'}} + }); + + bitstream = Object.assign(new Bitstream(), {_links: {content: {href: 'content-href'}}}); + + collection = Object.assign(new Collection(), { + uuid: 'fake-collection-id', + _links: {self: {href: 'collection-selflink'}} + }); + notificationsService = new NotificationsServiceStub(); + collectionService = jasmine.createSpyObj('collectionService', { + getContentSource: createSuccessfulRemoteDataObject$(contentSource), + findByHref: createSuccessfulRemoteDataObject$(collection) + }); + scriptDataService = jasmine.createSpyObj('scriptDataService', { + invoke: createSuccessfulRemoteDataObject$(process), + }); + processDataService = jasmine.createSpyObj('processDataService', { + findById: createSuccessfulRemoteDataObject$(process), + }); + bitstreamService = jasmine.createSpyObj('bitstreamService', { + findByHref: createSuccessfulRemoteDataObject$(bitstream), + }); + httpClient = jasmine.createSpyObj('httpClient', { + get: observableOf('Script text'), + }); + requestService = jasmine.createSpyObj('requestService', ['removeByHrefSubstring', 'setStaleByHrefSubstring']); + + TestBed.configureTestingModule({ + imports: [TranslateModule.forRoot(), RouterTestingModule], + declarations: [CollectionSourceControlsComponent, VarDirective], + providers: [ + {provide: ScriptDataService, useValue: scriptDataService}, + {provide: ProcessDataService, useValue: processDataService}, + {provide: RequestService, useValue: requestService}, + {provide: NotificationsService, useValue: notificationsService}, + {provide: CollectionDataService, useValue: collectionService}, + {provide: HttpClient, useValue: httpClient}, + {provide: BitstreamDataService, useValue: bitstreamService} + ], + schemas: [NO_ERRORS_SCHEMA] + }).compileComponents(); + })); + beforeEach(() => { + fixture = TestBed.createComponent(CollectionSourceControlsComponent); + comp = fixture.componentInstance; + comp.isEnabled = true; + comp.collection = collection; + comp.shouldShow = true; + fixture.detectChanges(); + }); + describe('init', () => { + it('should', () => { + expect(comp).toBeTruthy(); + }); + }); + describe('testConfiguration', () => { + it('should invoke a script and ping the resulting process until completed and show the resulting info', () => { + comp.testConfiguration(contentSource); + scheduler.flush(); + + expect(scriptDataService.invoke).toHaveBeenCalledWith('harvest', [ + {name: '-g', value: null}, + {name: '-a', value: contentSource.oaiSource}, + {name: '-i', value: contentSource.oaiSetId}, + ], []); + + expect(processDataService.findById).toHaveBeenCalledWith(process.processId, false); + expect(bitstreamService.findByHref).toHaveBeenCalledWith(process._links.output.href); + expect(notificationsService.info).toHaveBeenCalledWith(jasmine.anything() as any, 'Script text'); + }); + }); + describe('importNow', () => { + it('should invoke a script that will start the harvest', () => { + comp.importNow(); + scheduler.flush(); + + expect(scriptDataService.invoke).toHaveBeenCalledWith('harvest', [ + {name: '-r', value: null}, + {name: '-e', value: 'dspacedemo+admin@gmail.com'}, + {name: '-c', value: collection.uuid}, + ], []); + expect(processDataService.findById).toHaveBeenCalledWith(process.processId, false); + expect(notificationsService.success).toHaveBeenCalled(); + }); + }); + describe('the controls', () => { + it('should be shown when shouldShow is true', () => { + comp.shouldShow = true; + fixture.detectChanges(); + const buttons = fixture.debugElement.queryAll(By.css('button')); + expect(buttons.length).toEqual(3); + }); + it('should be shown when shouldShow is false', () => { + comp.shouldShow = false; + fixture.detectChanges(); + const buttons = fixture.debugElement.queryAll(By.css('button')); + expect(buttons.length).toEqual(0); + }); + it('should be disabled when isEnabled is false', () => { + comp.shouldShow = true; + comp.isEnabled = false; + + fixture.detectChanges(); + + const buttons = fixture.debugElement.queryAll(By.css('button')); + + expect(buttons[0].nativeElement.disabled).toBeTrue(); + expect(buttons[1].nativeElement.disabled).toBeTrue(); + expect(buttons[2].nativeElement.disabled).toBeTrue(); + }); + it('should be enabled when isEnabled is true', () => { + comp.shouldShow = true; + comp.isEnabled = true; + + fixture.detectChanges(); + + const buttons = fixture.debugElement.queryAll(By.css('button')); + + expect(buttons[0].nativeElement.disabled).toBeFalse(); + expect(buttons[1].nativeElement.disabled).toBeFalse(); + expect(buttons[2].nativeElement.disabled).toBeFalse(); + }); + it('should call the corresponding button when clicked', () => { + spyOn(comp, 'testConfiguration'); + spyOn(comp, 'importNow'); + spyOn(comp, 'resetAndReimport'); + + comp.shouldShow = true; + comp.isEnabled = true; + + fixture.detectChanges(); + + const buttons = fixture.debugElement.queryAll(By.css('button')); + + buttons[0].triggerEventHandler('click', null); + expect(comp.testConfiguration).toHaveBeenCalled(); + + buttons[1].triggerEventHandler('click', null); + expect(comp.importNow).toHaveBeenCalled(); + + buttons[2].triggerEventHandler('click', null); + expect(comp.resetAndReimport).toHaveBeenCalled(); + }); + }); + + +}); diff --git a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts index 417627cf46..4562772391 100644 --- a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts +++ b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts @@ -22,14 +22,28 @@ import { TranslateService } from '@ngx-translate/core'; import { HttpClient } from '@angular/common/http'; import { BitstreamDataService } from '../../../../core/data/bitstream-data.service'; +/** + * Component that contains the controls to run, reset and test the harvest + */ @Component({ selector: 'ds-collection-source-controls', templateUrl: './collection-source-controls.component.html', }) export class CollectionSourceControlsComponent implements OnDestroy { + /** + * Should the controls be enabled. + */ @Input() isEnabled: boolean; + + /** + * The current collection + */ @Input() collection: Collection; + + /** + * Should the control section be shown + */ @Input() shouldShow: boolean; contentSource$: Observable; @@ -47,6 +61,7 @@ export class CollectionSourceControlsComponent implements OnDestroy { } ngOnInit() { + // ensure the contentSource gets updated after being set to stale this.contentSource$ = this.collectionService.findByHref(this.collection._links.self.href, false).pipe( getAllSucceededRemoteDataPayload(), switchMap((collection) => this.collectionService.getContentSource(collection.uuid, false)), @@ -54,6 +69,10 @@ export class CollectionSourceControlsComponent implements OnDestroy { ); } + /** + * Tests the provided content source's configuration. + * @param contentSource - The content source to be tested + */ testConfiguration(contentSource) { this.subs.push(this.scriptDataService.invoke('harvest', [ {name: '-g', value: null}, @@ -63,9 +82,11 @@ export class CollectionSourceControlsComponent implements OnDestroy { getFirstCompletedRemoteData(), tap((rd) => { if (rd.hasFailed) { + // show a notification when the script invocation fails this.notificationsService.error(this.translateService.get('collection.source.controls.test.submit.error')); } }), + // filter out responses that aren't successful since the pinging of the process only needs to happen when the invocation was successful. filter((rd) => rd.hasSucceeded && hasValue(rd.payload)), switchMap((rd) => this.processDataService.findById(rd.payload.processId, false)), getAllCompletedRemoteData(), @@ -75,6 +96,7 @@ export class CollectionSourceControlsComponent implements OnDestroy { ).subscribe((process: Process) => { if (process.processStatus.toString() !== ProcessStatus[ProcessStatus.COMPLETED].toString() && process.processStatus.toString() !== ProcessStatus[ProcessStatus.FAILED].toString()) { + // Ping the current process state every 5s setTimeout(() => { this.requestService.setStaleByHrefSubstring(process._links.self.href); }, 5000); @@ -83,12 +105,12 @@ export class CollectionSourceControlsComponent implements OnDestroy { this.notificationsService.error(this.translateService.get('collection.source.controls.test.failed')); } if (process.processStatus.toString() === ProcessStatus[ProcessStatus.COMPLETED].toString()) { - this.bitstreamService.findByHref(process._links.output.href, false).pipe(getFirstSucceededRemoteDataPayload()).subscribe((bitstream) => { + this.bitstreamService.findByHref(process._links.output.href).pipe(getFirstSucceededRemoteDataPayload()).subscribe((bitstream) => { this.httpClient.get(bitstream._links.content.href, {responseType: 'text'}).subscribe((data: any) => { const output = data.replaceAll(new RegExp('.*\\@(.*)', 'g'), '$1') .replaceAll('The script has started', '') .replaceAll('The script has completed', ''); - this.notificationsService.success(this.translateService.get('collection.source.controls.test.completed'), output); + this.notificationsService.info(this.translateService.get('collection.source.controls.test.completed'), output); }); }); } @@ -96,6 +118,9 @@ export class CollectionSourceControlsComponent implements OnDestroy { )); } + /** + * Start the harvest for the current collection + */ importNow() { this.subs.push(this.scriptDataService.invoke('harvest', [ {name: '-r', value: null}, @@ -118,6 +143,7 @@ export class CollectionSourceControlsComponent implements OnDestroy { ).subscribe((process) => { if (process.processStatus.toString() !== ProcessStatus[ProcessStatus.COMPLETED].toString() && process.processStatus.toString() !== ProcessStatus[ProcessStatus.FAILED].toString()) { + // Ping the current process state every 5s setTimeout(() => { this.requestService.setStaleByHrefSubstring(process._links.self.href); this.requestService.setStaleByHrefSubstring(this.collection._links.self.href); @@ -128,14 +154,15 @@ export class CollectionSourceControlsComponent implements OnDestroy { } if (process.processStatus.toString() === ProcessStatus[ProcessStatus.COMPLETED].toString()) { this.notificationsService.success(this.translateService.get('collection.source.controls.import.completed')); - setTimeout(() => { this.requestService.setStaleByHrefSubstring(this.collection._links.self.href); - }, 5000); } } )); } + /** + * Reset and reimport the current collection + */ resetAndReimport() { // TODO implement when a single option is present } diff --git a/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.spec.ts b/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.spec.ts index 869238b956..3fb1a50bf1 100644 --- a/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.spec.ts +++ b/src/app/collection-page/edit-collection-page/collection-source/collection-source.component.spec.ts @@ -62,7 +62,8 @@ describe('CollectionSourceComponent', () => { label: 'DSpace Intermediate Metadata', nameSpace: 'http://www.dspace.org/xmlns/dspace/dim' } - ] + ], + _links: { self: { href: 'contentsource-selflink' } } }); fieldUpdate = { field: contentSource, @@ -115,7 +116,7 @@ describe('CollectionSourceComponent', () => { updateContentSource: observableOf(contentSource), getHarvesterEndpoint: observableOf('harvester-endpoint') }); - requestService = jasmine.createSpyObj('requestService', ['removeByHrefSubstring']); + requestService = jasmine.createSpyObj('requestService', ['removeByHrefSubstring', 'setStaleByHrefSubstring']); TestBed.configureTestingModule({ imports: [TranslateModule.forRoot(), RouterTestingModule], From e5e6e9c07aca19fc4edfb44b18a79e0f8f5742e3 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Tue, 28 Sep 2021 12:20:53 -0500 Subject: [PATCH 12/25] Also support using spacebar to open/close menu --- .../expandable-navbar-section.component.html | 1 + ...xpandable-navbar-section.component.spec.ts | 36 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html index 04bc6067ac..171530ecde 100644 --- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html +++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html @@ -1,6 +1,7 @@
{ }); }); + describe('when spacebar is pressed on section header (while inactive)', () => { + beforeEach(() => { + spyOn(menuService, 'activateSection'); + // Make sure section is 'inactive'. Requires calling ngOnInit() to update component 'active' property. + spyOn(menuService, 'isSectionActive').and.returnValue(observableOf(false)); + component.ngOnInit(); + fixture.detectChanges(); + + const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); + // dispatch the (keyup.space) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: ' ' })); + }); + + it('should call activateSection on the menuService', () => { + expect(menuService.activateSection).toHaveBeenCalled(); + }); + }); + + describe('when spacebar is pressed on section header (while active)', () => { + beforeEach(() => { + spyOn(menuService, 'deactivateSection'); + // Make sure section is 'active'. Requires calling ngOnInit() to update component 'active' property. + spyOn(menuService, 'isSectionActive').and.returnValue(observableOf(true)); + component.ngOnInit(); + fixture.detectChanges(); + + const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); + // dispatch the (keyup.space) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: ' ' })); + }); + + it('should call deactivateSection on the menuService', () => { + expect(menuService.deactivateSection).toHaveBeenCalled(); + }); + }); + describe('when a click occurs on the section header', () => { beforeEach(() => { spyOn(menuService, 'toggleActiveSection'); From 578b5afa0604af6eb58b2b1b6448da30032ba5a5 Mon Sep 17 00:00:00 2001 From: Bruno Roemers Date: Wed, 29 Sep 2021 18:37:39 +0200 Subject: [PATCH 13/25] 83783: Show owning and mapped collections on item page --- src/app/core/data/collection-data.service.ts | 12 +- .../collections/collections.component.html | 18 +- .../collections/collections.component.spec.ts | 316 +++++++++++++++--- .../collections/collections.component.ts | 123 +++++-- src/assets/i18n/en.json5 | 4 + 5 files changed, 393 insertions(+), 80 deletions(-) diff --git a/src/app/core/data/collection-data.service.ts b/src/app/core/data/collection-data.service.ts index f58f36450f..12bd12ec59 100644 --- a/src/app/core/data/collection-data.service.ts +++ b/src/app/core/data/collection-data.service.ts @@ -208,10 +208,20 @@ export class CollectionDataService extends ComColDataService { } /** - * Returns {@link RemoteData} of {@link Collection} that is the owing collection of the given item + * Returns {@link RemoteData} of {@link Collection} that is the owning collection of the given item * @param item Item we want the owning collection of */ findOwningCollectionFor(item: Item): Observable> { return this.findByHref(item._links.owningCollection.href); } + + /** + * Get a list of mapped collections for the given item. + * @param item Item for which the mapped collections should be retrieved. + * @param findListOptions Pagination and search options. + */ + findMappedCollectionsFor(item: Item, findListOptions?: FindListOptions): Observable>> { + return this.findAllByHref(item._links.mappedCollections.href, findListOptions); + } + } diff --git a/src/app/item-page/field-components/collections/collections.component.html b/src/app/item-page/field-components/collections/collections.component.html index e0f963b5bc..e8f682a182 100644 --- a/src/app/item-page/field-components/collections/collections.component.html +++ b/src/app/item-page/field-components/collections/collections.component.html @@ -1,7 +1,21 @@ - +
- + {{collection?.name}}
+ +
+ {{'item.page.collections.loading' | translate}} +
+ + + {{'item.page.collections.load-more' | translate}} + diff --git a/src/app/item-page/field-components/collections/collections.component.spec.ts b/src/app/item-page/field-components/collections/collections.component.spec.ts index 70ce5db760..d5278706da 100644 --- a/src/app/item-page/field-components/collections/collections.component.spec.ts +++ b/src/app/item-page/field-components/collections/collections.component.spec.ts @@ -9,46 +9,45 @@ import { Item } from '../../../core/shared/item.model'; import { getMockRemoteDataBuildService } from '../../../shared/mocks/remote-data-build.service.mock'; import { createFailedRemoteDataObject$, createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils'; import { CollectionsComponent } from './collections.component'; +import { FindListOptions } from '../../../core/data/request.models'; +import { buildPaginatedList, PaginatedList } from '../../../core/data/paginated-list.model'; +import { PageInfo } from '../../../core/shared/page-info.model'; -let collectionsComponent: CollectionsComponent; -let fixture: ComponentFixture; - -let collectionDataServiceStub; - -const mockCollection1: Collection = Object.assign(new Collection(), { - metadata: { - 'dc.description.abstract': [ - { - language: 'en_US', - value: 'Short description' - } - ] - }, - _links: { - self: { href: 'collection-selflink' } - } +const createMockCollection = (id: string) => Object.assign(new Collection(), { + id: id, + name: `collection-${id}`, }); -const succeededMockItem: Item = Object.assign(new Item(), {owningCollection: createSuccessfulRemoteDataObject$(mockCollection1)}); -const failedMockItem: Item = Object.assign(new Item(), {owningCollection: createFailedRemoteDataObject$('error', 500)}); +const mockItem: Item = new Item(); describe('CollectionsComponent', () => { - collectionDataServiceStub = { - findOwningCollectionFor(item: Item) { - if (item === succeededMockItem) { - return createSuccessfulRemoteDataObject$(mockCollection1); - } else { - return createFailedRemoteDataObject$('error', 500); - } - } - }; + let collectionDataService; + + let mockCollection1: Collection; + let mockCollection2: Collection; + let mockCollection3: Collection; + let mockCollection4: Collection; + + let component: CollectionsComponent; + let fixture: ComponentFixture; + beforeEach(waitForAsync(() => { + collectionDataService = jasmine.createSpyObj([ + 'findOwningCollectionFor', + 'findMappedCollectionsFor', + ]); + + mockCollection1 = createMockCollection('c1'); + mockCollection2 = createMockCollection('c2'); + mockCollection3 = createMockCollection('c3'); + mockCollection4 = createMockCollection('c4'); + TestBed.configureTestingModule({ imports: [TranslateModule.forRoot()], declarations: [ CollectionsComponent ], providers: [ { provide: RemoteDataBuildService, useValue: getMockRemoteDataBuildService()}, - { provide: CollectionDataService, useValue: collectionDataServiceStub }, + { provide: CollectionDataService, useValue: collectionDataService }, ], schemas: [ NO_ERRORS_SCHEMA ] @@ -59,33 +58,264 @@ describe('CollectionsComponent', () => { beforeEach(waitForAsync(() => { fixture = TestBed.createComponent(CollectionsComponent); - collectionsComponent = fixture.componentInstance; - collectionsComponent.label = 'test.test'; - collectionsComponent.separator = '
'; - + component = fixture.componentInstance; + component.item = mockItem; + component.label = 'test.test'; + component.separator = '
'; + component.pageSize = 2; })); - describe('When the requested item request has succeeded', () => { + describe('when the item has only an owning collection', () => { + let mockPage1: PaginatedList; + beforeEach(() => { - collectionsComponent.item = succeededMockItem; + mockPage1 = buildPaginatedList(Object.assign(new PageInfo(), { + currentPage: 1, + elementsPerPage: 2, + totalPages: 0, + totalElements: 0, + }), []); + + collectionDataService.findOwningCollectionFor.and.returnValue(createSuccessfulRemoteDataObject$(mockCollection1)); + collectionDataService.findMappedCollectionsFor.and.returnValue(createSuccessfulRemoteDataObject$(mockPage1)); fixture.detectChanges(); }); - it('should show the collection', () => { - const collectionField = fixture.debugElement.query(By.css('ds-metadata-field-wrapper div.collections')); - expect(collectionField).not.toBeNull(); + it('should display the owning collection', () => { + const collectionFields = fixture.debugElement.queryAll(By.css('ds-metadata-field-wrapper div.collections a')); + const loadMoreBtn = fixture.debugElement.query(By.css('ds-metadata-field-wrapper .load-more-btn')); + + expect(collectionDataService.findOwningCollectionFor).toHaveBeenCalledOnceWith(mockItem); + expect(collectionDataService.findMappedCollectionsFor).toHaveBeenCalledOnceWith(mockItem, Object.assign(new FindListOptions(), { + elementsPerPage: 2, + currentPage: 1, + })); + + expect(collectionFields.length).toBe(1); + expect(collectionFields[0].nativeElement.textContent).toEqual('collection-c1'); + + expect(component.lastPage$.getValue()).toBe(1); + expect(component.hasMore$.getValue()).toBe(false); + expect(component.isLoading$.getValue()).toBe(false); + + expect(loadMoreBtn).toBeNull(); }); }); - describe('When the requested item request has failed', () => { + describe('when the item has an owning collection and one mapped collection', () => { + let mockPage1: PaginatedList; + beforeEach(() => { - collectionsComponent.item = failedMockItem; + mockPage1 = buildPaginatedList(Object.assign(new PageInfo(), { + currentPage: 1, + elementsPerPage: 2, + totalPages: 1, + totalElements: 1, + }), [mockCollection2]); + + collectionDataService.findOwningCollectionFor.and.returnValue(createSuccessfulRemoteDataObject$(mockCollection1)); + collectionDataService.findMappedCollectionsFor.and.returnValue(createSuccessfulRemoteDataObject$(mockPage1)); fixture.detectChanges(); }); - it('should not show the collection', () => { - const collectionField = fixture.debugElement.query(By.css('ds-metadata-field-wrapper div.collections')); - expect(collectionField).toBeNull(); + it('should display the owning collection and the mapped collection', () => { + const collectionFields = fixture.debugElement.queryAll(By.css('ds-metadata-field-wrapper div.collections a')); + const loadMoreBtn = fixture.debugElement.query(By.css('ds-metadata-field-wrapper .load-more-btn')); + + expect(collectionDataService.findOwningCollectionFor).toHaveBeenCalledOnceWith(mockItem); + expect(collectionDataService.findMappedCollectionsFor).toHaveBeenCalledOnceWith(mockItem, Object.assign(new FindListOptions(), { + elementsPerPage: 2, + currentPage: 1, + })); + + expect(collectionFields.length).toBe(2); + expect(collectionFields[0].nativeElement.textContent).toEqual('collection-c1'); + expect(collectionFields[1].nativeElement.textContent).toEqual('collection-c2'); + + expect(component.lastPage$.getValue()).toBe(1); + expect(component.hasMore$.getValue()).toBe(false); + expect(component.isLoading$.getValue()).toBe(false); + + expect(loadMoreBtn).toBeNull(); }); }); + + describe('when the item has an owning collection and multiple mapped collections', () => { + let mockPage1: PaginatedList; + let mockPage2: PaginatedList; + + beforeEach(() => { + mockPage1 = buildPaginatedList(Object.assign(new PageInfo(), { + currentPage: 1, + elementsPerPage: 2, + totalPages: 2, + totalElements: 3, + }), [mockCollection2, mockCollection3]); + + mockPage2 = buildPaginatedList(Object.assign(new PageInfo(), { + currentPage: 2, + elementsPerPage: 2, + totalPages: 2, + totalElements: 1, + }), [mockCollection4]); + + collectionDataService.findOwningCollectionFor.and.returnValue(createSuccessfulRemoteDataObject$(mockCollection1)); + collectionDataService.findMappedCollectionsFor.and.returnValues( + createSuccessfulRemoteDataObject$(mockPage1), + createSuccessfulRemoteDataObject$(mockPage2), + ); + fixture.detectChanges(); + }); + + it('should display the owning collection, two mapped collections and a load more button', () => { + const collectionFields = fixture.debugElement.queryAll(By.css('ds-metadata-field-wrapper div.collections a')); + const loadMoreBtn = fixture.debugElement.query(By.css('ds-metadata-field-wrapper .load-more-btn')); + + expect(collectionDataService.findOwningCollectionFor).toHaveBeenCalledOnceWith(mockItem); + expect(collectionDataService.findMappedCollectionsFor).toHaveBeenCalledOnceWith(mockItem, Object.assign(new FindListOptions(), { + elementsPerPage: 2, + currentPage: 1, + })); + + expect(collectionFields.length).toBe(3); + expect(collectionFields[0].nativeElement.textContent).toEqual('collection-c1'); + expect(collectionFields[1].nativeElement.textContent).toEqual('collection-c2'); + expect(collectionFields[2].nativeElement.textContent).toEqual('collection-c3'); + + expect(component.lastPage$.getValue()).toBe(1); + expect(component.hasMore$.getValue()).toBe(true); + expect(component.isLoading$.getValue()).toBe(false); + + expect(loadMoreBtn).toBeTruthy(); + }); + + describe('when the load more button is clicked', () => { + beforeEach(() => { + const loadMoreBtn = fixture.debugElement.query(By.css('ds-metadata-field-wrapper .load-more-btn')); + loadMoreBtn.nativeElement.click(); + fixture.detectChanges(); + }); + + it('should display the owning collection and three mapped collections', () => { + const collectionFields = fixture.debugElement.queryAll(By.css('ds-metadata-field-wrapper div.collections a')); + const loadMoreBtn = fixture.debugElement.query(By.css('ds-metadata-field-wrapper .load-more-btn')); + + expect(collectionDataService.findOwningCollectionFor).toHaveBeenCalledOnceWith(mockItem); + expect(collectionDataService.findMappedCollectionsFor).toHaveBeenCalledTimes(2); + expect(collectionDataService.findMappedCollectionsFor).toHaveBeenCalledWith(mockItem, Object.assign(new FindListOptions(), { + elementsPerPage: 2, + currentPage: 1, + })); + expect(collectionDataService.findMappedCollectionsFor).toHaveBeenCalledWith(mockItem, Object.assign(new FindListOptions(), { + elementsPerPage: 2, + currentPage: 2, + })); + + expect(collectionFields.length).toBe(4); + expect(collectionFields[0].nativeElement.textContent).toEqual('collection-c1'); + expect(collectionFields[1].nativeElement.textContent).toEqual('collection-c2'); + expect(collectionFields[2].nativeElement.textContent).toEqual('collection-c3'); + expect(collectionFields[3].nativeElement.textContent).toEqual('collection-c4'); + + expect(component.lastPage$.getValue()).toBe(2); + expect(component.hasMore$.getValue()).toBe(false); + expect(component.isLoading$.getValue()).toBe(false); + + expect(loadMoreBtn).toBeNull(); + }); + }); + }); + + describe('when the request for the owning collection fails', () => { + let mockPage1: PaginatedList; + + beforeEach(() => { + mockPage1 = buildPaginatedList(Object.assign(new PageInfo(), { + currentPage: 1, + elementsPerPage: 2, + totalPages: 1, + totalElements: 1, + }), [mockCollection2]); + + collectionDataService.findOwningCollectionFor.and.returnValue(createFailedRemoteDataObject$()); + collectionDataService.findMappedCollectionsFor.and.returnValue(createSuccessfulRemoteDataObject$(mockPage1)); + fixture.detectChanges(); + }); + + it('should display the mapped collection only', () => { + const collectionFields = fixture.debugElement.queryAll(By.css('ds-metadata-field-wrapper div.collections a')); + const loadMoreBtn = fixture.debugElement.query(By.css('ds-metadata-field-wrapper .load-more-btn')); + + expect(collectionDataService.findOwningCollectionFor).toHaveBeenCalledOnceWith(mockItem); + expect(collectionDataService.findMappedCollectionsFor).toHaveBeenCalledOnceWith(mockItem, Object.assign(new FindListOptions(), { + elementsPerPage: 2, + currentPage: 1, + })); + + expect(collectionFields.length).toBe(1); + expect(collectionFields[0].nativeElement.textContent).toEqual('collection-c2'); + + expect(component.lastPage$.getValue()).toBe(1); + expect(component.hasMore$.getValue()).toBe(false); + expect(component.isLoading$.getValue()).toBe(false); + + expect(loadMoreBtn).toBeNull(); + }); + }); + + describe('when the request for the mapped collections fails', () => { + beforeEach(() => { + collectionDataService.findOwningCollectionFor.and.returnValue(createSuccessfulRemoteDataObject$(mockCollection1)); + collectionDataService.findMappedCollectionsFor.and.returnValue(createFailedRemoteDataObject$()); + fixture.detectChanges(); + }); + + it('should display the owning collection only', () => { + const collectionFields = fixture.debugElement.queryAll(By.css('ds-metadata-field-wrapper div.collections a')); + const loadMoreBtn = fixture.debugElement.query(By.css('ds-metadata-field-wrapper .load-more-btn')); + + expect(collectionDataService.findOwningCollectionFor).toHaveBeenCalledOnceWith(mockItem); + expect(collectionDataService.findMappedCollectionsFor).toHaveBeenCalledOnceWith(mockItem, Object.assign(new FindListOptions(), { + elementsPerPage: 2, + currentPage: 1, + })); + + expect(collectionFields.length).toBe(1); + expect(collectionFields[0].nativeElement.textContent).toEqual('collection-c1'); + + expect(component.lastPage$.getValue()).toBe(0); + expect(component.hasMore$.getValue()).toBe(true); + expect(component.isLoading$.getValue()).toBe(false); + + expect(loadMoreBtn).toBeTruthy(); + }); + }); + + describe('when both requests fail', () => { + beforeEach(() => { + collectionDataService.findOwningCollectionFor.and.returnValue(createFailedRemoteDataObject$()); + collectionDataService.findMappedCollectionsFor.and.returnValue(createFailedRemoteDataObject$()); + fixture.detectChanges(); + }); + + it('should display no collections', () => { + const collectionFields = fixture.debugElement.queryAll(By.css('ds-metadata-field-wrapper div.collections a')); + const loadMoreBtn = fixture.debugElement.query(By.css('ds-metadata-field-wrapper .load-more-btn')); + + expect(collectionDataService.findOwningCollectionFor).toHaveBeenCalledOnceWith(mockItem); + expect(collectionDataService.findMappedCollectionsFor).toHaveBeenCalledOnceWith(mockItem, Object.assign(new FindListOptions(), { + elementsPerPage: 2, + currentPage: 1, + })); + + expect(collectionFields.length).toBe(0); + + expect(component.lastPage$.getValue()).toBe(0); + expect(component.hasMore$.getValue()).toBe(true); + expect(component.isLoading$.getValue()).toBe(false); + + expect(loadMoreBtn).toBeTruthy(); + }); + }); + }); diff --git a/src/app/item-page/field-components/collections/collections.component.ts b/src/app/item-page/field-components/collections/collections.component.ts index 32dc8dfb73..23aff80160 100644 --- a/src/app/item-page/field-components/collections/collections.component.ts +++ b/src/app/item-page/field-components/collections/collections.component.ts @@ -1,14 +1,19 @@ import { Component, Input, OnInit } from '@angular/core'; -import { Observable } from 'rxjs'; -import { map } from 'rxjs/operators'; +import { BehaviorSubject, combineLatest, Observable } from 'rxjs'; +import {map, scan, startWith, switchMap, tap, withLatestFrom} from 'rxjs/operators'; import { CollectionDataService } from '../../../core/data/collection-data.service'; -import { PaginatedList, buildPaginatedList } from '../../../core/data/paginated-list.model'; -import { RemoteData } from '../../../core/data/remote-data'; +import { PaginatedList } from '../../../core/data/paginated-list.model'; import { Collection } from '../../../core/shared/collection.model'; import { Item } from '../../../core/shared/item.model'; -import { PageInfo } from '../../../core/shared/page-info.model'; import { hasValue } from '../../../shared/empty.util'; +import { FindListOptions } from '../../../core/data/request.models'; +import { + getAllCompletedRemoteData, + getAllSucceededRemoteDataPayload, + getFirstSucceededRemoteDataPayload, + getPaginatedListPayload, +} from '../../../core/shared/operators'; /** * This component renders the parent collections section of the item @@ -27,42 +32,92 @@ export class CollectionsComponent implements OnInit { separator = '
'; - collectionsRD$: Observable>>; + /** + * Amount of mapped collections that should be fetched at once. + */ + pageSize = 5; + + /** + * Last page of the mapped collections that has been fetched. + */ + lastPage$: BehaviorSubject = new BehaviorSubject(0); + + /** + * Push an event to this observable to fetch the next page of mapped collections. + * Because this observable is a behavior subject, the first page will be requested + * immediately after subscription. + */ + loadMore$: BehaviorSubject = new BehaviorSubject(undefined); + + /** + * Whether or not a page of mapped collections is currently being loaded. + */ + isLoading$: BehaviorSubject = new BehaviorSubject(true); + + /** + * Whether or not more pages of mapped collections are available. + */ + hasMore$: BehaviorSubject = new BehaviorSubject(true); + + /** + * All collections that have been retrieved so far. This includes the owning collection, + * as well as any number of pages of mapped collections. + */ + collections$: Observable; constructor(private cds: CollectionDataService) { } ngOnInit(): void { - // this.collections = this.item.parents.payload; + const owningCollection$: Observable = this.cds.findOwningCollectionFor(this.item).pipe( + getFirstSucceededRemoteDataPayload(), + startWith(null as Collection), + ); - // TODO: this should use parents, but the collections - // for an Item aren't returned by the REST API yet, - // only the owning collection - this.collectionsRD$ = this.cds.findOwningCollectionFor(this.item).pipe( - map((rd: RemoteData) => { - if (hasValue(rd.payload)) { - return new RemoteData( - rd.timeCompleted, - rd.msToLive, - rd.lastUpdated, - rd.state, - rd.errorMessage, - buildPaginatedList({ - elementsPerPage: 10, - totalPages: 1, - currentPage: 1, - totalElements: 1, - _links: { - self: rd.payload._links.self - } - } as PageInfo, [rd.payload]), - rd.statusCode - ); - } else { - return rd as any; - } - }) + const mappedCollections$: Observable = this.loadMore$.pipe( + // update isLoading$ + tap(() => this.isLoading$.next(true)), + + // request next batch of mapped collections + withLatestFrom(this.lastPage$), + switchMap(([_, lastPage]: [void, number]) => { + return this.cds.findMappedCollectionsFor(this.item, Object.assign(new FindListOptions(), { + elementsPerPage: this.pageSize, + currentPage: lastPage + 1, + })); + }), + + getAllCompletedRemoteData>(), + + // update isLoading$ + tap(() => this.isLoading$.next(false)), + + getAllSucceededRemoteDataPayload(), + + // update hasMore$ + tap((response: PaginatedList) => this.hasMore$.next(response.currentPage < response.totalPages)), + + // update lastPage$ + tap((response: PaginatedList) => this.lastPage$.next(response.currentPage)), + + getPaginatedListPayload(), + + // add current batch to list of collections + scan((prev: Collection[], current: Collection[]) => [...prev, ...current], []), + + startWith([]), + ) as Observable; + + this.collections$ = combineLatest([owningCollection$, mappedCollections$]).pipe( + map(([owningCollection, mappedCollections]: [Collection, Collection[]]) => { + return [owningCollection, ...mappedCollections].filter(collection => hasValue(collection)); + }), ); } + + handleLoadMore() { + this.loadMore$.next(); + } + } diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index aa76d2e422..1bacd39bbc 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -1874,6 +1874,10 @@ "item.page.collections": "Collections", + "item.page.collections.loading": "Loading...", + + "item.page.collections.load-more": "Load more", + "item.page.date": "Date", "item.page.edit": "Edit this item", From e434bb895246e8ce050b172dcb8fad9e2a4e13f3 Mon Sep 17 00:00:00 2001 From: Yana De Pauw Date: Thu, 30 Sep 2021 10:22:48 +0200 Subject: [PATCH 14/25] 83707: Implement feedback --- .../collection-source-controls.component.html | 4 ++ ...llection-source-controls.component.spec.ts | 17 ++++++- .../collection-source-controls.component.ts | 46 +++++++++++++++++-- .../content-source-set-serializer.spec.ts | 26 +++++++++++ .../shared/content-source-set-serializer.ts | 31 +++++++++++++ src/app/core/shared/content-source.model.ts | 22 ++++++++- src/assets/i18n/en.json5 | 5 ++ 7 files changed, 143 insertions(+), 8 deletions(-) create mode 100644 src/app/core/shared/content-source-set-serializer.spec.ts create mode 100644 src/app/core/shared/content-source-set-serializer.ts diff --git a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.html b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.html index 425d773e2d..7a0f07adda 100644 --- a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.html +++ b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.html @@ -11,6 +11,10 @@
{{'collection.source.controls.harvest.last' | translate}} + {{contentSource?.message ? contentSource?.message : 'collection.source.controls.harvest.no-information'|translate }} +
+
+ {{'collection.source.controls.harvest.message' | translate}} {{contentSource?.lastHarvested ? contentSource?.lastHarvested : 'collection.source.controls.harvest.no-information'|translate }}
diff --git a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.spec.ts b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.spec.ts index 3307b8fc79..3eb83ebe8a 100644 --- a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.spec.ts +++ b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.spec.ts @@ -21,6 +21,7 @@ import { getTestScheduler } from 'jasmine-marbles'; import { TestScheduler } from 'rxjs/testing'; import { By } from '@angular/platform-browser'; import { VarDirective } from '../../../../shared/utils/var.directive'; +import { ContentSourceSetSerializer } from '../../../../core/shared/content-source-set-serializer'; describe('CollectionSourceControlsComponent', () => { let comp: CollectionSourceControlsComponent; @@ -133,7 +134,7 @@ describe('CollectionSourceControlsComponent', () => { expect(scriptDataService.invoke).toHaveBeenCalledWith('harvest', [ {name: '-g', value: null}, {name: '-a', value: contentSource.oaiSource}, - {name: '-i', value: contentSource.oaiSetId}, + {name: '-i', value: new ContentSourceSetSerializer().Serialize(contentSource.oaiSetId)}, ], []); expect(processDataService.findById).toHaveBeenCalledWith(process.processId, false); @@ -148,7 +149,19 @@ describe('CollectionSourceControlsComponent', () => { expect(scriptDataService.invoke).toHaveBeenCalledWith('harvest', [ {name: '-r', value: null}, - {name: '-e', value: 'dspacedemo+admin@gmail.com'}, + {name: '-c', value: collection.uuid}, + ], []); + expect(processDataService.findById).toHaveBeenCalledWith(process.processId, false); + expect(notificationsService.success).toHaveBeenCalled(); + }); + }); + describe('resetAndReimport', () => { + it('should invoke a script that will start the harvest', () => { + comp.resetAndReimport(); + scheduler.flush(); + + expect(scriptDataService.invoke).toHaveBeenCalledWith('harvest', [ + {name: '-o', value: null}, {name: '-c', value: collection.uuid}, ], []); expect(processDataService.findById).toHaveBeenCalledWith(process.processId, false); diff --git a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts index 4562772391..ca6e1e9cd8 100644 --- a/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts +++ b/src/app/collection-page/edit-collection-page/collection-source/collection-source-controls/collection-source-controls.component.ts @@ -21,6 +21,7 @@ import { Process } from '../../../../process-page/processes/process.model'; import { TranslateService } from '@ngx-translate/core'; import { HttpClient } from '@angular/common/http'; import { BitstreamDataService } from '../../../../core/data/bitstream-data.service'; +import { ContentSourceSetSerializer } from '../../../../core/shared/content-source-set-serializer'; /** * Component that contains the controls to run, reset and test the harvest @@ -77,7 +78,7 @@ export class CollectionSourceControlsComponent implements OnDestroy { this.subs.push(this.scriptDataService.invoke('harvest', [ {name: '-g', value: null}, {name: '-a', value: contentSource.oaiSource}, - {name: '-i', value: contentSource.oaiSetId}, + {name: '-i', value: new ContentSourceSetSerializer().Serialize(contentSource.oaiSetId)}, ], []).pipe( getFirstCompletedRemoteData(), tap((rd) => { @@ -124,14 +125,15 @@ export class CollectionSourceControlsComponent implements OnDestroy { importNow() { this.subs.push(this.scriptDataService.invoke('harvest', [ {name: '-r', value: null}, - {name: '-e', value: 'dspacedemo+admin@gmail.com'}, {name: '-c', value: this.collection.uuid}, ], []) .pipe( getFirstCompletedRemoteData(), tap((rd) => { if (rd.hasFailed) { - this.notificationsService.error(this.translateService.get('collection.source.controls.test.submit.error')); + this.notificationsService.error(this.translateService.get('collection.source.controls.import.submit.error')); + } else { + this.notificationsService.success(this.translateService.get('collection.source.controls.import.submit.success')); } }), filter((rd) => rd.hasSucceeded && hasValue(rd.payload)), @@ -164,7 +166,43 @@ export class CollectionSourceControlsComponent implements OnDestroy { * Reset and reimport the current collection */ resetAndReimport() { - // TODO implement when a single option is present + this.subs.push(this.scriptDataService.invoke('harvest', [ + {name: '-o', value: null}, + {name: '-c', value: this.collection.uuid}, + ], []) + .pipe( + getFirstCompletedRemoteData(), + tap((rd) => { + if (rd.hasFailed) { + this.notificationsService.error(this.translateService.get('collection.source.controls.reset.submit.error')); + } else { + this.notificationsService.success(this.translateService.get('collection.source.controls.reset.submit.success')); + } + }), + filter((rd) => rd.hasSucceeded && hasValue(rd.payload)), + switchMap((rd) => this.processDataService.findById(rd.payload.processId, false)), + getAllCompletedRemoteData(), + filter((rd) => !rd.isStale && (rd.hasSucceeded || rd.hasFailed)), + map((rd) => rd.payload), + hasValueOperator(), + ).subscribe((process) => { + if (process.processStatus.toString() !== ProcessStatus[ProcessStatus.COMPLETED].toString() && + process.processStatus.toString() !== ProcessStatus[ProcessStatus.FAILED].toString()) { + // Ping the current process state every 5s + setTimeout(() => { + this.requestService.setStaleByHrefSubstring(process._links.self.href); + this.requestService.setStaleByHrefSubstring(this.collection._links.self.href); + }, 5000); + } + if (process.processStatus.toString() === ProcessStatus[ProcessStatus.FAILED].toString()) { + this.notificationsService.error(this.translateService.get('collection.source.controls.reset.failed')); + } + if (process.processStatus.toString() === ProcessStatus[ProcessStatus.COMPLETED].toString()) { + this.notificationsService.success(this.translateService.get('collection.source.controls.reset.completed')); + this.requestService.setStaleByHrefSubstring(this.collection._links.self.href); + } + } + )); } ngOnDestroy(): void { diff --git a/src/app/core/shared/content-source-set-serializer.spec.ts b/src/app/core/shared/content-source-set-serializer.spec.ts new file mode 100644 index 0000000000..2203481250 --- /dev/null +++ b/src/app/core/shared/content-source-set-serializer.spec.ts @@ -0,0 +1,26 @@ +import { ContentSourceSetSerializer } from './content-source-set-serializer'; + +describe('ContentSourceSetSerializer', () => { + let serializer: ContentSourceSetSerializer; + + beforeEach(() => { + serializer = new ContentSourceSetSerializer(); + }); + + describe('Serialize', () => { + it('should return all when the value is empty', () => { + expect(serializer.Serialize('')).toEqual('all'); + }); + it('should return the value when it is not empty', () => { + expect(serializer.Serialize('test-value')).toEqual('test-value'); + }); + }); + describe('Deserialize', () => { + it('should return an empty value when the value is \'all\'', () => { + expect(serializer.Deserialize('all')).toEqual(''); + }); + it('should return the value when it is not \'all\'', () => { + expect(serializer.Deserialize('test-value')).toEqual('test-value'); + }); + }); +}); diff --git a/src/app/core/shared/content-source-set-serializer.ts b/src/app/core/shared/content-source-set-serializer.ts new file mode 100644 index 0000000000..ec0baec5a6 --- /dev/null +++ b/src/app/core/shared/content-source-set-serializer.ts @@ -0,0 +1,31 @@ +import { isEmpty } from '../../shared/empty.util'; + +/** + * Serializer to create convert the 'all' value supported by the server to an empty string and vice versa. + */ +export class ContentSourceSetSerializer { + + /** + * Method to serialize a setId + * @param {string} setId + * @returns {string} the provided set ID, unless when an empty set ID is provided. In that case, 'all' will be returned. + */ + Serialize(setId: string): any { + if (isEmpty(setId)) { + return 'all'; + } + return setId; + } + + /** + * Method to deserialize a setId + * @param {string} setId + * @returns {string} the provided set ID. When 'all' is provided, an empty set ID will be returned. + */ + Deserialize(setId: string): string { + if (setId === 'all') { + return ''; + } + return setId; + } +} diff --git a/src/app/core/shared/content-source.model.ts b/src/app/core/shared/content-source.model.ts index c98901219d..cb78c85a7c 100644 --- a/src/app/core/shared/content-source.model.ts +++ b/src/app/core/shared/content-source.model.ts @@ -1,4 +1,4 @@ -import { autoserializeAs, deserializeAs, deserialize } from 'cerialize'; +import { autoserializeAs, deserializeAs, deserialize, serializeAs } from 'cerialize'; import { HALLink } from './hal-link.model'; import { MetadataConfig } from './metadata-config.model'; import { CacheableObject } from '../cache/object-cache.reducer'; @@ -6,6 +6,8 @@ import { typedObject } from '../cache/builders/build-decorators'; import { CONTENT_SOURCE } from './content-source.resource-type'; import { excludeFromEquals } from '../utilities/equals.decorators'; import { ResourceType } from './resource-type'; +import { IDToUUIDSerializer } from '../cache/id-to-uuid-serializer'; +import { ContentSourceSetSerializer } from './content-source-set-serializer'; /** * The type of content harvesting used @@ -49,7 +51,8 @@ export class ContentSource extends CacheableObject { /** * OAI Specific set ID */ - @autoserializeAs('oai_set_id') + @deserializeAs(new ContentSourceSetSerializer(), 'oai_set_id') + @serializeAs(new ContentSourceSetSerializer(), 'oai_set_id') oaiSetId: string; /** @@ -70,15 +73,30 @@ export class ContentSource extends CacheableObject { */ metadataConfigs: MetadataConfig[]; + /** + * The current harvest status + */ @autoserializeAs('harvest_status') harvestStatus: string; + /** + * The last's harvest start time + */ @autoserializeAs('harvest_start_time') harvestStartTime: string; + /** + * When the collection was last harvested + */ @autoserializeAs('last_harvested') lastHarvested: string; + /** + * The current harvest message + */ + @autoserializeAs('harvest_message') + message: string; + /** * The {@link HALLink}s for this ContentSource */ diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 8d89561d68..052f0ecc2b 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -891,10 +891,15 @@ "collection.source.controls.import.submit": "Import now", "collection.source.controls.import.failed": "An error occurred during the import", "collection.source.controls.import.completed": "The import completed", + "collection.source.controls.reset.submit.success": "The reset and reimport has been successfully initiated", + "collection.source.controls.reset.submit.error": "Something went wrong with initiating the reset and reimport", + "collection.source.controls.reset.failed": "An error occurred during the reset and reimport", + "collection.source.controls.reset.completed": "The reset and reimport completed", "collection.source.controls.reset.submit": "Reset and reimport", "collection.source.controls.harvest.status": "Harvest status:", "collection.source.controls.harvest.start": "Harvest start time:", "collection.source.controls.harvest.last": "Last time harvested:", + "collection.source.controls.harvest.message": "Harvest info:", "collection.source.controls.harvest.no-information": "N/A", From 7747a8c4e2c05a569e6c8db94d2ddd627f94835d Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Thu, 30 Sep 2021 17:11:34 -0500 Subject: [PATCH 15/25] Attempt to stop scroll to bottom of page when clicking spacebar --- .../expandable-navbar-section.component.html | 2 +- .../expandable-navbar-section.component.ts | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html index 171530ecde..fe31c73066 100644 --- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html +++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html @@ -1,7 +1,7 @@
{ @@ -57,6 +58,7 @@ export class ExpandableNavbarSectionComponent extends NavbarSectionComponent imp * @param {Event} event The user event that triggered this function */ deactivateSection(event): void { + event.preventDefault(); this.windowService.isXsOrSm().pipe( first() ).subscribe((isMobile) => { From ed3bcaa5747950820da8bea2f6bbc2a6e85d8b2e Mon Sep 17 00:00:00 2001 From: Yura Date: Fri, 1 Oct 2021 12:43:37 +0200 Subject: [PATCH 16/25] 83631: Reword & explain theme extension tests --- .../metadata-representation.decorator.spec.ts | 42 ++++++++++++------- .../listable-object.decorator.spec.ts | 42 ++++++++++++------- src/environments/environment.common.ts | 13 ++++++ 3 files changed, 67 insertions(+), 30 deletions(-) diff --git a/src/app/shared/metadata-representation/metadata-representation.decorator.spec.ts b/src/app/shared/metadata-representation/metadata-representation.decorator.spec.ts index a9988aad6f..d6239401d4 100644 --- a/src/app/shared/metadata-representation/metadata-representation.decorator.spec.ts +++ b/src/app/shared/metadata-representation/metadata-representation.decorator.spec.ts @@ -16,8 +16,8 @@ describe('MetadataRepresentation decorator function', () => { const type2 = 'TestType2'; const type3 = 'TestType3'; const type4 = 'RandomType'; - const typeHier1 = 'TestTypeHier1'; - const typeHier2 = 'TestTypeHier2'; + const typeAncestor = 'TestTypeAncestor'; + const typeUnthemed = 'TestTypeUnthemed'; let prefix; /* tslint:disable:max-classes-per-file */ @@ -36,10 +36,10 @@ describe('MetadataRepresentation decorator function', () => { class Test3ItemSubmission { } - class TestHier1Ancestor { + class TestAncestorComponent { } - class TestHier2Unthemed { + class TestUnthemedComponent { } /* tslint:enable:max-classes-per-file */ @@ -58,8 +58,9 @@ describe('MetadataRepresentation decorator function', () => { metadataRepresentationComponent(key + type3, MetadataRepresentationType.Item, Context.Workspace)(Test3ItemSubmission); - metadataRepresentationComponent(key + typeHier1, MetadataRepresentationType.Item, Context.Any, 'ancestor')(TestHier1Ancestor); - metadataRepresentationComponent(key + typeHier2, MetadataRepresentationType.Item, Context.Any)(TestHier2Unthemed); + // Register a metadata representation in the 'ancestor' theme + metadataRepresentationComponent(key + typeAncestor, MetadataRepresentationType.Item, Context.Any, 'ancestor')(TestAncestorComponent); + metadataRepresentationComponent(key + typeUnthemed, MetadataRepresentationType.Item, Context.Any)(TestUnthemedComponent); ogEnvironmentThemes = environment.themes; } @@ -98,22 +99,33 @@ describe('MetadataRepresentation decorator function', () => { }); describe('With theme extensions', () => { + // We're only interested in the cases that the requested theme doesn't match the requested entityType, + // as the cases where it does are already covered by the tests above describe('If requested theme has no match', () => { beforeEach(() => { environment.themes = [ - { name: 'requested', extends: 'intermediate' }, - { name: 'intermediate', extends: 'ancestor' }, + { + name: 'requested', // Doesn't match any entityType + extends: 'intermediate', + }, + { + name: 'intermediate', // Doesn't match any entityType + extends: 'ancestor', + }, + { + name: 'ancestor', // Matches typeAncestor, but not typeUnthemed + } ]; }); - it('should return component from ancestor theme if it has a match', () => { - const component = getMetadataRepresentationComponent(prefix + typeHier1, MetadataRepresentationType.Item, Context.Any, 'requested'); - expect(component).toEqual(TestHier1Ancestor); + it('should return component from the first ancestor theme that matches its entityType', () => { + const component = getMetadataRepresentationComponent(prefix + typeAncestor, MetadataRepresentationType.Item, Context.Any, 'requested'); + expect(component).toEqual(TestAncestorComponent); }); - it('should return default component if ancestor theme has no match', () => { - const component = getMetadataRepresentationComponent(prefix + typeHier2, MetadataRepresentationType.Item, Context.Any, 'requested'); - expect(component).toEqual(TestHier2Unthemed); + it('should return default component if none of the ancestor themes match its entityType', () => { + const component = getMetadataRepresentationComponent(prefix + typeUnthemed, MetadataRepresentationType.Item, Context.Any, 'requested'); + expect(component).toEqual(TestUnthemedComponent); }); }); @@ -129,7 +141,7 @@ describe('MetadataRepresentation decorator function', () => { it('should throw an error', () => { expect(() => { - getMetadataRepresentationComponent(prefix + typeHier1, MetadataRepresentationType.Item, Context.Any, 'extension-cycle'); + getMetadataRepresentationComponent(prefix + typeAncestor, MetadataRepresentationType.Item, Context.Any, 'extension-cycle'); }).toThrowError( 'Theme extension cycle detected: extension-cycle -> broken1 -> broken2 -> broken3 -> broken1' ); diff --git a/src/app/shared/object-collection/shared/listable-object/listable-object.decorator.spec.ts b/src/app/shared/object-collection/shared/listable-object/listable-object.decorator.spec.ts index d3c7a5a1e1..05302c4bd4 100644 --- a/src/app/shared/object-collection/shared/listable-object/listable-object.decorator.spec.ts +++ b/src/app/shared/object-collection/shared/listable-object/listable-object.decorator.spec.ts @@ -10,8 +10,8 @@ describe('ListableObject decorator function', () => { const type1 = 'TestType'; const type2 = 'TestType2'; const type3 = 'TestType3'; - const typeHier1 = 'TestTypeHier1'; - const typeHier2 = 'TestTypeHier2'; + const typeAncestor = 'TestTypeAncestor'; + const typeUnthemed = 'TestTypeUnthemed'; /* tslint:disable:max-classes-per-file */ class Test1List { @@ -32,10 +32,10 @@ describe('ListableObject decorator function', () => { class Test3DetailedSubmission { } - class TestHier1Ancestor { + class TestAncestorComponent { } - class TestHier2Unthemed { + class TestUnthemedComponent { } /* tslint:enable:max-classes-per-file */ @@ -50,8 +50,9 @@ describe('ListableObject decorator function', () => { listableObjectComponent(type3, ViewMode.ListElement)(Test3List); listableObjectComponent(type3, ViewMode.DetailedListElement, Context.Workspace)(Test3DetailedSubmission); - listableObjectComponent(typeHier1, ViewMode.ListElement, Context.Any, 'ancestor')(TestHier1Ancestor); - listableObjectComponent(typeHier2, ViewMode.ListElement, Context.Any)(TestHier2Unthemed); + // Register a metadata representation in the 'ancestor' theme + listableObjectComponent(typeAncestor, ViewMode.ListElement, Context.Any, 'ancestor')(TestAncestorComponent); + listableObjectComponent(typeUnthemed, ViewMode.ListElement, Context.Any)(TestUnthemedComponent); ogEnvironmentThemes = environment.themes; }); @@ -102,22 +103,33 @@ describe('ListableObject decorator function', () => { }); describe('With theme extensions', () => { + // We're only interested in the cases that the requested theme doesn't match the requested objectType, + // as the cases where it does are already covered by the tests above describe('If requested theme has no match', () => { beforeEach(() => { environment.themes = [ - { name: 'requested', extends: 'intermediate' }, - { name: 'intermediate', extends: 'ancestor' }, + { + name: 'requested', // Doesn't match any objectType + extends: 'intermediate', + }, + { + name: 'intermediate', // Doesn't match any objectType + extends: 'ancestor', + }, + { + name: 'ancestor', // Matches typeAncestor, but not typeUnthemed + } ]; }); - it('should return component from ancestor theme if it has a match', () => { - const component = getListableObjectComponent([typeHier1], ViewMode.ListElement, Context.Any, 'requested'); - expect(component).toEqual(TestHier1Ancestor); + it('should return component from the first ancestor theme that matches its objectType', () => { + const component = getListableObjectComponent([typeAncestor], ViewMode.ListElement, Context.Any, 'requested'); + expect(component).toEqual(TestAncestorComponent); }); - it('should return default component if ancestor theme has no match', () => { - const component = getListableObjectComponent([typeHier2], ViewMode.ListElement, Context.Any, 'requested'); - expect(component).toEqual(TestHier2Unthemed); + it('should return default component if none of the ancestor themes match its objectType', () => { + const component = getListableObjectComponent([typeUnthemed], ViewMode.ListElement, Context.Any, 'requested'); + expect(component).toEqual(TestUnthemedComponent); }); }); @@ -133,7 +145,7 @@ describe('ListableObject decorator function', () => { it('should throw an error', () => { expect(() => { - getListableObjectComponent([typeHier1], ViewMode.ListElement, Context.Any, 'extension-cycle'); + getListableObjectComponent([typeAncestor], ViewMode.ListElement, Context.Any, 'extension-cycle'); }).toThrowError( 'Theme extension cycle detected: extension-cycle -> broken1 -> broken2 -> broken3 -> broken1' ); diff --git a/src/environments/environment.common.ts b/src/environments/environment.common.ts index 9091773041..f6fd7f1e68 100644 --- a/src/environments/environment.common.ts +++ b/src/environments/environment.common.ts @@ -265,6 +265,19 @@ export const environment: GlobalConfig = { // uuid: '0958c910-2037-42a9-81c7-dca80e3892b4' // }, // { + // // The extends property specifies an ancestor theme (by name). Whenever a themed component is not found + // // in the current theme, its ancestor theme(s) will be checked recursively before falling back to default. + // name: 'custom-A', + // extends: 'custom-B', + // // Any of the matching properties above can be used + // handle: '10673/34', + // }, + // { + // name: 'custom-B', + // extends: 'custom', + // handle: '10673/12', + // }, + // { // // A theme with only a name will match every route // name: 'custom' // }, From 1d0a5c0e87c42a391acfb6a4f668721e951a5d89 Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Fri, 1 Oct 2021 16:39:17 -0500 Subject: [PATCH 17/25] Switch from keyup to keydown to fix spacebar scrolling issue. --- .../expandable-navbar-section.component.html | 4 ++-- .../expandable-navbar-section.component.spec.ts | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html index fe31c73066..1a86691c11 100644 --- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html +++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html @@ -1,7 +1,7 @@
{ fixture.detectChanges(); const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); - // dispatch the (keyup.enter) action used in our component HTML - sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: 'Enter' })); + // dispatch the (keydown.enter) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' })); }); it('should call activateSection on the menuService', () => { @@ -104,8 +104,8 @@ describe('ExpandableNavbarSectionComponent', () => { fixture.detectChanges(); const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); - // dispatch the (keyup.enter) action used in our component HTML - sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: 'Enter' })); + // dispatch the (keydown.enter) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' })); }); it('should call deactivateSection on the menuService', () => { @@ -122,8 +122,8 @@ describe('ExpandableNavbarSectionComponent', () => { fixture.detectChanges(); const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); - // dispatch the (keyup.space) action used in our component HTML - sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: ' ' })); + // dispatch the (keydown.space) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keydown', { key: ' ' })); }); it('should call activateSection on the menuService', () => { @@ -140,8 +140,8 @@ describe('ExpandableNavbarSectionComponent', () => { fixture.detectChanges(); const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); - // dispatch the (keyup.space) action used in our component HTML - sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: ' ' })); + // dispatch the (keydown.space) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keydown', { key: ' ' })); }); it('should call deactivateSection on the menuService', () => { From b4732f3c31b787e10abd5f82eb9003d6882a25ba Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Mon, 4 Oct 2021 10:05:43 -0500 Subject: [PATCH 18/25] Move actions to keyup while having preventDefault on keydown --- .../expandable-navbar-section.component.html | 5 +++-- .../expandable-navbar-section.component.spec.ts | 16 ++++++++-------- .../expandable-navbar-section.component.ts | 2 -- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html index 1a86691c11..08d1b54fab 100644 --- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html +++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.html @@ -1,7 +1,8 @@
{ fixture.detectChanges(); const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); - // dispatch the (keydown.enter) action used in our component HTML - sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' })); + // dispatch the (keyup.enter) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: 'Enter' })); }); it('should call activateSection on the menuService', () => { @@ -104,8 +104,8 @@ describe('ExpandableNavbarSectionComponent', () => { fixture.detectChanges(); const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); - // dispatch the (keydown.enter) action used in our component HTML - sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' })); + // dispatch the (keyup.enter) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: 'Enter' })); }); it('should call deactivateSection on the menuService', () => { @@ -122,8 +122,8 @@ describe('ExpandableNavbarSectionComponent', () => { fixture.detectChanges(); const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); - // dispatch the (keydown.space) action used in our component HTML - sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keydown', { key: ' ' })); + // dispatch the (keyup.space) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: ' ' })); }); it('should call activateSection on the menuService', () => { @@ -140,8 +140,8 @@ describe('ExpandableNavbarSectionComponent', () => { fixture.detectChanges(); const sidebarToggler = fixture.debugElement.query(By.css('div.nav-item.dropdown')); - // dispatch the (keydown.space) action used in our component HTML - sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keydown', { key: ' ' })); + // dispatch the (keyup.space) action used in our component HTML + sidebarToggler.nativeElement.dispatchEvent(new KeyboardEvent('keyup', { key: ' ' })); }); it('should call deactivateSection on the menuService', () => { diff --git a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.ts b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.ts index cebfa083ff..a0c0240159 100644 --- a/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.ts +++ b/src/app/navbar/expandable-navbar-section/expandable-navbar-section.component.ts @@ -42,7 +42,6 @@ export class ExpandableNavbarSectionComponent extends NavbarSectionComponent imp * @param {Event} event The user event that triggered this function */ activateSection(event): void { - event.preventDefault(); this.windowService.isXsOrSm().pipe( first() ).subscribe((isMobile) => { @@ -58,7 +57,6 @@ export class ExpandableNavbarSectionComponent extends NavbarSectionComponent imp * @param {Event} event The user event that triggered this function */ deactivateSection(event): void { - event.preventDefault(); this.windowService.isXsOrSm().pipe( first() ).subscribe((isMobile) => { From 25ea75cf54c22fc575ffc97ba10999b47d5bc28e Mon Sep 17 00:00:00 2001 From: Yura Date: Mon, 4 Oct 2021 19:00:15 +0200 Subject: [PATCH 19/25] Pause notification countdown on hover --- .../notification/notification.component.html | 2 ++ .../notification/notification.component.ts | 29 ++++++++++++------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/app/shared/notifications/notification/notification.component.html b/src/app/shared/notifications/notification/notification.component.html index e8b3d37b5f..57ac27d42f 100644 --- a/src/app/shared/notifications/notification/notification.component.html +++ b/src/app/shared/notifications/notification/notification.component.html @@ -1,4 +1,6 @@