From 56daa693dfdb26e0cdf0bb3888fab3c9cdf7aafe Mon Sep 17 00:00:00 2001 From: Vignesh Date: Mon, 11 Sep 2017 20:00:48 +0800 Subject: [PATCH 01/11] first version of fix only for min and max value --- rest_framework/fields.py | 4 +-- rest_framework/utils/field_mapping.py | 14 +++++---- tests/test_model_serializer.py | 2 +- tests/test_validators.py | 43 ++++++++++++++++++++++++--- 4 files changed, 50 insertions(+), 13 deletions(-) diff --git a/rest_framework/fields.py b/rest_framework/fields.py index 55c2fe48e..56fd543d8 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -987,8 +987,8 @@ class FloatField(Field): class DecimalField(Field): default_error_messages = { 'invalid': _('A valid number is required.'), - 'max_value': _('Ensure this value is less than or equal to {max_value}.'), - 'min_value': _('Ensure this value is greater than or equal to {min_value}.'), + 'max_value': _('Ensure this value is less than or equal to %(limit_value)s.'), + 'min_value': _('Ensure this value is greater than or equal to %(limit_value)s.'), 'max_digits': _('Ensure that there are no more than {max_digits} digits in total.'), 'max_decimal_places': _('Ensure that there are no more than {max_decimal_places} decimal places.'), 'max_whole_digits': _('Ensure that there are no more than {max_whole_digits} digits before the decimal point.'), diff --git a/rest_framework/utils/field_mapping.py b/rest_framework/utils/field_mapping.py index 722981b20..dbd145371 100644 --- a/rest_framework/utils/field_mapping.py +++ b/rest_framework/utils/field_mapping.py @@ -127,12 +127,13 @@ def get_field_kwargs(field_name, model_field): else: # Ensure that max_value is passed explicitly as a keyword arg, # rather than as a validator. - max_value = next(( - validator.limit_value for validator in validator_kwarg + max_value, messsage = next(( + (validator.limit_value, validator.message) for validator in validator_kwarg if isinstance(validator, validators.MaxValueValidator) - ), None) + ), (None, None)) if max_value is not None and isinstance(model_field, NUMERIC_FIELD_TYPES): kwargs['max_value'] = max_value + kwargs['error_messages'] = {'max_value': messsage} validator_kwarg = [ validator for validator in validator_kwarg if not isinstance(validator, validators.MaxValueValidator) @@ -140,12 +141,13 @@ def get_field_kwargs(field_name, model_field): # Ensure that min_value is passed explicitly as a keyword arg, # rather than as a validator. - min_value = next(( - validator.limit_value for validator in validator_kwarg + min_value, messsage = next(( + (validator.limit_value, validator.message) for validator in validator_kwarg if isinstance(validator, validators.MinValueValidator) - ), None) + ), (None, None)) if min_value is not None and isinstance(model_field, NUMERIC_FIELD_TYPES): kwargs['min_value'] = min_value + kwargs['error_messages'] = {**kwargs['error_messages'], **{'min_value': messsage}} validator_kwarg = [ validator for validator in validator_kwarg if not isinstance(validator, validators.MinValueValidator) diff --git a/tests/test_model_serializer.py b/tests/test_model_serializer.py index 203e1fe7f..af4f63ca2 100644 --- a/tests/test_model_serializer.py +++ b/tests/test_model_serializer.py @@ -200,7 +200,7 @@ class TestRegularFieldMappings(TestCase): expected = dedent(""" TestSerializer(): id = IntegerField(label='ID', read_only=True) - value_limit_field = IntegerField(max_value=10, min_value=1) + value_limit_field = IntegerField(error_messages={'max_value': 'Ensure this value is less than or equal to %(limit_value)s.', 'min_value': 'Ensure this value is greater than or equal to %(limit_value)s.'}, max_value=10, min_value=1) length_limit_field = CharField(max_length=12, min_length=3) blank_field = CharField(allow_blank=True, max_length=10, required=False) null_field = IntegerField(allow_null=True, required=False) diff --git a/tests/test_validators.py b/tests/test_validators.py index 62126ddb3..77a7fc6d9 100644 --- a/tests/test_validators.py +++ b/tests/test_validators.py @@ -1,6 +1,7 @@ import datetime import pytest +from django.core.validators import MinValueValidator, MaxValueValidator from django.db import DataError, models from django.test import TestCase @@ -36,7 +37,7 @@ class RelatedModel(models.Model): class RelatedModelSerializer(serializers.ModelSerializer): username = serializers.CharField(source='user.username', - validators=[UniqueValidator(queryset=UniquenessModel.objects.all(), lookup='iexact')]) # NOQA + validators=[UniqueValidator(queryset=UniquenessModel.objects.all(), lookup='iexact')]) # NOQA class Meta: model = RelatedModel @@ -245,10 +246,12 @@ class TestUniquenessTogetherValidation(TestCase): When model fields are not included in a serializer, then uniqueness validators should not be added for that field. """ + class ExcludedFieldSerializer(serializers.ModelSerializer): class Meta: model = UniquenessTogetherModel fields = ('id', 'race_name',) + serializer = ExcludedFieldSerializer() expected = dedent(""" ExcludedFieldSerializer(): @@ -262,6 +265,7 @@ class TestUniquenessTogetherValidation(TestCase): When serializer fields are read only, then uniqueness validators should not be added for that field. """ + class ReadOnlyFieldSerializer(serializers.ModelSerializer): class Meta: model = UniquenessTogetherModel @@ -281,6 +285,7 @@ class TestUniquenessTogetherValidation(TestCase): """ Ensure validators can be explicitly removed.. """ + class NoValidatorsSerializer(serializers.ModelSerializer): class Meta: model = UniquenessTogetherModel @@ -329,6 +334,7 @@ class TestUniquenessTogetherValidation(TestCase): filter_queryset should add value from existing instance attribute if it is not provided in attributes dict """ + class MockQueryset(object): def filter(self, **kwargs): self.called_with = kwargs @@ -411,6 +417,7 @@ class TestUniquenessForDateValidation(TestCase): 'published': datetime.date(2000, 1, 1) } + # Tests for `UniqueForMonthValidator` # ---------------------------------- @@ -427,7 +434,6 @@ class UniqueForMonthSerializer(serializers.ModelSerializer): class UniqueForMonthTests(TestCase): - def setUp(self): self.instance = UniqueForMonthModel.objects.create( slug='existing', published='2017-01-01' @@ -450,6 +456,7 @@ class UniqueForMonthTests(TestCase): 'published': datetime.date(2017, 2, 1) } + # Tests for `UniqueForYearValidator` # ---------------------------------- @@ -466,7 +473,6 @@ class UniqueForYearSerializer(serializers.ModelSerializer): class UniqueForYearTests(TestCase): - def setUp(self): self.instance = UniqueForYearModel.objects.create( slug='existing', published='2017-01-01' @@ -532,23 +538,25 @@ class TestHiddenFieldUniquenessForDateValidation(TestCase): class ValidatorsTests(TestCase): - def test_qs_exists_handles_type_error(self): class TypeErrorQueryset(object): def exists(self): raise TypeError + assert qs_exists(TypeErrorQueryset()) is False def test_qs_exists_handles_value_error(self): class ValueErrorQueryset(object): def exists(self): raise ValueError + assert qs_exists(ValueErrorQueryset()) is False def test_qs_exists_handles_data_error(self): class DataErrorQueryset(object): def exists(self): raise DataError + assert qs_exists(DataErrorQueryset()) is False def test_validator_raises_error_if_not_all_fields_are_provided(self): @@ -563,3 +571,30 @@ class ValidatorsTests(TestCase): date_field='bar') with pytest.raises(NotImplementedError): validator.filter_queryset(attrs=None, queryset=None) + + +class ItemModel(models.Model): + price = models.DecimalField(decimal_places=2, max_digits=10, validators=[MinValueValidator(limit_value=0, message='Price has to be >= 0.'), + MaxValueValidator(limit_value=10, message='Price has to be <= 10.')]) + + +class ItemSerializer(serializers.ModelSerializer): + class Meta: + model = ItemModel + fields = '__all__' + + +class ValidatorMessageTests(TestCase): + def test_min_value_validator_message_is_copied_from_model(self): + data = {'price': -1} + s = ItemSerializer(data=data, partial=True) + s.is_valid() + + assert s.errors['price'] == ['Price has to be >= 0.'] + + def test_max_value_validator_message_is_copied_from_model(self): + data = {'price': 11} + s = ItemSerializer(data=data, partial=True) + s.is_valid() + + assert s.errors['price'] == ['Price has to be <= 10.'] From 5019494042bbff68b8c264d5e29f90d70fa2f7a0 Mon Sep 17 00:00:00 2001 From: Vignesh Date: Sat, 23 Sep 2017 02:15:38 +0800 Subject: [PATCH 02/11] make changes based on review --- rest_framework/utils/field_mapping.py | 8 ++++---- tests/test_model_serializer.py | 3 +++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/rest_framework/utils/field_mapping.py b/rest_framework/utils/field_mapping.py index dbd145371..ab42fc605 100644 --- a/rest_framework/utils/field_mapping.py +++ b/rest_framework/utils/field_mapping.py @@ -127,13 +127,13 @@ def get_field_kwargs(field_name, model_field): else: # Ensure that max_value is passed explicitly as a keyword arg, # rather than as a validator. - max_value, messsage = next(( + max_value, message = next(( (validator.limit_value, validator.message) for validator in validator_kwarg if isinstance(validator, validators.MaxValueValidator) ), (None, None)) if max_value is not None and isinstance(model_field, NUMERIC_FIELD_TYPES): kwargs['max_value'] = max_value - kwargs['error_messages'] = {'max_value': messsage} + kwargs['error_messages'] = {'max_value': message} validator_kwarg = [ validator for validator in validator_kwarg if not isinstance(validator, validators.MaxValueValidator) @@ -141,13 +141,13 @@ def get_field_kwargs(field_name, model_field): # Ensure that min_value is passed explicitly as a keyword arg, # rather than as a validator. - min_value, messsage = next(( + min_value, message = next(( (validator.limit_value, validator.message) for validator in validator_kwarg if isinstance(validator, validators.MinValueValidator) ), (None, None)) if min_value is not None and isinstance(model_field, NUMERIC_FIELD_TYPES): kwargs['min_value'] = min_value - kwargs['error_messages'] = {**kwargs['error_messages'], **{'min_value': messsage}} + kwargs.setdefault('error_messages', {}).update(min_value=message) validator_kwarg = [ validator for validator in validator_kwarg if not isinstance(validator, validators.MinValueValidator) diff --git a/tests/test_model_serializer.py b/tests/test_model_serializer.py index af4f63ca2..814901331 100644 --- a/tests/test_model_serializer.py +++ b/tests/test_model_serializer.py @@ -214,6 +214,9 @@ class TestRegularFieldMappings(TestCase): expected = expected.replace( "('red', 'Red'), ('blue', 'Blue'), ('green', 'Green')", "(u'red', u'Red'), (u'blue', u'Blue'), (u'green', u'Green')" + ).replace( + "{'max_value': 'Ensure this value is less than or equal to %(limit_value)s.', 'min_value': 'Ensure this value is greater than or equal to %(limit_value)s.'}", + {'max_value': u'Ensure this value is less than or equal to %(limit_value)s.', 'min_value': u'Ensure this value is greater than or equal to %(limit_value)s.'} ) self.assertEqual(unicode_repr(TestSerializer()), expected) From 131976c06bb1584f368d480013f6a31dde7e4309 Mon Sep 17 00:00:00 2001 From: Vignesh Date: Sat, 23 Sep 2017 02:20:07 +0800 Subject: [PATCH 03/11] remove unnecessary code formatting changes --- tests/test_validators.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/tests/test_validators.py b/tests/test_validators.py index 77a7fc6d9..42f5635e5 100644 --- a/tests/test_validators.py +++ b/tests/test_validators.py @@ -37,7 +37,7 @@ class RelatedModel(models.Model): class RelatedModelSerializer(serializers.ModelSerializer): username = serializers.CharField(source='user.username', - validators=[UniqueValidator(queryset=UniquenessModel.objects.all(), lookup='iexact')]) # NOQA + validators=[UniqueValidator(queryset=UniquenessModel.objects.all(), lookup='iexact')]) # NOQA class Meta: model = RelatedModel @@ -246,12 +246,10 @@ class TestUniquenessTogetherValidation(TestCase): When model fields are not included in a serializer, then uniqueness validators should not be added for that field. """ - class ExcludedFieldSerializer(serializers.ModelSerializer): class Meta: model = UniquenessTogetherModel fields = ('id', 'race_name',) - serializer = ExcludedFieldSerializer() expected = dedent(""" ExcludedFieldSerializer(): @@ -265,7 +263,6 @@ class TestUniquenessTogetherValidation(TestCase): When serializer fields are read only, then uniqueness validators should not be added for that field. """ - class ReadOnlyFieldSerializer(serializers.ModelSerializer): class Meta: model = UniquenessTogetherModel @@ -285,7 +282,6 @@ class TestUniquenessTogetherValidation(TestCase): """ Ensure validators can be explicitly removed.. """ - class NoValidatorsSerializer(serializers.ModelSerializer): class Meta: model = UniquenessTogetherModel @@ -334,7 +330,6 @@ class TestUniquenessTogetherValidation(TestCase): filter_queryset should add value from existing instance attribute if it is not provided in attributes dict """ - class MockQueryset(object): def filter(self, **kwargs): self.called_with = kwargs @@ -417,7 +412,6 @@ class TestUniquenessForDateValidation(TestCase): 'published': datetime.date(2000, 1, 1) } - # Tests for `UniqueForMonthValidator` # ---------------------------------- @@ -434,6 +428,7 @@ class UniqueForMonthSerializer(serializers.ModelSerializer): class UniqueForMonthTests(TestCase): + def setUp(self): self.instance = UniqueForMonthModel.objects.create( slug='existing', published='2017-01-01' @@ -456,7 +451,6 @@ class UniqueForMonthTests(TestCase): 'published': datetime.date(2017, 2, 1) } - # Tests for `UniqueForYearValidator` # ---------------------------------- @@ -473,6 +467,7 @@ class UniqueForYearSerializer(serializers.ModelSerializer): class UniqueForYearTests(TestCase): + def setUp(self): self.instance = UniqueForYearModel.objects.create( slug='existing', published='2017-01-01' @@ -538,25 +533,23 @@ class TestHiddenFieldUniquenessForDateValidation(TestCase): class ValidatorsTests(TestCase): + def test_qs_exists_handles_type_error(self): class TypeErrorQueryset(object): def exists(self): raise TypeError - assert qs_exists(TypeErrorQueryset()) is False def test_qs_exists_handles_value_error(self): class ValueErrorQueryset(object): def exists(self): raise ValueError - assert qs_exists(ValueErrorQueryset()) is False def test_qs_exists_handles_data_error(self): class DataErrorQueryset(object): def exists(self): raise DataError - assert qs_exists(DataErrorQueryset()) is False def test_validator_raises_error_if_not_all_fields_are_provided(self): From 69ccc66133be151d22a8d134a2331357c4b03a2a Mon Sep 17 00:00:00 2001 From: Vignesh Date: Sat, 23 Sep 2017 02:37:14 +0800 Subject: [PATCH 04/11] fix broken test --- 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 814901331..2fdb35b97 100644 --- a/tests/test_model_serializer.py +++ b/tests/test_model_serializer.py @@ -216,7 +216,7 @@ class TestRegularFieldMappings(TestCase): "(u'red', u'Red'), (u'blue', u'Blue'), (u'green', u'Green')" ).replace( "{'max_value': 'Ensure this value is less than or equal to %(limit_value)s.', 'min_value': 'Ensure this value is greater than or equal to %(limit_value)s.'}", - {'max_value': u'Ensure this value is less than or equal to %(limit_value)s.', 'min_value': u'Ensure this value is greater than or equal to %(limit_value)s.'} + "{'max_value': u'Ensure this value is less than or equal to %(limit_value)s.', 'min_value': u'Ensure this value is greater than or equal to %(limit_value)s.'}" ) self.assertEqual(unicode_repr(TestSerializer()), expected) From 31eba2030154abae5ddc4c228bac68413c146e37 Mon Sep 17 00:00:00 2001 From: Vignesh Date: Sat, 23 Sep 2017 02:35:32 +0800 Subject: [PATCH 05/11] WIP Add failing tests for more fields --- tests/test_validators.py | 62 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/tests/test_validators.py b/tests/test_validators.py index 42f5635e5..f0bfdaab1 100644 --- a/tests/test_validators.py +++ b/tests/test_validators.py @@ -1,7 +1,7 @@ import datetime import pytest -from django.core.validators import MinValueValidator, MaxValueValidator +from django.core.validators import MinValueValidator, MaxValueValidator, URLValidator, EmailValidator, MinLengthValidator, MaxLengthValidator from django.db import DataError, models from django.test import TestCase @@ -591,3 +591,63 @@ class ValidatorMessageTests(TestCase): s.is_valid() assert s.errors['price'] == ['Price has to be <= 10.'] + + def test_url_validator_message_is_copied_from_model(self): + class BlogModel(models.Model): + url = models.URLField(validators=[URLValidator(message='This URL is not valid.')]) + + class BlogSerializer(serializers.ModelSerializer): + class Meta: + model = BlogModel + fields = '__all__' + + data = {'url': 'broken url'} + s = BlogSerializer(data=data) + s.is_valid() + + assert s.errors['url'] == ['This URL is not valid.'] + + def test_email_validator_message_is_copied_from_model(self): + class UserModel(models.Model): + email = models.EmailField(validators=[EmailValidator(message='Please enter a valid email.')]) + + class UserSerializer(serializers.ModelSerializer): + class Meta: + model = UserModel + fields = '__all__' + + data = {'email': 'invalid email'} + s = UserSerializer(data=data) + s.is_valid() + + assert s.errors['email'] == ['Please enter a valid email.'] + + def test_min_length_validator_message_is_copied_from_model(self): + class Review(models.Model): + text = models.CharField(max_length=100, validators=[MinLengthValidator(limit_value=5, message='This is too short.')]) + + class ReviewSerializer(serializers.ModelSerializer): + class Meta: + model = Review + fields = '__all__' + + data = {'text': 'Hi'} + s = ReviewSerializer(data=data) + s.is_valid() + + assert s.errors['text'] == ['This is too short.'] + + def test_max_length_validator_message_is_copied_from_model(self): + # Added this test because was expecting is_valid() to be false but it is not. + # Will investigate further + class Post(models.Model): + text = models.CharField(max_length=100, validators=[MaxLengthValidator(limit_value=1, message='This is too long.')]) + + class PostSerializer(serializers.ModelSerializer): + class Meta: + model = Post + fields = '__all__' + + data = {'text': 'A very long text'} + s = PostSerializer(data=data) + assert not s.is_valid() From 8f7c91d3a72881f4ba2a8f69e6341fe6a99d0b05 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Wed, 8 Nov 2017 11:00:32 +0100 Subject: [PATCH 06/11] isort --- tests/test_validators.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_validators.py b/tests/test_validators.py index f0bfdaab1..098bb4aff 100644 --- a/tests/test_validators.py +++ b/tests/test_validators.py @@ -1,7 +1,10 @@ import datetime import pytest -from django.core.validators import MinValueValidator, MaxValueValidator, URLValidator, EmailValidator, MinLengthValidator, MaxLengthValidator +from django.core.validators import ( + EmailValidator, MaxLengthValidator, MaxValueValidator, MinLengthValidator, + MinValueValidator, URLValidator +) from django.db import DataError, models from django.test import TestCase From e7dd68e8fa84e5f98e915bf32980c4119eced498 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Wed, 15 Nov 2017 11:07:37 +0100 Subject: [PATCH 07/11] Pickup custom message for EmailField Set via `error_messages`, since `validate_email` is already present on field. --- rest_framework/utils/field_mapping.py | 3 +++ tests/test_validators.py | 6 ++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/rest_framework/utils/field_mapping.py b/rest_framework/utils/field_mapping.py index ab42fc605..2911265a9 100644 --- a/rest_framework/utils/field_mapping.py +++ b/rest_framework/utils/field_mapping.py @@ -164,6 +164,9 @@ def get_field_kwargs(field_name, model_field): # EmailField does not need to include the validate_email argument, # as it is explicitly added in. if isinstance(model_field, models.EmailField): + custom_message = model_field.error_messages.get("invalid", None) + if custom_message is not None: + kwargs.setdefault('error_messages', {}).update(invalid=custom_message) validator_kwarg = [ validator for validator in validator_kwarg if validator is not validators.validate_email diff --git a/tests/test_validators.py b/tests/test_validators.py index 098bb4aff..7b4d9b654 100644 --- a/tests/test_validators.py +++ b/tests/test_validators.py @@ -2,7 +2,7 @@ import datetime import pytest from django.core.validators import ( - EmailValidator, MaxLengthValidator, MaxValueValidator, MinLengthValidator, + MaxLengthValidator, MaxValueValidator, MinLengthValidator, MinValueValidator, URLValidator ) from django.db import DataError, models @@ -612,7 +612,9 @@ class ValidatorMessageTests(TestCase): def test_email_validator_message_is_copied_from_model(self): class UserModel(models.Model): - email = models.EmailField(validators=[EmailValidator(message='Please enter a valid email.')]) + email = models.EmailField( + error_messages={"invalid": "Please enter a valid email."} + ) class UserSerializer(serializers.ModelSerializer): class Meta: From c3fd86d32311c4164b7de66db70e9aae95f48976 Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Wed, 15 Nov 2017 11:18:32 +0100 Subject: [PATCH 08/11] Pickup custom message for URLField --- rest_framework/utils/field_mapping.py | 3 +++ tests/test_validators.py | 6 ++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/rest_framework/utils/field_mapping.py b/rest_framework/utils/field_mapping.py index 2911265a9..6234c68ca 100644 --- a/rest_framework/utils/field_mapping.py +++ b/rest_framework/utils/field_mapping.py @@ -156,6 +156,9 @@ def get_field_kwargs(field_name, model_field): # URLField does not need to include the URLValidator argument, # as it is explicitly added in. if isinstance(model_field, models.URLField): + custom_message = model_field.error_messages.get("invalid", None) + if custom_message is not None: + kwargs.setdefault('error_messages', {}).update(invalid=custom_message) validator_kwarg = [ validator for validator in validator_kwarg if not isinstance(validator, validators.URLValidator) diff --git a/tests/test_validators.py b/tests/test_validators.py index 7b4d9b654..4df0685c4 100644 --- a/tests/test_validators.py +++ b/tests/test_validators.py @@ -3,7 +3,7 @@ import datetime import pytest from django.core.validators import ( MaxLengthValidator, MaxValueValidator, MinLengthValidator, - MinValueValidator, URLValidator + MinValueValidator ) from django.db import DataError, models from django.test import TestCase @@ -597,7 +597,9 @@ class ValidatorMessageTests(TestCase): def test_url_validator_message_is_copied_from_model(self): class BlogModel(models.Model): - url = models.URLField(validators=[URLValidator(message='This URL is not valid.')]) + url = models.URLField( + error_messages={"invalid": "This URL is not valid."} + ) class BlogSerializer(serializers.ModelSerializer): class Meta: From 4c7b3040d833dba60dc31890b28c86af2bd1f55f Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Wed, 15 Nov 2017 11:32:29 +0100 Subject: [PATCH 09/11] Pickup custom message for min_length validators --- rest_framework/utils/field_mapping.py | 7 ++++--- tests/test_validators.py | 5 ++++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/rest_framework/utils/field_mapping.py b/rest_framework/utils/field_mapping.py index 6234c68ca..f1305e850 100644 --- a/rest_framework/utils/field_mapping.py +++ b/rest_framework/utils/field_mapping.py @@ -209,12 +209,13 @@ def get_field_kwargs(field_name, model_field): # Ensure that min_length is passed explicitly as a keyword arg, # rather than as a validator. - min_length = next(( - validator.limit_value for validator in validator_kwarg + min_length, message = next(( + (validator.limit_value, validator.message) for validator in validator_kwarg if isinstance(validator, validators.MinLengthValidator) - ), None) + ), (None, None)) if min_length is not None and isinstance(model_field, models.CharField): kwargs['min_length'] = min_length + kwargs.setdefault('error_messages', {}).update(min_length=message) validator_kwarg = [ validator for validator in validator_kwarg if not isinstance(validator, validators.MinLengthValidator) diff --git a/tests/test_validators.py b/tests/test_validators.py index 4df0685c4..a008cbc21 100644 --- a/tests/test_validators.py +++ b/tests/test_validators.py @@ -631,7 +631,10 @@ class ValidatorMessageTests(TestCase): def test_min_length_validator_message_is_copied_from_model(self): class Review(models.Model): - text = models.CharField(max_length=100, validators=[MinLengthValidator(limit_value=5, message='This is too short.')]) + text = models.CharField( + max_length=100, + validators=[MinLengthValidator(limit_value=5, message='This is too short.')] + ) class ReviewSerializer(serializers.ModelSerializer): class Meta: From 682c98958ad798675f7710cace4414c80c563dab Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Wed, 15 Nov 2017 11:40:16 +0100 Subject: [PATCH 10/11] Pickup custom length for max_length --- rest_framework/utils/field_mapping.py | 3 +++ tests/test_validators.py | 11 ++++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/rest_framework/utils/field_mapping.py b/rest_framework/utils/field_mapping.py index f1305e850..12d72e9f7 100644 --- a/rest_framework/utils/field_mapping.py +++ b/rest_framework/utils/field_mapping.py @@ -202,6 +202,9 @@ def get_field_kwargs(field_name, model_field): isinstance(model_field, models.TextField) or isinstance(model_field, models.FileField)): kwargs['max_length'] = max_length + custom_message = model_field.error_messages.get("max_length", None) + if custom_message is not None: + kwargs.setdefault('error_messages', {}).update(max_length=custom_message) validator_kwarg = [ validator for validator in validator_kwarg if not isinstance(validator, validators.MaxLengthValidator) diff --git a/tests/test_validators.py b/tests/test_validators.py index a008cbc21..a71436b4f 100644 --- a/tests/test_validators.py +++ b/tests/test_validators.py @@ -2,8 +2,7 @@ import datetime import pytest from django.core.validators import ( - MaxLengthValidator, MaxValueValidator, MinLengthValidator, - MinValueValidator + MaxValueValidator, MinLengthValidator, MinValueValidator ) from django.db import DataError, models from django.test import TestCase @@ -648,10 +647,11 @@ class ValidatorMessageTests(TestCase): assert s.errors['text'] == ['This is too short.'] def test_max_length_validator_message_is_copied_from_model(self): - # Added this test because was expecting is_valid() to be false but it is not. - # Will investigate further class Post(models.Model): - text = models.CharField(max_length=100, validators=[MaxLengthValidator(limit_value=1, message='This is too long.')]) + text = models.CharField( + max_length=1, + error_messages={"max_length": "This is too long"} + ) class PostSerializer(serializers.ModelSerializer): class Meta: @@ -661,3 +661,4 @@ class ValidatorMessageTests(TestCase): data = {'text': 'A very long text'} s = PostSerializer(data=data) assert not s.is_valid() + assert s.errors['text'] == ['This is too long'] From 826fd76825eebde044539b376de965a5505f44db Mon Sep 17 00:00:00 2001 From: Carlton Gibson Date: Wed, 15 Nov 2017 12:26:04 +0100 Subject: [PATCH 11/11] Add workarounds for serializer __repr__ ordering --- rest_framework/utils/field_mapping.py | 22 +++++++++++++--------- tests/test_model_serializer.py | 3 ++- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/rest_framework/utils/field_mapping.py b/rest_framework/utils/field_mapping.py index 12d72e9f7..89e9a6675 100644 --- a/rest_framework/utils/field_mapping.py +++ b/rest_framework/utils/field_mapping.py @@ -3,6 +3,7 @@ Helper functions for mapping model fields to a dictionary of default keyword arguments that should be used for their equivalent serializer fields. """ import inspect +from collections import OrderedDict from django.core import validators from django.db import models @@ -130,10 +131,11 @@ def get_field_kwargs(field_name, model_field): max_value, message = next(( (validator.limit_value, validator.message) for validator in validator_kwarg if isinstance(validator, validators.MaxValueValidator) - ), (None, None)) + ), (None, '')) if max_value is not None and isinstance(model_field, NUMERIC_FIELD_TYPES): kwargs['max_value'] = max_value - kwargs['error_messages'] = {'max_value': message} + if message != '': + kwargs.setdefault('error_messages', OrderedDict()).update(max_value=message) validator_kwarg = [ validator for validator in validator_kwarg if not isinstance(validator, validators.MaxValueValidator) @@ -144,10 +146,11 @@ def get_field_kwargs(field_name, model_field): min_value, message = next(( (validator.limit_value, validator.message) for validator in validator_kwarg if isinstance(validator, validators.MinValueValidator) - ), (None, None)) + ), (None, '')) if min_value is not None and isinstance(model_field, NUMERIC_FIELD_TYPES): kwargs['min_value'] = min_value - kwargs.setdefault('error_messages', {}).update(min_value=message) + if message != '': + kwargs.setdefault('error_messages', OrderedDict()).update(min_value=message) validator_kwarg = [ validator for validator in validator_kwarg if not isinstance(validator, validators.MinValueValidator) @@ -202,9 +205,9 @@ def get_field_kwargs(field_name, model_field): isinstance(model_field, models.TextField) or isinstance(model_field, models.FileField)): kwargs['max_length'] = max_length - custom_message = model_field.error_messages.get("max_length", None) - if custom_message is not None: - kwargs.setdefault('error_messages', {}).update(max_length=custom_message) + custom_message = model_field.error_messages.get("max_length", '') + if custom_message != '': + kwargs.setdefault('error_messages', OrderedDict()).update(max_length=custom_message) validator_kwarg = [ validator for validator in validator_kwarg if not isinstance(validator, validators.MaxLengthValidator) @@ -215,10 +218,11 @@ def get_field_kwargs(field_name, model_field): min_length, message = next(( (validator.limit_value, validator.message) for validator in validator_kwarg if isinstance(validator, validators.MinLengthValidator) - ), (None, None)) + ), (None, '')) if min_length is not None and isinstance(model_field, models.CharField): kwargs['min_length'] = min_length - kwargs.setdefault('error_messages', {}).update(min_length=message) + if message != '': + kwargs.setdefault('error_messages', OrderedDict()).update(min_length=message) validator_kwarg = [ validator for validator in validator_kwarg if not isinstance(validator, validators.MinLengthValidator) diff --git a/tests/test_model_serializer.py b/tests/test_model_serializer.py index 2fdb35b97..f77b47a72 100644 --- a/tests/test_model_serializer.py +++ b/tests/test_model_serializer.py @@ -200,7 +200,7 @@ class TestRegularFieldMappings(TestCase): expected = dedent(""" TestSerializer(): id = IntegerField(label='ID', read_only=True) - value_limit_field = IntegerField(error_messages={'max_value': 'Ensure this value is less than or equal to %(limit_value)s.', 'min_value': 'Ensure this value is greater than or equal to %(limit_value)s.'}, max_value=10, min_value=1) + value_limit_field = IntegerField(error_messages=OrderedDict([('max_value', 'Ensure this value is less than or equal to %(limit_value)s.'), ('min_value', 'Ensure this value is greater than or equal to %(limit_value)s.')]), max_value=10, min_value=1) length_limit_field = CharField(max_length=12, min_length=3) blank_field = CharField(allow_blank=True, max_length=10, required=False) null_field = IntegerField(allow_null=True, required=False) @@ -218,6 +218,7 @@ class TestRegularFieldMappings(TestCase): "{'max_value': 'Ensure this value is less than or equal to %(limit_value)s.', 'min_value': 'Ensure this value is greater than or equal to %(limit_value)s.'}", "{'max_value': u'Ensure this value is less than or equal to %(limit_value)s.', 'min_value': u'Ensure this value is greater than or equal to %(limit_value)s.'}" ) + self.maxDiff = None self.assertEqual(unicode_repr(TestSerializer()), expected) def test_method_field(self):