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

Resourceful routes for traces API #5390

Merged

Conversation

AntonKhorev
Copy link
Collaborator

@AntonKhorev AntonKhorev commented Dec 10, 2024

This is only for /api/0.6/gpx/. There are other trace-related routes such as /api/0.6/user/gpx_files that are not changed here.

A couple of caveats:

  • We have a nonstandard trace metadata path gpx/:id/details since 01cfcbd. Later a standard path gpx/:id was added in 71f1554. Both path are currently supported. It's possible to convert details into a nested resource to give it a resourceful route. This will require keeping some copy-pasted code. How about instead we don't do that, keep the details path non-resourceful and deprecate it?
  • We have a nonstandard create trace path gpx/create. A standard one would be just POST at gpx. Again it's possible to map gpx/create in a hacky way to the create method in the traces controller, or make a nested resource just for creating traces. How about instead we:
    • add a standard create path
    • keep the gpx/create as non-resourceful and deprecate it?

@AntonKhorev AntonKhorev marked this pull request as ready for review December 10, 2024 11:47
@AntonKhorev AntonKhorev added the refactor Refactoring, or things that work but could be done better label Dec 10, 2024
@gravitystorm gravitystorm merged commit a155a2f into openstreetmap:master Dec 11, 2024
22 checks passed
@gravitystorm
Copy link
Collaborator

Looks good to me, thanks!

@AntonKhorev AntonKhorev deleted the trace-resourceful-routes branch December 11, 2024 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Refactoring, or things that work but could be done better
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants