[soc2009/model-validation] Split save_instance into make and save_made

had to add a magical attribute __adding on ModelForm to indicate whether
we are editting or adding a model. This will be later passed into the
model's .clean() method since it is necessary for correctly validating
uniques (see it's current use) and can also be used to define
force_insert or force_update params to model.save()

Also beginning with this commit, .instance attribute is present on ModelForm
and should be used to do any post-processing since manipulation of the
cleaned_data will not work

git-svn-id: http://code.djangoproject.com/svn/django/branches/soc2009/model-validation@10871 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/contrib/contenttypes/generic.py

@@ -296,7 +296,11 @@ class BaseGenericInlineFormSet(BaseModelFormSet):
 
     def __init__(self, data=None, files=None, instance=None, save_as_new=None, prefix=None):
         opts = self.model._meta
+        if instance is None:
+            self.instance = self.model()
+        else:
             self.instance = instance
+        self.save_as_new = save_as_new
         self.rel_name = '-'.join((
             opts.app_label, opts.object_name.lower(),
             self.ct_field.name, self.ct_fk_field.name,
@@ -324,15 +328,20 @@ class BaseGenericInlineFormSet(BaseModelFormSet):
             self.ct_fk_field.name: self.instance.pk,
         })
 
-    def save_new(self, form, commit=True):
+    def _construct_form(self, i, **kwargs):
         # Avoid a circular import.
         from django.contrib.contenttypes.models import ContentType
-        kwargs = {
-            self.ct_field.get_attname(): ContentType.objects.get_for_model(self.instance).pk,
-            self.ct_fk_field.get_attname(): self.instance.pk,
-        }
-        new_obj = self.model(**kwargs)
-        return save_instance(form, new_obj, commit=commit)
+        form = super(BaseGenericInlineFormSet, self)._construct_form(i, **kwargs)
+        if self.save_as_new:
+            # Remove the key from the form's data, we are only
+            # creating new instances
+            form.data[form.add_prefix(self.ct_fk_field.name)] = None
+            form.data[form.add_prefix(self.ct_field.name)] = None
+
+        # set the GenericFK value here so that the form can do it's validation
+        setattr(form.instance, self.ct_fk_field.attname, self.instance.pk)
+        setattr(form.instance, self.ct_field.attname, ContentType.objects.get_for_model(self.instance).pk)
+        return form
 
 def generic_inlineformset_factory(model, form=ModelForm,
                                   formset=BaseGenericInlineFormSet,

django/db/models/base.py

@@ -622,6 +622,13 @@ class Model(object):
             # TODO: run this only if not errors??
             self.validate()
         except ValidationError, e:
+            if hasattr(e, 'message_dict'):
+                if errors:
+                    for k, v in e.message_dict.items():
+                        errors.set_default(k, []).extend(v)
+                else:
+                    errors = e.message_dict
+            else:
                 errors[NON_FIELD_ERRORS] = e.messages
 
         if errors:

django/forms/models.py

@@ -8,7 +8,7 @@ from django.utils.datastructures import SortedDict
 from django.utils.text import get_text_list, capfirst
 from django.utils.translation import ugettext_lazy as _, ugettext
 
-from django.core.exceptions import ValidationError
+from django.core.exceptions import ValidationError, NON_FIELD_ERRORS
 from util import ErrorList
 from forms import BaseForm, get_declared_fields, NON_FIELD_ERRORS
 from fields import Field, ChoiceField, IntegerField, EMPTY_VALUES
@@ -27,20 +27,10 @@ __all__ = (
     'ModelMultipleChoiceField',
 )
 
-
-def save_instance(form, instance, fields=None, fail_message='saved',
-                  commit=True, exclude=None):
-    """
-    Saves bound Form ``form``'s cleaned_data into model instance ``instance``.
-
-    If commit=True, then the changes to ``instance`` will be saved to the
-    database. Returns ``instance``.
-    """
+def make_instance(form, instance, fields=None, exclude=None):
     from django.db import models
     opts = instance._meta
-    if form.errors:
-        raise ValueError("The %s could not be %s because the data didn't"
-                         " validate." % (opts.object_name, fail_message))
+
     cleaned_data = form.cleaned_data
     file_field_list = []
     for f in opts.fields:
@@ -65,9 +55,16 @@ def save_instance(form, instance, fields=None, fail_message='saved',
     for f in file_field_list:
         f.save_form_data(instance, cleaned_data[f.name])
 
+    return instance
+
+def save_made_instance(form, instance, fields=None, commit=True, fail_message='saved'):
+    opts = instance._meta
+    if form.errors:
+        raise ValueError("The %s could not be %s because the data didn't"
+                         " validate." % (opts.object_name, fail_message))
+
     # Wrap up the saving of m2m data as a function.
     def save_m2m():
-        opts = instance._meta
         cleaned_data = form.cleaned_data
         for f in opts.many_to_many:
             if fields and f.name not in fields:
@@ -84,6 +81,18 @@ def save_instance(form, instance, fields=None, fail_message='saved',
         form.save_m2m = save_m2m
     return instance
 
+
+def save_instance(form, instance, fields=None, fail_message='saved',
+                  commit=True, exclude=None):
+    """
+    Saves bound Form ``form``'s cleaned_data into model instance ``instance``.
+
+    If commit=True, then the changes to ``instance`` will be saved to the
+    database. Returns ``instance``.
+    """
+    instance = make_instance(form, instance, fields, exclude)
+    return save_made_instance(form, instance, fields, commit, fail_message)
+
 def make_model_save(model, fields, fail_message):
     """Returns the save() method for a Form."""
     def save(self, commit=True):
@@ -218,7 +227,9 @@ class BaseModelForm(BaseForm):
             # if we didn't get an instance, instantiate a new one
             self.instance = opts.model()
             object_data = {}
+            self.__adding = True
         else:
+            self.__adding = False
             self.instance = instance
             object_data = model_to_dict(instance, opts.fields, opts.exclude)
         # if initial was provided, it should override the values from instance
@@ -228,6 +239,8 @@ class BaseModelForm(BaseForm):
                                             error_class, label_suffix, empty_permitted)
 
     def clean(self):
+        opts = self._meta
+        self.instance = make_instance(self, self.instance, opts.fields, opts.exclude)
         self.validate_unique()
         return self.cleaned_data
 
@@ -317,7 +330,7 @@ class BaseModelForm(BaseForm):
 
             # Exclude the current object from the query if we are editing an
             # instance (as opposed to creating a new one)
-            if self.instance.pk is not None:
+            if not self.__adding and self.instance.pk is not None:
                 qs = qs.exclude(pk=self.instance.pk)
 
             # This cute trick with extra/values is the most efficient way to
@@ -404,8 +417,8 @@ class BaseModelForm(BaseForm):
             fail_message = 'created'
         else:
             fail_message = 'changed'
-        return save_instance(self, self.instance, self._meta.fields,
-                             fail_message, commit, exclude=self._meta.exclude)
+
+        return save_made_instance(self, self.instance, self._meta.fields, commit, fail_message)
 
     save.alters_data = True
 
@@ -731,6 +744,9 @@ class BaseInlineFormSet(BaseModelFormSet):
 
             # Remove the foreign key from the form's data
             form.data[form.add_prefix(self.fk.name)] = None
+
+        # set the FK value here so that the form can do it's validation
+        setattr(form.instance, self.fk.get_attname(), self.instance.pk)
         return form
 
     #@classmethod

tests/regressiontests/views/views.py

@@ -24,7 +24,7 @@ def custom_create(request):
             model = Article
 
         def save(self, *args, **kwargs):
-            self.cleaned_data['slug'] = 'some-other-slug'
+            self.instance.slug = 'some-other-slug'
             return super(SlugChangingArticleForm, self).save(*args, **kwargs)
 
     return create_object(request,