Fix issue with empty context with initial expression #1228

RaaziaTarique · 2022-03-10T08:06:25Z

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

Description
Added functionality to use FhirPath functions such as today(), now() and etc.
Previously these functions were not supported in SDK because it was only using the extracted resource from the FhirPath expression and in case of FhirPath functions, the FhirPath Expressions doesn't have any resource name in it.

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

Type
Choose one: Feature

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

codecov · 2022-03-10T09:20:47Z

Codecov Report

Merging #1228 (59ce913) into master (ae4c26a) will increase coverage by 0.02%.
The diff coverage is 86.66%.

@@             Coverage Diff              @@
##             master    #1228      +/-   ##
============================================
+ Coverage     84.50%   84.53%   +0.02%     
- Complexity      686      687       +1     
============================================
  Files           148      148              
  Lines         10697    10701       +4     
  Branches        832      832              
============================================
+ Hits           9040     9046       +6     
+ Misses         1244     1240       -4     
- Partials        413      415       +2

Impacted Files	Coverage Δ
...iews/QuestionnaireItemDropDownViewHolderFactory.kt	`54.28% <0.00%> (+2.85%)`	⬆️
...android/fhir/datacapture/mapping/ResourceMapper.kt	`83.50% <92.85%> (+0.84%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3db36db...59ce913. Read the comment docs.

…3_advance_behaviour

… evaluate method.

…3_advance_behaviour

- Add test cases in ResourceMapperTest.kt

ekigamba · 2022-03-22T13:27:11Z

datacapture/src/test/java/com/google/android/fhir/datacapture/mapping/ResourceMapperTest.kt

@@ -957,6 +990,231 @@ class ResourceMapperTest {
      .isEqualTo(true)
  }

+  @Test
+  fun `populate() should fill QuestionnaireResponse with values when given a single Resource should use definition based extraction`() =


Just a question. What does this test assert that the other population tests do not

I think that the should use definition based extraction should not be part of this tests function and name

As per the discussion, I have removed the definition based initial expression evaluation code and test cases in favour of itemPopulationContext http://hl7.org/fhir/uv/sdc/StructureDefinition/sdc-questionnaire-itemPopulationContext which will be handled in another PR

ekigamba · 2022-03-22T13:28:35Z

datacapture/src/test/java/com/google/android/fhir/datacapture/mapping/ResourceMapperTest.kt

@@ -490,6 +491,38 @@ class ResourceMapperTest {
    assertThat(patient.telecom[0].value).isEqualTo("+254711001122")
  }

+  @Test
+  fun `populate() should correctly populate current date in QuestionnaireResponse`() = runBlocking {
+    val ITEM_EXTRACTION_CONTEXT_EXTENSION_URL =


I think we can use this directly instead

jingtang10 · 2022-03-18T13:02:50Z

datacapture/src/test/java/com/google/android/fhir/datacapture/mapping/ResourceMapperTest.kt

@@ -490,6 +491,38 @@ class ResourceMapperTest {
    assertThat(patient.telecom[0].value).isEqualTo("+254711001122")
  }

+  @Test
+  fun `populate() should correctly populate current date in QuestionnaireResponse`() = runBlocking {
+    val ITEM_EXTRACTION_CONTEXT_EXTENSION_URL =


the name of this is wrong, this is not item extraction context. this is initial expression

jingtang10 · 2022-03-18T13:04:40Z

datacapture/src/main/java/com/google/android/fhir/datacapture/mapping/ResourceMapper.kt

@@ -198,17 +197,21 @@ object ResourceMapper {
  ) {
    if (questionnaireItem.type != Questionnaire.QuestionnaireItemType.GROUP) {
      questionnaireItem.fetchExpression?.let { exp ->


can you rename the fetchExpression property to intialExpression?

jingtang10 · 2022-04-20T08:57:48Z

datacapture/src/main/java/com/google/android/fhir/datacapture/mapping/ResourceMapper.kt

+        extractedAnswers =
+          fhirPathEngine.evaluate(resources[0], expression.expression.removePrefix("%"))


these lines and line 221-222 are doing the same except for the context resource. So what you should do is to have

val contextResource = if( ... ) { ... } else { ... }

and then have

extractedAnswers = fhirPathEngine.evaluate(contextResource, expression.expression.removePrefix("%"))

why is this logic not in the function getContextResource?

aditya-07

LGTM

datacapture/src/main/java/com/google/android/fhir/datacapture/mapping/ResourceMapper.kt

jingtang10 · 2022-04-22T12:38:28Z

Struggling to make the tests pass on GitHub even though they're passing locally...

jingtang10 · 2022-04-22T17:28:30Z

tests should hopefully pass after #1324

RaaziaTarique added 2 commits March 10, 2022 09:04

- today() support added in initial expression

01ba81c

- spotless apply

117f474

RaaziaTarique added 11 commits March 10, 2022 17:30

- Enhance initialExpression support

cb89b67

Merge branch 'master' into 1033_advance_behaviour

9e4ca7f

- Enhance initialExpression support modifications

2b96744

Merge branch 'master' into 1033_advance_behaviour

512f760

- Use ExpressionNode to evaluate expression

03ac5e9

Merge remote-tracking branch 'origin/1033_advance_behaviour' into 103…

dffe99c

…3_advance_behaviour

Merge branch 'master' into 1033_advance_behaviour

5b1d462

- Reduce step one of expression evaluation to use only one expression…

9a26aab

… evaluate method.

Merge remote-tracking branch 'origin/1033_advance_behaviour' into 103…

ed18ff5

…3_advance_behaviour

- Simplify ResourceMapper.kt

a3deda3

- Add test cases in ResourceMapperTest.kt

- Simply Code

2f004af

f-odhiambo marked this pull request as ready for review March 22, 2022 08:08

f-odhiambo requested review from aditya-07, deepankarb, jingtang10, joiskash, kevinmost, ktarasenko, stevenckngaa, vvikramraj and yigit as code owners March 22, 2022 08:08

ekigamba self-requested a review March 22, 2022 13:20

ekigamba suggested changes Mar 22, 2022

View reviewed changes

- Remove extract answers from expressions using definition code.

5d12a9e

RaaziaTarique requested a review from ekigamba March 24, 2022 08:23

Merge branch 'master' into 1033_advance_behaviour

6197c6f

jingtang10 requested changes Mar 24, 2022

View reviewed changes

RaaziaTarique and others added 4 commits April 4, 2022 17:57

Merge branch 'master' into 1033_advance_behaviour

b2b6187

Merge branch 'master' into 1033_advance_behaviour

34e7e81

- Resolve requested changes.

5a32c3f

- Resolve requested changes.

c67a406

RaaziaTarique requested a review from jingtang10 April 5, 2022 09:17

Merge branch 'master' into 1033_advance_behaviour

82f7a23

jingtang10 requested changes Apr 20, 2022

View reviewed changes

jingtang10 added 2 commits April 20, 2022 16:53

Refactor ResourceMapper.kt to make it more concise.

2f27423

Add documentation.

85398df

jingtang10 changed the title ~~Initial expression support extension~~ Fix issue with empty context with initial expression Apr 20, 2022

Make ITEM_INITIAL_EXPRESSION_URL internal

8c02a1a

jingtang10 approved these changes Apr 20, 2022

View reviewed changes

Merge branch 'master' into 1033_advance_behaviour

b978015

RaaziaTarique mentioned this pull request Apr 21, 2022

Support initial expression extension in questionnaire population #1295

Closed

7 tasks

aditya-07 approved these changes Apr 21, 2022

View reviewed changes

Merge branch 'master' into 1033_advance_behaviour

748fe39

stevenckngaa approved these changes Apr 21, 2022

View reviewed changes

datacapture/src/main/java/com/google/android/fhir/datacapture/mapping/ResourceMapper.kt Outdated Show resolved Hide resolved

jingtang10 added 2 commits April 22, 2022 11:20

Use Kotlin scope functions to replace if..else...

61d59bd

Merge branch 'master' into 1033_advance_behaviour

93e3b7a

Merge branch 'master' into 1033_advance_behaviour

957b991

jingtang10 enabled auto-merge (squash) April 22, 2022 18:43

Merge branch 'master' into 1033_advance_behaviour

59ce913

jingtang10 requested review from omarismail94 and wantingzhang77 as code owners April 22, 2022 21:24

jingtang10 merged commit 45a86af into master Apr 23, 2022

jingtang10 deleted the 1033_advance_behaviour branch April 23, 2022 05:23

Tarun-Bhardwaj added the Data capture label Jul 5, 2022

maimoonak mentioned this pull request Aug 30, 2022

Add a Wrapper For fhirPath expressions, cqf-calculatedValue, cqf-library and cqf-expression. #1246

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix issue with empty context with initial expression #1228

Fix issue with empty context with initial expression #1228

RaaziaTarique commented Mar 10, 2022 •

edited

Loading

codecov bot commented Mar 10, 2022 •

edited

Loading

ekigamba Mar 22, 2022

RaaziaTarique Mar 24, 2022 •

edited

Loading

ekigamba Mar 29, 2022

ekigamba Mar 22, 2022

jingtang10 Mar 18, 2022

jingtang10 Mar 18, 2022

jingtang10 Apr 20, 2022

jingtang10 Apr 20, 2022

aditya-07 left a comment

jingtang10 commented Apr 22, 2022

jingtang10 commented Apr 22, 2022

		extractedAnswers =
		fhirPathEngine.evaluate(resources[0], expression.expression.removePrefix("%"))

Fix issue with empty context with initial expression #1228

Fix issue with empty context with initial expression #1228

Conversation

RaaziaTarique commented Mar 10, 2022 • edited Loading

codecov bot commented Mar 10, 2022 • edited Loading

Codecov Report

ekigamba Mar 22, 2022

Choose a reason for hiding this comment

RaaziaTarique Mar 24, 2022 • edited Loading

Choose a reason for hiding this comment

ekigamba Mar 29, 2022

Choose a reason for hiding this comment

ekigamba Mar 22, 2022

Choose a reason for hiding this comment

jingtang10 Mar 18, 2022

Choose a reason for hiding this comment

jingtang10 Mar 18, 2022

Choose a reason for hiding this comment

jingtang10 Apr 20, 2022

Choose a reason for hiding this comment

jingtang10 Apr 20, 2022

Choose a reason for hiding this comment

aditya-07 left a comment

Choose a reason for hiding this comment

jingtang10 commented Apr 22, 2022

jingtang10 commented Apr 22, 2022

RaaziaTarique commented Mar 10, 2022 •

edited

Loading

codecov bot commented Mar 10, 2022 •

edited

Loading

RaaziaTarique Mar 24, 2022 •

edited

Loading