diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/opening_hours/AddOpeningHours.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/opening_hours/AddOpeningHours.kt index 7ae4a3867a..504c4821d1 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/opening_hours/AddOpeningHours.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/opening_hours/AddOpeningHours.kt @@ -14,6 +14,7 @@ import de.westnordost.streetcomplete.data.user.achievements.EditTypeAchievement. import de.westnordost.streetcomplete.osm.Tags import de.westnordost.streetcomplete.osm.isPlaceOrDisusedPlace import de.westnordost.streetcomplete.osm.opening_hours.parser.isSupportedOpeningHours +import de.westnordost.streetcomplete.osm.removeCheckDatesForKey import de.westnordost.streetcomplete.osm.updateCheckDateForKey import de.westnordost.streetcomplete.osm.updateWithCheckDate @@ -116,7 +117,7 @@ class AddOpeningHours( or barrier or amenity ~ toilets|bicycle_rental ) - and opening_hours:signed != no + and (opening_hours:signed != no or (opening_hours:signed = no and opening_hours:signed older today -1 years)) """).toElementFilterExpression() } override val changesetComment = "Survey opening hours" @@ -166,7 +167,8 @@ class AddOpeningHours( override fun applyAnswerTo(answer: OpeningHoursAnswer, tags: Tags, geometry: ElementGeometry, timestampEdited: Long) { if (answer is NoOpeningHoursSign) { tags["opening_hours:signed"] = "no" - tags.updateCheckDateForKey("opening_hours") + tags.updateCheckDateForKey("opening_hours:signed") + tags.removeCheckDatesForKey("opening_hours") // don't delete current opening hours: these may be the correct hours, they are just not visible anywhere on the door } else { val openingHoursString = when (answer) { @@ -178,6 +180,7 @@ class AddOpeningHours( tags.updateWithCheckDate("opening_hours", openingHoursString) if (tags["opening_hours:signed"] == "no") { tags.remove("opening_hours:signed") + tags.removeCheckDatesForKey("opening_hours:signed") } } tags.remove("opening_hours:covid19") diff --git a/app/src/main/java/de/westnordost/streetcomplete/quests/opening_hours_signed/CheckOpeningHoursSigned.kt b/app/src/main/java/de/westnordost/streetcomplete/quests/opening_hours_signed/CheckOpeningHoursSigned.kt index b4d8c002dd..5afa6710e7 100644 --- a/app/src/main/java/de/westnordost/streetcomplete/quests/opening_hours_signed/CheckOpeningHoursSigned.kt +++ b/app/src/main/java/de/westnordost/streetcomplete/quests/opening_hours_signed/CheckOpeningHoursSigned.kt @@ -11,6 +11,7 @@ import de.westnordost.streetcomplete.data.user.achievements.EditTypeAchievement. import de.westnordost.streetcomplete.osm.Tags import de.westnordost.streetcomplete.osm.getLastCheckDateKeys import de.westnordost.streetcomplete.osm.isPlaceOrDisusedPlace +import de.westnordost.streetcomplete.osm.removeCheckDatesForKey import de.westnordost.streetcomplete.osm.setCheckDateForKey import de.westnordost.streetcomplete.osm.toCheckDate import de.westnordost.streetcomplete.osm.updateCheckDateForKey @@ -28,6 +29,8 @@ class CheckOpeningHoursSigned( opening_hours:signed = no and ( $hasOldOpeningHoursCheckDateFilter + or + $hasOldOpeningHoursSignedCheckDateFilter or older today -1 years ) and access !~ private|no @@ -42,6 +45,11 @@ class CheckOpeningHoursSigned( "$it < today -1 years" } + private val hasOldOpeningHoursSignedCheckDateFilter: String get() = + getLastCheckDateKeys("opening_hours:signed").joinToString("\nor ") { + "$it < today -1 years" + } + override val changesetComment = "Survey whether opening hours are signed" override val wikiLink = "Key:opening_hours:signed" override val icon = R.drawable.ic_quest_opening_hours_signed @@ -64,6 +72,7 @@ class CheckOpeningHoursSigned( override fun applyAnswerTo(answer: Boolean, tags: Tags, geometry: ElementGeometry, timestampEdited: Long) { if (answer) { tags.remove("opening_hours:signed") + tags.removeCheckDatesForKey("opening_hours:signed") /* it is now signed: we set the check date for the opening hours to the previous edit timestamp because this or an older date is the date the opening hours were last checked. This is set so that the app will ask about the (signed) opening hours in @@ -80,7 +89,7 @@ class CheckOpeningHoursSigned( } else { tags["opening_hours:signed"] = "no" // still unsigned: just set the check date to now, user was on-site - tags.updateCheckDateForKey("opening_hours") + tags.updateCheckDateForKey("opening_hours:signed") } } diff --git a/app/src/test/java/de/westnordost/streetcomplete/quests/opening_hours/AddOpeningHoursTest.kt b/app/src/test/java/de/westnordost/streetcomplete/quests/opening_hours/AddOpeningHoursTest.kt index fad735f172..ff0d27531d 100644 --- a/app/src/test/java/de/westnordost/streetcomplete/quests/opening_hours/AddOpeningHoursTest.kt +++ b/app/src/test/java/de/westnordost/streetcomplete/quests/opening_hours/AddOpeningHoursTest.kt @@ -6,10 +6,12 @@ import de.westnordost.osm_opening_hours.model.Range import de.westnordost.osm_opening_hours.model.Rule import de.westnordost.osm_opening_hours.model.TimeSpan import de.westnordost.osm_opening_hours.model.Weekday +import de.westnordost.streetcomplete.data.elementfilter.filters.RelativeDate import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapEntryAdd import de.westnordost.streetcomplete.data.osm.edits.update_tags.StringMapEntryModify import de.westnordost.streetcomplete.osm.nowAsCheckDateString import de.westnordost.streetcomplete.osm.toCheckDate +import de.westnordost.streetcomplete.osm.toCheckDateString import de.westnordost.streetcomplete.quests.answerApplied import de.westnordost.streetcomplete.quests.answerAppliedTo import de.westnordost.streetcomplete.testutils.mock @@ -61,7 +63,7 @@ class AddOpeningHoursTest { assertEquals( setOf( StringMapEntryAdd("opening_hours:signed", "no"), - StringMapEntryAdd("check_date:opening_hours", nowAsCheckDateString()) + StringMapEntryAdd("check_date:opening_hours:signed", nowAsCheckDateString()) ), questType.answerApplied(NoOpeningHoursSign) ) @@ -71,7 +73,7 @@ class AddOpeningHoursTest { assertEquals( setOf( StringMapEntryAdd("opening_hours:signed", "no"), - StringMapEntryAdd("check_date:opening_hours", nowAsCheckDateString()) + StringMapEntryAdd("check_date:opening_hours:signed", nowAsCheckDateString()) ), questType.answerAppliedTo( NoOpeningHoursSign, @@ -84,7 +86,7 @@ class AddOpeningHoursTest { assertEquals( setOf( StringMapEntryAdd("opening_hours:signed", "no"), - StringMapEntryAdd("check_date:opening_hours", nowAsCheckDateString()) + StringMapEntryAdd("check_date:opening_hours:signed", nowAsCheckDateString()) ), questType.answerAppliedTo(NoOpeningHoursSign, mapOf("opening_hours" to "24/7")) ) @@ -279,14 +281,63 @@ class AddOpeningHoursTest { ))) } - @Test fun `isApplicableTo returns false if the opening hours are not signed`() { + @Test fun `isApplicableTo returns false if the opening hours are not signed and signed check date is 1 year old`() { + assertFalse(questType.isApplicableTo(node( + tags = mapOf( + "shop" to "supermarket", + "name" to "Supi", + "opening_hours:signed" to "no", + "check_date:opening_hours:signed" to RelativeDate(-365f).date.toCheckDateString() + ), + timestamp = RelativeDate(-365f).date.toCheckDateString().toCheckDate()?.toEpochMilli() + ))) + } + + @Test fun `isApplicableTo returns false if the opening hours check date is older than 1 year, while they are not signed and signed check date is less than 1 year old`() { + assertFalse(questType.isApplicableTo(node( + tags = mapOf( + "shop" to "supermarket", + "name" to "Supi", + "opening_hours:signed" to "no", + "check_date:opening_hours" to RelativeDate(-420f).date.toCheckDateString(), + "check_date:opening_hours:signed" to RelativeDate(-180f).date.toCheckDateString() + ), + timestamp = RelativeDate(-365f).date.toCheckDateString().toCheckDate()?.toEpochMilli() + ))) + } + + @Test fun `isApplicableTo returns false if the opening hours check date is older than 1 year, while they are not signed and no signed check date exists`() { assertFalse(questType.isApplicableTo(node( + tags = mapOf( + "shop" to "supermarket", + "name" to "Supi", + "opening_hours:signed" to "no", + "check_date:opening_hours" to RelativeDate(-420f).date.toCheckDateString(), + ), + timestamp = RelativeDate(-365f).date.toCheckDateString().toCheckDate()?.toEpochMilli() + ))) + } + + @Test fun `isApplicableTo returns true if the last edit is older than 1 year and no check dates exists`() { + assertTrue(questType.isApplicableTo(node( tags = mapOf( "shop" to "supermarket", "name" to "Supi", "opening_hours:signed" to "no" ), - timestamp = "2000-11-11".toCheckDate()?.toEpochMilli() + timestamp = RelativeDate(-420f).date.toCheckDateString().toCheckDate()?.toEpochMilli() + ))) + } + + @Test fun `isApplicableTo returns true if the opening hours are not signed and signed check date is older than 1 year`() { + assertTrue(questType.isApplicableTo(node( + tags = mapOf( + "shop" to "supermarket", + "name" to "Supi", + "opening_hours:signed" to "no", + "check_date:opening_hours:signed" to RelativeDate(-420f).date.toCheckDateString() + ), + timestamp = RelativeDate(-365f).date.toCheckDateString().toCheckDate()?.toEpochMilli() ))) } diff --git a/app/src/test/java/de/westnordost/streetcomplete/quests/opening_hours_signed/CheckOpeningHoursSignedTest.kt b/app/src/test/java/de/westnordost/streetcomplete/quests/opening_hours_signed/CheckOpeningHoursSignedTest.kt index 732e056150..e52a38bab4 100644 --- a/app/src/test/java/de/westnordost/streetcomplete/quests/opening_hours_signed/CheckOpeningHoursSignedTest.kt +++ b/app/src/test/java/de/westnordost/streetcomplete/quests/opening_hours_signed/CheckOpeningHoursSignedTest.kt @@ -4,9 +4,11 @@ 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.toCheckDate import de.westnordost.streetcomplete.quests.answerAppliedTo import de.westnordost.streetcomplete.testutils.mock import de.westnordost.streetcomplete.testutils.node +import de.westnordost.streetcomplete.util.ktx.toEpochMilli import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -32,7 +34,7 @@ class CheckOpeningHoursSignedTest { @Test fun `is applicable to place with old check_date`() { assertTrue(questType.isApplicableTo(node(tags = mapOf( "name" to "XYZ", - "check_date:opening_hours" to "2020-12-12", + "check_date:opening_hours:signed" to "2020-12-12", "opening_hours:signed" to "no" )))) } @@ -40,7 +42,7 @@ class CheckOpeningHoursSignedTest { @Test fun `is not applicable to place with new check_date`() { assertFalse(questType.isApplicableTo(node(tags = mapOf( "name" to "XYZ", - "check_date:opening_hours" to nowAsCheckDateString(), + "check_date:opening_hours:signed" to nowAsCheckDateString(), "opening_hours:signed" to "no" )))) } @@ -67,6 +69,15 @@ class CheckOpeningHoursSignedTest { "opening_hours:signed" to "yes" )))) } + @Test fun `is applicable if the opening hours not signed and only check date for OH exists`() { + assertTrue(questType.isApplicableTo(node( + tags = mapOf( + "name" to "rundumdieuhr kiosk", + "opening_hours:signed" to "no", + "check_date:opening_hours" to "2021-03-01" + ) + ))) + } @Test fun `apply yes answer with no prior check date`() { assertEquals( @@ -109,13 +120,17 @@ class CheckOpeningHoursSignedTest { @Test fun `apply yes answer with prior check date and existing opening hours`() { assertEquals( - setOf(StringMapEntryDelete("opening_hours:signed", "no")), + setOf( + StringMapEntryDelete("opening_hours:signed", "no"), + StringMapEntryDelete("check_date:opening_hours:signed", "2020-03-04"), + StringMapEntryAdd("check_date:opening_hours", "1970-01-01") + ), questType.answerAppliedTo( true, mapOf( "opening_hours" to "\"oh\"", "opening_hours:signed" to "no", - "check_date:opening_hours" to "2020-03-04" + "check_date:opening_hours:signed" to "2020-03-04" ), ) ) @@ -125,7 +140,7 @@ class CheckOpeningHoursSignedTest { assertEquals( setOf( StringMapEntryModify("opening_hours:signed", "no", "no"), - StringMapEntryAdd("check_date:opening_hours", nowAsCheckDateString()), + StringMapEntryAdd("check_date:opening_hours:signed", nowAsCheckDateString()), ), questType.answerAppliedTo( false, @@ -138,13 +153,13 @@ class CheckOpeningHoursSignedTest { assertEquals( setOf( StringMapEntryModify("opening_hours:signed", "no", "no"), - StringMapEntryModify("check_date:opening_hours", "2020-03-04", nowAsCheckDateString()), + StringMapEntryModify("check_date:opening_hours:signed", "2020-03-04", nowAsCheckDateString()), ), questType.answerAppliedTo( false, mapOf( "opening_hours:signed" to "no", - "check_date:opening_hours" to "2020-03-04" + "check_date:opening_hours:signed" to "2020-03-04" ) ) ) @@ -154,7 +169,7 @@ class CheckOpeningHoursSignedTest { assertEquals( setOf( StringMapEntryModify("opening_hours:signed", "no", "no"), - StringMapEntryAdd("check_date:opening_hours", nowAsCheckDateString()), + StringMapEntryAdd("check_date:opening_hours:signed", nowAsCheckDateString()), ), questType.answerAppliedTo( false, @@ -170,14 +185,14 @@ class CheckOpeningHoursSignedTest { assertEquals( setOf( StringMapEntryModify("opening_hours:signed", "no", "no"), - StringMapEntryModify("check_date:opening_hours", "2020-03-04", nowAsCheckDateString()), + StringMapEntryModify("check_date:opening_hours:signed", "2020-03-04", nowAsCheckDateString()), ), questType.answerAppliedTo( false, mapOf( "opening_hours" to "Mo 10:00-12:00", "opening_hours:signed" to "no", - "check_date:opening_hours" to "2020-03-04" + "check_date:opening_hours:signed" to "2020-03-04" ) ) )