Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extract more specific types in OnlyRules with target profiles #229

Merged
merged 1 commit into from
Jul 17, 2023

Conversation

mint-thompson
Copy link
Collaborator

Fixes #220 and completes task CIMPL-1100.

Reference, CodeableReference, and canonical types can all be specified with target profiles. When extracting an OnlyRule, set the corresponding boolean value on the extracted type along with the target profiles. This takes advantage of the newly available isCodeableReference flag on SUSHI's OnlyRuleType.

The changes in this PR ended up going a bit beyond the original task description, as I noticed that GoFSH was handling canonical types with target profiles by creating caret rules. But, everything appears to round-trip cleanly, so I thought it would be best to clean up all these types together.

Reference, CodeableReference, and canonical types can all be specified
with target profiles. When extracting an OnlyRule, set the corresponding
boolean value on the extracted type along with the target profiles. This
takes advantage of the newly available isCodeableReference flag on
SUSHI's OnlyRuleType.
Copy link
Member

@cmoesel cmoesel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, works well, so much simpler than when we were avoiding a CodeableReference keyword. I like it!

I tested it on US Core, but also made my own small IG that constrained elements of canonical and CodeableReference -- and it all checks out.

@@ -9,9 +10,17 @@ export class OnlyRuleExtractor {
}
const onlyRule = new ExportableOnlyRule(getPath(input));
input.type.forEach((t, i) => {
if (['Reference', 'CodeableReference'].includes(t.code) && t.targetProfile) {
if (['Reference', 'CodeableReference', 'canonical'].includes(t.code) && t.targetProfile) {
const targeting: Partial<fshrules.OnlyRuleType> = {};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oooh. We haven't leveraged Partial much before. Cool.

Copy link
Collaborator

@jafeltra jafeltra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me. I also tested this on the example provided in the original issue, and the warning is resolved and the JSON round trip seems correct.

@mint-thompson mint-thompson merged commit 67472b7 into master Jul 17, 2023
@mint-thompson mint-thompson deleted the cimpl-1100-codeablereference-keyword branch July 17, 2023 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot roundtrip R5 workflow reason extension
3 participants