From d4f389a3558c782303fa79df7e077f5972b14222 Mon Sep 17 00:00:00 2001 From: maimoonak Date: Thu, 13 Apr 2023 01:12:08 +0500 Subject: [PATCH 1/4] Fix unpack repeat group issue to linkId based mapping --- .../src/main/assets/behavior_skip_logic.json | 49 +++++++- .../extensions/MoreQuestionnaireResponses.kt | 10 +- .../MoreQuestionnaireResponsesTest.kt | 107 ++++++++++++++++++ 3 files changed, 159 insertions(+), 7 deletions(-) diff --git a/catalog/src/main/assets/behavior_skip_logic.json b/catalog/src/main/assets/behavior_skip_logic.json index d0081550ba..1b347eacbf 100644 --- a/catalog/src/main/assets/behavior_skip_logic.json +++ b/catalog/src/main/assets/behavior_skip_logic.json @@ -74,9 +74,9 @@ ] }, { - "text": "Date of Vaccination?", - "type": "date", - "linkId": "1.1", + "text": "Confirm that vaccine date is known", + "type": "boolean", + "linkId": "2.0.1", "enableWhen": [ { "question": "1.0", @@ -87,6 +87,49 @@ } } ] + }, + { + "text": "Confirm that vaccine date is unknown", + "type": "boolean", + "linkId": "2.0.2", + "enableWhen": [ + { + "question": "1.0", + "operator": "=", + "answerCoding": { + "system": "custom", + "code": "unknown" + } + } + ] + }, + { + "text": "Provide vaccine dates for 2 doses", + "type": "group", + "linkId": "3.0", + "repeats": true, + "enableWhen": [ + { + "question": "1.0", + "operator": "=", + "answerCoding": { + "system": "custom", + "code": "Y" + } + } + ], + "item": [ + { + "text": "Date of Vaccination?", + "type": "date", + "linkId": "3.1" + } + ] + }, + { + "text": "Additional details", + "type": "string", + "linkId": "4.0" } ] } \ No newline at end of file diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponses.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponses.kt index dd03cc4682..e8cb0f49f6 100644 --- a/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponses.kt +++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponses.kt @@ -89,10 +89,12 @@ private fun unpackRepeatedGroups( questionnaireItems: List, questionnaireResponseItems: List ): List { - return questionnaireItems.zip(questionnaireResponseItems).flatMap { - (questionnaireItem, questionnaireResponseItem) -> - unpackRepeatedGroups(questionnaireItem, questionnaireResponseItem) - } + return questionnaireItems + .map { qItem -> qItem to questionnaireResponseItems.find { it.linkId == qItem.linkId } } + .filter { it.second != null } + .flatMap { (questionnaireItem, questionnaireResponseItem) -> + unpackRepeatedGroups(questionnaireItem, questionnaireResponseItem!!) + } } private fun unpackRepeatedGroups( diff --git a/datacapture/src/test/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponsesTest.kt b/datacapture/src/test/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponsesTest.kt index 801f73ab57..dde5d9fb9c 100644 --- a/datacapture/src/test/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponsesTest.kt +++ b/datacapture/src/test/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponsesTest.kt @@ -25,6 +25,7 @@ import org.hl7.fhir.r4.model.Questionnaire.QuestionnaireItemComponent import org.hl7.fhir.r4.model.QuestionnaireResponse import org.hl7.fhir.r4.model.QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent import org.hl7.fhir.r4.model.QuestionnaireResponse.QuestionnaireResponseItemComponent +import org.hl7.fhir.r4.model.Reference import org.hl7.fhir.r4.model.Resource import org.junit.Test @@ -296,6 +297,112 @@ class MoreQuestionnaireResponsesTest { assertResourceEquals(questionnaireResponse, unpackedQuestionnaireResponse) } + @Test + fun `should unpack repeated groups correctly with missing questionnaire response items`() { + val questionnaire = + Questionnaire().apply { + addItem( + QuestionnaireItemComponent().apply { + linkId = "simple-question-1" + type = Questionnaire.QuestionnaireItemType.STRING + } + ) + addItem( + QuestionnaireItemComponent().apply { + linkId = "repeated-group" + type = Questionnaire.QuestionnaireItemType.GROUP + repeats = true + addItem( + QuestionnaireItemComponent().apply { + linkId = "nested-question-1" + type = Questionnaire.QuestionnaireItemType.BOOLEAN + } + ) + addItem( + QuestionnaireItemComponent().apply { + linkId = "nested-question-2" + type = Questionnaire.QuestionnaireItemType.REFERENCE + } + ) + } + ) + } + val questionnaireResponse = + QuestionnaireResponse().apply { + // linkId = "simple-question-1" not present due to enablement + addItem( + QuestionnaireResponseItemComponent().apply { + linkId = "repeated-group" + addAnswer( + QuestionnaireResponseItemAnswerComponent().apply { + // linkId = "nested-question-1" not present due to enablement + addItem( + QuestionnaireResponseItemComponent().apply { + linkId = "nested-question-2" + addAnswer( + QuestionnaireResponseItemAnswerComponent().apply { + value = Reference().apply { reference = "Patient/123" } + } + ) + } + ) + } + ) + addAnswer( + QuestionnaireResponseItemAnswerComponent().apply { + addItem( + QuestionnaireResponseItemComponent().apply { + linkId = "nested-question-2" + addAnswer( + QuestionnaireResponseItemAnswerComponent().apply { + value = Reference().apply { reference = "Patient/456" } + } + ) + } + ) + } + ) + } + ) + } + val unpackedQuestionnaireResponse = + QuestionnaireResponse().apply { + addItem( + QuestionnaireResponseItemComponent().apply { + linkId = "repeated-group" + addItem( + QuestionnaireResponseItemComponent().apply { + linkId = "nested-question-2" + addAnswer( + QuestionnaireResponseItemAnswerComponent().apply { + value = Reference().apply { reference = "Patient/123" } + } + ) + } + ) + } + ) + addItem( + QuestionnaireResponseItemComponent().apply { + linkId = "repeated-group" + addItem( + QuestionnaireResponseItemComponent().apply { + linkId = "nested-question-2" + addAnswer( + QuestionnaireResponseItemAnswerComponent().apply { + value = Reference().apply { reference = "Patient/456" } + } + ) + } + ) + } + ) + } + + questionnaireResponse.unpackRepeatedGroups(questionnaire) + assertResourceEquals(questionnaireResponse, unpackedQuestionnaireResponse) + } + @Test fun `should not modify other items while unpacking repeated groups`() { val questionnaire = From e7d28acc45018a316fc9e87cae6e4d0cf99b1191 Mon Sep 17 00:00:00 2001 From: maimoonak Date: Thu, 13 Apr 2023 14:03:56 +0500 Subject: [PATCH 2/4] Fix the zip mapping and use linkId mapping --- .../src/main/assets/behavior_skip_logic.json | 40 ++++++++++++++++--- 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/catalog/src/main/assets/behavior_skip_logic.json b/catalog/src/main/assets/behavior_skip_logic.json index 1b347eacbf..9c1fa6dd38 100644 --- a/catalog/src/main/assets/behavior_skip_logic.json +++ b/catalog/src/main/assets/behavior_skip_logic.json @@ -76,7 +76,7 @@ { "text": "Confirm that vaccine date is known", "type": "boolean", - "linkId": "2.0.1", + "linkId": "2", "enableWhen": [ { "question": "1.0", @@ -90,8 +90,22 @@ }, { "text": "Confirm that vaccine date is unknown", - "type": "boolean", - "linkId": "2.0.2", + "type": "choice", + "linkId": "3", + "answerOption": [ + { + "valueCoding": { + "code": "Y", + "display": "Yes" + } + }, + { + "valueCoding": { + "code": "N", + "display": "No" + } + } + ], "enableWhen": [ { "question": "1.0", @@ -103,11 +117,25 @@ } ] }, + { + "text": "Confirm that vaccine date is not known", + "type": "boolean", + "linkId": "4", + "enableWhen": [ + { + "question": "1.0", + "operator": "=", + "answerCoding": { + "system": "custom", + "code": "N" + } + } + ] + }, { "text": "Provide vaccine dates for 2 doses", "type": "group", - "linkId": "3.0", - "repeats": true, + "linkId": "5", "enableWhen": [ { "question": "1.0", @@ -129,7 +157,7 @@ { "text": "Additional details", "type": "string", - "linkId": "4.0" + "linkId": "6" } ] } \ No newline at end of file From 5d636d0dc31e4e673807f010fa8811dd855f2115 Mon Sep 17 00:00:00 2001 From: maimoonak Date: Fri, 28 Apr 2023 11:23:40 +0500 Subject: [PATCH 3/4] Fix faling test --- .../src/main/assets/behavior_skip_logic.json | 77 +------------------ .../datacapture/QuestionnaireViewModel.kt | 24 +----- .../MoreQuestionnaireItemComponents.kt | 27 +++++++ .../extensions/MoreQuestionnaireResponses.kt | 7 +- .../MoreQuestionnaireResponsesTest.kt | 48 ++++++++---- 5 files changed, 68 insertions(+), 115 deletions(-) diff --git a/catalog/src/main/assets/behavior_skip_logic.json b/catalog/src/main/assets/behavior_skip_logic.json index 9c1fa6dd38..d0081550ba 100644 --- a/catalog/src/main/assets/behavior_skip_logic.json +++ b/catalog/src/main/assets/behavior_skip_logic.json @@ -74,9 +74,9 @@ ] }, { - "text": "Confirm that vaccine date is known", - "type": "boolean", - "linkId": "2", + "text": "Date of Vaccination?", + "type": "date", + "linkId": "1.1", "enableWhen": [ { "question": "1.0", @@ -87,77 +87,6 @@ } } ] - }, - { - "text": "Confirm that vaccine date is unknown", - "type": "choice", - "linkId": "3", - "answerOption": [ - { - "valueCoding": { - "code": "Y", - "display": "Yes" - } - }, - { - "valueCoding": { - "code": "N", - "display": "No" - } - } - ], - "enableWhen": [ - { - "question": "1.0", - "operator": "=", - "answerCoding": { - "system": "custom", - "code": "unknown" - } - } - ] - }, - { - "text": "Confirm that vaccine date is not known", - "type": "boolean", - "linkId": "4", - "enableWhen": [ - { - "question": "1.0", - "operator": "=", - "answerCoding": { - "system": "custom", - "code": "N" - } - } - ] - }, - { - "text": "Provide vaccine dates for 2 doses", - "type": "group", - "linkId": "5", - "enableWhen": [ - { - "question": "1.0", - "operator": "=", - "answerCoding": { - "system": "custom", - "code": "Y" - } - } - ], - "item": [ - { - "text": "Date of Vaccination?", - "type": "date", - "linkId": "3.1" - } - ] - }, - { - "text": "Additional details", - "type": "string", - "linkId": "6" } ] } \ No newline at end of file diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt index 3e37c65bfe..ad6f9cff6b 100644 --- a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt +++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt @@ -46,6 +46,7 @@ import com.google.android.fhir.datacapture.extensions.packRepeatedGroups import com.google.android.fhir.datacapture.extensions.shouldHaveNestedItemsUnderAnswers import com.google.android.fhir.datacapture.extensions.unpackRepeatedGroups import com.google.android.fhir.datacapture.extensions.validateLaunchContext +import com.google.android.fhir.datacapture.extensions.zipByLinkId import com.google.android.fhir.datacapture.fhirpath.ExpressionEvaluator import com.google.android.fhir.datacapture.fhirpath.ExpressionEvaluator.detectExpressionCyclicDependency import com.google.android.fhir.datacapture.fhirpath.ExpressionEvaluator.evaluateCalculatedExpressions @@ -872,26 +873,3 @@ internal val QuestionnairePagination.hasPreviousPage: Boolean internal val QuestionnairePagination.hasNextPage: Boolean get() = pages.any { it.index > currentPageIndex && it.enabled } - -/** - * Returns a list of values built from the elements of `this` and the - * `questionnaireResponseItemList` with the same linkId using the provided `transform` function - * applied to each pair of questionnaire item and questionnaire response item. - * - * It is assumed that the linkIds are unique in `this` and in `questionnaireResponseItemList`. - * - * Although linkIds may appear more than once in questionnaire response, they would not appear more - * than once within a list of questionnaire response items sharing the same parent. - */ -private inline fun List.zipByLinkId( - questionnaireResponseItemList: List, - transform: (QuestionnaireItemComponent, QuestionnaireResponseItemComponent) -> T -): List { - val linkIdToQuestionnaireResponseItemMap = questionnaireResponseItemList.associateBy { it.linkId } - return mapNotNull { questionnaireItem -> - linkIdToQuestionnaireResponseItemMap[questionnaireItem.linkId]?.let { questionnaireResponseItem - -> - transform(questionnaireItem, questionnaireResponseItem) - } - } -} diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponents.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponents.kt index 2f9ad14109..996c0122ee 100644 --- a/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponents.kt +++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponents.kt @@ -433,6 +433,33 @@ internal val Questionnaire.QuestionnaireItemComponent.sliderStepValue: Int? return null } +/** + * Returns a list of values built from the elements of `this` and the + * `questionnaireResponseItemList` with the same linkId using the provided `transform` function + * applied to each pair of questionnaire item and questionnaire response item. + * + * It is assumed that the linkIds are unique in `this` and in `questionnaireResponseItemList`. + * + * Although linkIds may appear more than once in questionnaire response, they would not appear more + * than once within a list of questionnaire response items sharing the same parent. + */ +inline fun List.zipByLinkId( + questionnaireResponseItemList: List, + transform: + ( + Questionnaire.QuestionnaireItemComponent, + QuestionnaireResponse.QuestionnaireResponseItemComponent + ) -> T +): List { + val linkIdToQuestionnaireResponseItemMap = questionnaireResponseItemList.associateBy { it.linkId } + return mapNotNull { questionnaireItem -> + linkIdToQuestionnaireResponseItemMap[questionnaireItem.linkId]?.let { questionnaireResponseItem + -> + transform(questionnaireItem, questionnaireResponseItem) + } + } +} + /** * Whether the corresponding [QuestionnaireResponse.QuestionnaireResponseItemComponent] should have * [QuestionnaireResponse.QuestionnaireResponseItemComponent]s nested under diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponses.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponses.kt index e8cb0f49f6..974ffe2723 100644 --- a/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponses.kt +++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponses.kt @@ -90,11 +90,10 @@ private fun unpackRepeatedGroups( questionnaireResponseItems: List ): List { return questionnaireItems - .map { qItem -> qItem to questionnaireResponseItems.find { it.linkId == qItem.linkId } } - .filter { it.second != null } - .flatMap { (questionnaireItem, questionnaireResponseItem) -> - unpackRepeatedGroups(questionnaireItem, questionnaireResponseItem!!) + .zipByLinkId(questionnaireResponseItems) { questionnaireItem, questionnaireResponseItem -> + unpackRepeatedGroups(questionnaireItem, questionnaireResponseItem) } + .flatten() } private fun unpackRepeatedGroups( diff --git a/datacapture/src/test/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponsesTest.kt b/datacapture/src/test/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponsesTest.kt index dde5d9fb9c..0456874c30 100644 --- a/datacapture/src/test/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponsesTest.kt +++ b/datacapture/src/test/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponsesTest.kt @@ -421,8 +421,9 @@ class MoreQuestionnaireResponsesTest { ) addItem( QuestionnaireItemComponent().apply { - linkId = "non-repeated-group-2" + linkId = "repeated-group-2" type = Questionnaire.QuestionnaireItemType.GROUP + repeats = true addItem( QuestionnaireItemComponent().apply { linkId = "nested-question-2" @@ -438,7 +439,7 @@ class MoreQuestionnaireResponsesTest { QuestionnaireResponse().apply { addItem( QuestionnaireResponseItemComponent().apply { - linkId = "repeated-group-1" + linkId = "non-repeated-group-1" addAnswer( QuestionnaireResponseItemAnswerComponent().apply { addItem( @@ -470,6 +471,18 @@ class MoreQuestionnaireResponsesTest { ) } ) + addAnswer( + QuestionnaireResponseItemAnswerComponent().apply { + addItem( + QuestionnaireResponseItemComponent().apply { + linkId = "nested-question-2" + addAnswer( + QuestionnaireResponseItemAnswerComponent().apply { value = BooleanType(true) } + ) + } + ) + } + ) } ) } @@ -477,7 +490,7 @@ class MoreQuestionnaireResponsesTest { QuestionnaireResponse().apply { addItem( QuestionnaireResponseItemComponent().apply { - linkId = "repeated-group-1" + linkId = "non-repeated-group-1" addAnswer( QuestionnaireResponseItemAnswerComponent().apply { addItem( @@ -495,17 +508,24 @@ class MoreQuestionnaireResponsesTest { addItem( QuestionnaireResponseItemComponent().apply { linkId = "repeated-group-2" - addAnswer( - QuestionnaireResponseItemAnswerComponent().apply { - addItem( - QuestionnaireResponseItemComponent().apply { - linkId = "nested-question-2" - addAnswer( - QuestionnaireResponseItemAnswerComponent().apply { - value = BooleanType(false) - } - ) - } + addItem( + QuestionnaireResponseItemComponent().apply { + linkId = "nested-question-2" + addAnswer( + QuestionnaireResponseItemAnswerComponent().apply { value = BooleanType(false) } + ) + } + ) + } + ) + addItem( + QuestionnaireResponseItemComponent().apply { + linkId = "repeated-group-2" + addItem( + QuestionnaireResponseItemComponent().apply { + linkId = "nested-question-2" + addAnswer( + QuestionnaireResponseItemAnswerComponent().apply { value = BooleanType(true) } ) } ) From e0553c9455202467369b828a90cbbe998be28ca5 Mon Sep 17 00:00:00 2001 From: Jing Tang Date: Wed, 3 May 2023 11:51:10 +0100 Subject: [PATCH 4/4] Update datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponents.kt --- .../datacapture/extensions/MoreQuestionnaireItemComponents.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponents.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponents.kt index 996c0122ee..da0c36a36d 100644 --- a/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponents.kt +++ b/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponents.kt @@ -443,7 +443,7 @@ internal val Questionnaire.QuestionnaireItemComponent.sliderStepValue: Int? * Although linkIds may appear more than once in questionnaire response, they would not appear more * than once within a list of questionnaire response items sharing the same parent. */ -inline fun List.zipByLinkId( +internal inline fun List.zipByLinkId( questionnaireResponseItemList: List, transform: (