Allow caret and insert rules to be applied to concepts within a ValueSet #1315
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Elements within value set concept components can be set by caret value rules. The concept must already be added to the value set in order to use it in a caret rule. The concept can be in either the include or exclude arrays. Concept rules with a single code establish a path context that can be used by an indented rule that follows it.
Because caret value rules can be applied to concepts, it is also reasonable to want to apply a rule set containing caret value rules at a concept. This uses the existing grammar rule for a codeCaretRule, but only one concept is allowed for the insert rule's path.
cmoesel
requested changes
Jul 27, 2023
The syntax that was used to support listing multiple concepts in a single rule is removed. Tests related to this syntax are removed. Add more tests for making sure that rule context is handled properly in various combinations of rules on a ValueSet.
cmoesel
previously approved these changes
Aug 1, 2023
jafeltra
reviewed
Aug 3, 2023
Comment on lines
+1241
to
+1242
| /Only one concept may be listed before an insert rule on a ValueSet\..*File: Insert\.fsh.*Line: 3\D*/s | ||
| ); |
There was a problem hiding this comment.
I have a question about this case. We log an error if there are multiple concepts listed before the insert rule, but I don't think we log any error if the caret value rule is included after multiple concepts. For example, something like this doesn't log any errors and adds the designation to the hippo concept. Is that expected?
ValueSet: ZooVS
* ZOO#hippo "Hippopotamus"
* ZOO#rhino
* ZOO#hippo ZOO#rhino ^designation.value = "hipopótamo"
There was a problem hiding this comment.
Oh dang. Yeah, multiple concepts in a VS should trigger an error no matter what. Good find.
…IR/sushi into cimpl-1130-compose-caret-rule
|
@jafeltra thank you for pointing out that case with the caret rule! I've added a test for that and resolved the issue. |
cmoesel
approved these changes
Aug 4, 2023
jafeltra
approved these changes
Aug 4, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Completes task CIMPL-1130.
Some of the elements within
ValueSet.compose.include.conceptandValueSet.compose.exclude.conceptcan't be reached using the normal concept component rules on a ValueSet. To access those elements, you can now use a caret rule with a path that is a contained concept. Similarly, you can insert a RuleSet at such a path. These rules are similar to the code caret rules and code insert rules used by CodeSystem, but since theconceptelements are a flat list, the path has to be exactly once concept long.A concept component rule with exactly one concept in it establishes a context for an indented rule to appear after it. So, these two rules:
are equivalent to these two rules:
The grammar changes to support these rules resulted in some different parser errors in cases where commas are used to list concepts. Additionally, these rules represent a sort of weird case where a rule may set a context, but not itself be indented. Rather than change the set of rules that are allowed to be used in context operations, I added a flag to the context-operation functions so that errors relating to improper indents only get logged once.