Fix issue #5258 by elmccarthy · Pull Request #5259 · encode/django-rest-framework (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation7 Commits2 Checks0 Files changed
Conversation
This file contains hidden or 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
[ Show hidden characters]({{ revealButtonHref }})
Description
Sanitize serializer.data to remove hidden fields before rendering template
JSON instance to raw data form.
Sanitize serializer.data to remove hidden fields before rendering template JSON instance to raw data form.
I think this PR breaks serializers from the djangorestframework-gis add-on. This add-on serializes model objects into a GeoJSON-compatible dict with id, type, geometry, and properties fields. So this PR tries to look for these fields in the serializer class and will most probably result into a KeyError. This is obviously not DRF's fault but I think the removing of hidden fields should happen earlier, when the model object is being serialized, and not after the fact.
@seav First step here is a test showing the kind of call that is failing but shouldn't. From there we can think about adjustments.
Hi @jklaiho and @seav, I'm going to second @carltongibson here. It would be very helpful to have a minimal test case or a bit of sample code that demonstrates the issue.
rpkilby pushed a commit to rpkilby/django-rest-framework that referenced this pull request
Thanks @jklaiho, that and #5498 clarified the problem.
To recap, django-rest-framework-gis, does not use HiddenFields. The problem is that the HiddenField filtering in the browsable API assumes that the keys in the serializer.data will correspond to fields on the serializer. This assumption is incorrect and breaks the browsable API in these cases.