Merge branch 'master' into ashobhit/2893/choice-ux

config/detekt/detekt.yml

@@ -590,9 +590,9 @@ style:
                 value: 'FIXME:'
             -   reason: 'Forbidden STOPSHIP todo marker in comment, please address the problem before shipping the code.'
                 value: 'STOPSHIP:'
-            # -   reason: 'Forbidden TODO todo marker in comment, please do the changes.'
-            #    value: 'TODO:'
-        allowedPatterns: ''
+            -   reason: 'TODO comment format should be: // TODO: $description'
+                value: '// TODO:'
+        allowedPatterns: '^// TODO: .+$'
     ForbiddenImport:
         active: false
         imports: [ ]

ground/build.gradle

@@ -79,7 +79,8 @@ android {
         minSdkVersion rootProject.androidMinSdk
         targetSdkVersion rootProject.androidTargetSdk
 
-        // TODO(https://github.com/google/ground-android/pull/985): Calculate version code manually
+        // TODO: Calculate version code manually
+        // Issue URL: https://github.com/google/ground-android/pull/985
         versionCode gitVersioner.versionCode
         versionName gitVersioner.versionName + " " + getCommitSha1()
         testInstrumentationRunner "com.google.android.ground.CustomTestRunner"
@@ -358,7 +359,8 @@ dependencies {
 
     implementation "com.google.guava:guava:33.0.0-android"
 
-    // TODO(#1748): Move protos into shared module and set correct path here.
+    // TODO: Move protos into shared module and set correct path here.
+    // Issue URL: https://github.com/google/ground-android/issues/1748
     api("com.google.protobuf:protobuf-kotlin-lite:4.26.1")
 
     // Pulls protodefs from the specified commit in the ground-platform repo.

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

@@ -44,7 +44,9 @@ object Config {
    */
   const val CLUSTERING_ZOOM_THRESHOLD = 14f
 
-  // TODO(#1730): Make sub-paths configurable and stop hardcoding here.
+  // TODO: Make sub-paths configurable and
+  //  stop hardcoding here.
+  // Issue URL: https://github.com/google/ground-android/issues/1730
   const val DEFAULT_MOG_TILE_LOCATION = "/offline-imagery/default"
   private const val DEFAULT_MOG_MIN_ZOOM = 8
   private const val DEFAULT_MOG_MAX_ZOOM = 14

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

@@ -64,7 +64,8 @@ class MainActivity : AbstractActivity() {
   override fun onCreate(savedInstanceState: Bundle?) {
     // Make sure this is before calling super.onCreate()
     setTheme(R.style.AppTheme)
-    // TODO(#620): Remove this to enable dark theme.
+    // TODO: Remove this to enable dark theme.
+    // Issue URL: https://github.com/google/ground-android/issues/620
     AppCompatDelegate.setDefaultNightMode(AppCompatDelegate.MODE_NIGHT_NO)
     super.onCreate(savedInstanceState)
 
@@ -135,7 +136,9 @@ class MainActivity : AbstractActivity() {
             var showDialog by remember { mutableStateOf(true) }
             if (showDialog) {
               PermissionDeniedDialog(
-                // TODO(#2402): Read url from Firestore config/properties/signUpUrl
+                // TODO: Read url from
+                //  Firestore config/properties/signUpUrl
+                // Issue URL: https://github.com/google/ground-android/issues/2402
                 BuildConfig.SIGNUP_FORM_LINK,
                 onSignOut = {
                   showDialog = false

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

@@ -64,6 +64,7 @@ constructor(
   init {
     viewModelScope.launch {
       // TODO: Check auth status whenever fragments resumes
+      // Issue URL: https://github.com/google/ground-android/issues/2624
       authenticationManager.signInState.collect {
         _navigationRequests.emit(onSignInStateChange(it))
       }
@@ -86,7 +87,8 @@ constructor(
       Timber.d("User cancelled sign in")
       MainUiState.OnUserSignedOut
     } else {
-      // TODO(#1808): Display some error dialog to the user with a helpful user-readable message.
+      // TODO: Display some error dialog to the user with a helpful user-readable message.
+      // Issue URL: https://github.com/google/ground-android/issues/1808
       onUserSignedOut()
     }
   }
@@ -100,9 +102,10 @@ constructor(
     surveyRepository.clearActiveSurvey()
     userRepository.clearUserPreferences()
 
-    // TODO(#1691): Once multi-user login is supported, avoid clearing local db data. This is
+    // TODO: Once multi-user login is supported, avoid clearing local db data. This is
     //  currently being done to prevent one user's data to be submitted as another user after
     //  re-login.
+    // Issue URL: https://github.com/google/ground-android/issues/1691
     viewModelScope.launch { withContext(ioDispatcher) { localDatabase.clearAllTables() } }
     return MainUiState.OnUserSignedOut
   }

ground/src/main/java/com/google/android/ground/model/geometry/Geometry.kt

@@ -123,7 +123,8 @@ data class LinearRing(val coordinates: List<Coordinates>) : Geometry {
   override fun isEmpty() = coordinates.isEmpty()
 
   override fun validate() {
-    // TODO(#1647): Check for vertices count > 3
+    // TODO: Check for vertices count > 3
+    // Issue URL: https://github.com/google/ground-android/issues/1647
     if (coordinates.isEmpty()) {
       return
     }

ground/src/main/java/com/google/android/ground/model/job/Job.kt

@@ -41,8 +41,8 @@ data class Job(
   val tasksSorted: List<Task>
     get() = tasks.values.sortedBy { it.index }
 
-  // TODO(#2216): Consider using nulls to indicate absence of value here instead of throwing
-  // an exception.
+  // TODO: Consider using nulls to indicate absence of value here instead of throwing an exception.
+  // Issue URL: https://github.com/google/ground-android/issues/2216
   fun getTask(id: String): Task = tasks[id] ?: throw TaskNotFoundException(id)
 
   /** Job must contain at-most 1 `AddLoiTask`. */

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

@@ -59,6 +59,7 @@ data class LocationOfInterest(
    * database.
    */
   // TODO: Remove this test-only method
+  // Issue URL: https://github.com/google/ground-android/issues/2903
   fun toMutation(type: Mutation.Type, userId: String): LocationOfInterestMutation =
     LocationOfInterestMutation(
       jobId = job.id,

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

@@ -30,6 +30,7 @@ data class CaptureLocationTaskData(
 ) : GeometryTaskData(location) {
   override fun getDetailsText(): String {
     // TODO: Move to strings.xml for i18n
+    // Issue URL: https://github.com/google/ground-android/issues/1733
     val df = DecimalFormat("#.##")
     df.roundingMode = RoundingMode.DOWN
     val coordinatesString = location.coordinates.toDmsFormat()

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

@@ -23,7 +23,8 @@ import java.text.DecimalFormat
 /** User-provided response to a "drop a pin" data collection [Task]. */
 data class DropPinTaskData(val location: Point) : GeometryTaskData(location) {
   override fun getDetailsText(): String {
-    // TODO(#752): Move to strings.xml for i18n
+    // TODO: Move to strings.xml for i18n
+    // Issue URL: https://github.com/google/ground-android/issues/752
     val df = DecimalFormat("#.##")
     df.roundingMode = RoundingMode.DOWN
     return location.coordinates.toDmsFormat()

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

@@ -25,7 +25,8 @@ import com.google.android.ground.model.geometry.Polygon
 /** A user-provided response to a geometry-based task ("drop a pin" or "draw an area"). */
 abstract class GeometryTaskData(val geometry: Geometry) : TaskData {
 
-  // TODO(#1733): Move strings to view layer.
+  // TODO: Move strings to view layer.
+  // Issue URL: https://github.com/google/ground-android/issues/1733
   override fun getDetailsText(): String =
     when (geometry) {
       is Point -> "Point data"

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

@@ -40,7 +40,6 @@ class MultipleChoiceTaskData(
         suffix = MultipleChoiceTaskViewModel.OTHER_SUFFIX,
       ) ?: ""
 
-  // TODO: Make these inner classes non-static and access Task directly.
   override fun getDetailsText(): String =
     selectedOptionIds
       .mapNotNull { multipleChoice?.getOptionById(it) }

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

@@ -21,7 +21,6 @@ package com.google.android.ground.model.submission
  * @property data A map from task id to values. This map is mutable and therefore should never be
  *   exposed outside this class.
  */
-// TODO: Merge into Submission?
 data class SubmissionData(private val data: Map<String, TaskData?> = mapOf()) {
 
   /**

ground/src/main/java/com/google/android/ground/model/task/Task.kt

@@ -35,10 +35,9 @@ constructor(
   val condition: Condition? = null,
 ) {
 
-  /**
-   * Task type names as they appear in the remote db, but in uppercase. DO NOT RENAME! TODO: Define
-   * these in data layer!
-   */
+  // TODO: Define these in data layer!
+  // Issue URL: https://github.com/google/ground-android/issues/2910
+  // Task type names as they appear in the local db, but in uppercase. DO NOT RENAME!
   enum class Type {
     UNKNOWN,
     TEXT,

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

@@ -171,7 +171,8 @@ fun LocationOfInterestMutation.toLocalDataStoreObject(user: User): LocationOfInt
     surveyId = surveyId,
     jobId = jobId,
     deletionState = EntityDeletionState.DEFAULT,
-    // TODO(#1562): Preserve creation audit info for UPDATE mutations.
+    // TODO: Preserve creation audit info for UPDATE mutations.
+    // Issue URL: https://github.com/google/ground-android/issues/1562
     created = auditInfo,
     lastModified = auditInfo,
     geometry = geometry?.toLocalDataStoreObject(),
@@ -321,7 +322,8 @@ fun SubmissionMutation.toLocalDataStoreObject(created: AuditInfo): SubmissionEnt
     locationOfInterestId = this.locationOfInterestId,
     deletionState = EntityDeletionState.DEFAULT,
     data = SubmissionDataConverter.toString(SubmissionData().copyWithDeltas(this.deltas)),
-    // TODO(#1562): Preserve creation audit info for UPDATE mutations.
+    // TODO: Preserve creation audit info for UPDATE mutations.
+    // Issue URL: https://github.com/google/ground-android/issues/1562
     created = auditInfo,
     lastModified = auditInfo,
   )

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

@@ -61,7 +61,6 @@ internal object ValueJsonConverter {
   private fun toJsonArray(response: MultipleChoiceTaskData): JSONArray =
     JSONArray().apply { response.selectedOptionIds.forEach { this.put(it) } }
 
-  // TODO: Replace with proto conversion logic if this is still necessary
   fun toResponse(task: Task, obj: Any): TaskData? {
     if (JSONObject.NULL === obj) {
       return null

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

@@ -24,6 +24,7 @@ data class UserEntity(
   @ColumnInfo(name = "id") @PrimaryKey val id: String,
   @ColumnInfo(name = "email") val email: String,
   @ColumnInfo(name = "display_name") val displayName: String,
-  // TODO(https://github.com/google/ground-android/issues/964): Save to remote db
+  // TODO: Save to remote db
+  // Issue URL: https://github.com/google/ground-android/issues/964
   @ColumnInfo(name = "photo_url") val photoUrl: String?,
 )

ground/src/main/java/com/google/android/ground/persistence/local/room/fields/MutationEntitySyncStatus.kt

@@ -24,8 +24,9 @@ import com.google.android.ground.persistence.local.room.IntEnum.Companion.toInt
 /** Mutually exclusive mutations states. */
 enum class MutationEntitySyncStatus(private val intValue: Int, private val enumValue: SyncStatus) :
   IntEnum {
-  // TODO(#950): Set IN_PROGRESS and FAILED statuses when necessary. On failure, set retry count and
-  // error and update to PENDING.
+  // TODO: Set IN_PROGRESS and FAILED statuses
+  //  when necessary. On failure, set retry count and error and update to PENDING.
+  // Issue URL: https://github.com/google/ground-android/issues/950
   UNKNOWN(0, SyncStatus.UNKNOWN),
 
   /** Pending includes failed sync attempts pending retry. */

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

@@ -55,7 +55,8 @@ class RoomLocationOfInterestStore @Inject internal constructor() : LocalLocation
   ): LocationOfInterest? =
     locationOfInterestDao.findById(locationOfInterestId)?.toModelObject(survey)
 
-  // TODO(#706): Apply pending local mutations before saving.
+  // TODO: Apply pending local mutations before saving.
+  // Issue URL: https://github.com/google/ground-android/issues/706
   override suspend fun merge(model: LocationOfInterest) {
     locationOfInterestDao.insertOrUpdate(model.toLocalDataStoreObject())
   }

ground/src/main/java/com/google/android/ground/persistence/remote/DataStoreException.kt

@@ -29,7 +29,8 @@ open class DataStoreException(message: String?) : RuntimeException(message) {
     @JvmStatic
     @Throws(DataStoreException::class)
     fun <T : Any> checkType(expectedType: Class<*>, obj: T): T {
-      // TODO(#2743) - Handle Kotlin Long (java.lang.Long) vs Java primitive long (long)
+      // TODO: Handle Kotlin Long (java.lang.Long) vs Java primitive long (long)
+      // Issue URL: https://github.com/google/ground-android/issues/2743
       if (obj.javaClass == java.lang.Long::class.java && expectedType == Long::class.java) {
         return obj
       }

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

@@ -26,8 +26,6 @@ import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.tasks.await
 import timber.log.Timber
 
-// TODO: Add column to Submission table for storing uploaded media urls
-// TODO: Synced to remote db as well
 @Singleton
 class FirebaseStorageManager @Inject constructor() : RemoteStorageManager {
 
@@ -61,7 +59,6 @@ class FirebaseStorageManager @Inject constructor() : RemoteStorageManager {
      *
      * user-media/surveys/{survey_id}/submissions/{field_id-uuid.jpg}
      */
-    // TODO: Refactor this into MediaStorageRepository.
     fun getRemoteMediaPath(surveyId: String, filename: String): String =
       StringJoiner(File.separator)
         .add(MEDIA_ROOT_DIR)

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

@@ -64,7 +64,8 @@ internal constructor(
   override fun getSurveyList(user: User): Flow<List<SurveyListItem>> = flow {
     emitAll(
       db().surveys().getReadable(user).map { list ->
-        // TODO(#2031): Return SurveyListItem from getReadable(), only fetch required fields.
+        // TODO: Return SurveyListItem from getReadable(), only fetch required fields.
+        // Issue URL: https://github.com/google/ground-android/issues/2031
         list.map { it.toListItem(false) }
       }
     )

ground/src/main/java/com/google/android/ground/persistence/remote/firebase/base/FluentFirestore.kt

@@ -24,6 +24,5 @@ open class FluentFirestore protected constructor(private val db: FirebaseFiresto
 
   protected fun db(): FirebaseFirestore = db
 
-  // TODO: Wrap in fluent version of WriteBatch.
   fun batch(): WriteBatch = db.batch()
 }

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

@@ -144,5 +144,6 @@ private fun FirestoreValue.toMessageValue(targetType: KClass<*>): MessageValue =
       ?: throw IllegalArgumentException("Unrecognized enum number $this")
   } else {
     throw UnsupportedOperationException("Unsupported message field type $targetType")
-    // TODO(#1748): Handle arrays, GeoPoint, int, and other types.
+    // TODO: Handle arrays, GeoPoint, int, and other types.
+    // Issue URL: https://github.com/google/ground-android/issues/1748
   }

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

@@ -60,7 +60,6 @@ import com.google.protobuf.timestamp
 import java.util.Date
 import kotlinx.collections.immutable.toImmutableMap
 
-// TODO: Add test coverage
 fun SubmissionMutation.createSubmissionMessage(user: User) = submission {
   assert(userId == user.id) { "UserId doesn't match: expected $userId, found ${user.id}" }
 
@@ -127,7 +126,6 @@ fun LocationOfInterestMutation.createLoiMessage(user: User) = locationOfInterest
 }
 
 private fun toTaskData(id: String, newTaskData: TaskData) = taskData {
-  // TODO: What should be the ID?
   taskId = id
 
   when (newTaskData) {
@@ -147,7 +145,6 @@ private fun toTaskData(id: String, newTaskData: TaskData) = taskData {
         newTaskData.altitude?.let { altitude = it }
         newTaskData.accuracy?.let { accuracy = it }
         coordinates = newTaskData.location.coordinates.toMessage()
-        // TODO: Add timestamp
       }
     is GeometryTaskData -> drawGeometryResult = drawGeometryResult {
         geometry = newTaskData.geometry.toMessage()

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

@@ -67,7 +67,8 @@ private fun Message.hasValue(property: KProperty<*>): Boolean {
 }
 
 private fun MessageValue.toFirestoreValue(): FirestoreValue =
-  // TODO(#1748): Convert enums and other types.
+  // TODO: Convert enums and other types.
+  // Issue URL: https://github.com/google/ground-android/issues/1748
   when (this) {
     is List<*> -> map { it?.toFirestoreValue() }
     is Message -> toFirestoreMap()

ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/LoiCollectionReference.kt

@@ -43,7 +43,8 @@ class LoiCollectionReference internal constructor(ref: CollectionReference) :
   /** Retrieves all "predefined" LOIs in the specified survey. Main-safe. */
   suspend fun fetchPredefined(survey: Survey): List<LocationOfInterest> =
     // Use !=false rather than ==true to not break legacy dev surveys.
-    // TODO(#2375): Switch to whereEqualTo(true) once legacy dev surveys deleted or migrated.
+    // TODO: Switch to whereEqualTo(true) once legacy dev surveys deleted or migrated.
+    // Issue URL: https://github.com/google/ground-android/issues/2375
     fetchLois(
       survey,
       reference().whereEqualTo(SOURCE_FIELD, LocationOfInterestProto.Source.IMPORTED.number),

ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/LoiConverter.kt

@@ -24,10 +24,10 @@ import com.google.android.ground.proto.LocationOfInterest as LocationOfInterestP
 import com.google.android.ground.proto.LocationOfInterest.Source
 import com.google.firebase.firestore.DocumentSnapshot
 
-// TODO: Add tests.
 /** Converts between Firestore documents and [LocationOfInterest] instances. */
 object LoiConverter {
-  // TODO(#2392): Define field names on DocumentReference objects, not converters.
+  // TODO: Define field names on DocumentReference objects, not converters.
+  // Issue URL: https://github.com/google/ground-android/issues/2375
   const val GEOMETRY_TYPE = "type"
   const val POLYGON_TYPE = "Polygon"
 
@@ -71,7 +71,8 @@ object LoiConverter {
       job = job,
       created = created,
       lastModified = lastModified,
-      // TODO(#929): Set geometry once LOI has been updated to use our own model.
+      // TODO: Set geometry once LOI has been updated to use our own model.
+      // Issue URL: https://github.com/google/ground-android/issues/929
       geometry = geometry,
       submissionCount = submissionCount,
       properties = properties,

ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/SurveyDocumentReference.kt

@@ -39,8 +39,8 @@ class SurveyDocumentReference internal constructor(ref: DocumentReference) :
   suspend fun get(): Survey? {
     try {
       val surveyDoc = reference().get().await()
-      // TODO(https://github.com/google/ground-android/issues/2864): Move jobs fetch to outside this
-      // DocumentReference class.
+      // TODO: Move jobs fetch to outside this DocumentReference class.
+      // Issue URL: https://github.com/google/ground-android/issues/2864
       val jobs = jobs().get()
       return SurveyConverter.toSurvey(surveyDoc, jobs)
     } catch (e: CancellationException) {

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

@@ -69,8 +69,8 @@ constructor(
     try {
       val user = userRepository.getAuthenticatedUser()
       mutationRepository.markAsInProgress(mutations)
-      // TODO(https://github.com/google/ground-android/issues/2883):
-      //   Apply mutations via repository layer rather than accessing data store directly.
+      // TODO: Apply mutations via repository layer rather than accessing data store directly.
+      // Issue URL: https://github.com/google/ground-android/issues/2883
       remoteDataStore.applyMutations(mutations, user)
       mutationRepository.finalizePendingMutationsForMediaUpload(mutations)
 

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

@@ -76,8 +76,9 @@ constructor(
     } else {
       mutationRepository.markAsFailedMediaUpload(
         listOf(mutation),
-        // TODO(https://github.com/google/ground-android/issues/2120): Replace this workaround with
-        //   update of specific [MediaMutation], aggregate to [UploadQueueEntry] for display in UI.
+        // TODO: Replace this workaround with update of specific [MediaMutation],
+        //  aggregate to [UploadQueueEntry] for display in UI.
+        // Issue URL: https://github.com/google/ground-android/issues/2120
         results.firstNotNullOfOrNull { it.exceptionOrNull() } ?: UnknownError(),
       )
       return false