django.forms.BaseFormSet.is_valid does not respect TOTAL_FORM_COUNT

[Lorenzo Morandini]

#29113 (ticket) does not respect TOTAL_FORM_COUNT variable anymore when validating a formset.

Before:

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:

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

[Mariusz Felisiak]

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.

[Lorenzo Morandini]

The problem was actually from a third party library that overrided the property forms and I did not notice, sorry.