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

Implement calculated-expression extension #1380

Merged
merged 92 commits into from
Oct 12, 2022

Conversation

maimoonak
Copy link
Collaborator

@maimoonak maimoonak commented May 11, 2022

Fixes #971 #1173

Description
Implemented the calculated expression extension. The feature also calculated cyclic dependency, and prevents the infinite calls of callback

Type
Choose one: Feature

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

@maimoonak maimoonak marked this pull request as ready for review May 12, 2022 10:22
@f-odhiambo
Copy link
Collaborator

@RaaziaTarique kindly take a 1st pass review

@codecov
Copy link

codecov bot commented May 12, 2022

Codecov Report

Merging #1380 (6c2e0e4) into master (da1b5b3) will increase coverage by 32.61%.
The diff coverage is 14.07%.

❗ Current head 6c2e0e4 differs from pull request most recent head 86e7a2b. Consider uploading reports for the commit 86e7a2b to get more accurate results

@@              Coverage Diff              @@
##             master    #1380       +/-   ##
=============================================
+ Coverage          0   32.61%   +32.61%     
- Complexity        0      320      +320     
=============================================
  Files             0      151      +151     
  Lines             0     5273     +5273     
  Branches          0      942      +942     
=============================================
+ Hits              0     1720     +1720     
- Misses            0     3307     +3307     
- Partials          0      246      +246     
Impacted Files Coverage Δ
...hir/datacapture/MoreQuestionnaireItemComponents.kt 14.28% <0.00%> (ø)
...re/MoreQuestionnaireResponseItemAnswerComponent.kt 0.00% <0.00%> (ø)
...android/fhir/datacapture/QuestionnaireViewModel.kt 0.00% <0.00%> (ø)
...d/fhir/datacapture/fhirpath/ExpressionEvaluator.kt 0.00% <0.00%> (ø)
...android/fhir/datacapture/mapping/ResourceMapper.kt 0.00% <0.00%> (ø)
...roid/fhir/datacapture/validation/ValidationUtil.kt 33.33% <0.00%> (ø)
...ws/QuestionnaireItemDatePickerViewHolderFactory.kt 62.90% <50.00%> (ø)
...stionnaireItemEditTextQuantityViewHolderFactory.kt 73.07% <86.66%> (ø)
...droid/fhir/db/impl/entities/DateTimeIndexEntity.kt 100.00% <0.00%> (ø)
...fhir/search/filter/QuantityParamFilterCriterion.kt 100.00% <0.00%> (ø)
... and 149 more

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

Copy link
Contributor

@RaaziaTarique RaaziaTarique left a comment

Choose a reason for hiding this comment

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

LGTM

@maimoonak maimoonak requested a review from jingtang10 October 10, 2022 17:27
@maimoonak maimoonak assigned jingtang10 and unassigned maimoonak Oct 10, 2022
@maimoonak
Copy link
Collaborator Author

@jingtang10 I have found few libraries for graph generation. I am still exploring these and if its possible to use any for our expression dependency graph
https://github.com/systek/dataflow
https://github.com/jgrapht/jgrapht/
https://github.com/google/guava/wiki/GraphsExplained
https://github.com/nidi3/graphviz-java#user-content-api

@jingtang10
Copy link
Collaborator

@jingtang10 I have found few libraries for graph generation. I am still exploring these and if its possible to use any for our expression dependency graph https://github.com/systek/dataflow https://github.com/jgrapht/jgrapht/ https://github.com/google/guava/wiki/GraphsExplained https://github.com/nidi3/graphviz-java#user-content-api

Thanks Maimoona - what I had in mind was just to use a basic data structure to represent the dependencies. I wasn't sure that we'd need to use any external libraries for this.

We already have some auxiliary data strcuture such as the parent map in the view model - this would be similar to that I thought.

@jingtang10 jingtang10 enabled auto-merge (squash) October 11, 2022 17:46
@jingtang10
Copy link
Collaborator

@maimoonak i turned on auto-merge. so if you update the branch it should merge automatically.

thanks

auto-merge was automatically disabled October 12, 2022 08:53

Head branch was pushed to by a user without write access

@jingtang10 jingtang10 enabled auto-merge (squash) October 12, 2022 17:28
@jingtang10 jingtang10 merged commit 08357ef into google:master Oct 12, 2022
santosh-pingle pushed a commit to santosh-pingle/android-fhir that referenced this pull request Nov 1, 2022
* Implement calculated-expression extension

* Fix form value update bug

* Detect cyclic dependency | fix on init value loading

* Fix merge conflict

* Make birthdate age dependent | Handle and fix quantity values

* Fix failing test

* quantity viewholder delegate test covergae

* Test coverage for update flow

* spotless fix

* spotless fix | re-run ci

* Test covergae for questionnaire fragment

* test coverage for quantity types

* questionnaire fragment test with launchInFragmentContainer

* Update datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt

Co-authored-by: aditya-07 <[email protected]>

* Move widget to LayoutList | Run Calculation after state-change

* Revert the run-expression after state-flow

* remove empty line changes

* spotless fix

* Esperesso test | Fix failing test

* Remove unnessary changes

* Fix espresso tests

* Ignore Failing tests

* Revert ignore test | merge main | refactor

* spotless fix

* Rename tests

* Add tests and docs

* Move catalog calculation to behavior tab

* spotless fix

* Update datacapture/src/main/java/com/google/android/fhir/datacapture/MoreQuestionnaireItemComponents.kt

Co-authored-by: Jing Tang <[email protected]>

* Resolve feedback for naming

* Resolve feedback for doc and naming

* Refactor the update answer handling logic

* Resolve feedback and merge master

* Fix failing test

Co-authored-by: aditya-07 <[email protected]>
Co-authored-by: Benjamin Mwalimu <[email protected]>
Co-authored-by: Jing Tang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 High priority issue
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

Support calculatedExpression
9 participants