[Code health] Don't allow saving a duplicate vertex during polygon drawing session

app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskViewModel.kt

@@ -150,8 +150,6 @@ internal constructor(
       .stateIn(viewModelScope, WhileSubscribed(5_000), emptyList())
   }
 
-
-
   override fun initialize(
     job: Job,
     task: Task,
@@ -269,6 +267,9 @@ internal constructor(
     check(!sessionState.value.isMarkedComplete) {
       "Attempted to add last vertex after completing the drawing"
     }
+    if (sessionState.value.isTooClose) {
+      return
+    }
     val vertices = session.commitTentativeVertex(currentCameraTarget)
     syncSessionState()
     if (vertices != null) {
@@ -326,36 +327,37 @@ internal constructor(
    * This coroutine runs on [viewModelScope] to ensure lifecycle safety.
    */
   private fun refreshMap() = viewModelScope.launch {
-    if (session.vertices.isEmpty()) {
+    val vertices = session.displayVertices
+    if (vertices.isEmpty()) {
       _draftArea.emit(null)
       draftTag = null
     } else {
       if (draftTag == null) {
-        val feature = buildPolygonFeature()
+        val feature = buildPolygonFeature(vertices)
         draftTag = feature.tag
         _draftArea.emit(feature)
       } else {
-        val feature = buildPolygonFeature(id = draftTag!!.id)
+        val feature = buildPolygonFeature(vertices, id = draftTag!!.id)
         _draftUpdates.tryEmit(feature)
       }
     }
   }
 
-  private suspend fun buildPolygonFeature(id: String? = null) =
+  private suspend fun buildPolygonFeature(vertices: List<Coordinates>, id: String? = null) =
     Feature(
       id = id ?: uuidGenerator.generateUuid(),
       type = Feature.Type.USER_POLYGON,
-      geometry = LineString(session.vertices),
+      geometry = LineString(vertices),
       style = featureStyle,
       clusterable = false,
       selected = true,
-      tooltipText = getDistanceTooltipText(),
+      tooltipText = getDistanceTooltipText(vertices),
     )
 
   /** Returns the distance in meters between the last two vertices for displaying in the tooltip. */
-  private fun getDistanceTooltipText(): String? {
-    if (sessionState.value.isMarkedComplete || session.vertices.size <= 1) return null
-    val distance = session.vertices.penult().distanceTo(session.vertices.last())
+  private fun getDistanceTooltipText(vertices: List<Coordinates>): String? {
+    if (sessionState.value.isMarkedComplete || vertices.size <= 1) return null
+    val distance = vertices.penult().distanceTo(vertices.last())
     if (distance < TOOLTIP_MIN_DISTANCE_METERS) return null
     return localeAwareMeasureFormatter.formatDistance(distance, measurementUnits)
   }

app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSession.kt

@@ -28,6 +28,9 @@ interface PolygonDrawingSession {
   /** The list of committed vertices in the current drawing session. */
   val vertices: List<Coordinates>
 
+  /** The complete list of vertices (committed + tentative) to display on the map. */
+  val displayVertices: List<Coordinates>
+
   /** Whether the current polygon geometry self-intersects. */
   val hasSelfIntersection: Boolean
 

app/src/main/java/org/groundplatform/android/ui/datacollection/tasks/polygon/PolygonDrawingSessionImpl.kt

@@ -20,6 +20,7 @@ import kotlinx.collections.immutable.toImmutableList
 import org.groundplatform.android.ui.datacollection.tasks.polygon.PolygonDrawingSession.Companion.DISTANCE_THRESHOLD_DP
 import org.groundplatform.domain.model.geometry.Coordinates
 import org.groundplatform.domain.model.geometry.LineString
+import org.groundplatform.domain.util.isClosed
 import org.groundplatform.domain.util.isSelfIntersecting
 
 class PolygonDrawingSessionImpl : PolygonDrawingSession {
@@ -34,56 +35,57 @@ class PolygonDrawingSessionImpl : PolygonDrawingSession {
   override val vertices: List<Coordinates>
     get() = _vertices
 
+  private var _tentativeVertex: Coordinates? = null
+
+  override val displayVertices: List<Coordinates>
+    get() = _tentativeVertex?.let { _vertices + it } ?: _vertices
+
   @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
   internal val redoVertexStack = mutableListOf<Coordinates>()
 
   override val hasSelfIntersection: Boolean
-    get() = isSelfIntersecting(_vertices)
-
-  private fun addVertex(vertex: Coordinates, shouldOverwriteLastVertex: Boolean) {
-    val updatedVertices = _vertices.toMutableList()
-
-    if (shouldOverwriteLastVertex && updatedVertices.isNotEmpty()) {
-      updatedVertices.removeAt(updatedVertices.lastIndex)
-    }
-
-    updatedVertices.add(vertex)
-
-    _vertices = updatedVertices.toImmutableList()
-  }
+    get() = isSelfIntersecting(displayVertices)
 
   override fun setVertices(newVertices: List<Coordinates>) {
     _vertices = newVertices.toImmutableList()
+    _tentativeVertex = null
+    redoVertexStack.clear()
   }
 
   override fun updateTentativeVertex(
     target: Coordinates,
     calculateDistance: (Coordinates, Coordinates) -> Double,
   ) {
+    if (isClosed(_vertices)) {
+      _tentativeVertex = null
+      _isTooClose = true
+      return
+    }
+
     val firstVertex = _vertices.firstOrNull()
     var updatedTarget = target
-    if (firstVertex != null && _vertices.size > 2) {
+    if (firstVertex != null && _vertices.size >= 3) {
       val distance = calculateDistance(firstVertex, target)
 
       if (distance <= DISTANCE_THRESHOLD_DP) {
         updatedTarget = firstVertex
       }
     }
 
-    val prev = _vertices.dropLast(1).lastOrNull()
+    val lastCommitted = _vertices.lastOrNull()
     _isTooClose =
-      _vertices.size > 1 &&
-        prev?.let { calculateDistance(it, target) <= DISTANCE_THRESHOLD_DP } == true
+      lastCommitted?.let { calculateDistance(it, target) <= DISTANCE_THRESHOLD_DP } == true
 
-    addVertex(updatedTarget, true)
+    _tentativeVertex = updatedTarget
   }
 
   override fun commitTentativeVertex(currentCameraTarget: Coordinates?): List<Coordinates>? {
     redoVertexStack.clear()
-    val vertex = _vertices.lastOrNull() ?: currentCameraTarget
+    val vertex = _tentativeVertex ?: currentCameraTarget
     return vertex?.let {
-      _isTooClose = _vertices.size > 1
-      addVertex(it, false)
+      _vertices = (_vertices + it).toImmutableList()
+      _tentativeVertex = null
+      _isTooClose = true
       _vertices
     }
   }
@@ -97,20 +99,30 @@ class PolygonDrawingSessionImpl : PolygonDrawingSession {
   }
 
   override fun isValidPolygon(): Boolean =
-    LineString(_vertices).isClosed() && !isSelfIntersecting(_vertices)
+    LineString(displayVertices).isClosed() &&
+      !isSelfIntersecting(displayVertices) &&
+      displayVertices.distinct().size >= 3
 
   override fun complete() {
     check(isValidPolygon()) { "Polygon is not valid" }
     check(!_isMarkedComplete) { "Already marked complete" }
+    if (_tentativeVertex != null) {
+      commitTentativeVertex(null)
+    }
     _isMarkedComplete = true
   }
 
   override fun removeLastVertex(): Boolean {
-    if (_vertices.isEmpty()) return false
+    if (_tentativeVertex == null && _vertices.isEmpty()) return false
 
     _isMarkedComplete = false
-    redoVertexStack.add(_vertices.last())
-    _vertices = _vertices.dropLast(1).toImmutableList()
+    _tentativeVertex = null
+
+    if (_vertices.isNotEmpty()) {
+      redoVertexStack.add(_vertices.last())
+      _vertices = _vertices.dropLast(1).toImmutableList()
+    }
+
     return true
   }
 
@@ -120,6 +132,7 @@ class PolygonDrawingSessionImpl : PolygonDrawingSession {
     _isMarkedComplete = false
     val redoVertex = redoVertexStack.removeAt(redoVertexStack.lastIndex)
     _vertices = (_vertices + redoVertex).toImmutableList()
+    _tentativeVertex = null
     return redoVertex
   }
 

app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskScreenTest.kt

@@ -119,7 +119,6 @@ class DrawAreaTaskScreenTest : BaseHiltTest() {
               Coordinates(0.0, 0.0),
               Coordinates(10.0, 10.0),
               Coordinates(20.0, 20.0),
-              Coordinates(20.0, 20.0),
             )
           )
         )

app/src/test/java/org/groundplatform/android/ui/datacollection/tasks/polygon/DrawAreaTaskViewModelTest.kt

@@ -115,7 +115,7 @@ class DrawAreaTaskViewModelTest : BaseHiltTest() {
 
     viewModel.removeLastVertex()
 
-    assertGeometry(3, isLineString = true)
+    assertGeometry(2, isLineString = true)
   }
 
   @Test
@@ -199,11 +199,11 @@ class DrawAreaTaskViewModelTest : BaseHiltTest() {
 
     viewModel.removeLastVertex()
 
-    assertThat(featureTestObserver.value()?.tooltipText).isEqualTo("1,565,109 m")
+    assertThat(featureTestObserver.value()?.tooltipText).isNull()
 
     viewModel.removeLastVertex()
 
-    assertThat(featureTestObserver.value()?.tooltipText).isEqualTo(null)
+    assertThat(featureTestObserver.value()?.tooltipText).isNull()
   }
 
   @Test
@@ -320,23 +320,54 @@ class DrawAreaTaskViewModelTest : BaseHiltTest() {
     assertThat(firstLine.coordinates.size).isEqualTo(1)
     assertThat(firstFeature.tooltipText).isNull()
 
-    updateLastVertexAndAdd(COORDINATE_2)
-    advanceUntilIdle()
-    val secondFeature = viewModel.draftArea.first()
-    val secondLine = secondFeature!!.geometry as LineString
-    assertThat(secondLine.coordinates.size).isEqualTo(1)
-    assertThat(secondFeature.tooltipText).isNull()
+    viewModel.draftUpdates.test {
+      updateLastVertexAndAdd(COORDINATE_2)
+      advanceUntilIdle()
 
-    viewModel.removeLastVertex()
+      // 1. Consume the transient active target update (from updateLastVertex)
+      val secondFeature = awaitItem()
+      val secondLine = secondFeature.geometry as LineString
+      assertThat(secondLine.coordinates.size).isEqualTo(2)
+      assertThat(secondFeature.tooltipText).isEqualTo("1,565,109 m")
+
+      // 2. Consume the committed vertex update (from addLastVertex)
+      val committedFeature = awaitItem()
+      val committedLine = committedFeature.geometry as LineString
+      assertThat(committedLine.coordinates.size).isEqualTo(2)
+      assertThat(committedFeature.tooltipText).isEqualTo("1,565,109 m")
+
+      // 3. Revert/Undo last vertex
+      viewModel.removeLastVertex()
+      advanceUntilIdle()
+
+      // 4. Consume the undo update (from removeLastVertex)
+      val thirdFeature = awaitItem()
+      val thirdLine = thirdFeature.geometry as LineString
+      assertThat(thirdLine.coordinates.size).isEqualTo(1)
+      assertThat(thirdFeature.tooltipText).isNull()
+
+      cancelAndIgnoreRemainingEvents()
+    }
   }
 
   @Test
-  fun `isTooClose is false if only one vertex`() {
+  fun `isTooClose is true if only one vertex`() {
     setupViewModel()
     updateLastVertexAndAdd(COORDINATE_1)
 
     // Only 1 vertex.
-    assertThat(viewModel.sessionState.value.isTooClose).isFalse()
+    assertThat(viewModel.sessionState.value.isTooClose).isTrue()
+  }
+
+  @Test
+  fun `addLastVertex twice without moving camera only adds one vertex`() {
+    setupViewModel()
+    updateLastVertex(COORDINATE_1)
+    viewModel.addLastVertex()
+    viewModel.addLastVertex()
+
+    val draftGeometry = viewModel.draftArea.value?.geometry as? LineString
+    assertThat(draftGeometry?.coordinates).containsExactly(COORDINATE_1)
   }
 
   @Test
@@ -568,10 +599,10 @@ class DrawAreaTaskViewModelTest : BaseHiltTest() {
     updateLastVertexAndAdd(COORDINATE_2)
 
     viewModel.removeLastVertex()
-    assertGeometry(2, isLineString = true)
+    assertGeometry(1, isLineString = true)
 
     viewModel.redoLastVertex()
-    assertGeometry(3, isLineString = true)
+    assertGeometry(2, isLineString = true)
   }
 
   @Test