From b55e4327a88c993535219f6c6793a6888a015c8f Mon Sep 17 00:00:00 2001 From: Art Lowel Date: Fri, 24 Jun 2022 16:03:54 +0200 Subject: [PATCH] fix issue where curation tasks wouldn't work for collections and communities because the handle was in the wrong format --- .../curation-form.component.spec.ts | 26 ++++++++-- .../curation-form/curation-form.component.ts | 15 ++++-- src/app/shared/handle.service.spec.ts | 47 +++++++++++++++++++ src/app/shared/handle.service.ts | 41 ++++++++++++++++ src/assets/i18n/en.json5 | 2 + 5 files changed, 122 insertions(+), 9 deletions(-) create mode 100644 src/app/shared/handle.service.spec.ts create mode 100644 src/app/shared/handle.service.ts diff --git a/src/app/curation-form/curation-form.component.spec.ts b/src/app/curation-form/curation-form.component.spec.ts index 4ff013f77c1..dc70b925e83 100644 --- a/src/app/curation-form/curation-form.component.spec.ts +++ b/src/app/curation-form/curation-form.component.spec.ts @@ -15,6 +15,7 @@ import { By } from '@angular/platform-browser'; import { ConfigurationDataService } from '../core/data/configuration-data.service'; import { ConfigurationProperty } from '../core/shared/configuration-property.model'; import { getProcessDetailRoute } from '../process-page/process-page-routing.paths'; +import { HandleService } from '../shared/handle.service'; describe('CurationFormComponent', () => { let comp: CurationFormComponent; @@ -23,6 +24,7 @@ describe('CurationFormComponent', () => { let scriptDataService: ScriptDataService; let processDataService: ProcessDataService; let configurationDataService: ConfigurationDataService; + let handleService: HandleService; let notificationsService; let router; @@ -51,6 +53,10 @@ describe('CurationFormComponent', () => { })) }); + handleService = { + normalizeHandle: (a) => a + } as any; + notificationsService = new NotificationsServiceStub(); router = new RouterStub(); @@ -58,11 +64,12 @@ describe('CurationFormComponent', () => { imports: [TranslateModule.forRoot(), FormsModule, ReactiveFormsModule], declarations: [CurationFormComponent], providers: [ - {provide: ScriptDataService, useValue: scriptDataService}, - {provide: ProcessDataService, useValue: processDataService}, - {provide: NotificationsService, useValue: notificationsService}, - {provide: Router, useValue: router}, - {provide: ConfigurationDataService, useValue: configurationDataService}, + { provide: ScriptDataService, useValue: scriptDataService }, + { provide: ProcessDataService, useValue: processDataService }, + { provide: NotificationsService, useValue: notificationsService }, + { provide: HandleService, useValue: handleService }, + { provide: Router, useValue: router}, + { provide: ConfigurationDataService, useValue: configurationDataService }, ], schemas: [CUSTOM_ELEMENTS_SCHEMA] }).compileComponents(); @@ -143,4 +150,13 @@ describe('CurationFormComponent', () => { {name: '-i', value: 'all'}, ], []); }); + + it(`should show an error notification and return when an invalid dsoHandle is provided`, () => { + comp.dsoHandle = 'test-handle'; + spyOn(handleService, 'normalizeHandle').and.returnValue(null); + comp.submit(); + + expect(notificationsService.error).toHaveBeenCalled(); + expect(scriptDataService.invoke).not.toHaveBeenCalled(); + }); }); diff --git a/src/app/curation-form/curation-form.component.ts b/src/app/curation-form/curation-form.component.ts index 31501e70d77..422c9550373 100644 --- a/src/app/curation-form/curation-form.component.ts +++ b/src/app/curation-form/curation-form.component.ts @@ -5,7 +5,7 @@ import { getFirstCompletedRemoteData } from '../core/shared/operators'; import { find, map } from 'rxjs/operators'; import { NotificationsService } from '../shared/notifications/notifications.service'; import { TranslateService } from '@ngx-translate/core'; -import { hasValue, isEmpty, isNotEmpty } from '../shared/empty.util'; +import { hasValue, isEmpty, isNotEmpty, hasNoValue } from '../shared/empty.util'; import { RemoteData } from '../core/data/remote-data'; import { Router } from '@angular/router'; import { ProcessDataService } from '../core/data/processes/process-data.service'; @@ -14,9 +14,9 @@ import { ConfigurationDataService } from '../core/data/configuration-data.servic import { ConfigurationProperty } from '../core/shared/configuration-property.model'; import { Observable } from 'rxjs'; import { getProcessDetailRoute } from '../process-page/process-page-routing.paths'; +import { HandleService } from '../shared/handle.service'; export const CURATION_CFG = 'plugin.named.org.dspace.curate.CurationTask'; - /** * Component responsible for rendering the Curation Task form */ @@ -39,6 +39,7 @@ export class CurationFormComponent implements OnInit { private processDataService: ProcessDataService, private notificationsService: NotificationsService, private translateService: TranslateService, + private handleService: HandleService, private router: Router ) { } @@ -76,13 +77,19 @@ export class CurationFormComponent implements OnInit { const taskName = this.form.get('task').value; let handle; if (this.hasHandleValue()) { - handle = this.dsoHandle; + handle = this.handleService.normalizeHandle(this.dsoHandle); + if (isEmpty(handle)) { + this.notificationsService.error(this.translateService.get('curation.form.submit.error.head'), + this.translateService.get('curation.form.submit.error.invalid-handle')); + return; + } } else { - handle = this.form.get('handle').value; + handle = this.handleService.normalizeHandle(this.form.get('handle').value); if (isEmpty(handle)) { handle = 'all'; } } + this.scriptDataService.invoke('curate', [ { name: '-t', value: taskName }, { name: '-i', value: handle }, diff --git a/src/app/shared/handle.service.spec.ts b/src/app/shared/handle.service.spec.ts new file mode 100644 index 00000000000..b326eb04163 --- /dev/null +++ b/src/app/shared/handle.service.spec.ts @@ -0,0 +1,47 @@ +import { HandleService } from './handle.service'; + +describe('HandleService', () => { + let service: HandleService; + + beforeEach(() => { + service = new HandleService(); + }); + + describe(`normalizeHandle`, () => { + it(`should simply return an already normalized handle`, () => { + let input, output; + + input = '123456789/123456'; + output = service.normalizeHandle(input); + expect(output).toEqual(input); + + input = '12.3456.789/123456'; + output = service.normalizeHandle(input); + expect(output).toEqual(input); + }); + + it(`should normalize a handle url`, () => { + let input, output; + + input = 'https://hdl.handle.net/handle/123456789/123456'; + output = service.normalizeHandle(input); + expect(output).toEqual('123456789/123456'); + + 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`, () => { + let input, output; + + input = 'https://hdl.handle.net/handle/123456789'; + output = service.normalizeHandle(input); + expect(output).toBeNull(); + + input = 'something completely different'; + output = service.normalizeHandle(input); + expect(output).toBeNull(); + }); + }); +}); diff --git a/src/app/shared/handle.service.ts b/src/app/shared/handle.service.ts new file mode 100644 index 00000000000..da0f17f7de3 --- /dev/null +++ b/src/app/shared/handle.service.ts @@ -0,0 +1,41 @@ +import { Injectable } from '@angular/core'; +import { isNotEmpty, isEmpty } from './empty.util'; + +const PREFIX_REGEX = /handle\/([^\/]+\/[^\/]+)$/; +const NO_PREFIX_REGEX = /^([^\/]+\/[^\/]+)$/; + +@Injectable({ + providedIn: 'root' +}) +export class HandleService { + + + /** + * Turns a handle string into the default 123456789/12345 format + * + * @param handle the input handle + * + * normalizeHandle('123456789/123456') // '123456789/123456' + * normalizeHandle('12.3456.789/123456') // '12.3456.789/123456' + * normalizeHandle('https://hdl.handle.net/handle/123456789/123456') // '123456789/123456' + * normalizeHandle('https://rest.api/server/handle/123456789/123456') // '123456789/123456' + * normalizeHandle('https://rest.api/server/handle/123456789') // null + */ + normalizeHandle(handle: string): string { + let matches: string[]; + if (isNotEmpty(handle)) { + matches = handle.match(PREFIX_REGEX); + } + + if (isEmpty(matches) || matches.length < 2) { + matches = handle.match(NO_PREFIX_REGEX); + } + + if (isEmpty(matches) || matches.length < 2) { + return null; + } else { + return matches[1]; + } + } + +} diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 3d5f15b4f28..5d7be2a681d 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -1291,6 +1291,8 @@ "curation.form.submit.error.content": "An error occured when trying to start the curation task.", + "curation.form.submit.error.invalid-handle": "Couldn't determine the handle for this object", + "curation.form.handle.label": "Handle:", "curation.form.handle.hint": "Hint: Enter [your-handle-prefix]/0 to run a task across entire site (not all tasks may support this capability)",