Fix zip index based mapping issue when unpacking repeat groups (#1973)

* Fix unpack repeat group issue to linkId based mapping * Fix the zip mapping and use linkId mapping * Fix faling test * Update datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponents.kt --------- Co-authored-by: Benjamin Mwalimu <[email protected]> Co-authored-by: Jing Tang <[email protected]>
google · May 3, 2023 · 675237f · 675237f
1 parent 97a63ad
commit 675237f
Show file tree

Hide file tree

Showing 4 changed files with 174 additions and 41 deletions.
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
@@ -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 <T> List<QuestionnaireItemComponent>.zipByLinkId(
- questionnaireResponseItemList: List<QuestionnaireResponseItemComponent>,
- transform: (QuestionnaireItemComponent, QuestionnaireResponseItemComponent) -> T
-): List<T> {
- val linkIdToQuestionnaireResponseItemMap = questionnaireResponseItemList.associateBy { it.linkId }
- return mapNotNull { questionnaireItem ->
- linkIdToQuestionnaireResponseItemMap[questionnaireItem.linkId]?.let { questionnaireResponseItem
- ->
- transform(questionnaireItem, questionnaireResponseItem)
- }
- }
-}
diff --git a/...in/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponents.kt b/...in/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponents.kt
@@ -436,6 +436,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.
+ */
+internal inline fun <T> List<Questionnaire.QuestionnaireItemComponent>.zipByLinkId(
+ questionnaireResponseItemList: List<QuestionnaireResponse.QuestionnaireResponseItemComponent>,
+ transform:
+ (
+ Questionnaire.QuestionnaireItemComponent,
+ QuestionnaireResponse.QuestionnaireResponseItemComponent
+ ) -> T
+): List<T> {
+ 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/...rc/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponses.kt b/...rc/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponses.kt
@@ -89,10 +89,11 @@ private fun unpackRepeatedGroups(
  questionnaireItems: List<Questionnaire.QuestionnaireItemComponent>,
  questionnaireResponseItems: List<QuestionnaireResponse.QuestionnaireResponseItemComponent>
 ): List<QuestionnaireResponse.QuestionnaireResponseItemComponent> {
- return questionnaireItems.zip(questionnaireResponseItems).flatMap {
- (questionnaireItem, questionnaireResponseItem) ->
- unpackRepeatedGroups(questionnaireItem, questionnaireResponseItem)
- }
+ return questionnaireItems
+ .zipByLinkId(questionnaireResponseItems) { questionnaireItem, questionnaireResponseItem ->
+ unpackRepeatedGroups(questionnaireItem, questionnaireResponseItem)
+ }
+ .flatten()
 }
 
 private fun unpackRepeatedGroups(

diff --git a/...est/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponsesTest.kt b/...est/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 =
@@ -314,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"
@@ -331,7 +439,7 @@ class MoreQuestionnaireResponsesTest {
  QuestionnaireResponse().apply {
  addItem(
  QuestionnaireResponseItemComponent().apply {
- linkId = "repeated-group-1"
+ linkId = "non-repeated-group-1"
  addAnswer(
  QuestionnaireResponseItemAnswerComponent().apply {
  addItem(
@@ -363,14 +471,26 @@ class MoreQuestionnaireResponsesTest {
  )
  }
  )
+ addAnswer(
+ QuestionnaireResponseItemAnswerComponent().apply {
+ addItem(
+ QuestionnaireResponseItemComponent().apply {
+ linkId = "nested-question-2"
+ addAnswer(
+ QuestionnaireResponseItemAnswerComponent().apply { value = BooleanType(true) }
+ )
+ }
+ )
+ }
+ )
  }
  )
  }
  val unpackedQuestionnaireResponse =
  QuestionnaireResponse().apply {
  addItem(
  QuestionnaireResponseItemComponent().apply {
- linkId = "repeated-group-1"
+ linkId = "non-repeated-group-1"
  addAnswer(
  QuestionnaireResponseItemAnswerComponent().apply {
  addItem(
@@ -388,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) }
  )
  }
  )