Minor ktdoc and code cleanup ahead of #2840 (#2874)

* Refactor photo data accessor fun

* Import constants for readability

* Delete obsolete TODO

* Static import

* Tweak ktdoc

* Lift ktdoc into interface

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

@@ -16,6 +16,7 @@
 package com.google.android.ground.model.mutation
 
 import com.google.android.ground.model.job.Job
+import com.google.android.ground.model.submission.PhotoTaskData
 import com.google.android.ground.model.submission.ValueDelta
 import java.util.Date
 
@@ -45,4 +46,7 @@ data class SubmissionMutation(
   fun mediaUploadPending() =
     this.syncStatus == SyncStatus.MEDIA_UPLOAD_PENDING ||
       this.syncStatus == SyncStatus.MEDIA_UPLOAD_AWAITING_RETRY
+
+  fun getPhotoData(): List<PhotoTaskData> =
+    deltas.map { it.newTaskData }.filterIsInstance<PhotoTaskData>().filter { !it.isEmpty() }
 }

ground/src/main/java/com/google/android/ground/persistence/local/room/stores/RoomLocationOfInterestStore.kt

@@ -44,20 +44,11 @@ class RoomLocationOfInterestStore @Inject internal constructor() : LocalLocation
   @Inject lateinit var locationOfInterestMutationDao: LocationOfInterestMutationDao
   @Inject lateinit var userStore: RoomUserStore
 
-  /**
-   * Retrieves the complete set of [LocationOfInterest] associated with the given [Survey] from the
-   * local database and returns a [Flow] that continually emits the complete set anew any time the
-   * underlying table changes (insertions, deletions, updates).
-   */
   override fun getValidLois(survey: Survey): Flow<Set<LocationOfInterest>> =
     locationOfInterestDao.getByDeletionState(survey.id, EntityDeletionState.DEFAULT).map {
       toLocationsOfInterest(survey, it)
     }
 
-  /**
-   * Attempts to retrieve the [LocationOfInterest] with the given ID that's associated with the
-   * given [Survey].
-   */
   override suspend fun getLocationOfInterest(
     survey: Survey,
     locationOfInterestId: String,

ground/src/main/java/com/google/android/ground/persistence/local/stores/LocalLocationOfInterestStore.kt

@@ -25,12 +25,13 @@ import kotlinx.coroutines.flow.Flow
 interface LocalLocationOfInterestStore :
   LocalMutationStore<LocationOfInterestMutation, LocationOfInterest> {
   /**
-   * Returns a main-safe flow that emits the full set of LOIs for a survey on subscribe, and
-   * continues to return the full set each time a LOI is added/changed/removed.
+   * Retrieves the complete set of [LocationOfInterest] associated with the given [Survey] from the
+   * local database and returns a [Flow] that continually emits the complete set anew any time the
+   * underlying table changes (insertions, deletions, updates).
    */
   fun getValidLois(survey: Survey): Flow<Set<LocationOfInterest>>
 
-  /** Returns the LOI with the specified UUID from the local data store, if found. */
+  /** Returns the [LocationOfInterest] with the specified UUID from the local data store. */
   suspend fun getLocationOfInterest(
     survey: Survey,
     locationOfInterestId: String,

ground/src/main/java/com/google/android/ground/persistence/sync/MediaUploadWorker.kt

@@ -96,7 +96,7 @@ constructor(
    * status depending on whether or the uploads failed or succeeded.
    */
   private suspend fun uploadMedia(mutation: SubmissionMutation): SubmissionMutation {
-    val photoTasks = mutation.deltas.map { it.newTaskData }.filterIsInstance<PhotoTaskData>()
+    val photoTasks = mutation.getPhotoData()
     return uploadPhotos(photoTasks)
       .fold(
         onSuccess = { mutation.updateSyncStatus(Mutation.SyncStatus.COMPLETED) },
@@ -117,7 +117,6 @@ constructor(
   private suspend fun uploadPhotos(photoTaskDataList: List<PhotoTaskData>): kotlin.Result<Unit> =
     // TODO(#2120): Retry uploads on a per-photo basis, instead of per-response.
     photoTaskDataList
-      .filter { !it.isEmpty() }
       .map { uploadPhotoMedia(it) }
       .fold(kotlin.Result.success(Unit)) { a, b -> if (a.isSuccess) b else a }
 

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

@@ -19,6 +19,10 @@ package com.google.android.ground.repository
 import com.google.android.ground.model.Survey
 import com.google.android.ground.model.mutation.LocationOfInterestMutation
 import com.google.android.ground.model.mutation.Mutation
+import com.google.android.ground.model.mutation.Mutation.SyncStatus
+import com.google.android.ground.model.mutation.Mutation.SyncStatus.FAILED
+import com.google.android.ground.model.mutation.Mutation.SyncStatus.IN_PROGRESS
+import com.google.android.ground.model.mutation.Mutation.SyncStatus.MEDIA_UPLOAD_PENDING
 import com.google.android.ground.model.mutation.SubmissionMutation
 import com.google.android.ground.persistence.local.room.converter.toModelObject
 import com.google.android.ground.persistence.local.room.entity.LocationOfInterestMutationEntity
@@ -134,15 +138,15 @@ constructor(
       }
 
   suspend fun markAsInProgress(mutations: List<Mutation>) {
-    saveMutationsLocally(mutations.updateMutationStatus(Mutation.SyncStatus.IN_PROGRESS))
+    saveMutationsLocally(mutations.updateMutationStatus(IN_PROGRESS))
   }
 
   suspend fun markAsFailed(mutations: List<Mutation>, error: Throwable) {
-    saveMutationsLocally(mutations.updateMutationStatus(Mutation.SyncStatus.FAILED, error))
+    saveMutationsLocally(mutations.updateMutationStatus(FAILED, error))
   }
 
   private suspend fun markForMediaUpload(mutations: List<Mutation>) {
-    saveMutationsLocally(mutations.updateMutationStatus(Mutation.SyncStatus.MEDIA_UPLOAD_PENDING))
+    saveMutationsLocally(mutations.updateMutationStatus(MEDIA_UPLOAD_PENDING))
   }
 
   private fun combineAndSortMutations(
@@ -155,27 +159,11 @@ constructor(
       .sortedWith(Mutation.byDescendingClientTimestamp())
 }
 
-// TODO(#2119): Refactor this and the related markAs* methods out of this repository. Workers will
-// generally
-// want to have control over when work should be retried. This means they may need finer grained
-// control over when a mutation is marked as failed and when it is considered eligible for retry
-// based on various conditions. Batch marking sequences of mutations prevents this. Instead, let's
-// have
-// workers operate directly on values List<Mutation> updating them appropriately, then batch write
-// these via the repository using saveMutationsLocally.
-//
-// For example, a worker would do:
-//   repo.getMutations(....)
-//       .map { doRemoteOrBackgroundWork(it) }
-//       .map { if (condition...) it.updateStatus(RETRY) else it.updateStatus(FAILED) } // for
-// illustration; we'd likely just do this in "doRemoteOr..."
-//       .also { repo.saveMutationsLocally(it) } // write updated mutations to local storage to
-// exclude/include them in further processing runs.
 private fun List<Mutation>.updateMutationStatus(
-  syncStatus: Mutation.SyncStatus,
+  syncStatus: SyncStatus,
   error: Throwable? = null,
 ): List<Mutation> = map {
-  val hasSyncFailed = syncStatus == Mutation.SyncStatus.FAILED
+  val hasSyncFailed = syncStatus == FAILED
   val retryCount = if (hasSyncFailed) it.retryCount + 1 else it.retryCount
   val errorMessage = if (hasSyncFailed) error?.message ?: error.toString() else it.lastError
 

ground/src/main/java/com/google/android/ground/ui/home/HomeScreenViewModel.kt

@@ -67,8 +67,8 @@ internal constructor(
   /**
    * Enqueue data and photo upload workers for all pending mutations when home screen is first
    * opened as a workaround the get stuck mutations (i.e., PENDING or FAILED mutations with no
-   * scheduled workers) going again. Workaround for
-   * https://github.com/google/ground-android/issues/2751.
+   * scheduled workers) going again. If there are no mutations in the upload queue this will be a
+   * no-op. Workaround for https://github.com/google/ground-android/issues/2751.
    */
   private suspend fun kickLocalMutationSyncWorkers() {
     val mutations = mutationRepository.getAllMutationsFlow().first()