Permalink
Browse files

Fixed #12512. Changed ModelForm to stop performing model validation o…

…n fields that are not part of the form. Thanks, Honza Kral and Ivan Sagalaev.

This reverts some admin and test changes from [12098] and also fixes #12507, #12520, #12552 and #12553.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@12206 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
1 parent 26279c5 commit 2f9853b2dc90f30317e0374396f08e3d142844d2 @jkocherhans jkocherhans committed Jan 12, 2010
@@ -579,12 +579,12 @@ def message_user(self, request, message):
"""
messages.info(request, message)
- def save_form(self, request, form, change, commit=False):
+ def save_form(self, request, form, change):
"""
Given a ModelForm return an unsaved instance. ``change`` is True if
the object is being changed, and False if it's being added.
"""
- return form.save(commit=commit)
+ return form.save(commit=False)
def save_model(self, request, obj, form, change):
"""
@@ -758,11 +758,7 @@ def add_view(self, request, form_url='', extra_context=None):
if request.method == 'POST':
form = ModelForm(request.POST, request.FILES)
if form.is_valid():
- # Save the object, even if inline formsets haven't been
- # validated yet. We need to pass the valid model to the
- # formsets for validation. If the formsets do not validate, we
- # will delete the object.
- new_object = self.save_form(request, form, change=False, commit=True)
+ new_object = self.save_form(request, form, change=False)
form_validated = True
else:
form_validated = False
@@ -779,15 +775,13 @@ def add_view(self, request, form_url='', extra_context=None):
prefix=prefix, queryset=inline.queryset(request))
formsets.append(formset)
if all_valid(formsets) and form_validated:
+ self.save_model(request, new_object, form, change=False)
+ form.save_m2m()
for formset in formsets:
self.save_formset(request, form, formset, change=False)
self.log_addition(request, new_object)
return self.response_add(request, new_object)
- elif form_validated:
- # The form was valid, but formsets were not, so delete the
- # object we saved above.
- new_object.delete()
else:
# Prepare the dict of initial data from the request.
# We have to special-case M2Ms as a list of comma-separated PKs.
@@ -1,4 +1,4 @@
-from django.contrib.auth.models import User, UNUSABLE_PASSWORD
+from django.contrib.auth.models import User
from django.contrib.auth import authenticate
from django.contrib.auth.tokens import default_token_generator
from django.contrib.sites.models import Site
@@ -21,12 +21,6 @@ class Meta:
model = User
fields = ("username",)
- def clean(self):
- # Fill the password field so model validation won't complain about it
- # being blank. We'll set it with the real value below.
- self.instance.password = UNUSABLE_PASSWORD
- super(UserCreationForm, self).clean()
-
def clean_username(self):
username = self.cleaned_data["username"]
try:
@@ -40,9 +34,15 @@ def clean_password2(self):
password2 = self.cleaned_data["password2"]
if password1 != password2:
raise forms.ValidationError(_("The two password fields didn't match."))
- self.instance.set_password(password1)
return password2
+ def save(self, commit=True):
+ user = super(UserCreationForm, self).save(commit=False)
+ user.set_password(self.cleaned_data["password1"])
+ if commit:
+ user.save()
+ return user
+
class UserChangeForm(forms.ModelForm):
username = forms.RegexField(label=_("Username"), max_length=30, regex=r'^\w+$',
help_text = _("Required. 30 characters or fewer. Alphanumeric characters only (letters, digits and underscores)."),
@@ -33,7 +33,7 @@ class FieldError(Exception):
pass
NON_FIELD_ERRORS = '__all__'
-class BaseValidationError(Exception):
+class ValidationError(Exception):
"""An error while validating data."""
def __init__(self, message, code=None, params=None):
import operator
@@ -64,10 +64,14 @@ def __str__(self):
return repr(self.message_dict)
return repr(self.messages)
-class ValidationError(BaseValidationError):
- pass
-
-class UnresolvableValidationError(BaseValidationError):
- """Validation error that cannot be resolved by the user."""
- pass
+ def update_error_dict(self, error_dict):
+ if hasattr(self, 'message_dict'):
+ if error_dict:
+ for k, v in self.message_dict.items():
+ error_dict.setdefault(k, []).extend(v)
+ else:
+ error_dict = self.message_dict
+ else:
+ error_dict[NON_FIELD_ERRORS] = self.messages
+ return error_dict
@@ -640,17 +640,21 @@ def _get_next_or_previous_in_order(self, is_next):
def prepare_database_save(self, unused):
return self.pk
- def validate(self):
+ def clean(self):
"""
Hook for doing any extra model-wide validation after clean() has been
- called on every field. Any ValidationError raised by this method will
- not be associated with a particular field; it will have a special-case
- association with the field defined by NON_FIELD_ERRORS.
+ called on every field by self.clean_fields. Any ValidationError raised
+ by this method will not be associated with a particular field; it will
+ have a special-case association with the field defined by NON_FIELD_ERRORS.
"""
- self.validate_unique()
+ pass
- def validate_unique(self):
- unique_checks, date_checks = self._get_unique_checks()
+ def validate_unique(self, exclude=None):
+ """
+ Checks unique constraints on the model and raises ``ValidationError``
+ if any failed.
+ """
+ unique_checks, date_checks = self._get_unique_checks(exclude=exclude)
errors = self._perform_unique_checks(unique_checks)
date_errors = self._perform_date_checks(date_checks)
@@ -661,17 +665,35 @@ def validate_unique(self):
if errors:
raise ValidationError(errors)
- def _get_unique_checks(self):
- from django.db.models.fields import FieldDoesNotExist, Field as ModelField
+ def _get_unique_checks(self, exclude=None):
+ """
+ Gather a list of checks to perform. Since validate_unique could be
+ called from a ModelForm, some fields may have been excluded; we can't
+ perform a unique check on a model that is missing fields involved
+ in that check.
+ Fields that did not validate should also be exluded, but they need
+ to be passed in via the exclude argument.
+ """
+ if exclude is None:
+ exclude = []
+ unique_checks = []
+ for check in self._meta.unique_together:
+ for name in check:
+ # If this is an excluded field, don't add this check.
+ if name in exclude:
+ break
+ else:
+ unique_checks.append(check)
- unique_checks = list(self._meta.unique_together)
- # these are checks for the unique_for_<date/year/month>
+ # These are checks for the unique_for_<date/year/month>.
date_checks = []
# Gather a list of checks for fields declared as unique and add them to
- # the list of checks. Again, skip empty fields and any that did not validate.
+ # the list of checks.
for f in self._meta.fields:
name = f.name
+ if name in exclude:
+ continue
if f.unique:
unique_checks.append((name,))
if f.unique_for_date:
@@ -682,7 +704,6 @@ def _get_unique_checks(self):
date_checks.append(('month', name, f.unique_for_month))
return unique_checks, date_checks
-
def _perform_unique_checks(self, unique_checks):
errors = {}
@@ -779,34 +800,61 @@ def unique_error_message(self, unique_check):
'field_label': unicode(field_labels)
}
- def full_validate(self, exclude=[]):
+ def full_clean(self, exclude=None):
+ """
+ Calls clean_fields, clean, and validate_unique, on the model,
+ and raises a ``ValidationError`` for any errors that occured.
+ """
+ errors = {}
+ if exclude is None:
+ exclude = []
+
+ try:
+ self.clean_fields(exclude=exclude)
+ except ValidationError, e:
+ errors = e.update_error_dict(errors)
+
+ # Form.clean() is run even if other validation fails, so do the
+ # same with Model.clean() for consistency.
+ try:
+ self.clean()
+ except ValidationError, e:
+ errors = e.update_error_dict(errors)
+
+ # Run unique checks, but only for fields that passed validation.
+ for name in errors.keys():
+ if name != NON_FIELD_ERRORS and name not in exclude:
+ exclude.append(name)
+ try:
+ self.validate_unique(exclude=exclude)
+ except ValidationError, e:
+ errors = e.update_error_dict(errors)
+
+ if errors:
+ raise ValidationError(errors)
+
+ def clean_fields(self, exclude=None):
"""
- Cleans all fields and raises ValidationError containing message_dict
+ Cleans all fields and raises a ValidationError containing message_dict
of all validation errors if any occur.
"""
+ if exclude is None:
+ exclude = []
+
errors = {}
for f in self._meta.fields:
if f.name in exclude:
continue
+ # Skip validation for empty fields with blank=True. The developer
+ # is responsible for making sure they have a valid value.
+ raw_value = getattr(self, f.attname)
+ if f.blank and raw_value in validators.EMPTY_VALUES:
+ continue
try:
- setattr(self, f.attname, f.clean(getattr(self, f.attname), self))
+ setattr(self, f.attname, f.clean(raw_value, self))
except ValidationError, e:
errors[f.name] = e.messages
- # Form.clean() is run even if other validation fails, so do the
- # same with Model.validate() for consistency.
- try:
- self.validate()
- except ValidationError, e:
- if hasattr(e, 'message_dict'):
- if errors:
- for k, v in e.message_dict.items():
- errors.setdefault(k, []).extend(v)
- else:
- errors = e.message_dict
- else:
- errors[NON_FIELD_ERRORS] = e.messages
-
if errors:
raise ValidationError(errors)
@@ -740,6 +740,11 @@ def __init__(self, to, to_field=None, rel_class=ManyToOneRel, **kwargs):
def validate(self, value, model_instance):
if self.rel.parent_link:
return
+ # Don't validate the field if a value wasn't supplied. This is
+ # generally the case when saving new inlines in the admin.
+ # See #12507.
+ if value is None:
+ return
super(ForeignKey, self).validate(value, model_instance)
if not value:
return
@@ -9,7 +9,7 @@
from django.utils.text import get_text_list, capfirst
from django.utils.translation import ugettext_lazy as _, ugettext
-from django.core.exceptions import ValidationError, NON_FIELD_ERRORS, UnresolvableValidationError
+from django.core.exceptions import ValidationError, NON_FIELD_ERRORS
from django.core.validators import EMPTY_VALUES
from util import ErrorList
from forms import BaseForm, get_declared_fields
@@ -250,31 +250,51 @@ def __init__(self, data=None, files=None, auto_id='id_%s', prefix=None,
super(BaseModelForm, self).__init__(data, files, auto_id, prefix, object_data,
error_class, label_suffix, empty_permitted)
+
+ def _get_validation_exclusions(self):
+ """
+ For backwards-compatibility, several types of fields need to be
+ excluded from model validation. See the following tickets for
+ details: #12507, #12521, #12553
+ """
+ exclude = []
+ # Build up a list of fields that should be excluded from model field
+ # validation and unique checks.
+ for f in self.instance._meta.fields:
+ field = f.name
+ # Exclude fields that aren't on the form. The developer may be
+ # adding these values to the model after form validation.
+ if field not in self.fields:
+ exclude.append(f.name)
+ # Exclude fields that failed form validation. There's no need for
+ # the model fields to validate them as well.
+ elif field in self._errors.keys():
+ exclude.append(f.name)
+ # Exclude empty fields that are not required by the form. The
+ # underlying model field may be required, so this keeps the model
+ # field from raising that error.
+ else:
+ form_field = self.fields[field]
+ field_value = self.cleaned_data.get(field, None)
+ if field_value is None and not form_field.required:
+ exclude.append(f.name)
+ return exclude
+
def clean(self):
opts = self._meta
self.instance = construct_instance(self, self.instance, opts.fields, opts.exclude)
+ exclude = self._get_validation_exclusions()
try:
- self.instance.full_validate(exclude=self._errors.keys())
+ self.instance.full_clean(exclude=exclude)
except ValidationError, e:
for k, v in e.message_dict.items():
if k != NON_FIELD_ERRORS:
self._errors.setdefault(k, ErrorList()).extend(v)
-
# Remove the data from the cleaned_data dict since it was invalid
if k in self.cleaned_data:
del self.cleaned_data[k]
-
if NON_FIELD_ERRORS in e.message_dict:
raise ValidationError(e.message_dict[NON_FIELD_ERRORS])
-
- # If model validation threw errors for fields that aren't on the
- # form, the the errors cannot be corrected by the user. Displaying
- # those errors would be pointless, so raise another type of
- # exception that *won't* be caught and displayed by the form.
- if set(e.message_dict.keys()) - set(self.fields.keys() + [NON_FIELD_ERRORS]):
- raise UnresolvableValidationError(e.message_dict)
-
-
return self.cleaned_data
def save(self, commit=True):
@@ -412,17 +432,20 @@ def clean(self):
self.validate_unique()
def validate_unique(self):
- # Iterate over the forms so that we can find one with potentially valid
- # data from which to extract the error checks
+ # Collect unique_checks and date_checks to run from all the forms.
+ all_unique_checks = set()
+ all_date_checks = set()
for form in self.forms:
- if hasattr(form, 'cleaned_data'):
- break
- else:
- return
- unique_checks, date_checks = form.instance._get_unique_checks()
+ if not hasattr(form, 'cleaned_data'):
+ continue
+ exclude = form._get_validation_exclusions()
+ unique_checks, date_checks = form.instance._get_unique_checks(exclude=exclude)
+ all_unique_checks = all_unique_checks.union(set(unique_checks))
+ all_date_checks = all_date_checks.union(set(date_checks))
+
errors = []
# Do each of the unique checks (unique and unique_together)
- for unique_check in unique_checks:
+ for unique_check in all_unique_checks:
seen_data = set()
for form in self.forms:
# if the form doesn't have cleaned_data then we ignore it,
@@ -444,7 +467,7 @@ def validate_unique(self):
# mark the data as seen
seen_data.add(row_data)
# iterate over each of the date checks now
- for date_check in date_checks:
+ for date_check in all_date_checks:
seen_data = set()
lookup, field, unique_for = date_check
for form in self.forms:
Oops, something went wrong. Retry.

0 comments on commit 2f9853b

Please sign in to comment.