Fix type_slices method using correct elements variable#286
Fix type_slices method using correct elements variable#286karlnaden merged 1 commit intoinferno-framework:mainfrom
Conversation
karlnaden
left a comment
There was a problem hiding this comment.
Not convinced yet that this non-functional for this test kit change is worth making since we're essentially maintaining code without a clear use except to let people copy it into their new test kits.
If you still think this important to add, provide a positive test for this, like a modified US Core profile that will exercise this functionality and generate the right metadata (presumably the running test will use the other fix to fhir_resource_navigatioin).
| end | ||
| end | ||
|
|
||
| def type_slices |
There was a problem hiding this comment.
There is a version of the must_support_metadata_extractor.rb in inferno-core now that has an updated version of this method that appears to handle this case. Given that US Core doesn't need this, what's the rationale for updating the US Core code base as opposed to pointing people to inferno-core for a must support test generator? Is the US Core generator still the most complete and so is one that realistically gets copied when starting a new Inferno test kit? In that case, could it use the inferno-core code?
There was a problem hiding this comment.
I also had question if I should submit this PR since it does not affect any US core funcationalities. All the type slices in US Core has $this as path, so the specified type_path logic branch has never been hit. But on the other side, there is an invalid variables in the type_path section. I am OK with any decision from the Inferno team.
For your other questions
-
Yes. US Core generator is still the most complete and comprehensive generator. When I create my own generator, I inherit from US Core generator classes and override some methods as needed. Ex l override this method to correctly save type path into metadata. So this PR does not affect my generator either.
-
This PR creates metadata for the type slicing with speicifed path.
The PR in inferno_core is to use the metadata generated from here to find slicing using both type and path.
There was a problem hiding this comment.
I'm hitting this case in PAS, so I'll be testing this change in the corresponding core method and assuming it checks out will merge it here as well.
That said, I would strongly recommend using the core metadata extractor as a base going forward as that is where enhancements will generally be made.
There was a problem hiding this comment.
This is a good idea. I will change PACIO test kits to depend on inferno_core's must_support module
karlnaden
left a comment
There was a problem hiding this comment.
tested against the PAS v2.2.0 version which uses this. metadata generation looks good.
Summary
This PR fixes Issue #283
The first issue is that the
elementsvariable used in current version does not exist at all. This PR changes it to the correct instance variableprofile_elements.This PR also saves the
type_pathinto the metadata if it has value.Since US Core does not have type slice with path, so this code section has not been touched by US Core generator.
Testing Guidance
Run
us_core:generate, there SHALL NOT be any change to the generated us core tests.