Review comments: Refactored code and addded testcase to check cache r…

…estoration

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

@@ -582,21 +582,11 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
  .evaluate(questionnaireItem, questionnaireResponseItem)
  // Disabled questions should not get QuestionnaireItemViewItem instances
  if (!enabled) {
- // If the item is not enabled, clear the answers that it may have from the previous enabled
- // state. This will also prevent any questionnaire item that depends on the answer of this
- // questionnaire item to be wrongly evaluated as well.
- if (questionnaireResponseItem.hasAnswer()) {
- responseItemToAnswersMapForDisabledQuestionnaireItem[questionnaireResponseItem] =
- questionnaireResponseItem.answer
- questionnaireResponseItem.answer = listOf()
- }
+ cacheDisabledQuestionnaireItemAnswers(questionnaireResponseItem)
  return emptyList()
  }
 
- if (responseItemToAnswersMapForDisabledQuestionnaireItem.contains(questionnaireResponseItem)) {
- questionnaireResponseItem.answer =
- responseItemToAnswersMapForDisabledQuestionnaireItem.remove(questionnaireResponseItem)
- }
+ restoreFromDisabledQuestionnaireItemAnswersCache(questionnaireResponseItem)
 
  // Determine the validation result, which will be displayed on the item itself
  val validationResult =
@@ -654,6 +644,33 @@ internal class QuestionnaireViewModel(application: Application, state: SavedStat
  return items
  }
 
+ /**
+ * If the item is not enabled, clear the answers that it may have from the previous enabled state.
+ * This will also prevent any questionnaire item that depends on the answer of this questionnaire
+ * item to be wrongly evaluated as well.
+ */
+ private fun cacheDisabledQuestionnaireItemAnswers(
+ questionnaireResponseItem: QuestionnaireResponseItemComponent
+ ) {
+ if (questionnaireResponseItem.hasAnswer()) {
+ responseItemToAnswersMapForDisabledQuestionnaireItem[questionnaireResponseItem] =
+ questionnaireResponseItem.answer
+ questionnaireResponseItem.answer = listOf()
+ }
+ }
+
+ /**
+ * If the questionnaire item was previously disabled, check the cache to restore previous answers.
+ */
+ private fun restoreFromDisabledQuestionnaireItemAnswersCache(
+ questionnaireResponseItem: QuestionnaireResponseItemComponent
+ ) {
+ if (responseItemToAnswersMapForDisabledQuestionnaireItem.contains(questionnaireResponseItem)) {
+ questionnaireResponseItem.answer =
+ responseItemToAnswersMapForDisabledQuestionnaireItem.remove(questionnaireResponseItem)
+ }
+ }
+
  private fun getEnabledResponseItems(
  questionnaireItemList: List<Questionnaire.QuestionnaireItemComponent>,
  questionnaireResponseItemList: List<QuestionnaireResponseItemComponent>,

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

@@ -1113,94 +1113,206 @@ class QuestionnaireViewModelTest {
  }
 
  @Test
- fun `should disable all questions in a chain of dependent questions after top question is disabled`() =
- runBlocking {
- val questionnaire =
- Questionnaire().apply {
- id = "a-questionnaire"
- addItem(
- Questionnaire.QuestionnaireItemComponent().apply {
- linkId = "question-1"
- type = Questionnaire.QuestionnaireItemType.BOOLEAN
+ fun `should disable all questions in a chain of dependent questions after top question is disabled`() {
+ val questionnaire =
+ Questionnaire().apply {
+ id = "a-questionnaire"
+ addItem(
+ Questionnaire.QuestionnaireItemComponent().apply {
+ linkId = "question-1"
+ type = Questionnaire.QuestionnaireItemType.BOOLEAN
+ }
+ )
+ addItem(
+ Questionnaire.QuestionnaireItemComponent().apply {
+ linkId = "question-2"
+ type = Questionnaire.QuestionnaireItemType.BOOLEAN
+ addEnableWhen().apply {
+ answer = BooleanType(true)
+ question = "question-1"
+ operator = Questionnaire.QuestionnaireItemOperator.EQUAL
  }
- )
-  addItem(
-  Questionnaire.QuestionnaireItemComponent().apply {
-  linkId = "question-2"
-  type = Questionnaire.QuestionnaireItemType.BOOLEAN
-  addEnableWhen().apply {
-  answer = BooleanType(true)
-  question = "question-1"
-  operator = Questionnaire.QuestionnaireItemOperator.EQUAL
- }
+ }
+ )
+ addItem(
+ Questionnaire.QuestionnaireItemComponent().apply {
+ linkId = "question-3"
+ type = Questionnaire.QuestionnaireItemType.BOOLEAN
+ addEnableWhen().apply {
+ answer = BooleanType(true)
+ question = "question-2"
+ operator = Questionnaire.QuestionnaireItemOperator.EQUAL
  }
- )
- addItem(
- Questionnaire.QuestionnaireItemComponent().apply {
- linkId = "question-3"
- type = Questionnaire.QuestionnaireItemType.BOOLEAN
- addEnableWhen().apply {
- answer = BooleanType(true)
- question = "question-2"
- operator = Questionnaire.QuestionnaireItemOperator.EQUAL
+ }
+ )
+ }
+
+ val questionnaireResponse =
+ QuestionnaireResponse().apply {
+ id = "a-questionnaire-response"
+ addItem(
+ QuestionnaireResponse.QuestionnaireResponseItemComponent().apply {
+ linkId = "question-1"
+ addAnswer(
+ QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent().apply {
+ value = BooleanType(true)
+ }
+ )
+ }
+ )
+ addItem(
+ QuestionnaireResponse.QuestionnaireResponseItemComponent().apply {
+ linkId = "question-2"
+ addAnswer(
+ QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent().apply {
+ value = BooleanType(true)
+ }
+ )
+ }
+ )
+ addItem(
+ QuestionnaireResponse.QuestionnaireResponseItemComponent().apply {
+ linkId = "question-3"
+ addAnswer(
+ QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent().apply {
+ value = BooleanType(true)
  }
+ )
+ }
+ )
+ }
+
+ state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, printer.encodeResourceToString(questionnaire))
+ state.set(
+ EXTRA_QUESTIONNAIRE_RESPONSE_JSON_STRING,
+ printer.encodeResourceToString(questionnaireResponse)
+ )
+
+ val viewModel = QuestionnaireViewModel(context, state)
+ viewModel.runViewModelBlocking {
+ var items = viewModel.getQuestionnaireItemViewItemList().map { it.asQuestion() }
+ assertThat(items.map { it.questionnaireItem.linkId })
+ .containsExactly("question-1", "question-2", "question-3")
+
+ items.first { it.questionnaireItem.linkId == "question-1" }.clearAnswer()
+
+ items = viewModel.getQuestionnaireItemViewItemList().map { it.asQuestion() }
+ assertThat(items.map { it.questionnaireItem.linkId }).containsExactly("question-1")
+ }
+ }
+
+ @Test
+ fun `should restore previous state in a chain of dependent question items when item is disabled and enabled`() {
+ val questionnaire =
+ Questionnaire().apply {
+ id = "a-questionnaire"
+ addItem(
+ Questionnaire.QuestionnaireItemComponent().apply {
+ linkId = "question-1"
+ type = Questionnaire.QuestionnaireItemType.BOOLEAN
+ }
+ )
+ addItem(
+ Questionnaire.QuestionnaireItemComponent().apply {
+ linkId = "question-2"
+ type = Questionnaire.QuestionnaireItemType.BOOLEAN
+ addEnableWhen().apply {
+ answer = BooleanType(true)
+ question = "question-1"
+ operator = Questionnaire.QuestionnaireItemOperator.EQUAL
  }
- )
- }
+ }
+ )
+ addItem(
+ Questionnaire.QuestionnaireItemComponent().apply {
+ linkId = "question-3"
+ type = Questionnaire.QuestionnaireItemType.BOOLEAN
+ addEnableWhen().apply {
+ answer = BooleanType(true)
+ question = "question-2"
+ operator = Questionnaire.QuestionnaireItemOperator.EQUAL
+ }
+ }
+ )
+ }
 
- val questionnaireResponse =
+ val questionnaireResponse =
+ QuestionnaireResponse().apply {
+ id = "a-questionnaire-response"
+ addItem(
+ QuestionnaireResponse.QuestionnaireResponseItemComponent().apply {
+ linkId = "question-1"
+ addAnswer(
+ QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent().apply {
+ value = BooleanType(true)
+ }
+ )
+ }
+ )
+ addItem(
+ QuestionnaireResponse.QuestionnaireResponseItemComponent().apply {
+ linkId = "question-2"
+ addAnswer(
+ QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent().apply {
+ value = BooleanType(true)
+ }
+ )
+ }
+ )
+ addItem(
+ QuestionnaireResponse.QuestionnaireResponseItemComponent().apply {
+ linkId = "question-3"
+ addAnswer(
+ QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent().apply {
+ value = BooleanType(true)
+ }
+ )
+ }
+ )
+ }
+
+ state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, printer.encodeResourceToString(questionnaire))
+ state.set(
+ EXTRA_QUESTIONNAIRE_RESPONSE_JSON_STRING,
+ printer.encodeResourceToString(questionnaireResponse)
+ )
+
+ val viewModel = QuestionnaireViewModel(context, state)
+ viewModel.runViewModelBlocking {
+ val items = viewModel.getQuestionnaireItemViewItemList().map { it.asQuestion() }
+ // Clearing the answer disables question-2 that in turn disables question-3.
+ items.first { it.questionnaireItem.linkId == "question-1" }.clearAnswer()
+
+ assertResourceEquals(
+ viewModel.getQuestionnaireResponse(),
  QuestionnaireResponse().apply {
  id = "a-questionnaire-response"
  addItem(
  QuestionnaireResponse.QuestionnaireResponseItemComponent().apply {
  linkId = "question-1"
  addAnswer(
  QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent().apply {
- value = BooleanType(true)
- }
- )
- }
- )
- addItem(
- QuestionnaireResponse.QuestionnaireResponseItemComponent().apply {
- linkId = "question-2"
- addAnswer(
- QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent().apply {
- value = BooleanType(true)
- }
- )
- }
- )
- addItem(
- QuestionnaireResponse.QuestionnaireResponseItemComponent().apply {
- linkId = "question-3"
- addAnswer(
- QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent().apply {
- value = BooleanType(true)
+ value = BooleanType(false)
  }
  )
  }
  )
  }
-
- state.set(EXTRA_QUESTIONNAIRE_JSON_STRING, printer.encodeResourceToString(questionnaire))
- state.set(
- EXTRA_QUESTIONNAIRE_RESPONSE_JSON_STRING,
- printer.encodeResourceToString(questionnaireResponse)
  )
 
- val viewModel = QuestionnaireViewModel(context, state)
- viewModel.runViewModelBlocking {
- var items = viewModel.getQuestionnaireItemViewItemList().map { it.asQuestion() }
- assertThat(items.map { it.questionnaireItem.linkId })
- .containsExactly("question-1", "question-2", "question-3")
-
- items.first { it.questionnaireItem.linkId == "question-1" }.clearAnswer()
+ // Setting the answer of "question-1" to true should enable question-2 that in turn enables
+ // question-3 and restore their previous states.
+ items
+ .first { it.questionnaireItem.linkId == "question-1" }
+ .addAnswer(
+ QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent().apply {
+ value = BooleanType(true)
+ }
+ )
 
- items = viewModel.getQuestionnaireItemViewItemList().map { it.asQuestion() }
- assertThat(items.map { it.questionnaireItem.linkId }).containsExactly("question-1")
- }
+ assertResourceEquals(viewModel.getQuestionnaireResponse(), questionnaireResponse)
  }
+ }
 
  // Test cases for state flow