From 4d7390265373b5601d3a0a4c681a7f819baf7c4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Honza=20Kr=C3=A1l?= Date: Mon, 1 Jun 2009 15:39:46 +0000 Subject: [PATCH] [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 | 25 ++++++++----- django/db/models/base.py | 9 ++++- django/forms/models.py | 50 +++++++++++++++++--------- tests/regressiontests/views/views.py | 2 +- 4 files changed, 59 insertions(+), 27 deletions(-) diff --git a/django/contrib/contenttypes/generic.py b/django/contrib/contenttypes/generic.py index 5564548133..37a4b6ad39 100644 --- a/django/contrib/contenttypes/generic.py +++ b/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 - self.instance = instance + 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, diff --git a/django/db/models/base.py b/django/db/models/base.py index b581656f8d..32804c3622 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -622,7 +622,14 @@ class Model(object): # TODO: run this only if not errors?? self.validate() except ValidationError, e: - errors[NON_FIELD_ERRORS] = e.messages + 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: raise ValidationError(errors) diff --git a/django/forms/models.py b/django/forms/models.py index d5d5999c76..b0eb2f5bec 100644 --- a/django/forms/models.py +++ b/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 diff --git a/tests/regressiontests/views/views.py b/tests/regressiontests/views/views.py index 3447cbfdfd..20ee56f985 100644 --- a/tests/regressiontests/views/views.py +++ b/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,