diff --git a/django/contrib/auth/forms.py b/django/contrib/auth/forms.py index dbc55ca..e9a7fb2 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: @@ -43,6 +37,10 @@ class UserCreationForm(forms.ModelForm): 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)."), diff --git a/django/core/exceptions.py b/django/core/exceptions.py index fee7db4..29da67c 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 @@ -63,11 +63,3 @@ class BaseValidationError(Exception): 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 - diff --git a/django/db/models/base.py b/django/db/models/base.py index 06db7cc..cfe7fec 100644 --- a/django/db/models/base.py +++ b/django/db/models/base.py @@ -649,10 +649,12 @@ class Model(object): 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,17 +665,38 @@ 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): + 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_ + # 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. 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,7 +707,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 = {} @@ -781,11 +803,13 @@ class Model(object): '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,20 +819,35 @@ class Model(object): 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) diff --git a/django/forms/models.py b/django/forms/models.py index ff20c93..5435f2a 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 @@ -249,27 +249,36 @@ class BaseModelForm(BaseForm): 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): diff --git a/tests/modeltests/model_forms/models.py b/tests/modeltests/model_forms/models.py index ba59f9a..cb2aa14 100644 --- a/tests/modeltests/model_forms/models.py +++ b/tests/modeltests/model_forms/models.py @@ -1114,6 +1114,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 @@ -1152,7 +1161,7 @@ True >>> class BigIntForm(forms.ModelForm): ... class Meta: ... model = BigInt -... +... >>> bif = BigIntForm({'biggie': '-9223372036854775808'}) >>> bif.is_valid() True @@ -1425,16 +1434,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_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): diff --git a/tests/modeltests/model_formsets/models.py b/tests/modeltests/model_formsets/models.py index 5eab202..f41908b 100644 --- a/tests/modeltests/model_formsets/models.py +++ b/tests/modeltests/model_formsets/models.py @@ -1031,6 +1031,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/test_unique.py b/tests/modeltests/validation/test_unique.py index cbb56aa..3aea99d 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 @@ -13,7 +14,13 @@ class GetUniqueCheckTests(unittest.TestCase): ) 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,10 +31,18 @@ class GetUniqueCheckTests(unittest.TestCase): 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,3 +71,4 @@ class PerformUniqueChecksTest(unittest.TestCase): mtv = ModelToValidate(number=10, name='Some Name') mtv.full_validate() self.assertEqual(l, len(connection.queries)) + diff --git a/tests/modeltests/validation/tests.py b/tests/modeltests/validation/tests.py index c00070b..7bf3994 100644 --- a/tests/modeltests/validation/tests.py +++ b/tests/modeltests/validation/tests.py @@ -19,9 +19,14 @@ class BaseModelValidationTests(ValidationTestCase): 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) diff --git a/tests/regressiontests/admin_views/tests.py b/tests/regressiontests/admin_views/tests.py index 899fff8..351974b 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 @@ -1753,3 +1754,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."])