Fixed an issue with showing form error in FormSectionComponent and added comments

src/app/submission/sections/form/section-form.component.spec.ts

@@ -318,6 +318,23 @@ describe('FormSectionComponent test suite', () => {
 
     });
 
+    it('should update form error properly', () => {
+      spyOn(comp, 'initForm');
+      spyOn(comp, 'checksForErrors');
+      const sectionData: any = {
+        'dc.title': [new FormFieldMetadataValueObject('test')]
+      };
+      comp.sectionData.data = {};
+      comp.sectionData.errors = [];
+      compAsAny.formData = sectionData;
+
+      comp.updateForm(sectionData, parsedSectionErrors);
+
+      expect(comp.initForm).not.toHaveBeenCalled();
+      expect(comp.checksForErrors).toHaveBeenCalled();
+      expect(comp.sectionData.data).toEqual(sectionData);
+    });
+
     it('should update form error properly', () => {
       spyOn(comp, 'initForm');
       spyOn(comp, 'checksForErrors');

src/app/submission/sections/form/section-form.component.ts

@@ -17,7 +17,6 @@ import { JsonPatchOperationPathCombiner } from '../../../core/json-patch/builder
 import { SubmissionFormsModel } from '../../../core/config/models/config-submission-forms.model';
 import { SubmissionSectionError, SubmissionSectionObject } from '../../objects/submission-objects.reducer';
 import { FormFieldPreviousValueObject } from '../../../shared/form/builder/models/form-field-previous-value-object';
-import { WorkspaceitemSectionDataType } from '../../../core/submission/models/workspaceitem-sections.model';
 import { GLOBAL_CONFIG } from '../../../../config';
 import { GlobalConfig } from '../../../../config/global-config.interface';
 import { SectionDataObject } from '../models/section-data.model';
@@ -28,7 +27,11 @@ import { SectionFormOperationsService } from './section-form-operations.service'
 import { NotificationsService } from '../../../shared/notifications/notifications.service';
 import { SectionsService } from '../sections.service';
 import { difference } from '../../../shared/object.util';
+import { WorkspaceitemSectionFormObject } from '../../../core/submission/models/workspaceitem-section-form.model';
 
+/**
+ * This component represents a section that contains a Form.
+ */
 @Component({
   selector: 'ds-submission-section-form',
   styleUrls: ['./section-form.component.scss'],
@@ -37,19 +40,72 @@ import { difference } from '../../../shared/object.util';
 @renderSectionFor(SectionsType.SubmissionForm)
 export class FormSectionComponent extends SectionModelComponent {
 
+  /**
+   * The form id
+   */
   public formId;
+
+  /**
+   * The form model
+   */
   public formModel: DynamicFormControlModel[];
+
+  /**
+   * A boolean representing if this section is updating
+   */
   public isUpdating = false;
+
+  /**
+   * A boolean representing if this section is loading
+   */
   public isLoading = true;
 
+  /**
+   * The form config
+   */
   protected formConfig: SubmissionFormsModel;
+
+  /**
+   * The form data
+   */
   protected formData: any = Object.create({});
+
+  /**
+   * The [JsonPatchOperationPathCombiner] object
+   */
   protected pathCombiner: JsonPatchOperationPathCombiner;
+
+  /**
+   * The [FormFieldPreviousValueObject] object
+   */
   protected previousValue: FormFieldPreviousValueObject = new FormFieldPreviousValueObject();
+
+  /**
+   * The list of Subscription
+   */
   protected subs: Subscription[] = [];
 
+  /**
+   * The FormComponent reference
+   */
   @ViewChild('formRef') private formRef: FormComponent;
 
+  /**
+   * Initialize instance variables
+   * @param {ChangeDetectorRef} cdr
+   * @param {FormBuilderService} formBuilderService
+   * @param {SectionFormOperationsService} formOperationsService
+   * @param {FormService} formService
+   * @param {SubmissionFormsConfigService} formConfigService
+   * @param {NotificationsService} notificationsService
+   * @param {SectionsService} sectionService
+   * @param {SubmissionService} submissionService
+   * @param {TranslateService} translate
+   * @param {GlobalConfig} EnvConfig
+   * @param {string} injectedCollectionId
+   * @param {SectionDataObject} injectedSectionData
+   * @param {string} injectedSubmissionId
+   */
   constructor(protected cdr: ChangeDetectorRef,
               protected formBuilderService: FormBuilderService,
               protected formOperationsService: SectionFormOperationsService,
@@ -66,6 +122,9 @@ export class FormSectionComponent extends SectionModelComponent {
     super(injectedCollectionId, injectedSectionData, injectedSubmissionId);
   }
 
+  /**
+   * Initialize all instance variables and retrieve form configuration
+   */
   onSectionInit() {
     this.pathCombiner = new JsonPatchOperationPathCombiner('sections', this.sectionData.id);
     this.formId = this.formService.getUniqueId(this.sectionData.id);
@@ -75,30 +134,42 @@ export class FormSectionComponent extends SectionModelComponent {
       tap((config: SubmissionFormsModel) => this.formConfig = config),
       flatMap(() => this.sectionService.getSectionData(this.submissionId, this.sectionData.id)),
       take(1))
-      .subscribe((sectionData: WorkspaceitemSectionDataType) => {
+      .subscribe((sectionData: WorkspaceitemSectionFormObject) => {
-          if (isUndefined(this.formModel)) {
+        if (isUndefined(this.formModel)) {
-            this.sectionData.errors = [];
+          this.sectionData.errors = [];
-            // Is the first loading so init form
+          // Is the first loading so init form
-            this.initForm(sectionData);
+          this.initForm(sectionData);
-            this.sectionData.data = sectionData;
+          this.sectionData.data = sectionData;
-            this.subscriptions();
+          this.subscriptions();
-            this.isLoading = false;
+          this.isLoading = false;
-            this.cdr.detectChanges();
+          this.cdr.detectChanges();
-          }
+        }
-        })
+      })
   }
 
+  /**
+   * Unsubscribe from all subscriptions
+   */
   onSectionDestroy() {
     this.subs
       .filter((subscription) => hasValue(subscription))
       .forEach((subscription) => subscription.unsubscribe());
   }
 
+  /**
+   * Get section status
+   */
   protected getSectionStatus(): Observable<boolean> {
     return this.formService.isValid(this.formId);
   }
 
-  hasMetadataEnrichment(sectionData): boolean {
+  /**
+   * Check if the section data has been enriched by the server
+   *
+   * @param sectionData
+   *    the section data retrieved from the server
+   */
+  hasMetadataEnrichment(sectionData: WorkspaceitemSectionFormObject): boolean {
     const diffResult = [];
 
     // compare current form data state with section data retrieved from store
@@ -116,7 +187,13 @@ export class FormSectionComponent extends SectionModelComponent {
     return isNotEmpty(diffResult);
   }
 
-  initForm(sectionData: WorkspaceitemSectionDataType) {
+  /**
+   * Initialize form model
+   *
+   * @param sectionData
+   *    the section data retrieved from the server
+   */
+  initForm(sectionData: WorkspaceitemSectionFormObject): void {
     try {
       this.formModel = this.formBuilderService.modelFromConfiguration(
         this.formConfig,
@@ -124,28 +201,32 @@ export class FormSectionComponent extends SectionModelComponent {
         sectionData,
         this.submissionService.getSubmissionScope());
     } catch (e) {
-      this.translate.get('error.submission.sections.init-form-error')
+      const msg = this.translate.instant('error.submission.sections.init-form-error');
-        .subscribe((msg) => {
+      const sectionError: SubmissionSectionError = {
-          const sectionError: SubmissionSectionError = {
+        message: msg + e.toString(),
-            message: msg + e.toString(),
+        path: '/sections/' + this.sectionData.id
-            path: '/sections/' + this.sectionData.id
+      };
-          };
+      this.sectionService.setSectionError(this.submissionId, this.sectionData.id, sectionError);
-          this.sectionService.setSectionError(this.submissionId, this.sectionData.id, sectionError);
-        })
-
     }
   }
 
-  updateForm(sectionData: WorkspaceitemSectionDataType, errors: SubmissionSectionError[]) {
+  /**
+   * Update form model
+   *
+   * @param sectionData
+   *    the section data retrieved from the server
+   * @param errors
+   *    the section errors retrieved from the server
+   */
+  updateForm(sectionData: WorkspaceitemSectionFormObject, errors: SubmissionSectionError[]): void {
 
     if (isNotEmpty(sectionData) && !isEqual(sectionData, this.sectionData.data)) {
       this.sectionData.data = sectionData;
       if (this.hasMetadataEnrichment(sectionData)) {
-        this.translate.get('submission.sections.general.metadata-extracted', { sectionId: this.sectionData.id })
+        const msg = this.translate.instant(
-          .pipe(take(1))
+          'submission.sections.general.metadata-extracted',
-          .subscribe((m) => {
+          { sectionId: this.sectionData.id });
-            this.notificationsService.info(null, m, null, true);
+        this.notificationsService.info(null, msg, null, true);
-          });
         this.isUpdating = true;
         this.formModel = null;
         this.cdr.detectChanges();
@@ -153,6 +234,8 @@ export class FormSectionComponent extends SectionModelComponent {
         this.checksForErrors(errors);
         this.isUpdating = false;
         this.cdr.detectChanges();
+      } else if (isNotEmpty(errors) || isNotEmpty(this.sectionData.errors)) {
+        this.checksForErrors(errors);
       }
     } else if (isNotEmpty(errors) || isNotEmpty(this.sectionData.errors)) {
       this.checksForErrors(errors);
@@ -160,7 +243,13 @@ export class FormSectionComponent extends SectionModelComponent {
 
   }
 
-  checksForErrors(errors: SubmissionSectionError[]) {
+  /**
+   * Check if there are form validation error retrieved from server
+   *
+   * @param errors
+   *    the section errors retrieved from the server
+   */
+  checksForErrors(errors: SubmissionSectionError[]): void {
     this.formService.isFormInitialized(this.formId).pipe(
       find((status: boolean) => status === true && !this.isUpdating))
       .subscribe(() => {
@@ -170,9 +259,11 @@ export class FormSectionComponent extends SectionModelComponent {
       });
   }
 
-  subscriptions() {
+  /**
+   * Initialize all subscriptions
+   */
+  subscriptions(): void {
     this.subs.push(
-
       /**
        * Subscribe to form's data
        */
@@ -181,6 +272,7 @@ export class FormSectionComponent extends SectionModelComponent {
         .subscribe((formData) => {
           this.formData = formData;
         }),
+
       /**
        * Subscribe to section state
        */
@@ -190,12 +282,16 @@ export class FormSectionComponent extends SectionModelComponent {
         }),
         distinctUntilChanged())
         .subscribe((sectionState: SubmissionSectionObject) => {
-          this.updateForm(sectionState.data, sectionState.errors);
+          this.updateForm(sectionState.data as WorkspaceitemSectionFormObject, sectionState.errors);
         })
     )
   }
 
-  onChange(event: DynamicFormControlEvent) {
+  /**
+   * Method called when a form dfChange event is fired.
+   * Dispatch form operations based on changes.
+   */
+  onChange(event: DynamicFormControlEvent): void {
     this.formOperationsService.dispatchOperationsFromEvent(
       this.pathCombiner,
       event,
@@ -209,7 +305,11 @@ export class FormSectionComponent extends SectionModelComponent {
     }
   }
 
-  onFocus(event: DynamicFormControlEvent) {
+  /**
+   * Method called when a form dfFocus event is fired.
+   * Initialize [FormFieldPreviousValueObject] instance.
+   */
+  onFocus(event: DynamicFormControlEvent): void {
     const value = this.formOperationsService.getFieldValueFromChangeEvent(event);
     const path = this.formBuilderService.getPath(event.model);
     if (this.formBuilderService.hasMappedGroupValue(event.model)) {
@@ -221,7 +321,11 @@ export class FormSectionComponent extends SectionModelComponent {
     }
   }
 
-  onRemove(event: DynamicFormControlEvent) {
+  /**
+   * Method called when a form remove event is fired.
+   * Dispatch form operations based on changes.
+   */
+  onRemove(event: DynamicFormControlEvent): void {
     this.formOperationsService.dispatchOperationsFromEvent(
       this.pathCombiner,
       event,
@@ -229,7 +333,15 @@ export class FormSectionComponent extends SectionModelComponent {
       this.hasStoredValue(this.formBuilderService.getId(event.model), this.formOperationsService.getArrayIndexFromEvent(event)));
   }
 
-  hasStoredValue(fieldId, index) {
+  /**
+   * Check if the specified form field has already a value stored
+   *
+   * @param fieldId
+   *    the section data retrieved from the serverù
+   * @param index
+   *    the section data retrieved from the server
+   */
+  hasStoredValue(fieldId, index): boolean {
     if (isNotEmpty(this.sectionData.data)) {
       return this.sectionData.data.hasOwnProperty(fieldId) && isNotEmpty(this.sectionData.data[fieldId][index]);
     } else {