[Django] #32911: django.forms.BaseFormSet.is_valid does not respect TOTAL_FORM_COUNT
Django
-------------------------------------+-------------------------------------
Reporter: | Owner: nobody
lorenzomorandini |
Type: Bug | Status: new
Component: Forms | Version: 3.2
Severity: Normal | Keywords: form, formset
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
#29113 (ticket) does not respect TOTAL_FORM_COUNT variable anymore when
validating a formset.
Before:
{{{#!python
def is_valid(self):
"""Return True if every form in self.forms is valid."""
if not self.is_bound:
return False
# We loop over every form.errors here rather than short circuiting on
the
# first failure to make sure validation gets triggered for every form.
forms_valid = True
# This triggers a full clean.
self.errors
for i in range(0, self.total_form_count()):
form = self.forms[i]
if self.can_delete and self._should_delete_form(form):
# This form is going to be deleted so any of its errors
# shouldn't cause the entire formset to be invalid.
continue
forms_valid &= form.is_valid()
return forms_valid and not self.non_form_errors()
}}}
After:
{{{#!python
def is_valid(self):
"""Return True if every form in self.forms is valid."""
if not self.is_bound:
return False
# Accessing errors triggers a full clean the first time only.
self.errors
# List comprehension ensures is_valid() is called for all forms.
# Forms due to be deleted shouldn't cause the formset to be invalid.
forms_valid = all([
form.is_valid() for form in self.forms
if not (self.can_delete and self._should_delete_form(form))
])
return forms_valid and not self.non_form_errors()
}}}
The previous version used `total_form_count()` while the new version just
iterates over `self.forms`.
This broke a view in my case where I had:
* a formset with some forms rendered in a page
* the last form of the formset was hidden and did NOT count in
`TOTAL_FORM_COUNT`
Before the last form was ignored (and correctly not submitted), now it
gets validated (even if it will not be submitted).
Depending on the `can_delete` param of the formset, this issue could lead
to two outcomes:
* `can_delete=True`: `_should_delete_form()` throws `AttributeError:
<form> has no attribute cleaned_data`
* `can_delete=False`: formset is invalid because of the last form
I consider it a bug since all the rest of the code of `BaseFormset` takes
`TOTAL_FORM_COUNT` in consideration
--
Ticket URL: <https://code.djangoproject.com/ticket/32911>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Lorenzo Morandini | Owner: nobody
Type: Bug | Status: closed
Component: Forms | Version: 3.2
Severity: Normal | Resolution: needsinfo
Keywords: form, formset | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* cc: Jon Dufresne (added)
* status: new => closed
* resolution: => needsinfo
Comment:
Thanks for the report, however I have some doubts.
> #29113 (ticket) does not respect TOTAL_FORM_COUNT variable anymore when
validating a formset.
That's not true. This was changed in
b2717c7532cd35ab9e80c92c6b9f070e62e7ae88 which has nothing to do with
#29113. Also, `.forms` property uses `TOTAL_FORM_COUNT` so I'm not sure
how this could introduce a regression. Maybe you're modifying
`TOTAL_FORM_COUNT` dynamically 🤔, but that's not really supported. Can
you provide a sample project?
> I consider it a bug since all the rest of the code of `BaseFormset`
takes `TOTAL_FORM_COUNT` in consideration.
As far as I'm aware all methods iterate over `.forms`.
--
Ticket URL: <https://code.djangoproject.com/ticket/32911#comment:1>
Django
Reporter: Lorenzo Morandini | Owner: nobody
Type: Bug | Status: closed
Severity: Normal | Resolution: needsinfo
Keywords: form, formset | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Lorenzo Morandini):
The problem was actually from a third party library that overrided the
property `forms` and I did not notice, sorry.
--
Ticket URL: <https://code.djangoproject.com/ticket/32911#comment:2>
Django
Reporter: Lorenzo Morandini | Owner: nobody
Type: Bug | Status: closed
Severity: Normal | Resolution: invalid
Keywords: form, formset | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz):
* resolution: needsinfo => invalid
--
Ticket URL: <https://code.djangoproject.com/ticket/32911#comment:3>