Opened 14 years ago

Closed 10 years ago

Last modified 9 years ago

#13147 closed Cleanup/optimization (fixed)

auth.UserCreationForm : unicity check on username not necessary

Reported by: peyman Owned by: nobody
Component: contrib.auth Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Hi,

In django.contrib.auth.UserCreationForm, there is clean_username

that checks if the username already exist in the db. This should not
be necessary because this form is a ModelForm and the associated model
User has unique=True.

Thanks
-- Peyman

Attachments (1)

13147.diff (1.1 KB ) - added by Jeff Schenck 14 years ago.
Patch for 13147

Download all attachments as: .zip

Change History (15)

comment:1 by Karen Tracey, 14 years ago

Triage Stage: UnreviewedAccepted

And checking the queries issued when adding a user you can see that the existence check is done twice. There was a need, I believe, for the custom auth form to do that check manually when it was initially created, but that need is gone now that it's a model form.

by Jeff Schenck, 14 years ago

Attachment: 13147.diff added

Patch for 13147

comment:2 by Luke Plant, 13 years ago

The advantage to the current method is that the validation error issued, while very similar, is significantly nicer grammatically. Since the translated string is a custom one, rather than a generic one generated by the ORM, I imagine in other languages it could make a big difference, even to the level of affecting comprehensibility. I would be reluctant to do this change and produce a regression of usability for some people.

comment:3 by Luke Plant, 13 years ago

Type: Cleanup/optimization

comment:4 by Luke Plant, 13 years ago

Severity: Normal

comment:5 by Julien Phalip, 13 years ago

Triage Stage: AcceptedDesign decision needed

Marking as DDN to reflect Luke's comment.

comment:6 by Aymeric Augustin, 12 years ago

Easy pickings: unset
Resolution: wontfix
Status: newclosed
UI/UX: unset

I agree with Luke.

comment:7 by Aymeric Augustin, 12 years ago

In [17225]:

Explained why UserCreationForm performs custom validation of usernames. Refs #13147.

comment:8 by flisky, 10 years ago

A real fix comes from https://github.com/django/django/pull/2521 with some cleanup.

comment:9 by Tim Graham, 10 years ago

Resolution: wontfix
Status: closednew
Triage Stage: Design decision neededAccepted
Version: 1.1master

The PR looks good to me. I'm going to add a release note to indicate that UserCreationForm.errors_messages['duplicate_username'] is no longer used just in case someone is customizing that.

comment:10 by Tim Graham <timograham@…>, 10 years ago

Resolution: fixed
Status: newclosed

In 849538d03df21b69f0754a38ee4ec5f48fa02c52:

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

comment:11 by Tim Graham <timograham@…>, 10 years ago

In d5e1a2d5eb269df40979cda3eff3ed92c6d06d9c:

Added contrib.auth migration for refs #13147.

comment:12 by Tim Graham <timograham@…>, 10 years ago

In 7affb4ad58e7ee93e4cca0c350315c63035ed648:

Fixed/improved release note for refs #13147; thanks Loic.

comment:13 by Loic Bistuer <loic.bistuer@…>, 10 years ago

In 671e0c937c112b1908048b4d8deb14c0116b8946:

Further fix the release notes for refs #13147.

Mention of custom user models has been removed since UserCreationForm
didn't support custom user models anyway. Refs #19353.

comment:14 by Tim Graham <timograham@…>, 9 years ago

In 8047e366:

Added contrib.auth migration for refs #13147.

Note: See TracTickets for help on using tickets.
Back to Top