Repository: FHIR/sushi (ts)

Issue:
Issues w/ cardinality counting and re-slices
SUSHI seems to struggle with cardinality counting on instances of profiles with re-slicing.  There are two issues:
* SUSHI infers the wrong minimum cardinality for the element that is sliced in the profile
* SUSHI does not recognize re-slices in an instance as fulfilling cardinality of the parent slice

Given this example (https://fshschool.org/FSHOnline/#/share/3opCADp):
```
Alias: HeartRate = http://hl7.org/fhir/StructureDefinition/heartrate
Alias: LOINC = http://loinc.org

Profile: MyBundleProfile
Parent: Bundle
* type = #collection
* entry ^slicing.discriminator.type = #type
* entry ^slicing.discriminator.path = "resource"
* entry ^slicing.rules = #open
* entry contains observations 1..*
* entry[observations].resource only Observation
* entry[observations] ^slicing.discriminator.type = #profile
* entry[observations] ^slicing.discriminator.path = "resource"
* entry[observations] ^slicing.rules = #open
* entry[observations] contains heartrate 1..1 MS
* entry[observations][heartrate].resource only HeartRate

Instance: MyBundleInstance
InstanceOf: MyBundleProfile
* entry[observations][heartrate].resource = MyHeartRate

Instance: MyHeartRate
InstanceOf: HeartRate
* status = #final
* subject = Reference(MyPatient)
* effectiveDateTime = "2021-01-28"
* valueQuantity = 44 '/min' "beats/min"

Instance: MyPatient
InstanceOf: Patient
* name.given = "Bob"
```

SUSHI produces these errors:
```
error Element Bundle.entry has minimum cardinality 2 but occurs 1 time(s).
  Line: 18 - 20

error Element Bundle.entry:observations has minimum cardinality 1 but occurs 0 time(s).
  Line: 18 - 20
```

Reported on Zulip: https://chat.fhir.org/#narrow/stream/215610-shorthand/topic/Slicing.20on.20slices.3F

Pull request:
Calculate cardinality correctly when reslicing elements and exporting instances
Fixes #742 and completes task [CIMPL-651](https://standardhealthrecord.atlassian.net/browse/CIMPL-651).

When constraining the cardinality of a reslice, it may be necessary to update the minimum cardinality of the parent slice, that is, the slice being resliced. Check the sliced element and all ancestor slices when constraining the cardinality, updating other cardinalities as necessary.

The new parentSliceName field on ElementDefinition is used to track slice lineage. A slice name may legally contain a forward slash, which is also used as the character to separate the slice name from the reslice name. Therefore, splitting a slice name on the forward slash character is not a reliable way of determining the slice name of ancestor slices.

When validating required child elements in an instance, the required elements may be on the element, a slice on that element, or a reslice on that element. All of these should count when determining how many elements are present for the purpose of validating element cardinality.

Patch:
diff --git a/src/export/InstanceExporter.ts b/src/export/InstanceExporter.ts
index 846b2da..a9c3cc7 100644
--- a/src/export/InstanceExporter.ts
+++ b/src/export/InstanceExporter.ts
@@ -168,8 +168,13 @@ export class InstanceExporter implements Fishable {
           return;
         }
         // Filter so that if the child is a slice, we only count relevant slices
+        // A slice is relevant if it is the child slice or is a reslice of child.
+        // Typically, a sliceName that starts with child.sliceName + '/' is a reslice.
         instanceChild = instanceChild.filter(
-          (arrayEl: any) => !child.sliceName || arrayEl?._sliceName === child.sliceName
+          (arrayEl: any) =>
+            !child.sliceName ||
+            arrayEl?._sliceName === child.sliceName ||
+            arrayEl?._sliceName?.toString().startsWith(`${child.sliceName}/`)
         );
         instanceChild.forEach((arrayEl: any) => {
           if (arrayEl != null) this.validateRequiredChildElements(arrayEl, child, fshDefinition);
diff --git a/src/fhirtypes/ElementDefinition.ts b/src/fhirtypes/ElementDefinition.ts
index 3db5541..2a0d4ac 100644
--- a/src/fhirtypes/ElementDefinition.ts
+++ b/src/fhirtypes/ElementDefinition.ts
@@ -535,11 +535,24 @@ export class ElementDefinition {
     // If element is a slice
     const slicedElement = this.slicedElement();
     if (slicedElement) {
-      // Check that slicedElement max >= new sum of mins
-      const sumOfMins = slicedElement.checkSumOfSliceMins(slicedElement.max, min - this.min);
-      // If new sum of mins > slicedElement min, increase slicedElement min
-      if (sumOfMins > slicedElement.min) {
-        slicedElement.min = sumOfMins;
+      const parentSlice = this.findParentSlice();
+      const sliceSiblings = this.structDef.elements.filter(
+        el =>
+          this !== el &&
+          slicedElement === el.slicedElement() &&
+          parentSlice === el.findParentSlice()
+      );
+      const newParentMin = min + sliceSiblings.reduce((sum, el) => sum + el.min, 0);
+      // if this is a reslice, the parent element will also be a slice of the sliced element.
+      // if this is not a reslice, the parent element is the sliced element.
+      const parentElement = parentSlice ?? slicedElement;
+      // Check that parentElement max >= new sum of mins
+      if (parentElement.max !== '*' && newParentMin > parseInt(parentElement.max)) {
+        throw new InvalidSumOfSliceMinsError(newParentMin, parentElement.max, parentElement.id);
+      }
+      // If new sum of mins > parentElement min, increase parentElement min
+      if (newParentMin > parentElement.min) {
+        parentElement.constrainCardinality(newParentMin, '');
       }
     }
 
@@ -588,6 +601,26 @@ export class ElementDefinition {
     }
   }
 
+  findParentSlice(): ElementDefinition {
+    if (this.sliceName) {
+      const slicedElement = this.slicedElement();
+      const parentNameParts = this.sliceName.split('/').slice(0, -1);
+      const potentialParentNames = parentNameParts
+        .map((_part, i) => {
+          return parentNameParts.slice(0, i + 1).join('/');
+        })
+        .reverse();
+      for (const parentName of potentialParentNames) {
+        const potentialParent = this.structDef.elements.find(el => {
+          return el.sliceName === parentName && el.slicedElement() === slicedElement;
+        });
+        if (potentialParent) {
+          return potentialParent;
+        }
+      }
+    }
+  }
+
   /**
    * Checks if the sum of slice mins exceeds the max of sliced element, and returns
    * the sum if so.
@@ -1931,7 +1964,6 @@ export class ElementDefinition {
 
     // Capture the original so that the differential only contains changes from this point on.
     slice.captureOriginal();
-
     slice.sliceName = this.sliceName ? `${this.sliceName}/${name}` : name;
 
     // Usually, when we slice, we do not inherit min cardinality, but rather make it 0.


Test patch:
diff --git a/test/export/InstanceExporter.test.ts b/test/export/InstanceExporter.test.ts
index ca3f8ad..afb8627 100644
--- a/test/export/InstanceExporter.test.ts
+++ b/test/export/InstanceExporter.test.ts
@@ -1983,6 +1983,60 @@ describe('InstanceExporter', () => {
       expect(loggerSpy.getAllMessages('error')).toHaveLength(0);
     });
 
+    it('should not log an error when a reslice element fulfills a cardinality constraint', () => {
+      // Profile: TestPatient
+      // Parent: Patient
+      // * identifier ^slicing.discriminator.type = #value
+      // * identifier ^slicing.discriminator.path = "value"
+      // * identifier contains ParentSlice 1..1
+      // * identifier[ParentSlice] ^slicing.discriminator.type = #value
+      // * identifier[ParentSlice] ^slicing.discriminator.path = "value"
+      // * identifier[ParentSlice] contains ChildSlice 1..1
+      const identifierSlicing = new CaretValueRule('identifier');
+      identifierSlicing.caretPath = 'slicing.discriminator.path';
+      identifierSlicing.value = 'value';
+      const identifierContains = new ContainsRule('identifier');
+      identifierContains.items.push({ name: 'ParentSlice' });
+      const parentCard = new CardRule('identifier[ParentSlice]');
+      parentCard.min = 1;
+      parentCard.max = '1';
+      const parentSlicing = new CaretValueRule('identifier[ParentSlice]');
+      parentSlicing.caretPath = 'slicing.discriminator.path';
+      parentSlicing.value = 'value';
+      const parentContains = new ContainsRule('identifier[ParentSlice]');
+      parentContains.items.push({ name: 'ChildSlice' });
+      const childCard = new CardRule('identifier[ParentSlice][ChildSlice]');
+      childCard.min = 1;
+      childCard.max = '1';
+      patient.rules.push(
+        identifierSlicing,
+        identifierContains,
+        parentCard,
+        parentSlicing,
+        parentContains,
+        childCard
+      );
+      // Instance: PatientInstance
+      // InstanceOf: TestPatient
+      // * identifier[ParentSlice][ChildSlice] = SomeIdentifier
+      const identifierAssignment = new AssignmentRule('identifier[ParentSlice][ChildSlice]');
+      identifierAssignment.isInstance = true;
+      identifierAssignment.value = 'SomeIdentifier';
+      patientInstance.rules.push(identifierAssignment);
+      // Instance: SomeIdentifier
+      // InstanceOf: Identifier
+      // * value = "Something"
+      const someIdentifier = new Instance('SomeIdentifier');
+      someIdentifier.instanceOf = 'Identifier';
+      const valueAssignment = new AssignmentRule('value');
+      valueAssignment.value = 'Something';
+      someIdentifier.rules.push(valueAssignment);
+      doc.instances.set(someIdentifier.name, someIdentifier);
+
+      exportInstance(patientInstance);
+      expect(loggerSpy.getAllMessages('error')).toHaveLength(0);
+    });
+
     it('should only export an instance once', () => {
       const bundleInstance = new Instance('MyBundle');
       bundleInstance.instanceOf = 'Bundle';
diff --git a/test/fhirtypes/ElementDefinition.constrainCardinality.test.ts b/test/fhirtypes/ElementDefinition.constrainCardinality.test.ts
index 4fac116..c8b5cce 100644
--- a/test/fhirtypes/ElementDefinition.constrainCardinality.test.ts
+++ b/test/fhirtypes/ElementDefinition.constrainCardinality.test.ts
@@ -168,6 +168,57 @@ describe('ElementDefinition', () => {
       expect(category.min).toBe(3);
     });
 
+    it('should not change the sliced element min when a slice is resliced with the same min', () => {
+      const category = respRate.elements.find(e => e.id === 'Observation.category');
+      const vsCat = respRate.elements.find(e => e.id === 'Observation.category:VSCat');
+      // apply slicing to vsCat
+      vsCat.sliceIt('value', 'coding.code');
+      // * category[VSCat] contains FooSlice 1..1
+      const fooSlice = vsCat.addSlice('FooSlice');
+      fooSlice.constrainCardinality(1, '1');
+      expect(category.min).toBe(1);
+      expect(vsCat.min).toBe(1);
+    });
+
+    it('should update the min for the sliced element and the parent slice when the sum of reslice mins is constrained greater than them', () => {
+      const category = respRate.elements.find(e => e.id === 'Observation.category');
+      const parentSlice = category.addSlice('ParentSlice');
+      parentSlice.constrainCardinality(1, '*');
+      parentSlice.sliceIt('value', 'coding.code');
+      const childSlice = parentSlice.addSlice('ChildSlice');
+      childSlice.constrainCardinality(2, '2');
+      expect(parentSlice.min).toBe(2); // contains 2 ChildSlice
+      expect(category.min).toBe(3); // contains 2 ParentSlice/ChildSlice + 1 VSCat
+    });
+
+    it('should update the min for the sliced element and ancestor slices, but not other slices', () => {
+      const category = respRate.elements.find(e => e.id === 'Observation.category');
+      const parentOne = category.addSlice('ParentOne');
+      parentOne.sliceIt('value', 'coding.code');
+      const childOne = parentOne.addSlice('ChildOne');
+      const childTwo = parentOne.addSlice('ChildTwo');
+      const parentTwo = category.addSlice('ParentTwo');
+      parentTwo.sliceIt('value', 'coding.code');
+      const childThree = parentTwo.addSlice('ChildThree');
+      const childFour = parentTwo.addSlice('ChildFour');
+
+      childOne.constrainCardinality(1, '*');
+      childTwo.constrainCardinality(2, '*');
+      childThree.constrainCardinality(3, '*');
+      childFour.constrainCardinality(4, '*');
+
+      // the children should be unchanged from the values that were set
+      expect(childOne.min).toBe(1);
+      expect(childTwo.min).toBe(2);
+      expect(childThree.min).toBe(3);
+      expect(childFour.min).toBe(4);
+      // the parents should be the sums of only their children, regardless of other slices
+      expect(parentOne.min).toBe(3);
+      expect(parentTwo.min).toBe(7);
+      // the sliced element should be the sum of VSCat + ParentOne + ParentTwo = 1 + 3 + 7 = 11
+      expect(category.min).toBe(11);
+    });
+
     it('should throw InvalidSumOfSliceMinsError when sliced element max is constrained less than sum of slice mins', () => {
       const category = respRate.elements.find(e => e.id === 'Observation.category');
       const fooSlice = category.addSlice('FooSlice');