android-fhir icon indicating copy to clipboard operation
android-fhir copied to clipboard

Support evaluation of variables, launchContext and other FHIR contexts for cqf-calculatedValue expressions

Open LZRS opened this issue 1 year ago • 4 comments

IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).

Fixes #2292 #1759

Description Add support for evaluation of variable and x-fhir-query expressions(through launchContext) in cqf-calculatedValue expressions, currently supported in minValue and maxValue extensions

Some of the changes included in this PR are

  • Refactor Validator object classes in the validation module to accept expressionEvaluator function that would be used in validation to evaluate cqf-calculatedValue expressions
  • Remove Type.valueOrCalculateValue and replaced it's usage by passing the evaluated calculated value to the validate method of the different validator classes
  • Remove resolveCqfExpression and replaced it with evaluateExpressionValue in the ExpressionEvaluator class
  • Refactor QuestionnaireViewItem to accept two more parameters minAnswerValue and maxAnswerValue that could then be used within view factory classes to validate constraints

Alternative(s) considered Have you considered any alternatives? And if so, why have you chosen the approach in this PR?

Type Choose one: (Bug fix | Feature | Documentation | Testing | Code health | Builds | Releases | Other)

Screenshots (if applicable)

Checklist

  • [x] I have read and acknowledged the Code of conduct.
  • [x] I have read the Contributing page.
  • [x] I have signed the Google Individual CLA, or I am covered by my company's Corporate CLA.
  • [x] I have discussed my proposed solution with code owners in the linked issue(s) and we have agreed upon the general approach.
  • [x] I have run ./gradlew spotlessApply and ./gradlew spotlessCheck to check my code follows the style guide of this project.
  • [x] I have run ./gradlew check and ./gradlew connectedCheck to test my changes locally.
  • [x] I have built and run the demo app(s) to verify my change fixes the issue and/or does not break the demo app(s).

LZRS avatar Nov 02 '23 10:11 LZRS

@LZRS please address the above comments by replying and resolving them. just so that it's clear when everything's been addressed. thanks!

jingtang10 avatar Feb 23 '24 14:02 jingtang10

can you resolve the merge conflicts

jingtang10 avatar Feb 27 '24 17:02 jingtang10

Its a little difficult to follow the changes. Can you please update the description with major changes in this PR that supports the said feature ? Thanks.

I've updated the description of the PR, describing some of the changes included in the PR. You could give it another look

LZRS avatar Feb 27 '24 23:02 LZRS

thanks! will take a look shortly. in the future please also reply to comments you've addressed if you've addressed them and resolve them.

jingtang10 avatar Feb 28 '24 17:02 jingtang10

thanks! will take a look shortly. in the future please also reply to comments you've addressed if you've addressed them and resolve them.

Thank you! Noted

LZRS avatar Feb 29 '24 06:02 LZRS