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

MR tasks in bounding box is missing task property information, mustache tags are not displayed correctly #955

Closed
govvin opened this issue Oct 21, 2019 · 24 comments

Comments

@govvin
Copy link

govvin commented Oct 21, 2019

This issue was discovered while using Vespucci, and was reported here

The Vespucci developer writes:

OK the problem seems to be that if you retrieve the tasks per bounding box you don't get the per-task property information. Matter of fact the data returned is significantly different than for an individual task query. Go figure.

@nrotstan
Copy link
Contributor

This issue is very specific to the API and probably should be opened on the MapRoulette API repo. Off the top of my head, I believe that particular API endpoint is considered a search function and so only returns a summary of matching tasks instead of the full task data.

Mustache tags are currently parsed and interpreted by the client, not the server, so other clients would need to do that as well if the same functionality is desired.

Maybe some discussion of what all is needed by Vespucci would be warranted.

@mvexel
Copy link
Member

mvexel commented Oct 21, 2019

This could also be relevant to openstreetmap/iD#5856 (comment). External applications that would like to incorporate a 'MapRoulette Task Layer' would ideally need only one call that gives them enough information to show a meaningful layer. @simonpoole @quincylvania what bits of information would you need?

@simonpoole
Copy link

@mvexel yes, definitely it is going to be very painful to add an API call per task to retrieve the missing data (in the case at hand it is mainly the properties data). While we could -in principle- delay this to when the task is being displayed, there is no guarantee that we actually have inet connectivity at that point in time.

@govvin
Copy link
Author

govvin commented Oct 22, 2019

@mvexel yes, definitely it is going to be very painful to add an API call per task to retrieve the missing data (in the case at hand it is mainly the properties data). While we could -in principle- delay this to when the task is being displayed, there is no guarantee that we actually have inet connectivity at that point in time.

@simonpoole I love how Vespucci is also great offline!

Perhaps a desirable behavior in this case is that basic, offline information is accessible when a marker is clicked, and the user is given the option to retrieve additional information when they click another button?

@mvexel
Copy link
Member

mvexel commented May 18, 2022

Realizing this is still open, and the linked issue on the Vespucci side as well, but with no newer comments.

Is there still work to be done here on the MapRoulette side?

@simonpoole
Copy link

Realizing this is still open, and the linked issue on the Vespucci side as well, but with no newer comments.

Is there still work to be done here on the MapRoulette side?

Well as you say, there has been no progress on the issue(s) as that is dependent on changing the MR api.

Note that better integration of MR also depends on maproulette/maproulette-backend#476

@mvexel
Copy link
Member

mvexel commented May 19, 2022

@simonpoole so if I understand this correctly, and apologies for letting this sit for so long, you would need to have a task metadata bbox call that would include expanded task instructions (with the {{mustache}} tags replaced by the values as they would be displayed by the client?

If that could be satisfied and we could somehow solve maproulette/maproulette-backend#476, would that remove all barriers to full support for displaying MR tasks in Vespucci?

@simonpoole
Copy link

@mvexel given that the last time I looked at the MR API is a couple of years back, I'm assuming things may have changed and that it would be a good idea to give the current implementation a look before making a statement. But I didn't find a pointer to the API doc from any of the current MR pages (there are a lot of links to supplementary documentation so I likely simply missed it).

@mvexel
Copy link
Member

mvexel commented May 21, 2022

@simonpoole You can find the Swagger API docs here.

@simonpoole
Copy link

Does anybody have a pointer to a current challenge that utilizes the mustache placeholders?

@govvin
Copy link
Author

govvin commented Jun 13, 2022

Does anybody have a pointer to a current challenge that utilizes the mustache placeholders?

This one does.

@simonpoole
Copy link

Thanks.

Seems as if adding includeGeometries=true to the bounding box query additionally returns a GeoJSON object that contains the data that is necessary to replace the placeholders in the text returned with the challenge. If @mvexel could confirm that this is true as a rule, that would be helpful.

See https://maproulette.org/api/v2/tasks/box/123.3912048/10.5870772/123.4169148/10.6317919?includeGeometries=true

@mvexel
Copy link
Member

mvexel commented Jun 14, 2022

@jschwarz2030 can you confirm the above?

@jschwarz2030
Copy link
Contributor

Apologies I haven't looked at this sooner, but @simonpoole 's findings are correct and provide the properties object of the task. I'm not sure if adding this param to our discoverability pages is wise, or needed, and may incur a performance hit for larger queries (its primary use is as a search function for our frontend, as pointed out by @nrotstan), but for API users, it seems to satisfy the original issue.

Are there additional pieces to this request I'm not seeing here before closing the ticket?

@simonpoole
Copy link

Thanks @jschwarz2030 it seems as if this works in a reasonable fashion for us as in the end the end there are, compared to other things, not that many MR tasks. The additional load on the API is likely to be minimal in any case.

@govvin this is what the current implementation of the "Explanation" modal looks like for the task in question:
Screenshot_1655401499

There are numerous rough edges for a number of reasons (note for example broken MD in the example challenge), but it is likely a lot better than previously.

@govvin
Copy link
Author

govvin commented Jun 17, 2022

That looks good, @simonpoole .

Is there a way for me to test that, or will I have to wait for the next release?

@govvin this is what the current implementation of the "Explanation" modal looks like for the task in question: Screenshot_1655401499

There are numerous rough edges for a number of reasons (note for example broken MD in the example challenge), but it is likely a lot better than previously.

@simonpoole
Copy link

simonpoole commented Jun 17, 2022

That looks good, @simonpoole .

Is there a way for me to test that, or will I have to wait for the next release?

I can produce a build signed with my dev key, that implies de-installing and side loading though.

It is likely still going to take a couple of days as the Maproulette changes are part of a larger task handling refactoring. What would help is an example of an MR challenge that utilizes the OSM element short codes somewhere.

@govvin
Copy link
Author

govvin commented Jun 18, 2022

@simonpoole Is your challenge requirement for the short codes, need be the same challenge that uses mustache tags? If it's not, I can modify one of my existing challenges to utiltize the short code somewhere.

That looks good, @simonpoole .
Is there a way for me to test that, or will I have to wait for the next release?

I can produce a build signed with my dev key, that implies de-installing and side loading though.

It is likely still going to take a couple of days as the Maproulette changes are part of a larger task handling refactoring. What would help is an example of an MR challenge that utilizes the OSM element short codes somewhere.

@simonpoole
Copy link

@simonpoole Is your challenge requirement for the short codes, need be the same challenge that uses mustache tags? If it's not, I can modify one of my existing challenges to utiltize the short code somewhere.

No doesn't need mustache tags but no harm if it does. It would just be nice to have a real world example that I can use as a fixture for testing.

@govvin
Copy link
Author

govvin commented Jun 18, 2022 via email

@simonpoole
Copy link

simonpoole commented Jun 18, 2022

Simon, you can try this challenge: https://maproulette.org/browse/challenges/27672

Hmm, don't see anything there. The use case I was thinking of is for example this task

https://maproulette.org/api/v2/task/104895860

where in the instruction field (at last an example that actually has that :-) though in the data from the bounding box query it is in the blurb field) there's the text

"1. Building (id=674063184) intersects with another building (id=456691647)."

which I would have thought would be more useful with

"1. Building [w674063184] intersects with another building [w456691647]."

but if such short codes are not in actual use, then I'll just continue to ignore them.

@simonpoole
Copy link

@govvin you can try the debug build here https://drive.google.com/drive/folders/0B9pKLmh8s1h8bFI5bGd4VnhYWkk?resourcekey=0-i_x3v1pmu3067r8CB4XE-w (you can ignore the request for a further example, the feature doesn't seem to be in use, its supported anyway though).

@govvin
Copy link
Author

govvin commented Jul 2, 2022

@simonpoole , I tried this again today, but couldn't get the MR api key to work. And MR isn't displaying the tasks in the browser.

@govvin you can try the debug build here https://drive.google.com/drive/folders/0B9pKLmh8s1h8bFI5bGd4VnhYWkk?resourcekey=0-i_x3v1pmu3067r8CB4XE-w (you can ignore the request for a further example, the feature doesn't seem to be in use, its supported anyway though).

@simonpoole
Copy link

simonpoole commented Jul 2, 2022

@simonpoole , I tried this again today, but couldn't get the MR api key to work. And MR isn't displaying the tasks in the browser.

The apikey handling didn't actually change, in particular if you still have a valid key stored in your OSM preferences it should just work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

6 participants