Merge pull request #2177 from alexandrevryghem/fix-metadata-fields-containing-dots_contribute-main

Added input validation for new metadata field submissions & fixed some other small bugs

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';
@@ -29,14 +28,16 @@ describe('MetadataSchemaFormComponent', () => {
     createFormGroup: () => {
       return {
         patchValue: () => {
-        }
+        },
+        reset(_value?: any, _options?: { onlySelf?: boolean; emitEvent?: boolean; }): void {
+        },
       };
     }
   };
   /* 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: [
@@ -64,7 +65,7 @@ describe('MetadataSchemaFormComponent', () => {
     const expected = Object.assign(new MetadataSchema(), {
       namespace: namespace,
       prefix: prefix
-    });
+    } as MetadataSchema);
 
     beforeEach(() => {
       spyOn(component.submitForm, 'emit');
@@ -79,11 +80,10 @@ describe('MetadataSchemaFormComponent', () => {
         fixture.detectChanges();
       });
 
-      it('should emit a new schema using the correct values', waitForAsync(() => {
+      it('should emit a new schema using the correct values', async () => {
-        fixture.whenStable().then(() => {
+        await fixture.whenStable();
-          expect(component.submitForm.emit).toHaveBeenCalledWith(expected);
+        expect(component.submitForm.emit).toHaveBeenCalledWith(expected);
-        });
+      });
-      }));
     });
 
     describe('with an active schema', () => {
@@ -91,7 +91,7 @@ describe('MetadataSchemaFormComponent', () => {
         id: 1,
         namespace: namespace,
         prefix: prefix
-      });
+      } as MetadataSchema);
 
       beforeEach(() => {
         spyOn(registryService, 'getActiveMetadataSchema').and.returnValue(observableOf(expectedWithId));
@@ -99,11 +99,10 @@ describe('MetadataSchemaFormComponent', () => {
         fixture.detectChanges();
       });
 
-      it('should edit the existing schema using the correct values', waitForAsync(() => {
+      it('should edit the existing schema using the correct values', async () => {
-        fixture.whenStable().then(() => {
+        await fixture.whenStable();
-          expect(component.submitForm.emit).toHaveBeenCalledWith(expectedWithId);
+        expect(component.submitForm.emit).toHaveBeenCalledWith(expectedWithId);
-        });
+      });
-      }));
     });
   });
 });

src/app/admin/admin-registries/metadata-registry/metadata-schema-form/metadata-schema-form.component.ts

@@ -77,19 +77,24 @@ 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,
           name: 'name',
           validators: {
             required: null,
-            pattern: '^[^ ,_]{1,32}$'
+            pattern: '^[^. ,]*$',
+            maxLength: 32,
           },
           required: true,
+          errorMessages: {
+            pattern: 'error.validation.metadata.name.invalid-pattern',
+            maxLength: 'error.validation.metadata.name.max-length',
+          },
         });
       this.namespace = new DynamicInputModel({
           id: 'namespace',
@@ -97,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(
@@ -108,13 +117,18 @@ export class MetadataSchemaFormComponent implements OnInit, OnDestroy {
           })
       ];
       this.formGroup = this.formBuilderService.createFormGroup(this.formModel);
-      this.registryService.getActiveMetadataSchema().subscribe((schema) => {
+      this.registryService.getActiveMetadataSchema().subscribe((schema: MetadataSchema) => {
-        this.formGroup.patchValue({
+        if (schema == null) {
-          metadatadataschemagroup:{
+          this.clearFields();
-            name: schema != null ? schema.prefix : '',
+        } else {
-            namespace: schema != null ? schema.namespace : ''
+          this.formGroup.patchValue({
-          }
+            metadatadataschemagroup: {
-        });
+              name: schema.prefix,
+              namespace: schema.namespace,
+            },
+          });
+          this.name.disabled = true;
+        }
       });
     });
   }
@@ -132,10 +146,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
@@ -147,9 +161,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),
+            prefix: schema.prefix,
-            namespace: (values.namespace ? values.namespace : schema.namespace)
+            namespace: values.namespace,
-          })).subscribe((updatedSchema) => {
+          })).subscribe((updatedSchema: MetadataSchema) => {
             this.submitForm.emit(updatedSchema);
           });
         }
@@ -162,13 +176,9 @@ export class MetadataSchemaFormComponent implements OnInit, OnDestroy {
   /**
    * Reset all input-fields to be empty
    */
-  clearFields() {
+  clearFields(): void {
-    this.formGroup.patchValue({
+    this.formGroup.reset('metadatadataschemagroup');
-      metadatadataschemagroup:{
+    this.name.disabled = false;
-        prefix: '',
-        namespace: ''
-      }
-    });
   }
 
   /**

src/app/admin/admin-registries/metadata-schema/metadata-field-form/metadata-field-form.component.spec.ts

@@ -39,14 +39,16 @@ describe('MetadataFieldFormComponent', () => {
     createFormGroup: () => {
       return {
         patchValue: () => {
-        }
+        },
+        reset(_value?: any, _options?: { onlySelf?: boolean; emitEvent?: boolean; }): void {
+        },
       };
     }
   };
   /* 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: [
@@ -98,11 +100,10 @@ describe('MetadataFieldFormComponent', () => {
         fixture.detectChanges();
       });
 
-      it('should emit a new field using the correct values', waitForAsync(() => {
+      it('should emit a new field using the correct values', async () => {
-        fixture.whenStable().then(() => {
+        await fixture.whenStable();
-          expect(component.submitForm.emit).toHaveBeenCalledWith(expected);
+        expect(component.submitForm.emit).toHaveBeenCalledWith(expected);
-        });
+      });
-      }));
     });
 
     describe('with an active field', () => {
@@ -120,11 +121,10 @@ describe('MetadataFieldFormComponent', () => {
         fixture.detectChanges();
       });
 
-      it('should edit the existing field using the correct values', waitForAsync(() => {
+      it('should edit the existing field using the correct values', async () => {
-        fixture.whenStable().then(() => {
+        await fixture.whenStable();
-          expect(component.submitForm.emit).toHaveBeenCalledWith(expectedWithId);
+        expect(component.submitForm.emit).toHaveBeenCalledWith(expectedWithId);
-        });
+      });
-      }));
     });
   });
 });

src/app/admin/admin-registries/metadata-schema/metadata-field-form/metadata-field-form.component.ts

@@ -98,25 +98,39 @@ 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: '^[^. ,]*$',
+          maxLength: 64,
         },
         required: true,
+        errorMessages: {
+          pattern: 'error.validation.metadata.element.invalid-pattern',
+          maxLength: 'error.validation.metadata.element.max-length',
+        },
       });
       this.qualifier = new DynamicInputModel({
         id: 'qualifier',
         label: qualifier,
         name: 'qualifier',
+        validators: {
+          pattern: '^[^. ,]*$',
+          maxLength: 64,
+        },
         required: false,
+        errorMessages: {
+          pattern: 'error.validation.metadata.qualifier.invalid-pattern',
+          maxLength: 'error.validation.metadata.qualifier.max-length',
+        },
       });
       this.scopeNote = new DynamicInputModel({
         id: 'scopeNote',
@@ -132,14 +146,20 @@ export class MetadataFieldFormComponent implements OnInit, OnDestroy {
         })
       ];
       this.formGroup = this.formBuilderService.createFormGroup(this.formModel);
-      this.registryService.getActiveMetadataField().subscribe((field) => {
+      this.registryService.getActiveMetadataField().subscribe((field: MetadataField): void => {
-        this.formGroup.patchValue({
+        if (field == null) {
-          metadatadatafieldgroup: {
+          this.clearFields();
-            element: field != null ? field.element : '',
+        } else {
-            qualifier: field != null ? field.qualifier : '',
+          this.formGroup.patchValue({
-            scopeNote: field != null ? field.scopeNote : ''
+            metadatadatafieldgroup: {
-          }
+              element: field.element,
-        });
+              qualifier: field.qualifier,
+              scopeNote: field.scopeNote,
+            },
+          });
+          this.element.disabled = true;
+          this.qualifier.disabled = true;
+        }
       });
     });
   }
@@ -157,25 +177,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) => {
+      (field: MetadataField) => {
-        const values = {
-          element: this.element.value,
-          qualifier: this.qualifier.value,
-          scopeNote: this.scopeNote.value
-        };
         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),
+            element: field.element,
-            qualifier: (values.qualifier ? values.qualifier : field.qualifier),
+            qualifier: field.qualifier,
-            scopeNote: (values.scopeNote ? values.scopeNote : field.scopeNote)
+            scopeNote: this.scopeNote.value,
-          })).subscribe((updatedField) => {
+          })).subscribe((updatedField: MetadataField) => {
             this.submitForm.emit(updatedField);
           });
         }
@@ -188,14 +207,10 @@ export class MetadataFieldFormComponent implements OnInit, OnDestroy {
   /**
    * Reset all input-fields to be empty
    */
-  clearFields() {
+  clearFields(): void {
-    this.formGroup.patchValue({
+    this.formGroup.reset('metadatadatafieldgroup');
-      metadatadatafieldgroup: {
+    this.element.disabled = false;
-        element: '',
+    this.qualifier.disabled = false;
-        qualifier: '',
-        scopeNote: ''
-      }
-    });
   }
 
   /**

src/app/dso-shared/dso-edit-metadata/metadata-field-selector/metadata-field-selector.component.spec.ts

@@ -12,6 +12,7 @@ import { createPaginatedList } from '../../../shared/testing/utils.test';
 import { followLink } from '../../../shared/utils/follow-link-config.model';
 import { By } from '@angular/platform-browser';
 import { NotificationsService } from '../../../shared/notifications/notifications.service';
+import { SortDirection, SortOptions } from '../../../core/cache/models/sort-options.model';
 
 describe('MetadataFieldSelectorComponent', () => {
   let component: MetadataFieldSelectorComponent;
@@ -79,7 +80,7 @@ describe('MetadataFieldSelectorComponent', () => {
     });
 
     it('should query the registry service for metadata fields and include the schema', () => {
-      expect(registryService.queryMetadataFields).toHaveBeenCalledWith(query, null, true, false, followLink('schema'));
+      expect(registryService.queryMetadataFields).toHaveBeenCalledWith(query, { elementsPerPage: 10, sort: new SortOptions('fieldName', SortDirection.ASC) }, true, false, followLink('schema'));
     });
   });
 

src/app/dso-shared/dso-edit-metadata/metadata-field-selector/metadata-field-selector.component.ts

@@ -9,10 +9,11 @@ import {
   Output,
   ViewChild
 } from '@angular/core';
-import { switchMap, debounceTime, distinctUntilChanged, map, tap, take } from 'rxjs/operators';
+import { debounceTime, distinctUntilChanged, map, switchMap, take, tap } from 'rxjs/operators';
 import { followLink } from '../../../shared/utils/follow-link-config.model';
 import {
-  getAllSucceededRemoteData, getFirstCompletedRemoteData,
+  getAllSucceededRemoteData,
+  getFirstCompletedRemoteData,
   metadataFieldsToString
 } from '../../../core/shared/operators';
 import { Observable } from 'rxjs/internal/Observable';
@@ -24,6 +25,7 @@ import { Subscription } from 'rxjs/internal/Subscription';
 import { of } from 'rxjs/internal/observable/of';
 import { NotificationsService } from '../../../shared/notifications/notifications.service';
 import { TranslateService } from '@ngx-translate/core';
+import { SortDirection, SortOptions } from '../../../core/cache/models/sort-options.model';
 
 @Component({
   selector: 'ds-metadata-field-selector',
@@ -127,7 +129,7 @@ export class MetadataFieldSelectorComponent implements OnInit, OnDestroy, AfterV
       switchMap((query: string) => {
         this.showInvalid = false;
         if (query !== null) {
-          return this.registryService.queryMetadataFields(query, null, true, false, followLink('schema')).pipe(
+          return this.registryService.queryMetadataFields(query, { elementsPerPage: 10, sort: new SortOptions('fieldName', SortDirection.ASC) }, true, false, followLink('schema')).pipe(
             getAllSucceededRemoteData(),
             metadataFieldsToString(),
           );

src/assets/i18n/en.json5

@@ -1628,6 +1628,19 @@
 
   "error.validation.groupExists": "This group already exists",
 
+  "error.validation.metadata.name.invalid-pattern": "This field cannot contain dots, commas or spaces. Please use the Element & Qualifier fields instead",
+
+  "error.validation.metadata.name.max-length": "This field may not contain more than 32 characters",
+
+  "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",