diff --git a/ground/src/main/java/com/google/android/ground/ui/datacollection/tasks/multiplechoice/MultipleChoiceTaskViewModel.kt b/ground/src/main/java/com/google/android/ground/ui/datacollection/tasks/multiplechoice/MultipleChoiceTaskViewModel.kt index bf2cc0ab2a..2d173c4558 100644 --- a/ground/src/main/java/com/google/android/ground/ui/datacollection/tasks/multiplechoice/MultipleChoiceTaskViewModel.kt +++ b/ground/src/main/java/com/google/android/ground/ui/datacollection/tasks/multiplechoice/MultipleChoiceTaskViewModel.kt @@ -46,15 +46,7 @@ class MultipleChoiceTaskViewModel @Inject constructor() : AbstractTaskViewModel( // Set the other option. _items.value .firstOrNull { it.isOtherOption } - ?.let { - val selected = - if (task.isRequired) { - otherText != "" - } else { - true - } - setItem(it, selected, it.cardinality == SELECT_MULTIPLE) - } + ?.let { setItem(item = it, selection = isOtherTextValid()) } updateResponse() } @@ -73,8 +65,8 @@ class MultipleChoiceTaskViewModel @Inject constructor() : AbstractTaskViewModel( updateMultipleChoiceItems() } - fun setItem(item: MultipleChoiceItem, selection: Boolean, canSelectMultiple: Boolean) { - if (!canSelectMultiple && selection) { + fun setItem(item: MultipleChoiceItem, selection: Boolean) { + if (item.cardinality != SELECT_MULTIPLE && selection) { selectedIds.clear() } if (selection) { @@ -86,12 +78,18 @@ class MultipleChoiceTaskViewModel @Inject constructor() : AbstractTaskViewModel( updateMultipleChoiceItems() } - fun toggleItem(item: MultipleChoiceItem, canSelectMultiple: Boolean) { + fun toggleItem(item: MultipleChoiceItem) { val wasSelected = selectedIds.contains(item.option.id) - setItem(item, !wasSelected, canSelectMultiple) + setItem(item, !wasSelected) } fun updateResponse() { + // Check if "other" text is missing or not. + if (selectedIds.contains(OTHER_ID) && !isOtherTextValid()) { + clearResponse() + return + } + setValue( fromList( task.multipleChoice, @@ -103,7 +101,19 @@ class MultipleChoiceTaskViewModel @Inject constructor() : AbstractTaskViewModel( ) } - /* Reads the list of options in a multiple choice object and converts them to MultipleChoiceItems*/ + /** Returns true if the value for "other" text is valid, otherwise false. */ + private fun isOtherTextValid(): Boolean { + if (!task.isRequired) { + // If task is not required, then the text value can be empty. Hence, it is always valid. + return true + } + + return otherText.isNotBlank() + } + + /** + * Reads the list of options in a multiple choice object and converts them to MultipleChoiceItems. + */ private fun updateMultipleChoiceItems() { val multipleChoice = checkNotNull(task.multipleChoice) val itemsFromOptions: MutableList = mutableListOf() @@ -130,7 +140,7 @@ class MultipleChoiceTaskViewModel @Inject constructor() : AbstractTaskViewModel( this._items.value = itemsFromOptions } - /* Reads the saved task value and adds selected items to the selected list*/ + /** Reads the saved task value and adds selected items to the selected list. */ private fun loadPendingSelections() { val selectedOptionIds = (taskTaskData.value as? MultipleChoiceTaskData)?.selectedOptionIds val multipleChoice = checkNotNull(task.multipleChoice) diff --git a/ground/src/main/res/layout/multiple_choice_checkbox_item.xml b/ground/src/main/res/layout/multiple_choice_checkbox_item.xml index 786e35c400..d20736c954 100644 --- a/ground/src/main/res/layout/multiple_choice_checkbox_item.xml +++ b/ground/src/main/res/layout/multiple_choice_checkbox_item.xml @@ -42,7 +42,7 @@ android:layout_width="match_parent" android:layout_height="wrap_content" android:checked="@{item.isSelected}" - android:onClickListener="@{() -> viewModel.toggleItem(item, true)}" + android:onClickListener="@{() -> viewModel.toggleItem(item)}" android:padding="16dp" android:text="@{ item.isOtherOption ? @string/other : item.option.label }" app:layout_constraintStart_toStartOf="parent" diff --git a/ground/src/main/res/layout/multiple_choice_radiobutton_item.xml b/ground/src/main/res/layout/multiple_choice_radiobutton_item.xml index 2f24b9a93f..43370390be 100644 --- a/ground/src/main/res/layout/multiple_choice_radiobutton_item.xml +++ b/ground/src/main/res/layout/multiple_choice_radiobutton_item.xml @@ -41,7 +41,7 @@ android:layout_height="wrap_content" android:buttonTint="?attr/colorPrimary" android:checked="@{item.isSelected}" - android:onClickListener="@{() -> viewModel.toggleItem(item, false)}" + android:onClickListener="@{() -> viewModel.toggleItem(item)}" android:padding="16dp" android:text="@{ item.isOtherOption ? @string/other : item.option.label }" app:layout_constraintStart_toStartOf="parent" diff --git a/ground/src/test/java/com/google/android/ground/ui/datacollection/tasks/multiplechoice/MultipleChoiceTaskFragmentTest.kt b/ground/src/test/java/com/google/android/ground/ui/datacollection/tasks/multiplechoice/MultipleChoiceTaskFragmentTest.kt index 9a82a42fd4..b3a9943606 100644 --- a/ground/src/test/java/com/google/android/ground/ui/datacollection/tasks/multiplechoice/MultipleChoiceTaskFragmentTest.kt +++ b/ground/src/test/java/com/google/android/ground/ui/datacollection/tasks/multiplechoice/MultipleChoiceTaskFragmentTest.kt @@ -147,29 +147,21 @@ class MultipleChoiceTaskFragmentTest : } @Test - fun `saves other text when selected`() = runWithTestDispatcher { + fun `saves other text`() = runWithTestDispatcher { val multipleChoice = MultipleChoice(options, MultipleChoice.Cardinality.SELECT_MULTIPLE, true) - setupTaskFragment(job, task.copy(multipleChoice = multipleChoice)) + setupTaskFragment( + job, + task.copy(multipleChoice = multipleChoice, isRequired = true), + ) val userInput = "User inputted text" onView(withText("Other")).perform(click()) onView(allOf(isDisplayed(), withId(R.id.user_response_text))) .perform(CustomViewActions.forceTypeText(userInput)) - hasValue(MultipleChoiceTaskData(multipleChoice, listOf("[ $userInput ]"))) - runner().assertButtonIsEnabled("Next") - } - - @Test - fun `selects other option on text input`() = runWithTestDispatcher { - val multipleChoice = MultipleChoice(options, MultipleChoice.Cardinality.SELECT_MULTIPLE, true) - setupTaskFragment(job, task.copy(multipleChoice = multipleChoice)) - val userInput = "A" - onView(withText("Other")).perform(click()) - onView(allOf(isDisplayed(), withId(R.id.user_response_text))) - .perform(CustomViewActions.forceTypeText(userInput)) onView(withText("Other")).check(matches(isChecked())) hasValue(MultipleChoiceTaskData(multipleChoice, listOf("[ $userInput ]"))) + runner().assertButtonIsEnabled("Next") } @Test @@ -315,4 +307,40 @@ class MultipleChoiceTaskFragmentTest : .assertButtonIsDisabled("Next") .assertButtonIsEnabled("Skip") } + + @Test + fun `doesn't save response when other text is missing and task is required`() = + runWithTestDispatcher { + val multipleChoice = MultipleChoice(options, MultipleChoice.Cardinality.SELECT_ONE, true) + setupTaskFragment( + job, + task.copy(multipleChoice = multipleChoice, isRequired = true), + ) + + onView(withText("Other")).perform(click()) + onView(allOf(isDisplayed(), withId(R.id.user_response_text))) + .perform(CustomViewActions.forceTypeText("")) + + hasValue(null) + runner().assertButtonIsDisabled("Next") + } + + @Test + fun `doesn't save response when multiple options selected but other text is missing and task is required`() = + runWithTestDispatcher { + val multipleChoice = MultipleChoice(options, MultipleChoice.Cardinality.SELECT_MULTIPLE, true) + setupTaskFragment( + job, + task.copy(multipleChoice = multipleChoice, isRequired = true), + ) + + onView(withText("Option 1")).perform(click()) + onView(withText("Option 2")).perform(click()) + onView(withText("Other")).perform(click()) + onView(allOf(isDisplayed(), withId(R.id.user_response_text))) + .perform(CustomViewActions.forceTypeText("")) + + hasValue(null) + runner().assertButtonIsDisabled("Next") + } }