`BaseFormSet._should_delete_form` should use `form.cleaned_data`

[Simon Charette]

Now that #5524 is ​fixed the BaseFormSet._should_delete_form method could be cleaned-up a bit. Original ​method comment:

# The way we lookup the value of the deletion field here takes
# more code than we'd like, but the form's cleaned_data will
# not exist if the form is invalid.

Patch coming up.

[Simon Charette]

All tests pass on python 2.7.3rc1 sqlite3. I'm wondering if we shouldn't add and accessor for Form.cleaned_data that would call trigger validation under the hood, the same way Form.errors does. We could still raise an AttributeError if form.is_bound() == False:

def _get_cleaned_data(self):
    if not self.is_bound():
        raise AttributeError("'%s' object has no attribute 'cleaned_data'" % self.__class__.__name__)
    self.errors # Accessing `errors` since it knows whether or not it should call `full_clean`
    return self._cleaned_data

def _set_cleaned_data(self, value):
    self._cleaned_data = cleaned_data

cleaned_data = property(_get_cleaned_data, _set_cleaned_data)

[Simon Charette]

Added a new patch that fixes a bogus testcase which was saving a formset before making sure it was valid.

[Claude Paroz]

Interestingly, I have uploaded a similar patch to #10711 (https://code.djangoproject.com/attachment/ticket/10711/10711-2.diff). I will close it in favour of this one. Maybe there is something to merge between both patches?

[Simon Charette]

this patch looks better thank mines since you don't have to change the test suite and you reuse the deleted_forms code path.

[Simon Charette]

claudep's patch for #10711 which happens to fix this issue

[Simon Charette]

claudep, I'm marking the patch you've written for #10711 as RFC since it passes all tests (python 2.7.3rc1 sqlite3) and is the definitive way to clean up _should_delete_form with backward compatibility in mind. My patches we're incomplete since they introduced an obscure raises of AttributeError when attempting to save a not-yet-validated formset (which is a foolish thing to do anyway), see ticket-18751.2.diff testsuite changes.

[Claude Paroz <claude@…>]

In [fb3d916c201d5882098bbcc94f60765412ba7aff]:

Fixed #18751 -- Cleaned up BaseFormSet._should_delete_form

We can do that now that cleaned_data is guaranteed to be
present. Related to [121fd109].
Thanks Simon Charette for his work on the ticket.