From e2c2423e53391773cc165c956118bb0b15bc54f4 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 23 Sep 2021 11:27:38 +0200 Subject: [PATCH] 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 {