-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Add support for CRMI Effective Data Requirements Extension to CDS on FHIR Service Discovery #5876
base: master
Are you sure you want to change the base?
Conversation
…r CRMI extensions.
Formatting check succeeded! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5876 +/- ##
==========================================
Coverage 83.39% 83.39%
- Complexity 26927 27002 +75
==========================================
Files 1681 1688 +7
Lines 103965 104251 +286
Branches 13189 13224 +35
==========================================
+ Hits 86702 86941 +239
- Misses 11613 11656 +43
- Partials 5650 5654 +4 ☔ View full report in Codecov by Sentry. |
// Use a Module Definition Library with Effective Data Requirements for the Plan Definition if it exists | ||
if (dataReqExt != null && dataReqExt.hasValue()) { | ||
StringType moduleDefCanonical = (StringType) dataReqExt.getValue(); | ||
library = (Library) SearchHelper.searchRepositoryByCanonical(myRepository, moduleDefCanonical); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: The behavior here is that when a module-definition library is specified, but absent, it falls back to the primary library. Is that the expected behavior when using a module-definition? Those are usually used when a specific library version is required, correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same question for the other implementations as well
protected List<String> resolveValueCodingCodes(List<Coding> theValueCodings) { | ||
List<String> result = new ArrayList<>(); | ||
StringBuilder codes = new StringBuilder(); | ||
for (Coding coding : theValueCodings) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: theValueCodings.stream().filter(x -> x.hasCode().findFirst() will give you the same behavior without the ReassignedVariable
warning. Also makes it clearer that we just pick the first.
for (ValueSet.ConceptSetComponent concepts : valueSet.getCompose().getInclude()) { | ||
String system = concepts.getSystem(); | ||
if (concepts.hasConcept()) { | ||
for (ValueSet.ConceptReferenceComponent concept : concepts.getConcept()) { | ||
String code = concept.getCode(); | ||
|
||
codes = getCodesStringBuilder(result, codes, system, code); | ||
} | ||
} | ||
} | ||
} | ||
result.add(codes.toString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: Similar refactoring here as above comment. Make it explicit that this is effectively "last wins".
if (codes.length() > 0 && postAppendLength < myMaxUriLength) { | ||
codes.append(","); | ||
} else if (postAppendLength > myMaxUriLength) { | ||
theStrings.add(codes.toString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: I'd suggest a refactoring here to get rid of the warnings:
String codeToken = theSystem + "|" + theCode;
int postAppendLength = codes.length() + codeToken.length();
// We're at maximum length, capture the current string and start a new builder.
if (postAppendLength >= myMaxUriLength) {
theStrings.add(codes.toString());
return new StringBuilder().append(codeToken);
}
// There's already a code in the Builder, append a command
else if (theCodes.length() > 0) {
theCodes.append(",");
}
return theCodes.append(codeToken);
return codes; | ||
} | ||
|
||
protected String mapCodePathToSearchParam(String theDataType, String thePath) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: The CQL engine already has a bunch of code to support this mapping generally across all FHIR versions:
https://github.com/cqframework/clinical_quality_language/blob/master/Src/java/engine-fhir/src/main/java/org/opencds/cqf/cql/engine/fhir/model/R4FhirModelResolver.java#L407
return thePath.replace('.', '-').toLowerCase(); | ||
} | ||
|
||
protected static boolean isPatientCompartment(String theDataType) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Same as above.
} | ||
} | ||
|
||
protected String getPatientSearchParam(String theDataType) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Same as above
Closes #5873
Refactored the prefetch template generation from the CDS on FHIR Service Discovery so it can be reused.
Added support for the CRMI Effective Data Requirements extension. This extension, if present, will be used to determine the library to use when generating the prefetch templates for the CDS Service Discovery.
Also added support for the FHIR Query Pattern extension when generating prefetch templates. If any of these extensions exist on a DataRequirement, they will be used as the prefetch templates rather than being generated.