Merge pull request #592 from hmcts/bugfix/EUI-2681-retain-hidden-value-fix-for-complex-types

EUI-2681: Fix retain hidden value support for complex field types

Author: Vamshi-Muniganti

RELEASE-NOTES.md

@@ -1,4 +1,7 @@
 ## RELEASE NOTES
+### Version 2.64.43-retain-hidden-value-fix-for-complex-types
+**EUI-2681** Fix: Do not set a field control's value to `null` if it corresponds to a complex `CaseField` type
+
 ### Version 2.64.42-retain-hidden-value-support
 **EUI-1783** Delete hidden field value except if `retain_hidden_value` flag is true
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@hmcts/ccd-case-ui-toolkit",
-  "version": "2.64.42-retain-hidden-value-support",
+  "version": "2.64.43-retain-hidden-value-fix-for-complex-types",
   "engines": {
     "yarn": "^1.12.3",
     "npm": "^5.6.0"
@@ -173,11 +173,12 @@
     "mem": "^6.0.0",
     "minimatch": "^3.0.2",
     "minimist": "1.2.3",
+    "node-fetch": "2.6.1",
+    "node-forge": "^0.10.0",
     "npm-registry-fetch": "^4.0.5",
     "serialize-javascript": "^3.1.0",
     "tree-kill": "1.2.2",
-    "yargs-parser": "18.1.2",
-    "node-fetch": "2.6.1"
+    "yargs-parser": "18.1.2"
   },
   "publishConfig": {
     "access": "public"

src/shared/components/case-editor/case-edit-submit/case-edit-submit.component.spec.ts

@@ -43,11 +43,16 @@ describe('CaseEditSubmitComponent', () => {
   const FORM_GROUP = new FormGroup({
     'data': new FormGroup({ 'PersonLastName': new FormControl('Khaleesi') })
   });
+  const COMPLEX_ELEMENT_HIDDEN = new FormGroup({
+    'childField1': new FormControl('Child field of complex type')
+  });
+  COMPLEX_ELEMENT_HIDDEN.disable();
   const FORM_GROUP_WITH_HIDDEN_FIELDS = new FormGroup({
     'data': new FormGroup({
       'field1': new FormControl({ value: 'Hidden value to be retained', disabled: true }),
       'field2': new FormControl({ value: 'Hidden value not to be retained', disabled: true }),
-      'field3': new FormControl('Hide both')
+      'field3': new FormControl('Hide all'),
+      'complexField1': COMPLEX_ELEMENT_HIDDEN
     })
   });
   let caseEditComponent: any;
@@ -58,10 +63,12 @@ describe('CaseEditSubmitComponent', () => {
   let profileNotifier;
   let profileNotifierSpy;
   let casesReferencePipe: any;
-  let caseField1: CaseField = aCaseField('field1', 'field1', 'Text', 'OPTIONAL', 3);
-  let caseField2: CaseField = aCaseField('field2', 'field2', 'Text', 'OPTIONAL', 2);
-  let caseField3: CaseField = aCaseField('field3', 'field3', 'Text', 'OPTIONAL', 1);
-  let caseFieldRetainHiddenValue: CaseField = aCaseField('field1', 'field1', 'Text', 'OPTIONAL', 3, null, true);
+  const caseField1: CaseField = aCaseField('field1', 'field1', 'Text', 'OPTIONAL', 4);
+  const caseField2: CaseField = aCaseField('field2', 'field2', 'Text', 'OPTIONAL', 3);
+  const caseField3: CaseField = aCaseField('field3', 'field3', 'Text', 'OPTIONAL', 2);
+  const complexSubField: CaseField = aCaseField('childField1', 'childField1', 'Text', 'OPTIONAL', 1);
+  const complexCaseField: CaseField = aCaseField('complexField1', 'complexField1', 'Complex', 'OPTIONAL', 1, [complexSubField]);
+  const caseFieldRetainHiddenValue: CaseField = aCaseField('field1', 'field1', 'Text', 'OPTIONAL', 4, null, true);
   const $EVENT_NOTES = By.css('#fieldset-event');
   let cancelled: any;
   let snapshot: any;
@@ -341,16 +348,16 @@ describe('CaseEditSubmitComponent', () => {
     });
 
     it('should sort case fields with show_summary_content_option', () => {
-      expect(comp.eventTrigger.case_fields[0].show_summary_content_option).toBe(3);
-      expect(comp.eventTrigger.case_fields[1].show_summary_content_option).toBe(2);
-      expect(comp.eventTrigger.case_fields[2].show_summary_content_option).toBe(1);
+      expect(comp.eventTrigger.case_fields[0].show_summary_content_option).toBe(4);
+      expect(comp.eventTrigger.case_fields[1].show_summary_content_option).toBe(3);
+      expect(comp.eventTrigger.case_fields[2].show_summary_content_option).toBe(2);
       expect(orderService.sort).toHaveBeenCalledWith(
         comp.eventTrigger.case_fields,
         CaseEditSubmitComponent.SHOW_SUMMARY_CONTENT_COMPARE_FUNCTION);
       expect(comp.showSummaryFields.length).toBe(3);
-      expect(comp.showSummaryFields[0].show_summary_content_option).toBe(1);
-      expect(comp.showSummaryFields[1].show_summary_content_option).toBe(2);
-      expect(comp.showSummaryFields[2].show_summary_content_option).toBe(3);
+      expect(comp.showSummaryFields[0].show_summary_content_option).toBe(2);
+      expect(comp.showSummaryFields[1].show_summary_content_option).toBe(3);
+      expect(comp.showSummaryFields[2].show_summary_content_option).toBe(4);
     });
 
     it('should return "Cancel" text label for cancel button when save and resume disabled', () => {
@@ -709,12 +716,14 @@ describe('CaseEditSubmitComponent', () => {
       aWizardPage('page1', 'Page 1', 1),
     ];
     const firstPage = pages[0];
-    caseFieldRetainHiddenValue.show_condition = 'field3!="Hide both"';
-    caseField2.show_condition = 'field3!="Hide both"';
+    caseFieldRetainHiddenValue.show_condition = 'field3!="Hide all"';
+    caseField2.show_condition = 'field3!="Hide all"';
+    complexCaseField.show_condition = 'field3!="Hide all"';
     const WP_FIELD_1: WizardPageField = { case_field_id: caseFieldRetainHiddenValue.id };
     const WP_FIELD_2: WizardPageField = { case_field_id: caseField2.id };
     const WP_FIELD_3: WizardPageField = { case_field_id: caseField3.id };
-    firstPage.wizard_page_fields = [WP_FIELD_1, WP_FIELD_2, WP_FIELD_3];
+    const WP_FIELD_4: WizardPageField = { case_field_id: complexCaseField.id };
+    firstPage.wizard_page_fields = [WP_FIELD_1, WP_FIELD_2, WP_FIELD_3, WP_FIELD_4];
     wizard = new Wizard(pages);
     const queryParamMapNoProfile = createSpyObj('queryParamMap', ['get']);
     const snapshotNoProfile = {
@@ -749,15 +758,18 @@ describe('CaseEditSubmitComponent', () => {
     beforeEach(async(() => {
       // Need to set the page case_fields here because, for some reason, if set initially then these get overridden
       // by some unknown default!
-      firstPage.case_fields = [caseFieldRetainHiddenValue, caseField2, caseField3];
+      firstPage.case_fields = [caseFieldRetainHiddenValue, caseField2, caseField3, complexCaseField];
       orderService = new OrderService();
       casesReferencePipe = createSpyObj<CaseReferencePipe>('caseReference', ['transform']);
       cancelled = createSpyObj('cancelled', ['emit'])
       caseEditComponent = {
         'form': FORM_GROUP_WITH_HIDDEN_FIELDS,
         'fieldsPurger': new FieldsPurger(fieldsUtils),
         'data': '',
-        'eventTrigger': { 'case_fields': [caseFieldRetainHiddenValue, caseField2, caseField3], 'can_save_draft': true },
+        'eventTrigger': {
+          'case_fields': [caseFieldRetainHiddenValue, caseField2, caseField3, complexCaseField],
+          'can_save_draft': true
+        },
         'wizard': wizard,
         'hasPrevious': () => true,
         'getPage': () => firstPage,
@@ -812,7 +824,7 @@ describe('CaseEditSubmitComponent', () => {
       fixture.detectChanges();
     });
 
-    it('should submit CaseEventData with null for any hidden fields whose values are not to be retained', () => {
+    it('should submit CaseEventData with null for any hidden fields (excluding Complex types) whose values are not to be retained', () => {
       // Trigger the clearing of hidden fields by invoking next()
       caseEditComponent.next();
 
@@ -821,7 +833,7 @@ describe('CaseEditSubmitComponent', () => {
       expect(caseEditComponent.submit).toHaveBeenCalledWith({
         data: {
           field2: null,
-          field3: 'Hide both'
+          field3: 'Hide all'
         },
         event_token: undefined,
         ignore_warning: false

src/shared/services/fields/fields.purger.ts

@@ -70,7 +70,14 @@ export class FieldsPurger {
     // Removing the field means that it is *NOT* sent to the CCD backend, which means no changes are made in the data.
     // This is OK *if* the hidden value needs to be retained, i.e. field.retain_hidden_value = true, but the default
     // should be to set it to null and allow it to be sent as such.
-    if (field.retain_hidden_value) {
+    //
+    // *Complex* field types, i.e. those that contain other fields, should assume the same behaviour as before, which
+    // is to reset the `CaseField` value and remove the form control. The value of the *control* itself should be left
+    // alone.
+    //
+    // The only reliable check if a field is a complex type or not is to obtain its corresponding control and check
+    // whether it's a `FormControl` (simple field) or `FormGroup` (complex field) instance.
+    if (field.retain_hidden_value || (form.get('data') as FormGroup).get(field.id) instanceof FormGroup) {
       // Reset the field value and remove its control. This does NOT update it in the CCD backend, since it is just
       // removed from the JSON structure
       if (Array.isArray(field.value)) {
@@ -84,7 +91,9 @@ export class FieldsPurger {
     } else {
       // Set the value of the field's control to null. This DOES update the value in the CCD backend
       const fieldControl = (form.get('data') as FormGroup).get(field.id);
-      fieldControl.setValue(null);
+      if (fieldControl) {
+        fieldControl.setValue(null);
+      }
     }
   }
 

yarn.lock

@@ -7497,10 +7497,10 @@ [email protected], node-fetch@^2.2.0:
   resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-2.6.1.tgz#045bd323631f76ed2e2b55573394416b639a0052"
   integrity sha512-V4aYg89jEoVRxRb2fJdAg8FHvI7cEyYdVAh94HH0UIK8oJxUfkjlDQN9RbMx+bEjP7+ggMiFRprSti032Oipxw==
 
-[email protected]:
-  version "0.9.0"
-  resolved "https://registry.yarnpkg.com/node-forge/-/node-forge-0.9.0.tgz#d624050edbb44874adca12bb9a52ec63cb782579"
-  integrity sha512-7ASaDa3pD+lJ3WvXFsxekJQelBKRpne+GOVbLbtHYdd7pFspyeuJHnWfLplGf3SwKGbfs/aYl5V/JCIaHVUKKQ==
+[email protected], node-forge@^0.10.0:
+  version "0.10.0"
+  resolved "https://registry.yarnpkg.com/node-forge/-/node-forge-0.10.0.tgz#32dea2afb3e9926f02ee5ce8794902691a676bf3"
+  integrity sha512-PPmu8eEeG9saEUvI97fm4OYxXVB6bFvyNTyiUOBichBpFG8A1Ljw3bY62+5oOjDEMHRnd0Y7HQ+x7uzxOzC6JA==
 
 node-gyp@^3.8.0:
   version "3.8.0"