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.

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

SC asks for opening_hours check for disused:shop #3718

Closed
Strubbl opened this issue Feb 4, 2022 · 13 comments · Fixed by #3719
Closed

SC asks for opening_hours check for disused:shop #3718

Strubbl opened this issue Feb 4, 2022 · 13 comments · Fixed by #3719
Assignees
Labels
bug feedback required more info is needed, issue will be likely closed if it is not provided

Comments

@Strubbl
Copy link
Contributor

Strubbl commented Feb 4, 2022

SC asks for opening_hours check for vacant shops. This makes no sense, because the shop is closed forever.

How to Reproduce

  • Have a former shop node with disused:shop key and also the tag opening_hours with a meaningful value (the value of the former shop)
  • SC asks to check the opening_hours although the node is already marked as vacant

I am not sure about the desired behavior of the app. Maybe it should not even ask for a opening hours check or it could help to remove that tagging.

Versions affected

v40.0-beta1

@Strubbl Strubbl added the bug label Feb 4, 2022
@matkoniecz
Copy link
Member

Can you link a specific object where it happened?

How this object was described in a question?

I am betting that OSM data is somehow wrong there.

@matkoniecz matkoniecz added the feedback required more info is needed, issue will be likely closed if it is not provided label Feb 4, 2022
@Helium314
Copy link
Collaborator

see #2712

@Strubbl
Copy link
Contributor Author

Strubbl commented Feb 4, 2022

So yes, it's an error in the OSM data: disused:shop, which has opening:hours. Don't we want to fix this with SC?

@matkoniecz
Copy link
Member

I think it would be fine to skip such objects and require old opening hours to match the same criteria as new opening hours ( https://github.com/streetcomplete/StreetComplete/blob/master/app/src/main/java/de/westnordost/streetcomplete/quests/opening_hours/AddOpeningHours.kt#L98 )

Possible loss is that SC would not ask anymore about opening hours on say gates with old opening hours.

@matkoniecz matkoniecz self-assigned this Feb 4, 2022
matkoniecz added a commit to matkoniecz/Zazolc that referenced this issue Feb 4, 2022
fixes streetcomplete#3718 and fixes streetcomplete#2712

now old opening hours will be asked only when on objects eligible for new opening hours
and for some additional limited cases when opening hours are tagged already and become old
such as parks, barriers, toilets
@peternewman
Copy link
Collaborator

If this is fixed the same thing wants applying to CheckOpeningHoursSigned too I think.

@matkoniecz
Copy link
Member

If this is fixed the same thing wants applying to CheckOpeningHoursSigned too I think.

answered in PR

@westnordost
Copy link
Member

Have a former shop node with disused:shop key and also the tag opening_hours with a meaningful value (the value of the former shop)

@Strubbl How is this meaningful and not a tagging mistake? A disused shop has no opening hours.

@Strubbl
Copy link
Contributor Author

Strubbl commented Feb 5, 2022

Of course this is a tagging mistake. I thought SC might want to detect that it is asking for checking opening hours for a disused shop and finally don't ask and maybe ask to open a note or fix that data issue.

@Strubbl
Copy link
Contributor Author

Strubbl commented Feb 6, 2022

I just searched for an example node, which has wrong data: https://www.openstreetmap.org/node/3663837425
If you go there with SC, then you hide the question regarding "is this shop still vacant", i get the opening hours question although it is a disused:shop.
Now, in the opening hours quest, i say "does not exist". It is only possible to open a note now. I cannot mark it as vacant.

@matkoniecz
Copy link
Member

If you go there with SC, then you hide the question regarding "is this shop still vacant"

Yes, the problem happens only when someone either

  • disables "is this shop still vacant" quest
  • moves quest order so OH quest is above vacant quest
  • hides specific "is this shop still vacant" quest instead of answering it

@Strubbl
Copy link
Contributor Author

Strubbl commented Feb 6, 2022

In my case, because i opened the issue, the opening hours quest occured at first. In my opinion the quest order was okay and i did not see the "is this shop still vacant" quest. Unfortunately i cannot reproduce it anymore since i corrected the node. Please see version 8 of https://www.openstreetmap.org/node/4294027197/history

So, maybe it is because of check_date:opening_hours

@matkoniecz
Copy link
Member

i did not see the "is this shop still vacant" quest

Weird. Maybe you hid it earlier and forgot? Or there is another path to that?

Or you reordered/disable quest types and forgot about it? (can you check quest order and enablement in settings?)

@Helium314
Copy link
Collaborator

i did not see the "is this shop still vacant" quest

Weird. Maybe you hid it earlier and forgot? Or there is another path to that?

Or you reordered/disable quest types and forgot about it? (can you check quest order and enablement in settings?)

Before the fix a week ago, the shop was and last edited (and closed) 5 months ago, and has no general check_date or similar (only one for opening hours). So even with more frequent resurveys, "is this shop still vacant" will not be asked.

But check_date:opening_hours is very clear here, older than 6 months, and is preferred over the last edit date. So opening hours will be asked. (well, maybe not any more after the recent PR)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug feedback required more info is needed, issue will be likely closed if it is not provided
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants