Ticket #13878: 01-formset-refactoring.diff

django/forms/formsets.py

@@ -42 +42 @@
         self.initial = initial
         self.error_class = error_class
         self._errors = None
-        self._non_form_errors = None
         # construct the forms in the formset
         self._construct_forms()
 
@@ -152 +151 @@
         return [form.cleaned_data for form in self.forms]
     cleaned_data = property(_get_cleaned_data)
 
+    def _get_valid_forms(self):
+        """
+        Returns a list of valid and filled forms, possibly in the order
+        specified by the incoming data (if ordering is allowed).
+        """
+        if self._errors is None:
+            self.full_clean()
+        return self._valid_forms
+    valid_forms = property(_get_valid_forms)
+
     def _get_deleted_forms(self):
         """
         Returns a list of forms that have been marked for deletion. Raises an
         AttributeError if deletion is not allowed.
         """
-        if not self.is_valid() or not self.can_delete:
-            raise AttributeError("'%s' object has no attribute 'deleted_forms'" % self.__class__.__name__)
-        # construct _deleted_form_indexes which is just a list of form indexes
-        # that have had their deletion widget set to True
-        if not hasattr(self, '_deleted_form_indexes'):
-            self._deleted_form_indexes = []
-            for i in range(0, self.total_form_count()):
-                form = self.forms[i]
-                # if this is an extra form and hasn't changed, don't consider it
-                if i >= self.initial_form_count() and not form.has_changed():
-                    continue
-                if self._should_delete_form(form):
-                    self._deleted_form_indexes.append(i)
-        return [self.forms[i] for i in self._deleted_form_indexes]
+        if self._errors is None:
+            self.full_clean()
+        return self._deleted_forms
     deleted_forms = property(_get_deleted_forms)
 
     def _get_ordered_forms(self):
         """
-        Returns a list of form in the order specified by the incoming data.
+        Returns a list of forms in the order specified by the incoming data.
         Raises an AttributeError if ordering is not allowed.
         """
-        if not self.is_valid() or not self.can_order:
-            raise AttributeError("'%s' object has no attribute 'ordered_forms'" % self.__class__.__name__)
-        # Construct _ordering, which is a list of (form_index, order_field_value)
-        # tuples. After constructing this list, we'll sort it by order_field_value
-        # so we have a way to get to the form indexes in the order specified
-        # by the form data.
-        if not hasattr(self, '_ordering'):
-            self._ordering = []
-            for i in range(0, self.total_form_count()):
-                form = self.forms[i]
-                # if this is an extra form and hasn't changed, don't consider it
-                if i >= self.initial_form_count() and not form.has_changed():
-                    continue
-                # don't add data marked for deletion to self.ordered_data
-                if self.can_delete and self._should_delete_form(form):
-                    continue
-                self._ordering.append((i, form.cleaned_data[ORDERING_FIELD_NAME]))
-            # After we're done populating self._ordering, sort it.
-            # A sort function to order things numerically ascending, but
-            # None should be sorted below anything else. Allowing None as
-            # a comparison value makes it so we can leave ordering fields
-            # blamk.
-            def compare_ordering_values(x, y):
-                if x[1] is None:
-                    return 1
-                if y[1] is None:
-                    return -1
-                return x[1] - y[1]
-            self._ordering.sort(compare_ordering_values)
-        # Return a list of form.cleaned_data dicts in the order spcified by
-        # the form data.
-        return [self.forms[i[0]] for i in self._ordering]
+        if self._errors is None:
+            self.full_clean()
+        return self._ordered_forms
     ordered_forms = property(_get_ordered_forms)
 
     #@classmethod
@@ -223 +192 @@
         form -- i.e., from formset.clean(). Returns an empty ErrorList if there
         are none.
         """
-        if self._non_form_errors is not None:
-            return self._non_form_errors
-        return self.error_class()
+        if self._errors is None:
+            self.full_clean()
+        return self._non_form_errors
 
     def _get_errors(self):
         """
@@ -247 +216 @@
 
     def is_valid(self):
         """
-        Returns True if form.errors is empty for every form in self.forms.
+        Returns True if form 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
-        for i in range(0, self.total_form_count()):
-            form = self.forms[i]
-            if self.can_delete:
-                if self._should_delete_form(form):
-                    # This form is going to be deleted so any of its errors
-                    # should not cause the entire formset to be invalid.
-                    continue
-            if bool(self.errors[i]):
-                forms_valid = False
-        return forms_valid and not bool(self.non_form_errors())
+        if self._errors is None:
+            self.full_clean()
+        return self._is_valid
 
     def full_clean(self):
         """
-        Cleans all of self.data and populates self._errors.
+        Cleans all forms, populates self._errors, self._non_form_errors and
+        self._is_valid. If valid, populates also self._valid_forms,
+        self._ordered_forms (if can_order is True) and self._deleted_forms
+        (if can_delete is True).
         """
         self._errors = []
+        self._non_form_errors = self.error_class()
         if not self.is_bound: # Stop further processing.
+            self._is_valid = False
             return
-        for i in range(0, self.total_form_count()):
+        # Loop over every form here.
+        self._is_valid = True
+        valid_form_indexes, deleted_form_indexes = [], []
+        for i in range(self.total_form_count()):
             form = self.forms[i]
+            # Add errors from form.
             self._errors.append(form.errors)
+            # Consider only forms that are initial or changed.
+            if i < self.initial_form_count() or form.has_changed():
+                if self.can_delete and self._should_delete_form(form):
+                    # Add index of form that should be deleted.
+                    deleted_form_indexes.append(i)
+                else:
+                    if form.is_valid():
+                        if self.can_order:
+                            # Add index and order field value of valid form.
+                            valid_form_indexes.append((i, form.cleaned_data[ORDERING_FIELD_NAME]))
+                        else:
+                            # Add index of valid form.
+                            valid_form_indexes.append(i)
+                    else:
+                        # Mark formset as not valid but do not stop to make
+                        # sure validation gets triggered for every form.
+                        self._is_valid = False
         # Give self.clean() a chance to do cross-form validation.
         try:
             self.clean()
         except ValidationError, e:
             self._non_form_errors = self.error_class(e.messages)
+            self._is_valid = False
+        # If formset is valid, populates self._valid_forms,
+        # self._ordered_forms (if can_order is True) and self._deleted_forms
+        # (if can_delete is True).
+        if self._is_valid:
+            if self.can_order:
+                # A sort function to order things numerically ascending, but
+                # None should be sorted below anything else. Allowing None as
+                # a comparison value makes it so we can leave ordering fields
+                # blank.
+                def compare_ordering_values(x, y):
+                    if x[1] is None:
+                        return 1
+                    if y[1] is None:
+                        return -1
+                    return x[1] - y[1]
+                valid_form_indexes.sort(compare_ordering_values)
+                self._valid_forms = [self.forms[i[0]] for i in valid_form_indexes]
+                self._ordered_forms = self._valid_forms
+            else:
+                self._valid_forms = [self.forms[i] for i in valid_form_indexes]
+            if self.can_delete:
+                self._deleted_forms = [self.forms[i] for i in deleted_form_indexes]
 
     def clean(self):
         """