search

Styria-Digital / django-rest-framework-jwt

JSON Web Token Authentication support for Django REST Framework
https://styria-digital.github.io/django-rest-framework-jwt/
MIT License
191 stars 57 forks source link

Fixing the status codes #80

Closed creyD closed 3 years ago

creyD commented 3 years ago

As I said in #79 I adjusted the status codes to return what is documented.

fitodic commented 3 years ago

@creyD Thanks for the PR 🙂 Would you mind adding a changelog and updating the returned status code in the test suite?

creyD commented 3 years ago

@fitodic Sure! Sorry I didn't see the Contributing.md before.

creyD commented 3 years ago

Can you test it for you again? There was one test failing, but i don't think this is because of my changes. test_authentication.py Line 120 for reference.

fitodic commented 3 years ago

Can you test it for you again? There was one test failing, but i don't think this is because of my changes. test_authentication.py Line 120 for reference.

If that's the one regarding sets and payload key's, it's already been fixed in #81. The only thing that remains is to pull the master branch and push the changes 🙂

UPDATE: I see the latest pipeline has passed so I guess that's it.

creyD commented 3 years ago

Thanks for merging this so fast! When will there be a new release?

fitodic commented 3 years ago

Thanks for merging this so fast! When will there be a new release?

1.17.3 is available on PyPI 🙂

creyD commented 3 years ago

Thanks!

dominik-bln commented 3 years ago

Quick Feedback: This change was a bit surprising in a patch update because it alters the behaviour of the API, i. e. it is a breaking change. For us it hopefully only broke the tests, but changing the docs or putting this in a major update would have seemed more appropriate.

marcosox commented 3 years ago

Agree, it's a breaking change which should have gone in a major version update. Thankfully I read changelogs before updating :)