Ticket #12521: 12521.3.diff

django/contrib/admin/options.py

@@ -579 +579 @@
         """
         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 +758 @@
         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 +775 @@
                                   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.

django/contrib/auth/forms.py

@@ -1 @@
-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 +21 @@
         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:
@@ -43 +37 @@
         self.instance.set_password(password1)
         return password2
 
+    def save(self, *args, **kwargs):
+        self.instance.set_password(self.cleaned_data["password1"])
+        return super(UserCreationForm, self).save(*args, **kwargs)
+
 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)."),

django/core/exceptions.py

@@ -33 +33 @@
     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 +64 @@
             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_errors(errors, 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
+    return errors
 

django/db/models/base.py

@@ -3 +3 @@
 import os
 from itertools import izip
 import django.db.models.manager     # Imported to register signal handler.
-from django.core.exceptions import ObjectDoesNotExist, MultipleObjectsReturned, FieldError, ValidationError, NON_FIELD_ERRORS
+from django.core.exceptions import ObjectDoesNotExist, MultipleObjectsReturned, FieldError, ValidationError, NON_FIELD_ERRORS, update_errors
 from django.core import validators
 from django.db.models.fields import AutoField, FieldDoesNotExist
 from django.db.models.fields.related import OneToOneRel, ManyToOneRel, OneToOneField
@@ -642 +642 @@
     def prepare_database_save(self, unused):
         return self.pk
 
-    def validate(self):
+    def full_validate(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.
+        Calls validate_fields, validate_unique, and validate on the model,
+        and combines and reraises any ``ValidationError``s that occur.
         """
-        self.validate_unique()
+        errors = {}
+
+        try:
+            self.validate_fields()
+        except ValidationError, e:
+            errors = update_errors(errors, e)
 
-    def validate_unique(self):
-        unique_checks, date_checks = self._get_unique_checks()
+        try:
+            self.validate_unique()
+        except ValidationError, e:
+            errors = update_errors(errors, e)
+
+        # Form.clean() is run even if other validation fails, so do the
+        # same with Model.validate() for consistency.
+        try:
+            self.validate()
+        except ValidationError, e:
+            errors = update_errors(errors, e)
+
+        if errors:
+            raise ValidationError(errors)
+
+    def validate_fields(self, fields=None):
+        """
+        Cleans all fields and raises a ValidationError containing message_dict
+        of all validation errors if any occur.
+
+        If ``fields`` is supplied, only validate the specified fields.
+        """
+        errors = {}
+        if fields is None:
+            fields = self._meta.fields
+        else:
+            fields = [self._meta.get_field_by_name(f)[0] for f in fields]
+        for f in fields:
+            try:
+                setattr(self, f.attname, f.clean(getattr(self, f.attname), self))
+            except ValidationError, e:
+                errors[f.name] = e.messages
+        if errors:
+            raise ValidationError(errors)
+
+    def validate_unique(self, fields=None):
+        unique_checks, date_checks = self._get_unique_checks(fields=fields)
 
         errors = self._perform_unique_checks(unique_checks)
         date_errors = self._perform_date_checks(date_checks)
@@ -663 +701 @@
         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, fields=None):
+        if fields is None:
+            fields = [f.name for f in self._meta.fields]
+        unique_checks = []
+        # 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. It also does not make sense to check data that didn't
+        # validate, and since NULL does not equal NULL in SQL we should not do
+        # any unique checking for NULL values.
+        for check in self._meta.unique_together:
+            for name in check:
+                # If this is an excluded field, short circuit and don't do a
+                # unique check.
+                if name not in fields:
+                    break
+                # Also, Skip fields that don't have a value, we can't check for
+                # their uniqueness.
+                if getattr(self, name, None) is None:
+                    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.
         for f in self._meta.fields:
             name = f.name
+            # Don't add excluded fields to unique for date checks.
+            if name not in fields:
+                continue
             if f.unique:
                 unique_checks.append((name,))
             if f.unique_for_date:
@@ -684 +744 @@
                 date_checks.append(('month', name, f.unique_for_month))
         return unique_checks, date_checks
 
-
     def _perform_unique_checks(self, unique_checks):
         errors = {}
 
@@ -781 +840 @@
                 'field_label': unicode(field_labels)
             }
 
-    def full_validate(self, exclude=[]):
+    def validate(self):
         """
-        Cleans all fields and raises ValidationError containing message_dict
-        of all validation errors if any occur.
+        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.
         """
-        errors = {}
-        for f in self._meta.fields:
-            if f.name in exclude:
-                continue
-            try:
-                setattr(self, f.attname, f.clean(getattr(self, f.attname), 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.set_default(k, []).extend(v)
-                else:
-                    errors = e.message_dict
-            else:
-                errors[NON_FIELD_ERRORS] = e.messages
-
-        if errors:
-            raise ValidationError(errors)
+        pass
 
 
 ############################################

django/db/models/fields/related.py

@@ -740 +740 @@
     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

django/forms/models.py

@@ -9 +9 @@
 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, update_errors, NON_FIELD_ERRORS
 from django.core.validators import EMPTY_VALUES
 from util import ErrorList
 from forms import BaseForm, get_declared_fields
@@ -248 +248 @@
     def clean(self):
         opts = self._meta
         self.instance = construct_instance(self, self.instance, opts.fields, opts.exclude)
+        # For backwards-compatibility, several types of fields need to be
+        # excluded from model validation. See the following tickets for
+        # details: #12507, #12521, #12553
+        errors = {}
+        # Build up a list of fields that should be included in model validation
+        # and unique checks.
+        fields = []
+        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:
+                continue
+            # Exclude fields that failed form validation. There's no need for
+            # the model to validate them as well.
+            elif field in self._errors.keys():
+                continue
+            # Exclude empty fields that are not required by the form. Model
+            # validation might complain that they are empty, but the
+            # developer is reponsible for setting their value after calling
+            # form validation.
+            elif (not self.fields[field].required) and self.cleaned_data.get(field, None) is None:
+                continue
+            else:
+                fields.append(field)
+        try:
+            self.instance.validate_fields(fields=list(fields))
+        except ValidationError, e:
+            errors = update_errors(errors, e)
         try:
-            self.instance.full_validate(exclude=self._errors.keys())
+            self.instance.validate_unique(fields=fields)
         except ValidationError, e:
-            for k, v in e.message_dict.items():
+            errors = update_errors(errors, e)
+        if errors:
+            for k, v in errors.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)
-
-
+            if NON_FIELD_ERRORS in errors:
+                raise ValidationError(errors[NON_FIELD_ERRORS])
         return self.cleaned_data
 
     def save(self, commit=True):

docs/ref/models/instances.txt

@@ -36 +36 @@
 
 To validate your model, call its ``full_validate()`` method:
 
-.. method:: Model.full_validate([exclude=[]])
+.. method:: Model.full_validate()
 
-The optional ``exclude`` argument can contain a list of field names to omit
-when validating. This method raises ``ValidationError`` containing a
+This method calls ``Model.validate_fields()``, ``Model.validate_unique()`` and
+``Model.validate()`` in order and raises a ``ValidationError`` containing a
 message dictionary with errors from all fields.
 
 To add your own validation logic, override the supplied ``validate()`` method:
 
-Note that ``full_validate`` will NOT be called automatically when you call
+Note that ``full_validate()`` 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 your model validators. (This is for backwards compatibility.)
+
+If you want to do partial validation on your model, you should use
+``validate_fields`` and ``validate_unique``.
+
+.. method:: Model.validate_fields(fields=None)
+
+This method will validate all fields on your model. If you only want to
+validate a subset of fields, provide a list of field names as the ``fields``
+argument.
+
+.. method:: Model.validate_unique(fields=None)
+
+This method is similar to ``validate_fields``, but validates all uniqueness
+constraints on your model instead of individual field values. If you only want
+to check a subset of fields, provide a list of field names as the ``fields``
+argument.
+
+Note that if you provide a ``fields`` argument to ``validate_unique``, you
+need to provide every field in the constraints you want checked. In other
+words, if your model defines ``unique_together = ('author', 'title')`` you
+must include 'author' and 'title' in the list of fields or the constraint
+won't be checked.
 
 .. method:: Model.validate()
 

tests/modeltests/model_forms/models.py

@@ -1114 +1114 @@
 
 >>> 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
+<FieldFile: None>
+
 >>> f = TextFileForm(data={'description': u'Assistance'}, files={'file': SimpleUploadedFile('test3.txt', 'hello world')}, instance=instance)
 >>> f.is_valid()
 True
@@ -1152 +1161 @@
 >>> class BigIntForm(forms.ModelForm):
 ...     class Meta:
 ...         model = BigInt
-... 
+...
 >>> bif = BigIntForm({'biggie': '-9223372036854775808'})
 >>> bif.is_valid()
 True
@@ -1425 +1434 @@
 >>> 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_validate()
 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):

tests/modeltests/model_formsets/models.py

@@ -543 +543 @@
 ...     '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 +1035 @@
 >>> 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)

tests/modeltests/validation/test_unique.py

@@ -1 +1 @@
 import unittest
+import datetime
 from django.conf import settings
 from django.db import connection
 from models import CustomPKModel, UniqueTogetherModel, UniqueFieldsModel, UniqueForDateModel, ModelToValidate
@@ -13 +14 @@
         )
 
     def test_unique_together_gets_picked_up(self):
-        m = UniqueTogetherModel()
+        # Give the fields values, otherwise they won't be picked up in
+        # unique checks.
+        m = UniqueTogetherModel(
+            cfield='test',
+            ifield=1,
+            efield='test@example.com'
+        )
         self.assertEqual(
             ([('ifield', 'cfield',),('ifield', 'efield'), ('id',), ], []),
             m._get_unique_checks()
@@ -24 +31 @@
         self.assertEqual(([('my_pk_field',)], []), m._get_unique_checks())
 
     def test_unique_for_date_gets_picked_up(self):
-        m = UniqueForDateModel()
+        # Give the fields values, otherwise they won't be picked up in
+        # unique checks.
+        m = UniqueForDateModel(
+            start_date=datetime.date(2010, 1, 9),
+            end_date=datetime.datetime(2010, 1, 9, 9, 0, 0),
+            count=1,
+            order=1,
+            name='test'
+        )
         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()
         )
 
@@ -56 +71 @@
         mtv = ModelToValidate(number=10, name='Some Name')
         mtv.full_validate()
         self.assertEqual(l, len(connection.queries))
+

tests/modeltests/validation/tests.py

@@ -1 @@
-from django.core.exceptions import ValidationError, NON_FIELD_ERRORS
-from django.db import models
-
+from django.core.exceptions import NON_FIELD_ERRORS
 from modeltests.validation import ValidationTestCase
-from models import *
+from modeltests.validation.models import 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):
@@ -19 +18 @@
         mtv = ModelToValidate(number=10, name='Some Name')
         self.assertEqual(None, mtv.full_validate())
 
-    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'])
 

tests/regressiontests/admin_views/tests.py

@@ -4 +4 @@
 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
@@ -1753 +1754 @@
         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."])