Browse files

Fixed #17840 -- Generalized named placeholders in form error messages

Also fixed plural messages for DecimalField.
  • Loading branch information...
1 parent 9ac4dbd commit be9ae693c46021fd3a70c0ec21dd566960b29ffb @claudep claudep committed Apr 13, 2013
Showing with 28 additions and 15 deletions.
  1. +20 −7 django/forms/fields.py
  2. +4 −4 django/forms/models.py
  3. +4 −4 tests/forms_tests/tests/test_error_messages.py
View
27 django/forms/fields.py
@@ -292,9 +292,18 @@ def widget_attrs(self, widget):
class DecimalField(IntegerField):
default_error_messages = {
'invalid': _('Enter a number.'),
- 'max_digits': _('Ensure that there are no more than %s digits in total.'),
- 'max_decimal_places': _('Ensure that there are no more than %s decimal places.'),
- 'max_whole_digits': _('Ensure that there are no more than %s digits before the decimal point.')
+ 'max_digits': ungettext_lazy(
+ 'Ensure that there are no more than %(max)s digit in total.',
+ 'Ensure that there are no more than %(max)s digits in total.',
+ 'max'),
+ 'max_decimal_places': ungettext_lazy(
+ 'Ensure that there are no more than %(max)s decimal place.',
+ 'Ensure that there are no more than %(max)s decimal places.',
+ 'max'),
+ 'max_whole_digits': ungettext_lazy(
+ 'Ensure that there are no more than %(max)s digit before the decimal point.',
+ 'Ensure that there are no more than %(max)s digits before the decimal point.',
+ 'max'),
}
def __init__(self, max_value=None, min_value=None, max_digits=None, decimal_places=None, *args, **kwargs):
@@ -341,11 +350,15 @@ def validate(self, value):
whole_digits = digits - decimals
if self.max_digits is not None and digits > self.max_digits:
- raise ValidationError(self.error_messages['max_digits'] % self.max_digits)
+ raise ValidationError(self.error_messages['max_digits'] % {
+ 'max': self.max_digits})
if self.decimal_places is not None and decimals > self.decimal_places:
- raise ValidationError(self.error_messages['max_decimal_places'] % self.decimal_places)
- if self.max_digits is not None and self.decimal_places is not None and whole_digits > (self.max_digits - self.decimal_places):
- raise ValidationError(self.error_messages['max_whole_digits'] % (self.max_digits - self.decimal_places))
+ raise ValidationError(self.error_messages['max_decimal_places'] % {
+ 'max': self.decimal_places})
+ if (self.max_digits is not None and self.decimal_places is not None
+ and whole_digits > (self.max_digits - self.decimal_places)):
+ raise ValidationError(self.error_messages['max_whole_digits'] % {
+ 'max': (self.max_digits - self.decimal_places)})
return value
def widget_attrs(self, widget):
View
8 django/forms/models.py
@@ -1039,9 +1039,9 @@ class ModelMultipleChoiceField(ModelChoiceField):
hidden_widget = MultipleHiddenInput
default_error_messages = {
'list': _('Enter a list of values.'),
- 'invalid_choice': _('Select a valid choice. %s is not one of the'
+ 'invalid_choice': _('Select a valid choice. %(value)s is not one of the'
' available choices.'),
- 'invalid_pk_value': _('"%s" is not a valid value for a primary key.')
+ 'invalid_pk_value': _('"%(pk)s" is not a valid value for a primary key.')
}
def __init__(self, queryset, cache_choices=False, required=True,
@@ -1063,12 +1063,12 @@ def clean(self, value):
try:
self.queryset.filter(**{key: pk})
except ValueError:
- raise ValidationError(self.error_messages['invalid_pk_value'] % pk)
+ raise ValidationError(self.error_messages['invalid_pk_value'] % {'pk': pk})
qs = self.queryset.filter(**{'%s__in' % key: value})
pks = set([force_text(getattr(o, key)) for o in qs])
for val in value:
if force_text(val) not in pks:
- raise ValidationError(self.error_messages['invalid_choice'] % val)
+ raise ValidationError(self.error_messages['invalid_choice'] % {'value': val})
# Since this overrides the inherited ModelChoiceField.clean
# we run custom validators here
self.run_validators(value)
View
8 tests/forms_tests/tests/test_error_messages.py
@@ -60,9 +60,9 @@ def test_decimalfield(self):
'invalid': 'INVALID',
'min_value': 'MIN VALUE IS %(limit_value)s',
'max_value': 'MAX VALUE IS %(limit_value)s',
- 'max_digits': 'MAX DIGITS IS %s',
- 'max_decimal_places': 'MAX DP IS %s',
- 'max_whole_digits': 'MAX DIGITS BEFORE DP IS %s',
+ 'max_digits': 'MAX DIGITS IS %(max)s',
+ 'max_decimal_places': 'MAX DP IS %(max)s',
+ 'max_whole_digits': 'MAX DIGITS BEFORE DP IS %(max)s',
}
f = DecimalField(min_value=5, max_value=10, error_messages=e)
self.assertFormErrors(['REQUIRED'], f.clean, '')
@@ -254,7 +254,7 @@ def test_modelchoicefield(self):
# ModelMultipleChoiceField
e = {
'required': 'REQUIRED',
- 'invalid_choice': '%s IS INVALID CHOICE',
+ 'invalid_choice': '%(value)s IS INVALID CHOICE',
'list': 'NOT A LIST OF VALUES',
}
f = ModelMultipleChoiceField(queryset=ChoiceModel.objects.all(), error_messages=e)

4 comments on commit be9ae69

@carljm
Django member

@claudep Don't you think this deserves a mention in the backwards-incompatibility release notes? Overriding these error messages is definitely intended as public API, and if I'm not mistaken this change will break anyone who has done that and still has positional keys in their overridden message. (I agree it's still a change we need to make, just think it needs to be in the release notes.)

@claudep
Django member
@carljm
Django member

@claudep Looks pretty good, although it seems like the release note is focused on the wrong aspect. Generally users won't be "using a dictionary when formatting" these error messages, because Django is the one that provides the dictionary and does the formatting. So it seems like it would make more sense for the release note to focus on the construction of custom error message placeholders; for instance "You must use the appropriate named placeholders (e.g. %(max)s) rather than positional placeholders (e.g. %s) in custom error messages for DecimalField and ModelMultipleChoiceField; see the corresponding field documentation etc." Does that make sense, or am I missing something?

@claudep
Django member

Of course, that makes sense!

Please sign in to comment.