Merge pull request #4615 from mnalis/update-same-surface2

Update surface to generic paved if footway and cycleway parts are different but paved
streetcomplete · Dec 3, 2022 · a4b353e · a4b353e
2 parents 54e6ff1 + 7c890ac
commit a4b353e
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 20 deletions.
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/surface/AddCyclewayPartSurface.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/surface/AddCyclewayPartSurface.kt
@@ -38,12 +38,6 @@ class AddCyclewayPartSurface : OsmFilterQuestType<SurfaceAnswer>() {
 
     override fun applyAnswerTo(answer: SurfaceAnswer, tags: Tags, timestampEdited: Long) {
         answer.applyTo(tags, "cycleway")
-        if (tags["cycleway:surface"] != null && tags["footway:surface"] != null) {
-            if (tags["footway:surface"] == tags["cycleway:surface"]) {
-                tags["surface"] = tags["cycleway:surface"]!!
-            } else {
-                tags.remove("surface")
-            }
-        }
+        answer.updateSegregatedFootAndCycleway(tags)
     }
 }
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/surface/AddFootwayPartSurface.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/surface/AddFootwayPartSurface.kt
@@ -39,12 +39,6 @@ class AddFootwayPartSurface : OsmFilterQuestType<SurfaceAnswer>() {
 
     override fun applyAnswerTo(answer: SurfaceAnswer, tags: Tags, timestampEdited: Long) {
         answer.applyTo(tags, "footway")
-        if (tags["cycleway:surface"] != null && tags["footway:surface"] != null) {
-            if (tags["footway:surface"] == tags["cycleway:surface"]) {
-                tags["surface"] = tags["footway:surface"]!!
-            } else {
-                tags.remove("surface")
-            }
-        }
+        answer.updateSegregatedFootAndCycleway(tags)
     }
 }
diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/surface/SurfaceAnswer.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/surface/SurfaceAnswer.kt
@@ -1,5 +1,6 @@
 package de.westnordost.streetcomplete.quests.surface
 
+import de.westnordost.streetcomplete.osm.ANYTHING_FULLY_PAVED
 import de.westnordost.streetcomplete.osm.Tags
 import de.westnordost.streetcomplete.osm.isSurfaceAndTracktypeMismatching
 import de.westnordost.streetcomplete.osm.removeCheckDatesForKey
@@ -34,6 +35,7 @@ fun SurfaceAnswer.applyTo(tags: Tags, prefix: String? = null) {
         tags.remove("$key:grade")
         tags.remove("${pre}smoothness")
         tags.remove("${pre}smoothness:date")
+        tags.remove("source:smoothness")
         tags.removeCheckDatesForKey("${pre}smoothness")
     }
 
@@ -49,3 +51,32 @@ fun SurfaceAnswer.applyTo(tags: Tags, prefix: String? = null) {
     // clean up old source tags - source should be in changeset tags
     tags.remove("source:$key")
 }
+
+fun SurfaceAnswer.updateSegregatedFootAndCycleway(tags: Tags) {
+    val footwaySurface = tags["footway:surface"]
+    val cyclewaySurface = tags["cycleway:surface"]
+    if (cyclewaySurface != null && footwaySurface != null) {
+        val commonSurface = when {
+            footwaySurface == cyclewaySurface -> this
+            footwaySurface in ANYTHING_FULLY_PAVED && cyclewaySurface in ANYTHING_FULLY_PAVED -> SurfaceAnswer(Surface.PAVED_ROAD)
+            else -> null
+        }
+        if (commonSurface != null) {
+            commonSurface.applyTo(tags)
+        } else {
+            removeSurface(tags)
+        }
+    }
+}
+
+private fun removeSurface(tags: Tags) {
+    tags.remove("surface")
+    tags.remove("surface:note")
+    tags.remove("source:surface")
+    tags.removeCheckDatesForKey("surface")
+    tags.remove("surface:grade")
+    tags.remove("smoothness")
+    tags.remove("smoothness:date")
+    tags.remove("source:smoothness")
+    tags.removeCheckDatesForKey("smoothness")
+}
diff --git a/app/src/test/java/de/westnordost/streetcomplete/quests/surface/AddCyclewayPartSurfaceTest.kt b/app/src/test/java/de/westnordost/streetcomplete/quests/surface/AddCyclewayPartSurfaceTest.kt
@@ -109,13 +109,31 @@ class AddCyclewayPartSurfaceTest {
 
     @Test fun `smoothness tag removed when cycleway surface changes`() {
         questType.verifyAnswer(
-            mapOf("cycleway:surface" to "asphalt", "cycleway:smoothness" to "excellent"),
+            mapOf(
+                "footway:surface" to "gravel",
+                "cycleway:surface" to "asphalt",
+                "cycleway:smoothness" to "intermediate"
+            ),
             SurfaceAnswer(Surface.PAVING_STONES),
-            StringMapEntryDelete("cycleway:smoothness", "excellent"),
+            StringMapEntryDelete("cycleway:smoothness", "intermediate"),
             StringMapEntryModify("cycleway:surface", "asphalt", "paving_stones")
         )
     }
 
+    @Test fun `smoothness tag not removed when surface did not change`() {
+        questType.verifyAnswer(
+            mapOf(
+                "footway:surface" to "paving_stones",
+                "surface" to "paving_stones",
+                "smoothness" to "good"
+            ),
+            SurfaceAnswer(Surface.PAVING_STONES),
+            StringMapEntryAdd("cycleway:surface", "paving_stones"),
+            StringMapEntryModify("surface", "paving_stones", "paving_stones"),
+            StringMapEntryAdd("check_date:surface", nowAsCheckDateString()),
+        )
+    }
+
     @Test fun `cycleway surface changes`() {
         questType.verifyAnswer(
             mapOf("cycleway:surface" to "asphalt"),
@@ -149,12 +167,25 @@ class AddCyclewayPartSurfaceTest {
         )
     }
 
+    @Test fun `surface changes to generic paved when similar for footway and cycleway`() {
+        questType.verifyAnswer(
+            mapOf(
+                "surface" to "paving_stones",
+                "cycleway:surface" to "paving_stones",
+                "footway:surface" to "asphalt",
+            ),
+            SurfaceAnswer(Surface.CONCRETE),
+            StringMapEntryModify("cycleway:surface", "paving_stones", "concrete"),
+            StringMapEntryModify("surface", "paving_stones", "paved")
+        )
+    }
+
     @Test fun `surface removed when different for footway and cycleway`() {
         questType.verifyAnswer(
             mapOf(
                 "surface" to "paving_stones",
                 "cycleway:surface" to "paving_stones",
-                "footway:surface" to "paving_stones",
+                "footway:surface" to "gravel",
             ),
             SurfaceAnswer(Surface.CONCRETE),
             StringMapEntryModify("cycleway:surface", "paving_stones", "concrete"),

diff --git a/app/src/test/java/de/westnordost/streetcomplete/quests/surface/AddFootwayPartSurfaceTest.kt b/app/src/test/java/de/westnordost/streetcomplete/quests/surface/AddFootwayPartSurfaceTest.kt
@@ -4,6 +4,7 @@ import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapEntryAd
 import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapEntryDelete
 import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapEntryModify
 import de.westnordost.streetcomplete.osm.nowAsCheckDateString
+import de.westnordost.streetcomplete.osm.nowAsCheckDateString
 import de.westnordost.streetcomplete.quests.TestMapDataWithGeometry
 import de.westnordost.streetcomplete.quests.verifyAnswer
 import de.westnordost.streetcomplete.testutils.way
@@ -110,13 +111,31 @@ class AddFootwayPartSurfaceTest {
 
     @Test fun `smoothness tag removed when footway surface changes`() {
         questType.verifyAnswer(
-            mapOf("footway:surface" to "asphalt", "footway:smoothness" to "excellent"),
+            mapOf(
+                "footway:surface" to "asphalt",
+                "cycleway:surface" to "gravel",
+                "footway:smoothness" to "intermediate"
+            ),
             SurfaceAnswer(Surface.PAVING_STONES),
-            StringMapEntryDelete("footway:smoothness", "excellent"),
+            StringMapEntryDelete("footway:smoothness", "intermediate"),
             StringMapEntryModify("footway:surface", "asphalt", "paving_stones")
         )
     }
 
+    @Test fun `smoothness tag not removed when surface did not change`() {
+        questType.verifyAnswer(
+            mapOf(
+                "cycleway:surface" to "paving_stones",
+                "surface" to "paving_stones",
+                "smoothness" to "good"
+            ),
+            SurfaceAnswer(Surface.PAVING_STONES),
+            StringMapEntryAdd("footway:surface", "paving_stones"),
+            StringMapEntryModify("surface", "paving_stones", "paving_stones"),
+            StringMapEntryAdd("check_date:surface", nowAsCheckDateString()),
+        )
+    }
+
     @Test fun `footway surface changes`() {
         questType.verifyAnswer(
             mapOf("footway:surface" to "asphalt"),
@@ -150,11 +169,24 @@ class AddFootwayPartSurfaceTest {
         )
     }
 
+    @Test fun `surface changes to generic paved when similar for footway and cycleway`() {
+        questType.verifyAnswer(
+            mapOf(
+                "surface" to "paving_stones",
+                "footway:surface" to "paving_stones",
+                "cycleway:surface" to "asphalt",
+            ),
+            SurfaceAnswer(Surface.CONCRETE),
+            StringMapEntryModify("footway:surface", "paving_stones", "concrete"),
+            StringMapEntryModify("surface", "paving_stones", "paved")
+        )
+    }
+
     @Test fun `surface removed when different for footway and cycleway`() {
         questType.verifyAnswer(
             mapOf(
                 "surface" to "paving_stones",
-                "cycleway:surface" to "paving_stones",
+                "cycleway:surface" to "gravel",
                 "footway:surface" to "paving_stones",
             ),
             SurfaceAnswer(Surface.CONCRETE),