Refactoring based on review

src/app/core/json-patch/json-patch-operations.reducer.ts

@@ -102,8 +102,6 @@ function startTransactionPatchOperations(state: JsonPatchOperationsState, action
     && isNull(state[ action.payload.resourceType ].transactionStartTime)) {
     return Object.assign({}, state, {
       [action.payload.resourceType]: Object.assign({}, state[ action.payload.resourceType ], {
-        children: state[ action.payload.resourceType ].children,
-        transactionStartTime: action.payload.startTime,
         commitPending: true
       })
     });
@@ -127,8 +125,6 @@ function commitOperations(state: JsonPatchOperationsState, action: CommitPatchOp
     && state[ action.payload.resourceType ].commitPending) {
     return Object.assign({}, state, {
       [action.payload.resourceType]: Object.assign({}, state[ action.payload.resourceType ], {
-        children: state[ action.payload.resourceType ].children,
-        transactionStartTime: state[ action.payload.resourceType ].transactionStartTime,
         commitPending: false
       })
     });
@@ -152,7 +148,6 @@ function rollbackOperations(state: JsonPatchOperationsState, action: RollbacktPa
     && state[ action.payload.resourceType ].commitPending) {
     return Object.assign({}, state, {
       [action.payload.resourceType]: Object.assign({}, state[ action.payload.resourceType ], {
-        children: state[ action.payload.resourceType ].children,
         transactionStartTime: null,
         commitPending: false
       })
@@ -174,12 +169,10 @@ function rollbackOperations(state: JsonPatchOperationsState, action: RollbacktPa
  */
 function newOperation(state: JsonPatchOperationsState, action): JsonPatchOperationsState {
   const newState = Object.assign({}, state);
+  const body: any[] = hasValidBody(newState, action.payload.resourceType, action.payload.resourceId)
+    ? newState[ action.payload.resourceType ].children[ action.payload.resourceId ].body : Array.of();
   const newBody = addOperationToList(
-    (hasValue(newState[ action.payload.resourceType ])
+    body,
-      && hasValue(newState[ action.payload.resourceType ].children)
-      && hasValue(newState[ action.payload.resourceType ].children[ action.payload.resourceId ])
-      && isNotEmpty(newState[ action.payload.resourceType ].children[ action.payload.resourceId ].body))
-      ? newState[ action.payload.resourceType ].children[ action.payload.resourceId ].body : Array.of(),
     action.type,
     action.payload.path,
     hasValue(action.payload.value) ? action.payload.value : null);
@@ -193,7 +186,6 @@ function newOperation(state: JsonPatchOperationsState, action): JsonPatchOperati
             body: newBody,
           }
         }),
-        transactionStartTime: state[ action.payload.resourceType ].transactionStartTime,
         commitPending: isNotUndefined(state[ action.payload.resourceType ].commitPending) ? state[ action.payload.resourceType ].commitPending : false
       })
     });
@@ -212,6 +204,24 @@ function newOperation(state: JsonPatchOperationsState, action): JsonPatchOperati
   }
 }
 
+/**
+ * Check if state has a valid body.
+ *
+ * @param state
+ *    the current state
+ * @param resourceType
+ *    an resource type
+ * @param resourceId
+ *    an resource ID
+ * @return boolean
+ */
+function hasValidBody(state: JsonPatchOperationsState, resourceType: any, resourceId: any): boolean {
+  return (hasValue(state[ resourceType ])
+    && hasValue(state[ resourceType ].children)
+    && hasValue(state[ resourceType ].children[ resourceId ])
+    && isNotEmpty(state[ resourceType ].children[ resourceId ].body))
+}
+
 /**
  * Set the section validity.
  *
@@ -255,7 +265,6 @@ function flushOperation(state: JsonPatchOperationsState, action: FlushPatchOpera
       [action.payload.resourceType]: Object.assign({}, state[ action.payload.resourceType ], {
         children: newChildren,
         transactionStartTime: null,
-        commitPending: state[ action.payload.resourceType ].commitPending
       })
     });
   } else {

src/app/shared/notifications/notification/notification.component.spec.ts

@@ -1,19 +1,15 @@
-import { async, ComponentFixture, inject, TestBed } from '@angular/core/testing';
+import { async, ComponentFixture, TestBed } from '@angular/core/testing';
 import { BrowserModule, By } from '@angular/platform-browser';
 import { ChangeDetectorRef, DebugElement } from '@angular/core';
+import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
+
+import { Store, StoreModule } from '@ngrx/store';
 
 import { NotificationComponent } from './notification.component';
 import { NotificationsService } from '../notifications.service';
 import { NotificationType } from '../models/notification-type';
 import { notificationsReducer } from '../notifications.reducers';
-import { Store, StoreModule } from '@ngrx/store';
 import { NotificationOptions } from '../models/notification-options.model';
-import { BrowserAnimationsModule } from '@angular/platform-browser/animations';
-import { Router } from '@angular/router';
-import { NotificationsServiceStub } from '../../testing/notifications-service-stub';
-import { AppState } from '../../../app.reducer';
-import { Observable } from 'rxjs/Observable';
-import { SearchPageComponent } from '../../../+search-page/search-page.component';
 import { INotificationBoardOptions } from '../../../../config/notifications-config.interfaces';
 import { GlobalConfig } from '../../../../config/global-config.interface';
 import { Notification } from '../models/notification.model';
@@ -96,10 +92,10 @@ describe('NotificationComponent', () => {
     expect(elType).toBeDefined();
   });
 
-  it('should has html content', () => {
+  it('should have html content', () => {
     fixture = TestBed.createComponent(NotificationComponent);
     comp = fixture.componentInstance;
-    const htmlContent = `<a class="btn btn-link p-0 m-0 pb-1" href="/test"><strong>test</strong></a>`
+    const htmlContent = '<a class="btn btn-link p-0 m-0 pb-1" href="/test"><strong>test</strong></a>';
     comp.notification = {
       id: '1',
       type: NotificationType.Info,

src/app/submission/sections/form/form-operations.service.ts

@@ -30,7 +30,7 @@ export class FormOperationsService {
   dispatchOperationsFromEvent(pathCombiner: JsonPatchOperationPathCombiner,
                               event: DynamicFormControlEvent,
                               previousValue: FormFieldPreviousValueObject,
-                              hasStoredValue: boolean) {
+                              hasStoredValue: boolean): void {
     switch (event.type) {
       case 'remove':
         this.dispatchOperationsFromRemoveEvent(pathCombiner, event, previousValue);
@@ -43,28 +43,31 @@ export class FormOperationsService {
     }
   }
 
-  getArrayIndexFromEvent(event: DynamicFormControlEvent) {
+  getArrayIndexFromEvent(event: DynamicFormControlEvent): number {
     let fieldIndex: number;
     if (isNotEmpty(event)) {
       if (isNull(event.context)) {
-        if (isNotNull(event.model.parent)) {
+        // Check whether model is part of an Array of group
-          if ((event.model.parent as any).type === DYNAMIC_FORM_CONTROL_TYPE_GROUP) {
+        if (this.isPartOfArrayOfGroup(event.model)) {
-            if ((event.model.parent as any).parent) {
-              if ((event.model.parent as any).parent.context) {
-                if ((event.model.parent as any).parent.context.type === DYNAMIC_FORM_CONTROL_TYPE_ARRAY) {
           fieldIndex = (event.model.parent as any).parent.index;
         }
-              }
-            }
-          }
-        }
       } else {
         fieldIndex = event.context.index;
       }
     }
+
+    // if field index is undefined model is not part of array of fields
     return isNotUndefined(fieldIndex) ? fieldIndex : 0;
   }
 
+  isPartOfArrayOfGroup(model: any): boolean {
+    return (isNotNull(model.parent)
+      && (model.parent as any).type === DYNAMIC_FORM_CONTROL_TYPE_GROUP
+      && (model.parent as any).parent
+      && (model.parent as any).parent.context
+      && (model.parent as any).parent.context.type === DYNAMIC_FORM_CONTROL_TYPE_ARRAY);
+  }
+
   public getQualdropValueMap(event): Map<string, any> {
     const metadataValueMap = new Map();
 
@@ -90,10 +93,10 @@ export class FormOperationsService {
     return (isNotUndefined(fieldIndex)) ? fieldId + '/' + fieldIndex : fieldId;
   }
 
-  public getQualdropItemPathFromEvent(event: DynamicFormControlEvent, valueMap: Map<string, any>): string {
+  public getQualdropItemPathFromEvent(event: DynamicFormControlEvent): string {
     const fieldIndex = this.getArrayIndexFromEvent(event);
     const metadataValueMap = new Map();
-    let path;
+    let path = null;
 
     const context = this.formBuilder.isQualdropGroup(event.model)
       ? (event.model.parent as DynamicFormArrayGroupModel).context
@@ -113,7 +116,7 @@ export class FormOperationsService {
     return path;
   }
 
-  public getFieldPathSegmentedFromChangeEvent(event: DynamicFormControlEvent) {
+  public getFieldPathSegmentedFromChangeEvent(event: DynamicFormControlEvent): string {
     let fieldId;
     if (this.formBuilder.isQualdropGroup(event.model as DynamicFormControlModel)) {
       fieldId = (event.model as any).qualdropId;
@@ -125,7 +128,7 @@ export class FormOperationsService {
     return fieldId;
   }
 
-  public getFieldValueFromChangeEvent(event: DynamicFormControlEvent) {
+  public getFieldValueFromChangeEvent(event: DynamicFormControlEvent): any {
     let fieldValue;
     const value = (event.model as any).value;
 
@@ -175,7 +178,7 @@ export class FormOperationsService {
 
   protected dispatchOperationsFromRemoveEvent(pathCombiner: JsonPatchOperationPathCombiner,
                                               event: DynamicFormControlEvent,
-                                              previousValue: FormFieldPreviousValueObject) {
+                                              previousValue: FormFieldPreviousValueObject): void {
     const path = this.getFieldPathFromEvent(event);
     const value = this.getFieldValueFromChangeEvent(event);
     if (this.formBuilder.isQualdropGroup(event.model as DynamicFormControlModel)) {
@@ -188,7 +191,7 @@ export class FormOperationsService {
   protected dispatchOperationsFromChangeEvent(pathCombiner: JsonPatchOperationPathCombiner,
                                               event: DynamicFormControlEvent,
                                               previousValue: FormFieldPreviousValueObject,
-                                              hasStoredValue: boolean) {
+                                              hasStoredValue: boolean): void {
     const path = this.getFieldPathFromEvent(event);
     const segmentedPath = this.getFieldPathSegmentedFromChangeEvent(event);
     const value = this.getFieldValueFromChangeEvent(event);
@@ -242,10 +245,10 @@ export class FormOperationsService {
   protected dispatchOperationsFromMap(valueMap: Map<string, any>,
                                       pathCombiner: JsonPatchOperationPathCombiner,
                                       event: DynamicFormControlEvent,
-                                      previousValue: FormFieldPreviousValueObject) {
+                                      previousValue: FormFieldPreviousValueObject): void {
     const currentValueMap = valueMap;
     if (event.type === 'remove') {
-      const path = this.getQualdropItemPathFromEvent(event, currentValueMap);
+      const path = this.getQualdropItemPathFromEvent(event);
       this.operationsBuilder.remove(pathCombiner.getPath(path));
     } else {
       if (previousValue.isPathEqual(this.formBuilder.getPath(event.model))) {