Repository: FHIR/sushi (ts)

Issue:
SUSHI changes min of `value[x]` to zero when type-slicing a choice narrowed to a single type
Given the following definition:
```
Alias: CitizenshipExtension = http://hl7.org/fhir/StructureDefinition/patient-citizenship

Profile: CitizenshipTest
Parent: Patient
* extension contains CitizenshipExtension named citizenship 0..1
* extension[citizenship].extension[code].valueCodeableConcept from http://foo.org
```

SUSHI creates a differential with this:
```
{
  "id": "Patient.extension:citizenship.extension:code.valueCodeableConcept",
  "path": "Patient.extension.extension.valueCodeableConcept",
  "min": 0,
  "max": "1",
  "binding": {
    "strength": "required",
    "valueSet": "http://foo.org"
  }
}
```

When the IG Publisher processes this, since `extension[citizenship].extension[code].valueCodeableConcept` is the same as `extension[citizenship].extension[code].value[x]` (since `value[x]` is constrained to `CodeableConcept` only), it applies `0..1` to `extension[citizenship].extension[code].value[x]` -- even though it was originally `1..1`.

I suspect SUSHI is making `valueCodeableConcept` `0..1` because when it does a type slice, it _assumes_ there is more than one choice, so it must make the slice `0..1`.  This is obviously an invalid assumption.

Pull request:
Retain choice slice min for choice of exactly 1 type
Usually, we make the min of a new type slice 0 because in a type slice, if min isn't 0, then it's not really giving you much of a choice, is it?

But in the case of applying a type slice to a choice that has been constrained to just one element... we should keep the min since that type slice really is the same as the choice it slices (since it is no choice at all anymore).

Fixes #596

To test, you can try the example in the bug report on the master branch and on this branch.  If you compare output, you'll see `min: 0` (master) vs `min: 1` (this branch).  If you run IG Publisher on the results, QA will report errors on the master version and no errors on the PR version.

Patch:
diff --git a/src/fhirtypes/ElementDefinition.ts b/src/fhirtypes/ElementDefinition.ts
index 9cdd029..b465a41 100644
--- a/src/fhirtypes/ElementDefinition.ts
+++ b/src/fhirtypes/ElementDefinition.ts
@@ -1838,7 +1838,6 @@ export class ElementDefinition {
 
   /**
    * Creates a new slice on the element.
-   * TODO: Handle re-slicing?
    * @see {@link http://hl7.org/fhir/R4/profiling.html#slicing}
    * @param {string} name - the name of the new slice
    * @param { ElementDefinitionType } [type] - the type of the new slice; if undefined it copies over this element's types
@@ -1861,7 +1860,6 @@ export class ElementDefinition {
 
     // On a new slice, delete slice.min and slice.max and then reset them
     // so that they are always captured in diff
-    const originalMax = slice.max;
     delete slice.min;
     delete slice.max;
 
@@ -1869,12 +1867,26 @@ export class ElementDefinition {
     slice.captureOriginal();
 
     slice.sliceName = this.sliceName ? `${this.sliceName}/${name}` : name;
-    // When we slice, we do not inherit min cardinality, but rather make it 0
-    // Allows multiple slices to be defined without violating cardinality of sliced element
-    // Cardinality can be later narrowed by card constraints, which check validity of narrowing
-    // According to https://chat.fhir.org/#narrow/stream/179239-tooling/topic/Slicing.201.2E.2E.3F.20element
-    slice.min = 0;
-    slice.max = originalMax;
+
+    // Usually, when we slice, we do not inherit min cardinality, but rather make it 0.
+    // This allows multiple slices to be defined without violating cardinality of sliced element.
+    // Cardinality can be later narrowed by card constraints, which check validity of narrowing.
+    // See: https://chat.fhir.org/#narrow/stream/179239-tooling/topic/Slicing.201.2E.2E.3F.20element
+    // BUT... if it's a choice that has already been constrained to a single type (thus, not really a choice),
+    // and this is a type slice, then the slice is essentially the *same* as what is being sliced and therefore
+    // should retain min card. See: https://github.com/FHIR/sushi/issues/596
+    if (
+      this.path.endsWith('[x]') &&
+      this.type.length === 1 &&
+      this.slicing?.discriminator?.[0].type === 'type' &&
+      this.slicing?.discriminator?.[0].path === '$this'
+    ) {
+      slice.min = this.min;
+    } else {
+      slice.min = 0;
+    }
+
+    slice.max = this.max;
     if (type) {
       slice.type = [type];
     }


Test patch:
diff --git a/test/fhirtypes/ElementDefinition.slicing.test.ts b/test/fhirtypes/ElementDefinition.slicing.test.ts
index a493900..e3e6d52 100644
--- a/test/fhirtypes/ElementDefinition.slicing.test.ts
+++ b/test/fhirtypes/ElementDefinition.slicing.test.ts
@@ -306,6 +306,33 @@ describe('ElementDefinition', () => {
       expect(observation.elements[valueXIdx + 2]).toEqual(integer);
     });
 
+    it('should reduce min to 0 when addings slices for specific types in a choice of types', () => {
+      const valueX = observation.elements.find(e => e.path === 'Observation.value[x]');
+      // set value[x] min to 1 to test that slice mins are reduced to 0 (to allow other choices)
+      valueX.min = 1;
+      valueX.sliceIt('type', '$this', false, 'open');
+      const quantity = valueX.addSlice('valueQuantity', new ElementDefinitionType('Quantity'));
+      const integer = valueX.addSlice('valueInteger', new ElementDefinitionType('integer'));
+      expect(quantity.min).toBe(0);
+      expect(quantity.max).toBe('1');
+      expect(integer.min).toBe(0);
+      expect(integer.max).toBe('1');
+    });
+
+    it('should NOT reduce min to 0 when adding a slice for specific types in a choice of only one type', () => {
+      // NOTE: It's up for debate whether we should make explicit choices when there only is one option anyway,
+      // but for now, we do.  So... we need to test this use case.
+      const valueX = observation.elements.find(e => e.path === 'Observation.value[x]');
+      // Remove all but the Quantity choice from value[x]
+      valueX.type = valueX.type.filter(t => t.code === 'Quantity');
+      // set value[x] min to 1 to test that slice min remains at 1
+      valueX.min = 1;
+      valueX.sliceIt('type', '$this', false, 'open');
+      const quantity = valueX.addSlice('valueQuantity', new ElementDefinitionType('Quantity'));
+      expect(quantity.min).toBe(1);
+      expect(quantity.max).toBe('1');
+    });
+
     it('should reslice when slicing a slice', () => {
       const component = observation.elements.find(e => e.id === 'Observation.component');
       const lastComponentIndex = findLastIndex(observation.elements, e =>