Repository: FHIR/sushi (ts)

Issue:
Nested Choice elements in differential can cause IG Publisher to fail
See this thread https://chat.fhir.org/#narrow/stream/215610-shorthand/topic/Mapping.20value.5Bx.5D.2Eproperty for full details. When a Profile changes nested elements of an extension as in the following example:
```
Profile: IssueProfile
Parent: http://hl7.org/fhir/StructureDefinition/Basic
Id: issue-report
Title: "IssueReport"
* extension contains IssueExtension named issue 1..1 MS
* extension[issue].valuePeriod.start MS

Extension: IssueExtension
Id: issue-extension
Title: "IssuePeriod"
* ^context.expression = "Basic"
* value[x] only Period
* valuePeriod 1..1
```
This can cause the following error in snapshot generation:
```
Error in snapshot generation: The profile http://hl7.org/fhir/sushi-test/StructureDefinition/issue-report has 1 element in the differential (id: Basic.extension:issue.valuePeriod.start) that don't have a matching element in the snapshot: check that the path and definitions are legal in the differential (including order)
```
This could potentially be a bug in the IG Publisher, or it could be an issue in how we are representing paths in our differentials, or this could be something that we are allowing to happen in SUSHI that actually is not valid. Regardless, this needs further investigation.




Pull request:
Include slices with changed children in the differential
This addresses https://standardhealthrecord.atlassian.net/browse/CIMPL-324.
If a child of a slice is referenced in the differential of the IG Publisher, the IG Publisher will incorrectly position that element if the slice itself is not present, since the Publisher relies on the sliceName on that element. So for example, the following FSH:
```
Profile: Hrm
Parent: resprate
* category[VSCat].coding.id ^short = "SOME ID"
```
would not result in the short being set on the `VSCat` slice as you would expect, because there is no element in the differential for the `VSCat` slice. This PR fixes this by counting a slice as changed if either it itself has changed, or any of its children have changed. This will cause the slice to be placed in the differential, which allows the IG Publisher to correctly render the output.

As far as I can tell, this also fixes #293, which was being caused by the same issue with children of slices in the differential without the slice itself being present.

Patch:
diff --git a/src/fhirtypes/ElementDefinition.ts b/src/fhirtypes/ElementDefinition.ts
index ad03751..ba83fc9 100644
--- a/src/fhirtypes/ElementDefinition.ts
+++ b/src/fhirtypes/ElementDefinition.ts
@@ -312,17 +312,23 @@ export class ElementDefinition {
    */
   hasDiff(): boolean {
     const original = this._original ? this._original : new ElementDefinition();
-    return PROPS.some(prop => {
-      if (prop.endsWith('[x]')) {
-        const re = new RegExp(`^${prop.slice(0, -3)}[A-Z].*$`);
-        prop = Object.keys(this).find(p => re.test(p));
-        if (prop == null) {
-          prop = Object.keys(original).find(p => re.test(p));
+    return (
+      PROPS.some(prop => {
+        if (prop.endsWith('[x]')) {
+          const re = new RegExp(`^${prop.slice(0, -3)}[A-Z].*$`);
+          prop = Object.keys(this).find(p => re.test(p));
+          if (prop == null) {
+            prop = Object.keys(original).find(p => re.test(p));
+          }
         }
-      }
-      // @ts-ignore
-      return prop && !isEqual(this[prop], original[prop]);
-    });
+        // @ts-ignore
+        return prop && !isEqual(this[prop], original[prop]);
+      }) ||
+      // When a slice has children that changed, we must treat the slice as if it
+      // differs from the original. The IG Publisher requires slices with changed
+      // children to be in the differential, or the snapshot is incorrectly generated
+      (this.sliceName && this.children().some(c => c.hasDiff()))
+    );
   }
 
   /**
@@ -361,7 +367,6 @@ export class ElementDefinition {
     }
     return diff;
   }
-
   /**
    * Gets the id of an element on the differential using the shortcut syntax described here
    * https://blog.fire.ly/2019/09/13/type-slicing-in-fhir-r4/


Test patch:
diff --git a/test/fhirtypes/ElementDefinition.test.ts b/test/fhirtypes/ElementDefinition.test.ts
index 6732f5d..48c3b75 100644
--- a/test/fhirtypes/ElementDefinition.test.ts
+++ b/test/fhirtypes/ElementDefinition.test.ts
@@ -13,6 +13,7 @@ describe('ElementDefinition', () => {
   let jsonValueX: any;
   let jsonValueId: any;
   let observation: StructureDefinition;
+  let resprate: StructureDefinition;
   let valueX: ElementDefinition;
   let valueId: ElementDefinition;
   let fisher: TestFisher;
@@ -26,12 +27,14 @@ describe('ElementDefinition', () => {
     fisher = new TestFisher().withFHIR(defs);
     // resolve observation once to ensure it is present in defs
     observation = fisher.fishForStructureDefinition('Observation');
+    resprate = fisher.fishForStructureDefinition('resprate');
     jsonObservation = defs.fishForFHIR('Observation', Type.Resource);
     jsonValueX = jsonObservation.snapshot.element[21];
     jsonValueId = jsonObservation.snapshot.element[1];
   });
   beforeEach(() => {
     observation = StructureDefinition.fromJSON(jsonObservation);
+    resprate = fisher.fishForStructureDefinition('resprate');
     valueX = ElementDefinition.fromJSON(jsonValueX);
     valueId = ElementDefinition.fromJSON(jsonValueId);
     valueX.structDef = observation;
@@ -271,6 +274,38 @@ describe('ElementDefinition', () => {
       expect(codeCoding.hasDiff()).toBeTruthy();
       expect(codeText.hasDiff()).toBeFalsy();
     });
+
+    it('should detect a diff on a slice element with changed children', () => {
+      const vsCatCoding = resprate.elements.find(e => e.id === 'Observation.category:VSCat.coding');
+      const vsCat = resprate.elements.find(e => e.id === 'Observation.category:VSCat');
+      expect(vsCatCoding.hasDiff()).toBeFalsy();
+      expect(vsCat.hasDiff()).toBeFalsy();
+      vsCatCoding.min = 2;
+      expect(vsCatCoding.hasDiff()).toBeTruthy();
+      expect(vsCat.hasDiff()).toBeTruthy();
+    });
+
+    it('should detect a diff on a slice element with changed grandchildren', () => {
+      const vsCatCodingId = resprate.elements.find(
+        e => e.id === 'Observation.category:VSCat.coding.id'
+      );
+      const vsCat = resprate.elements.find(e => e.id === 'Observation.category:VSCat');
+      expect(vsCatCodingId.hasDiff()).toBeFalsy();
+      expect(vsCat.hasDiff()).toBeFalsy();
+      vsCatCodingId.short = 'id';
+      expect(vsCatCodingId.hasDiff()).toBeTruthy();
+      expect(vsCat.hasDiff()).toBeTruthy();
+    });
+
+    it('should not detect a diff on a slice element with no changed descendents', () => {
+      const vsCatCoding = resprate.elements.find(e => e.id === 'Observation.category:VSCat.coding');
+      const vsCat = resprate.elements.find(e => e.id === 'Observation.category:VSCat');
+      expect(vsCatCoding.hasDiff()).toBeFalsy();
+      expect(vsCat.hasDiff()).toBeFalsy();
+      vsCatCoding.min = 1;
+      expect(vsCatCoding.hasDiff()).toBeFalsy();
+      expect(vsCat.hasDiff()).toBeFalsy();
+    });
   });
 
   describe('#calculateDiff', () => {
diff --git a/test/fhirtypes/StructureDefinition.test.ts b/test/fhirtypes/StructureDefinition.test.ts
index b66ed06..0dac1f1 100644
--- a/test/fhirtypes/StructureDefinition.test.ts
+++ b/test/fhirtypes/StructureDefinition.test.ts
@@ -14,6 +14,7 @@ describe('StructureDefinition', () => {
   let defs: FHIRDefinitions;
   let jsonObservation: any;
   let observation: StructureDefinition;
+  let resprate: StructureDefinition;
   let fisher: TestFisher;
   beforeAll(() => {
     defs = new FHIRDefinitions();
@@ -25,10 +26,12 @@ describe('StructureDefinition', () => {
     fisher = new TestFisher().withFHIR(defs);
     // resolve observation once to ensure it is present in defs
     observation = fisher.fishForStructureDefinition('Observation');
+
     jsonObservation = defs.fishForFHIR('Observation', Type.Resource);
   });
   beforeEach(() => {
     observation = StructureDefinition.fromJSON(jsonObservation);
+    resprate = fisher.fishForStructureDefinition('resprate');
   });
   describe('#fromJSON', () => {
     it('should load a resource properly', () => {
@@ -336,6 +339,49 @@ describe('StructureDefinition', () => {
       expect(valueStringDiff.sliceName).toBeUndefined();
       expect(valueStringSnapshot.short).toBe('the string choice');
     });
+
+    it('should properly serialize snapshot and differential for constraints on children of slices', () => {
+      let json = resprate.toJSON();
+      const vsCatCodingOriginal = json.snapshot.element.find(
+        (e: any) => e.id === 'Observation.category:VSCat.coding'
+      );
+      const vsCatOriginal = json.snapshot.element.find(
+        (e: any) => e.id === 'Observation.category:VSCat'
+      );
+      let vsCatCoding = resprate.elements.find(
+        (e: any) => e.id === 'Observation.category:VSCat.coding'
+      );
+      vsCatCoding.short = 'Hello I am a change';
+      json = resprate.toJSON();
+      vsCatCoding = json.snapshot.element.find(
+        (e: any) => e.id === 'Observation.category:VSCat.coding'
+      );
+      const vsCat = json.snapshot.element.find((e: any) => e.id === 'Observation.category:VSCat');
+
+      // Check Observation.category:VSCat and Observation.category:VSCat.coding elements on the snapshot
+      expect(vsCatCoding.short).toBe('Hello I am a change');
+      delete vsCatCoding.short;
+      delete vsCatCodingOriginal.short;
+      expect(vsCatCoding).toEqual(vsCatCodingOriginal);
+      expect(vsCat).toEqual(vsCatOriginal);
+
+      const vsCatCodingDiff = json.differential.element.find(
+        (e: any) => e.id === 'Observation.category:VSCat.coding'
+      );
+      const vsCatDiff = json.differential.element.find(
+        (e: any) => e.id === 'Observation.category:VSCat'
+      );
+      expect(vsCatCodingDiff).toEqual({
+        id: 'Observation.category:VSCat.coding',
+        path: 'Observation.category.coding',
+        short: 'Hello I am a change'
+      });
+      expect(vsCatDiff).toEqual({
+        id: 'Observation.category:VSCat',
+        path: 'Observation.category',
+        sliceName: 'VSCat'
+      });
+    });
   });
 
   describe('#newElement', () => {