Merge branch 'master' of https://github.com/streetcomplete/StreetComp…

…lete

.github/workflows/build-debug-apk.yml

@@ -12,12 +12,12 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-    - uses: actions/checkout@v3
-    - name: set up JDK 11
+    - uses: actions/checkout@v4
+    - name: set up JDK 17
       uses: actions/setup-java@v3
       with:
-        java-version: '11'
-        distribution: 'adopt'
+        java-version: '17'
+        distribution: 'temurin'
 
     - name: Grant execute permission for gradlew
       run: chmod +x gradlew

app/src/main/java/de/westnordost/streetcomplete/osm/surface/SurfaceCreator.kt

@@ -16,7 +16,7 @@ fun SurfaceAndNote.applyTo(tags: Tags, prefix: String? = null, updateCheckDate:
     val key = "${pre}surface"
     val previousOsmValue = tags[key]
 
-    val shouldRemoveTracktype = prefix == null && isSurfaceAndTracktypeConflicting(osmValue, tags["tracktype"])
+    val shouldRemoveTracktype = prefix == null && isSurfaceAndTracktypeCombinationSuspicious(osmValue, tags["tracktype"])
     if (shouldRemoveTracktype) {
         tags.remove("tracktype")
         tags.removeCheckDatesForKey("tracktype")

app/src/main/java/de/westnordost/streetcomplete/osm/surface/SurfaceUtils.kt

@@ -12,11 +12,13 @@ val SOFT_SURFACES = setOf(
     "earth", "dirt", "soil", "grass", "sand", "mud", "ice", "salt", "snow", "woodchips"
 )
 
-val ANYTHING_UNPAVED = SOFT_SURFACES + setOf(
+val UNPAVED_BUT_NOT_ALWAYS_SOFT = setOf(
     "ground", // see https://community.openstreetmap.org/t/is-tracktype-grade2-also-for-trails-with-large-naturally-occuring-pieces-of-rock/96850
     "unpaved", "compacted", "gravel", "fine_gravel", "pebblestone", "grass_paver"
 )
 
+val ANYTHING_UNPAVED = SOFT_SURFACES + UNPAVED_BUT_NOT_ALWAYS_SOFT
+
 val ANYTHING_FULLY_PAVED = setOf(
     "paved", "asphalt", "cobblestone", "cobblestone:flattened", "sett",
     "concrete", "concrete:plates", "paving_stones",
@@ -41,6 +43,23 @@ val INVALID_SURFACES_FOR_TRACKTYPES = mapOf(
 fun isSurfaceAndTracktypeConflicting(surface: String, tracktype: String?): Boolean =
     INVALID_SURFACES_FOR_TRACKTYPES[tracktype]?.contains(surface) == true
 
+val EXPECTED_SURFACES_FOR_TRACKTYPES = mapOf(
+    "grade1" to ANYTHING_FULLY_PAVED,
+    "grade2" to UNPAVED_BUT_NOT_ALWAYS_SOFT,
+    "grade3" to ANYTHING_UNPAVED,
+    "grade4" to ANYTHING_UNPAVED,
+    "grade5" to SOFT_SURFACES,
+)
+
+/** @return whether the given tag value for [surface] likely contradicts the tag value for [tracktype].
+ *  E.g. surface=asphalt but tracktype=grade2.
+ *  some such combinations may be actually valid, so should not be assumed to be always be wrong
+ *  but if someone edits surface it is preferable to remove suspicious tracktype and trigger resurvey
+ *  see https://github.com/streetcomplete/StreetComplete/issues/5236
+ *  */
+fun isSurfaceAndTracktypeCombinationSuspicious(surface: String, tracktype: String?): Boolean =
+    tracktype != null && EXPECTED_SURFACES_FOR_TRACKTYPES[tracktype]?.contains(surface) != true
+
 /** Sets the common surface of the foot- and cycleway parts into the surface tag, if any. If the
  *  surfaces of the foot- and cycleway parts have nothing in common, removes the surface tag */
 fun updateCommonSurfaceFromFootAndCyclewaySurface(tags: Tags) {

app/src/main/java/de/westnordost/streetcomplete/quests/AListQuestForm.kt

@@ -19,6 +19,8 @@ abstract class AListQuestForm<T> : AbstractOsmQuestForm<T>() {
 
     private val radioButtonIds = HashMap<Int, TextItem<T>>()
 
+    val checkedItem get() = radioButtonIds[binding.radioButtonGroup.checkedRadioButtonId]
+
     override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
         super.onViewCreated(view, savedInstanceState)
         for (item in items) {
@@ -38,7 +40,7 @@ abstract class AListQuestForm<T> : AbstractOsmQuestForm<T>() {
     }
 
     override fun onClickOk() {
-        applyAnswer(radioButtonIds.getValue(binding.radioButtonGroup.checkedRadioButtonId).value)
+        applyAnswer(checkedItem!!.value)
     }
 
     override fun isFormComplete() = binding.radioButtonGroup.checkedRadioButtonId != -1

app/src/main/java/de/westnordost/streetcomplete/quests/bus_stop_ref/AddBusStopRef.kt

@@ -22,11 +22,12 @@ class AddBusStopRef : OsmFilterQuestType<BusStopRefAnswer>() {
         "CA",
         "IE",
         "JE",
-        "AU", // see https://github.com/streetcomplete/StreetComplete/issues/4487
-        "TR", // see https://github.com/streetcomplete/StreetComplete/issues/4489
+        "AU", // https://github.com/streetcomplete/StreetComplete/issues/4487
+        "TR", // https://github.com/streetcomplete/StreetComplete/issues/4489
         "US",
-        "IL", // see https://github.com/streetcomplete/StreetComplete/issues/5119
+        "IL", // https://github.com/streetcomplete/StreetComplete/issues/5119
         "CO", // https://github.com/streetcomplete/StreetComplete/issues/5124
+        "NZ", // https://wiki.openstreetmap.org/w/index.php?title=Talk:StreetComplete/Quests&oldid=2599288#Quests_in_New_Zealand
     )
     override val changesetComment = "Determine bus/tram stop refs"
     override val wikiLink = "Tag:public_transport=platform"

app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossing.kt

@@ -25,16 +25,10 @@ class AddCrossing : OsmElementQuestType<CrossingAnswer> {
     private val footwaysFilter by lazy { """
         ways with
           (highway ~ footway|steps or highway ~ path|cycleway and foot ~ designated|yes)
-          and footway !~ sidewalk|crossing
           and area != yes
           and access !~ private|no
     """.toElementFilterExpression() }
 
-    /* It is neither asked for sidewalks nor crossings (=separately mapped sidewalk infrastructure)
-    *  because a "no" answer would require to also delete/adapt the crossing ways, rather than just
-    *  tagging crossing=no on the vertex.
-    *  See https://github.com/streetcomplete/StreetComplete/pull/2999#discussion_r681516203 */
-
     override val changesetComment = "Specify whether there are crossings at intersections of paths and roads"
     override val wikiLink = "Tag:highway=crossing"
     override val icon = R.drawable.ic_quest_pedestrian

app/src/main/java/de/westnordost/streetcomplete/quests/crossing/AddCrossingForm.kt

@@ -1,15 +1,48 @@
 package de.westnordost.streetcomplete.quests.crossing
 
+import androidx.appcompat.app.AlertDialog
 import de.westnordost.streetcomplete.R
+import de.westnordost.streetcomplete.data.osm.edits.MapDataWithEditsSource
 import de.westnordost.streetcomplete.quests.AListQuestForm
 import de.westnordost.streetcomplete.quests.TextItem
 import de.westnordost.streetcomplete.quests.crossing.CrossingAnswer.*
+import org.koin.android.ext.android.inject
 
 class AddCrossingForm : AListQuestForm<CrossingAnswer>() {
+    private val mapDataSource: MapDataWithEditsSource by inject()
 
     override val items = listOf(
         TextItem(YES, R.string.quest_crossing_yes),
         TextItem(NO, R.string.quest_crossing_no),
         TextItem(PROHIBITED, R.string.quest_crossing_prohibited),
     )
+
+    /*  PROHIBITED is not possible for sidewalks or crossings (=separately mapped sidewalk
+        infrastructure) because if the crossing does not exist, it would require to also
+        delete/adapt the crossing ways, rather than just tagging crossing=no on the vertex.
+
+        This situation needs to be solved in a different editor, so we ask the user to leave a note.
+        See https://github.com/streetcomplete/StreetComplete/pull/2999#discussion_r681516203
+        and https://github.com/streetcomplete/StreetComplete/issues/5160
+
+        NO on the other hand would be okay because crossing=informal would not require deleting
+        the crossing ways (I would say... it is in edge case...)
+        */
+    override fun onClickOk() {
+        if (checkedItem?.value == PROHIBITED && isOnSidewalkOrCrossing()) {
+            AlertDialog.Builder(requireContext())
+                .setMessage(R.string.quest_leave_new_note_as_answer)
+                .setPositiveButton(R.string.quest_leave_new_note_yes) { _, _ -> composeNote() }
+                .setNegativeButton(android.R.string.cancel, null)
+                .show()
+        } else {
+            super.onClickOk()
+        }
+    }
+
+    private fun isOnSidewalkOrCrossing(): Boolean =
+        mapDataSource.getWaysForNode(element.id).any {
+            val footway = it.tags["footway"]
+            footway == "sidewalk" || footway == "crossing"
+        }
 }

app/src/main/java/de/westnordost/streetcomplete/quests/drinking_water_type/AddDrinkingWaterType.kt

@@ -26,7 +26,7 @@ class AddDrinkingWaterType : OsmFilterQuestType<DrinkingWaterType>() {
     override val isDeleteElementEnabled = true
     override val achievements = listOf(CITIZEN, OUTDOORS)
 
-    override fun getTitle(tags: Map<String, String>) = R.string.quest_drinking_water_type_title
+    override fun getTitle(tags: Map<String, String>) = R.string.quest_drinking_water_type_title2
 
     override fun createForm() = AddDrinkingWaterTypeForm()
 

app/src/main/java/de/westnordost/streetcomplete/quests/postbox_royal_cypher/AddPostboxRoyalCypher.kt

@@ -19,10 +19,11 @@ class AddPostboxRoyalCypher : OsmFilterQuestType<PostboxRoyalCypher>() {
     override val isDeleteElementEnabled = true
     override val achievements = listOf(POSTMAN)
     override val enabledInCountries = NoCountriesExcept(
-        // United Kingdom and some former nations of the British Empire, members of the Commonwealth of Nations and British overseas territories etc
-        "GB", "GI", "CY", "HK", "MT", "NZ", "LK",
+        // United Kingdom and some former nations of the British Empire, members of the Commonwealth of Nations and British overseas territories etc.
+        "GB", "GI", "CY", "HK", "MT", "LK",
         // territories with agency postal services provided by the British Post Office
         "KW", "BH", "MA"
+        // Not New Zealand: https://wiki.openstreetmap.org/w/index.php?title=Talk:StreetComplete/Quests&oldid=2599288#Quests_in_New_Zealand
     )
 
     override fun getTitle(tags: Map<String, String>) = R.string.quest_postboxRoyalCypher_title

app/src/main/java/de/westnordost/streetcomplete/screens/settings/questselection/QuestPresetsAdapter.kt

@@ -141,6 +141,7 @@ class QuestPresetsAdapter(
             val dialog = EditTextDialog(ctx,
                 title = ctx.getString(R.string.quest_presets_rename),
                 text = preset.name,
+                hint = ctx.getString(R.string.quest_presets_preset_name),
                 callback = { name -> renameQuestPreset(preset.id, name) }
             )
             dialog.editText.filters = arrayOf(InputFilter.LengthFilter(60))
@@ -157,6 +158,7 @@ class QuestPresetsAdapter(
             val dialog = EditTextDialog(ctx,
                 title = ctx.getString(R.string.quest_presets_duplicate),
                 text = preset.name,
+                hint = ctx.getString(R.string.quest_presets_preset_name),
                 callback = { name -> duplicateQuestPreset(preset.id, name) }
             )
             dialog.editText.filters = arrayOf(InputFilter.LengthFilter(60))
@@ -168,6 +170,7 @@ class QuestPresetsAdapter(
                 val newPresetId = questPresetsController.add(name)
                 questTypeOrderController.copyOrders(presetId, newPresetId)
                 visibleQuestTypeController.copyVisibilities(presetId, newPresetId)
+                questPresetsController.selectedId = newPresetId
             }
         }
 

app/src/main/java/de/westnordost/streetcomplete/screens/settings/questselection/QuestPresetsFragment.kt

@@ -43,16 +43,17 @@ class QuestPresetsFragment : TwoPaneDetailFragment(R.layout.fragment_quest_prese
         val ctx = context ?: return
         val dialog = EditTextDialog(ctx,
             title = ctx.getString(R.string.quest_presets_preset_add),
+            hint = ctx.getString(R.string.quest_presets_preset_name),
             callback = { name -> addQuestPreset(name) }
         )
-        dialog.editText.hint = ctx.getString(R.string.quest_presets_preset_name)
         dialog.editText.filters = arrayOf(InputFilter.LengthFilter(60))
         dialog.show()
     }
 
     private fun addQuestPreset(name: String) {
         viewLifecycleScope.launch(Dispatchers.IO) {
-            questPresetsController.add(name)
+            val newPresetId = questPresetsController.add(name)
+            questPresetsController.selectedId = newPresetId
         }
     }
 }

app/src/main/java/de/westnordost/streetcomplete/view/dialogs/EditTextDialog.kt

@@ -9,12 +9,14 @@ import androidx.appcompat.app.AlertDialog
 import androidx.core.widget.doAfterTextChanged
 import de.westnordost.streetcomplete.R
 import de.westnordost.streetcomplete.util.ktx.nonBlankTextOrNull
+import de.westnordost.streetcomplete.util.ktx.showKeyboard
 
 /** A dialog in which you input a text */
 class EditTextDialog(
     context: Context,
     title: CharSequence? = null,
     text: String? = null,
+    hint: String? = null,
     @LayoutRes layoutResId: Int = R.layout.dialog_edit_text,
     private val callback: (value: String) -> Unit
 ) : AlertDialog(context) {
@@ -28,6 +30,7 @@ class EditTextDialog(
 
         editText = view.findViewById(R.id.editText)
         editText.setText(text)
+        editText.hint = hint
         editText.doAfterTextChanged {
             updateEditButtonEnablement()
         }
@@ -46,6 +49,13 @@ class EditTextDialog(
         updateEditButtonEnablement()
     }
 
+    override fun onWindowFocusChanged(hasWindowFocus: Boolean) {
+        if (hasWindowFocus) {
+            editText.requestFocus()
+            editText.showKeyboard()
+        }
+    }
+
     private fun updateEditButtonEnablement() {
         getButton(BUTTON_POSITIVE)?.isEnabled = editText.nonBlankTextOrNull != null
     }

app/src/main/res/values/strings.xml

@@ -131,6 +131,8 @@ The info you enter is directly added to OpenStreetMap in your name, without the
     <string name="quest_leave_new_note_description">"You can leave a public note for other mappers to resolve at this location, or hide this quest for yourself only"</string>
     <string name="quest_leave_new_note_yes">"Leave note"</string>
     <string name="quest_leave_new_note_no">"Hide"</string>
+    <string name="quest_leave_new_note_as_answer">In this case, you need to leave a note in which you explain the situation.</string>
+
 
     <string name="quest_generic_otherAnswers">"Other answers…"</string>
     <!-- aka "not applicable", "not answerable" -->
@@ -997,7 +999,7 @@ Before uploading your changes, the app checks with a &lt;a href=\"https://www.we
     <string name="quest_drinking_water_not_potable_signed">Not safe to drink (signed)</string>
     <string name="quest_drinking_water_not_potable_unsigned">Not safe to drink (unsigned)</string>
 
-    <string name="quest_drinking_water_type_title">How is drinking water provided here?</string>
+    <string name="quest_drinking_water_type_title2">Is drinking water available here? If so, what is its source?</string>
     <string name="quest_drinking_water_type_generic_water_fountain">Other water fountain</string>
     <string name="quest_drinking_water_type_jet_water_fountain">Water fountain emitting small jet of water</string>
     <string name="quest_drinking_water_type_bottle_refill_only_fountain">Refilling bottles only</string>

app/src/test/java/de/westnordost/streetcomplete/osm/surface/SurfaceUtilsKtTest.kt

@@ -11,22 +11,42 @@ class SurfaceUtilsKtTest {
 
     @Test fun `poor tracktype conflicts with paved surface`() {
         assertTrue(isSurfaceAndTracktypeConflicting("asphalt", "grade5"))
+        assertTrue(isSurfaceAndTracktypeCombinationSuspicious("asphalt", "grade5"))
     }
 
     @Test fun `high quality tracktype conflicts with poor surface`() {
         assertTrue(isSurfaceAndTracktypeConflicting("gravel", "grade1"))
+        assertTrue(isSurfaceAndTracktypeCombinationSuspicious("gravel", "grade1"))
     }
 
     @Test fun `high quality tracktype fits good surface`() {
         assertFalse(isSurfaceAndTracktypeConflicting("paving_stones", "grade1"))
+        assertFalse(isSurfaceAndTracktypeCombinationSuspicious("paving_stones", "grade1"))
     }
 
     @Test fun `unknown tracktype does not crash or conflict`() {
         assertFalse(isSurfaceAndTracktypeConflicting("paving_stones", "lorem ipsum"))
+        assertTrue(isSurfaceAndTracktypeCombinationSuspicious("paving_stones", "lorem ipsum"))
     }
 
     @Test fun `unknown surface does not crash or conflict`() {
         assertFalse(isSurfaceAndTracktypeConflicting("zażółć", "grade1"))
+        assertTrue(isSurfaceAndTracktypeCombinationSuspicious("zażółć", "grade1"))
+    }
+
+    @Test fun `lower tracktype on paved is suspicious but not always conflicting`() {
+        assertFalse(isSurfaceAndTracktypeConflicting("paving_stones", "grade2"))
+        assertTrue(isSurfaceAndTracktypeCombinationSuspicious("paving_stones", "grade2"))
+    }
+
+    @Test fun `sand surface is conflicting and suspicious on tracktype=grade2`() {
+        assertTrue(isSurfaceAndTracktypeConflicting("sand", "grade2"))
+        assertTrue(isSurfaceAndTracktypeCombinationSuspicious("sand", "grade2"))
+    }
+
+    @Test fun `missing tracktype is not conflicting`() {
+        assertFalse(isSurfaceAndTracktypeConflicting("paving_stones", null))
+        assertFalse(isSurfaceAndTracktypeCombinationSuspicious("paving_stones", null))
     }
 
     @Test fun `update foot and cycleway with identical surface`() {

app/src/test/java/de/westnordost/streetcomplete/quests/surface/AddRoadSurfaceTest.kt

@@ -1,5 +1,10 @@
 package de.westnordost.streetcomplete.quests.surface
 
+import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapEntryAdd
+import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapEntryDelete
+import de.westnordost.streetcomplete.osm.surface.Surface
+import de.westnordost.streetcomplete.osm.surface.SurfaceAndNote
+import de.westnordost.streetcomplete.quests.verifyAnswer
 import de.westnordost.streetcomplete.testutils.way
 import kotlin.test.Test
 import kotlin.test.assertFalse
@@ -42,4 +47,38 @@ class AddRoadSurfaceTest {
     private fun assertIsNotApplicable(vararg pairs: Pair<String, String>) {
         assertFalse(questType.isApplicableTo(way(nodes = listOf(1, 2, 3), tags = mapOf(*pairs))))
     }
+
+    @Test fun `not applicable where very poor tracktype and surface match is suspicious, but not conflicting`() {
+        assertIsNotApplicable("highway" to "track", "surface" to "gravel", "tracktype" to "grade5")
+    }
+
+    @Test fun `not applicable where tracktype and very good surface match is suspicious, but not conflicting`() {
+        assertIsNotApplicable("highway" to "track", "surface" to "asphalt", "tracktype" to "grade2")
+    }
+
+    @Test fun `tracktype tag is removed when surface match is suspicious`() {
+        questType.verifyAnswer(
+            mapOf(
+                "tracktype" to "grade2",
+                "smoothness" to "good"
+            ),
+            SurfaceAndNote(Surface.ASPHALT),
+            StringMapEntryAdd("surface", "asphalt"),
+            StringMapEntryDelete("tracktype", "grade2"),
+            StringMapEntryDelete("smoothness", "good"),
+        )
+    }
+
+    @Test fun `tracktype tag is removed when surface match is conflicting`() {
+        questType.verifyAnswer(
+            mapOf(
+                "tracktype" to "grade2",
+                "smoothness" to "good"
+            ),
+            SurfaceAndNote(Surface.SAND),
+            StringMapEntryAdd("surface", "sand"),
+            StringMapEntryDelete("tracktype", "grade2"),
+            StringMapEntryDelete("smoothness", "good"),
+        )
+    }
 }