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

encounters return is different than other endpoints' #332

Closed
jrubinator opened this issue Apr 17, 2018 · 4 comments
Closed

encounters return is different than other endpoints' #332

jrubinator opened this issue Apr 17, 2018 · 4 comments
Labels
question v3 Changes to integrate in a possible new API version

Comments

@jrubinator
Copy link

Right now the encounters endpoint lives at https://pokeapi.co/api/v2/pokemon/{id}/encounters

For a given id, the URL returns a list, rather than a hash. On the other hand, the endpoint URLs (eg. /api/v2/pokemon) return very similar data, but prefix that data with results: Would it make sense to do this here? Unfortunately, that's not a backwards-compatible change.

I suggest this because I believe it would make it easier for API clients to handle the response - see eg.
PokeAPI/pokebase#10

@jrubinator jrubinator changed the title encounters return is different than other endpoints encounters return is different than other endpoints' Apr 17, 2018
@Naramsim
Copy link
Member

Hello, back in time the encounter endpoint wasn't there, but part of /api/v2/pokemon/{id}/. It was created to lighten the responses of this pokemon endpoint. So it was a quick improvement. Right now the repository is in a stalemate state, and probably no one will implement this alignment with /api/v2/pokemon/

@jrubinator
Copy link
Author

jrubinator commented Apr 18, 2018

Interesting - that migration makes sense.

I guess I was wondering if the following change makes sense at the last line of pokemon_v2/urls.py:

-        return Response(encounters_list)
+        return Response({'results': encounters_list})

I haven't worked with Django before, or I might be able to suggest something more mature (with proper pagination). That sounds like the alignment you doubt will happen though.

Let me know what you think.

@Naramsim
Copy link
Member

Yeah, probably is that simple fix 😄 So maybe you can open a PR and put me in the 'request review' and one day maybe it will be merged, but this change obviously could break lots of applications. So we have to be careful about deciding what to do.

jrubinator added a commit to jrubinator/pokeapi that referenced this issue Apr 21, 2018
This makes it consistent with the every other resource
Unfortunately this isn't tested or testable in the current framework,
as distinct() is not supported by the sqlite3 backend
@Naramsim
Copy link
Member

Closing this issue and marking it for a future v3 version.

Right now we cannot change the endpoint format since it's used by many people. We acknowledge that your proposed solution would be the best option.

@Naramsim Naramsim added the v3 Changes to integrate in a possible new API version label Aug 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question v3 Changes to integrate in a possible new API version
Projects
None yet
Development

No branches or pull requests

2 participants