Sync: Fix double mutation detail rendering in sync status. (#2609)

* Sync: Fix double mutation detail rendering in sync status.

Fixes #2389.

When a user collects data, they may need to add a new LOI depending on the job configuration. We treat the addition of the LOI and the eventual submission of data against it as two distinct data mutations. This resulted in duplicate mutation details in sync status for what, to the user, is perceived as a single instance of data collection/submission, which can prove confusing.

To resolve this, we now associate mutations together using a "collection ID" which indicates whether or not two distinct mutations originated from the same data collection flow. When we read mutations out of the local store, we then combine them using this identifier, taking only the more recent mutation as indicative of the overall status for the submission.

* Chore: Fix formatting.

* Chore: Remove unused variable.

---------

Co-authored-by: Gino Miceli <228050+gino-m@users.noreply.github.com>

ground/src/main/java/com/google/android/ground/Config.kt

@@ -25,7 +25,7 @@ object Config {
   const val SHARED_PREFS_MODE = Context.MODE_PRIVATE
 
   // Local db settings.
-  const val DB_VERSION = 118
+  const val DB_VERSION = 119
   const val DB_NAME = "ground.db"
 
   // Firebase Cloud Firestore settings.

ground/src/main/java/com/google/android/ground/domain/usecases/submission/SubmitDataUseCase.kt

@@ -44,12 +44,13 @@ constructor(
     surveyId: String,
     deltas: List<ValueDelta>,
     loiName: String?,
+    collectionId: String,
   ) {
     Timber.v("Submitting data for LOI: $selectedLoiId")
     val deltasToSubmit = deltas.toMutableList()
     val submissionLoiId =
-      selectedLoiId ?: addLocationOfInterest(surveyId, job, deltasToSubmit, loiName)
-    submissionRepository.saveSubmission(surveyId, submissionLoiId, deltasToSubmit)
+      selectedLoiId ?: addLocationOfInterest(surveyId, job, deltasToSubmit, loiName, collectionId)
+    submissionRepository.saveSubmission(surveyId, submissionLoiId, deltasToSubmit, collectionId)
   }
 
   /**
@@ -61,12 +62,19 @@ constructor(
     job: Job,
     deltas: MutableList<ValueDelta>,
     loiName: String?,
+    collectionId: String,
   ): String {
     val addLoiTask = job.getAddLoiTask() ?: error("Null LOI ID but no add LOI task")
     val addLoiTaskId = deltas.indexOfFirst { it.taskId == addLoiTask.id }
     if (addLoiTaskId < 0) error("Add LOI task response missing")
     val addLoiValue = deltas.removeAt(addLoiTaskId).newTaskData
     if (addLoiValue !is GeometryTaskData) error("Invalid add LOI task response")
-    return locationOfInterestRepository.saveLoi(addLoiValue.geometry, job, surveyId, loiName)
+    return locationOfInterestRepository.saveLoi(
+      addLoiValue.geometry,
+      job,
+      surveyId,
+      loiName,
+      collectionId,
+    )
   }
 }

ground/src/main/java/com/google/android/ground/model/locationofinterest/LocationOfInterest.kt

@@ -73,6 +73,7 @@ data class LocationOfInterest(
       submissionCount = submissionCount,
       properties = properties,
       isPredefined = isPredefined,
+      collectionId = "",
     )
 
   fun getProperty(key: String): String = properties[key]?.toString() ?: ""

ground/src/main/java/com/google/android/ground/model/mutation/LocationOfInterestMutation.kt

@@ -29,6 +29,7 @@ data class LocationOfInterestMutation(
   override val clientTimestamp: Date = Date(),
   override val retryCount: Long = 0,
   override val lastError: String = "",
+  override val collectionId: String,
   val jobId: String = "",
   val customId: String = "",
   val geometry: Geometry? = null,

ground/src/main/java/com/google/android/ground/model/mutation/Mutation.kt

@@ -22,15 +22,32 @@ import java.util.Date
  * store.
  */
 sealed class Mutation {
+  /** A unique identifier for this mutation. */
   abstract val id: Long?
+  /** The kind of data manipulation operation this mutation represents. See [Mutation.Type]. */
   abstract val type: Type
+  /**
+   * Indicates whether or not this mutation has been reflected in remote storage. See
+   * [Mutation.SyncStatus].
+   */
   abstract val syncStatus: SyncStatus
+  /** The ID of the survey containing the data this mutation alters. */
   abstract val surveyId: String
+  /** The ID of the LOI containing the data this mutation alters. */
   abstract val locationOfInterestId: String
+  /** The ID of the user who initiated this mutation. */
   abstract val userId: String
+  /** The time at which this mutation was issued on the client. */
   abstract val clientTimestamp: Date
+  /** The number of times the system has attempted to apply this mutation to remote storage. */
   abstract val retryCount: Long
+  /** The error last encountered when attempting to apply this mutation to the remote storage. */
   abstract val lastError: String
+  /**
+   * A unique identifier tying this mutation to a singular instance of data collection. Multiple
+   * mutations are associated using this identifier.
+   */
+  abstract val collectionId: String
 
   enum class Type {
     /** Indicates a new entity should be created. */

ground/src/main/java/com/google/android/ground/model/mutation/SubmissionMutation.kt

@@ -29,6 +29,7 @@ data class SubmissionMutation(
   override val clientTimestamp: Date = Date(),
   override val retryCount: Long = 0,
   override val lastError: String = "",
+  override val collectionId: String,
   val job: Job,
   val submissionId: String = "",
   val deltas: List<ValueDelta> = listOf(),

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

@@ -197,6 +197,7 @@ fun LocationOfInterestMutation.toLocalDataStoreObject() =
     retryCount = retryCount,
     newProperties = properties,
     isPredefined = isPredefined,
+    collectionId = collectionId,
   )
 
 fun LocationOfInterestMutationEntity.toModelObject() =
@@ -215,6 +216,7 @@ fun LocationOfInterestMutationEntity.toModelObject() =
     retryCount = retryCount,
     properties = newProperties,
     isPredefined = isPredefined,
+    collectionId = collectionId,
   )
 
 fun MultipleChoiceEntity.toModelObject(optionEntities: List<OptionEntity>): MultipleChoice {
@@ -343,6 +345,7 @@ fun SubmissionMutationEntity.toModelObject(survey: Survey): SubmissionMutation {
     lastError = lastError,
     userId = userId,
     clientTimestamp = Date(clientTimestamp),
+    collectionId = collectionId,
   )
 }
 
@@ -360,6 +363,7 @@ fun SubmissionMutation.toLocalDataStoreObject() =
     lastError = lastError,
     userId = userId,
     clientTimestamp = clientTimestamp.time,
+    collectionId = collectionId,
   )
 
 fun SurveyEntityAndRelations.toModelObject(): Survey {

ground/src/main/java/com/google/android/ground/persistence/local/room/entity/LocationOfInterestMutationEntity.kt

@@ -54,6 +54,7 @@ data class LocationOfInterestMutationEntity(
   @ColumnInfo(name = "location_of_interest_id") val locationOfInterestId: String,
   @ColumnInfo(name = "job_id") val jobId: String,
   @ColumnInfo(name = "is_predefined") val isPredefined: Boolean?,
+  @ColumnInfo(name = "collection_id") val collectionId: String,
   /** Non-null if the LOI's geometry was updated, null if unchanged. */
   val newGeometry: GeometryWrapper?,
   val newProperties: LoiProperties,

ground/src/main/java/com/google/android/ground/persistence/local/room/entity/SubmissionMutationEntity.kt

@@ -53,6 +53,7 @@ data class SubmissionMutationEntity(
   @ColumnInfo(name = "location_of_interest_id") val locationOfInterestId: String,
   @ColumnInfo(name = "job_id") val jobId: String,
   @ColumnInfo(name = "submission_id") val submissionId: String,
+  @ColumnInfo(name = "collection_id") val collectionId: String,
   /**
    * For mutations of type [MutationEntityType.CREATE] and [MutationEntityType.UPDATE], returns a
    * [JSONObject] with the new values of modified submission data, with `null` values representing

ground/src/main/java/com/google/android/ground/repository/LocationOfInterestRepository.kt

@@ -81,7 +81,13 @@ constructor(
   }
 
   /** Saves a new LOI in the local db and enqueues a sync worker. */
-  suspend fun saveLoi(geometry: Geometry, job: Job, surveyId: String, loiName: String?): String {
+  suspend fun saveLoi(
+    geometry: Geometry,
+    job: Job,
+    surveyId: String,
+    loiName: String?,
+    collectionId: String,
+  ): String {
     val newId = uuidGenerator.generateUuid()
     val user = userRepository.getAuthenticatedUser()
     val mutation =
@@ -95,6 +101,7 @@ constructor(
         geometry = geometry,
         properties = generateProperties(loiName),
         isPredefined = false,
+        collectionId = collectionId,
       )
     applyAndEnqueue(mutation)
     return newId

ground/src/main/java/com/google/android/ground/repository/MutationRepository.kt

@@ -141,9 +141,10 @@ constructor(
     locationOfInterestMutations: List<LocationOfInterestMutation>,
     submissionMutations: List<SubmissionMutation>,
   ): List<Mutation> =
-    (locationOfInterestMutations + submissionMutations).sortedWith(
-      Mutation.byDescendingClientTimestamp()
-    )
+    (locationOfInterestMutations + submissionMutations)
+      .groupBy { it.collectionId }
+      .map { it.value.reduce { a, b -> if (a.clientTimestamp > b.clientTimestamp) a else b } }
+      .sortedWith(Mutation.byDescendingClientTimestamp())
 }
 
 // TODO(#2119): Refactor this and the related markAs* methods out of this repository. Workers will

ground/src/main/java/com/google/android/ground/repository/SubmissionRepository.kt

@@ -52,6 +52,7 @@ constructor(
     surveyId: String,
     locationOfInterestId: String,
     deltas: List<ValueDelta>,
+    collectionId: String,
   ) {
     val newId = uuidGenerator.generateUuid()
     val userId = userRepository.getAuthenticatedUser().id
@@ -66,6 +67,7 @@ constructor(
         surveyId = surveyId,
         locationOfInterestId = locationOfInterestId,
         userId = userId,
+        collectionId = collectionId,
       )
     applyAndEnqueue(mutation)
   }

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

@@ -30,6 +30,7 @@ import com.google.android.ground.model.submission.isNullOrEmpty
 import com.google.android.ground.model.task.Condition
 import com.google.android.ground.model.task.Task
 import com.google.android.ground.persistence.local.room.converter.SubmissionDeltasConverter
+import com.google.android.ground.persistence.uuid.OfflineUuidGenerator
 import com.google.android.ground.repository.LocationOfInterestRepository
 import com.google.android.ground.repository.SubmissionRepository
 import com.google.android.ground.repository.SurveyRepository
@@ -82,6 +83,7 @@ internal constructor(
   @IoDispatcher private val ioDispatcher: CoroutineDispatcher,
   private val savedStateHandle: SavedStateHandle,
   private val submissionRepository: SubmissionRepository,
+  private val offlineUuidGenerator: OfflineUuidGenerator,
   locationOfInterestRepository: LocationOfInterestRepository,
   surveyRepository: SurveyRepository,
 ) : AbstractViewModel() {
@@ -263,8 +265,9 @@ internal constructor(
 
   /** Persists the changes locally and enqueues a worker to sync with remote datastore. */
   private fun saveChanges(deltas: List<ValueDelta>) {
+    val collectionId = offlineUuidGenerator.generateUuid()
     externalScope.launch(ioDispatcher) {
-      submitDataUseCase.invoke(loiId, job, surveyId, deltas, customLoiName)
+      submitDataUseCase.invoke(loiId, job, surveyId, deltas, customLoiName, collectionId)
     }
   }
 

ground/src/main/java/com/google/android/ground/ui/syncstatus/SyncListItem.kt

@@ -142,7 +142,12 @@ fun PreviewSyncListItem(
       user = "Jane Doe",
       loiLabel = "Map the farms",
       loiSubtitle = "IDX21311",
-      mutation = SubmissionMutation(job = Job(id = "123"), syncStatus = Mutation.SyncStatus.PENDING),
+      mutation =
+        SubmissionMutation(
+          job = Job(id = "123"),
+          syncStatus = Mutation.SyncStatus.PENDING,
+          collectionId = "example",
+        ),
     )
 ) {
   SyncListItem(Modifier, detail)

ground/src/test/java/com/google/android/ground/persistence/local/LocalDataStoreTests.kt

@@ -411,6 +411,7 @@ class LocalDataStoreTests : BaseHiltTest() {
         surveyId = FakeData.SURVEY_ID,
         locationOfInterestId = FakeData.LOI_ID,
         userId = FakeData.USER_ID,
+        collectionId = "",
       )
     private val TEST_OFFLINE_AREA =
       OfflineArea("id_1", OfflineArea.State.PENDING, Bounds(0.0, 0.0, 0.0, 0.0), "Test Area", 0..14)

ground/src/test/java/com/google/android/ground/persistence/remote/firebase/protobuf/LoiMutationConverterTest.kt

@@ -136,6 +136,7 @@ class LoiMutationConverterTest {
         userId = TEST_USER.id,
         type = Mutation.Type.CREATE,
         properties = mapOf(LOI_NAME_PROPERTY to LOCATION_OF_INTEREST_NAME),
+        collectionId = "collectionId",
       )
 
     val map = mutation.createLoiMessage(TEST_USER).toFirestoreMap()
@@ -219,6 +220,7 @@ class LoiMutationConverterTest {
         submissionCount = 10,
         properties = mapOf(LOI_NAME_PROPERTY to LOCATION_OF_INTEREST_NAME),
         customId = "a custom loi",
+        collectionId = "collectionId",
       )
 
     fun newAoiMutation(
@@ -238,6 +240,7 @@ class LoiMutationConverterTest {
         clientTimestamp = Date.from(Instant.ofEpochSecond(987654321)),
         properties = mapOf(LOI_NAME_PROPERTY to LOCATION_OF_INTEREST_NAME),
         customId = "a custom loi",
+        collectionId = "collectionId",
       )
   }
 }

ground/src/test/java/com/google/android/ground/persistence/remote/firebase/protobuf/ModelToProtoExtKtTest.kt

@@ -54,6 +54,7 @@ class ModelToProtoExtKtTest {
         submissionCount = 1,
         properties = generateProperties("loiName"),
         isPredefined = false,
+        collectionId = "collectionId",
       )
 
     val output = mutation.createLoiMessage(user)
@@ -103,6 +104,7 @@ class ModelToProtoExtKtTest {
         submissionCount = 1,
         properties = generateProperties("loiName"),
         isPredefined = null,
+        collectionId = "collectionId",
       )
 
     val output = mutation.createLoiMessage(user)
@@ -159,6 +161,7 @@ class ModelToProtoExtKtTest {
         submissionCount = 1,
         properties = generateProperties("loiName"),
         isPredefined = true,
+        collectionId = "collectionId",
       )
 
     val output = mutation.createLoiMessage(user)
@@ -216,6 +219,7 @@ class ModelToProtoExtKtTest {
         submissionCount = 1,
         properties = generateProperties("loiName"),
         isPredefined = false,
+        collectionId = "collectionId",
       )
 
     val output = mutation.createLoiMessage(user)
@@ -273,6 +277,7 @@ class ModelToProtoExtKtTest {
         submissionCount = 1,
         properties = generateProperties("loiName"),
         isPredefined = false,
+        collectionId = "collectionId",
       )
 
     val output = mutation.createLoiMessage(user)
@@ -322,6 +327,7 @@ class ModelToProtoExtKtTest {
         submissionCount = 1,
         properties = generateProperties("loiName"),
         isPredefined = false,
+        collectionId = "collectionId",
       )
 
     assertThrows(UnsupportedOperationException::class.java) { mutation.createLoiMessage(user) }
@@ -344,6 +350,7 @@ class ModelToProtoExtKtTest {
         submissionCount = 1,
         properties = generateProperties("loiName"),
         isPredefined = false,
+        collectionId = "collectionId",
       )
 
     assertThrows(UnsupportedOperationException::class.java) { mutation.createLoiMessage(user) }

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

@@ -150,6 +150,7 @@ class SubmissionMutationConverterTest {
       userId = user.id,
       clientTimestamp = clientTimestamp,
       job = job,
+      collectionId = "collectionId",
       deltas =
         listOf(
           ValueDelta(taskId = "text_task", taskType = Task.Type.TEXT, newTaskData = textTaskData),

ground/src/test/java/com/google/android/ground/persistence/sync/LocalMutationSyncWorkerTest.kt

@@ -203,6 +203,7 @@ class LocalMutationSyncWorkerTest : BaseHiltTest() {
       userId = TEST_USER_ID,
       surveyId = TEST_SURVEY_ID,
       geometry = TEST_GEOMETRY,
+      collectionId = "collectionId",
     )
 
   private fun createSubmissionMutation() =
@@ -213,6 +214,7 @@ class LocalMutationSyncWorkerTest : BaseHiltTest() {
       userId = TEST_USER_ID,
       job = TEST_JOB,
       surveyId = TEST_SURVEY_ID,
+      collectionId = "collectionId",
     )
 
   private suspend fun createAndDoWork(context: Context, loiId: String?): ListenableWorker.Result =

ground/src/test/java/com/google/android/ground/persistence/sync/MediaUploadWorkerTest.kt

@@ -223,6 +223,7 @@ class MediaUploadWorkerTest : BaseHiltTest() {
         userId = FakeData.USER.id,
         job = FakeData.JOB,
         surveyId = FakeData.SURVEY.id,
+        collectionId = "collectionId",
         deltas =
           listOf(ValueDelta(PHOTO_TASK_ID, Task.Type.PHOTO, TextTaskData("foo/$PHOTO_TASK_ID.jpg"))),
       )