Fix value for multiple choice tasks with "other" option selected (#2621)

ground/src/main/java/com/google/android/ground/model/submission/MultipleChoiceTaskData.kt

@@ -17,6 +17,7 @@
 package com.google.android.ground.model.submission
 
 import com.google.android.ground.model.task.MultipleChoice
+import com.google.android.ground.ui.datacollection.tasks.multiplechoice.MultipleChoiceTaskViewModel
 import kotlinx.serialization.Serializable
 
 /** User responses to a select-one (radio) or select-multiple (checkbox) field. */
@@ -26,13 +27,18 @@ class MultipleChoiceTaskData(
   val selectedOptionIds: List<String>,
 ) : TaskData {
 
-  // TODO: Reuse the key value here and in the view model
-  fun hasOtherText(): Boolean = selectedOptionIds.contains("OTHER_ID")
+  fun getSelectedOptionsIdsExceptOther(): List<String> =
+    selectedOptionIds.filterNot { it.isOtherText() }
 
-  fun getOtherText(): String {
-    val otherId = selectedOptionIds.firstOrNull { it == "OTHER_ID" } ?: return ""
-    return multipleChoice?.getOptionById(otherId)?.label ?: ""
-  }
+  fun hasOtherText(): Boolean = selectedOptionIds.any { it.isOtherText() }
+
+  fun getOtherText(): String =
+    selectedOptionIds
+      .firstOrNull { it.isOtherText() }
+      ?.removeSurrounding(
+        prefix = MultipleChoiceTaskViewModel.OTHER_PREFIX,
+        suffix = MultipleChoiceTaskViewModel.OTHER_SUFFIX,
+      ) ?: ""
 
   // TODO: Make these inner classes non-static and access Task directly.
   override fun getDetailsText(): String =
@@ -55,6 +61,10 @@ class MultipleChoiceTaskData(
 
   override fun toString(): String = selectedOptionIds.sorted().joinToString()
 
+  private fun String.isOtherText(): Boolean =
+    startsWith(MultipleChoiceTaskViewModel.OTHER_PREFIX) &&
+      endsWith(MultipleChoiceTaskViewModel.OTHER_SUFFIX)
+
   companion object {
     fun fromList(multipleChoice: MultipleChoice?, ids: List<String>): TaskData? =
       if (ids.isEmpty()) {

ground/src/main/java/com/google/android/ground/persistence/remote/firebase/protobuf/ModelToProtoExt.kt

@@ -148,7 +148,7 @@ private fun ValueDelta.toMessage() = taskData {
         dateTime = timestamp { seconds = (newTaskData as TimeTaskData).time.time / 1000 }
       }
     Task.Type.MULTIPLE_CHOICE -> multipleChoiceResponses = multipleChoiceResponses {
-        (newTaskData as MultipleChoiceTaskData).selectedOptionIds.forEach {
+        (newTaskData as MultipleChoiceTaskData).getSelectedOptionsIdsExceptOther().forEach {
           selectedOptionIds.add(it)
         }
         if (newTaskData.hasOtherText()) {

ground/src/main/java/com/google/android/ground/ui/datacollection/DataCollectionViewModel.kt

@@ -240,7 +240,7 @@ internal constructor(
       return
     }
 
-    data[taskViewModel.task] = taskViewModel.taskTaskData.firstOrNull()
+    data[taskViewModel.task] = taskViewModel.taskTaskData.value
 
     if (!isLastPosition()) {
       step(1)

ground/src/main/java/com/google/android/ground/ui/datacollection/tasks/multiplechoice/MultipleChoiceTaskViewModel.kt

@@ -148,8 +148,8 @@ class MultipleChoiceTaskViewModel @Inject constructor(resources: Resources) :
   }
 
   companion object {
-    private const val OTHER_ID: String = "OTHER_ID"
-    private const val OTHER_PREFIX: String = "[ "
-    private const val OTHER_SUFFIX: String = " ]"
+    const val OTHER_ID: String = "OTHER_ID"
+    const val OTHER_PREFIX: String = "[ "
+    const val OTHER_SUFFIX: String = " ]"
   }
 }

ground/src/test/java/com/google/android/ground/persistence/remote/firebase/schema/SubmissionMutationConverterTest.kt

@@ -60,6 +60,7 @@ import com.google.android.ground.proto.TaskData.DRAW_GEOMETRY_RESULT_FIELD_NUMBE
 import com.google.android.ground.proto.TaskData.DateTimeResponse.DATE_TIME_FIELD_NUMBER
 import com.google.android.ground.proto.TaskData.DrawGeometryResult.GEOMETRY_FIELD_NUMBER
 import com.google.android.ground.proto.TaskData.MULTIPLE_CHOICE_RESPONSES_FIELD_NUMBER
+import com.google.android.ground.proto.TaskData.MultipleChoiceResponses.OTHER_TEXT_FIELD_NUMBER
 import com.google.android.ground.proto.TaskData.MultipleChoiceResponses.SELECTED_OPTION_IDS_FIELD_NUMBER
 import com.google.android.ground.proto.TaskData.NUMBER_RESPONSE_FIELD_NUMBER
 import com.google.android.ground.proto.TaskData.NumberResponse.NUMBER_FIELD_NUMBER
@@ -86,7 +87,6 @@ class SubmissionMutationConverterTest {
 
   private val textTaskData = TextTaskData.fromString("some data")
 
-  // TODO: Add test coverage for "other" value
   private val singleChoiceResponse =
     MultipleChoiceTaskData.fromList(
       MultipleChoice(
@@ -99,7 +99,18 @@ class SubmissionMutationConverterTest {
       ids = listOf("option id 1"),
     )
 
-  // TODO: Add test coverage for "other" value
+  private val singleChoiceResponseOther =
+    MultipleChoiceTaskData.fromList(
+      MultipleChoice(
+        persistentListOf(
+          Option("option id 1", "code1", "Option 1"),
+          Option("option id 2", "code2", "Option 2"),
+        ),
+        MultipleChoice.Cardinality.SELECT_ONE,
+      ),
+      ids = listOf("[ other value ]"),
+    )
+
   private val multipleChoiceTaskData =
     MultipleChoiceTaskData.fromList(
       MultipleChoice(
@@ -112,6 +123,18 @@ class SubmissionMutationConverterTest {
       ids = listOf("option id 1", "option id 2"),
     )
 
+  private val multipleChoiceTaskDataOther =
+    MultipleChoiceTaskData.fromList(
+      MultipleChoice(
+        persistentListOf(
+          Option("option id 1", "code1", "Option 1"),
+          Option("option id 2", "code2", "Option 2"),
+        ),
+        MultipleChoice.Cardinality.SELECT_MULTIPLE,
+      ),
+      ids = listOf("option id 1", "option id 2", "[ other value ]"),
+    )
+
   private val numberTaskData = NumberTaskData.fromNumber("123")
 
   private val dropPinTaskResult = DropPinTaskData(Point(Coordinates(10.0, 20.0)))
@@ -159,11 +182,21 @@ class SubmissionMutationConverterTest {
             taskType = Task.Type.MULTIPLE_CHOICE,
             newTaskData = singleChoiceResponse,
           ),
+          ValueDelta(
+            taskId = "single_choice_task_other",
+            taskType = Task.Type.MULTIPLE_CHOICE,
+            newTaskData = singleChoiceResponseOther,
+          ),
           ValueDelta(
             taskId = "multiple_choice_task",
             taskType = Task.Type.MULTIPLE_CHOICE,
             newTaskData = multipleChoiceTaskData,
           ),
+          ValueDelta(
+            taskId = "multiple_choice_task_other",
+            taskType = Task.Type.MULTIPLE_CHOICE,
+            newTaskData = multipleChoiceTaskDataOther,
+          ),
           ValueDelta(
             taskId = "number_task",
             taskType = Task.Type.NUMBER,
@@ -200,13 +233,26 @@ class SubmissionMutationConverterTest {
           mapOf(SELECTED_OPTION_IDS_FIELD_NUMBER.toString() to listOf("option id 1")),
         TASK_ID_FIELD_NUMBER.toString() to "single_choice_task",
       ),
+      mapOf(
+        MULTIPLE_CHOICE_RESPONSES_FIELD_NUMBER.toString() to
+          mapOf(OTHER_TEXT_FIELD_NUMBER.toString() to "other value"),
+        TASK_ID_FIELD_NUMBER.toString() to "single_choice_task_other",
+      ),
       mapOf(
         MULTIPLE_CHOICE_RESPONSES_FIELD_NUMBER.toString() to
           mapOf(
             SELECTED_OPTION_IDS_FIELD_NUMBER.toString() to listOf("option id 1", "option id 2")
           ),
         TASK_ID_FIELD_NUMBER.toString() to "multiple_choice_task",
       ),
+      mapOf(
+        MULTIPLE_CHOICE_RESPONSES_FIELD_NUMBER.toString() to
+          mapOf(
+            SELECTED_OPTION_IDS_FIELD_NUMBER.toString() to listOf("option id 1", "option id 2"),
+            OTHER_TEXT_FIELD_NUMBER.toString() to "other value",
+          ),
+        TASK_ID_FIELD_NUMBER.toString() to "multiple_choice_task_other",
+      ),
       mapOf(
         NUMBER_RESPONSE_FIELD_NUMBER.toString() to mapOf(NUMBER_FIELD_NUMBER.toString() to 123.0),
         TASK_ID_FIELD_NUMBER.toString() to "number_task",