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

Add variable Extension support #1328

Merged
merged 165 commits into from
Sep 1, 2022

Conversation

shoaibmushtaq25
Copy link
Contributor

@shoaibmushtaq25 shoaibmushtaq25 commented Apr 25, 2022

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

Fixes #1059

Description
The implemented approach is,

  1. Made a child-to-parent map for QuestionnaireItems as we have it for QuestionnaireResponseItems because we need to go through the hierarchy upwards to trace/evaluate and put the values of variables down in the descendants.
  2. Separated out APIs into ExpressionEvaluator file which has two different methods one for evaluation of root level variable expressions internal fun evaluateQuestionnaireRootVariableExpression( expression: Expression, questionnaire: Questionnaire, questionnaireResponse: QuestionnaireResponse ): Base? and the other for evaluation of questionnaire item level variable expressions internal fun evaluateQuestionnaireItemVariableExpression( expression: Expression, questionnaire: Questionnaire, questionnaireResponse: QuestionnaireResponse, questionnaireItemParentMap: Map<Questionnaire.QuestionnaireItemComponent, Questionnaire.QuestionnaireItemComponent>, origin: Questionnaire.QuestionnaireItemComponent ): Base?

In these methods, we try to calculate/evaluate an expression, if it depends on other variables which probably defined up in the hierarchy, we recursively go through the hierarchy with the help of the child-parent map (questionnaire item level expressions) prepared in step 1 above and get the value of the variable and put it's value into original expression, evaluate the expression and return the result. We will calculate variables recursively on the fly and not storing/calculating variables beforehand.

Alternative(s) considered

Type
Choose one: Feature

Screenshots (if applicable)
More details and screenshots are available here on issue#1059

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).

@codecov
Copy link

codecov bot commented Apr 27, 2022

Codecov Report

Merging #1328 (5189736) into master (b12ff3a) will not change coverage.
The diff coverage is n/a.

❗ Current head 5189736 differs from pull request most recent head 203687c. Consider uploading reports for the commit 203687c to get more accurate results

@@      Coverage Diff       @@
##   master   #1328   +/-   ##
==============================
==============================

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@shoaibmushtaq25 shoaibmushtaq25 requested review from jingtang10 and removed request for maimoonak, aditya-07 and omarismail94 August 19, 2022 12:32
@shoaibmushtaq25
Copy link
Contributor Author

Thanks, @jingtang10 for the feedback. PR is updated and ready for review. cc: @maimoonak @f-odhiambo

@shoaibmushtaq25 shoaibmushtaq25 requested review from omarismail94 and jingtang10 and removed request for jingtang10 and omarismail94 August 19, 2022 12:46
…fhirpath/ExpressionEvaluator.kt

Co-authored-by: Jing Tang <[email protected]>
@shoaibmushtaq25
Copy link
Contributor Author

Thanks @jingtang10 for the feedback. I left some comments for you. Please have a look at it. cc: @f-odhiambo

@jingtang10 jingtang10 merged commit 9e43cce into google:master Sep 1, 2022
@shoaibmushtaq25 shoaibmushtaq25 deleted the sm/1059_support_variable_extension branch September 6, 2022 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Medium priority issue type:enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Add variable Extension support
7 participants