Fix DictField.get_value to return `empty`

[blueyed]

Without this, an empty field is not considered to be empty and then validators might fail, e.g. django.contrib.postgres.validators.RangeMinValueValidator with

E TypeError: '<' not supported between instances of 'NoneType' and 'int'

TODO:

• [ ] full test coverage?! https://codecov.io/gh/encode/django-rest-framework/pull/6009/diff

[blueyed]

There might be a better place for the test?

[xordoquy]

How does a dict trigger a RangeMinValueValidator ?

[blueyed]

We have this:

from django.contrib.postgres.fields import IntegerRangeField

    weekdays = IntegerRangeField(default=None, null=True, blank=True,
                                 validators=[RangeMinValueValidator(1),
                                             RangeMaxValueValidator(7)])

I have not investigated too deeply, but found that this fixes a bug when using the HTML form to POST empty data for this field.

[xordoquy]

I don't think DRF supports IntegerRangeField I'm going to close this assuming this is a third party issue and based on the fact that I have no guidance how DRF is supposed to support this field through forms. Note that forms are harder to handle than "regular" input since we have no way to distinguish missing keys from null values from empty values.

[rpkilby]

This should be compared against similar behavior in ListField, specifically using the default argument instead of or empty.

https://github.com/encode/django-rest-framework/blob/fe54575e6a0b1abc43a84814cc1b8625e6187a8b/rest_framework/fields.py#L1625-L1638

---

Note that forms are harder to handle than "regular" input since we have no way to distinguish missing keys from null values from empty values.

I'm still half a mind to completely disabling HTML form support for the composite field types. They're currently disabled in the browsable API templates, but I'm still tempted to remove it entirely.

[xordoquy]

good catch, then it should use the same styling. Note that I'm fine with keeping it for now.

[blueyed]

Thanks for the feedback - changed to use the same pattern as with ListField.

Previously it was using the pattern from the serializer (https://github.com/encode/django-rest-framework/blob/38b1303b365b134b5397b721d5c9773bc18655f6/rest_framework/serializers.py#L421).

[blueyed]

Support from it comes with drf-extra-fields by the way: https://github.com/Hipo/drf-extra-fields/blob/3aeb157db58168053aaecfdf5754666c3621127e/drf_extra_fields/fields.py#L206.

[blueyed]

@carltongibson Thanks for the pointer, I've started consolidating them to gain full coverage also, but it's not easy - therefore WIP for now.

[carltongibson]

@blueyed How are you getting on with this? It would be good to get it in.

[carltongibson]

@blueyed Any life in this? Are you able to look at it now-ish? (If time is short, we can defer the test refactoring.)

[blueyed]

@carltongibson Rebased it for now, but haven't looked at it in a while.

I'd say it's better to have a bug fixed than imperfect tests.

[rpkilby]

I'm going to de-milestone this for now, pending #6779.

[stale[bot]]

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]

most probably we can close this for now