Repeated Group: Fix Definition based extraction for multiple answers (#…

…1911) * Iterating questionnaire response items and finding questionnaire item to extract * feedback changes * Replace find for currentQuestionnaireItem with map as per the discussion in the PR * Modified logic for zipByLinkId and used it. * feedback and Test * remove additional comments --------- Co-authored-by: Jing Tang <[email protected]>
google · Dec 8, 2023 · 088d381 · 088d381
1 parent 65d7c1f
commit 088d381
Show file tree

Hide file tree

Showing 4 changed files with 211 additions and 54 deletions.
diff --git a/...in/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponents.kt b/...in/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponents.kt
@@ -769,10 +769,9 @@ internal fun Questionnaire.QuestionnaireItemComponent.extractAnswerOptions(
  * `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.
+ * In case of repeated group item, `questionnaireResponseItemList` will contain
+ * QuestionnaireResponseItemComponent with same linkId. So these items are grouped with linkId and
+ * associated with its questionnaire item linkId.
  */
 internal inline fun <T> List<Questionnaire.QuestionnaireItemComponent>.zipByLinkId(
   questionnaireResponseItemList: List<QuestionnaireResponse.QuestionnaireResponseItemComponent>,
@@ -782,12 +781,13 @@ internal inline fun <T> List<Questionnaire.QuestionnaireItemComponent>.zipByLink
       QuestionnaireResponse.QuestionnaireResponseItemComponent,
     ) -> T,
 ): List<T> {
-  val linkIdToQuestionnaireResponseItemMap = questionnaireResponseItemList.associateBy { it.linkId }
-  return mapNotNull { questionnaireItem ->
-    linkIdToQuestionnaireResponseItemMap[questionnaireItem.linkId]?.let { questionnaireResponseItem,
-      ->
+  val linkIdToQuestionnaireResponseItemListMap = questionnaireResponseItemList.groupBy { it.linkId }
+  return flatMap { questionnaireItem ->
+    linkIdToQuestionnaireResponseItemListMap[questionnaireItem.linkId]?.mapNotNull {
+      questionnaireResponseItem ->
       transform(questionnaireItem, questionnaireResponseItem)
     }
+      ?: emptyList()
   }
 }
 

diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/mapping/ResourceMapper.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/mapping/ResourceMapper.kt
@@ -28,6 +28,7 @@ import com.google.android.fhir.datacapture.extensions.toCoding
 import com.google.android.fhir.datacapture.extensions.toIdType
 import com.google.android.fhir.datacapture.extensions.toUriType
 import com.google.android.fhir.datacapture.extensions.validateLaunchContextExtensions
+import com.google.android.fhir.datacapture.extensions.zipByLinkId
 import com.google.android.fhir.datacapture.fhirpath.fhirPathEngine
 import java.lang.reflect.Field
 import java.lang.reflect.Method
@@ -290,31 +291,17 @@ object ResourceMapper {
     extractionResult: MutableList<Resource>,
     profileLoader: ProfileLoader,
   ) {
-    val questionnaireItemListIterator = questionnaireItemList.iterator()
-    val questionnaireResponseItemListIterator = questionnaireResponseItemList.iterator()
-    while (
-      questionnaireItemListIterator.hasNext() && questionnaireResponseItemListIterator.hasNext()
-    ) {
-      val currentQuestionnaireResponseItem = questionnaireResponseItemListIterator.next()
-      var currentQuestionnaireItem = questionnaireItemListIterator.next()
-      // Find the next questionnaire item with the same link ID. This is necessary because some
-      // questionnaire items that are disabled might not have corresponding questionnaire response
-      // items.
-      while (
-        questionnaireItemListIterator.hasNext() &&
-          currentQuestionnaireItem.linkId != currentQuestionnaireResponseItem.linkId
-      ) {
-        currentQuestionnaireItem = questionnaireItemListIterator.next()
-      }
-      if (currentQuestionnaireItem.linkId == currentQuestionnaireResponseItem.linkId) {
-        extractByDefinition(
-          currentQuestionnaireItem,
-          currentQuestionnaireResponseItem,
-          extractionContext,
-          extractionResult,
-          profileLoader,
-        )
-      }
+    questionnaireItemList.zipByLinkId(questionnaireResponseItemList) {
+      questionnaireItem,
+      questionnaireResponseItem,
+      ->
+      extractByDefinition(
+        questionnaireItem,
+        questionnaireResponseItem,
+        extractionContext,
+        extractionResult,
+        profileLoader,
+      )
     }
   }
 

diff --git a/...ava/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponentsTest.kt b/...ava/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponentsTest.kt
@@ -39,6 +39,7 @@ import org.hl7.fhir.r4.model.IntegerType
 import org.hl7.fhir.r4.model.Patient
 import org.hl7.fhir.r4.model.Quantity
 import org.hl7.fhir.r4.model.Questionnaire
+import org.hl7.fhir.r4.model.QuestionnaireResponse
 import org.hl7.fhir.r4.model.StringType
 import org.hl7.fhir.r4.utils.ToolingExtensions
 import org.junit.Assert.assertThrows
@@ -2295,6 +2296,81 @@ class MoreQuestionnaireItemComponentsTest {
     assertThat(questionnaireItem.dateEntryFormatOrSystemDefault).isEqualTo("y-MM-dd")
   }
 
+  @Test
+  fun `should return empty list for empty response item list with same linkId`() {
+    val questionnaireItemComponentList =
+      listOf(
+        Questionnaire.QuestionnaireItemComponent().apply { linkId = "1" },
+      )
+
+    val questionnaireResponseItemComponentList =
+      listOf<QuestionnaireResponse.QuestionnaireResponseItemComponent>()
+
+    val result =
+      questionnaireItemComponentList.zipByLinkId(questionnaireResponseItemComponentList) { _, _ -> }
+    assertThat(result.size).isEqualTo(0)
+  }
+
+  @Test
+  fun `should return non empty list for valid questionnaire item and response list`() {
+    val questionnaireItemComponentList =
+      listOf(
+        Questionnaire.QuestionnaireItemComponent().apply { linkId = "1" },
+        Questionnaire.QuestionnaireItemComponent().apply { linkId = "2" },
+      )
+
+    val questionnaireResponseItemComponentList =
+      listOf(
+        QuestionnaireResponse.QuestionnaireResponseItemComponent().apply { linkId = "1" },
+        QuestionnaireResponse.QuestionnaireResponseItemComponent().apply { linkId = "2" },
+      )
+
+    val zipList =
+      questionnaireItemComponentList.zipByLinkId(questionnaireResponseItemComponentList) { _, _ -> }
+    assertThat(zipList.size).isEqualTo(2)
+  }
+
+  @Test
+  fun `should return non empty list for valid questionnaire item and repeated response list with same linkId`() {
+    val questionnaireItemComponentList =
+      listOf(
+        Questionnaire.QuestionnaireItemComponent().apply { linkId = "1" },
+        Questionnaire.QuestionnaireItemComponent().apply { linkId = "2" },
+      )
+
+    val questionnaireResponseItemComponentList =
+      listOf(
+        QuestionnaireResponse.QuestionnaireResponseItemComponent().apply { linkId = "1" },
+        QuestionnaireResponse.QuestionnaireResponseItemComponent().apply { linkId = "2" },
+        QuestionnaireResponse.QuestionnaireResponseItemComponent().apply { linkId = "2" },
+      )
+
+    val zipList =
+      questionnaireItemComponentList.zipByLinkId(questionnaireResponseItemComponentList) { _, _ -> }
+    assertThat(zipList.size).isEqualTo(3)
+  }
+
+  @Test
+  fun `should return non empty list for out of order questionnaire item and response item`() {
+    val questionnaireItemComponentList =
+      listOf(
+        Questionnaire.QuestionnaireItemComponent().apply { linkId = "1" },
+        Questionnaire.QuestionnaireItemComponent().apply { linkId = "3" },
+        Questionnaire.QuestionnaireItemComponent().apply { linkId = "2" },
+      )
+
+    val questionnaireResponseItemComponentList =
+      listOf(
+        QuestionnaireResponse.QuestionnaireResponseItemComponent().apply { linkId = "3" },
+        QuestionnaireResponse.QuestionnaireResponseItemComponent().apply { linkId = "2" },
+        QuestionnaireResponse.QuestionnaireResponseItemComponent().apply { linkId = "1" },
+      )
+
+    val zipList =
+      questionnaireItemComponentList.zipByLinkId(questionnaireResponseItemComponentList) { _, _ -> }
+    assertThat(zipList.size).isEqualTo(3)
+  }
+
   private val displayCategoryExtensionWithInstructionsCode =
     Extension().apply {
       url = EXTENSION_DISPLAY_CATEGORY_URL

diff --git a/datacapture/src/test/java/com/google/android/fhir/datacapture/mapping/ResourceMapperTest.kt b/datacapture/src/test/java/com/google/android/fhir/datacapture/mapping/ResourceMapperTest.kt
@@ -1027,27 +1027,6 @@ class ResourceMapperTest {
                     }
                   ]
                 },
-                {
-                  "linkId": "PR-address",
-                  "item": [
-                    {
-                      "linkId": "PR-address-city",
-                      "answer": [
-                        {
-                          "valueString": "Nairobi"
-                        }
-                      ]
-                    },
-                    {
-                      "linkId": "PR-address-country",
-                      "answer": [
-                        {
-                          "valueString": "Kenya"
-                        }
-                      ]
-                    }
-                  ]
-                },
                 {
                   "linkId": "PR-active"
                 }
@@ -1177,6 +1156,121 @@ class ResourceMapperTest {
       assertThat(observation.valueQuantity.value).isEqualTo(BigDecimal(90))
     }
 
+  @Test
+  fun `extract() should perform definition-based extraction for repeated groups`() = runBlocking {
+    @Language("JSON")
+    val questionnaireJson =
+      """
+      {
+        "resourceType": "Questionnaire",
+        "item": [
+          {
+            "linkId": "repeated-parent",
+            "type": "group",
+            "repeats": true,
+            "extension": [
+              {
+                "url": "http://hl7.org/fhir/uv/sdc/StructureDefinition/sdc-questionnaire-itemExtractionContext",
+                "valueExpression": {
+                  "expression": "Observation"
+                }
+              }
+            ],
+            "item": [
+              {
+                "linkId": "1.0",
+                "type": "group",
+                "definition": "http://hl7.org/fhir/StructureDefinition/Observation#Observation.valueCodeableConcept",
+                "item": [
+                  {
+                    "linkId": "1.0.1",
+                    "type": "choice",
+                    "definition": "http://hl7.org/fhir/StructureDefinition/Observation#Observation.valueCodeableConcept.coding"
+                  }
+                ]
+              }
+            ]
+          }
+        ]
+      }
+            """
+        .trimIndent()
+
+    @Language("JSON")
+    val questionnaireResponseJson =
+      """
+        {
+          "resourceType": "QuestionnaireResponse",
+          "item": [
+            {
+              "linkId": "repeated-parent",
+              "item": [
+                {
+                  "linkId": "1.0",
+                  "item": [
+                    {
+                      "linkId": "1.0.1",
+                      "answer": [
+                        {
+                          "valueCoding": {
+                            "system": "test-coding-system",
+                            "code": "test-coding-code-1",
+                            "display": "Test Coding Display 1"
+                          }
+                        }
+                      ]
+                    }
+                  ]
+                }
+              ]
+            },
+            {
+              "linkId": "repeated-parent",
+              "item": [
+                {
+                  "linkId": "1.0",
+                  "item": [
+                    {
+                      "linkId": "1.0.1",
+                      "answer": [
+                        {
+                          "valueCoding": {
+                            "system": "test-coding-system",
+                            "code": "test-coding-code-2",
+                            "display": "Test Coding Display 2"
+                          }
+                        }
+                      ]
+                    }
+                  ]
+                }
+              ]
+            }
+          ]
+        }
+            """
+        .trimIndent()
+
+    val uriTestQuestionnaire =
+      iParser.parseResource(Questionnaire::class.java, questionnaireJson) as Questionnaire
+
+    val uriTestQuestionnaireResponse =
+      iParser.parseResource(QuestionnaireResponse::class.java, questionnaireResponseJson)
+        as QuestionnaireResponse
+
+    val bundle = ResourceMapper.extract(uriTestQuestionnaire, uriTestQuestionnaireResponse)
+
+    val observation1 = bundle.entry.first().resource as Observation
+    assertThat(observation1.valueCodeableConcept.coding[0].code).isEqualTo("test-coding-code-1")
+    assertThat(observation1.valueCodeableConcept.coding[0].display)
+      .isEqualTo("Test Coding Display 1")
+
+    val observation2 = bundle.entry[1].resource as Observation
+    assertThat(observation2.valueCodeableConcept.coding[0].code).isEqualTo("test-coding-code-2")
+    assertThat(observation2.valueCodeableConcept.coding[0].display)
+      .isEqualTo("Test Coding Display 2")
+  }
+
   @Test
   fun `populate() should fill QuestionnaireResponse with values when given a single Resource`() =
     runBlocking {