8000 Improve composite field child errors by rpkilby · Pull Request #5655 · encode/django-rest-framework · GitHub
[go: up one dir, main page]
Skip to content

Improve composite field child errors #5655

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jan 2, 2018
Merged

Improve composite field child errors #5655

merged 3 commits into from
Jan 2, 2018

Conversation

rpkilby
Copy link
Member
@rpkilby rpkilby commented Dec 5, 2017 •
edited
Loading

Fixes #3274

This is an extension of the discussion in #5644. Errors raised on composite fields (DictField & ListField) are confusing, since child validation errors don't contain any indication of what index/key failed.

As stated by the django docs, the hstore field accepts nulls as values.

class Product(models.Model):
    attributes = HStoreField()

class ProductSerializer(serializers.ModelSerializer):
    class Meta:
        model = Product
        fields = ['attributes']

With the below input data, we currently exhibit the following error:

>>> q = ProductSerializer(data={"attributes": {"first": "value", "second": None}})
>>> q.is_valid()
>>> q.errors
{'attributes': ['This field may not be null.']}

The above error is not helpful, since it fails to mention that this applies to the "second" key. The proposed solution is to nest the errors by index/key.

before:
{'attributes': ['This field may not be null.']}

after:
{'attributes': {'second': ["This field may not be null."]}}

TODO:

@rpkilby rpkilby added this to the v3.8 milestone Dec 5, 2017
@rpkilby rpkilby force-pushed the child-field-errors branch from 29b4ab2 to 9fc2a5d Compare December 5, 2017 06:42
@rpkilby rpkilby mentioned this pull request Dec 5, 2017
Closed
@rpkilby rpkilby changed the title [wip] Improve composite field child errors Improve composite field child errors Dec 21, 2017
@rpkilby rpkilby force-pushed the child-field-errors branch 2 times, most recently from 9704f0f to 1bb3669 Compare December 21, 2017 23:53
@rpkilby
Copy link
Member Author
rpkilby commented Dec 22, 2017

I think this is ready for review.

One note - the error dict for a ListField is keyed by a value's index, however JSON dictionaries only accept strings as keys. So, while the underlying error detail may look like:

{0: ['This field may not be null.']}

The rendered JSON will look like

{
    "0": ["This field may not be null."]
}

carltongibson
carltongibson approved these changes Jan 2, 2018
View reviewed changes
Copy link
Collaborator
@carltongibson carltongibson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Great. Thanks @rpkilby!

@carltongibson carltongibson merged commit 6bd773e into encode:master Jan 2, 2018
@rpkilby rpkilby deleted the child-field-errors branch January 3, 2018 22:53
@emiel emiel mentioned this pull request Jun 19, 2018
Closed
6 tasks
markopy mentioned this pull request Apr 18, 2020
Open
pchiquet pushed a commit to pchiquet/django-rest-framework that referenced this pull request Nov 17, 2020
Improve composite field child errors (encode#5655)
5751d58 
* Fixup DictField test descriptions

* Nest ListField/DictField errors under the idx/key

* Add nested ListField/DictField tests
joshuarli added a commit to getsentry/sentry that referenced this pull request Mar 23, 2021
@joshuarli
test: adapt to encode/django-rest-framework#5655
907e07e
@joshuarli joshuarli mentioned this pull request Mar 24, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carltongibson carltongibson carltongibson approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.8.0 Release
Development

Successfully merging this pull request may close these issues.

Nested ListField serializer errors should include object reference
2 participants
@rpkilby @carltongibson
0