107671: Fix handle theme not working with canonical prefix https://hdl.handle.net/

This commit is contained in:
Alexandre Vryghem
2023-10-16 22:16:22 +02:00
parent ff0c1a256d
commit a7faf7d449
10 changed files with 347 additions and 165 deletions

View File

@@ -1,4 +1,3 @@
/* eslint-disable max-classes-per-file */
import { Injectable } from '@angular/core'; import { Injectable } from '@angular/core';
import { Observable } from 'rxjs'; import { Observable } from 'rxjs';
import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service';

View File

@@ -1,4 +1,4 @@
import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing'; import { ComponentFixture, TestBed, waitForAsync, fakeAsync, flush } from '@angular/core/testing';
import { TranslateModule } from '@ngx-translate/core'; import { TranslateModule } from '@ngx-translate/core';
import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core'; import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core';
import { CurationFormComponent } from './curation-form.component'; import { CurationFormComponent } from './curation-form.component';
@@ -16,6 +16,7 @@ import { ConfigurationDataService } from '../core/data/configuration-data.servic
import { ConfigurationProperty } from '../core/shared/configuration-property.model'; import { ConfigurationProperty } from '../core/shared/configuration-property.model';
import { getProcessDetailRoute } from '../process-page/process-page-routing.paths'; import { getProcessDetailRoute } from '../process-page/process-page-routing.paths';
import { HandleService } from '../shared/handle.service'; import { HandleService } from '../shared/handle.service';
import { of as observableOf } from 'rxjs';
describe('CurationFormComponent', () => { describe('CurationFormComponent', () => {
let comp: CurationFormComponent; let comp: CurationFormComponent;
@@ -54,7 +55,7 @@ describe('CurationFormComponent', () => {
}); });
handleService = { handleService = {
normalizeHandle: (a) => a normalizeHandle: (a: string) => observableOf(a),
} as any; } as any;
notificationsService = new NotificationsServiceStub(); notificationsService = new NotificationsServiceStub();
@@ -151,12 +152,13 @@ describe('CurationFormComponent', () => {
], []); ], []);
}); });
it(`should show an error notification and return when an invalid dsoHandle is provided`, () => { it(`should show an error notification and return when an invalid dsoHandle is provided`, fakeAsync(() => {
comp.dsoHandle = 'test-handle'; comp.dsoHandle = 'test-handle';
spyOn(handleService, 'normalizeHandle').and.returnValue(null); spyOn(handleService, 'normalizeHandle').and.returnValue(observableOf(null));
comp.submit(); comp.submit();
flush();
expect(notificationsService.error).toHaveBeenCalled(); expect(notificationsService.error).toHaveBeenCalled();
expect(scriptDataService.invoke).not.toHaveBeenCalled(); expect(scriptDataService.invoke).not.toHaveBeenCalled();
}); }));
}); });

View File

@@ -1,22 +1,22 @@
import { ChangeDetectorRef, Component, Input, OnInit } from '@angular/core'; import { ChangeDetectorRef, Component, Input, OnDestroy, OnInit } from '@angular/core';
import { ScriptDataService } from '../core/data/processes/script-data.service'; import { ScriptDataService } from '../core/data/processes/script-data.service';
import { UntypedFormControl, UntypedFormGroup } from '@angular/forms'; import { UntypedFormControl, UntypedFormGroup } from '@angular/forms';
import { getFirstCompletedRemoteData } from '../core/shared/operators'; import { getFirstCompletedRemoteData, getFirstSucceededRemoteDataPayload } from '../core/shared/operators';
import { find, map } from 'rxjs/operators'; import { map } from 'rxjs/operators';
import { NotificationsService } from '../shared/notifications/notifications.service'; import { NotificationsService } from '../shared/notifications/notifications.service';
import { TranslateService } from '@ngx-translate/core'; import { TranslateService } from '@ngx-translate/core';
import { hasValue, isEmpty, isNotEmpty } from '../shared/empty.util'; import { hasValue, isEmpty, isNotEmpty } from '../shared/empty.util';
import { RemoteData } from '../core/data/remote-data'; import { RemoteData } from '../core/data/remote-data';
import { Router } from '@angular/router'; import { Router } from '@angular/router';
import { ProcessDataService } from '../core/data/processes/process-data.service';
import { Process } from '../process-page/processes/process.model'; import { Process } from '../process-page/processes/process.model';
import { ConfigurationDataService } from '../core/data/configuration-data.service'; import { ConfigurationDataService } from '../core/data/configuration-data.service';
import { ConfigurationProperty } from '../core/shared/configuration-property.model'; import { ConfigurationProperty } from '../core/shared/configuration-property.model';
import { Observable } from 'rxjs'; import { Observable, Subscription } from 'rxjs';
import { getProcessDetailRoute } from '../process-page/process-page-routing.paths'; import { getProcessDetailRoute } from '../process-page/process-page-routing.paths';
import { HandleService } from '../shared/handle.service'; import { HandleService } from '../shared/handle.service';
export const CURATION_CFG = 'plugin.named.org.dspace.curate.CurationTask'; export const CURATION_CFG = 'plugin.named.org.dspace.curate.CurationTask';
/** /**
* Component responsible for rendering the Curation Task form * Component responsible for rendering the Curation Task form
*/ */
@@ -24,7 +24,7 @@ export const CURATION_CFG = 'plugin.named.org.dspace.curate.CurationTask';
selector: 'ds-curation-form', selector: 'ds-curation-form',
templateUrl: './curation-form.component.html' templateUrl: './curation-form.component.html'
}) })
export class CurationFormComponent implements OnInit { export class CurationFormComponent implements OnDestroy, OnInit {
config: Observable<RemoteData<ConfigurationProperty>>; config: Observable<RemoteData<ConfigurationProperty>>;
tasks: string[]; tasks: string[];
@@ -33,10 +33,11 @@ export class CurationFormComponent implements OnInit {
@Input() @Input()
dsoHandle: string; dsoHandle: string;
subs: Subscription[] = [];
constructor( constructor(
private scriptDataService: ScriptDataService, private scriptDataService: ScriptDataService,
private configurationDataService: ConfigurationDataService, private configurationDataService: ConfigurationDataService,
private processDataService: ProcessDataService,
private notificationsService: NotificationsService, private notificationsService: NotificationsService,
private translateService: TranslateService, private translateService: TranslateService,
private handleService: HandleService, private handleService: HandleService,
@@ -45,6 +46,10 @@ export class CurationFormComponent implements OnInit {
) { ) {
} }
ngOnDestroy(): void {
this.subs.forEach((sub: Subscription) => sub.unsubscribe());
}
ngOnInit(): void { ngOnInit(): void {
this.form = new UntypedFormGroup({ this.form = new UntypedFormGroup({
task: new UntypedFormControl(''), task: new UntypedFormControl(''),
@@ -52,16 +57,15 @@ export class CurationFormComponent implements OnInit {
}); });
this.config = this.configurationDataService.findByPropertyName(CURATION_CFG); this.config = this.configurationDataService.findByPropertyName(CURATION_CFG);
this.config.pipe( this.subs.push(this.config.pipe(
find((rd: RemoteData<ConfigurationProperty>) => rd.hasSucceeded), getFirstSucceededRemoteDataPayload(),
map((rd: RemoteData<ConfigurationProperty>) => rd.payload) ).subscribe((configProperties: ConfigurationProperty) => {
).subscribe((configProperties) => {
this.tasks = configProperties.values this.tasks = configProperties.values
.filter((value) => isNotEmpty(value) && value.includes('=')) .filter((value) => isNotEmpty(value) && value.includes('='))
.map((value) => value.split('=')[1].trim()); .map((value) => value.split('=')[1].trim());
this.form.get('task').patchValue(this.tasks[0]); this.form.get('task').patchValue(this.tasks[0]);
this.cdr.detectChanges(); this.cdr.detectChanges();
}); }));
} }
/** /**
@@ -77,33 +81,41 @@ export class CurationFormComponent implements OnInit {
*/ */
submit() { submit() {
const taskName = this.form.get('task').value; const taskName = this.form.get('task').value;
let handle; let handle$: Observable<string | null>;
if (this.hasHandleValue()) { if (this.hasHandleValue()) {
handle = this.handleService.normalizeHandle(this.dsoHandle); handle$ = this.handleService.normalizeHandle(this.dsoHandle).pipe(
if (isEmpty(handle)) { map((handle: string | null) => {
this.notificationsService.error(this.translateService.get('curation.form.submit.error.head'), if (isEmpty(handle)) {
this.translateService.get('curation.form.submit.error.invalid-handle')); this.notificationsService.error(this.translateService.get('curation.form.submit.error.head'),
return; this.translateService.get('curation.form.submit.error.invalid-handle'));
} }
return handle;
}),
);
} else { } else {
handle = this.handleService.normalizeHandle(this.form.get('handle').value); handle$ = this.handleService.normalizeHandle(this.form.get('handle').value).pipe(
if (isEmpty(handle)) { map((handle: string | null) => isEmpty(handle) ? 'all' : handle),
handle = 'all'; );
}
} }
this.scriptDataService.invoke('curate', [ this.subs.push(handle$.subscribe((handle: string) => {
{ name: '-t', value: taskName }, if (hasValue(handle)) {
{ name: '-i', value: handle }, this.subs.push(this.scriptDataService.invoke('curate', [
], []).pipe(getFirstCompletedRemoteData()).subscribe((rd: RemoteData<Process>) => { { name: '-t', value: taskName },
if (rd.hasSucceeded) { { name: '-i', value: handle },
this.notificationsService.success(this.translateService.get('curation.form.submit.success.head'), ], []).pipe(
this.translateService.get('curation.form.submit.success.content')); getFirstCompletedRemoteData(),
this.router.navigateByUrl(getProcessDetailRoute(rd.payload.processId)); ).subscribe((rd: RemoteData<Process>) => {
} else { if (rd.hasSucceeded) {
this.notificationsService.error(this.translateService.get('curation.form.submit.error.head'), this.notificationsService.success(this.translateService.get('curation.form.submit.success.head'),
this.translateService.get('curation.form.submit.error.content')); this.translateService.get('curation.form.submit.success.content'));
void this.router.navigateByUrl(getProcessDetailRoute(rd.payload.processId));
} else {
this.notificationsService.error(this.translateService.get('curation.form.submit.error.head'),
this.translateService.get('curation.form.submit.error.content'));
}
}));
} }
}); }));
} }
} }

View File

@@ -1,47 +1,79 @@
import { HandleService } from './handle.service'; import { HandleService } from './handle.service';
import { TestBed } from '@angular/core/testing';
import { ConfigurationDataServiceStub } from './testing/configuration-data.service.stub';
import { ConfigurationDataService } from '../core/data/configuration-data.service';
import { of as observableOf } from 'rxjs';
describe('HandleService', () => { describe('HandleService', () => {
let service: HandleService; let service: HandleService;
let configurationService: ConfigurationDataServiceStub;
beforeEach(() => { beforeEach(() => {
service = new HandleService(); configurationService = new ConfigurationDataServiceStub();
TestBed.configureTestingModule({
providers: [
{ provide: ConfigurationDataService, useValue: configurationService },
],
});
service = TestBed.inject(HandleService);
}); });
describe(`normalizeHandle`, () => { describe(`normalizeHandle`, () => {
it(`should simply return an already normalized handle`, () => { it('should normalize a handle url with custom conical prefix with trailing slash', (done: DoneFn) => {
let input, output; service.canonicalPrefix$ = observableOf('https://hdl.handle.net/');
input = '123456789/123456'; service.normalizeHandle('https://hdl.handle.net/123456789/123456').subscribe((handle: string | null) => {
output = service.normalizeHandle(input); expect(handle).toBe('123456789/123456');
expect(output).toEqual(input); done();
});
input = '12.3456.789/123456';
output = service.normalizeHandle(input);
expect(output).toEqual(input);
}); });
it(`should normalize a handle url`, () => { it('should normalize a handle url with custom conical prefix without trailing slash', (done: DoneFn) => {
let input, output; service.canonicalPrefix$ = observableOf('https://hdl.handle.net');
input = 'https://hdl.handle.net/handle/123456789/123456'; service.normalizeHandle('https://hdl.handle.net/123456789/123456').subscribe((handle: string | null) => {
output = service.normalizeHandle(input); expect(handle).toBe('123456789/123456');
expect(output).toEqual('123456789/123456'); done();
});
input = 'https://rest.api/server/handle/123456789/123456';
output = service.normalizeHandle(input);
expect(output).toEqual('123456789/123456');
}); });
it(`should return null if the input doesn't contain a handle`, () => { describe('should simply return an already normalized handle', () => {
let input, output; it('123456789/123456', (done: DoneFn) => {
service.normalizeHandle('123456789/123456').subscribe((handle: string | null) => {
expect(handle).toBe('123456789/123456');
done();
});
});
input = 'https://hdl.handle.net/handle/123456789'; it('12.3456.789/123456', (done: DoneFn) => {
output = service.normalizeHandle(input); service.normalizeHandle('12.3456.789/123456').subscribe((handle: string | null) => {
expect(output).toBeNull(); expect(handle).toBe('12.3456.789/123456');
done();
});
});
});
input = 'something completely different'; it('should normalize handle urls starting with handle', (done: DoneFn) => {
output = service.normalizeHandle(input); service.normalizeHandle('https://rest.api/server/handle/123456789/123456').subscribe((handle: string | null) => {
expect(output).toBeNull(); expect(handle).toBe('123456789/123456');
done();
});
});
it('should return null if the input doesn\'t contain a valid handle', (done: DoneFn) => {
service.normalizeHandle('https://hdl.handle.net/123456789').subscribe((handle: string | null) => {
expect(handle).toBeNull();
done();
});
});
it('should return null if the input doesn\'t contain a handle', (done: DoneFn) => {
service.normalizeHandle('something completely different').subscribe((handle: string | null) => {
expect(handle).toBeNull();
done();
});
}); });
}); });
}); });

View File

@@ -1,7 +1,18 @@
import { Injectable } from '@angular/core'; import { Injectable } from '@angular/core';
import { isNotEmpty, isEmpty } from './empty.util'; import { isEmpty, hasNoValue } from './empty.util';
import { ConfigurationDataService } from '../core/data/configuration-data.service';
import { getFirstCompletedRemoteData } from '../core/shared/operators';
import { map, take } from 'rxjs/operators';
import { ConfigurationProperty } from '../core/shared/configuration-property.model';
import { Observable } from 'rxjs';
import { RemoteData } from '../core/data/remote-data';
const PREFIX_REGEX = /handle\/([^\/]+\/[^\/]+)$/; export const CANONICAL_PREFIX_KEY = 'handle.canonical.prefix';
const PREFIX_REGEX = (prefix: string | undefined) => {
const formattedPrefix: string = prefix?.replace(/\/$/, '');
return new RegExp(`(${formattedPrefix ? formattedPrefix + '|' : '' }handle)\/([^\/]+\/[^\/]+)$`);
};
const NO_PREFIX_REGEX = /^([^\/]+\/[^\/]+)$/; const NO_PREFIX_REGEX = /^([^\/]+\/[^\/]+)$/;
@Injectable({ @Injectable({
@@ -9,33 +20,62 @@ const NO_PREFIX_REGEX = /^([^\/]+\/[^\/]+)$/;
}) })
export class HandleService { export class HandleService {
canonicalPrefix$: Observable<string | undefined>;
constructor(
protected configurationService: ConfigurationDataService,
) {
this.canonicalPrefix$ = this.configurationService.findByPropertyName(CANONICAL_PREFIX_KEY).pipe(
getFirstCompletedRemoteData(),
take(1),
map((configurationPropertyRD: RemoteData<ConfigurationProperty>) => {
if (configurationPropertyRD.hasSucceeded) {
return configurationPropertyRD.payload.values.length >= 1 ? configurationPropertyRD.payload.values[0] : undefined;
} else {
return undefined;
}
}),
);
}
/** /**
* Turns a handle string into the default 123456789/12345 format * Turns a handle string into the default 123456789/12345 format
* *
* @param handle the input handle * When the <b>handle.canonical.prefix</b> doesn't end with handle, be sure to expose the variable so that the
* frontend can find the handle
* *
* normalizeHandle('123456789/123456') // '123456789/123456' * @param handle the input handle
* normalizeHandle('12.3456.789/123456') // '12.3456.789/123456' * @return
* normalizeHandle('https://hdl.handle.net/handle/123456789/123456') // '123456789/123456' * <ul>
* normalizeHandle('https://rest.api/server/handle/123456789/123456') // '123456789/123456' * <li>normalizeHandle('123456789/123456') // '123456789/123456'</li>
* normalizeHandle('https://rest.api/server/handle/123456789') // null * <li>normalizeHandle('12.3456.789/123456') // '12.3456.789/123456'</li>
* <li>normalizeHandle('https://hdl.handle.net/123456789/123456') // '123456789/123456'</li>
* <li>normalizeHandle('https://rest.api/server/handle/123456789/123456') // '123456789/123456'</li>
* <li>normalizeHandle('https://rest.api/server/handle/123456789') // null</li>
* </ul>
*/ */
normalizeHandle(handle: string): string { normalizeHandle(handle: string): Observable<string | null> {
let matches: string[]; return this.canonicalPrefix$.pipe(
if (isNotEmpty(handle)) { map((prefix: string | undefined) => {
matches = handle.match(PREFIX_REGEX); let matches: string[];
} if (hasNoValue(handle)) {
return null;
}
if (isEmpty(matches) || matches.length < 2) { matches = handle.match(PREFIX_REGEX(prefix));
matches = handle.match(NO_PREFIX_REGEX);
}
if (isEmpty(matches) || matches.length < 2) { if (isEmpty(matches) || matches.length < 3) {
return null; matches = handle.match(NO_PREFIX_REGEX);
} else { }
return matches[1];
} if (isEmpty(matches) || matches.length < 2) {
return null;
} else {
return matches[matches.length - 1];
}
}),
take(1),
);
} }
} }

View File

@@ -0,0 +1,14 @@
import { Observable } from 'rxjs';
import { RemoteData } from '../../core/data/remote-data';
import { ConfigurationProperty } from '../../core/shared/configuration-property.model';
import { createSuccessfulRemoteDataObject$ } from '../remote-data.utils';
export class ConfigurationDataServiceStub {
findByPropertyName(_name: string): Observable<RemoteData<ConfigurationProperty>> {
const configurationProperty = new ConfigurationProperty();
configurationProperty.values = [];
return createSuccessfulRemoteDataObject$(configurationProperty);
}
}

View File

@@ -24,6 +24,8 @@ import { ROUTER_NAVIGATED } from '@ngrx/router-store';
import { ActivatedRouteSnapshot, Router } from '@angular/router'; import { ActivatedRouteSnapshot, Router } from '@angular/router';
import { CommonModule, DOCUMENT } from '@angular/common'; import { CommonModule, DOCUMENT } from '@angular/common';
import { RouterMock } from '../mocks/router.mock'; import { RouterMock } from '../mocks/router.mock';
import { ConfigurationDataServiceStub } from '../testing/configuration-data.service.stub';
import { ConfigurationDataService } from '../../core/data/configuration-data.service';
/** /**
* LinkService able to mock recursively resolving DSO parent links * LinkService able to mock recursively resolving DSO parent links
@@ -49,6 +51,7 @@ class MockLinkService {
describe('ThemeService', () => { describe('ThemeService', () => {
let themeService: ThemeService; let themeService: ThemeService;
let linkService: LinkService; let linkService: LinkService;
let configurationService: ConfigurationDataServiceStub;
let initialState; let initialState;
let ancestorDSOs: DSpaceObject[]; let ancestorDSOs: DSpaceObject[];
@@ -78,6 +81,7 @@ describe('ThemeService', () => {
currentTheme: 'custom', currentTheme: 'custom',
}, },
}; };
configurationService = new ConfigurationDataServiceStub();
} }
function setupServiceWithActions(mockActions) { function setupServiceWithActions(mockActions) {
@@ -96,6 +100,7 @@ describe('ThemeService', () => {
provideMockActions(() => mockActions), provideMockActions(() => mockActions),
{ provide: DSpaceObjectDataService, useValue: mockDsoService }, { provide: DSpaceObjectDataService, useValue: mockDsoService },
{ provide: Router, useValue: new RouterMock() }, { provide: Router, useValue: new RouterMock() },
{ provide: ConfigurationDataService, useValue: configurationService },
] ]
}); });
@@ -112,7 +117,7 @@ describe('ThemeService', () => {
function spyOnPrivateMethods() { function spyOnPrivateMethods() {
spyOn((themeService as any), 'getAncestorDSOs').and.returnValue(() => observableOf([dso])); spyOn((themeService as any), 'getAncestorDSOs').and.returnValue(() => observableOf([dso]));
spyOn((themeService as any), 'matchThemeToDSOs').and.returnValue(new Theme({ name: 'custom' })); spyOn((themeService as any), 'matchThemeToDSOs').and.returnValue(observableOf(new Theme({ name: 'custom' })));
spyOn((themeService as any), 'getActionForMatch').and.returnValue(new SetThemeAction('custom')); spyOn((themeService as any), 'getActionForMatch').and.returnValue(new SetThemeAction('custom'));
} }
@@ -283,13 +288,13 @@ describe('ThemeService', () => {
beforeEach(() => { beforeEach(() => {
nonMatchingTheme = Object.assign(new Theme({ name: 'non-matching-theme' }), { nonMatchingTheme = Object.assign(new Theme({ name: 'non-matching-theme' }), {
matches: () => false matches: () => observableOf(false),
}); });
itemMatchingTheme = Object.assign(new Theme({ name: 'item-matching-theme' }), { itemMatchingTheme = Object.assign(new Theme({ name: 'item-matching-theme' }), {
matches: (url, dso) => (dso as any).type === ITEM.value matches: (url, dso) => observableOf((dso as any).type === ITEM.value),
}); });
communityMatchingTheme = Object.assign(new Theme({ name: 'community-matching-theme' }), { communityMatchingTheme = Object.assign(new Theme({ name: 'community-matching-theme' }), {
matches: (url, dso) => (dso as any).type === COMMUNITY.value matches: (url, dso) => observableOf((dso as any).type === COMMUNITY.value),
}); });
dsos = [ dsos = [
Object.assign(new Item(), { Object.assign(new Item(), {
@@ -313,8 +318,11 @@ describe('ThemeService', () => {
themeService.themes = themes; themeService.themes = themes;
}); });
it('should return undefined', () => { it('should return undefined', (done: DoneFn) => {
expect((themeService as any).matchThemeToDSOs(dsos, '')).toBeUndefined(); (themeService as any).matchThemeToDSOs(dsos, '').subscribe((theme: Theme) => {
expect(theme).toBeUndefined();
done();
});
}); });
}); });
@@ -324,20 +332,31 @@ describe('ThemeService', () => {
themeService.themes = themes; themeService.themes = themes;
}); });
it('should return the matching theme', () => { it('should return the matching theme', (done: DoneFn) => {
expect((themeService as any).matchThemeToDSOs(dsos, '')).toEqual(itemMatchingTheme); (themeService as any).matchThemeToDSOs(dsos, '').subscribe((theme: Theme) => {
expect(theme).toBe(itemMatchingTheme);
done();
});
}); });
}); });
describe('when multiple themes match some of the DSOs', () => { describe('when multiple themes match some of the DSOs', () => {
it('should return the first matching theme', () => { it('should return the first matching theme (itemMatchingTheme)', (done: DoneFn) => {
themes = [ nonMatchingTheme, itemMatchingTheme, communityMatchingTheme ]; themes = [ nonMatchingTheme, itemMatchingTheme, communityMatchingTheme ];
themeService.themes = themes; themeService.themes = themes;
expect((themeService as any).matchThemeToDSOs(dsos, '')).toEqual(itemMatchingTheme); (themeService as any).matchThemeToDSOs(dsos, '').subscribe((theme: Theme) => {
expect(theme).toBe(itemMatchingTheme);
done();
});
});
it('should return the first matching theme (communityMatchingTheme)', (done: DoneFn) => {
themes = [ nonMatchingTheme, communityMatchingTheme, itemMatchingTheme ]; themes = [ nonMatchingTheme, communityMatchingTheme, itemMatchingTheme ];
themeService.themes = themes; themeService.themes = themes;
expect((themeService as any).matchThemeToDSOs(dsos, '')).toEqual(communityMatchingTheme); (themeService as any).matchThemeToDSOs(dsos, '').subscribe((theme: Theme) => {
expect(theme).toBe(communityMatchingTheme);
done();
});
}); });
}); });
}); });
@@ -382,6 +401,7 @@ describe('ThemeService', () => {
const mockDsoService = { const mockDsoService = {
findById: () => createSuccessfulRemoteDataObject$(mockCommunity) findById: () => createSuccessfulRemoteDataObject$(mockCommunity)
}; };
configurationService = new ConfigurationDataServiceStub();
TestBed.configureTestingModule({ TestBed.configureTestingModule({
imports: [ imports: [
@@ -393,6 +413,7 @@ describe('ThemeService', () => {
provideMockStore({ initialState }), provideMockStore({ initialState }),
{ provide: DSpaceObjectDataService, useValue: mockDsoService }, { provide: DSpaceObjectDataService, useValue: mockDsoService },
{ provide: Router, useValue: new RouterMock() }, { provide: Router, useValue: new RouterMock() },
{ provide: ConfigurationDataService, useValue: configurationService },
] ]
}); });

View File

@@ -1,17 +1,13 @@
import { Injectable, Inject, Injector } from '@angular/core'; import { Injectable, Inject, Injector } from '@angular/core';
import { Store, createFeatureSelector, createSelector, select } from '@ngrx/store'; import { Store, createFeatureSelector, createSelector, select } from '@ngrx/store';
import { BehaviorSubject, EMPTY, Observable, of as observableOf } from 'rxjs'; import { BehaviorSubject, EMPTY, Observable, of as observableOf, from, concatMap } from 'rxjs';
import { ThemeState } from './theme.reducer'; import { ThemeState } from './theme.reducer';
import { SetThemeAction, ThemeActionTypes } from './theme.actions'; import { SetThemeAction, ThemeActionTypes } from './theme.actions';
import { expand, filter, map, switchMap, take, toArray } from 'rxjs/operators'; import { defaultIfEmpty, expand, filter, map, switchMap, take, toArray } from 'rxjs/operators';
import { hasNoValue, hasValue, isNotEmpty } from '../empty.util'; import { hasNoValue, hasValue, isNotEmpty } from '../empty.util';
import { RemoteData } from '../../core/data/remote-data'; import { RemoteData } from '../../core/data/remote-data';
import { DSpaceObject } from '../../core/shared/dspace-object.model'; import { DSpaceObject } from '../../core/shared/dspace-object.model';
import { import { getFirstCompletedRemoteData, getFirstSucceededRemoteData, getRemoteDataPayload } from '../../core/shared/operators';
getFirstCompletedRemoteData,
getFirstSucceededRemoteData,
getRemoteDataPayload
} from '../../core/shared/operators';
import { HeadTagConfig, Theme, ThemeConfig, themeFactory } from '../../../config/theme.model'; import { HeadTagConfig, Theme, ThemeConfig, themeFactory } from '../../../config/theme.model';
import { NO_OP_ACTION_TYPE, NoOpAction } from '../ngrx/no-op.action'; import { NO_OP_ACTION_TYPE, NoOpAction } from '../ngrx/no-op.action';
import { followLink } from '../utils/follow-link-config.model'; import { followLink } from '../utils/follow-link-config.model';
@@ -219,7 +215,7 @@ export class ThemeService {
// create new head tags (not yet added to DOM) // create new head tags (not yet added to DOM)
const headTagFragment = this.document.createDocumentFragment(); const headTagFragment = this.document.createDocumentFragment();
this.createHeadTags(themeName) this.createHeadTags(themeName)
.forEach(newHeadTag => headTagFragment.appendChild(newHeadTag)); .forEach(newHeadTag => headTagFragment.appendChild(newHeadTag));
// add new head tags to DOM // add new head tags to DOM
head.appendChild(headTagFragment); head.appendChild(headTagFragment);
@@ -268,7 +264,7 @@ export class ThemeService {
if (hasValue(headTagConfig.attributes)) { if (hasValue(headTagConfig.attributes)) {
Object.entries(headTagConfig.attributes) Object.entries(headTagConfig.attributes)
.forEach(([key, value]) => tag.setAttribute(key, value)); .forEach(([key, value]) => tag.setAttribute(key, value));
} }
// 'class' attribute should always be 'theme-head-tag' for removal // 'class' attribute should always be 'theme-head-tag' for removal
@@ -292,7 +288,7 @@ export class ThemeService {
// and the current theme from the store // and the current theme from the store
const currentTheme$: Observable<string> = this.store.pipe(select(currentThemeSelector)); const currentTheme$: Observable<string> = this.store.pipe(select(currentThemeSelector));
const action$ = currentTheme$.pipe( const action$: Observable<SetThemeAction | NoOpAction> = currentTheme$.pipe(
switchMap((currentTheme: string) => { switchMap((currentTheme: string) => {
const snapshotWithData = this.findRouteData(activatedRouteSnapshot); const snapshotWithData = this.findRouteData(activatedRouteSnapshot);
if (this.hasDynamicTheme === true && isNotEmpty(this.themes)) { if (this.hasDynamicTheme === true && isNotEmpty(this.themes)) {
@@ -302,8 +298,10 @@ export class ThemeService {
// Start with the resolved dso and go recursively through its parents until you reach the top-level community // Start with the resolved dso and go recursively through its parents until you reach the top-level community
return observableOf(dsoRD.payload).pipe( return observableOf(dsoRD.payload).pipe(
this.getAncestorDSOs(), this.getAncestorDSOs(),
map((dsos: DSpaceObject[]) => { switchMap((dsos: DSpaceObject[]) => {
const dsoMatch = this.matchThemeToDSOs(dsos, currentRouteUrl); return this.matchThemeToDSOs(dsos, currentRouteUrl);
}),
map((dsoMatch: Theme) => {
return this.getActionForMatch(dsoMatch, currentTheme); return this.getActionForMatch(dsoMatch, currentTheme);
}) })
); );
@@ -316,33 +314,41 @@ export class ThemeService {
getFirstSucceededRemoteData(), getFirstSucceededRemoteData(),
getRemoteDataPayload(), getRemoteDataPayload(),
this.getAncestorDSOs(), this.getAncestorDSOs(),
map((dsos: DSpaceObject[]) => { switchMap((dsos: DSpaceObject[]) => {
const dsoMatch = this.matchThemeToDSOs(dsos, currentRouteUrl); return this.matchThemeToDSOs(dsos, currentRouteUrl);
}),
map((dsoMatch: Theme) => {
return this.getActionForMatch(dsoMatch, currentTheme); return this.getActionForMatch(dsoMatch, currentTheme);
}) })
); );
} }
// check whether the route itself matches // check whether the route itself matches
const routeMatch = this.themes.find((theme: Theme) => theme.matches(currentRouteUrl, undefined)); return from(this.themes).pipe(
concatMap((theme: Theme) => theme.matches(currentRouteUrl, undefined).pipe(
return [this.getActionForMatch(routeMatch, currentTheme)]; filter((result: boolean) => result === true),
map(() => theme),
take(1),
)),
take(1),
map((theme: Theme) => this.getActionForMatch(theme, currentTheme))
);
} else {
// If there are no themes configured, do nothing
return observableOf(new NoOpAction());
} }
// If there are no themes configured, do nothing
return [new NoOpAction()];
}), }),
take(1), take(1),
); );
action$.pipe( action$.pipe(
filter((action) => action.type !== NO_OP_ACTION_TYPE), filter((action: SetThemeAction | NoOpAction) => action.type !== NO_OP_ACTION_TYPE),
).subscribe((action) => { ).subscribe((action: SetThemeAction | NoOpAction) => {
this.store.dispatch(action); this.store.dispatch(action);
}); });
return action$.pipe( return action$.pipe(
map((action) => action.type === ThemeActionTypes.SET), map((action: SetThemeAction | NoOpAction) => action.type === ThemeActionTypes.SET),
); );
} }
@@ -433,14 +439,17 @@ export class ThemeService {
* @param currentRouteUrl The url for the current route * @param currentRouteUrl The url for the current route
* @private * @private
*/ */
private matchThemeToDSOs(dsos: DSpaceObject[], currentRouteUrl: string): Theme { private matchThemeToDSOs(dsos: DSpaceObject[], currentRouteUrl: string): Observable<Theme> {
// iterate over the themes in order, and return the first one that matches return from(this.themes).pipe(
return this.themes.find((theme: Theme) => { concatMap((theme: Theme) => from(dsos).pipe(
// iterate over the dsos's in order (most specific one first, so Item, Collection, concatMap((dso: DSpaceObject) => theme.matches(currentRouteUrl, dso)),
// Community), and return the first one that matches the current theme filter((result: boolean) => result === true),
const match = dsos.find((dso: DSpaceObject) => theme.matches(currentRouteUrl, dso)); map(() => theme),
return hasValue(match); take(1),
}); )),
take(1),
defaultIfEmpty(undefined),
);
} }
/** /**

View File

@@ -9,12 +9,15 @@ import { Item } from '../app/core/shared/item.model';
import { ITEM } from '../app/core/shared/item.resource-type'; import { ITEM } from '../app/core/shared/item.resource-type';
import { getItemModuleRoute } from '../app/item-page/item-page-routing-paths'; import { getItemModuleRoute } from '../app/item-page/item-page-routing-paths';
import { HandleService } from '../app/shared/handle.service'; import { HandleService } from '../app/shared/handle.service';
import { TestBed } from '@angular/core/testing';
import { ConfigurationDataService } from '../app/core/data/configuration-data.service';
import { ConfigurationDataServiceStub } from '../app/shared/testing/configuration-data.service.stub';
describe('Theme Models', () => { describe('Theme Models', () => {
let theme: Theme; let theme: Theme;
describe('RegExTheme', () => { describe('RegExTheme', () => {
it('should return true when the regex matches the community\'s DSO route', () => { it('should return true when the regex matches the community\'s DSO route', (done: DoneFn) => {
theme = new RegExTheme({ theme = new RegExTheme({
name: 'community', name: 'community',
regex: getCommunityModuleRoute() + '/.*', regex: getCommunityModuleRoute() + '/.*',
@@ -23,10 +26,13 @@ describe('Theme Models', () => {
type: COMMUNITY.value, type: COMMUNITY.value,
uuid: 'community-uuid', uuid: 'community-uuid',
}); });
expect(theme.matches('', dso)).toEqual(true); theme.matches('', dso).subscribe((matches: boolean) => {
expect(matches).toBeTrue();
done();
});
}); });
it('should return true when the regex matches the collection\'s DSO route', () => { it('should return true when the regex matches the collection\'s DSO route', (done: DoneFn) => {
theme = new RegExTheme({ theme = new RegExTheme({
name: 'collection', name: 'collection',
regex: getCollectionModuleRoute() + '/.*', regex: getCollectionModuleRoute() + '/.*',
@@ -35,10 +41,13 @@ describe('Theme Models', () => {
type: COLLECTION.value, type: COLLECTION.value,
uuid: 'collection-uuid', uuid: 'collection-uuid',
}); });
expect(theme.matches('', dso)).toEqual(true); theme.matches('', dso).subscribe((matches: boolean) => {
expect(matches).toBeTrue();
done();
});
}); });
it('should return true when the regex matches the item\'s DSO route', () => { it('should return true when the regex matches the item\'s DSO route', (done: DoneFn) => {
theme = new RegExTheme({ theme = new RegExTheme({
name: 'item', name: 'item',
regex: getItemModuleRoute() + '/.*', regex: getItemModuleRoute() + '/.*',
@@ -47,32 +56,51 @@ describe('Theme Models', () => {
type: ITEM.value, type: ITEM.value,
uuid: 'item-uuid', uuid: 'item-uuid',
}); });
expect(theme.matches('', dso)).toEqual(true); theme.matches('', dso).subscribe((matches: boolean) => {
expect(matches).toBeTrue();
done();
});
}); });
it('should return true when the regex matches the url', () => { it('should return true when the regex matches the url', (done: DoneFn) => {
theme = new RegExTheme({ theme = new RegExTheme({
name: 'url', name: 'url',
regex: '.*partial.*', regex: '.*partial.*',
}); });
expect(theme.matches('theme/partial/url/match', null)).toEqual(true); theme.matches('theme/partial/url/match', null).subscribe((matches: boolean) => {
expect(matches).toBeTrue();
done();
});
}); });
it('should return false when the regex matches neither the url, nor the DSO route', () => { it('should return false when the regex matches neither the url, nor the DSO route', (done: DoneFn) => {
theme = new RegExTheme({ theme = new RegExTheme({
name: 'no-match', name: 'no-match',
regex: '.*no/match.*', regex: '.*no/match.*',
}); });
expect(theme.matches('theme/partial/url/match', null)).toEqual(false); theme.matches('theme/partial/url/match', null).subscribe((matches: boolean) => {
expect(matches).toBeFalse();
done();
});
}); });
}); });
describe('HandleTheme', () => { describe('HandleTheme', () => {
let handleService; let handleService: HandleService;
let configurationService: ConfigurationDataServiceStub;
beforeEach(() => { beforeEach(() => {
handleService = new HandleService(); configurationService = new ConfigurationDataServiceStub();
TestBed.configureTestingModule({
providers: [
{ provide: ConfigurationDataService, useValue: configurationService },
],
}); });
it('should return true when the DSO\'s handle matches the theme\'s handle', () => { handleService = TestBed.inject(HandleService);
});
it('should return true when the DSO\'s handle matches the theme\'s handle', (done: DoneFn) => {
theme = new HandleTheme({ theme = new HandleTheme({
name: 'matching-handle', name: 'matching-handle',
handle: '1234/5678', handle: '1234/5678',
@@ -82,9 +110,12 @@ describe('Theme Models', () => {
uuid: 'item-uuid', uuid: 'item-uuid',
handle: '1234/5678', handle: '1234/5678',
}, handleService); }, handleService);
expect(theme.matches('', matchingDso)).toEqual(true); theme.matches('', matchingDso).subscribe((matches: boolean) => {
expect(matches).toBeTrue();
done();
});
}); });
it('should return false when the DSO\'s handle contains the theme\'s handle as a subpart', () => { it('should return false when the DSO\'s handle contains the theme\'s handle as a subpart', (done: DoneFn) => {
theme = new HandleTheme({ theme = new HandleTheme({
name: 'matching-handle', name: 'matching-handle',
handle: '1234/5678', handle: '1234/5678',
@@ -94,10 +125,13 @@ describe('Theme Models', () => {
uuid: 'item-uuid', uuid: 'item-uuid',
handle: '1234/567891011', handle: '1234/567891011',
}); });
expect(theme.matches('', dso)).toEqual(false); theme.matches('', dso).subscribe((matches: boolean) => {
expect(matches).toBeFalse();
done();
});
}); });
it('should return false when the handles don\'t match', () => { it('should return false when the handles don\'t match', (done: DoneFn) => {
theme = new HandleTheme({ theme = new HandleTheme({
name: 'no-matching-handle', name: 'no-matching-handle',
handle: '1234/5678', handle: '1234/5678',
@@ -107,12 +141,15 @@ describe('Theme Models', () => {
uuid: 'item-uuid', uuid: 'item-uuid',
handle: '1234/6789', handle: '1234/6789',
}); });
expect(theme.matches('', dso)).toEqual(false); theme.matches('', dso).subscribe((matches: boolean) => {
expect(matches).toBeFalse();
done();
});
}); });
}); });
describe('UUIDTheme', () => { describe('UUIDTheme', () => {
it('should return true when the DSO\'s UUID matches the theme\'s UUID', () => { it('should return true when the DSO\'s UUID matches the theme\'s UUID', (done: DoneFn) => {
theme = new UUIDTheme({ theme = new UUIDTheme({
name: 'matching-uuid', name: 'matching-uuid',
uuid: 'item-uuid', uuid: 'item-uuid',
@@ -121,10 +158,13 @@ describe('Theme Models', () => {
type: ITEM.value, type: ITEM.value,
uuid: 'item-uuid', uuid: 'item-uuid',
}); });
expect(theme.matches('', dso)).toEqual(true); theme.matches('', dso).subscribe((matches: boolean) => {
expect(matches).toBeTrue();
done();
});
}); });
it('should return true when the UUIDs don\'t match', () => { it('should return true when the UUIDs don\'t match', (done: DoneFn) => {
theme = new UUIDTheme({ theme = new UUIDTheme({
name: 'matching-uuid', name: 'matching-uuid',
uuid: 'item-uuid', uuid: 'item-uuid',
@@ -133,7 +173,10 @@ describe('Theme Models', () => {
type: COLLECTION.value, type: COLLECTION.value,
uuid: 'collection-uuid', uuid: 'collection-uuid',
}); });
expect(theme.matches('', dso)).toEqual(false); theme.matches('', dso).subscribe((matches: boolean) => {
expect(matches).toBeFalse();
done();
});
}); });
}); });
}); });

View File

@@ -6,6 +6,8 @@ import { getDSORoute } from '../app/app-routing-paths';
import { HandleObject } from '../app/core/shared/handle-object.model'; import { HandleObject } from '../app/core/shared/handle-object.model';
import { Injector } from '@angular/core'; import { Injector } from '@angular/core';
import { HandleService } from '../app/shared/handle.service'; import { HandleService } from '../app/shared/handle.service';
import { combineLatest, Observable, of as observableOf } from 'rxjs';
import { map, take } from 'rxjs/operators';
export interface NamedThemeConfig extends Config { export interface NamedThemeConfig extends Config {
name: string; name: string;
@@ -55,8 +57,8 @@ export class Theme {
constructor(public config: NamedThemeConfig) { constructor(public config: NamedThemeConfig) {
} }
matches(url: string, dso: DSpaceObject): boolean { matches(url: string, dso: DSpaceObject): Observable<boolean> {
return true; return observableOf(true);
} }
} }
@@ -68,7 +70,7 @@ export class RegExTheme extends Theme {
this.regex = new RegExp(this.config.regex); this.regex = new RegExp(this.config.regex);
} }
matches(url: string, dso: DSpaceObject): boolean { matches(url: string, dso: DSpaceObject): Observable<boolean> {
let match; let match;
const route = getDSORoute(dso); const route = getDSORoute(dso);
@@ -80,25 +82,33 @@ export class RegExTheme extends Theme {
match = url.match(this.regex); match = url.match(this.regex);
} }
return hasValue(match); return observableOf(hasValue(match));
} }
} }
export class HandleTheme extends Theme { export class HandleTheme extends Theme {
private normalizedHandle; private normalizedHandle$: Observable<string | null>;
constructor(public config: HandleThemeConfig, constructor(public config: HandleThemeConfig,
protected handleService: HandleService protected handleService: HandleService
) { ) {
super(config); super(config);
this.normalizedHandle = this.handleService.normalizeHandle(this.config.handle); this.normalizedHandle$ = this.handleService.normalizeHandle(this.config.handle).pipe(
take(1),
);
} }
matches<T extends DSpaceObject & HandleObject>(url: string, dso: T): boolean { matches<T extends DSpaceObject & HandleObject>(url: string, dso: T): Observable<boolean> {
return hasValue(dso) && hasValue(dso.handle) return combineLatest([
&& this.handleService.normalizeHandle(dso.handle) === this.normalizedHandle; this.handleService.normalizeHandle(dso?.handle),
this.normalizedHandle$,
]).pipe(
map(([handle, normalizedHandle]: [string | null, string | null]) => {
return hasValue(dso) && hasValue(dso.handle) && handle === normalizedHandle;
}),
take(1),
);
} }
} }
@@ -107,8 +117,8 @@ export class UUIDTheme extends Theme {
super(config); super(config);
} }
matches(url: string, dso: DSpaceObject): boolean { matches(url: string, dso: DSpaceObject): Observable<boolean> {
return hasValue(dso) && dso.uuid === this.config.uuid; return observableOf(hasValue(dso) && dso.uuid === this.config.uuid);
} }
} }