[835] Auto-save in new Item Submission form breaks the form

Improved form touched state in ngrx store.

src/app/shared/form/builder/form-builder.service.ts

@@ -384,7 +384,7 @@ export class FormBuilderService extends DynamicFormService {
 
     const result = iterateControlModels([model]);
 
-    return result;
+    return Object.keys(result);
   }
 
 }

src/app/shared/form/form.actions.ts

@@ -13,7 +13,7 @@ import { type } from '../ngrx/type';
 export const FormActionTypes = {
   FORM_INIT: type('dspace/form/FORM_INIT'),
   FORM_CHANGE: type('dspace/form/FORM_CHANGE'),
-  FORM_ADDITIONAL: type('dspace/form/FORM_ADDITIONAL'),
+  FORM_ADD_TOUCHED: type('dspace/form/FORM_ADD_TOUCHED'),
   FORM_REMOVE: type('dspace/form/FORM_REMOVE'),
   FORM_STATUS_CHANGE: type('dspace/form/FORM_STATUS_CHANGE'),
   FORM_ADD_ERROR: type('dspace/form/FORM_ADD_ERROR'),
@@ -28,7 +28,6 @@ export class FormInitAction implements Action {
     formId: string;
     formData: any;
     valid: boolean;
-    formAdditional: any;
   };
 
   /**
@@ -41,8 +40,8 @@ export class FormInitAction implements Action {
    * @param valid
    *    the Form validation status
    */
-  constructor(formId: string, formData: any, valid: boolean, formAdditional?: any) {
+  constructor(formId: string, formData: any, valid: boolean) {
-    this.payload = {formId, formData, valid, formAdditional};
+    this.payload = {formId, formData, valid};
   }
 }
 
@@ -66,23 +65,23 @@ export class FormChangeAction implements Action {
   }
 }
 
-export class FormSetAdditionalAction implements Action {
+export class FormAddTouchedAction implements Action {
-  type = FormActionTypes.FORM_ADDITIONAL;
+  type = FormActionTypes.FORM_ADD_TOUCHED;
   payload: {
     formId: string;
-    additionalData: any;
+    touched: string[];
   };
 
   /**
-   * Create a new FormSetAdditionalAction
+   * Create a new FormAddTouchedAction
    *
    * @param formId
    *    the Form's ID
-   * @param additionalData
+   * @param touched
-   *    the additionalData Object
+   *    the array containing new touched fields
    */
-  constructor(formId: string, additionalData: any) {
+  constructor(formId: string, touched: string[]) {
-    this.payload = {formId, additionalData};
+    this.payload = {formId, touched};
   }
 }
 
@@ -169,7 +168,7 @@ export class FormClearErrorsAction implements Action {
  */
 export type FormAction = FormInitAction
   | FormChangeAction
-  | FormSetAdditionalAction
+  | FormAddTouchedAction
   | FormRemoveAction
   | FormStatusChangeAction
   | FormAddError

src/app/shared/form/form.component.spec.ts

@@ -120,7 +120,7 @@ function init() {
       },
       valid: false,
       errors: [],
-      additional: {}
+      touched: {}
     }
   };
 

src/app/shared/form/form.reducer.spec.ts

@@ -6,7 +6,7 @@ import {
   FormInitAction,
   FormRemoveAction,
   FormRemoveErrorAction,
-  FormSetAdditionalAction,
+  FormAddTouchedAction,
   FormStatusChangeAction
 } from './form.actions';
 
@@ -23,7 +23,7 @@ describe('formReducer', () => {
         },
         valid: false,
         errors: [],
-        additional: {}
+        touched: {}
       }
     };
     const formId = 'testForm';
@@ -51,7 +51,7 @@ describe('formReducer', () => {
         },
         valid: false,
         errors: [],
-        additional: {}
+        touched: {}
       }
     };
     const formId = 'testForm';
@@ -71,7 +71,7 @@ describe('formReducer', () => {
         },
         valid: false,
         errors: [],
-        additional: {}
+        touched: {}
       }
     };
 
@@ -93,7 +93,7 @@ describe('formReducer', () => {
         },
         valid: false,
         errors: [],
-        additional: {}
+        touched: {}
       }
     };
     const state = {
@@ -106,7 +106,7 @@ describe('formReducer', () => {
         },
         valid: false,
         errors: [],
-        additional: {}
+        touched: {}
       }
     };
     const formId = 'testForm';
@@ -134,7 +134,7 @@ describe('formReducer', () => {
         },
         valid: false,
         errors: [],
-        additional: {}
+        touched: {}
       }
     };
     const state = {
@@ -147,7 +147,7 @@ describe('formReducer', () => {
         },
         valid: true,
         errors: [],
-        additional: {}
+        touched: {}
       }
     };
     const formId = 'testForm';
@@ -169,7 +169,7 @@ describe('formReducer', () => {
         },
         valid: true,
         errors: [],
-        additional: {}
+        touched: {}
       }
     };
 
@@ -214,7 +214,7 @@ describe('formReducer', () => {
             message: 'error.validation.required'
           }
         ],
-        additional: {}
+        touched: {}
       }
     };
 
@@ -247,7 +247,7 @@ describe('formReducer', () => {
         },
         valid: true,
         errors: [],
-        additional: {}
+        touched: {}
       }
     };
 
@@ -276,7 +276,7 @@ describe('formReducer', () => {
             message: 'error.validation.required'
           }
         ],
-        additional: {}
+        touched: {}
       }
     };
 
@@ -299,7 +299,7 @@ describe('formReducer', () => {
         },
         valid: false,
         errors: [],
-        additional: {}
+        touched: {}
       }
     };
     const state = {
@@ -312,21 +312,15 @@ describe('formReducer', () => {
         },
         valid: false,
         errors: [],
-        additional: {
         touched: {
           title: true
         }
       }
-      }
     };
     const formId = 'testForm';
-    const additionalData = {
+    const touched = ['title'];
-      touched: {
-       title: true
-      }
-    };
 
-    const action = new FormSetAdditionalAction(formId, additionalData);
+    const action = new FormAddTouchedAction(formId, touched);
     const newState = formReducer(initState, action);
 
     expect(newState).toEqual(state);
@@ -343,12 +337,10 @@ describe('formReducer', () => {
         },
         valid: false,
         errors: [],
-        additional: {
         touched: {
           title: true
         }
       }
-      }
     };
     const state = {
       testForm: {
@@ -360,22 +352,16 @@ describe('formReducer', () => {
         },
         valid: false,
         errors: [],
-        additional: {
         touched: {
           title: true,
           author: true
         }
       }
-      }
     };
     const formId = 'testForm';
-    const additionalData = {
+    const touched = ['author'];
-      touched: {
-        author: true
-      }
-    };
 
-    const action = new FormSetAdditionalAction(formId, additionalData);
+    const action = new FormAddTouchedAction(formId, touched);
     const newState = formReducer(initState, action);
 
     expect(newState).toEqual(state);

src/app/shared/form/form.reducer.ts

@@ -4,9 +4,8 @@ import {
   FormAddError,
   FormChangeAction, FormClearErrorsAction,
   FormInitAction,
-  FormRemoveAction,
+  FormRemoveAction, FormRemoveErrorAction,
-  FormRemoveErrorAction, FormSetAdditionalAction,
+  FormStatusChangeAction, FormAddTouchedAction
-  FormStatusChangeAction
 } from './form.actions';
 import { hasValue } from '../empty.util';
 import { isEqual, uniqWith } from 'lodash';
@@ -17,11 +16,15 @@ export interface FormError {
   fieldIndex: number;
 }
 
+export interface FormTouchedState {
+  [key: string]: boolean
+}
+
 export interface FormEntry {
   data: any;
   valid: boolean;
   errors: FormError[];
-  additional: any;
+  touched: FormTouchedState;
 }
 
 export interface FormState {
@@ -41,8 +44,8 @@ export function formReducer(state = initialState, action: FormAction): FormState
       return changeDataForm(state, action as FormChangeAction);
     }
 
-    case FormActionTypes.FORM_ADDITIONAL: {
+    case FormActionTypes.FORM_ADD_TOUCHED: {
-      return additionalData(state, action as FormSetAdditionalAction);
+      return changeTouchedState(state, action as FormAddTouchedAction);
     }
 
     case FormActionTypes.FORM_REMOVE: {
@@ -132,8 +135,8 @@ function initForm(state: FormState, action: FormInitAction): FormState {
   const formState = {
     data: action.payload.formData,
     valid: action.payload.valid,
+    touched: {},
     errors: [],
-    additional: action.payload.formAdditional ? action.payload.formAdditional : {}
   };
   if (!hasValue(state[action.payload.formId])) {
     return Object.assign({}, state, {
@@ -220,25 +223,19 @@ function removeForm(state: FormState, action: FormRemoveAction): FormState {
 }
 
 /**
- * Compute the additional data state of the form. New touched fields are merged with the previous ones.
+ * Compute the touched state of the form. New touched fields are merged with the previous ones.
  * @param state
  * @param action
  */
-function additionalData(state: FormState, action: FormSetAdditionalAction): FormState {
+function changeTouchedState(state: FormState, action: FormAddTouchedAction): FormState {
   if (hasValue(state[action.payload.formId])) {
-
     const newState = Object.assign({}, state);
 
-    const newAdditional = newState[action.payload.formId].additional ? {...newState[action.payload.formId].additional} : {};
+    const newForm = Object.assign({}, newState[action.payload.formId]);
+    newState[action.payload.formId] = newForm;
 
-    const newTouchedValue = newAdditional.touched ? {...newAdditional.touched,
+    newForm.touched = { ... newForm.touched};
-      ...action.payload.additionalData.touched} : { ...action.payload.additionalData.touched};
+    action.payload.touched.forEach((field) => newForm[field] = true);
-    newAdditional.touched = newTouchedValue;
-
-    newState[action.payload.formId] = Object.assign({}, newState[action.payload.formId], {
-        additional: newAdditional
-      }
-    );
 
     return newState;
   } else {

src/app/shared/form/form.service.spec.ts

@@ -85,7 +85,7 @@ describe('FormService test suite', () => {
       data: formData,
       valid: false,
       errors: [],
-      additional: {}
+      touched: {}
     }
   };
 

src/app/shared/form/form.service.ts

@@ -1,5 +1,5 @@
 import { map, distinctUntilChanged, filter } from 'rxjs/operators';
-import { Inject, Injectable } from '@angular/core';
+import { Injectable } from '@angular/core';
 import { AbstractControl, FormArray, FormControl, FormGroup } from '@angular/forms';
 import { Observable } from 'rxjs';
 import { select, Store } from '@ngrx/store';
@@ -13,10 +13,10 @@ import { uniqueId } from 'lodash';
 import {
   FormChangeAction,
   FormInitAction,
-  FormRemoveAction, FormRemoveErrorAction, FormSetAdditionalAction,
+  FormRemoveAction, FormRemoveErrorAction, FormAddTouchedAction,
   FormStatusChangeAction
 } from './form.actions';
-import { FormEntry } from './form.reducer';
+import { FormEntry, FormTouchedState } from './form.reducer';
 import { environment } from '../../../environments/environment';
 
 @Injectable()
@@ -52,13 +52,13 @@ export class FormService {
   }
 
   /**
-   * Method to retrieve form's additional data from state
+   * Method to retrieve form's touched state
    */
-  public getFormAdditionalData(formId: string): Observable<any> {
+  public getFormTouchedState(formId: string): Observable<FormTouchedState> {
     return this.store.pipe(
       select(formObjectFromIdSelector(formId)),
       filter((state) => isNotUndefined(state)),
-      map((state) => state.additional),
+      map((state) => state.touched),
       distinctUntilChanged()
     );
   }
@@ -183,7 +183,7 @@ export class FormService {
 
   public setTouched(formId: string, model: DynamicFormControlModel[], event: DynamicFormControlEvent) {
     const ids = this.formBuilderService.getMetadataIdsFromEvent(event);
-    this.store.dispatch(new FormSetAdditionalAction(formId, { touched: ids}));
+    this.store.dispatch(new FormAddTouchedAction(formId, ids));
   }
 
   public removeError(formId: string, eventModelId: string, fieldIndex: number) {

src/app/submission/objects/submission-objects.effects.spec.ts

@@ -384,13 +384,11 @@ describe('SubmissionObjectEffects test suite', () => {
         },
         forms: {
           '2_traditionalpageone': {
-            additional: {
             touched: {
               'dc.title': true
             }
           }
         }
-        }
       } as any);
 
       const response = [Object.assign({}, mockSubmissionRestResponse[0], {

src/app/submission/objects/submission-objects.effects.ts

@@ -59,6 +59,7 @@ import { getFirstSucceededRemoteDataPayload } from '../../core/shared/operators'
 import { SubmissionObjectDataService } from '../../core/submission/submission-object-data.service';
 import { followLink } from '../../shared/utils/follow-link-config.model';
 import parseSectionErrorPaths, {SectionErrorPath} from '../utils/parseSectionErrorPaths';
+import { FormState } from '../../shared/form/form.reducer';
 
 @Injectable()
 export class SubmissionObjectEffects {
@@ -431,24 +432,29 @@ function getForm(forms, currentState, sectionId) {
 /**
  * Filter sectionErrors accordingly to this rules:
  * 1. if notifications are enabled return all errors
- * 2. if sectionType is different from submission-form return all errors
+ * 2. if sectionType is different from 'submission-form' return all errors
  * 3. otherwise return errors only for those fields marked as touched inside the section form
  * @param sectionForm
+ *  The form related to the section
  * @param sectionErrors
+ *  The section errors array
+ * @param sectionType
+ *  The section type
  * @param notify
+ *  Whether notifications are enabled
  */
-function filterErrors(sectionForm, sectionErrors, sectionType, notify): any {
+function filterErrors(sectionForm: FormState, sectionErrors: SubmissionSectionError[], sectionType: string, notify: boolean): any {
   if (notify || sectionType !== SectionsType.SubmissionForm) {
     return sectionErrors;
   }
-  if (!sectionForm || !sectionForm.additional || !sectionForm.additional.touched) {
+  if (!sectionForm || !sectionForm.touched) {
     return [];
   }
   const filteredErrors = [];
   sectionErrors.forEach((error: SubmissionSectionError) => {
     const errorPaths: SectionErrorPath[] = parseSectionErrorPaths(error.path);
     errorPaths.forEach((path: SectionErrorPath) => {
-      if (path.fieldId && sectionForm.additional.touched[path.fieldId]) {
+      if (path.fieldId && sectionForm.touched[path.fieldId]) {
         filteredErrors.push(error);
       }
     });