Fix issue with empty context with initial expression #1228
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -46,7 +46,6 @@ import org.hl7.fhir.r4.model.Questionnaire | |||||||||
| import org.hl7.fhir.r4.model.QuestionnaireResponse | ||||||||||
| import org.hl7.fhir.r4.model.Resource | ||||||||||
| import org.hl7.fhir.r4.model.StringType | ||||||||||
| import org.hl7.fhir.r4.model.Type | ||||||||||
| import org.hl7.fhir.r4.model.UriType | ||||||||||
| import org.hl7.fhir.r4.utils.FHIRPathEngine | ||||||||||
|
|
@@ -197,27 +196,54 @@ object ResourceMapper { | |||||||||
| vararg resources: Resource | ||||||||||
| ) { | ||||||||||
| if (questionnaireItem.type != Questionnaire.QuestionnaireItemType.GROUP) { | ||||||||||
| questionnaireItem.initialExpression?.let { exp -> | ||||||||||
| evaluateInitialExpressionAndSetInitialValue(exp, resources, questionnaireItem) | ||||||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| populateInitialValues(questionnaireItem.item, *resources) | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private fun evaluateInitialExpressionAndSetInitialValue( | ||||||||||
| exp: Expression, | ||||||||||
|
There was a problem hiding this comment. avoid using abbreviations
Suggested change
|
||||||||||
| resources: Array<out Resource>, | ||||||||||
|
There was a problem hiding this comment. i don't feel like this out here is justified? I thought it's only useful for declarations on class/interface. can you elaborate why this is needed? There was a problem hiding this comment. Actually |
||||||||||
| questionnaireItem: Questionnaire.QuestionnaireItemComponent | ||||||||||
| ) { | ||||||||||
| val resourceType = exp.expression.substringBefore(".").removePrefix("%") | ||||||||||
| val contextResource = | ||||||||||
| resources.firstOrNull { it.resourceType.name.lowercase().equals(resourceType.lowercase()) } | ||||||||||
|
|
||||||||||
| val extractedAnswers = getAnswers(contextResource, resources, exp) | ||||||||||
| extractedAnswers.firstOrNull()?.let { answer -> | ||||||||||
| // Resetting QuestionnaireItemInitialComponent value using provided initialExpression | ||||||||||
| questionnaireItem.initial = | ||||||||||
| mutableListOf( | ||||||||||
| Questionnaire.QuestionnaireItemInitialComponent().setValue(answer.asExpectedType()) | ||||||||||
| ) | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private fun getAnswers( | ||||||||||
| contextResource: Resource?, | ||||||||||
| resources: Array<out Resource>, | ||||||||||
|
|
||||||||||
| exp: Expression | ||||||||||
| ): MutableList<Base> { | ||||||||||
| var extractedAnswers = mutableListOf<Base>() | ||||||||||
| if (contextResource == null) { | ||||||||||
| if (resources.isNotEmpty()) { | ||||||||||
| // Using the first provided resource as a base resource inorder to fetch answers from the | ||||||||||
| // functions | ||||||||||
| extractedAnswers = fhirPathEngine.evaluate(resources[0], exp.expression.removePrefix("%")) | ||||||||||
| } | ||||||||||
| } else { | ||||||||||
| // Using the resource extracted from provided expression as a base resource inorder to fetch | ||||||||||
| // answers from the functions | ||||||||||
| extractedAnswers = fhirPathEngine.evaluate(contextResource, exp.expression.removePrefix("%")) | ||||||||||
| } | ||||||||||
|
There was a problem hiding this comment. this code seems duplicated to me. you're just saying if the context resource is empty just take the first resource as the context resource right? i feel this code is actually harder to read and doesn't add any more functionality. There was a problem hiding this comment. @jingtang10, this code added functionality to use There was a problem hiding this comment. the only thing that's different in the code is that if the type doesn't match the expected expression the first resource is passed anyway as context. |
||||||||||
| return extractedAnswers | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private val Questionnaire.QuestionnaireItemComponent.initialExpression: Expression? | ||||||||||
| get() { | ||||||||||
| return this.extension.firstOrNull { it.url == ITEM_INITIAL_EXPRESSION_URL }?.let { | ||||||||||
| it.value as Expression | ||||||||||
|
|
@@ -514,7 +540,7 @@ private fun wrapAnswerInFieldType(answer: Base, fieldType: Field): Base { | |||||||||
| return answer | ||||||||||
| } | ||||||||||
|
|
||||||||||
| const val ITEM_INITIAL_EXPRESSION_URL: String = | ||||||||||
| "http://hl7.org/fhir/uv/sdc/StructureDefinition/sdc-questionnaire-initialExpression" | ||||||||||
|
|
||||||||||
| private val Field.isList: Boolean | ||||||||||
|
|
||||||||||
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.
I think this is much better as a extension function on questionnaireItem.
fun QuestionnaireItem.setInitialValueFromInitialExpression()seems appropriate.you're passing the questionnaire item to the
evaluateInitialExpressionAndSetInitialValuefunction which seems odd to me. All this happens within the context of the questionnaire item, so should probably be an extension function.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.
@jingtang10 I have created an extension function with slight modifications. Please re-review the PR.