BaseFormSet.is_valid() produces ValidationError when there is no management form
|Reported by:||anonymous||Owned by:||patrys|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||yes|
I was torn between reporting this as a bug or a feature request, but then I thought if I make it a feature request it will most likely break a lot of Django apps and hence I guess that means it's more of a bug...
Anyway so the line in question is django/forms/formsets.py:292 (in Django version 1.6.1):
for i in range(0, self.total_form_count()):
...where the self.total_form_count() executes this line django/forms/formsets.py:106 (in Django version 1.6.1):
return min(self.management_form.cleaned_data[TOTAL_FORM_COUNT], self.absolute_max)
..which then raises this exception django/forms/formsets.py:87 (in Django version 1.6.1):
raise ValidationError( _('ManagementForm data is missing or has been tampered with'), code='missing_management_form', )
That stack trace occurs if/when a user submits a formset after stripping out the management form hidden fields.
I have been using Django for a few years now and have never come across an exception being raised by a form/formset is_valid() call before. So my point is that I believe this exception should never be allowed to leave the BaseFormSet.is_valid() call, because it is an oddball behaviour compared to the rest of the is_valid() implementations.
I.e. I believe there should be a check in BaseFormSet.is_valid() which checks for the presence of a valid management form (first) and returns False if it is not present, as opposed to raising an exception.
Yes I could wrap the is_valid() call in a try/catch, but I believe this is an unnecessary hack caused by a bad design deviation of the implementation of the BaseFormSet.is_valid() method.
I didn't bother creating a patch and test cases, because I have a feeling this will get rejected or something like that, but I just thought I should bring this up, as I can't find mention of it anywhere and it seems important to me.
Change History (13)
comment:1 Changed 14 months ago by anonymous
- Needs documentation unset
- Needs tests unset
- Patch needs improvement unset
comment:2 Changed 14 months ago by anonymous
- Resolution set to invalid
- Status changed from new to closed
comment:3 Changed 5 months ago by patrys
- Resolution invalid deleted
- Status changed from closed to new
comment:4 Changed 5 months ago by patrys
- Owner changed from nobody to patrys
- Status changed from new to assigned
comment:5 Changed 5 months ago by timgraham
- Triage Stage changed from Unreviewed to Accepted
- Version changed from 1.6 to master