fix URLPathVersioning reverse fallback #7247
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi @jornvanwier. Could you add a test case that demonstrates the failure? |
|
Hi @rpkilby, I was unable to get the ReverseView that's used in the other test to show the issue, so I manually called into the reverse function (much like how I'm using it where I first encountered the problem). I hope that's ok. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
auvipy
requested changes
Apr 2, 2023
tfranzel
approved these changes
May 17, 2023
There was a problem hiding this comment.
This changes 2 things. It makes version_param in kwargs take precedence over the request.version and it stops modifying the given kwargs, thus stop potentially leaking into other contexts.
This might impact existing users, but it is imho the right thing to do. I'll approve but would prefer if someone else had a quick look too.
auvipy
approved these changes
May 18, 2023
saadullahaleem
pushed a commit
to saadullahaleem/django-rest-framework
that referenced
this pull request
May 27, 2023
* fix URLPathVersioning reverse fallback * add test for URLPathVersioning reverse fallback * Update tests/test_versioning.py --------- Co-authored-by: Jorn van Wier <[email protected]> Co-authored-by: Asif Saif Uddin <[email protected]>
auvipy
added a commit
that referenced
this pull request
May 29, 2023
* fix: Make the instance variable of child serializer point to the correct list object instead of the entire list when validating ListSerializer * fix formatting issues for list serializer validation fix * fix imports sorting for list serializer tests * remove django 2.2 from docs index (#8982) * Declared Django 4.2 support in README.md (#8985) * Fix Links in Documentation to Django `reverse` and `reverse_lazy` (#8986) * Fix Django Docs url in reverse.md Django URLs of the documentation of `reverse` and `reverse_lazy` were wrong. * Update reverse.md * fix URLPathVersioning reverse fallback (#7247) * fix URLPathVersioning reverse fallback * add test for URLPathVersioning reverse fallback * Update tests/test_versioning.py --------- Co-authored-by: Jorn van Wier <[email protected]> Co-authored-by: Asif Saif Uddin <[email protected]> * Make set_value a method within `Serializer` (#8001) * Make set_value a static method for Serializers As an alternative to #7671, let the method be overridden if needed. As the function is only used for serializers, it has a better place in the Serializer class. * Set `set_value` as an object (non-static) method * Add tests for set_value() These tests follow the examples given in the method. * fix: Make the instance variable of child serializer point to the correct list object instead of the entire list when validating ListSerializer * Make set_value a method within `Serializer` (#8001) * Make set_value a static method for Serializers As an alternative to #7671, let the method be overridden if needed. As the function is only used for serializers, it has a better place in the Serializer class. * Set `set_value` as an object (non-static) method * Add tests for set_value() These tests follow the examples given in the method. * fix: Make the instance variable of child serializer point to the correct list object instead of the entire list when validating ListSerializer * fix: Make the instance variable of child serializer point to the correct list object instead of the entire list when validating ListSerializer * fix formatting issues for list serializer validation fix * fix: Make the instance variable of child serializer point to the correct list object instead of the entire list when validating ListSerializer * fix formatting issues for list serializer validation fix * fix linting * Update rest_framework/serializers.py Co-authored-by: Sergei Shishov <[email protected]> * Update rest_framework/serializers.py Co-authored-by: Sergei Shishov <[email protected]> * fix: instance variable in list serializer, remove commented code --------- Co-authored-by: Mathieu Dupuy <[email protected]> Co-authored-by: Mehraz Hossain Rumman <[email protected]> Co-authored-by: Dominik Bruhn <[email protected]> Co-authored-by: jornvanwier <[email protected]> Co-authored-by: Jorn van Wier <[email protected]> Co-authored-by: Asif Saif Uddin <[email protected]> Co-authored-by: Étienne Beaulé <[email protected]> Co-authored-by: Sergei Shishov <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When using URLPathVersioning
reverseaddsrequest.versionto the kwargs and tries to find a match. If that fails, it falls back to the defaultreversefunction (which doesn't add the version to the kwargs.However, without this fix the kwargs parameter is modified, which leads to the fallback
reversefunction having the same behaviour as the URLPathVersioning variant, which makes it impossible toreversea view that doesn't have versioning.With this change a copy of kwargs is created instead.
I ran
toxdidn't see any new issues caused by my changes.