-
Notifications
You must be signed in to change notification settings - Fork 305
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
Improve initial load of calculated expression #2522
Improve initial load of calculated expression #2522
Conversation
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.
What if there is an item (maybe display item) which is dependent on the initial calculated expression ?
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/QuestionnaireViewModel.kt
Show resolved
Hide resolved
…rove-calculated-expression-initial-load
@jingtang10 @MJ1998 this is ready for review |
@MJ1998 hmm, I might not understand what you're trying to say. Could you elaborate? And some examples. Display item can't really depend on other items unless the display item is using cqf expression, display item has no answer, so it cannot use calculated expression. |
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.
This is really great optimization! Thanks @FikriMilano!
This is not your code, but I've just noticed in the file ExpressionEvaluator.kt
function evaluateCalculatedExpressions
the use of updatedQuestionnaireResponseItemComponent
is actually wrong. What should be passed as the %context should be the corresponding questionnaire response item of the questionnaire item with calculated expression that is affected. Can you please fix that in this PR? I think what you can do is to actually just pass a null value there and add a comment to say something like "to properly use %context we will need to find questionnaire item and questionnaire response item pairs etc.. but we won't do that now." You can then just simplify that function signature.
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/fhirpath/ExpressionEvaluator.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/QuestionnaireViewModel.kt
Outdated
Show resolved
Hide resolved
datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt
Show resolved
Hide resolved
datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt
Show resolved
Hide resolved
…rove-calculated-expression-initial-load
@jingtang10 this is ready for review |
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.
Thanks for the PR @FikriMilano great job!
datacapture/src/main/java/com/google/android/fhir/datacapture/fhirpath/ExpressionEvaluator.kt
Outdated
Show resolved
Hide resolved
datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt
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/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/QuestionnaireViewModel.kt
Outdated
Show resolved
Hide resolved
…rove-calculated-expression-initial-load
IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).
Fixes #2519
Description
On initial load of forms:
Alternative(s) considered
N/A
Type
Improvement
Screenshots (if applicable)
Checklist
./gradlew spotlessApply
and./gradlew spotlessCheck
to check my code follows the style guide of this project../gradlew check
and./gradlew connectedCheck
to test my changes locally.