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

Improve composite field child errors #5655

Merged
merged 3 commits into from Jan 2, 2018

Conversation

rpkilby
Copy link
Member

@rpkilby rpkilby commented Dec 5, 2017

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 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."]
}

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
pchiquet pushed a commit to pchiquet/django-rest-framework that referenced this pull request Nov 17, 2020
* 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
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 this pull request may close these issues.

Nested ListField serializer errors should include object reference
2 participants