From 9bab640b0a9908cf9777da6efae8dcd1e158a980 Mon Sep 17 00:00:00 2001 From: Kevin Brown Date: Thu, 24 Dec 2015 14:00:49 -0500 Subject: [PATCH 1/4] Added tests for min_value and max_value on a DecimalField This adds tests for a regression where the `min_value` and `max_value` arguments are not being set for a DRF `DecimalField` even though the corresponding `MinValueValidator` and `MaxValueValidator` is being set on the model fields. Note that this only appears to be a regression for Django < 1.9, as these regression tests pass on newer versions of Django. --- tests/test_model_serializer.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/test_model_serializer.py b/tests/test_model_serializer.py index 57e540e7a5..e2c3a7a476 100644 --- a/tests/test_model_serializer.py +++ b/tests/test_model_serializer.py @@ -899,3 +899,29 @@ class Meta: serializer = TestSerializer() assert len(serializer.fields['decimal_field'].validators) == 2 + + def test_min_value_is_passed(self): + """ + Test that the `MinValueValidator` is converted to the `min_value` + argument for the field. + """ + class TestSerializer(serializers.ModelSerializer): + class Meta: + model = DecimalFieldModel + + serializer = TestSerializer() + + assert serializer.fields['decimal_field'].min_value == 1 + + def test_max_value_is_passed(self): + """ + Test that the `MaxValueValidator` is converted to the `max_value` + argument for the field. + """ + class TestSerializer(serializers.ModelSerializer): + class Meta: + model = DecimalFieldModel + + serializer = TestSerializer() + + assert serializer.fields['decimal_field'].max_value == 3 From 87605e1e39d8069805345ac72ae5a13088a2df53 Mon Sep 17 00:00:00 2001 From: Kevin Brown Date: Thu, 24 Dec 2015 14:10:48 -0500 Subject: [PATCH 2/4] Don't filter out the DecimalValidator if it is not supported Previously, all validators set on a DecimalField in Django would be stripped when converted to a Django REST framework field. This was because any validator that was an instance of `DecimalValidator` would be removed, and when `DecimalValidator` wasn't supported (so it was `None`), all validators would be removed. This fixes the issue by only removing the `DecimalValidator` instances if the `DecimalValidator` is supported. --- rest_framework/utils/field_mapping.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest_framework/utils/field_mapping.py b/rest_framework/utils/field_mapping.py index 1d8cb22f2f..22a38050d3 100644 --- a/rest_framework/utils/field_mapping.py +++ b/rest_framework/utils/field_mapping.py @@ -130,7 +130,7 @@ def get_field_kwargs(field_name, model_field): # Our decimal validation is handled in the field code, not validator code. # (In Django 1.9+ this differs from previous style) - if isinstance(model_field, models.DecimalField): + if isinstance(model_field, models.DecimalField) and DecimalValidator: validator_kwarg = [ validator for validator in validator_kwarg if DecimalValidator and not isinstance(validator, DecimalValidator) From d797389cf784b3e55179f5bcea51379d2d37f42c Mon Sep 17 00:00:00 2001 From: Kevin Brown Date: Thu, 24 Dec 2015 18:17:58 -0500 Subject: [PATCH 3/4] Fixed broken test for Django < 1.9 This test was incorrectly checking that there were no validators set in older versions of Django, even though it should have been checking for the two validators that were set up on the model field level. The originally regression test that this fixes was added in https://github.com/tomchristie/django-rest-framework/commit/7d79cf35b7be01b175d8c25276a4414e8144a16b when fixing an issue with the `DecimalValidator`. --- tests/test_model_serializer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_model_serializer.py b/tests/test_model_serializer.py index e2c3a7a476..2976b1984d 100644 --- a/tests/test_model_serializer.py +++ b/tests/test_model_serializer.py @@ -884,7 +884,7 @@ class Meta: serializer = TestSerializer() - assert len(serializer.fields['decimal_field'].validators) == 0 + assert len(serializer.fields['decimal_field'].validators) == 2 @pytest.mark.skipif(DecimalValidator is None, reason='DecimalValidator is available in Django 1.9+') From a7723261124c2fff7033f20680dd0bc3bc78fa8e Mon Sep 17 00:00:00 2001 From: Kevin Brown Date: Mon, 28 Dec 2015 10:38:53 -0500 Subject: [PATCH 4/4] Merged two DecimalValidator tests together These two tests were previously added in https://github.com/tomchristie/django-rest-framework/commit/7d79cf35b7be01b175d8c25276a4414e8144a16b but we have now discovered that there are not actually two separate cases, there was just a bug in the code that made it look that way. This also removes a redundant check to see if `DecimalValidator` was defined. --- rest_framework/utils/field_mapping.py | 2 +- tests/test_model_serializer.py | 20 ++------------------ 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/rest_framework/utils/field_mapping.py b/rest_framework/utils/field_mapping.py index 22a38050d3..af7ab02311 100644 --- a/rest_framework/utils/field_mapping.py +++ b/rest_framework/utils/field_mapping.py @@ -133,7 +133,7 @@ def get_field_kwargs(field_name, model_field): if isinstance(model_field, models.DecimalField) and DecimalValidator: validator_kwarg = [ validator for validator in validator_kwarg - if DecimalValidator and not isinstance(validator, DecimalValidator) + if not isinstance(validator, DecimalValidator) ] # Ensure that max_length is passed explicitly as a keyword arg, diff --git a/tests/test_model_serializer.py b/tests/test_model_serializer.py index 2976b1984d..af8ce66dd7 100644 --- a/tests/test_model_serializer.py +++ b/tests/test_model_serializer.py @@ -22,7 +22,7 @@ from rest_framework import serializers from rest_framework.compat import DurationField as ModelDurationField -from rest_framework.compat import DecimalValidator, unicode_repr +from rest_framework.compat import unicode_repr def dedent(blocktext): @@ -872,25 +872,9 @@ class DecimalFieldModel(models.Model): class TestDecimalFieldMappings(TestCase): - @pytest.mark.skipif(DecimalValidator is not None, - reason='DecimalValidator is available in Django 1.9+') - def test_decimal_field_has_no_decimal_validator(self): - """ - Test that a DecimalField has no validators before Django 1.9. - """ - class TestSerializer(serializers.ModelSerializer): - class Meta: - model = DecimalFieldModel - - serializer = TestSerializer() - - assert len(serializer.fields['decimal_field'].validators) == 2 - - @pytest.mark.skipif(DecimalValidator is None, - reason='DecimalValidator is available in Django 1.9+') def test_decimal_field_has_decimal_validator(self): """ - Test that a DecimalField has DecimalValidator in Django 1.9+. + Test that a `DecimalField` has no `DecimalValidator`. """ class TestSerializer(serializers.ModelSerializer): class Meta: