From a791c6d4d79e3134f810c4dd9f3728623046edcf Mon Sep 17 00:00:00 2001 From: Tobias Zwick Date: Thu, 15 Aug 2024 22:04:49 +0200 Subject: [PATCH] tag disused:oldkey=oldvalue when selecting that something is vacant now (fixes #5548) --- .../westnordost/streetcomplete/osm/Place.kt | 19 ++++++++++ .../overlays/places/PlacesOverlayForm.kt | 7 +++- .../quests/AbstractOsmQuestForm.kt | 2 +- .../quests/shop_type/CheckShopExistence.kt | 2 +- .../quests/shop_type/CheckShopType.kt | 2 +- .../quests/shop_type/ShopGoneDialog.kt | 14 ++++--- .../quests/shop_type/SpecifyShopType.kt | 2 +- .../quests/shop_type/SpecifyShopTypeTest.kt | 37 ++++++++++++++++++- 8 files changed, 73 insertions(+), 12 deletions(-) diff --git a/app/src/main/java/de/westnordost/streetcomplete/osm/Place.kt b/app/src/main/java/de/westnordost/streetcomplete/osm/Place.kt index 3bc9123bec..28aea1ca51 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/osm/Place.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/osm/Place.kt @@ -202,6 +202,25 @@ private val IS_PLACE_EXPRESSION by lazy { """.toElementFilterExpression() } +/** Get tags to denote the element with the given [tags] as disused */ +fun getDisusedPlaceTags(tags: Map?): Map { + val (key, value) = tags?.entries?.find { it.key in placeTypeKeys }?.toPair() ?: ("shop" to "yes") + return mapOf("disused:$key" to value) +} + +private val placeTypeKeys = setOf( + "amenity", + "club", + "craft", + "emergency", + "healthcare", + "leisure", + "office", + "military", + "shop", + "tourism" +) + /** Expression to see if an element is some kind of vacant shop */ private val IS_VACANT_PLACE_EXPRESSION = """ nodes, ways, relations with diff --git a/app/src/main/java/de/westnordost/streetcomplete/overlays/places/PlacesOverlayForm.kt b/app/src/main/java/de/westnordost/streetcomplete/overlays/places/PlacesOverlayForm.kt index 997b844e71..88cff74e53 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/overlays/places/PlacesOverlayForm.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/overlays/places/PlacesOverlayForm.kt @@ -21,6 +21,7 @@ import de.westnordost.streetcomplete.databinding.FragmentOverlayPlacesBinding import de.westnordost.streetcomplete.osm.LocalizedName import de.westnordost.streetcomplete.osm.POPULAR_PLACE_FEATURE_IDS import de.westnordost.streetcomplete.osm.applyTo +import de.westnordost.streetcomplete.osm.getDisusedPlaceTags import de.westnordost.streetcomplete.osm.isDisusedPlace import de.westnordost.streetcomplete.osm.isPlace import de.westnordost.streetcomplete.osm.parseLocalizedNames @@ -295,7 +296,11 @@ private suspend fun createEditAction( } if (doReplaceShop) { - tagChanges.replacePlace(newFeature.addTags) + if (isVacant) { + tagChanges.replacePlace(getDisusedPlaceTags(element?.tags)) + } else { + tagChanges.replacePlace(newFeature.addTags) + } } else { for ((key, value) in previousFeature?.removeTags.orEmpty()) { tagChanges.remove(key) diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/AbstractOsmQuestForm.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/AbstractOsmQuestForm.kt index e4f7f21f0a..7ad6bb5e7d 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/AbstractOsmQuestForm.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/AbstractOsmQuestForm.kt @@ -256,7 +256,7 @@ abstract class AbstractOsmQuestForm : AbstractQuestForm(), IsShowingQuestDeta if (element.isPlaceOrDisusedPlace()) { ShopGoneDialog( requireContext(), - element.geometryType, + element, countryOrSubdivisionCode, featureDictionary, onSelectedFeature = this::onShopReplacementSelected, diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/shop_type/CheckShopExistence.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/shop_type/CheckShopExistence.kt index 0cd09cd14a..ee782693c5 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/shop_type/CheckShopExistence.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/shop_type/CheckShopExistence.kt @@ -37,7 +37,7 @@ class CheckShopExistence( and (name or brand or noname = yes or name:signed = no) """).toElementFilterExpression() } - override val changesetComment = "Survey if places (shops and other shop-like) still exist" + override val changesetComment = "Survey if places still exist" override val wikiLink = "Key:disused:" override val icon = R.drawable.ic_quest_check_shop override val achievements = listOf(CITIZEN) diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/shop_type/CheckShopType.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/shop_type/CheckShopType.kt index a21df92a50..aceae70b48 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/shop_type/CheckShopType.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/shop_type/CheckShopType.kt @@ -39,7 +39,7 @@ class CheckShopType : OsmElementQuestType { ) """.toElementFilterExpression() } - override val changesetComment = "Survey if vacant shops are still vacant" + override val changesetComment = "Survey if vacant places are still vacant" override val wikiLink = "Key:disused:" override val icon = R.drawable.ic_quest_check_shop override val achievements = listOf(CITIZEN) diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/shop_type/ShopGoneDialog.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/shop_type/ShopGoneDialog.kt index 64f8e32db1..c9745a3af4 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/shop_type/ShopGoneDialog.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/shop_type/ShopGoneDialog.kt @@ -8,20 +8,22 @@ import android.widget.RadioButton import androidx.appcompat.app.AlertDialog import de.westnordost.osmfeatures.Feature import de.westnordost.osmfeatures.FeatureDictionary -import de.westnordost.osmfeatures.GeometryType import de.westnordost.streetcomplete.R +import de.westnordost.streetcomplete.data.osm.mapdata.Element import de.westnordost.streetcomplete.data.osm.mapdata.LatLon import de.westnordost.streetcomplete.data.osm.mapdata.Node import de.westnordost.streetcomplete.databinding.DialogShopGoneBinding import de.westnordost.streetcomplete.databinding.ViewShopTypeBinding import de.westnordost.streetcomplete.osm.POPULAR_PLACE_FEATURE_IDS +import de.westnordost.streetcomplete.osm.getDisusedPlaceTags import de.westnordost.streetcomplete.osm.isPlace +import de.westnordost.streetcomplete.util.ktx.geometryType import de.westnordost.streetcomplete.view.controller.FeatureViewController import de.westnordost.streetcomplete.view.dialogs.SearchFeaturesDialog class ShopGoneDialog( context: Context, - private val geometryType: GeometryType?, + private val element: Element, private val countryCode: String?, private val featureDictionary: FeatureDictionary, private val onSelectedFeature: (Map) -> Unit, @@ -52,10 +54,10 @@ class ShopGoneDialog( SearchFeaturesDialog( context, featureDictionary, - geometryType, + element.geometryType, countryCode, featureCtrl.feature?.name, - ::filterOnlyShops, + ::filterOnlyPlaces, ::onSelectedFeature, POPULAR_PLACE_FEATURE_IDS, true @@ -74,7 +76,7 @@ class ShopGoneDialog( updateOkButtonEnablement() } - private fun filterOnlyShops(feature: Feature): Boolean { + private fun filterOnlyPlaces(feature: Feature): Boolean { val fakeElement = Node(-1L, LatLon(0.0, 0.0), feature.tags, 0) return fakeElement.isPlace() } @@ -89,7 +91,7 @@ class ShopGoneDialog( // to override the default OK=dismiss() behavior getButton(DialogInterface.BUTTON_POSITIVE).setOnClickListener { when (selectedRadioButtonId) { - R.id.vacantRadioButton -> onSelectedFeature(mapOf("disused:shop" to "yes")) + R.id.vacantRadioButton -> onSelectedFeature(getDisusedPlaceTags(element.tags)) R.id.replaceRadioButton -> onSelectedFeature(featureCtrl.feature!!.addTags) R.id.leaveNoteRadioButton -> onLeaveNote() } diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/shop_type/SpecifyShopType.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/shop_type/SpecifyShopType.kt index 309e44e820..19716a5879 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/shop_type/SpecifyShopType.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/shop_type/SpecifyShopType.kt @@ -47,8 +47,8 @@ class SpecifyShopType : OsmFilterQuestType() { tags.removeCheckDates() when (answer) { is IsShopVacant -> { + tags["disused:shop"] = tags["shop"] ?: "yes" tags.remove("shop") - tags["disused:shop"] = "yes" } is ShopType -> { tags.remove("disused:shop") diff --git a/app/src/test/java/de/westnordost/streetcomplete/quests/shop_type/SpecifyShopTypeTest.kt b/app/src/test/java/de/westnordost/streetcomplete/quests/shop_type/SpecifyShopTypeTest.kt index befe6ce607..6f8fd646a1 100644 --- a/app/src/test/java/de/westnordost/streetcomplete/quests/shop_type/SpecifyShopTypeTest.kt +++ b/app/src/test/java/de/westnordost/streetcomplete/quests/shop_type/SpecifyShopTypeTest.kt @@ -1,14 +1,20 @@ package de.westnordost.streetcomplete.quests.shop_type +import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapEntryAdd +import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapEntryDelete +import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapEntryModify +import de.westnordost.streetcomplete.quests.answerApplied +import de.westnordost.streetcomplete.quests.answerAppliedTo import de.westnordost.streetcomplete.testutils.node import kotlin.test.Test +import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertTrue class SpecifyShopTypeTest { private val questType = SpecifyShopType() - @Test fun `is applicable to undefine shop`() { + @Test fun `is applicable to undefined shop`() { assertTrue(questType.isApplicableTo( node(tags = mapOf("shop" to "yes")) )) @@ -25,4 +31,33 @@ class SpecifyShopTypeTest { node(tags = mapOf("power" to "plant", "shop" to "yes")) )) } + + @Test fun `mentions old value in disused tag`() { + assertEquals( + setOf( + StringMapEntryAdd("disused:shop", "supermarket"), + StringMapEntryDelete("shop", "supermarket") + ), + questType.answerAppliedTo( + IsShopVacant, + mapOf("shop" to "supermarket") + ) + ) + assertEquals( + setOf( + StringMapEntryAdd("disused:amenity", "cafe"), + StringMapEntryDelete("amenity", "cafe") + ), + questType.answerAppliedTo( + IsShopVacant, + mapOf("amenity" to "cafe") + ) + ) + assertEquals( + setOf( + StringMapEntryAdd("disused:shop", "yes"), + ), + questType.answerApplied(IsShopVacant) + ) + } }