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

Reoccuring Surface quest cannot be solved when splitting that way #4520

Closed
Strubbl opened this issue Oct 17, 2022 · 9 comments
Closed

Reoccuring Surface quest cannot be solved when splitting that way #4520

Strubbl opened this issue Oct 17, 2022 · 9 comments

Comments

@Strubbl
Copy link
Contributor

Strubbl commented Oct 17, 2022

How to Reproduce

  1. Select a quest which asks for the surface of a way
  2. This quest must be a reoccuring one. It means the way has already a surface tagged, but after x years this is asked a again to confirm or update the way
  3. Use the further option to split the way, because it has different surfaces
  4. Split the way according the different surfaces and confirm the split
  5. The surface quest is gone now

Expected Behavior
The surface quest shall be asked for all splitted parts of the way and shall not disappear.

Versions affected
48.0-beta1, but this behaviour is older than this version. I do not know the first version when this happened.

@Strubbl Strubbl added the bug label Oct 17, 2022
@Strubbl Strubbl changed the title splitting a way during a reoccuring Surface quest cannot be solved A reoccuring Surface quest cannot be solved when splitting a way Oct 17, 2022
@Strubbl Strubbl changed the title A reoccuring Surface quest cannot be solved when splitting a way Reoccuring Surface quest cannot be solved when splitting that way Oct 17, 2022
@Helium314
Copy link
Collaborator

see #3567
This has been around for some time, probably since the start or resurvey quests.

@mnalis
Copy link
Member

mnalis commented Oct 17, 2022

After rereading those, it still seems to me that it should be solvable in SC. Sure, uploading check_date:surface to OSM as suggested in #2883 (comment) woud be quite kludgy, but SC should be able to keep that state internally without any such changes being sent to OSM (e.g. remembering that both parts of the split way should be eligible to all quests, e.g. making all surface older today -12 years etc. succeed on ways that are internally marked as being split.)

It is a matter of how hard (and/or kludgy) such functionality would be to implement. As someone who meddled with such code recently @Helium314, do you think it would be possible to add something like boolean wasRecentlySplit = false to downloaded elements (and setting it to true when way is split, and using it later when deciding weather to show a quest)?

@Strubbl
Copy link
Contributor Author

Strubbl commented Oct 17, 2022

Sorry, that i opened a topic which was already closed as wontfix. I did not find it before. If there is no easy solution and the status is still wontfix, please feel free to close this issue.

@Helium314
Copy link
Collaborator

do you think it would be possible to add something like boolean wasRecentlySplit = false to downloaded elements (and setting it to true when way is split, and using it later when deciding weather to show a quest)?

You could check ElementEditsDao.getForElement for each dowloaded element, and wasRecentlySplit = edits.any { it.action is SplitWayAction }. Just please don't do this on a bbox download for performance reasons.
Any actually it might be better to do this check when creating quests for a single element (OsmQuestController.createQuestsForElementDeferred).

@mnalis
Copy link
Member

mnalis commented Oct 19, 2022

This also seems to be fixed by #4523, right @Helium314 ?

@Helium314
Copy link
Collaborator

Helium314 commented Oct 20, 2022

Is it? I didn't try it with auto-upload switched on, but I expect after upload at best the "old" segment with the original id could maybe be re-surveyed because of #4472

@matkoniecz
Copy link
Member

This also seems to be fixed by #4523, right @Helium314 ?

Only with upload set to manual.

@mnalis
Copy link
Member

mnalis commented Oct 20, 2022

Yes, I only tried it with manual upload (on latest master + PR4523), and it worked there for both segments.

@westnordost westnordost added duplicate and removed bug labels Oct 21, 2022
@westnordost
Copy link
Member

Duplicate of #3567

@westnordost westnordost marked this as a duplicate of #3567 Oct 21, 2022
@westnordost westnordost closed this as not planned Won't fix, can't repro, duplicate, stale Oct 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants