Create nested items correctly (#1881)

* Create nested items correctly * Revert incorrect questionnaire change * Update MoreQuestionnaireItemComponents.kt * Re-enable test case * Update nit * Update doc for zipByLinkId * Update datacapture/src/main/java/com/google/android/fhir/datacapture/MoreQuestionnaireItemComponents.kt Co-authored-by: Omar Ismail <[email protected]> * Update doc in QuestionnaireViewModel * Add another level to the test for nested items --------- Co-authored-by: Omar Ismail <[email protected]>
google · Feb 22, 2023 · b80747a · b80747a
1 parent a9d95db
commit b80747a
Show file tree

Hide file tree

Showing 6 changed files with 178 additions and 225 deletions.
diff --git a/...ture/src/main/java/com/google/android/fhir/datacapture/MoreQuestionnaireItemComponents.kt b/...ture/src/main/java/com/google/android/fhir/datacapture/MoreQuestionnaireItemComponents.kt
@@ -284,14 +284,6 @@ internal val Questionnaire.QuestionnaireItemComponent.isHelpCode: Boolean
       }
     }
   }
-
-/**
- * Whether the corresponding [QuestionnaireResponse.QuestionnaireResponseItemComponent] should have
- * nested items within [QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent](s).
- */
-internal val Questionnaire.QuestionnaireItemComponent.hasNestedItemsWithinAnswers: Boolean
-  get() = item.isNotEmpty() && type != Questionnaire.QuestionnaireItemType.GROUP
-
 /** Converts Text with HTML Tag to formated text. */
 private fun String.toSpanned(): Spanned {
   return HtmlCompat.fromHtml(this, HtmlCompat.FROM_HTML_MODE_COMPACT)
@@ -424,6 +416,33 @@ internal val Questionnaire.QuestionnaireItemComponent.sliderStepValue: Int?
     return null
   }
 
+/**
+ * Whether the corresponding [QuestionnaireResponse.QuestionnaireResponseItemComponent] should have
+ * [QuestionnaireResponse.QuestionnaireResponseItemComponent]s nested under
+ * [QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent]s.
+ *
+ * This is true for the following two cases:
+ * 1. Questions with nested items
+ * 2. Repeated groups with nested items (Note that this is how repeated groups are organized in the
+ * [QuestionnaireViewModel], and that they will be flattened in the final [QuestionnaireResponse].)
+ *
+ * Non-repeated groups should have child items nested directly under the group itself.
+ *
+ * For background, see https://build.fhir.org/questionnaireresponse.html#link.
+ */
+internal val Questionnaire.QuestionnaireItemComponent.shouldHaveNestedItemsUnderAnswers: Boolean
+  get() = item.isNotEmpty() && (type != Questionnaire.QuestionnaireItemType.GROUP || !repeats)
+
+/**
+ * Creates a list of [QuestionnaireResponse.QuestionnaireResponseItemComponent]s from the nested
+ * items in the [Questionnaire.QuestionnaireItemComponent].
+ *
+ * The hierarchy and order of child items will be retained as specified in the standard. See
+ * https://www.hl7.org/fhir/questionnaireresponse.html#notes for more details.
+ */
+fun Questionnaire.QuestionnaireItemComponent.getNestedQuestionnaireResponseItems() =
+  item.map { it.createQuestionnaireResponseItem() }
+
 /**
  * Creates a [QuestionnaireResponse.QuestionnaireResponseItemComponent] from the provided
  * [Questionnaire.QuestionnaireItemComponent].
@@ -436,10 +455,10 @@ fun Questionnaire.QuestionnaireItemComponent.createQuestionnaireResponseItem():
   return QuestionnaireResponse.QuestionnaireResponseItemComponent().apply {
     linkId = this@createQuestionnaireResponseItem.linkId
     answer = createQuestionnaireResponseItemAnswers()
-    if (hasNestedItemsWithinAnswers && answer.isNotEmpty()) {
+    if (shouldHaveNestedItemsUnderAnswers && answer.isNotEmpty()) {
       this.addNestedItemsToAnswer(this@createQuestionnaireResponseItem)
     } else if (this@createQuestionnaireResponseItem.type ==
-        Questionnaire.QuestionnaireItemType.GROUP
+        Questionnaire.QuestionnaireItemType.GROUP && !repeats
     ) {
       this@createQuestionnaireResponseItem.item.forEach {
         this.addItem(it.createQuestionnaireResponseItem())
@@ -492,20 +511,6 @@ private fun Questionnaire.QuestionnaireItemComponent.createQuestionnaireResponse
   )
 }
 
-/**
- * Add items within [QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent] from the
- * provided parent [Questionnaire.QuestionnaireItemComponent] with nested items. The hierarchy and
- * order of child items will be retained as specified in the standard. See
- * https://www.hl7.org/fhir/questionnaireresponse.html#notes for more details.
- */
-fun QuestionnaireResponse.QuestionnaireResponseItemComponent.addNestedItemsToAnswer(
-  questionnaireItemComponent: Questionnaire.QuestionnaireItemComponent
-) {
-  if (answer.isNotEmpty()) {
-    answer.first().item = questionnaireItemComponent.getNestedQuestionnaireResponseItems()
-  }
-}
-
 internal val Questionnaire.QuestionnaireItemComponent.answerExpression: Expression?
   get() =
     ToolingExtensions.getExtension(this, EXTENSION_ANSWER_EXPRESSION_URL)?.value?.let {
@@ -599,16 +604,6 @@ fun List<Questionnaire.QuestionnaireItemComponent>.flattened():
   return this + this.flatMap { it.item.flattened() }
 }
 
-/**
- * Creates a list of [QuestionnaireResponse.QuestionnaireResponseItemComponent]s from the nested
- * items in the [Questionnaire.QuestionnaireItemComponent].
- *
- * The hierarchy and order of child items will be retained as specified in the standard. See
- * https://www.hl7.org/fhir/questionnaireresponse.html#notes for more details.
- */
-fun Questionnaire.QuestionnaireItemComponent.getNestedQuestionnaireResponseItems() =
-  item.map { it.createQuestionnaireResponseItem() }
-
 val Resource.logicalId: String
   get() {
     return this.idElement?.idPart.orEmpty()

diff --git a/...main/java/com/google/android/fhir/datacapture/MoreQuestionnairesResponseItemComponents.kt b/...main/java/com/google/android/fhir/datacapture/MoreQuestionnairesResponseItemComponents.kt
@@ -16,6 +16,7 @@
 
 package com.google.android.fhir.datacapture
 
+import org.hl7.fhir.r4.model.Questionnaire
 import org.hl7.fhir.r4.model.QuestionnaireResponse
 
 /**
@@ -28,3 +29,16 @@ val QuestionnaireResponse.QuestionnaireResponseItemComponent.descendant:
       this.item.flatMap { it.descendant } +
       this.answer.flatMap { answer -> answer.item.flatMap { it.descendant } }
   }
+
+/**
+ * Add nested items under the provided `questionnaireItem` to each answer in the questionnaire
+ * response item. The hierarchy and order of nested items will be retained as specified in the
+ * standard.
+ *
+ * See https://www.hl7.org/fhir/questionnaireresponse.html#notes for more details.
+ */
+fun QuestionnaireResponse.QuestionnaireResponseItemComponent.addNestedItemsToAnswer(
+  questionnaireItem: Questionnaire.QuestionnaireItemComponent
+) {
+  answer.forEach { it.item = questionnaireItem.getNestedQuestionnaireResponseItems() }
+}
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 kotlinx.coroutines.flow.update
 import org.hl7.fhir.r4.model.Coding
 import org.hl7.fhir.r4.model.Expression
 import org.hl7.fhir.r4.model.Questionnaire
+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
@@ -89,9 +90,6 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
       }
   }
 
-  @VisibleForTesting
-  val entryMode: EntryMode by lazy { questionnaire.entryMode ?: EntryMode.RANDOM }
-
   /** The current questionnaire response as questions are being answered. */
   private val questionnaireResponse: QuestionnaireResponse
 
@@ -133,12 +131,12 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
 
   /** The map from each item in the [Questionnaire] to its parent. */
   private var questionnaireItemParentMap:
-    Map<Questionnaire.QuestionnaireItemComponent, Questionnaire.QuestionnaireItemComponent>
+    Map<QuestionnaireItemComponent, QuestionnaireItemComponent>
 
   init {
     /** Adds each child-parent pair in the [Questionnaire] to the parent map. */
     fun buildParentList(
-      item: Questionnaire.QuestionnaireItemComponent,
+      item: QuestionnaireItemComponent,
       questionnaireItemToParentMap: ItemToParentMap,
     ) {
       for (child in item.item) {
@@ -154,6 +152,9 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
     }
   }
 
+  @VisibleForTesting
+  val entryMode: EntryMode by lazy { questionnaire.entryMode ?: EntryMode.RANDOM }
+
   /** Flag to determine if the questionnaire should be read-only. */
   private val isReadOnly = state[QuestionnaireFragment.EXTRA_READ_ONLY] ?: false
 
@@ -244,7 +245,7 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
    */
   private val answersChangedCallback:
     (
-      Questionnaire.QuestionnaireItemComponent,
+      QuestionnaireItemComponent,
       QuestionnaireResponseItemComponent,
       List<QuestionnaireResponseItemAnswerComponent>,
       Any?
@@ -264,7 +265,7 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
           }
         }
       }
-      if (questionnaireItem.hasNestedItemsWithinAnswers) {
+      if (questionnaireItem.shouldHaveNestedItemsUnderAnswers) {
         questionnaireResponseItem.addNestedItemsToAnswer(questionnaireItem)
       }
       modifiedQuestionnaireResponseItemSet.add(questionnaireResponseItem)
@@ -390,7 +391,7 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
       )
 
   private fun updateDependentQuestionnaireResponseItems(
-    updatedQuestionnaireItem: Questionnaire.QuestionnaireItemComponent,
+    updatedQuestionnaireItem: QuestionnaireItemComponent,
   ) {
     evaluateCalculatedExpressions(
         updatedQuestionnaireItem,
@@ -464,7 +465,7 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
   // https://build.fhir.org/ig/HL7/sdc/expressions.html#x-fhir-query-enhancements
   @PublishedApi
   internal suspend fun resolveAnswerExpression(
-    item: Questionnaire.QuestionnaireItemComponent,
+    item: QuestionnaireItemComponent,
   ): List<Questionnaire.QuestionnaireItemAnswerOptionComponent> {
     // Check cache first for database queries
     val answerExpression = item.answerExpression ?: return emptyList()
@@ -481,7 +482,7 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
   }
 
   private suspend fun loadAnswerExpressionOptions(
-    item: Questionnaire.QuestionnaireItemComponent,
+    item: QuestionnaireItemComponent,
     expression: Expression,
   ): List<Questionnaire.QuestionnaireItemAnswerOptionComponent> {
     val data =
@@ -573,34 +574,22 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
    * questionnaire response items.
    */
   private fun getQuestionnaireAdapterItems(
-    questionnaireItemList: List<Questionnaire.QuestionnaireItemComponent>,
+    questionnaireItemList: List<QuestionnaireItemComponent>,
     questionnaireResponseItemList: List<QuestionnaireResponseItemComponent>,
   ): List<QuestionnaireAdapterItem> {
-    var responseIndex = 0
     return questionnaireItemList
-      .asSequence()
-      .flatMap { questionnaireItem ->
-        var questionnaireResponseItem = questionnaireItem.createQuestionnaireResponseItem()
-        // If there is an enabled questionnaire response available then we use that. Or else we
-        // just use an empty questionnaireResponse Item
-        if (responseIndex < questionnaireResponseItemList.size &&
-            questionnaireItem.linkId == questionnaireResponseItemList[responseIndex].linkId
-        ) {
-          questionnaireResponseItem = questionnaireResponseItemList[responseIndex]
-          responseIndex += 1
-        }
-
+      .zipByLinkId(questionnaireResponseItemList) { questionnaireItem, questionnaireResponseItem ->
         getQuestionnaireAdapterItems(questionnaireItem, questionnaireResponseItem)
       }
-      .toList()
+      .flatten()
   }
 
   /**
    * Returns the list of [QuestionnaireItemViewItem]s generated for the questionnaire item and
    * questionnaire response item.
    */
   private fun getQuestionnaireAdapterItems(
-    questionnaireItem: Questionnaire.QuestionnaireItemComponent,
+    questionnaireItem: QuestionnaireItemComponent,
     questionnaireResponseItem: QuestionnaireResponseItemComponent,
   ): List<QuestionnaireAdapterItem> {
     // Hidden questions should not get QuestionnaireItemViewItem instances
@@ -630,31 +619,42 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
       } else {
         NotValidated
       }
-    val items =
-      buildList<QuestionnaireAdapterItem> {
-        // Add an item for the question itself
-        add(
-          QuestionnaireAdapterItem.Question(
-            QuestionnaireItemViewItem(
-              questionnaireItem,
-              questionnaireResponseItem,
-              validationResult = validationResult,
-              answersChangedCallback = answersChangedCallback,
-              resolveAnswerValueSet = { resolveAnswerValueSet(it) },
-              resolveAnswerExpression = { resolveAnswerExpression(it) },
-              draftAnswer = draftAnswerMap[questionnaireResponseItem]
-            )
+    val items = buildList {
+      // Add an item for the question itself
+      add(
+        QuestionnaireAdapterItem.Question(
+          QuestionnaireItemViewItem(
+            questionnaireItem,
+            questionnaireResponseItem,
+            validationResult = validationResult,
+            answersChangedCallback = answersChangedCallback,
+            resolveAnswerValueSet = { resolveAnswerValueSet(it) },
+            resolveAnswerExpression = { resolveAnswerExpression(it) },
+            draftAnswer = draftAnswerMap[questionnaireResponseItem]
           )
         )
-        val nestedResponses: List<List<QuestionnaireResponseItemComponent>> =
-          when {
-            // Repeated questions have one answer item per response instance, which we must display
-            // after the question.
-            questionnaireItem.repeats -> questionnaireResponseItem.answer.map { it.item }
-            // Non-repeated questions may have nested items, which we should display
-            else -> listOf(questionnaireResponseItem.item)
-          }
-        nestedResponses.forEach { nestedResponse ->
+      )
+
+      // Add nested questions after the parent item. We need to get the questionnaire items and
+      // (possibly multiple sets of) matching questionnaire response items and generate the adapter
+      // items. There are three different cases:
+      // 1. Questions nested under a non-repeated group: Simply take the nested question items and
+      // the nested question response items and "zip" them.
+      // 2. Questions nested under a question: In this case, the nested questions are repeated for
+      // each answer to the parent question. Therefore, we need to take the questions and lists of
+      // questionnaire response items nested under each answer and generate multiple sets of adapter
+      // items.
+      // 3. Questions nested under a repeated group: In the in-memory questionnaire response in the
+      // view model, we create dummy answers for each repeated group. As a result the processing of
+      // this case is similar to the case of questions nested under a question.
+      // For background, see https://build.fhir.org/questionnaireresponse.html#link.
+      buildList {
+          // Case 1
+          add(questionnaireResponseItem.item)
+          // Case 2 and 3
+          addAll(questionnaireResponseItem.answer.map { it.item })
+        }
+        .forEach { nestedResponseItemList ->
           addAll(
             getQuestionnaireAdapterItems(
               // If nested display item is identified as instructions or flyover, then do not create
@@ -664,11 +664,11 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
                   it.type == Questionnaire.QuestionnaireItemType.DISPLAY &&
                     (it.isInstructionsCode || it.isFlyoverCode || it.isHelpCode)
                 },
-              questionnaireResponseItemList = nestedResponse,
+              questionnaireResponseItemList = nestedResponseItemList,
             )
           )
         }
-      }
+    }
     currentPageItems = items
     return items
   }
@@ -701,7 +701,7 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
   }
 
   private fun getEnabledResponseItems(
-    questionnaireItemList: List<Questionnaire.QuestionnaireItemComponent>,
+    questionnaireItemList: List<QuestionnaireItemComponent>,
     questionnaireResponseItemList: List<QuestionnaireResponseItemComponent>,
   ): List<QuestionnaireResponseItemComponent> {
     val enablementEvaluator = EnablementEvaluator(questionnaireResponse)
@@ -749,7 +749,7 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
    * individual questions within this particular group instance).
    */
   private fun createRepeatedGroupResponse(
-    questionnaireItem: Questionnaire.QuestionnaireItemComponent,
+    questionnaireItem: QuestionnaireItemComponent,
     questionnaireResponseItem: QuestionnaireResponseItemComponent,
   ): List<QuestionnaireResponseItemComponent> {
     val individualQuestions = questionnaireItem.item
@@ -795,8 +795,7 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
     }
 }
 
-typealias ItemToParentMap =
-  MutableMap<Questionnaire.QuestionnaireItemComponent, Questionnaire.QuestionnaireItemComponent>
+typealias ItemToParentMap = MutableMap<QuestionnaireItemComponent, QuestionnaireItemComponent>
 
 /** Questionnaire state for the Fragment to consume. */
 internal data class QuestionnaireState(
@@ -833,3 +832,26 @@ 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/.../java/com/google/android/fhir/datacapture/validation/ValueConstraintExtensionValidator.kt b/.../java/com/google/android/fhir/datacapture/validation/ValueConstraintExtensionValidator.kt
@@ -33,7 +33,7 @@ internal open class ValueConstraintExtensionValidator(
     answers: List<QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent>,
     context: Context
   ): ConstraintValidator.ConstraintValidationResult {
-    if (questionnaireItem.hasExtension(url) && !answers.isEmpty()) {
+    if (questionnaireItem.hasExtension(url) && answers.isNotEmpty()) {
       val extension = questionnaireItem.getExtensionByUrl(url)
       // TODO(https://github.com/google/android-fhir/issues/487): Validates all answers.
       val answer = answers[0]