Check Date for opening_hours:signed

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")

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")
         }
     }
 

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()
         )))
     }
 

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,15 +34,15 @@ 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"
         ))))
     }
 
     @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"
                 )
             )
         )