Change areItemsTheSame to check if internal objects are the same with…

… === (google#1185) * Call onAnswerChanged Callback instead of questionnaireResponseCHangeCallback * Remove unnecessary questionnaireResponseItemChangedCallback since onAnswerChanged calls this internally * questionnaireResponseItem has to be a variable since we clear answers which may contain nested questionnaireReponseItems * Reverted change from val to var and added override for equals function * Changed areItemsTheSame to check internal objects * Added UTs to check the overridden equals function for questionnaireItemViewItem * Update comments Co-authored-by: Jing Tang <[email protected]> * Add new equalsDeep function for questionnaireItemViewItem * Early return and feedback in code comments Co-authored-by: Jing Tang <[email protected]>
aurangzaibumer · Feb 24, 2022 · f31f5b7 · f31f5b7
1 parent e6bb4c4
commit f31f5b7
Show file tree

Hide file tree

Showing 5 changed files with 73 additions and 9 deletions.
diff --git a/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireItemAdapter.kt b/datacapture/src/main/java/com/google/android/fhir/datacapture/QuestionnaireItemAdapter.kt
@@ -195,12 +195,10 @@ internal object DiffCallback : DiffUtil.ItemCallback<QuestionnaireItemViewItem>(
  override fun areItemsTheSame(
  oldItem: QuestionnaireItemViewItem,
  newItem: QuestionnaireItemViewItem
- ) = oldItem.questionnaireItem.linkId == newItem.questionnaireItem.linkId
+ ) = oldItem == newItem
 
  override fun areContentsTheSame(
  oldItem: QuestionnaireItemViewItem,
  newItem: QuestionnaireItemViewItem
- ) =
- oldItem.questionnaireItem.equalsDeep(newItem.questionnaireItem) &&
- oldItem.questionnaireResponseItem.equalsDeep(newItem.questionnaireResponseItem)
+ ) = oldItem.equalsDeep(newItem)
 }
diff --git a/...fhir/datacapture/contrib/views/barcode/QuestionnaireItemBarCodeReaderViewHolderFactory.kt b/...fhir/datacapture/contrib/views/barcode/QuestionnaireItemBarCodeReaderViewHolderFactory.kt
@@ -76,8 +76,7 @@ object QuestionnaireItemBarCodeReaderViewHolderFactory :
  }
 
  setInitial(questionnaireItemViewItem.singleAnswerOrNull, reScanView)
-
- questionnaireItemViewItem.questionnaireResponseItemChangedCallback()
+ onAnswerChanged(context)
  }
  }
  )

diff --git a/...a/com/google/android/fhir/datacapture/views/QuestionnaireItemDropDownViewHolderFactory.kt b/...a/com/google/android/fhir/datacapture/views/QuestionnaireItemDropDownViewHolderFactory.kt
@@ -71,7 +71,6 @@ internal object QuestionnaireItemDropDownViewHolderFactory :
  questionnaireItemViewItem.singleAnswerOrNull =
  QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent()
  .setValue(questionnaireItemViewItem.answerOption[position].valueCoding)
- questionnaireItemViewItem.questionnaireResponseItemChangedCallback()
  onAnswerChanged(autoCompleteTextView.context)
  }
  }

diff --git a/...ture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemViewItem.kt b/...ture/src/main/java/com/google/android/fhir/datacapture/views/QuestionnaireItemViewItem.kt
@@ -107,4 +107,40 @@ data class QuestionnaireItemViewItem(
  else -> emptyList()
  }
  }
+
+ /**
+ * [QuestionnaireItemViewItem] is a transient object for the UI only. Whenever the user makes any
+ * change via the UI, a new list of [QuestionnaireItemViewItem]s will be created, each holding
+ * references to the underlying [QuestionnaireItem] and [QuestionnaireResponseItem]. To avoid
+ * refreshing the UI unnecessarily with the same [QuestionnaireItem]s and
+ * [QuestionnaireResponseItem]s, we consider two [QuestionnaireItemViewItem]s to be the same if
+ * they have the same underlying [QuestionnaireItem] and [QuestionnaireResponseItem]. See
+ * [QuestionnaireItemAdapter.DiffCallback].
+ *
+ * On the other hand, under certain circumstances, the underlying [QuestionnaireResponseItem]
+ * might be recreated for the same question. For example, if a [QuestionnaireItem] is nested under
+ * another [QuestionnaireItem], the [QuestionnaireResponseItem](s) will be nested under the parent
+ * [QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent], and if the
+ * [QuestionnaireResponse.QuestionnaireResponseItemAnswerComponent] is changed, the nested
+ * [QuestionnaireResponseItem] will be recreated, too. In such cases, it would be incorrect to
+ * simply check that the `linkId` of the underlying [QuestionnaireItem] and
+ * [QuestionnaireResponseItem] match.
+ */
+ override fun equals(other: Any?): Boolean {
+ if (other !is QuestionnaireItemViewItem) return false
+ return this.questionnaireItem === other.questionnaireItem &&
+ this.questionnaireResponseItem === other.questionnaireResponseItem
+ }
+
+ /**
+ * Comparing the contents of two [QuestionnaireItemViewItem]s by traversing the underlying
+ * [Questionnaire.QuestionnaireItemComponent] and
+ * [QuestionnaireResponse.QuestionnaireResponseItemComponent] and comparing values of all the
+ * properties. This is done by using the [Questionnaire.QuestionnaireItemComponent.equalsDeep] and
+ * [QuestionnaireResponse.QuestionnaireResponseItemComponent.equalsDeep].
+ */
+ fun equalsDeep(other: QuestionnaireItemViewItem): Boolean {
+ return this.questionnaireItem.equalsDeep(other.questionnaireItem) &&
+ this.questionnaireResponseItem.equalsDeep(other.questionnaireResponseItem)
+ }
 }
diff --git a/...capture/src/test/java/com/google/android/fhir/datacapture/QuestionnaireItemAdapterTest.kt b/...capture/src/test/java/com/google/android/fhir/datacapture/QuestionnaireItemAdapterTest.kt
@@ -347,7 +347,7 @@ class QuestionnaireItemAdapterTest {
  // TODO: test errors thrown for unsupported types
 
  @Test
- fun diffCallback_areItemsTheSame_sameLinkId_shouldReturnTrue() {
+ fun diffCallback_areItemsTheSame_sameLinkIdDifferentObjectId_shouldReturnFalse() {
  assertThat(
  DiffCallback.areItemsTheSame(
  QuestionnaireItemViewItem(
@@ -360,19 +360,51 @@ class QuestionnaireItemAdapterTest {
  ) {}
  )
  )
+ .isFalse()
+ }
+
+ @Test
+ fun diffCallback_areItemsTheSame_sameLinkIdSameObjectId_shouldReturnTrue() {
+ val questionnaireItem =
+ Questionnaire.QuestionnaireItemComponent().setLinkId("link-id-1").setText("text")
+ val questionnaireResponseItem = QuestionnaireResponse.QuestionnaireResponseItemComponent()
+ assertThat(
+ DiffCallback.areItemsTheSame(
+ QuestionnaireItemViewItem(questionnaireItem, questionnaireResponseItem) {},
+ QuestionnaireItemViewItem(questionnaireItem, questionnaireResponseItem) {}
+ )
+ )
  .isTrue()
  }
 
  @Test
  fun diffCallback_areItemsTheSame_differentLinkId_shouldReturnFalse() {
+ val questionnaireResponseItem = QuestionnaireResponse.QuestionnaireResponseItemComponent()
  assertThat(
  DiffCallback.areItemsTheSame(
  QuestionnaireItemViewItem(
  Questionnaire.QuestionnaireItemComponent().setLinkId("link-id-1"),
- QuestionnaireResponse.QuestionnaireResponseItemComponent()
+ questionnaireResponseItem
  ) {},
  QuestionnaireItemViewItem(
  Questionnaire.QuestionnaireItemComponent().setLinkId("link-id-2"),
+ questionnaireResponseItem
+ ) {}
+ )
+ )
+ .isFalse()
+ }
+
+ @Test
+ fun diffCallback_areItemsTheSame_differentQuestionnaireResponseItem_shouldReturnFalse() {
+ val questionnaireItem =
+ Questionnaire.QuestionnaireItemComponent().setLinkId("link-id-1").setText("text")
+ val questionnaireResponseItem = QuestionnaireResponse.QuestionnaireResponseItemComponent()
+ assertThat(
+ DiffCallback.areItemsTheSame(
+ QuestionnaireItemViewItem(questionnaireItem, questionnaireResponseItem) {},
+ QuestionnaireItemViewItem(
+ questionnaireItem,
  QuestionnaireResponse.QuestionnaireResponseItemComponent()
  ) {}
  )