Repository: FHIR/sushi
Pull request #296 (2020-03-25 17:16:36)

Fix find element by path slice of slice bug
This is a small PR that fixes a bug I came across. In `findElementByPath`, if you have an element that has two levels of slicing, `findElementByPath` will fail to find the root of the second level of slicing. So for an example, if the element `Observation.category:VSCat.coding` is sliced to contain a `FooSlice`, then we will be able to use `findElementByPath` on the path `category[VSCat].coding[FooSlice]` to find the desired element, but when we do`findElementByPath` on the path `category[VSCat].coding`, we will get undefined back. This PR fixes that so that in both cases the correct element is returned.

To test, you can check out https://github.com/FHIR/sushi/commit/f9879737deb7f2ffa59823f75c40637c1337c0d1 and run the tests, and see that the one added test in this commit fails, and then you can checkout https://github.com/FHIR/sushi/commit/0411bcc25e3fa64f700f252ffe4d10acca5533d9 and see that the code added to `findElementByPath` makes this test pass now.

cmoesel: Yeah, I'm reviewing it too -- comparing outputs in mcode.  There were a lot of diffs between this and master -- but it turns out because this branch is out of date.  Ooops.

cmoesel: Oops.  Didn't see @mint-thompson's more thorough review.  Good sleuthing, @mint-thompson.  @ngfreiter -- what do you think?
ngfreiter: Yeah @mint-thompson 's test case does fail. This is definitely a good find because it's made me question everything. I'm not sure what to think. I think it is probably not in the scope of this PR, since it isn't necessarily related to the bug this fixes. It would be a different fix. But this does make me question the work I am doing on `containsRule`. If `findElementByPath` worked as @mint-thompson  is describing, then the stuff I am doing to cascade `ContainsRule`s down to slices may not be needed.

But I'm also not so sure that the element should be found. It doesn't exist on the `VSCat` slice. Does a slice existing on a root element mean that the slice exists on slices of the root? I'm not entirely convinced, but that's something to figure out for the `ContainsRule` task and I don't think for this task.
cmoesel: I agree.  I don't think it's directly related to this PR.  But it does seem somewhat related to the task around cascading contains rules...  If a new slice is added on a child path of a sliced element, then I *think* that new slice is also in the slices of the sliced element (at the appropriate child path in each), right?
cmoesel: Going to merge this one.  We can decide what to do about the situation @mint-thompson brought up when addressing the cascading contains rule PR.

Patch:
diff --git a/src/fhirtypes/StructureDefinition.ts b/src/fhirtypes/StructureDefinition.ts
index c8b99c5..151d224 100644
--- a/src/fhirtypes/StructureDefinition.ts
+++ b/src/fhirtypes/StructureDefinition.ts
@@ -251,11 +251,16 @@ export class StructureDefinition {
 
       // If there are no brackets, remove any slices that don't match exactly
       if (!pathPart.brackets) {
-        matchingElements = matchingElements.filter(
-          e =>
-            e.id.includes(`${fhirPathString}:${pathPart.base}`) ||
-            !e.id.includes(`${fhirPathString}:`)
-        );
+        // Get the end of the current fhirPath (e.g. path1.path2.path3 -> path3)
+        const pathDepth = fhirPathString.split('.').length - 1;
+        const pathEnd = fhirPathString.split('.')[pathDepth];
+        matchingElements = matchingElements.filter(e => {
+          const idEnd = e.id.split('.')[pathDepth];
+          // If matchingElement id ends with pathEnd:, then it is a slice
+          // pathPart.brackets is null, so keep nonslices (no ":" in idEnd)
+          // and choice slices since valueString is equivalent to value[x]:valueString
+          return !idEnd.includes(`${pathEnd}:`) || idEnd.includes(`${pathEnd}:${pathPart.base}`);
+        });
       }
     }
 


Test patch:
diff --git a/test/fhirtypes/StructureDefinition.test.ts b/test/fhirtypes/StructureDefinition.test.ts
index 8bcdb5b..b66ed06 100644
--- a/test/fhirtypes/StructureDefinition.test.ts
+++ b/test/fhirtypes/StructureDefinition.test.ts
@@ -465,6 +465,23 @@ describe('StructureDefinition', () => {
       expect(VSCatID.id).toBe('Observation.category:VSCat.id');
     });
 
+    it('should find a root of a slicing that is a child of a slice by path', () => {
+      const VSCatCoding = respRate.findElement('Observation.category:VSCat.coding');
+      VSCatCoding.slicing = { discriminator: [{ type: 'pattern', path: '$this' }], rules: 'open' };
+      VSCatCoding.addSlice('TestSlice');
+      const testSlice = respRate.findElement('Observation.category:VSCat.coding:TestSlice');
+      expect(testSlice).toBeDefined();
+      const foundTestSlice = respRate.findElementByPath(
+        'category[VSCat].coding[TestSlice]',
+        fisher
+      );
+      const foundRoot = respRate.findElementByPath('category[VSCat].coding', fisher);
+      expect(foundTestSlice).toBeDefined();
+      expect(foundTestSlice.id).toBe('Observation.category:VSCat.coding:TestSlice');
+      expect(foundRoot).toBeDefined();
+      expect(foundRoot.id).toBe('Observation.category:VSCat.coding');
+    });
+
     it('should find a re-sliced element by path', () => {
       const jsonReslice = JSON.parse(
         fs.readFileSync(