Ticket #12521: 12521.diff

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
@@ -63 +63 @@
         if hasattr(self, 'message_dict'):
             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
-

django/db/models/base.py

@@ -649 +649 @@
         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):
+        if exclude is None:
+            exclude = []
+        unique_checks, date_checks = self._get_unique_checks(exclude)
 
         errors = self._perform_unique_checks(unique_checks)
         date_errors = self._perform_date_checks(date_checks)
@@ -663 +665 @@
         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):
+        if exclude is None:
+            exclude = []
+        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 a unique_check.
+                if name in exclude:
+                    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 in exclude:
+                continue
             if f.unique:
                 unique_checks.append((name,))
             if f.unique_for_date:
@@ -684 +707 @@
                 date_checks.append(('month', name, f.unique_for_month))
         return unique_checks, date_checks
 
-
     def _perform_unique_checks(self, unique_checks):
         errors = {}
 
@@ -781 +803 @@
                 'field_label': unicode(field_labels)
             }
 
-    def full_validate(self, exclude=[]):
+    def full_validate(self, exclude=None):
         """
         Cleans all fields and raises 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:
@@ -795 +819 @@
             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()
+            self.validate_unique(exclude=exclude.extend(errors.keys()))
         except ValidationError, e:
             if hasattr(e, 'message_dict'):
                 if errors:
                     for k, v in e.message_dict.items():
-                        errors.set_default(k, []).extend(v)
+                        errors.setdefault(k, []).extend(v)
                 else:
                     errors = e.message_dict
             else:
                 errors[NON_FIELD_ERRORS] = e.messages
 
+        # Form.clean() is run even if other validation fails, so do the
+        # same with Model.validate() for consistency.
+        # However, do not run Model.validate() on incomplete forms when
+        # creating new instance.
+        if not (exclude and getattr(self, '_adding', True)):
+            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)
 

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, NON_FIELD_ERRORS
 from django.core.validators import EMPTY_VALUES
 from util import ErrorList
 from forms import BaseForm, get_declared_fields
@@ -249 +249 @@
         opts = self._meta
         self.instance = construct_instance(self, self.instance, opts.fields, opts.exclude)
         try:
-            self.instance.full_validate(exclude=self._errors.keys())
+            # For backwards-compatibility, several types of fields need to be
+            # excluded from model validation. See the following tickets for
+            # details: #12507, #12521, #12553
+            exclude = []
+            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(field)
+                # Exclude fields that failed form validation.
+                elif field in self._errors.keys():
+                    exclude.append(field)
+                # Exclude empty fields that are not required by the form. Model
+                # validation might complain that they are null, 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) in EMPTY_VALUES):
+                    exclude.append(field)
+            self.instance.full_validate(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):

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

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

@@ -19 +19 @@
         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'])
+        # model.validate() will not be called on unsaved models that otherwise
+        # fail validation.
+        self.assertFailsValidation(mtv.full_validate, ['name'])
+        mtv.name = 'test'
+        # model.validate() *should* be called if the model otherwise *does* validate
+        self.assertFailsValidation(mtv.full_validate, [NON_FIELD_ERRORS])
 
     def test_wrong_FK_value_raises_error(self):
         mtv=ModelToValidate(number=10, name='Some Name', parent_id=3)

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."])