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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 16 commits into from
Mar 13, 2024
Merged

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

merged 16 commits into from
Mar 13, 2024

Conversation

LZRS
Copy link
Collaborator

@LZRS LZRS commented Nov 2, 2023 •
edited
Loading

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

  • I have read and acknowledged the Code of conduct.
  • I have read the Contributing page.
  • I have signed the Google Individual CLA, or I am covered by my company's Corporate CLA.
  • I have discussed my proposed solution with code owners in the linked issue(s) and we have agreed upon the general approach.
  • I have run ./gradlew spotlessApply and ./gradlew spotlessCheck to check my code follows the style guide of this project.
  • I have run ./gradlew check and ./gradlew connectedCheck to test my changes locally.
  • 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 LZRS force-pushed the eval-variables-for-cqf-expressions branch 3 times, most recently from b6d50ed to 859a1be Compare November 8, 2023 05:26
@LZRS LZRS marked this pull request as ready for review November 8, 2023 05:37
@LZRS LZRS requested review from santosh-pingle and a team as code owners November 8, 2023 05:37
@LZRS LZRS requested a review from aditya-07 November 8, 2023 05:37
MJ1998
MJ1998 requested changes Nov 15, 2023
View reviewed changes
Copy link
Collaborator

@MJ1998 MJ1998 left a comment

Choose a reason for hiding this comment

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

Instead of changing the original questionnaire's extensions, we can resolve the min and max values on-fly, ie, when creating the QuestionnaireViewItem instance.

datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt Outdated Show resolved Hide resolved
datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt Outdated Show resolved Hide resolved
datacapture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireViewItem.kt Outdated Show resolved Hide resolved
@LZRS LZRS force-pushed the eval-variables-for-cqf-expressions branch from 79fdbe7 to 3e9a620 Compare November 22, 2023 03:22
@LZRS LZRS requested a review from MJ1998 November 22, 2023 07:20
LZRS added 2 commits January 10, 2024 00:19
@LZRS
Merge remote-tracking branch 'upstream/master' into eval-variables-fo…
080e4b3 
…r-cqf-expressions
@LZRS
Refactor to not modify extension in questionnaire
3cd10dc 
When evaluating cqf-calculatedValue expression for validation
@LZRS LZRS force-pushed the eval-variables-for-cqf-expressions branch from a49d803 to 3cd10dc Compare January 12, 2024 00:06
@LZRS LZRS requested a review from jingtang10 January 25, 2024 06:59
@ageryck ageryck mentioned this pull request Feb 12, 2024
Closed
3 tasks
MJ1998
MJ1998 previously requested changes Feb 14, 2024
View reviewed changes
Copy link
Collaborator

@MJ1998 MJ1998 left a comment

Choose a reason for hiding this comment

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

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.

...apture/src/test/java/com/google/android/fhir/datacapture/fhirpath/ExpressionEvaluatorTest.kt Outdated Show resolved Hide resolved
...t/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponentsTest.kt Outdated Show resolved Hide resolved
datacapture/src/test/java/com/google/android/fhir/datacapture/QuestionnaireViewModelTest.kt Outdated Show resolved Hide resolved
datacapture/src/test/java/com/google/android/fhir/datacapture/QuestionnaireViewModelTest.kt Show resolved Hide resolved
datacapture/src/test/java/com/google/android/fhir/datacapture/QuestionnaireViewModelTest.kt Outdated Show resolved Hide resolved
datacapture/src/test/java/com/google/android/fhir/datacapture/QuestionnaireViewModelTest.kt Outdated Show resolved Hide resolved
datacapture/src/test/java/com/google/android/fhir/datacapture/QuestionnaireViewModelTest.kt Outdated Show resolved Hide resolved
datacapture/src/test/java/com/google/android/fhir/datacapture/QuestionnaireViewModelTest.kt Outdated Show resolved Hide resolved
datacapture/src/test/java/com/google/android/fhir/datacapture/QuestionnaireViewModelTest.kt Outdated Show resolved Hide resolved
datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt Outdated Show resolved Hide resolved
@jingtang10
Copy link
Collaborator

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

@jingtang10
Copy link
Collaborator

can you resolve the merge conflicts

@LZRS
Copy link
Collaborator Author

LZRS commented Feb 27, 2024

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 LZRS requested a review from MJ1998 February 27, 2024 23:38
@LZRS LZRS force-pushed the eval-variables-for-cqf-expressions branch from d7dd15f to 6eeca2c Compare February 28, 2024 00:26
@jingtang10
Copy link
Collaborator

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.

@LZRS
Copy link
Collaborator Author

LZRS commented Feb 29, 2024

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 LZRS force-pushed the eval-variables-for-cqf-expressions branch from 1bb2c0e to c0fc3c6 Compare March 6, 2024 13:48
@jingtang10 jingtang10 enabled auto-merge (squash) March 13, 2024 18:44
@jingtang10 jingtang10 merged commit 6ca5cbc into google:master Mar 13, 2024
4 checks passed
@MJ1998 MJ1998 mentioned this pull request Mar 14, 2024
Merged
7 tasks
@LZRS LZRS deleted the eval-variables-for-cqf-expressions branch March 27, 2024 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jingtang10 jingtang10 jingtang10 approved these changes

@santosh-pingle santosh-pingle Awaiting requested review from santosh-pingle

@aditya-07 aditya-07 Awaiting requested review from aditya-07 aditya-07 was automatically assigned from google/android-fhir

@MJ1998 MJ1998 Awaiting requested review from MJ1998

Assignees
No one assigned
Labels
None yet
Projects
Android FHIR SDK
Status: Complete
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enable cqf expressions to evaluate variables on questionnaire extensions
4 participants
@LZRS @jingtang10 @MJ1998 @android-fhir-bot