Repository: FHIR/sushi (ts)

Issue:
Repeated Slices are Allowed
If we have profile A with a slice on component called foo:
```
Profile: A
Parent: Observation
* component contains foo 0..1
```
And then we make a profile B that inherits from A, and we incorrectly slice component again with foo:
```
Profile: B
Parent: A
* component contains foo 1..1
```
We end up with two `Observation.component:foo` elements in the snapshot and differential. This is isn't correct, and we should probably either:
1) Ignore the `contains` and apply the card rules or flag rules to the original foo slice, while warning the user that using `contains` has no effect.
2) Ignore all the rules including card and flag rules, and emit an error indicating that you can't put an existing slice in a contains.

See https://chat.fhir.org/#narrow/stream/215610-shorthand/topic/profiling.20on.20external.20IG.20and.20named.20slices for some more details.

Pull request description:
Do not add the same slice twice
Fixes #199 and completes task [CIMPL-372](https://standardhealthrecord.atlassian.net/browse/CIMPL-372).

When a slice is added to an element, check to see if there is an existing slice on that element with the same name. If there is, log an error and do not add the duplicate slice.

Patch:
diff --git a/src/errors/DuplicateSliceError.ts b/src/errors/DuplicateSliceError.ts
new file mode 100644
index 0000000..df9de76
--- /dev/null
+++ b/src/errors/DuplicateSliceError.ts
@@ -0,0 +1,5 @@
+export class DuplicateSliceError extends Error {
+  constructor(structDef: string, element: string, slice: string) {
+    super(`Slice named ${slice} already exists on element ${element} of ${structDef}`);
+  }
+}
diff --git a/src/errors/index.ts b/src/errors/index.ts
index 296b7d9..eba507e 100644
--- a/src/errors/index.ts
+++ b/src/errors/index.ts
@@ -19,6 +19,7 @@ export * from './ValueSetFilterMissingValueError';
 export * from './ValueSetFilterValueTypeError';
 export * from './SlicingDefinitionError';
 export * from './SlicingNotDefinedError';
+export * from './DuplicateSliceError';
 export * from './TypeNotFoundError';
 export * from './UnitMismatchError';
 export * from './WideningCardinalityError';
diff --git a/src/fhirtypes/ElementDefinition.ts b/src/fhirtypes/ElementDefinition.ts
index 05d6279..bdab058 100644
--- a/src/fhirtypes/ElementDefinition.ts
+++ b/src/fhirtypes/ElementDefinition.ts
@@ -28,7 +28,8 @@ import {
   MultipleStandardsStatusError,
   InvalidMappingError,
   InvalidFHIRIdError,
-  FixingNonResourceError
+  FixingNonResourceError,
+  DuplicateSliceError
 } from '../errors';
 import { setPropertyOnDefinitionInstance, splitOnPathPeriods, isInheritedResource } from './common';
 import { Fishable, Type, Metadata, logger } from '../utils';
@@ -1692,10 +1693,17 @@ export class ElementDefinition {
     if (!this.slicing) {
       throw new SlicingNotDefinedError(this.id, name);
     }
+
     const slice = this.clone(true);
     delete slice.slicing;
     slice.id = `${this.id}:${name}`;
 
+    // if a slice with the same id already exists, don't add it again
+    const existingSlice = this.structDef.findElement(slice.id);
+    if (existingSlice) {
+      throw new DuplicateSliceError(this.structDef.name, this.id, name);
+    }
+
     // Capture the original so that the differential only contains changes from this point on.
     slice.captureOriginal();
 


Test patch:
diff --git a/test/export/StructureDefinitionExporter.test.ts b/test/export/StructureDefinitionExporter.test.ts
index c0d1d98..19979aa 100644
--- a/test/export/StructureDefinitionExporter.test.ts
+++ b/test/export/StructureDefinitionExporter.test.ts
@@ -1734,6 +1734,70 @@ describe('StructureDefinitionExporter', () => {
     expect(fooSlice).toBeDefined();
   });
 
+  it('should report an error and not add the slice when a ContainsRule tries to add a slice that already exists', () => {
+    // Profile: DoubleSlice
+    // Parent: resprate
+    // * code.coding contains onlySlice
+    // * code.coding contains onlySlice
+
+    const profile = new Profile('DoubleSlice');
+    profile.parent = 'resprate';
+
+    const rule1 = new ContainsRule('code.coding');
+    rule1.items = [{ name: 'onlySlice' }];
+    const rule2 = new ContainsRule('code.coding');
+    rule2.items = [{ name: 'onlySlice' }];
+    profile.rules.push(rule1, rule2);
+
+    exporter.exportStructDef(profile);
+    const sd = pkg.profiles[0];
+    const baseStructDef = fisher.fishForStructureDefinition('resprate');
+    const onlySlice = sd.elements.find(e => e.id === 'Observation.code.coding:onlySlice');
+
+    expect(sd.elements.length).toBe(baseStructDef.elements.length + 1);
+    expect(onlySlice).toBeDefined();
+    expect(loggerSpy.getLastMessage('error')).toMatch(
+      /Slice named onlySlice already exists on element Observation\.code\.coding of DoubleSlice/s
+    );
+  });
+
+  it('should report an error and not add the slice when a ContainsRule tries to add a slice that was created on the parent', () => {
+    // Profile: FirstProfile
+    // Parent: resprate
+    // * code.coding contains onlySlice
+
+    // Profile: SecondProfile
+    // Parent: FirstProfile
+    // * code.coding contains onlySlice
+
+    const firstProfile = new Profile('FirstProfile');
+    firstProfile.parent = 'resprate';
+    const firstRule = new ContainsRule('code.coding');
+    firstRule.items = [{ name: 'onlySlice' }];
+    firstProfile.rules.push(firstRule);
+    doc.profiles.set(firstProfile.name, firstProfile);
+
+    const secondProfile = new Profile('SecondProfile');
+    secondProfile.parent = 'FirstProfile';
+    const secondRule = new ContainsRule('code.coding');
+    secondRule.items = [{ name: 'onlySlice' }];
+    secondProfile.rules.push(secondRule);
+    doc.profiles.set(secondProfile.name, secondProfile);
+
+    exporter.exportStructDef(firstProfile);
+    exporter.exportStructDef(secondProfile);
+    const [firstSd, secondSd] = pkg.profiles;
+    const firstSlice = firstSd.elements.find(e => e.id === 'Observation.code.coding:onlySlice');
+    const secondSlice = secondSd.elements.find(e => e.id === 'Observation.code.coding:onlySlice');
+
+    expect(firstSlice).toBeDefined();
+    expect(secondSlice).toBeDefined();
+    expect(firstSd.elements.length).toEqual(secondSd.elements.length);
+    expect(loggerSpy.getLastMessage('error')).toMatch(
+      /Slice named onlySlice already exists on element Observation\.code\.coding of SecondProfile/s
+    );
+  });
+
   it('should not apply a ContainsRule on an element without defined slicing', () => {
     const profile = new Profile('Foo');
     profile.parent = 'resprate';