From 2f9853b2dc90f30317e0374396f08e3d142844d2 Mon Sep 17 00:00:00 2001 From: Joseph Kocherhans Date: Tue, 12 Jan 2010 02:29:45 +0000 Subject: [PATCH] Fixed #12512. Changed ModelForm to stop performing model validation on 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 --- django/contrib/admin/options.py | 16 +-- django/contrib/auth/forms.py | 16 +-- django/core/exceptions.py | 18 +-- django/db/models/base.py | 108 +++++++++++++----- django/db/models/fields/related.py | 5 + django/forms/models.py | 67 +++++++---- docs/ref/models/instances.txt | 91 ++++++++++++--- docs/topics/forms/modelforms.txt | 12 ++ tests/modeltests/model_forms/models.py | 40 ++++++- tests/modeltests/model_formsets/models.py | 17 +++ tests/modeltests/validation/models.py | 18 ++- .../validation/test_custom_messages.py | 4 +- tests/modeltests/validation/test_unique.py | 10 +- tests/modeltests/validation/tests.py | 87 +++++++++++--- tests/modeltests/validation/validators.py | 6 +- tests/regressiontests/admin_views/tests.py | 37 +++++- .../regressiontests/inline_formsets/tests.py | 10 +- 17 files changed, 427 insertions(+), 135 deletions(-) diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 6dc707eefb..8f68eb13ec 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -579,12 +579,12 @@ class ModelAdmin(BaseModelAdmin): """ 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 @@ class ModelAdmin(BaseModelAdmin): 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 @@ class ModelAdmin(BaseModelAdmin): 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. diff --git a/django/contrib/auth/forms.py b/django/contrib/auth/forms.py index dbc55ca0f9..55e770e553 100644 --- a/django/contrib/auth/forms.py +++ b/django/contrib/auth/forms.py @@ -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 UserCreationForm(forms.ModelForm): 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 @@ class UserCreationForm(forms.ModelForm): 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)."), diff --git a/django/core/exceptions.py b/django/core/exceptions.py index fee7db4778..5ccd3e8c63 100644 --- a/django/core/exceptions.py +++ b/django/core/exceptions.py @@ -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 @@ class BaseValidationError(Exception): 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 diff --git a/django/db/models/base.py b/django/db/models/base.py index 46e1822fc5..886c74e27a 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -640,17 +640,21 @@ class Model(object): 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 @@ class Model(object): 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_ + # These are checks for the unique_for_. 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 @@ class Model(object): 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 @@ class Model(object): 'field_label': unicode(field_labels) } - def full_validate(self, exclude=[]): + def full_clean(self, exclude=None): """ - Cleans all fields and raises ValidationError containing message_dict + 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 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) diff --git a/django/db/models/fields/related.py b/django/db/models/fields/related.py index 828200cca5..857222f048 100644 --- a/django/db/models/fields/related.py +++ b/django/db/models/fields/related.py @@ -740,6 +740,11 @@ class ForeignKey(RelatedField, Field): 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 diff --git a/django/forms/models.py b/django/forms/models.py index 30f1e2838c..817de5e9b2 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -9,7 +9,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, 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 @@ class BaseModelForm(BaseForm): 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 @@ class BaseModelFormSet(BaseFormSet): 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 @@ class BaseModelFormSet(BaseFormSet): # 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: diff --git a/docs/ref/models/instances.txt b/docs/ref/models/instances.txt index 9b720bec56..487ec33f5e 100644 --- a/docs/ref/models/instances.txt +++ b/docs/ref/models/instances.txt @@ -34,31 +34,88 @@ Validating objects .. versionadded:: 1.2 -To validate your model, call its ``full_validate()`` method: +There are three steps in validating a model, and all three are called by a +model's ``full_clean()`` method. Most of the time, this method will be called +automatically by a ``ModelForm``. (See the :ref:`ModelForm documentation +` for more information.) You should only need to call +``full_clean()`` if you plan to handle validation errors yourself. -.. method:: Model.full_validate([exclude=[]]) +.. method:: Model.full_clean(exclude=None) -The optional ``exclude`` argument can contain a list of field names to omit -when validating. This method raises ``ValidationError`` containing a -message dictionary with errors from all fields. +This method calls ``Model.clean_fields()``, ``Model.clean()``, and +``Model.validate_unique()``, in that order and raises a ``ValidationError`` +that has a ``message_dict`` attribute containing errors from all three stages. -To add your own validation logic, override the supplied ``validate()`` method: +The optional ``exclude`` argument can be used to provide a list of field names +that can be excluded from validation and cleaning. ``ModelForm`` uses this +argument to exclude fields that aren't present on your form from being +validated since any errors raised could not be corrected by the user. -Note that ``full_validate`` will NOT be called automatically when you call +Note that ``full_clean()`` will NOT be called automatically when you call your model's ``save()`` method. You'll need to call it manually if you want -to run your model validators. (This is for backwards compatibility.) However, -if you're using a ``ModelForm``, it will call ``full_validate`` for you and -will present any errors along with the other form error messages. +to run model validation outside of a ``ModelForm``. (This is for backwards +compatibility.) -.. method:: Model.validate() +Example:: -The ``validate()`` method on ``Model`` by default checks for uniqueness of -fields and group of fields that are declared to be unique, so remember to call -``self.validate_unique()`` or the superclass' ``validate`` method if you want -this validation to run. + try: + article.full_validate() + except ValidationError, e: + # Do something based on the errors contained in e.error_dict. + # Display them to a user, or handle them programatically. + +The first step ``full_clean()`` performs is to clean each individual field. + +.. method:: Model.clean_fields(exclude=None) + +This method will validate all fields on your model. The optional ``exclude`` +argument lets you provide a list of field names to exclude from validation. It +will raise a ``ValidationError`` if any fields fail validation. + +The second step ``full_clean()`` performs is to call ``Model.clean()``. +This method should be overridden to perform custom validation on your model. + +.. method:: Model.clean() + +This method should be used to provide custom model validation, and to modify +attributes on your model if desired. For instance, you could use it to +automatically provide a value for a field, or to do validation that requires +access to more than a single field:: + + def clean(self): + from django.core.exceptions import ValidationError + # Don't allow draft entries to have a pub_date. + if self.status == 'draft' and self.pub_date is not None: + raise ValidationError('Draft entries may not have a publication date.') + # Set the pub_date for published items if it hasn't been set already. + if self.status == 'published' and self.pub_date is None: + self.pub_date = datetime.datetime.now() + +Any ``ValidationError`` raised by ``Model.clean()`` will be stored under a +special key that is used for errors that are tied to the entire model instead +of to a specific field. You can access these errors with ``NON_FIELD_ERRORS``:: + + + from django.core.validators import ValidationError, NON_FIELD_ERRORS + try: + article.full_clean(): + except ValidationError, e: + non_field_errors = e.message_dict[NON_FIELD_ERRORS] + +Finally, ``full_clean()`` will check any unique constraints on your model. + +.. method:: Model.validate_unique(exclude=None) + +This method is similar to ``clean_fields``, but validates all uniqueness +constraints on your model instead of individual field values. The optional +``exclude`` argument allows you to provide a list of field names to exclude +from validation. It will raise a ``ValidationError`` if any fields fail +validation. + +Note that if you provide an ``exclude`` argument to ``validate_unique``, any +``unique_together`` constraint that contains one of the fields you provided +will not be checked. -Any ``ValidationError`` raised in this method will be included in the -``message_dict`` under ``NON_FIELD_ERRORS``. Saving objects ============== diff --git a/docs/topics/forms/modelforms.txt b/docs/topics/forms/modelforms.txt index e1a90262d9..1269dd030c 100644 --- a/docs/topics/forms/modelforms.txt +++ b/docs/topics/forms/modelforms.txt @@ -515,6 +515,18 @@ There are a couple of things to note, however. Chances are these notes won't affect you unless you're trying to do something tricky with subclassing. +Interaction with model validation +--------------------------------- + +As part of its validation process, ``ModelForm`` will call the ``clean()`` +method of each field on your model that has a corresponding field on your form. +If you have excluded any model fields, validation will not be run on those +fields. See the :ref:`form validation ` documentation +for more on how field cleaning and validation work. Also, your model's +``clean()`` method will be called before any uniqueness checks are made. See +:ref:`Validating objects ` for more information on the +model's ``clean()`` hook. + .. _model-formsets: Model formsets diff --git a/tests/modeltests/model_forms/models.py b/tests/modeltests/model_forms/models.py index c488bf1659..400229a9c7 100644 --- a/tests/modeltests/model_forms/models.py +++ b/tests/modeltests/model_forms/models.py @@ -1135,6 +1135,15 @@ True >>> instance.delete() +# Test the non-required FileField +>>> f = TextFileForm(data={'description': u'Assistance'}) +>>> f.fields['file'].required = False +>>> f.is_valid() +True +>>> instance = f.save() +>>> instance.file + + >>> f = TextFileForm(data={'description': u'Assistance'}, files={'file': SimpleUploadedFile('test3.txt', 'hello world')}, instance=instance) >>> f.is_valid() True @@ -1173,7 +1182,7 @@ True >>> class BigIntForm(forms.ModelForm): ... class Meta: ... model = BigInt -... +... >>> bif = BigIntForm({'biggie': '-9223372036854775808'}) >>> bif.is_valid() True @@ -1446,16 +1455,41 @@ False >>> form._errors {'__all__': [u'Price with this Price and Quantity already exists.']} -# This form is never valid because quantity is blank=False. +This Price instance generated by this form is not valid because the quantity +field is required, but the form is valid because the field is excluded from +the form. This is for backwards compatibility. + >>> class PriceForm(ModelForm): ... class Meta: ... model = Price ... exclude = ('quantity',) >>> form = PriceForm({'price': '6.00'}) >>> form.is_valid() +True +>>> price = form.save(commit=False) +>>> price.full_clean() Traceback (most recent call last): ... -UnresolvableValidationError: {'quantity': [u'This field cannot be null.']} +ValidationError: {'quantity': [u'This field cannot be null.']} + +The form should not validate fields that it doesn't contain even if they are +specified using 'fields', not 'exclude'. +... class Meta: +... model = Price +... fields = ('price',) +>>> form = PriceForm({'price': '6.00'}) +>>> form.is_valid() +True + +The form should still have an instance of a model that is not complete and +not saved into a DB yet. + +>>> form.instance.price +Decimal('6.00') +>>> form.instance.quantity is None +True +>>> form.instance.pk is None +True # Unique & unique together with null values >>> class BookForm(ModelForm): diff --git a/tests/modeltests/model_formsets/models.py b/tests/modeltests/model_formsets/models.py index 5eab202962..702523eb7c 100644 --- a/tests/modeltests/model_formsets/models.py +++ b/tests/modeltests/model_formsets/models.py @@ -543,6 +543,10 @@ This is used in the admin for save_as functionality. ... 'book_set-2-title': '', ... } +>>> formset = AuthorBooksFormSet(data, instance=Author(), save_as_new=True) +>>> formset.is_valid() +True + >>> new_author = Author.objects.create(name='Charles Baudelaire') >>> formset = AuthorBooksFormSet(data, instance=new_author, save_as_new=True) >>> [book for book in formset.save() if book.author.pk == new_author.pk] @@ -1031,6 +1035,19 @@ False >>> formset._non_form_errors [u'Please correct the duplicate data for price and quantity, which must be unique.'] +# Only the price field is specified, this should skip any unique checks since +# the unique_together is not fulfilled. This will fail with a KeyError if broken. +>>> FormSet = modelformset_factory(Price, fields=("price",), extra=2) +>>> data = { +... 'form-TOTAL_FORMS': '2', +... 'form-INITIAL_FORMS': '0', +... 'form-0-price': '24', +... 'form-1-price': '24', +... } +>>> formset = FormSet(data) +>>> formset.is_valid() +True + >>> FormSet = inlineformset_factory(Author, Book, extra=0) >>> author = Author.objects.order_by('id')[0] >>> book_ids = author.book_set.values_list('id', flat=True) diff --git a/tests/modeltests/validation/models.py b/tests/modeltests/validation/models.py index f1b0c5188c..017373c52a 100644 --- a/tests/modeltests/validation/models.py +++ b/tests/modeltests/validation/models.py @@ -17,8 +17,8 @@ class ModelToValidate(models.Model): url = models.URLField(blank=True) f_with_custom_validator = models.IntegerField(blank=True, null=True, validators=[validate_answer_to_universe]) - def validate(self): - super(ModelToValidate, self).validate() + def clean(self): + super(ModelToValidate, self).clean() if self.number == 11: raise ValidationError('Invalid number supplied!') @@ -36,7 +36,7 @@ class UniqueTogetherModel(models.Model): efield = models.EmailField() class Meta: - unique_together = (('ifield', 'cfield',),('ifield', 'efield'), ) + unique_together = (('ifield', 'cfield',), ('ifield', 'efield')) class UniqueForDateModel(models.Model): start_date = models.DateField() @@ -51,3 +51,15 @@ class CustomMessagesModel(models.Model): error_messages={'null': 'NULL', 'not42': 'AAARGH', 'not_equal': '%s != me'}, validators=[validate_answer_to_universe] ) + +class Author(models.Model): + name = models.CharField(max_length=100) + +class Article(models.Model): + title = models.CharField(max_length=100) + author = models.ForeignKey(Author) + pub_date = models.DateTimeField(blank=True) + + def clean(self): + if self.pub_date is None: + self.pub_date = datetime.now() diff --git a/tests/modeltests/validation/test_custom_messages.py b/tests/modeltests/validation/test_custom_messages.py index 9a958a0a3f..05bb651b7e 100644 --- a/tests/modeltests/validation/test_custom_messages.py +++ b/tests/modeltests/validation/test_custom_messages.py @@ -5,9 +5,9 @@ from models import CustomMessagesModel class CustomMessagesTest(ValidationTestCase): def test_custom_simple_validator_message(self): cmm = CustomMessagesModel(number=12) - self.assertFieldFailsValidationWithMessage(cmm.full_validate, 'number', ['AAARGH']) + self.assertFieldFailsValidationWithMessage(cmm.full_clean, 'number', ['AAARGH']) def test_custom_null_message(self): cmm = CustomMessagesModel() - self.assertFieldFailsValidationWithMessage(cmm.full_validate, 'number', ['NULL']) + self.assertFieldFailsValidationWithMessage(cmm.full_clean, 'number', ['NULL']) diff --git a/tests/modeltests/validation/test_unique.py b/tests/modeltests/validation/test_unique.py index cbb56aa8f5..0130208bba 100644 --- a/tests/modeltests/validation/test_unique.py +++ b/tests/modeltests/validation/test_unique.py @@ -1,4 +1,5 @@ import unittest +import datetime from django.conf import settings from django.db import connection from models import CustomPKModel, UniqueTogetherModel, UniqueFieldsModel, UniqueForDateModel, ModelToValidate @@ -26,8 +27,8 @@ class GetUniqueCheckTests(unittest.TestCase): def test_unique_for_date_gets_picked_up(self): m = UniqueForDateModel() self.assertEqual(( - [('id',)], - [('date', 'count', 'start_date'), ('year', 'count', 'end_date'), ('month', 'order', 'end_date')] + [('id',)], + [('date', 'count', 'start_date'), ('year', 'count', 'end_date'), ('month', 'order', 'end_date')] ), m._get_unique_checks() ) @@ -47,12 +48,13 @@ class PerformUniqueChecksTest(unittest.TestCase): l = len(connection.queries) mtv = ModelToValidate(number=10, name='Some Name') setattr(mtv, '_adding', True) - mtv.full_validate() + mtv.full_clean() self.assertEqual(l+1, len(connection.queries)) def test_primary_key_unique_check_not_performed_when_not_adding(self): """Regression test for #12132""" l = len(connection.queries) mtv = ModelToValidate(number=10, name='Some Name') - mtv.full_validate() + mtv.full_clean() self.assertEqual(l, len(connection.queries)) + diff --git a/tests/modeltests/validation/tests.py b/tests/modeltests/validation/tests.py index c00070b2ab..d7fc8172da 100644 --- a/tests/modeltests/validation/tests.py +++ b/tests/modeltests/validation/tests.py @@ -1,58 +1,107 @@ -from django.core.exceptions import ValidationError, NON_FIELD_ERRORS -from django.db import models - +from django import forms +from django.test import TestCase +from django.core.exceptions import NON_FIELD_ERRORS from modeltests.validation import ValidationTestCase -from models import * +from modeltests.validation.models import Author, Article, ModelToValidate -from validators import TestModelsWithValidators -from test_unique import GetUniqueCheckTests, PerformUniqueChecksTest -from test_custom_messages import CustomMessagesTest +# Import other tests for this package. +from modeltests.validation.validators import TestModelsWithValidators +from modeltests.validation.test_unique import GetUniqueCheckTests, PerformUniqueChecksTest +from modeltests.validation.test_custom_messages import CustomMessagesTest class BaseModelValidationTests(ValidationTestCase): def test_missing_required_field_raises_error(self): mtv = ModelToValidate(f_with_custom_validator=42) - self.assertFailsValidation(mtv.full_validate, ['name', 'number']) + self.assertFailsValidation(mtv.full_clean, ['name', 'number']) def test_with_correct_value_model_validates(self): mtv = ModelToValidate(number=10, name='Some Name') - self.assertEqual(None, mtv.full_validate()) + self.assertEqual(None, mtv.full_clean()) - def test_custom_validate_method_is_called(self): + def test_custom_validate_method(self): mtv = ModelToValidate(number=11) - self.assertFailsValidation(mtv.full_validate, [NON_FIELD_ERRORS, 'name']) + self.assertFailsValidation(mtv.full_clean, [NON_FIELD_ERRORS, 'name']) def test_wrong_FK_value_raises_error(self): mtv=ModelToValidate(number=10, name='Some Name', parent_id=3) - self.assertFailsValidation(mtv.full_validate, ['parent']) + self.assertFailsValidation(mtv.full_clean, ['parent']) def test_correct_FK_value_validates(self): parent = ModelToValidate.objects.create(number=10, name='Some Name') mtv=ModelToValidate(number=10, name='Some Name', parent_id=parent.pk) - self.assertEqual(None, mtv.full_validate()) + self.assertEqual(None, mtv.full_clean()) def test_wrong_email_value_raises_error(self): mtv = ModelToValidate(number=10, name='Some Name', email='not-an-email') - self.assertFailsValidation(mtv.full_validate, ['email']) + self.assertFailsValidation(mtv.full_clean, ['email']) def test_correct_email_value_passes(self): mtv = ModelToValidate(number=10, name='Some Name', email='valid@email.com') - self.assertEqual(None, mtv.full_validate()) + self.assertEqual(None, mtv.full_clean()) def test_wrong_url_value_raises_error(self): mtv = ModelToValidate(number=10, name='Some Name', url='not a url') - self.assertFieldFailsValidationWithMessage(mtv.full_validate, 'url', [u'Enter a valid value.']) + self.assertFieldFailsValidationWithMessage(mtv.full_clean, 'url', [u'Enter a valid value.']) def test_correct_url_but_nonexisting_gives_404(self): mtv = ModelToValidate(number=10, name='Some Name', url='http://google.com/we-love-microsoft.html') - self.assertFieldFailsValidationWithMessage(mtv.full_validate, 'url', [u'This URL appears to be a broken link.']) + self.assertFieldFailsValidationWithMessage(mtv.full_clean, 'url', [u'This URL appears to be a broken link.']) def test_correct_url_value_passes(self): mtv = ModelToValidate(number=10, name='Some Name', url='http://www.djangoproject.com/') - self.assertEqual(None, mtv.full_validate()) # This will fail if there's no Internet connection + self.assertEqual(None, mtv.full_clean()) # This will fail if there's no Internet connection def test_text_greater_that_charfields_max_length_eaises_erros(self): mtv = ModelToValidate(number=10, name='Some Name'*100) - self.assertFailsValidation(mtv.full_validate, ['name',]) + self.assertFailsValidation(mtv.full_clean, ['name',]) + +class ArticleForm(forms.ModelForm): + class Meta: + model = Article + exclude = ['author'] + +class ModelFormsTests(TestCase): + def setUp(self): + self.author = Author.objects.create(name='Joseph Kocherhans') + + def test_partial_validation(self): + # Make sure the "commit=False and set field values later" idiom still + # works with model validation. + data = { + 'title': 'The state of model validation', + 'pub_date': '2010-1-10 14:49:00' + } + form = ArticleForm(data) + self.assertEqual(form.errors.keys(), []) + article = form.save(commit=False) + article.author = self.author + article.save() + + def test_validation_with_empty_blank_field(self): + # Since a value for pub_date wasn't provided and the field is + # blank=True, model-validation should pass. + # Also, Article.clean() should be run, so pub_date will be filled after + # validation, so the form should save cleanly even though pub_date is + # not allowed to be null. + data = { + 'title': 'The state of model validation', + } + article = Article(author_id=self.author.id) + form = ArticleForm(data, instance=article) + self.assertEqual(form.errors.keys(), []) + self.assertNotEqual(form.instance.pub_date, None) + article = form.save() + + def test_validation_with_invalid_blank_field(self): + # Even though pub_date is set to blank=True, an invalid value was + # provided, so it should fail validation. + data = { + 'title': 'The state of model validation', + 'pub_date': 'never' + } + article = Article(author_id=self.author.id) + form = ArticleForm(data, instance=article) + self.assertEqual(form.errors.keys(), ['pub_date']) diff --git a/tests/modeltests/validation/validators.py b/tests/modeltests/validation/validators.py index dc4cd4eba1..3ad2c40f03 100644 --- a/tests/modeltests/validation/validators.py +++ b/tests/modeltests/validation/validators.py @@ -6,13 +6,13 @@ from models import * class TestModelsWithValidators(ValidationTestCase): def test_custom_validator_passes_for_correct_value(self): mtv = ModelToValidate(number=10, name='Some Name', f_with_custom_validator=42) - self.assertEqual(None, mtv.full_validate()) + self.assertEqual(None, mtv.full_clean()) def test_custom_validator_raises_error_for_incorrect_value(self): mtv = ModelToValidate(number=10, name='Some Name', f_with_custom_validator=12) - self.assertFailsValidation(mtv.full_validate, ['f_with_custom_validator']) + self.assertFailsValidation(mtv.full_clean, ['f_with_custom_validator']) self.assertFieldFailsValidationWithMessage( - mtv.full_validate, + mtv.full_clean, 'f_with_custom_validator', [u'This is not the answer to life, universe and everything!'] ) diff --git a/tests/regressiontests/admin_views/tests.py b/tests/regressiontests/admin_views/tests.py index 0c7fbc0876..c580491486 100644 --- a/tests/regressiontests/admin_views/tests.py +++ b/tests/regressiontests/admin_views/tests.py @@ -4,7 +4,8 @@ import re import datetime from django.core.files import temp as tempfile from django.test import TestCase -from django.contrib.auth.models import User, Permission +from django.contrib.auth import admin # Register auth models with the admin. +from django.contrib.auth.models import User, Permission, UNUSABLE_PASSWORD from django.contrib.contenttypes.models import ContentType from django.contrib.admin.models import LogEntry, DELETION from django.contrib.admin.sites import LOGIN_FORM_KEY @@ -1766,3 +1767,37 @@ class ReadonlyTest(TestCase): self.assertEqual(Post.objects.count(), 2) p = Post.objects.order_by('-id')[0] self.assertEqual(p.posted, datetime.date.today()) + +class IncompleteFormTest(TestCase): + """ + Tests validation of a ModelForm that doesn't explicitly have all data + corresponding to model fields. Model validation shouldn't fail + such a forms. + """ + fixtures = ['admin-views-users.xml'] + + def setUp(self): + self.client.login(username='super', password='secret') + + def tearDown(self): + self.client.logout() + + def test_user_creation(self): + response = self.client.post('/test_admin/admin/auth/user/add/', { + 'username': 'newuser', + 'password1': 'newpassword', + 'password2': 'newpassword', + }) + new_user = User.objects.order_by('-id')[0] + self.assertRedirects(response, '/test_admin/admin/auth/user/%s/' % new_user.pk) + self.assertNotEquals(new_user.password, UNUSABLE_PASSWORD) + + def test_password_mismatch(self): + response = self.client.post('/test_admin/admin/auth/user/add/', { + 'username': 'newuser', + 'password1': 'newpassword', + 'password2': 'mismatch', + }) + self.assertEquals(response.status_code, 200) + self.assert_('password' not in response.context['form'].errors) + self.assertFormError(response, 'form', 'password2', ["The two password fields didn't match."]) diff --git a/tests/regressiontests/inline_formsets/tests.py b/tests/regressiontests/inline_formsets/tests.py index be313f3bf6..aef6b3f10a 100644 --- a/tests/regressiontests/inline_formsets/tests.py +++ b/tests/regressiontests/inline_formsets/tests.py @@ -81,7 +81,7 @@ class DeletionTests(TestCase): regression for #10750 """ # exclude some required field from the forms - ChildFormSet = inlineformset_factory(School, Child) + ChildFormSet = inlineformset_factory(School, Child, exclude=['father', 'mother']) school = School.objects.create(name=u'test') mother = Parent.objects.create(name=u'mother') father = Parent.objects.create(name=u'father') @@ -89,13 +89,13 @@ class DeletionTests(TestCase): 'child_set-TOTAL_FORMS': u'1', 'child_set-INITIAL_FORMS': u'0', 'child_set-0-name': u'child', - 'child_set-0-mother': unicode(mother.pk), - 'child_set-0-father': unicode(father.pk), } formset = ChildFormSet(data, instance=school) self.assertEqual(formset.is_valid(), True) objects = formset.save(commit=False) - self.assertEqual(school.child_set.count(), 0) - objects[0].save() + for obj in objects: + obj.mother = mother + obj.father = father + obj.save() self.assertEqual(school.child_set.count(), 1)