Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Check Date for opening_hours:signed #5903

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Check Date for opening_hours:signed #5903

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2e9ddf1
Check Date for opening_hours:signed - v1
kmpoppe Sep 16, 2024
0d3626f
Update AddOpeningHoursTest.kt
kmpoppe Sep 16, 2024
ea10d1f
Update CheckOpeningHoursSignedTest.kt
kmpoppe Sep 18, 2024
752a0cd
Update CheckSignedFilter to include both checkdate, updated tests
kmpoppe Oct 2, 2024
2942958
Update AddOpeningHours.kt
kmpoppe Oct 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions app/src/main/java/de/westnordost/streetcomplete/quests/opening_hours/AddOpeningHours.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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))
kmpoppe marked this conversation as resolved.
Show resolved Hide resolved
""").toElementFilterExpression() }

override val changesetComment = "Survey opening hours"
Expand Down Expand Up @@ -166,7 +167,7 @@ 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")
westnordost marked this conversation as resolved.
Show resolved Hide resolved
// 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) {
Expand All @@ -178,6 +179,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")
Expand Down
10 changes: 6 additions & 4 deletions ...java/de/westnordost/streetcomplete/quests/opening_hours_signed/CheckOpeningHoursSigned.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -27,7 +28,7 @@ class CheckOpeningHoursSigned(
nodes, ways with
opening_hours:signed = no
and (
$hasOldOpeningHoursCheckDateFilter
$hasOldOpeningHoursSignedCheckDateFilter
westnordost marked this conversation as resolved.
Show resolved Hide resolved
or older today -1 years
)
and access !~ private|no
Expand All @@ -37,8 +38,8 @@ class CheckOpeningHoursSigned(
)
""".toElementFilterExpression() }

private val hasOldOpeningHoursCheckDateFilter: String get() =
getLastCheckDateKeys("opening_hours").joinToString("\nor ") {
private val hasOldOpeningHoursSignedCheckDateFilter: String get() =
getLastCheckDateKeys("opening_hours:signed").joinToString("\nor ") {
"$it < today -1 years"
}

Expand All @@ -64,6 +65,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
Expand All @@ -80,7 +82,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")
}
}

Expand Down
27 changes: 21 additions & 6 deletions app/src/test/java/de/westnordost/streetcomplete/quests/opening_hours/AddOpeningHoursTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
)
Expand All @@ -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,
Expand All @@ -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"))
)
Expand Down Expand Up @@ -279,14 +281,27 @@ 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"
"opening_hours:signed" to "no",
"check_date:opening_hours:signed" to RelativeDate(-365f).date.toCheckDateString()
),
timestamp = "2000-11-11".toCheckDate()?.toEpochMilli()
timestamp = RelativeDate(-365f).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()
)))
}

Expand Down
24 changes: 14 additions & 10 deletions .../de/westnordost/streetcomplete/quests/opening_hours_signed/CheckOpeningHoursSignedTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,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"
))))
}
Expand Down Expand Up @@ -109,13 +109,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"
),
)
)
Expand All @@ -125,7 +129,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,
Expand All @@ -138,13 +142,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"
)
)
)
Expand All @@ -154,7 +158,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,
Expand All @@ -170,14 +174,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"
)
)
)
Expand Down