Django custom form validation best practices?
I have a form that contains 5 pairs of locations and descriptions. I have three sets of validations that need to be done
- you need to enter at least one location
- for the first location, you must have a description
- for each remaining pair of locations and description
After reading the Django documentation, I came up with the following code to do these custom validations
def clean(self):
cleaned_data = self.cleaned_data
location1 = cleaned_data.get('location1')
location2 = cleaned_data.get('location2')
location3 = cleaned_data.get('location3')
location4 = cleaned_data.get('location4')
location5 = cleaned_data.get('location5')
description1 = cleaned_data.get('description1')
description2 = cleaned_data.get('description2')
description3 = cleaned_data.get('description3')
description4 = cleaned_data.get('description4')
description5 = cleaned_data.get('description5')
invalid_pairs_msg = u"You must specify a location and description"
# We need to make sure that we have pairs of locations and descripti开发者_运维技巧ons
if not location1:
self._errors['location1'] = ErrorList([u"At least one location is required"])
if location1 and not description1:
self._errors['description1'] = ErrorList([u"Description for this location required"])
if (description2 and not location2) or (location2 and not description2):
self._errors['description2'] = ErrorList([invalid_pairs_msg])
if (description3 and not location3) or (location3 and not description3):
self._errors['description3'] = ErrorList([invalid_pairs_msg])
if (description4 and not location4) or (location4 and not description4):
self._errors['description4'] = ErrorList([invalid_pairs_msg])
if (description5 and not location5) or (location5 and not description5):
self._errors['description5'] = ErrorList([invalid_pairs_msg])
return cleaned_data
Now, it works but it looks really ugly. I'm looking for a more "Pythonic" and "Djangoist"(?) way to do this. Thanks in advance.
First thing you can do is simplify your testing for those cases where you want to see if only one of the two fields is populated. You can implement logical xor
this way:
if bool(description2) != bool(location2):
or this way:
if bool(description2) ^ bool(location2):
I also think this would be more clear if you implemented a clean method for each field separately, as explained in the docs. This makes sure the error will show up on the right field and lets you just raise a forms.ValidationError
rather than accessing the _errors
object directly.
For example:
def _require_together(self, field1, field2):
a = self.cleaned_data.get(field1)
b = self.cleaned_data.get(field2)
if bool(a) ^ bool(b):
raise forms.ValidationError(u'You must specify a location and description')
return a
# use clean_description1 rather than clean_location1 since
# we want the error to be on description1
def clean_description1(self):
return _require_together('description1', 'location1')
def clean_description2(self):
return _require_together('description2', 'location2')
def clean_description3(self):
return _require_together('description3', 'location3')
def clean_description4(self):
return _require_together('description4', 'location4')
def clean_description5(self):
return _require_together('description5', 'location5')
In order to get the behavior where location1
is required, just use required=True
for that field and it'll be handled automatically.
At least you can reduce some code. Have 'location1' and 'description1' Required=True (as TM and stefanw pointed out). Then,
def clean(self):
n=5
data = self.cleaned_data
l_d = [(data.get('location'+i),data.get('description'+i)) for i in xrange(1,n+1)]
invalid_pairs_msg = u"You must specify a location and description"
for i in xrange(1,n+1):
if (l_d[i][1] and not l_d[i][0]) or (l_d[i][0] and not l_d[i][1]):
self._errors['description'+i] = ErrorList([invalid_pairs_msg])
return data
still ugly though...
精彩评论