hauke96 / simple-task-manager

A simple tasking manager made for OpenStreetMap.
https://stm.hauke-stieler.de
Other
34 stars 6 forks source link

pass GPX task boundary to iD #157

Open russbiggs opened 3 years ago

russbiggs commented 3 years ago

It would be nice to pass a gpx boundary of the selected to iD so help provide context of where the task is. This functionality exists in the HOT tasking manager. Especially with a 1km grid its hard to tell where a user should stop to stay within a task area. Happy to open a PR.

hauke96 commented 3 years ago

True, that would be more accurate than the current solution.

Feel free to work on this, the important method is openInOsmOrg in the TaskService (here). To convert a polygon into a GPX track, you should use the OpenLayers formatter for that (s. here). I think the best place for a featureToGpx kind of method would be the GeometryService because there's already some parsing going on. Also keep in mind that a task could be a multi-polygon feature.

Feel free to ask any questions :)

If you can't or don't want to work on this, that totally ok. However I'm quite busy lately, so don't expect results soon from me.

russbiggs commented 3 years ago

Thanks for pointing out those services. I'll put something together and submit a PR.

russbiggs commented 3 years ago

After a little more research, iD requires a URL for the gpx argument in the URL see here. One option to implement this we would need to have a gpx endpoint in the API. So I'm thinking an endpoint here along the lines of /tasks/{id}/gpx, this would need to be unauthenticated so that iD can pull without a token. How does that sound?

hauke96 commented 3 years ago

The client already has everything it needs and can easily convert the task into a GPX string. The easiest way would be to use the GPX format feature of OpenLayers, which has a writeFeature method turning a feature into a GPX string (s. here).

So something like this should be fine:

const gpxString = new GPX().writeFeature(task.geometry);

The GeometryService already uses the format classes from OpenLayers to do the opposite when importing tasks. So I would simply add a method to that service like this:

featureToGpx(feature: Feature): string {
   return new GPX().writeFeature(task.geometry);
}

Maybe create the GPX-instance once and reuse it everytime, but I think you get the idea. The TaskService (which opens iD) then just calls this method to get the GPX string which can then be appended to the URL.

Interesting problem that may rise: Depending on the Browser and/or the API of iD, the URL could become too long. Maybe that's not a problem but one should at least try that with a task which has a lot of edges.

russbiggs commented 3 years ago

The problem is we cannot pass a gpx string to the URL for iD to consume, we need a URL which hosts the GPX string. See this in the iD documentation:

gpx - A custom URL for loading a gpx track. Specifying a gpx parameter will automatically enable the gpx layer for display.
Example: gpx=https://tasks.hotosm.org/project/592/task/16.gpx
hauke96 commented 3 years ago

Ohhh ok, I misread the doc, sorry. Yeah, then we need a service side solution.

The code location you mentioned above is correct. Using the simpleHandler (as for /config) instead of authenticatedTransactionHandler would add a simple unauthenticated request handler.

However: STM intentionally only knows private projects and doesn't publish data from projects on its API. This is a principle I want to keep here as well as STM doesn't has a concept for public projects yet. I talked to some people about it but public projects do cause a lot of problems, but that's a different story.

I currently have two solutions in mind:

  1. Use the simpleHandler but add an additional param to the URL like an API key. So something like this: /tasks/{id}/gpx?apikey=af5ebd051.... This key then has a lifespan of one minute (that sould be enough time for iD to load the GPS, maybe the amount of minutes is configurable via the config file?). The client would first request such a key and then simply pass the URL (including the key-param) to iD.
  2. Similar idea: The client asks the server via e.g. tasks/{id}/gpx-temp-url (or something similar) to generate a whole temporary URL like .../v2.7/temp/af5ebd051 which again is valid for a minute.

Both possible solutions would guarantee a certain degree of privacy. Personally I prefer the first one as the URL still looks nice and the implementation should be simple as it's just a simple parameter. The apikey/temp-url sould be stateless, so the server does not need to store a list of apikeys and handle them.

What do you think about this? Do you have other/additional ideas for this?

russbiggs commented 3 years ago

Solution 1 seems like the cleaner option to me as well. That should keep the endpoint private for long enough and maintain the private project principle you're trying to maintain. I'll start working on the PR following this model.