From af9b4a292e4c289c50fcd96eeabcc49cf3e3045e Mon Sep 17 00:00:00 2001 From: Enea Jahollari Date: Thu, 30 Mar 2023 12:28:48 +0200 Subject: [PATCH 01/11] Fix error message when updating ORCID settings --- .../orcid-sync-settings/orcid-sync-settings.component.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/item-page/orcid-page/orcid-sync-settings/orcid-sync-settings.component.ts b/src/app/item-page/orcid-page/orcid-sync-settings/orcid-sync-settings.component.ts index 494075c0f0..1ed8e95616 100644 --- a/src/app/item-page/orcid-page/orcid-sync-settings/orcid-sync-settings.component.ts +++ b/src/app/item-page/orcid-page/orcid-sync-settings/orcid-sync-settings.component.ts @@ -156,7 +156,8 @@ export class OrcidSyncSettingsComponent implements OnInit { } }), ).subscribe((remoteData: RemoteData) => { - if (remoteData.isSuccess) { + // hasSucceeded is true if the response is success or successStale + if (remoteData.hasSucceeded) { this.notificationsService.success(this.translateService.get(this.messagePrefix + '.synchronization-settings-update.success')); this.settingsUpdated.emit(); } else { From b00a0f5be9b28a4e44d35514503b4cf7acc22ec9 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Wed, 29 Mar 2023 17:56:48 +0200 Subject: [PATCH 02/11] 100553: Added extra regex validation to prevent users from adding namespaces, elements and qualifiers with spaces Minor fixes: - Metadata Registry's name field was not being emptied after successful submission - The first input from both forms both had the red error border after clearing the fields --- .../metadata-schema-form.component.spec.ts | 4 +++- .../metadata-schema-form.component.ts | 11 ++++++++--- .../metadata-field-form.component.spec.ts | 4 +++- .../metadata-field-form.component.ts | 15 +++++++++++++-- src/assets/i18n/en.json5 | 6 ++++++ 5 files changed, 33 insertions(+), 7 deletions(-) diff --git a/src/app/admin/admin-registries/metadata-registry/metadata-schema-form/metadata-schema-form.component.spec.ts b/src/app/admin/admin-registries/metadata-registry/metadata-schema-form/metadata-schema-form.component.spec.ts index 8d416c2df8..98e98a6646 100644 --- a/src/app/admin/admin-registries/metadata-registry/metadata-schema-form/metadata-schema-form.component.spec.ts +++ b/src/app/admin/admin-registries/metadata-registry/metadata-schema-form/metadata-schema-form.component.spec.ts @@ -29,7 +29,9 @@ describe('MetadataSchemaFormComponent', () => { createFormGroup: () => { return { patchValue: () => { - } + }, + markAsUntouched(opts?: any) { + }, }; } }; diff --git a/src/app/admin/admin-registries/metadata-registry/metadata-schema-form/metadata-schema-form.component.ts b/src/app/admin/admin-registries/metadata-registry/metadata-schema-form/metadata-schema-form.component.ts index 5c6885ae72..adb649696c 100644 --- a/src/app/admin/admin-registries/metadata-registry/metadata-schema-form/metadata-schema-form.component.ts +++ b/src/app/admin/admin-registries/metadata-registry/metadata-schema-form/metadata-schema-form.component.ts @@ -77,10 +77,10 @@ export class MetadataSchemaFormComponent implements OnInit, OnDestroy { } ngOnInit() { - combineLatest( + combineLatest([ this.translateService.get(`${this.messagePrefix}.name`), this.translateService.get(`${this.messagePrefix}.namespace`) - ).subscribe(([name, namespace]) => { + ]).subscribe(([name, namespace]) => { this.name = new DynamicInputModel({ id: 'name', label: name, @@ -97,8 +97,12 @@ export class MetadataSchemaFormComponent implements OnInit, OnDestroy { name: 'namespace', validators: { required: null, + pattern: '^[^.]*$', }, required: true, + errorMessages: { + pattern: 'error.validation.metadata.namespace.invalid-pattern', + }, }); this.formModel = [ new DynamicFormGroupModel( @@ -163,9 +167,10 @@ export class MetadataSchemaFormComponent implements OnInit, OnDestroy { * Reset all input-fields to be empty */ clearFields() { + this.formGroup.markAsUntouched(); this.formGroup.patchValue({ metadatadataschemagroup:{ - prefix: '', + name: '', namespace: '' } }); diff --git a/src/app/admin/admin-registries/metadata-schema/metadata-field-form/metadata-field-form.component.spec.ts b/src/app/admin/admin-registries/metadata-schema/metadata-field-form/metadata-field-form.component.spec.ts index e13180d633..fcd8dd395d 100644 --- a/src/app/admin/admin-registries/metadata-schema/metadata-field-form/metadata-field-form.component.spec.ts +++ b/src/app/admin/admin-registries/metadata-schema/metadata-field-form/metadata-field-form.component.spec.ts @@ -39,7 +39,9 @@ describe('MetadataFieldFormComponent', () => { createFormGroup: () => { return { patchValue: () => { - } + }, + markAsUntouched(opts?: any) { + }, }; } }; diff --git a/src/app/admin/admin-registries/metadata-schema/metadata-field-form/metadata-field-form.component.ts b/src/app/admin/admin-registries/metadata-schema/metadata-field-form/metadata-field-form.component.ts index 1c000c3c76..14013f3cb7 100644 --- a/src/app/admin/admin-registries/metadata-schema/metadata-field-form/metadata-field-form.component.ts +++ b/src/app/admin/admin-registries/metadata-schema/metadata-field-form/metadata-field-form.component.ts @@ -98,25 +98,35 @@ export class MetadataFieldFormComponent implements OnInit, OnDestroy { * Initialize the component, setting up the necessary Models for the dynamic form */ ngOnInit() { - combineLatest( + combineLatest([ this.translateService.get(`${this.messagePrefix}.element`), this.translateService.get(`${this.messagePrefix}.qualifier`), this.translateService.get(`${this.messagePrefix}.scopenote`) - ).subscribe(([element, qualifier, scopenote]) => { + ]).subscribe(([element, qualifier, scopenote]) => { this.element = new DynamicInputModel({ id: 'element', label: element, name: 'element', validators: { required: null, + pattern: '^[^.]*$', }, required: true, + errorMessages: { + pattern: 'error.validation.metadata.element.invalid-pattern', + }, }); this.qualifier = new DynamicInputModel({ id: 'qualifier', label: qualifier, name: 'qualifier', + validators: { + pattern: '^[^.]*$', + }, required: false, + errorMessages: { + pattern: 'error.validation.metadata.qualifier.invalid-pattern', + }, }); this.scopeNote = new DynamicInputModel({ id: 'scopeNote', @@ -189,6 +199,7 @@ export class MetadataFieldFormComponent implements OnInit, OnDestroy { * Reset all input-fields to be empty */ clearFields() { + this.formGroup.markAsUntouched(); this.formGroup.patchValue({ metadatadatafieldgroup: { element: '', diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 597f226cc7..c0edb83a3b 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -1478,6 +1478,12 @@ "error.validation.groupExists": "This group already exists", + "error.validation.metadata.namespace.invalid-pattern": "This field cannot contain dots, please use the Element & Qualifier fields instead", + + "error.validation.metadata.element.invalid-pattern": "This field cannot contain dots, please use the Qualifier field instead", + + "error.validation.metadata.qualifier.invalid-pattern": "This field cannot contain dots", + "feed.description": "Syndication feed", From 792a614631dcc6e915ea0301355f06b387a3db4f Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Thu, 30 Mar 2023 17:09:50 +0200 Subject: [PATCH 03/11] 100553: Set the maximum amount of metadata fields shown on EditInPlaceFieldComponent back to 10 --- .../edit-in-place-field/edit-in-place-field.component.html | 2 +- .../edit-in-place-field.component.spec.ts | 2 +- .../edit-in-place-field/edit-in-place-field.component.ts | 6 ++++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.html b/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.html index 46299c1b08..64e863b8bf 100644 --- a/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.html +++ b/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.html @@ -12,7 +12,7 @@ (clickSuggestion)="update(suggestionControl)" (typeSuggestion)="update(suggestionControl)" (dsClickOutside)="checkValidity(suggestionControl)" - (findSuggestions)="findMetadataFieldSuggestions($event)" + (findSuggestions)="findMetadataFieldSuggestions($event, { elementsPerPage: 10 })" #suggestionControl="ngModel" [valid]="(valid | async) !== false" dsAutoFocus autoFocusSelector=".suggestion_input" diff --git a/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.spec.ts b/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.spec.ts index 121ab4580e..2a2ca7ffaa 100644 --- a/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.spec.ts +++ b/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.spec.ts @@ -228,7 +228,7 @@ describe('EditInPlaceFieldComponent', () => { })); it('it should call queryMetadataFields on the metadataFieldService with the correct query', () => { - expect(metadataFieldService.queryMetadataFields).toHaveBeenCalledWith(query, null, true, false, followLink('schema')); + expect(metadataFieldService.queryMetadataFields).toHaveBeenCalledWith(query, {}, true, false, followLink('schema')); }); it('it should set metadataFieldSuggestions to the right value', () => { diff --git a/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.ts b/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.ts index f1ebecb84f..6ed057bc6a 100644 --- a/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.ts +++ b/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.ts @@ -15,6 +15,7 @@ import { InputSuggestion } from '../../../../shared/input-suggestions/input-sugg import { followLink } from '../../../../shared/utils/follow-link-config.model'; import { FieldUpdate } from '../../../../core/data/object-updates/field-update.model'; import { FieldChangeType } from '../../../../core/data/object-updates/field-change-type.model'; +import { FindListOptions } from '../../../../core/data/find-list-options.model'; @Component({ // eslint-disable-next-line @angular-eslint/component-selector @@ -124,10 +125,11 @@ export class EditInPlaceFieldComponent implements OnInit, OnChanges { * Then sets all found metadata fields as metadataFieldSuggestions * Ignores fields from metadata schemas "relation" and "relationship" * @param query The query to look for + * @param options The options that need to be given to the backend */ - findMetadataFieldSuggestions(query: string) { + findMetadataFieldSuggestions(query: string, options: FindListOptions = {}) { if (isNotEmpty(query)) { - return this.registryService.queryMetadataFields(query, null, true, false, followLink('schema')).pipe( + return this.registryService.queryMetadataFields(query, options, true, false, followLink('schema')).pipe( getFirstSucceededRemoteData(), metadataFieldsToString(), ).subscribe((fieldNames: string[]) => { From 24e6cdd3ecbc30b9941f28564f9613be56a37928 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Mon, 3 Apr 2023 14:13:06 +0200 Subject: [PATCH 04/11] 100553: Removed possibility to updated schema name, element and qualifier --- .../metadata-schema-form.component.spec.ts | 27 ++++----- .../metadata-schema-form.component.ts | 49 ++++++++-------- .../metadata-field-form.component.spec.ts | 22 ++++--- .../metadata-field-form.component.ts | 58 +++++++++---------- 4 files changed, 75 insertions(+), 81 deletions(-) diff --git a/src/app/admin/admin-registries/metadata-registry/metadata-schema-form/metadata-schema-form.component.spec.ts b/src/app/admin/admin-registries/metadata-registry/metadata-schema-form/metadata-schema-form.component.spec.ts index 98e98a6646..b758767ddb 100644 --- a/src/app/admin/admin-registries/metadata-registry/metadata-schema-form/metadata-schema-form.component.spec.ts +++ b/src/app/admin/admin-registries/metadata-registry/metadata-schema-form/metadata-schema-form.component.spec.ts @@ -1,5 +1,4 @@ import { ComponentFixture, inject, TestBed, waitForAsync } from '@angular/core/testing'; - import { MetadataSchemaFormComponent } from './metadata-schema-form.component'; import { NO_ERRORS_SCHEMA } from '@angular/core'; import { CommonModule } from '@angular/common'; @@ -30,7 +29,7 @@ describe('MetadataSchemaFormComponent', () => { return { patchValue: () => { }, - markAsUntouched(opts?: any) { + reset(_value?: any, _options?: { onlySelf?: boolean; emitEvent?: boolean; }): void { }, }; } @@ -38,7 +37,7 @@ describe('MetadataSchemaFormComponent', () => { /* eslint-enable no-empty, @typescript-eslint/no-empty-function */ beforeEach(waitForAsync(() => { - TestBed.configureTestingModule({ + return TestBed.configureTestingModule({ imports: [CommonModule, RouterTestingModule.withRoutes([]), TranslateModule.forRoot(), NgbModule], declarations: [MetadataSchemaFormComponent, EnumKeysPipe], providers: [ @@ -66,7 +65,7 @@ describe('MetadataSchemaFormComponent', () => { const expected = Object.assign(new MetadataSchema(), { namespace: namespace, prefix: prefix - }); + } as MetadataSchema); beforeEach(() => { spyOn(component.submitForm, 'emit'); @@ -81,11 +80,10 @@ describe('MetadataSchemaFormComponent', () => { fixture.detectChanges(); }); - it('should emit a new schema using the correct values', waitForAsync(() => { - fixture.whenStable().then(() => { - expect(component.submitForm.emit).toHaveBeenCalledWith(expected); - }); - })); + it('should emit a new schema using the correct values', async () => { + await fixture.whenStable(); + expect(component.submitForm.emit).toHaveBeenCalledWith(expected); + }); }); describe('with an active schema', () => { @@ -93,7 +91,7 @@ describe('MetadataSchemaFormComponent', () => { id: 1, namespace: namespace, prefix: prefix - }); + } as MetadataSchema); beforeEach(() => { spyOn(registryService, 'getActiveMetadataSchema').and.returnValue(observableOf(expectedWithId)); @@ -101,11 +99,10 @@ describe('MetadataSchemaFormComponent', () => { fixture.detectChanges(); }); - it('should edit the existing schema using the correct values', waitForAsync(() => { - fixture.whenStable().then(() => { - expect(component.submitForm.emit).toHaveBeenCalledWith(expectedWithId); - }); - })); + it('should edit the existing schema using the correct values', async () => { + await fixture.whenStable(); + expect(component.submitForm.emit).toHaveBeenCalledWith(expectedWithId); + }); }); }); }); diff --git a/src/app/admin/admin-registries/metadata-registry/metadata-schema-form/metadata-schema-form.component.ts b/src/app/admin/admin-registries/metadata-registry/metadata-schema-form/metadata-schema-form.component.ts index adb649696c..b7c16bc83f 100644 --- a/src/app/admin/admin-registries/metadata-registry/metadata-schema-form/metadata-schema-form.component.ts +++ b/src/app/admin/admin-registries/metadata-registry/metadata-schema-form/metadata-schema-form.component.ts @@ -87,9 +87,12 @@ export class MetadataSchemaFormComponent implements OnInit, OnDestroy { name: 'name', validators: { required: null, - pattern: '^[^ ,_]{1,32}$' + pattern: '^[^. ,_]{1,32}$', }, required: true, + errorMessages: { + pattern: 'error.validation.metadata.namespace.invalid-pattern', + }, }); this.namespace = new DynamicInputModel({ id: 'namespace', @@ -97,12 +100,8 @@ export class MetadataSchemaFormComponent implements OnInit, OnDestroy { name: 'namespace', validators: { required: null, - pattern: '^[^.]*$', }, required: true, - errorMessages: { - pattern: 'error.validation.metadata.namespace.invalid-pattern', - }, }); this.formModel = [ new DynamicFormGroupModel( @@ -112,13 +111,18 @@ export class MetadataSchemaFormComponent implements OnInit, OnDestroy { }) ]; this.formGroup = this.formBuilderService.createFormGroup(this.formModel); - this.registryService.getActiveMetadataSchema().subscribe((schema) => { - this.formGroup.patchValue({ - metadatadataschemagroup:{ - name: schema != null ? schema.prefix : '', - namespace: schema != null ? schema.namespace : '' - } - }); + this.registryService.getActiveMetadataSchema().subscribe((schema: MetadataSchema) => { + if (schema == null) { + this.clearFields(); + } else { + this.formGroup.patchValue({ + metadatadataschemagroup: { + name: schema.prefix, + namespace: schema.namespace, + }, + }); + this.name.disabled = true; + } }); }); } @@ -136,10 +140,10 @@ export class MetadataSchemaFormComponent implements OnInit, OnDestroy { * When the schema has no id attached -> Create new schema * Emit the updated/created schema using the EventEmitter submitForm */ - onSubmit() { + onSubmit(): void { this.registryService.clearMetadataSchemaRequests().subscribe(); this.registryService.getActiveMetadataSchema().pipe(take(1)).subscribe( - (schema) => { + (schema: MetadataSchema) => { const values = { prefix: this.name.value, namespace: this.namespace.value @@ -151,9 +155,9 @@ export class MetadataSchemaFormComponent implements OnInit, OnDestroy { } else { this.registryService.createOrUpdateMetadataSchema(Object.assign(new MetadataSchema(), schema, { id: schema.id, - prefix: (values.prefix ? values.prefix : schema.prefix), - namespace: (values.namespace ? values.namespace : schema.namespace) - })).subscribe((updatedSchema) => { + prefix: schema.prefix, + namespace: values.namespace, + })).subscribe((updatedSchema: MetadataSchema) => { this.submitForm.emit(updatedSchema); }); } @@ -166,14 +170,9 @@ export class MetadataSchemaFormComponent implements OnInit, OnDestroy { /** * Reset all input-fields to be empty */ - clearFields() { - this.formGroup.markAsUntouched(); - this.formGroup.patchValue({ - metadatadataschemagroup:{ - name: '', - namespace: '' - } - }); + clearFields(): void { + this.formGroup.reset('metadatadataschemagroup'); + this.name.disabled = false; } /** diff --git a/src/app/admin/admin-registries/metadata-schema/metadata-field-form/metadata-field-form.component.spec.ts b/src/app/admin/admin-registries/metadata-schema/metadata-field-form/metadata-field-form.component.spec.ts index fcd8dd395d..ad7b54945d 100644 --- a/src/app/admin/admin-registries/metadata-schema/metadata-field-form/metadata-field-form.component.spec.ts +++ b/src/app/admin/admin-registries/metadata-schema/metadata-field-form/metadata-field-form.component.spec.ts @@ -40,7 +40,7 @@ describe('MetadataFieldFormComponent', () => { return { patchValue: () => { }, - markAsUntouched(opts?: any) { + reset(_value?: any, _options?: { onlySelf?: boolean; emitEvent?: boolean; }): void { }, }; } @@ -48,7 +48,7 @@ describe('MetadataFieldFormComponent', () => { /* eslint-enable no-empty, @typescript-eslint/no-empty-function */ beforeEach(waitForAsync(() => { - TestBed.configureTestingModule({ + return TestBed.configureTestingModule({ imports: [CommonModule, RouterTestingModule.withRoutes([]), TranslateModule.forRoot(), NgbModule], declarations: [MetadataFieldFormComponent, EnumKeysPipe], providers: [ @@ -100,11 +100,10 @@ describe('MetadataFieldFormComponent', () => { fixture.detectChanges(); }); - it('should emit a new field using the correct values', waitForAsync(() => { - fixture.whenStable().then(() => { - expect(component.submitForm.emit).toHaveBeenCalledWith(expected); - }); - })); + it('should emit a new field using the correct values', async () => { + await fixture.whenStable(); + expect(component.submitForm.emit).toHaveBeenCalledWith(expected); + }); }); describe('with an active field', () => { @@ -122,11 +121,10 @@ describe('MetadataFieldFormComponent', () => { fixture.detectChanges(); }); - it('should edit the existing field using the correct values', waitForAsync(() => { - fixture.whenStable().then(() => { - expect(component.submitForm.emit).toHaveBeenCalledWith(expectedWithId); - }); - })); + it('should edit the existing field using the correct values', async () => { + await fixture.whenStable(); + expect(component.submitForm.emit).toHaveBeenCalledWith(expectedWithId); + }); }); }); }); diff --git a/src/app/admin/admin-registries/metadata-schema/metadata-field-form/metadata-field-form.component.ts b/src/app/admin/admin-registries/metadata-schema/metadata-field-form/metadata-field-form.component.ts index 14013f3cb7..55950a8773 100644 --- a/src/app/admin/admin-registries/metadata-schema/metadata-field-form/metadata-field-form.component.ts +++ b/src/app/admin/admin-registries/metadata-schema/metadata-field-form/metadata-field-form.component.ts @@ -142,14 +142,20 @@ export class MetadataFieldFormComponent implements OnInit, OnDestroy { }) ]; this.formGroup = this.formBuilderService.createFormGroup(this.formModel); - this.registryService.getActiveMetadataField().subscribe((field) => { - this.formGroup.patchValue({ - metadatadatafieldgroup: { - element: field != null ? field.element : '', - qualifier: field != null ? field.qualifier : '', - scopeNote: field != null ? field.scopeNote : '' - } - }); + this.registryService.getActiveMetadataField().subscribe((field: MetadataField): void => { + if (field == null) { + this.clearFields(); + } else { + this.formGroup.patchValue({ + metadatadatafieldgroup: { + element: field.element, + qualifier: field.qualifier, + scopeNote: field.scopeNote, + }, + }); + this.element.disabled = true; + this.qualifier.disabled = true; + } }); }); } @@ -167,25 +173,24 @@ export class MetadataFieldFormComponent implements OnInit, OnDestroy { * When the field has no id attached -> Create new field * Emit the updated/created field using the EventEmitter submitForm */ - onSubmit() { + onSubmit(): void { this.registryService.getActiveMetadataField().pipe(take(1)).subscribe( - (field) => { - const values = { - element: this.element.value, - qualifier: this.qualifier.value, - scopeNote: this.scopeNote.value - }; + (field: MetadataField) => { if (field == null) { - this.registryService.createMetadataField(Object.assign(new MetadataField(), values), this.metadataSchema).subscribe((newField) => { + this.registryService.createMetadataField(Object.assign(new MetadataField(), { + element: this.element.value, + qualifier: this.qualifier.value, + scopeNote: this.scopeNote.value, + }), this.metadataSchema).subscribe((newField: MetadataField) => { this.submitForm.emit(newField); }); } else { this.registryService.updateMetadataField(Object.assign(new MetadataField(), field, { id: field.id, - element: (values.element ? values.element : field.element), - qualifier: (values.qualifier ? values.qualifier : field.qualifier), - scopeNote: (values.scopeNote ? values.scopeNote : field.scopeNote) - })).subscribe((updatedField) => { + element: field.element, + qualifier: field.qualifier, + scopeNote: this.scopeNote.value, + })).subscribe((updatedField: MetadataField) => { this.submitForm.emit(updatedField); }); } @@ -198,15 +203,10 @@ export class MetadataFieldFormComponent implements OnInit, OnDestroy { /** * Reset all input-fields to be empty */ - clearFields() { - this.formGroup.markAsUntouched(); - this.formGroup.patchValue({ - metadatadatafieldgroup: { - element: '', - qualifier: '', - scopeNote: '' - } - }); + clearFields(): void { + this.formGroup.reset('metadatadatafieldgroup'); + this.element.disabled = false; + this.qualifier.disabled = false; } /** From 89eb4e3cb2a8a07f45ee939f74387f1624c4e0c5 Mon Sep 17 00:00:00 2001 From: Enea Jahollari Date: Wed, 3 May 2023 15:29:12 +0200 Subject: [PATCH 05/11] [DURACOM-131] Show output files in separate lines --- src/app/process-page/detail/process-detail.component.html | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/app/process-page/detail/process-detail.component.html b/src/app/process-page/detail/process-detail.component.html index 29cbfc113f..8825c71cee 100644 --- a/src/app/process-page/detail/process-detail.component.html +++ b/src/app/process-page/detail/process-detail.component.html @@ -17,10 +17,12 @@
+
- {{getFileName(file)}} - ({{(file?.sizeBytes) | dsFileSize }}) + {{getFileName(file)}} + ({{(file?.sizeBytes) | dsFileSize }}) +
From c066bc9d54e7ea0c9256ffedc7bb02b4ff2ee6b9 Mon Sep 17 00:00:00 2001 From: Enea Jahollari Date: Wed, 3 May 2023 17:22:20 +0200 Subject: [PATCH 06/11] [DURACOM-131] Added automatic refresh every 5 seconds if the status is running --- .../detail/process-detail.component.html | 19 ++-- .../detail/process-detail.component.ts | 102 ++++++++++++++---- 2 files changed, 93 insertions(+), 28 deletions(-) diff --git a/src/app/process-page/detail/process-detail.component.html b/src/app/process-page/detail/process-detail.component.html index 8825c71cee..9e7a24d3af 100644 --- a/src/app/process-page/detail/process-detail.component.html +++ b/src/app/process-page/detail/process-detail.component.html @@ -1,10 +1,15 @@ -
-
-

{{'process.detail.title' | translate:{ - id: process?.processId, - name: process?.scriptName - } }}

+
+
+
+

+ {{ 'process.detail.title' | translate:{ id: process?.processId, name: process?.scriptName } }} +

+
+
+ Refreshing in {{ seconds }}s +
+
{{ process?.scriptName }}
 @@ -72,7 +77,7 @@ -
+
diff --git a/src/app/process-page/detail/process-detail.component.ts b/src/app/process-page/detail/process-detail.component.ts index 2e7dea5def..01ec3df19c 100644 --- a/src/app/process-page/detail/process-detail.component.ts +++ b/src/app/process-page/detail/process-detail.component.ts @@ -1,7 +1,7 @@ import { HttpClient } from '@angular/common/http'; -import { Component, NgZone, OnInit } from '@angular/core'; +import { Component, NgZone, OnDestroy, OnInit } from '@angular/core'; import { ActivatedRoute, Router } from '@angular/router'; -import { BehaviorSubject, Observable } from 'rxjs'; +import { BehaviorSubject, interval, Observable, shareReplay, Subscription } from 'rxjs'; import { finalize, map, switchMap, take, tap } from 'rxjs/operators'; import { AuthService } from '../../core/auth/auth.service'; import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; @@ -26,6 +26,7 @@ import { NgbModal, NgbModalRef } from '@ng-bootstrap/ng-bootstrap'; import { getProcessListRoute } from '../process-page-routing.paths'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { TranslateService } from '@ngx-translate/core'; +import { followLink } from '../../shared/utils/follow-link-config.model'; @Component({ selector: 'ds-process-detail', @@ -34,7 +35,7 @@ import { TranslateService } from '@ngx-translate/core'; /** * A component displaying detailed information about a DSpace Process */ -export class ProcessDetailComponent implements OnInit { +export class ProcessDetailComponent implements OnInit, OnDestroy { /** * The AlertType enumeration @@ -65,48 +66,55 @@ export class ProcessDetailComponent implements OnInit { /** * Boolean on whether or not to show the output logs */ - showOutputLogs; + showOutputLogs = false; /** * When it's retrieving the output logs from backend, to show loading component */ - retrievingOutputLogs$: BehaviorSubject; + retrievingOutputLogs$ = new BehaviorSubject(false); /** * Date format to use for start and end time of processes */ dateFormat = 'yyyy-MM-dd HH:mm:ss ZZZZ'; + refreshCounter$ = new BehaviorSubject(0); + /** * Reference to NgbModal */ protected modalRef: NgbModalRef; - constructor(protected route: ActivatedRoute, - protected router: Router, - protected processService: ProcessDataService, - protected bitstreamDataService: BitstreamDataService, - protected nameService: DSONameService, - private zone: NgZone, - protected authService: AuthService, - protected http: HttpClient, - protected modalService: NgbModal, - protected notificationsService: NotificationsService, - protected translateService: TranslateService - ) { - } + private refreshTimerSub?: Subscription; + + constructor( + protected route: ActivatedRoute, + protected router: Router, + protected processService: ProcessDataService, + protected bitstreamDataService: BitstreamDataService, + protected nameService: DSONameService, + private zone: NgZone, + protected authService: AuthService, + protected http: HttpClient, + protected modalService: NgbModal, + protected notificationsService: NotificationsService, + protected translateService: TranslateService + ) {} /** * Initialize component properties * Display a 404 if the process doesn't exist */ ngOnInit(): void { - this.showOutputLogs = false; - this.retrievingOutputLogs$ = new BehaviorSubject(false); this.processRD$ = this.route.data.pipe( map((data) => { + if (!this.isProcessFinished(data.process.payload)) { + this.startRefreshTimer(); + } + return data.process as RemoteData; }), - redirectOn4xx(this.router, this.authService) + redirectOn4xx(this.router, this.authService), + shareReplay(1) ); this.filesRD$ = this.processRD$.pipe( @@ -115,6 +123,54 @@ export class ProcessDetailComponent implements OnInit { ); } + refresh() { + this.processRD$ = this.processService.findById( + this.route.snapshot.params.id, + false, + true, + followLink('script') + ).pipe( + getFirstSucceededRemoteData(), + redirectOn4xx(this.router, this.authService), + tap((processRemoteData: RemoteData) => { + console.log('refresh', processRemoteData); + if (!this.isProcessFinished(processRemoteData.payload)) { + this.startRefreshTimer(); + } + }), + shareReplay(1) + ); + + this.filesRD$ = this.processRD$.pipe( + getFirstSucceededRemoteDataPayload(), + switchMap((process: Process) => this.processService.getFiles(process.processId)) + ); + } + + startRefreshTimer() { + this.refreshCounter$.next(0); + + this.refreshTimerSub = interval(1000).subscribe( + value => { + if (value > 5) { + setTimeout(() => { + this.refresh(); + this.stopRefreshTimer(); + this.refreshCounter$.next(0); + }, 1); + } else { + this.refreshCounter$.next(5 - value); + } + }); + } + + stopRefreshTimer() { + if (hasValue(this.refreshTimerSub)) { + this.refreshTimerSub.unsubscribe(); + this.refreshTimerSub = undefined; + } + } + /** * Get the name of a bitstream * @param bitstream @@ -210,6 +266,7 @@ export class ProcessDetailComponent implements OnInit { openDeleteModal(content) { this.modalRef = this.modalService.open(content); } + /** * Close the modal. */ @@ -217,4 +274,7 @@ export class ProcessDetailComponent implements OnInit { this.modalRef.close(); } + ngOnDestroy(): void { + this.stopRefreshTimer(); + } } From efedd10d9c12da1909854d6661d2db264748b9ac Mon Sep 17 00:00:00 2001 From: Enea Jahollari Date: Wed, 3 May 2023 17:25:25 +0200 Subject: [PATCH 07/11] [DURACOM-131] Add check to run the refresh only on browser --- .../process-page/detail/process-detail.component.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/app/process-page/detail/process-detail.component.ts b/src/app/process-page/detail/process-detail.component.ts index 01ec3df19c..a379dfe337 100644 --- a/src/app/process-page/detail/process-detail.component.ts +++ b/src/app/process-page/detail/process-detail.component.ts @@ -1,5 +1,5 @@ import { HttpClient } from '@angular/common/http'; -import { Component, NgZone, OnDestroy, OnInit } from '@angular/core'; +import { Component, Inject, NgZone, OnDestroy, OnInit, PLATFORM_ID } from '@angular/core'; import { ActivatedRoute, Router } from '@angular/router'; import { BehaviorSubject, interval, Observable, shareReplay, Subscription } from 'rxjs'; import { finalize, map, switchMap, take, tap } from 'rxjs/operators'; @@ -27,6 +27,7 @@ import { getProcessListRoute } from '../process-page-routing.paths'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { TranslateService } from '@ngx-translate/core'; import { followLink } from '../../shared/utils/follow-link-config.model'; +import { isPlatformBrowser } from '@angular/common'; @Component({ selector: 'ds-process-detail', @@ -87,6 +88,7 @@ export class ProcessDetailComponent implements OnInit, OnDestroy { private refreshTimerSub?: Subscription; constructor( + @Inject(PLATFORM_ID) protected platformId: object, protected route: ActivatedRoute, protected router: Router, protected processService: ProcessDataService, @@ -107,8 +109,10 @@ export class ProcessDetailComponent implements OnInit, OnDestroy { ngOnInit(): void { this.processRD$ = this.route.data.pipe( map((data) => { - if (!this.isProcessFinished(data.process.payload)) { - this.startRefreshTimer(); + if (isPlatformBrowser(this.platformId)) { + if (!this.isProcessFinished(data.process.payload)) { + this.startRefreshTimer(); + } } return data.process as RemoteData; @@ -133,7 +137,6 @@ export class ProcessDetailComponent implements OnInit, OnDestroy { getFirstSucceededRemoteData(), redirectOn4xx(this.router, this.authService), tap((processRemoteData: RemoteData) => { - console.log('refresh', processRemoteData); if (!this.isProcessFinished(processRemoteData.payload)) { this.startRefreshTimer(); } From 091d0fccf7121f1cfce2e7f98dfdec8d2e6e5528 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Fri, 5 May 2023 17:45:00 +0100 Subject: [PATCH 08/11] 100553: Explicitly request EditInPlaceFieldComponent's metadata field suggestions with the sort by fieldName,ASC --- .../edit-in-place-field/edit-in-place-field.component.html | 2 +- .../edit-in-place-field/edit-in-place-field.component.ts | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.html b/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.html index 64e863b8bf..c6b3a62007 100644 --- a/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.html +++ b/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.html @@ -12,7 +12,7 @@ (clickSuggestion)="update(suggestionControl)" (typeSuggestion)="update(suggestionControl)" (dsClickOutside)="checkValidity(suggestionControl)" - (findSuggestions)="findMetadataFieldSuggestions($event, { elementsPerPage: 10 })" + (findSuggestions)="findMetadataFieldSuggestions($event, { elementsPerPage: 10, sort: defaultMetadataSort })" #suggestionControl="ngModel" [valid]="(valid | async) !== false" dsAutoFocus autoFocusSelector=".suggestion_input" diff --git a/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.ts b/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.ts index 6ed057bc6a..f069e3edfc 100644 --- a/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.ts +++ b/src/app/item-page/edit-item-page/item-metadata/edit-in-place-field/edit-in-place-field.component.ts @@ -16,6 +16,7 @@ import { followLink } from '../../../../shared/utils/follow-link-config.model'; import { FieldUpdate } from '../../../../core/data/object-updates/field-update.model'; import { FieldChangeType } from '../../../../core/data/object-updates/field-change-type.model'; import { FindListOptions } from '../../../../core/data/find-list-options.model'; +import { SortDirection, SortOptions } from '../../../../core/cache/models/sort-options.model'; @Component({ // eslint-disable-next-line @angular-eslint/component-selector @@ -57,6 +58,8 @@ export class EditInPlaceFieldComponent implements OnInit, OnChanges { */ metadataFieldSuggestions: BehaviorSubject = new BehaviorSubject([]); + defaultMetadataSort: SortOptions = new SortOptions('fieldName', SortDirection.ASC); + constructor( private registryService: RegistryService, private objectUpdatesService: ObjectUpdatesService, From dbccda4330eb7ece591cfab4795bfa75e786e958 Mon Sep 17 00:00:00 2001 From: Enea Jahollari Date: Tue, 6 Jun 2023 14:13:13 +0200 Subject: [PATCH 09/11] rollback change for orcid sync settings --- .../orcid-sync-settings/orcid-sync-settings.component.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/app/item-page/orcid-page/orcid-sync-settings/orcid-sync-settings.component.ts b/src/app/item-page/orcid-page/orcid-sync-settings/orcid-sync-settings.component.ts index 1aec416d62..0bcbc295ac 100644 --- a/src/app/item-page/orcid-page/orcid-sync-settings/orcid-sync-settings.component.ts +++ b/src/app/item-page/orcid-page/orcid-sync-settings/orcid-sync-settings.component.ts @@ -156,8 +156,7 @@ export class OrcidSyncSettingsComponent implements OnInit { } }), ).subscribe((remoteData: RemoteData) => { - // hasSucceeded is true if the response is success or successStale - if (remoteData.hasSucceeded) { + if (remoteData.isSuccess) { this.notificationsService.success(this.translateService.get(this.messagePrefix + '.synchronization-settings-update.success')); this.settingsUpdated.emit(); } else { From 3fdef20dc2555d1760e53bdbfe4c37d41055f64e Mon Sep 17 00:00:00 2001 From: Enea Jahollari Date: Tue, 6 Jun 2023 15:54:57 +0200 Subject: [PATCH 10/11] added unit testing --- .../detail/process-detail.component.html | 2 +- .../detail/process-detail.component.spec.ts | 100 +++++++++++++++++- 2 files changed, 100 insertions(+), 2 deletions(-) diff --git a/src/app/process-page/detail/process-detail.component.html b/src/app/process-page/detail/process-detail.component.html index 9e7a24d3af..5f905cbfff 100644 --- a/src/app/process-page/detail/process-detail.component.html +++ b/src/app/process-page/detail/process-detail.component.html @@ -5,7 +5,7 @@ {{ 'process.detail.title' | translate:{ id: process?.processId, name: process?.scriptName } }}
-
+
Refreshing in {{ seconds }}s
diff --git a/src/app/process-page/detail/process-detail.component.spec.ts b/src/app/process-page/detail/process-detail.component.spec.ts index 8749553eae..9552f9a092 100644 --- a/src/app/process-page/detail/process-detail.component.spec.ts +++ b/src/app/process-page/detail/process-detail.component.spec.ts @@ -35,6 +35,7 @@ import { NotificationsServiceStub } from '../../shared/testing/notifications-ser import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { getProcessListRoute } from '../process-page-routing.paths'; +import {ProcessStatus} from '../processes/process-status.model'; describe('ProcessDetailComponent', () => { let component: ProcessDetailComponent; @@ -44,6 +45,7 @@ describe('ProcessDetailComponent', () => { let nameService: DSONameService; let bitstreamDataService: BitstreamDataService; let httpClient: HttpClient; + let route: ActivatedRoute; let process: Process; let fileName: string; @@ -106,7 +108,8 @@ describe('ProcessDetailComponent', () => { }); processService = jasmine.createSpyObj('processService', { getFiles: createSuccessfulRemoteDataObject$(createPaginatedList(files)), - delete: createSuccessfulRemoteDataObject$(null) + delete: createSuccessfulRemoteDataObject$(null), + findById: createSuccessfulRemoteDataObject$(process), }); bitstreamDataService = jasmine.createSpyObj('bitstreamDataService', { findByHref: createSuccessfulRemoteDataObject$(logBitstream) @@ -127,6 +130,13 @@ describe('ProcessDetailComponent', () => { router = jasmine.createSpyObj('router', { navigateByUrl:{} }); + + route = jasmine.createSpyObj('route', { + data: observableOf({ process: createSuccessfulRemoteDataObject(process) }), + snapshot: { + params: { id: process.processId } + } + }); } beforeEach(waitForAsync(() => { @@ -263,4 +273,92 @@ describe('ProcessDetailComponent', () => { }); }); + describe('refresh counter', () => { + const queryRefreshCounter = () => fixture.debugElement.query(By.css('.refresh-counter')); + + describe('if process is completed', () => { + beforeEach(() => { + process.processStatus = ProcessStatus.COMPLETED; + route.data = observableOf({process: createSuccessfulRemoteDataObject(process)}); + }); + + it('should not show', () => { + spyOn(component, 'startRefreshTimer'); + + const refreshCounter = queryRefreshCounter(); + expect(refreshCounter).toBeNull(); + + expect(component.startRefreshTimer).not.toHaveBeenCalled(); + }); + }); + + describe('if process is not finished', () => { + beforeEach(() => { + process.processStatus = ProcessStatus.RUNNING; + route.data = observableOf({process: createSuccessfulRemoteDataObject(process)}); + fixture.detectChanges(); + component.stopRefreshTimer(); + }); + + it('should call startRefreshTimer', () => { + spyOn(component, 'startRefreshTimer'); + + component.ngOnInit(); + fixture.detectChanges(); // subscribe to process observable with async pipe + + expect(component.startRefreshTimer).toHaveBeenCalled(); + }); + + it('should call refresh method every 5 seconds, until process is completed', fakeAsync(() => { + spyOn(component, 'refresh'); + spyOn(component, 'stopRefreshTimer'); + + process.processStatus = ProcessStatus.COMPLETED; + // set findbyId to return a completed process + (processService.findById as jasmine.Spy).and.returnValue(observableOf(createSuccessfulRemoteDataObject(process))); + + component.ngOnInit(); + fixture.detectChanges(); // subscribe to process observable with async pipe + + expect(component.refresh).not.toHaveBeenCalled(); + + expect(component.refreshCounter$.value).toBe(0); + + tick(1001); // 1 second + 1 ms by the setTimeout + expect(component.refreshCounter$.value).toBe(5); // 5 - 0 + + tick(2001); // 2 seconds + 1 ms by the setTimeout + expect(component.refreshCounter$.value).toBe(3); // 5 - 2 + + tick(2001); // 2 seconds + 1 ms by the setTimeout + expect(component.refreshCounter$.value).toBe(1); // 3 - 2 + + tick(1001); // 1 second + 1 ms by the setTimeout + expect(component.refreshCounter$.value).toBe(0); // 1 - 1 + + tick(1000); // 1 second + + expect(component.refresh).toHaveBeenCalledTimes(1); + expect(component.stopRefreshTimer).toHaveBeenCalled(); + + expect(component.refreshCounter$.value).toBe(0); + + tick(1001); // 1 second + 1 ms by the setTimeout + // startRefreshTimer not called again + expect(component.refreshCounter$.value).toBe(0); + + discardPeriodicTasks(); // discard any periodic tasks that have not yet executed + })); + + it('should show if refreshCounter is different from 0', () => { + component.refreshCounter$.next(1); + fixture.detectChanges(); + + const refreshCounter = queryRefreshCounter(); + expect(refreshCounter).not.toBeNull(); + }); + + }); + + }); }); From 755451191c0dbb571f0af1ad4210518724702325 Mon Sep 17 00:00:00 2001 From: Alexandre Vryghem Date: Tue, 6 Jun 2023 18:27:07 +0200 Subject: [PATCH 11/11] 100553: Updated form validation errors from metadata schema and field form --- .../metadata-schema-form.component.ts | 10 ++++++++-- .../metadata-field-form.component.ts | 8 ++++++-- src/assets/i18n/en.json5 | 13 ++++++++++--- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/app/admin/admin-registries/metadata-registry/metadata-schema-form/metadata-schema-form.component.ts b/src/app/admin/admin-registries/metadata-registry/metadata-schema-form/metadata-schema-form.component.ts index b7c16bc83f..3be4d80fa9 100644 --- a/src/app/admin/admin-registries/metadata-registry/metadata-schema-form/metadata-schema-form.component.ts +++ b/src/app/admin/admin-registries/metadata-registry/metadata-schema-form/metadata-schema-form.component.ts @@ -87,11 +87,13 @@ export class MetadataSchemaFormComponent implements OnInit, OnDestroy { name: 'name', validators: { required: null, - pattern: '^[^. ,_]{1,32}$', + pattern: '^[^. ,]*$', + maxLength: 32, }, required: true, errorMessages: { - pattern: 'error.validation.metadata.namespace.invalid-pattern', + pattern: 'error.validation.metadata.name.invalid-pattern', + maxLength: 'error.validation.metadata.name.max-length', }, }); this.namespace = new DynamicInputModel({ @@ -100,8 +102,12 @@ export class MetadataSchemaFormComponent implements OnInit, OnDestroy { name: 'namespace', validators: { required: null, + maxLength: 256, }, required: true, + errorMessages: { + maxLength: 'error.validation.metadata.namespace.max-length', + }, }); this.formModel = [ new DynamicFormGroupModel( diff --git a/src/app/admin/admin-registries/metadata-schema/metadata-field-form/metadata-field-form.component.ts b/src/app/admin/admin-registries/metadata-schema/metadata-field-form/metadata-field-form.component.ts index 55950a8773..df8373e8d2 100644 --- a/src/app/admin/admin-registries/metadata-schema/metadata-field-form/metadata-field-form.component.ts +++ b/src/app/admin/admin-registries/metadata-schema/metadata-field-form/metadata-field-form.component.ts @@ -109,11 +109,13 @@ export class MetadataFieldFormComponent implements OnInit, OnDestroy { name: 'element', validators: { required: null, - pattern: '^[^.]*$', + pattern: '^[^. ,]*$', + maxLength: 64, }, required: true, errorMessages: { pattern: 'error.validation.metadata.element.invalid-pattern', + maxLength: 'error.validation.metadata.element.max-length', }, }); this.qualifier = new DynamicInputModel({ @@ -121,11 +123,13 @@ export class MetadataFieldFormComponent implements OnInit, OnDestroy { label: qualifier, name: 'qualifier', validators: { - pattern: '^[^.]*$', + pattern: '^[^. ,]*$', + maxLength: 64, }, required: false, errorMessages: { pattern: 'error.validation.metadata.qualifier.invalid-pattern', + maxLength: 'error.validation.metadata.qualifier.max-length', }, }); this.scopeNote = new DynamicInputModel({ diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index c0edb83a3b..55c788ffac 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -1478,12 +1478,19 @@ "error.validation.groupExists": "This group already exists", - "error.validation.metadata.namespace.invalid-pattern": "This field cannot contain dots, please use the Element & Qualifier fields instead", + "error.validation.metadata.name.invalid-pattern": "This field cannot contain dots, commas or spaces. Please use the Element & Qualifier fields instead", - "error.validation.metadata.element.invalid-pattern": "This field cannot contain dots, please use the Qualifier field instead", + "error.validation.metadata.name.max-length": "This field may not contain more than 32 characters", - "error.validation.metadata.qualifier.invalid-pattern": "This field cannot contain dots", + "error.validation.metadata.namespace.max-length": "This field may not contain more than 256 characters", + "error.validation.metadata.element.invalid-pattern": "This field cannot contain dots, commas or spaces. Please use the Qualifier field instead", + + "error.validation.metadata.element.max-length": "This field may not contain more than 64 characters", + + "error.validation.metadata.qualifier.invalid-pattern": "This field cannot contain dots, commas or spaces", + + "error.validation.metadata.qualifier.max-length": "This field may not contain more than 64 characters", "feed.description": "Syndication feed",