Repository: FHIR/sushi (ts)

Issue:
Export StructureDefinition with only new mappings
The `StructureDefinition.mapping` element does not need to repeat all mappings from its parent. The IG Publisher will add in the parent mappings during processing. SUSHI should export StructureDefinitions with only the newly introduced mappings and then allow the IG Publisher to handle how they are integrated. Currently, this will result in new mappings being _first_ in the array, which then results in them being listed first in the rendered IG -- which is what authors generally want.

We should, however, consider an exception when SUSHI is run w/ snapshot mode on -- because in that case, we most likely want a complete StructureDefinition with all the mappings (since IG Publisher is less likely to be used in this case). If we do this, consider putting new mappings at the start of the array, like the publisher does.

We may be able to use an approach similar to the `ADDITIVE_PROPS` approach we use in `ElementDefinition`.

See: https://chat.fhir.org/#narrow/stream/179252-IG-creation/topic/mapping.20from.20IG.20listed.20first/near/416509917

Pull request:
Export new mappings only (or at top of list in snapshot mode)
Fixes #1422.

This PR makes it so that only new mappings are included in the `StructureDefinition.mapping` list when running SUSHI normally (i.e. not with the `--snapshot` flag). When running SUSHI with the `--snapshot -s` flag, the new mappings will appear at the top of the `StructureDefinition.mapping` list.

SUSHI also supports using FSH `Mapping`s to add a new `comment` to existing mappings on the base definition. When that happens, those mappings are included in the list (or moved to the top with `--snapshot`).

I ran a regression for this in both regular (no `-s`) mode and `-s` mode. Without the `-s` (so a regular regression against `master`), there are a lot of changes because every `mapping` list changes. It is either removed entirely or the inherited mapping elements are removed. Running a regression comparing repos with `-s` on results in 35 repos with changes, which are all repos that added mappings, so the mapping array order has changed. There were a couple repos that directly added mappings with assignment rules. This previously overwrote inherited mapping entries, but those mappings are now re-added at the end of the list, so I think this is an improvement.

Patch:
diff --git a/src/export/StructureDefinitionExporter.ts b/src/export/StructureDefinitionExporter.ts
index 9062934..332b8d5 100644
--- a/src/export/StructureDefinitionExporter.ts
+++ b/src/export/StructureDefinitionExporter.ts
@@ -662,6 +662,7 @@ export class StructureDefinitionExporter implements Fishable {
       // for consistency, delete rather than leaving null-valued
       delete structDef.elements[0].modifierExtension;
     }
+    structDef.captureOriginalMapping();
     structDef.elements[0].captureOriginal();
 
     // The remaining logic only pertains to logicals and resources, so return here if otherwise
diff --git a/src/fhirtypes/StructureDefinition.ts b/src/fhirtypes/StructureDefinition.ts
index 10b1eed..fce5ef5 100644
--- a/src/fhirtypes/StructureDefinition.ts
+++ b/src/fhirtypes/StructureDefinition.ts
@@ -1,6 +1,9 @@
 import upperFirst from 'lodash/upperFirst';
 import cloneDeep from 'lodash/cloneDeep';
 import escapeRegExp from 'lodash/escapeRegExp';
+import differenceWith from 'lodash/differenceWith';
+import isEqual from 'lodash/isEqual';
+import isEmpty from 'lodash/isEmpty';
 import sanitize from 'sanitize-filename';
 import { ElementDefinition, ElementDefinitionType, LooseElementDefJSON } from './ElementDefinition';
 import { Meta } from './specialTypes';
@@ -88,6 +91,7 @@ export class StructureDefinition {
    * A StructureDefinition instance of StructureDefinition itself.  Needed for supporting escape syntax.
    */
   private _sdStructureDefinition: StructureDefinition;
+  private originalMapping: StructureDefinitionMapping[] = [];
 
   validate(): ValidationError[] {
     const validationErrors: ValidationError[] = [];
@@ -403,6 +407,36 @@ export class StructureDefinition {
     this.elements.forEach(e => e.clearOriginal());
   }
 
+  /**
+   * Capture the current state of the mapping array. This is used to determine
+   * which mappings have been added and which are inherited.
+   */
+  captureOriginalMapping(): void {
+    this.originalMapping = cloneDeep(this.mapping) ?? [];
+  }
+
+  buildMappingJSON(j: LooseStructDefJSON, snapshot: boolean) {
+    const newMappings: StructureDefinitionMapping[] = differenceWith(
+      this.mapping,
+      this.originalMapping,
+      isEqual
+    );
+    if (snapshot) {
+      const filteredOriginalMappings = this.originalMapping.filter(
+        m =>
+          !newMappings.some(
+            nm => nm.identity === m.identity && nm.name === m.name && nm.uri === m.uri
+          )
+      );
+      j.mapping = [...newMappings, ...filteredOriginalMappings];
+    } else {
+      j.mapping = newMappings;
+    }
+    if (isEmpty(j.mapping)) {
+      delete j.mapping;
+    }
+  }
+
   /**
    * Exports the StructureDefinition to a properly formatted FHIR JSON representation.
    * @returns {any} the FHIR JSON representation of the StructureDefinition
@@ -413,8 +447,12 @@ export class StructureDefinition {
     for (const prop of PROPS_AND_UNDERPROPS) {
       // @ts-ignore
       if (this[prop] !== undefined) {
-        // @ts-ignore
-        j[prop] = this[prop];
+        if (prop === 'mapping') {
+          this.buildMappingJSON(j, snapshot);
+        } else {
+          // @ts-ignore
+          j[prop] = this[prop];
+        }
       }
     }
 
@@ -936,6 +974,7 @@ export type PathPart = {
 interface LooseStructDefJSON {
   resourceType: string;
   derivation?: string;
+  mapping?: StructureDefinitionMapping[];
   snapshot?: { element: LooseElementDefJSON[] };
   differential?: { element: LooseElementDefJSON[] };
   inProgress?: boolean;


Test patch:
diff --git a/test/fhirtypes/StructureDefinition.test.ts b/test/fhirtypes/StructureDefinition.test.ts
index 24cbe56..0ab5f48 100644
--- a/test/fhirtypes/StructureDefinition.test.ts
+++ b/test/fhirtypes/StructureDefinition.test.ts
@@ -5,13 +5,13 @@ import { FHIRDefinitions } from '../../src/fhirdefs/FHIRDefinitions';
 import { StructureDefinition } from '../../src/fhirtypes/StructureDefinition';
 import { ElementDefinition, ElementDefinitionType } from '../../src/fhirtypes/ElementDefinition';
 import { loggerSpy, TestFisher } from '../testhelpers';
-import { FshCode, Invariant, Logical, Resource } from '../../src/fshtypes';
+import { FshCode, Invariant, Logical, Mapping, Profile, Resource } from '../../src/fshtypes';
 import { Type } from '../../src/utils/Fishable';
 import { AddElementRule, ObeysRule, OnlyRule } from '../../src/fshtypes/rules';
 import { InstanceDefinition } from '../../src/fhirtypes';
 import { FSHDocument, FSHTank } from '../../src/import';
 import { minimalConfig } from '../utils/minimalConfig';
-import { Package, StructureDefinitionExporter } from '../../src/export';
+import { FHIRExporter, Package, StructureDefinitionExporter } from '../../src/export';
 import { ValidationError } from '../../src/errors';
 import { cloneDeep } from 'lodash';
 
@@ -501,6 +501,129 @@ describe('StructureDefinition', () => {
       });
     });
 
+    it('should include only new mappings and updated mappings in not-snapshot mode', () => {
+      const doc = new FSHDocument('fileName');
+      const input = new FSHTank([doc], minimalConfig);
+      const pkg = new Package(input.config);
+      const fisher = new TestFisher(input, defs, pkg);
+      const exporter = new FHIRExporter(input, pkg, fisher);
+      const observationProfile = new Profile('OneExtraMapping');
+      observationProfile.parent = 'Observation';
+      doc.profiles.set(observationProfile.name, observationProfile);
+      const mapping = new Mapping('ExtraMapping');
+      mapping.source = 'OneExtraMapping';
+      mapping.title = 'Extra Mapping';
+      mapping.id = 'extra-mapping';
+      mapping.target = 'http://example.org';
+      doc.mappings.set(mapping.name, mapping);
+      const updatedMapping = new Mapping('');
+      updatedMapping.source = 'OneExtraMapping';
+      updatedMapping.title = 'FiveWs Pattern Mapping';
+      updatedMapping.id = 'w5';
+      updatedMapping.target = 'http://hl7.org/fhir/fivews';
+      updatedMapping.description = 'A new comment on an existing mapping';
+      doc.mappings.set(updatedMapping.name, updatedMapping);
+      const exported = exporter.export().profiles[0];
+      const exportedJSON = exported.toJSON(false); // not snapshot mode
+      expect(exportedJSON.mapping).toEqual([
+        {
+          // Existing mapping with new comment is included
+          identity: 'w5',
+          name: 'FiveWs Pattern Mapping',
+          uri: 'http://hl7.org/fhir/fivews',
+          comment: 'A new comment on an existing mapping'
+        },
+        {
+          // New mapping is included
+          identity: 'extra-mapping',
+          name: 'Extra Mapping',
+          uri: 'http://example.org'
+        }
+      ]);
+    });
+
+    it('should not include mapping when no new mappings are added in not-snapshot mode', () => {
+      const doc = new FSHDocument('fileName');
+      const input = new FSHTank([doc], minimalConfig);
+      const pkg = new Package(input.config);
+      const fisher = new TestFisher(input, defs, pkg);
+      const exporter = new FHIRExporter(input, pkg, fisher);
+      const observationProfile = new Profile('NoExtraMappings');
+      observationProfile.parent = 'Observation';
+      doc.profiles.set(observationProfile.name, observationProfile);
+      const exported = exporter.export().profiles[0];
+      const exportedJSON = exported.toJSON(false); // not snapshot mode
+      expect(exportedJSON.mapping).toBeUndefined();
+    });
+
+    it('should add all mappings (new, updated, and inherited) when in snapshot mode', () => {
+      const doc = new FSHDocument('fileName');
+      const input = new FSHTank([doc], minimalConfig);
+      const pkg = new Package(input.config);
+      const fisher = new TestFisher(input, defs, pkg);
+      const exporter = new FHIRExporter(input, pkg, fisher);
+      const observationProfile = new Profile('OneExtraMapping');
+      observationProfile.parent = 'Observation';
+      doc.profiles.set(observationProfile.name, observationProfile);
+      const mapping = new Mapping('ExtraMapping');
+      mapping.source = 'OneExtraMapping';
+      mapping.title = 'Extra Mapping';
+      mapping.id = 'extra-mapping';
+      mapping.target = 'http://example.org';
+      doc.mappings.set(mapping.name, mapping);
+      const updatedMapping = new Mapping('');
+      updatedMapping.source = 'OneExtraMapping';
+      updatedMapping.title = 'FiveWs Pattern Mapping';
+      updatedMapping.id = 'w5';
+      updatedMapping.target = 'http://hl7.org/fhir/fivews';
+      updatedMapping.description = 'A new comment on an existing mapping';
+      doc.mappings.set(updatedMapping.name, updatedMapping);
+      const exported = exporter.export().profiles[0];
+      const exportedJSON = exported.toJSON(true); // snapshot mode
+      expect(exportedJSON.mapping).toHaveLength(7); // inherited mappings are included
+      expect(exportedJSON.mapping).toEqual([
+        {
+          // Existing mapping with new comment is included only once
+          // and comes at the beginning of the list with new mappings
+          identity: 'w5',
+          name: 'FiveWs Pattern Mapping',
+          uri: 'http://hl7.org/fhir/fivews',
+          comment: 'A new comment on an existing mapping'
+        },
+        {
+          // New mapping appears at the beginning of the list
+          identity: 'extra-mapping',
+          name: 'Extra Mapping',
+          uri: 'http://example.org'
+        },
+        {
+          identity: 'workflow',
+          uri: 'http://hl7.org/fhir/workflow',
+          name: 'Workflow Pattern'
+        },
+        {
+          identity: 'sct-concept',
+          uri: 'http://snomed.info/conceptdomain',
+          name: 'SNOMED CT Concept Domain Binding'
+        },
+        {
+          identity: 'v2',
+          uri: 'http://hl7.org/v2',
+          name: 'HL7 v2 Mapping'
+        },
+        {
+          identity: 'rim',
+          uri: 'http://hl7.org/v3',
+          name: 'RIM Mapping'
+        },
+        {
+          identity: 'sct-attr',
+          uri: 'http://snomed.org/attributebinding',
+          name: 'SNOMED CT Attribute Binding'
+        }
+      ]);
+    });
+
     describe('#toJson - Logicals and Resources', () => {
       let addElementRule1: AddElementRule;
       let addElementRule2: AddElementRule;