Support code caret rules in ValueSets #249
Conversation
A ValueSet can use caret rules with a code path to set values on included or excluded concepts. The typical use of this is to set a designation, but other elements may also appear. Elements outside of a concept are still unsupported and require the creation of an Instance.
There was a problem hiding this comment.
I really like the re-use of code. That made this much simpler!
I would like to see one change regarding the ordering of exported rules. Here's a ValueSet JSON to test with if you'd like:
ValueSet-VSWithConceptCaretRules.json
| @@ -693,7 +695,7 @@ describe('CaretValueRuleExtractor', () => { | |||
| }) | |||
| ); | |||
| expect(loggerSpy.getLastMessage('error')).toMatch( | |||
| 'Value in CodeSytem testCS at concept testConcept for element property[0] is empty. No caret value rule will be created.' | |||
| 'Value in CodeSystem testCS at concept testConcept for element property[0] is empty. No caret value rule will be created.' | |||
|
It turns out there are a bunch of issues with |
The SUSHI dependency is updated to 3.6.0, which supports including the system in the path array. This is needed for rules on ValueSets. Add the system to the path array when extracting caret rules on ValueSet concept components. Caret rules on CodeSystem concepts don't need a system, but still need the leading # as a separator. Add optimizer that reorders rules on ValueSets so that concept rules are grouped with their corresponding caret rules. Update other tests for Invariants, ValueSets, and ConceptRules based on other changes included in SUSHI 3.6.0.
Resolve URLs on caret rules after resolving URLs on ValueSet components so that the caret rules will be able to use aliases that were created when resolving the components. Create new arrays to assign to path array when extracting caret rules on concepts. This avoids the possibility of inadvertantly modifying multiple rules with a single operation that mutates the path array.
| // ' valueset http://fhir.food-pyramid.org/FoodPyramidGuide/ValueSets/DeliciousVS' | ||
| // ].join(EOL); | ||
| // expect(result).toEqual(expectedResult); | ||
| // }); |
There was a problem hiding this comment.
Is there a reason this test is commented out now?
There was a problem hiding this comment.
This looks good! I only have two minor requests:
- Either uncomment or remove the commented out test that Julia noted
- Add a few more codes to the test case that I commented on
After that, I think we're good! I tested manually as well and confirmed it is working as expected.
| zooConcepts.concepts = [ | ||
| new fshtypes.FshCode('bear', 'http://example.org/zoo'), | ||
| new fshtypes.FshCode('pelican', 'http://example.org/zoo') | ||
| ]; |
There was a problem hiding this comment.
I think it would be good to add a couple of codes that won't have caret rules too -- just to confirm that we're not dropping those. (I don't think we are, but I think it's a good thing to have a test case for).
Fixes #236 and completes task CIMPL-1165.
A ValueSet can use caret rules with a code path to set values on included or excluded concepts. The typical use of this is to set a designation, but other elements may also appear. Elements outside of a concept are still unsupported and require the creation of an Instance.