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

docs: JSON input to JSONField is not parsed correctly #5873

Closed
4 of 6 tasks
beruic opened this issue Mar 12, 2018 · 7 comments · Fixed by #5878
Closed
4 of 6 tasks

docs: JSON input to JSONField is not parsed correctly #5873

beruic opened this issue Mar 12, 2018 · 7 comments · Fixed by #5878
Milestone

Comments

@beruic
Copy link
Contributor

beruic commented Mar 12, 2018

Checklist

  • I have verified that that issue exists against the master branch of Django REST framework.
  • I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
  • This is not a usage question. (Those should be directed to the discussion group instead.)
  • This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third party libraries where possible.)
  • I have reduced the issue to the simplest possible case.
  • I have included a failing test as a pull request. (If you are unable to do so we can still accept the issue.)

Steps to reproduce

  1. Create a JSONField on a serializer
  2. Submit {"a": "test"} to the field through docs
  3. Breakpoint at the return of JSONField.to_internal_value()

Expected behavior

The returned data is a dictionary with a single key a that has a value which is string test.

Actual behavior

The returned data is the string {"a": "test"}

@beruic
Copy link
Contributor Author

beruic commented Mar 12, 2018

This verified on release version 3.7.7.

@beruic
Copy link
Contributor Author

beruic commented Mar 13, 2018

I have narrowed this down to the point where I am confident to conclude that the issue is that the data-type attribute is set to string on the input element of the JSONField. Setting data-type to object produce the desired behaviour. Now I just need to figure out how to do just that.

@carltongibson
Copy link
Collaborator

carltongibson commented Mar 13, 2018

Hey @beruic. Thanks for the report.

Sounds like this will be coming out of coreschema. I'm just in the process of pulling that into DRF for v3.8 now. (#5855)

There issue will be here:

https://github.com/core-api/python-coreschema/blob/494435d8fbaff12285094e99c54c38f5a1434afb/coreschema/encodings/html.py#L54-L74

I'll see if I can close this whilst I'm porting this to DRF.

@carltongibson carltongibson added this to the 3.9 Release milestone Mar 13, 2018
@beruic
Copy link
Contributor Author

beruic commented Mar 13, 2018

You are right that this is related to coreschema, but the fix lies in rest_framework.schemas.inspectors.field_to_schema(). I have solved this just now in DRF. Expect a pull request soon.

I cannot however fix it for DictField, which has the same problem with input, because coreschema.schema.Object does not have an items argument for child schema like coreschema.schema.Array does. I think that a coreschema.schema.Dictionary is needed because, while coreschema.schema.Object has a properties argument, it seems to validate per key. I was considering making a feature request for coreschema, but if I understand you correctly, that is not going to be necessary, but we have to fix it in DRF.
Thoughts?

@beruic
Copy link
Contributor Author

beruic commented Mar 14, 2018

Just a quick update: It seems that DictField does return coreschema.Object in rest_framework.schemas.inspectors.field_to_schema(), so I may have gotten something wrong. I will see if I can get time investigating this more, but it is likely that I do not get around it again before I actually need DictField for something.

@beruic
Copy link
Contributor Author

beruic commented Mar 14, 2018

I just realized that I hadn't gotten anything wrong. DictField handling has been added since version 3.7.7 authored by @jlaine and committed by @carltongibson. There is no tests for this code either, and it is already in the master, so I cannot see why this PR should not be able to make it into 3.8 as well.

@beruic
Copy link
Contributor Author

beruic commented Mar 14, 2018

I do however see a potential bug in that the code for handling DictField does not handle the child schema.

carltongibson pushed a commit that referenced this issue Apr 20, 2018
Fixes #5873.
* Use Object type. 
* Add test for field_to_schema
pchiquet pushed a commit to pchiquet/django-rest-framework that referenced this issue Nov 17, 2020
Fixes encode#5873.
* Use Object type. 
* Add test for field_to_schema
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants