Only copy nested items to childless answers (#2600)

* Only copy nested items to childless answers

* Correct broken test case

* Fix bug in questionnaire edit adapter

* Suppress lint error

datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireEditAdapter.kt

@@ -16,6 +16,7 @@
 
 package com.google.android.fhir.datacapture
 
+import android.annotation.SuppressLint
 import android.view.View
 import android.view.ViewGroup
 import androidx.recyclerview.widget.DiffUtil
@@ -328,8 +329,28 @@ internal object DiffCallbacks {
               QUESTIONS.areContentsTheSame(oldItem, newItem)
           }
           is QuestionnaireAdapterItem.RepeatedGroupHeader -> {
-            newItem is QuestionnaireAdapterItem.RepeatedGroupHeader &&
-              oldItem.responses == newItem.responses
+            if (newItem is QuestionnaireAdapterItem.RepeatedGroupHeader) {
+              // The `onDeleteClicked` function is a function closure generated in the questionnaire
+              // viewmodel with a reference to the parent questionnaire view item. When it is
+              // invoked, it deletes the current repeated group instance from the parent
+              // questionnaire view item by removing it from the list of children in the parent
+              // questionnaire view.
+              // In other words, although the `onDeleteClicked` function is not a data field, it is
+              // a function closure with references to data structures. Because
+              // `RepeatedGroupHeader` does not include any other data fields besides the index, it
+              // is particularly important to distinguish between different `RepeatedGroupHeader`s
+              // by the `onDeleteClicked` function.
+              // If this check is not here, an old RepeatedGroupHeader might be mistakenly
+              // considered up-to-date and retained in the recycler view even though a newer
+              // version includes a different `onDeleteClicked` function referencing a parent item
+              // with a different list of children. As a result clicking the delete function might
+              // result in deleting from an old list.
+              @SuppressLint("DiffUtilEquals")
+              val onDeleteClickedCallbacksEqual = oldItem.onDeleteClicked == newItem.onDeleteClicked
+              onDeleteClickedCallbacksEqual
+            } else {
+              false
+            }
           }
           is QuestionnaireAdapterItem.Navigation -> {
             newItem is QuestionnaireAdapterItem.Navigation &&

datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireViewModel.kt

@@ -28,8 +28,8 @@ import ca.uhn.fhir.parser.IParser
 import com.google.android.fhir.datacapture.enablement.EnablementEvaluator
 import com.google.android.fhir.datacapture.expressions.EnabledAnswerOptionsEvaluator
 import com.google.android.fhir.datacapture.extensions.EntryMode
-import com.google.android.fhir.datacapture.extensions.addNestedItemsToAnswer
 import com.google.android.fhir.datacapture.extensions.allItems
+import com.google.android.fhir.datacapture.extensions.copyNestedItemsToChildlessAnswers
 import com.google.android.fhir.datacapture.extensions.cqfExpression
 import com.google.android.fhir.datacapture.extensions.createQuestionnaireResponseItem
 import com.google.android.fhir.datacapture.extensions.entryMode
@@ -361,7 +361,7 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
         }
       }
       if (questionnaireItem.shouldHaveNestedItemsUnderAnswers) {
-        questionnaireResponseItem.addNestedItemsToAnswer(questionnaireItem)
+        questionnaireResponseItem.copyNestedItemsToChildlessAnswers(questionnaireItem)
 
         // If nested items are added to the answer, the enablement evaluator needs to be
         // reinitialized in order for it to rebuild the pre-order map and parent map of

datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireItemComponents.kt

@@ -887,29 +887,52 @@ internal val QuestionnaireItemComponent.shouldHaveNestedItemsUnderAnswers: Boole
   get() = item.isNotEmpty() && (type != Questionnaire.QuestionnaireItemType.GROUP || repeats)
 
 /**
- * Creates a list of [QuestionnaireResponse.QuestionnaireResponseItemComponent]s from the nested
- * items in the [Questionnaire.QuestionnaireItemComponent].
+ * Creates a list of [QuestionnaireResponse.QuestionnaireResponseItemComponent]s corresponding to
+ * the nested items under the questionnaire item.
+ *
+ * The list can be added as nested items under answers in a corresponding questionnaire response
+ * item. This may be because
+ * 1. the questionnaire item is a question with nested questions, in which case each answer in the
+ *    questionnaire response item needs to have the same nested questions, or
+ * 2. the questionnaire item is a repeated group, in which case each answer in the questionnaire
+ *    response item represents an instance of the repeated group, and needs to have the same nested
+ *    questions.
  *
  * 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 QuestionnaireItemComponent.getNestedQuestionnaireResponseItems() =
+internal fun QuestionnaireItemComponent.createNestedQuestionnaireResponseItems() =
   item.map { it.createQuestionnaireResponseItem() }
 
 /**
- * Creates a [QuestionnaireResponse.QuestionnaireResponseItemComponent] from the provided
- * [Questionnaire.QuestionnaireItemComponent].
+ * Creates a corresponding [QuestionnaireResponse.QuestionnaireResponseItemComponent] for the
+ * questionnaire item with the following properties:
+ * - same `linkId` as the questionnaire item,
+ * - any initial answer(s) specified either in the `initial` element or as `initialSelected`
+ *   `answerOption`(s),
+ * - any nested questions under the initial answers (there will be no user input yet since this is
+ *   just being created) if this is a question with nested questions, and
+ * - any nested questions if this is a non-repeated group.
+ *
+ * Note that although initial answers to a repeated group may be interpreted as initial instances of
+ * the repeated group in the in-memory representation of questionnaire response, they are not
+ * defined as such in the standard. As a result, we are not treating them as such in this function
+ * to be conformant.
  *
  * 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 QuestionnaireItemComponent.createQuestionnaireResponseItem():
+internal fun QuestionnaireItemComponent.createQuestionnaireResponseItem():
   QuestionnaireResponse.QuestionnaireResponseItemComponent {
   return QuestionnaireResponse.QuestionnaireResponseItemComponent().apply {
     linkId = this@createQuestionnaireResponseItem.linkId
     answer = createQuestionnaireResponseItemAnswers()
-    if (shouldHaveNestedItemsUnderAnswers && answer.isNotEmpty()) {
-      this.addNestedItemsToAnswer(this@createQuestionnaireResponseItem)
+    if (
+      type != Questionnaire.QuestionnaireItemType.GROUP &&
+        this@createQuestionnaireResponseItem.item.isNotEmpty() &&
+        answer.isNotEmpty()
+    ) {
+      this.copyNestedItemsToChildlessAnswers(this@createQuestionnaireResponseItem)
     } else if (
       this@createQuestionnaireResponseItem.type == Questionnaire.QuestionnaireItemType.GROUP &&
         !repeats

datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponseItemComponents.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright 2023 Google LLC
+ * Copyright 2023-2024 Google LLC
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -38,14 +38,24 @@ private fun QuestionnaireResponse.QuestionnaireResponseItemComponent.appendDesce
 }
 
 /**
- * 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.
+ * Copies nested items under `questionnaireItem` to each answer without children. The hierarchy and
+ * order of nested items will be retained as specified in the standard.
+ *
+ * Existing answers with nested items will not be modified because the nested items may contain
+ * answers already.
+ *
+ * This should be used when
+ * - a new answer is added to a question with nested questions, or
+ * - a new answer is added to a repeated group (in which case this indicates a new instance of the
+ *   repeated group will be added to the final questionnaire response).
  *
  * See https://www.hl7.org/fhir/questionnaireresponse.html#notes for more details.
  */
-fun QuestionnaireResponse.QuestionnaireResponseItemComponent.addNestedItemsToAnswer(
+internal fun QuestionnaireResponse.QuestionnaireResponseItemComponent
+  .copyNestedItemsToChildlessAnswers(
   questionnaireItem: Questionnaire.QuestionnaireItemComponent,
 ) {
-  answer.forEach { it.item = questionnaireItem.getNestedQuestionnaireResponseItems() }
+  answer
+    .filter { it.item.isEmpty() }
+    .forEach { it.item = questionnaireItem.createNestedQuestionnaireResponseItems() }
 }

datacapture/src/main/java/com/google/android/fhir/datacapture/views/factories/GroupViewHolderFactory.kt

@@ -22,7 +22,6 @@ import android.widget.TextView
 import androidx.appcompat.app.AppCompatActivity
 import androidx.lifecycle.lifecycleScope
 import com.google.android.fhir.datacapture.R
-import com.google.android.fhir.datacapture.extensions.getNestedQuestionnaireResponseItems
 import com.google.android.fhir.datacapture.extensions.tryUnwrapContext
 import com.google.android.fhir.datacapture.validation.Invalid
 import com.google.android.fhir.datacapture.validation.NotValidated
@@ -62,11 +61,8 @@ internal object GroupViewHolderFactory :
         addItemButton.setOnClickListener {
           context.lifecycleScope.launch {
             questionnaireViewItem.addAnswer(
-              QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent().apply {
-                // TODO(jingtang10): This can be removed since we already do this in the
-                // answerChangedCallback in the QuestionnaireViewModel.
-                item = questionnaireViewItem.questionnaireItem.getNestedQuestionnaireResponseItems()
-              },
+              // Nested items will be added in answerChangedCallback in the QuestionnaireViewModel
+              QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent(),
             )
           }
         }

datacapture/src/test/java/com/google/android/fhir/datacapture/QuestionnaireViewModelTest.kt

@@ -52,8 +52,8 @@ import com.google.android.fhir.datacapture.extensions.EXTENSION_SDC_QUESTIONNAIR
 import com.google.android.fhir.datacapture.extensions.EXTENSION_VARIABLE_URL
 import com.google.android.fhir.datacapture.extensions.EntryMode
 import com.google.android.fhir.datacapture.extensions.asStringValue
+import com.google.android.fhir.datacapture.extensions.createNestedQuestionnaireResponseItems
 import com.google.android.fhir.datacapture.extensions.entryMode
-import com.google.android.fhir.datacapture.extensions.getNestedQuestionnaireResponseItems
 import com.google.android.fhir.datacapture.extensions.logicalId
 import com.google.android.fhir.datacapture.extensions.maxValue
 import com.google.android.fhir.datacapture.testing.DataCaptureTestApplication
@@ -4332,7 +4332,7 @@ class QuestionnaireViewModelTest {
                 getQuestionnaireAdapterItemListA()
                   .asQuestion()
                   .questionnaireItem
-                  .getNestedQuestionnaireResponseItems()
+                  .createNestedQuestionnaireResponseItems()
             },
           )
         getQuestionnaireAdapterItemListB()
@@ -4343,7 +4343,7 @@ class QuestionnaireViewModelTest {
                 getQuestionnaireAdapterItemListB()
                   .asQuestion()
                   .questionnaireItem
-                  .getNestedQuestionnaireResponseItems()
+                  .createNestedQuestionnaireResponseItems()
             },
           )
       }

datacapture/src/test/java/com/google/android/fhir/datacapture/extensions/MoreQuestionnaireResponseItemComponentsTest.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright 2022-2023 Google LLC
+ * Copyright 2022-2024 Google LLC
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,6 +17,10 @@
 package com.google.android.fhir.datacapture.extensions
 
 import com.google.common.truth.Truth.assertThat
+import org.hl7.fhir.r4.model.DecimalType
+import org.hl7.fhir.r4.model.IntegerType
+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.QuestionnaireResponseItemComponent
 import org.junit.Test
@@ -67,4 +71,100 @@ class MoreQuestionnaireResponseItemComponentsTest {
         questionnaireResponseItem22,
       )
   }
+
+  @Test
+  fun `should copy nested item to childless answer`() {
+    val questionnaireItem =
+      QuestionnaireItemComponent().apply {
+        linkId = "question"
+        type = Questionnaire.QuestionnaireItemType.GROUP
+        repeats = true
+        addItem(
+          QuestionnaireItemComponent().apply {
+            linkId = "nested-question-1"
+            type = Questionnaire.QuestionnaireItemType.INTEGER
+          },
+        )
+        addItem(
+          QuestionnaireItemComponent().apply {
+            linkId = "nested-question-2"
+            type = Questionnaire.QuestionnaireItemType.DECIMAL
+          },
+        )
+      }
+    val questionnaireResponseItem =
+      QuestionnaireResponseItemComponent().apply {
+        linkId = "question"
+        addAnswer(QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent())
+      }
+
+    questionnaireResponseItem.copyNestedItemsToChildlessAnswers(questionnaireItem)
+
+    val nestedItems = questionnaireResponseItem.answer.single().item
+    assertThat(nestedItems).hasSize(2)
+    assertThat(nestedItems.map { it.linkId })
+      .containsExactly("nested-question-1", "nested-question-2")
+    assertThat(nestedItems.first().answer).isEmpty()
+    assertThat(nestedItems.last().answer).isEmpty()
+  }
+
+  @Test
+  fun `should not copy nested item to existing answer with children`() {
+    val questionnaireItem =
+      QuestionnaireItemComponent().apply {
+        linkId = "question"
+        type = Questionnaire.QuestionnaireItemType.GROUP
+        repeats = true
+        addItem(
+          QuestionnaireItemComponent().apply {
+            linkId = "nested-question-1"
+            type = Questionnaire.QuestionnaireItemType.INTEGER
+          },
+        )
+        addItem(
+          QuestionnaireItemComponent().apply {
+            linkId = "nested-question-2"
+            type = Questionnaire.QuestionnaireItemType.DECIMAL
+          },
+        )
+      }
+    val questionnaireResponseItem =
+      QuestionnaireResponseItemComponent().apply {
+        linkId = "question"
+        addAnswer(
+          QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent().apply {
+            addItem(
+              QuestionnaireResponseItemComponent().apply {
+                linkId = "nested-question-1"
+                addAnswer(
+                  QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent().apply {
+                    value = IntegerType(1)
+                  },
+                )
+              },
+            )
+            addItem(
+              QuestionnaireResponseItemComponent().apply {
+                linkId = "nested-question-2"
+                addAnswer(
+                  QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent().apply {
+                    value = DecimalType(1.0)
+                  },
+                )
+              },
+            )
+          },
+        )
+      }
+
+    questionnaireResponseItem.copyNestedItemsToChildlessAnswers(questionnaireItem)
+
+    val nestedItems = questionnaireResponseItem.answer.single().item
+    assertThat(nestedItems).hasSize(2)
+    assertThat(nestedItems.map { it.linkId })
+      .containsExactly("nested-question-1", "nested-question-2")
+    assertThat(nestedItems.first().answer.single().valueIntegerType.value).isEqualTo(1)
+    assertThat(nestedItems.last().answer.single().valueDecimalType.value.toString())
+      .isEqualTo("1.0")
+  }
 }