[Data submission] Set "skipped" to true when user skips a task (#2718)

config/detekt/baseline.xml

@@ -2,6 +2,7 @@
 <SmellBaseline>
   <ManuallySuppressedIssues></ManuallySuppressedIssues>
   <CurrentIssues>
+    <ID>CyclomaticComplexMethod:ModelToProtoExt.kt$private fun toTaskData(id: String, newTaskData: TaskData)</ID>
     <ID>ReturnCount:HomeScreenViewModel.kt$HomeScreenViewModel$suspend fun getDraftSubmission(): DraftSubmission?</ID>
     <ID>UnusedPrivateProperty:HomeScreenFragmentTest.kt$NavigationDrawerItemClickTest$private val testLabel: String</ID>
   </CurrentIssues>

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

@@ -0,0 +1,27 @@
+/*
+ * Copyright 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.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.google.android.ground.model.submission
+
+import kotlinx.serialization.Serializable
+
+/** User skipped the response for the given task. */
+@Serializable
+class SkippedTaskData : TaskData {
+
+  override fun getDetailsText(): String = ""
+
+  override fun isEmpty(): Boolean = false
+}

ground/src/main/java/com/google/android/ground/persistence/local/room/converter/ValueJsonConverter.kt

@@ -25,6 +25,7 @@ import com.google.android.ground.model.submission.DropPinTaskData
 import com.google.android.ground.model.submission.MultipleChoiceTaskData
 import com.google.android.ground.model.submission.NumberTaskData
 import com.google.android.ground.model.submission.PhotoTaskData
+import com.google.android.ground.model.submission.SkippedTaskData
 import com.google.android.ground.model.submission.TaskData
 import com.google.android.ground.model.submission.TextTaskData
 import com.google.android.ground.model.submission.TimeTaskData
@@ -44,6 +45,7 @@ import timber.log.Timber
 
 internal object ValueJsonConverter {
 
+  private const val SKIPPED_KEY = "skipped"
   private const val ISO_DATE_TIME_FORMAT = "yyyy-MM-dd'T'HH:mmZ"
 
   fun toJsonObject(taskData: TaskData?): Any {
@@ -58,6 +60,7 @@ internal object ValueJsonConverter {
       is DrawAreaTaskData -> GeometryWrapperTypeConverter.toString(taskData.geometry)
       is DropPinTaskData -> GeometryWrapperTypeConverter.toString(taskData.geometry)
       is CaptureLocationTaskData -> taskData.toJSONObject()
+      is SkippedTaskData -> JSONObject().put(SKIPPED_KEY, true)
       else -> throw UnsupportedOperationException("Unimplemented value class ${taskData.javaClass}")
     }
   }
@@ -80,6 +83,11 @@ internal object ValueJsonConverter {
     if (JSONObject.NULL === obj) {
       return null
     }
+
+    if (obj is JSONObject && obj.getBoolean(SKIPPED_KEY)) {
+      return SkippedTaskData()
+    }
+
     return when (task.type) {
       Task.Type.TEXT -> {
         DataStoreException.checkType(String::class.java, obj)

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

@@ -33,11 +33,11 @@ import com.google.android.ground.model.submission.GeometryTaskData
 import com.google.android.ground.model.submission.MultipleChoiceTaskData
 import com.google.android.ground.model.submission.NumberTaskData
 import com.google.android.ground.model.submission.PhotoTaskData
+import com.google.android.ground.model.submission.SkippedTaskData
+import com.google.android.ground.model.submission.TaskData
 import com.google.android.ground.model.submission.TextTaskData
 import com.google.android.ground.model.submission.TimeTaskData
-import com.google.android.ground.model.submission.ValueDelta
 import com.google.android.ground.model.submission.isNotNullOrEmpty
-import com.google.android.ground.model.task.Task
 import com.google.android.ground.proto.LinearRing as LinearRingProto
 import com.google.android.ground.proto.LocationOfInterest.Property
 import com.google.android.ground.proto.LocationOfInterest.Source
@@ -74,7 +74,7 @@ fun SubmissionMutation.createSubmissionMessage(user: User) = submission {
 
   deltas.forEach {
     if (it.newTaskData.isNotNullOrEmpty()) {
-      taskData.add(it.toMessage())
+      taskData.add(toTaskData(it.taskId, it.newTaskData!!))
     }
   }
 
@@ -128,48 +128,39 @@ fun LocationOfInterestMutation.createLoiMessage(user: User) = locationOfInterest
   }
 }
 
-private fun ValueDelta.toMessage() = taskData {
-  val me = this@toMessage
+private fun toTaskData(id: String, newTaskData: TaskData) = taskData {
   // TODO: What should be the ID?
-  taskId = me.taskId
-  // TODO: Add "skipped" field
-  when (taskType) {
-    Task.Type.TEXT -> textResponse = textResponse { text = (newTaskData as TextTaskData).text }
-    Task.Type.NUMBER -> numberResponse = numberResponse {
-        number = (newTaskData as NumberTaskData).value
-      }
+  taskId = id
+
+  when (newTaskData) {
+    is TextTaskData -> textResponse = textResponse { text = newTaskData.text }
+    is NumberTaskData -> numberResponse = numberResponse { number = newTaskData.value }
     // TODO: Ensure the dates are always converted to UTC time zone.
-    Task.Type.DATE -> dateTimeResponse = dateTimeResponse {
-        dateTime = timestamp { seconds = (newTaskData as DateTaskData).date.time / 1000 }
+    is DateTaskData -> dateTimeResponse = dateTimeResponse {
+        dateTime = timestamp { seconds = newTaskData.date.time / 1000 }
       }
     // TODO: Ensure the dates are always converted to UTC time zone.
-    Task.Type.TIME -> dateTimeResponse = dateTimeResponse {
-        dateTime = timestamp { seconds = (newTaskData as TimeTaskData).time.time / 1000 }
+    is TimeTaskData -> dateTimeResponse = dateTimeResponse {
+        dateTime = timestamp { seconds = newTaskData.time.time / 1000 }
       }
-    Task.Type.MULTIPLE_CHOICE -> multipleChoiceResponses = multipleChoiceResponses {
-        (newTaskData as MultipleChoiceTaskData).getSelectedOptionsIdsExceptOther().forEach {
-          selectedOptionIds.add(it)
-        }
+    is MultipleChoiceTaskData -> multipleChoiceResponses = multipleChoiceResponses {
+        newTaskData.getSelectedOptionsIdsExceptOther().forEach { selectedOptionIds.add(it) }
         if (newTaskData.hasOtherText()) {
           otherText = newTaskData.getOtherText()
         }
       }
-    Task.Type.DROP_PIN,
-    Task.Type.DRAW_AREA -> drawGeometryResult = drawGeometryResult {
-        geometry = (newTaskData as GeometryTaskData).geometry.toMessage()
-      }
-    Task.Type.CAPTURE_LOCATION -> captureLocationResult = captureLocationResult {
-        val data = newTaskData as CaptureLocationTaskData
-        data.altitude?.let { altitude = it }
-        data.accuracy?.let { accuracy = it }
-        coordinates = data.location.coordinates.toMessage()
+    is CaptureLocationTaskData -> captureLocationResult = captureLocationResult {
+        newTaskData.altitude?.let { altitude = it }
+        newTaskData.accuracy?.let { accuracy = it }
+        coordinates = newTaskData.location.coordinates.toMessage()
         // TODO: Add timestamp
       }
-    Task.Type.PHOTO -> takePhotoResult = takePhotoResult {
-        val data = newTaskData as PhotoTaskData
-        photoPath = data.remoteFilename
+    is GeometryTaskData -> drawGeometryResult = drawGeometryResult {
+        geometry = newTaskData.geometry.toMessage()
       }
-    Task.Type.UNKNOWN -> error("Unknown task type")
+    is PhotoTaskData -> takePhotoResult = takePhotoResult { photoPath = newTaskData.remoteFilename }
+    is SkippedTaskData -> skipped = true
+    else -> error("Unknown task type")
   }
 }
 

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

@@ -72,6 +72,7 @@ private fun MessageValue.toFirestoreValue(): FirestoreValue =
     is List<*> -> map { it?.toFirestoreValue() }
     is Message -> toFirestoreMap()
     is Map<*, *> -> mapValues { it.value?.toFirestoreValue() }
+    is Boolean,
     is String,
     is Number -> this
     else -> throw UnsupportedOperationException("Unsupported type ${this::class}")

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

@@ -164,6 +164,7 @@ abstract class AbstractTaskFragment<T : AbstractTaskViewModel> : AbstractFragmen
 
   private fun onSkip() {
     check(viewModel.hasNoData()) { "User should not be able to skip a task with data." }
+    viewModel.setSkipped()
     moveToNext()
   }
 

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

@@ -19,6 +19,7 @@ import androidx.lifecycle.LiveData
 import androidx.lifecycle.asLiveData
 import com.google.android.ground.R
 import com.google.android.ground.model.job.Job
+import com.google.android.ground.model.submission.SkippedTaskData
 import com.google.android.ground.model.submission.TaskData
 import com.google.android.ground.model.submission.isNullOrEmpty
 import com.google.android.ground.model.task.Task
@@ -63,6 +64,10 @@ open class AbstractTaskViewModel internal constructor() : AbstractViewModel() {
     return null
   }
 
+  fun setSkipped() {
+    setValue(SkippedTaskData())
+  }
+
   fun setValue(taskData: TaskData?) {
     _taskDataFlow.value = taskData
   }

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

@@ -27,6 +27,7 @@ import com.google.android.ground.model.submission.DrawAreaTaskData
 import com.google.android.ground.model.submission.DropPinTaskData
 import com.google.android.ground.model.submission.MultipleChoiceTaskData
 import com.google.android.ground.model.submission.NumberTaskData
+import com.google.android.ground.model.submission.SkippedTaskData
 import com.google.android.ground.model.submission.TextTaskData
 import com.google.android.ground.model.submission.TimeTaskData
 import com.google.android.ground.model.submission.ValueDelta
@@ -64,6 +65,7 @@ import com.google.android.ground.proto.TaskData.MultipleChoiceResponses.OTHER_TE
 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
+import com.google.android.ground.proto.TaskData.SKIPPED_FIELD_NUMBER
 import com.google.android.ground.proto.TaskData.TASK_ID_FIELD_NUMBER
 import com.google.android.ground.proto.TaskData.TEXT_RESPONSE_FIELD_NUMBER
 import com.google.android.ground.proto.TaskData.TextResponse.TEXT_FIELD_NUMBER
@@ -385,4 +387,97 @@ class SubmissionMutationConverterTest {
       submissionMutation.copy(type = Mutation.Type.UNKNOWN).createSubmissionMessage(user)
     }
   }
+
+  @Test
+  fun testToMap_allTaskDataSkipped() {
+    val mutation =
+      submissionMutation.copy(
+        job =
+          job.copy(
+            tasks =
+              mapOf(
+                Pair("task 1", Task("task 1", 1, Task.Type.TEXT, "task 1", true)),
+                Pair("task 2", Task("task 2", 2, Task.Type.NUMBER, "task 2", true)),
+              )
+          ),
+        deltas =
+          listOf(
+            ValueDelta(
+              taskId = "task 1",
+              taskType = Task.Type.TEXT,
+              newTaskData = SkippedTaskData(),
+            ),
+            ValueDelta(
+              taskId = "task 2",
+              taskType = Task.Type.NUMBER,
+              newTaskData = SkippedTaskData(),
+            ),
+          ),
+        type = Mutation.Type.CREATE,
+      )
+
+    val map = mutation.createSubmissionMessage(user).toFirestoreMap()
+
+    // Only verify "skipped" field as rest of the details have already been verified in previous one
+    assertThat(map[TASK_DATA_FIELD_NUMBER.toString()])
+      .isEqualTo(
+        listOf(
+          mapOf(
+            TASK_ID_FIELD_NUMBER.toString() to "task 1",
+            SKIPPED_FIELD_NUMBER.toString() to true,
+          ),
+          mapOf(
+            TASK_ID_FIELD_NUMBER.toString() to "task 2",
+            SKIPPED_FIELD_NUMBER.toString() to true,
+          ),
+        )
+      )
+  }
+
+  @Test
+  fun testToMap_someTaskDataSkipped() {
+    val mutation =
+      submissionMutation.copy(
+        job =
+          job.copy(
+            tasks =
+              mapOf(
+                Pair("task 1", Task("task 1", 1, Task.Type.TEXT, "task 1", true)),
+                Pair("task 2", Task("task 2", 2, Task.Type.NUMBER, "task 2", true)),
+              )
+          ),
+        deltas =
+          listOf(
+            ValueDelta(
+              taskId = "task 1",
+              taskType = Task.Type.TEXT,
+              newTaskData = TextTaskData.fromString("some data"),
+            ),
+            ValueDelta(
+              taskId = "task 2",
+              taskType = Task.Type.NUMBER,
+              newTaskData = SkippedTaskData(),
+            ),
+          ),
+        type = Mutation.Type.CREATE,
+      )
+
+    val map = mutation.createSubmissionMessage(user).toFirestoreMap()
+
+    // Only verify "skipped" field as rest of the details have already been verified in previous one
+    assertThat(map[TASK_DATA_FIELD_NUMBER.toString()])
+      .isEqualTo(
+        listOf(
+          mapOf(
+            TEXT_RESPONSE_FIELD_NUMBER.toString() to
+              mapOf(TEXT_FIELD_NUMBER.toString() to "some data"),
+            TASK_ID_FIELD_NUMBER.toString() to "task 1",
+          ),
+          mapOf(
+            TASK_ID_FIELD_NUMBER.toString() to "task 2",
+            SKIPPED_FIELD_NUMBER.toString() to true,
+          ),
+        )
+      )
+  }
 }