Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fixed #13147 -- Moved User validation logic from form to model.

  • Loading branch information...
commit 849538d03df21b69f0754a38ee4ec5f48fa02c52 1 parent d883336
尹吉峰 flisky authored timgraham committed
28 django/contrib/auth/forms.py
View
@@ -74,16 +74,8 @@ class UserCreationForm(forms.ModelForm):
password.
"""
error_messages = {
- 'duplicate_username': _("A user with that username already exists."),
'password_mismatch': _("The two password fields didn't match."),
}
- username = forms.RegexField(label=_("Username"), max_length=30,
- regex=r'^[\w.@+-]+$',
- help_text=_("Required. 30 characters or fewer. Letters, digits and "
- "@/./+/-/_ only."),
- error_messages={
- 'invalid': _("This value may contain only letters, numbers and "
- "@/./+/-/_ characters.")})
password1 = forms.CharField(label=_("Password"),
widget=forms.PasswordInput)
password2 = forms.CharField(label=_("Password confirmation"),
@@ -94,19 +86,6 @@ class Meta:
model = User
fields = ("username",)
- def clean_username(self):
- # Since User.username is unique, this check is redundant,
- # but it sets a nicer error message than the ORM. See #13147.
- username = self.cleaned_data["username"]
- try:
- User._default_manager.get(username=username)
- except User.DoesNotExist:
- return username
- raise forms.ValidationError(
- self.error_messages['duplicate_username'],
- code='duplicate_username',
- )
-
def clean_password2(self):
password1 = self.cleaned_data.get("password1")
password2 = self.cleaned_data.get("password2")
@@ -126,13 +105,6 @@ def save(self, commit=True):
class UserChangeForm(forms.ModelForm):
- username = forms.RegexField(
- label=_("Username"), max_length=30, regex=r"^[\w.@+-]+$",
- help_text=_("Required. 30 characters or fewer. Letters, digits and "
- "@/./+/-/_ only."),
- error_messages={
- 'invalid': _("This value may contain only letters, numbers and "
- "@/./+/-/_ characters.")})
password = ReadOnlyPasswordHashField(label=_("Password"),
help_text=_("Raw passwords are not stored, so there is no way to see "
"this user's password, but you can change the password "
10 django/contrib/auth/models.py
View
@@ -383,8 +383,14 @@ class AbstractUser(AbstractBaseUser, PermissionsMixin):
help_text=_('Required. 30 characters or fewer. Letters, digits and '
'@/./+/-/_ only.'),
validators=[
- validators.RegexValidator(r'^[\w.@+-]+$', _('Enter a valid username.'), 'invalid')
- ])
+ validators.RegexValidator(r'^[\w.@+-]+$',
+ _('Enter a valid username. '
+ 'This value may contain only letters, numbers '
+ 'and @/./+/-/_ characters.'), 'invalid'),
+ ],
+ error_messages={
+ 'unique': _("A user with that username already exists."),
+ })
first_name = models.CharField(_('first name'), max_length=30, blank=True)
last_name = models.CharField(_('last name'), max_length=30, blank=True)
email = models.EmailField(_('email address'), blank=True)
14 django/contrib/auth/tests/test_forms.py
View
@@ -4,7 +4,6 @@
import re
from django import forms
-from django.contrib.auth import get_user_model
from django.contrib.auth.models import User
from django.contrib.auth.forms import (UserCreationForm, AuthenticationForm,
PasswordChangeForm, SetPasswordForm, UserChangeForm, PasswordResetForm,
@@ -36,7 +35,7 @@ def test_user_already_exists(self):
form = UserCreationForm(data)
self.assertFalse(form.is_valid())
self.assertEqual(form["username"].errors,
- [force_text(form.error_messages['duplicate_username'])])
+ [force_text(User._meta.get_field('username').error_messages['unique'])])
def test_invalid_data(self):
data = {
@@ -46,8 +45,8 @@ def test_invalid_data(self):
}
form = UserCreationForm(data)
self.assertFalse(form.is_valid())
- self.assertEqual(form["username"].errors,
- [force_text(form.fields['username'].error_messages['invalid'])])
+ validator = next(v for v in User._meta.get_field('username').validators if v.code == 'invalid')
+ self.assertEqual(form["username"].errors, [force_text(validator.message)])
def test_password_verification(self):
# The verification password is incorrect.
@@ -190,8 +189,7 @@ class CustomAuthenticationForm(AuthenticationForm):
username = CharField()
form = CustomAuthenticationForm()
- UserModel = get_user_model()
- username_field = UserModel._meta.get_field(UserModel.USERNAME_FIELD)
+ username_field = User._meta.get_field(User.USERNAME_FIELD)
self.assertEqual(form.fields['username'].label, capfirst(username_field.verbose_name))
def test_username_field_label_empty_string(self):
@@ -291,8 +289,8 @@ def test_username_validity(self):
data = {'username': 'not valid'}
form = UserChangeForm(data, instance=user)
self.assertFalse(form.is_valid())
- self.assertEqual(form['username'].errors,
- [force_text(form.fields['username'].error_messages['invalid'])])
+ validator = next(v for v in User._meta.get_field('username').validators if v.code == 'invalid')
+ self.assertEqual(form["username"].errors, [force_text(validator.message)])
def test_bug_14242(self):
# A regression test, introduce by adding an optimization for the
4 docs/releases/1.8.txt
View
@@ -414,6 +414,10 @@ Miscellaneous
parameters. Internally, Django will continue to provide the
``pk`` parameter in ``params`` for backwards compatibility.
+* ``UserCreationForm.errors_messages['duplicate_username']`` is no longer used.
+ If you wish to customize that error message, use
+ ``User.error_messages['unique']`` instead.
Loïc Bistuer Collaborator
loic added a note

I'm not too sure what User.error_messages['unique'] means.

From my understanding of the patch we simply let the username model validation happen (and provide a custom error message). To customize it at the form level, users need to rely on a new feature from Django 1.7: https://docs.djangoproject.com/en/dev/topics/forms/modelforms/#considerations-regarding-model-s-error-messages. Since this is a recent feature most people wouldn't be familiar with it, so I think it'd be good to be more explicit.

Tim Graham Owner

Here is what I meant, plus your suggestion:

diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt
index 259ee00..50d553a 100644
--- a/docs/releases/1.8.txt
+++ b/docs/releases/1.8.txt
@@ -415,8 +415,10 @@ Miscellaneous
   ``pk`` parameter in ``params`` for backwards compatibility.

 * ``UserCreationForm.errors_messages['duplicate_username']`` is no longer used.
-  If you wish to customize that error message, use
-  ``User.error_messages['unique']`` instead.
+  If you wish to customize that error message, use either the ``'unique'``
+  key in the :attr:`~django.db.models.Field.error_messages` option of the field
+  designated by :attr:`~django.contrib.auth.models.CustomUser.USERNAME_FIELD` or
+  :ref:`override it on the form <considerations-regarding-model-errormessages>`.

 .. _deprecated-features-1.8:
Loïc Bistuer Collaborator
loic added a note

The first option only works for people with custom user models, right? If so I'd put the general case first then specify "if you have a custom user model you can also, etc.".

Tim Graham Owner

Sounds good. merged in 7affb4a, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
.. _deprecated-features-1.8:
Features deprecated in 1.8
Loïc Bistuer
Collaborator

I'm not too sure what User.error_messages['unique'] means.

From my understanding of the patch we simply let the username model validation happen (and provide a custom error message). To customize it at the form level, users need to rely on a new feature from Django 1.7: https://docs.djangoproject.com/en/dev/topics/forms/modelforms/#considerations-regarding-model-s-error-messages. Since this is a recent feature most people wouldn't be familiar with it, so I think it'd be good to be more explicit.

Tim Graham

Here is what I meant, plus your suggestion:

diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt
index 259ee00..50d553a 100644
--- a/docs/releases/1.8.txt
+++ b/docs/releases/1.8.txt
@@ -415,8 +415,10 @@ Miscellaneous
   ``pk`` parameter in ``params`` for backwards compatibility.

 * ``UserCreationForm.errors_messages['duplicate_username']`` is no longer used.
-  If you wish to customize that error message, use
-  ``User.error_messages['unique']`` instead.
+  If you wish to customize that error message, use either the ``'unique'``
+  key in the :attr:`~django.db.models.Field.error_messages` option of the field
+  designated by :attr:`~django.contrib.auth.models.CustomUser.USERNAME_FIELD` or
+  :ref:`override it on the form <considerations-regarding-model-errormessages>`.

 .. _deprecated-features-1.8:
Loïc Bistuer
Collaborator

The first option only works for people with custom user models, right? If so I'd put the general case first then specify "if you have a custom user model you can also, etc.".

Tim Graham

Sounds good. merged in 7affb4a, thanks.

Please sign in to comment.
Something went wrong with that request. Please try again.