newforms-admin: Cleaned up FormSet internals and renamed formset.form_list to formset.forms. Better, but still not quite there.

git-svn-id: http://code.djangoproject.com/svn/django/branches/newforms-admin@5325 bcc190cf-cafb-0310-a4f2-bffc1f526a37

3 files changed, 84 insertions, 74 deletions

diff --git a/django/newforms/forms.py b/django/newforms/forms.py
index a6409d8504..952530bbd5 100644
--- a/django/newforms/forms.py
+++ b/django/newforms/forms.py
@@ -226,6 +226,12 @@ class BaseForm(StrAndUnicode):
         """
         return self.cleaned_data
 
+    def reset(self):
+        """Return this form to the state it was in before data was passed to it."""
+        self.data = {}
+        self.is_bound = False
+        self.__errors = None
+
 class Form(BaseForm):
     "A collection of Fields, plus their associated data."
     # This is a separate class from BaseForm in order to abstract the way
diff --git a/django/newforms/formsets.py b/django/newforms/formsets.py
index 6eaaf12a5e..d493c04a5d 100644
--- a/django/newforms/formsets.py
+++ b/django/newforms/formsets.py
@@ -5,11 +5,6 @@ FORM_COUNT_FIELD_NAME = 'COUNT'
 ORDERING_FIELD_NAME = 'ORDER'
 DELETION_FIELD_NAME = 'DELETE'
 
-def formset_for_form(form, num_extra=1, orderable=False, deletable=False):
-    """Return a FormSet for the given form class."""
-    attrs = {'form_class': form, 'num_extra': num_extra, 'orderable': orderable, 'deletable': deletable}
-    return type(form.__name__ + 'FormSet', (BaseFormSet,), attrs)
-
 class ManagementForm(forms.Form):
     """
     ``ManagementForm`` is used to keep track of how many form instances
@@ -35,42 +30,62 @@ class BaseFormSet(object):
             if self.management_form.is_valid():
                 self.total_forms = self.management_form.cleaned_data[FORM_COUNT_FIELD_NAME]
                 self.required_forms = self.total_forms - self.num_extra
+                self.change_form_count = self.total_forms - self.num_extra
             else:
                 # not sure that ValidationError is the best thing to raise here
                 raise forms.ValidationError('ManagementForm data is missing or has been tampered with')
         elif initial:
+            self.change_form_count = len(initial)
             self.required_forms = len(initial)
             self.total_forms = self.required_forms + self.num_extra
             self.management_form = ManagementForm(initial={FORM_COUNT_FIELD_NAME: self.total_forms}, auto_id=self.auto_id, prefix=self.prefix)
         else:
+            self.change_form_count = 0
             self.required_forms = 0
             self.total_forms = self.num_extra
             self.management_form = ManagementForm(initial={FORM_COUNT_FIELD_NAME: self.total_forms}, auto_id=self.auto_id, prefix=self.prefix)
 
-    def _get_form_list(self):
-        """Return a list of Form instances."""
-        if not hasattr(self, '_form_list'):
-            self._form_list = []
-            for i in range(0, self.total_forms):
-                kwargs = {'data': self.data, 'auto_id': self.auto_id, 'prefix': self.add_prefix(i)}
-                if self.initial and i < self.required_forms:
-                     kwargs['initial'] = self.initial[i]
-                form_instance = self.form_class(**kwargs)
-                # HACK: if the form was not completed, replace it with a blank one
-                if self.data and i >= self.required_forms and form_instance.is_empty():
-                    form_instance = self.form_class(auto_id=self.auto_id, prefix=self.add_prefix(i))
-                self.add_fields(form_instance, i)
-                self._form_list.append(form_instance)
-        return self._form_list
+    def _get_add_forms(self):
+        """Return a list of all the change forms in this ``FormSet``."""
+        Form = self.form_class
+        if not hasattr(self, '_add_forms'):
+            add_forms = []
+            for i in range(self.change_form_count, self.total_forms):
+                kwargs = {'auto_id': self.auto_id, 'prefix': self.add_prefix(i)}
+                if self.data:
+                    kwargs['data'] = self.data
+                add_form = Form(**kwargs)
+                self.add_fields(add_form, i)
+                add_forms.append(add_form)
+            self._add_forms = add_forms
+        return self._add_forms
+    add_forms = property(_get_add_forms)
+
+    def _get_change_forms(self):
+        """Return a list of all the change forms in this ``FormSet``."""
+        Form = self.form_class
+        if not hasattr(self, '_add_forms'):
+            change_forms = []
+            for i in range(0, self.change_form_count):
+                kwargs = {'auto_id': self.auto_id, 'prefix': self.add_prefix(i)}
+                if self.data:
+                    kwargs['data'] = self.data
+                if self.initial:
+                    kwargs['initial'] = self.initial[i]
+                change_form = Form(**kwargs)
+                self.add_fields(change_form, i)
+                change_forms.append(change_form)
+            self._change_forms= change_forms
+        return self._change_forms
+    change_forms = property(_get_change_forms)
 
-    form_list = property(_get_form_list)
+    def _forms(self):
+        return self.change_forms + self.add_forms
+    forms = property(_forms)
 
     def full_clean(self):
-        """
-        Cleans all of self.data and populates self.__errors and self.cleaned_data.
-        """
+        """Cleans all of self.data and populates self.__errors and self.cleaned_data."""
         is_valid = True
-        
         errors = []
         if not self.is_bound: # Stop further processing.
             self.__errors = errors
@@ -78,59 +93,46 @@ class BaseFormSet(object):
         cleaned_data = []
         deleted_data = []
         
-        self._form_list = []
-        # step backwards through the forms so when we hit the first filled one
-        # we can easily require the rest without backtracking
+        # Process change forms
+        for form in self.change_forms:
+            if form.is_valid():
+                if self.deletable and form.cleaned_data[DELETION_FIELD_NAME]:
+                    deleted_data.append(form.cleaned_data)
+                else:
+                    cleaned_data.append(form.cleaned_data)
+            else:
+                is_valid = False
+            errors.append(form.errors)
+        
+        # Process add forms in reverse so we can easily tell when the remaining
+        # ones should be required.
         required = False
-        for i in range(self.total_forms-1, -1, -1):
-            kwargs = {'data': self.data, 'auto_id': self.auto_id, 'prefix': self.add_prefix(i)}
-            
-            # prep initial data if there is some
-            if self.initial and i < self.required_forms:
-                kwargs['initial'] = self.initial[i]
-                
-            # create the form instance
-            form = self.form_class(**kwargs)
-            self.add_fields(form, i)
-            
-            if self.data and (i < self.required_forms or not form.is_empty(exceptions=[ORDERING_FIELD_NAME])):
-                required = True # forms cannot be empty anymore
-            
-            # HACK: if the form is empty and not required, replace it with a blank one
-            # this is necessary to keep form.errors empty
-            if not required and self.data and form.is_empty(exceptions=[ORDERING_FIELD_NAME]):
-                form = self.form_class(auto_id=self.auto_id, prefix=self.add_prefix(i))
-                self.add_fields(form, i)
+        add_errors = []
+        for i in range(len(self.add_forms)-1, -1, -1):
+            form = self.add_forms[i]
+            # If an add form is empty, reset it so it won't have any errors
+            if form.is_empty([ORDERING_FIELD_NAME]) and not required:
+                form.reset()
+                continue
             else:
-                # if the formset is still vaild overall and this form instance
-                # is valid, keep appending to cleaned_data
-                if is_valid and form.is_valid():
-                    if self.deletable and form.cleaned_data[DELETION_FIELD_NAME]:
-                        deleted_data.append(form.cleaned_data)
-                    else:
-                        cleaned_data.append(form.cleaned_data)
+                required = True
+                if form.is_valid():
+                    cleaned_data.append(form.cleaned_data)
                 else:
                     is_valid = False
-                # append to errors regardless
-                errors.append(form.errors)
-            self._form_list.append(form)
+            add_errors.append(form.errors)
+        add_errors.reverse()
+        errors.extend(add_errors)
 
-        deleted_data.reverse()
         if self.orderable:
             cleaned_data.sort(lambda x,y: x[ORDERING_FIELD_NAME] - y[ORDERING_FIELD_NAME])
-        else:
-            cleaned_data.reverse()
-        errors.reverse()
-        self._form_list.reverse()
-        
+
         if is_valid:
             self.cleaned_data = cleaned_data
             self.deleted_data = deleted_data
         self.errors = errors
         self._is_valid = is_valid
 
-        # TODO: user defined formset validation.
-        
     def add_fields(self, form, index):
         """A hook for adding extra fields on to each form instance."""
         if self.orderable:
@@ -145,5 +147,7 @@ class BaseFormSet(object):
         self.full_clean()
         return self._is_valid
 
-# TODO: handle deletion and ordering in the same FormSet
-# TODO: model integration: form_for_instance and form_for_model type functions
+def formset_for_form(form, formset=BaseFormSet, num_extra=1, orderable=False, deletable=False):
+    """Return a FormSet for the given form class."""
+    attrs = {'form_class': form, 'num_extra': num_extra, 'orderable': orderable, 'deletable': deletable}
+    return type(form.__name__ + 'FormSet', (formset,), attrs)
diff --git a/tests/regressiontests/forms/formsets.py b/tests/regressiontests/forms/formsets.py
index 96aa86a9b2..1c4f3d91b8 100644
--- a/tests/regressiontests/forms/formsets.py
+++ b/tests/regressiontests/forms/formsets.py
@@ -20,7 +20,7 @@ for adding data. By default, it displays 1 blank form. It can display more,
 but we'll look at how to do so later.
 
 >>> formset = ChoiceFormSet(auto_id=False, prefix='choices')
->>> for form in formset.form_list:
+>>> for form in formset.forms:
 ...    print form.as_ul()
 <li>Choice: <input type="text" name="choices-0-choice" /></li>
 <li>Votes: <input type="text" name="choices-0-votes" /></li>
@@ -78,7 +78,7 @@ an extra blank form is included.
 
 >>> initial = [{'choice': u'Calexico', 'votes': 100}]
 >>> formset = ChoiceFormSet(initial=initial, auto_id=False, prefix='choices')
->>> for form in formset.form_list:
+>>> for form in formset.forms:
 ...    print form.as_ul()
 <li>Choice: <input type="text" name="choices-0-choice" value="Calexico" /></li>
 <li>Votes: <input type="text" name="choices-0-votes" value="100" /></li>
@@ -150,7 +150,7 @@ num_extra argument to formset_for_form.
 >>> ChoiceFormSet = formset_for_form(Choice, num_extra=3)
 
 >>> formset = ChoiceFormSet(auto_id=False, prefix='choices')
->>> for form in formset.form_list:
+>>> for form in formset.forms:
 ...    print form.as_ul()
 <li>Choice: <input type="text" name="choices-0-choice" /></li>
 <li>Votes: <input type="text" name="choices-0-votes" /></li>
@@ -223,7 +223,7 @@ data.
 
 >>> initial = [{'choice': u'Calexico', 'votes': 100}]
 >>> formset = ChoiceFormSet(initial=initial, auto_id=False, prefix='choices')
->>> for form in formset.form_list:
+>>> for form in formset.forms:
 ...    print form.as_ul()
 <li>Choice: <input type="text" name="choices-0-choice" value="Calexico" /></li>
 <li>Votes: <input type="text" name="choices-0-votes" value="100" /></li>
@@ -268,7 +268,7 @@ rather than formset.cleaned_data
 
 >>> initial = [{'choice': u'Calexico', 'votes': 100}, {'choice': u'Fergie', 'votes': 900}]
 >>> formset = ChoiceFormSet(initial=initial, auto_id=False, prefix='choices')
->>> for form in formset.form_list:
+>>> for form in formset.forms:
 ...    print form.as_ul()
 <li>Choice: <input type="text" name="choices-0-choice" value="Calexico" /></li>
 <li>Votes: <input type="text" name="choices-0-votes" value="100" /></li>
@@ -318,7 +318,7 @@ something at the front of the list, you'd need to set it's order to 0.
 
 >>> initial = [{'choice': u'Calexico', 'votes': 100}, {'choice': u'Fergie', 'votes': 900}]
 >>> formset = ChoiceFormSet(initial=initial, auto_id=False, prefix='choices')
->>> for form in formset.form_list:
+>>> for form in formset.forms:
 ...    print form.as_ul()
 <li>Choice: <input type="text" name="choices-0-choice" value="Calexico" /></li>
 <li>Votes: <input type="text" name="choices-0-votes" value="100" /></li>
@@ -364,7 +364,7 @@ Let's try throwing ordering and deletion into the same form.
 ...     {'choice': u'The Decemberists', 'votes': 500},
 ... ]
 >>> formset = ChoiceFormSet(initial=initial, auto_id=False, prefix='choices')
->>> for form in formset.form_list:
+>>> for form in formset.forms:
 ...    print form.as_ul()
 <li>Choice: <input type="text" name="choices-0-choice" value="Calexico" /></li>
 <li>Votes: <input type="text" name="choices-0-votes" value="100" /></li>