Opened 12 years ago

Closed 7 years ago

Last modified 6 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 11 years ago.
Patch for 13147

Download all attachments as: .zip

Change History (15)

comment:1 Changed 12 years ago by Karen Tracey

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.

Changed 11 years ago by Jeff Schenck

Attachment: 13147.diff added

Patch for 13147

comment:2 Changed 11 years ago by Luke Plant

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 Changed 10 years ago by Luke Plant

Type: Cleanup/optimization

comment:4 Changed 10 years ago by Luke Plant

Severity: Normal

comment:5 Changed 10 years ago by Julien Phalip

Triage Stage: AcceptedDesign decision needed

Marking as DDN to reflect Luke's comment.

comment:6 Changed 10 years ago by Aymeric Augustin

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

I agree with Luke.

comment:7 Changed 10 years ago by Aymeric Augustin

In [17225]:

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

comment:8 Changed 7 years ago by flisky

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

comment:9 Changed 7 years ago by Tim Graham

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 Changed 7 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In 849538d03df21b69f0754a38ee4ec5f48fa02c52:

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

comment:11 Changed 7 years ago by Tim Graham <timograham@…>

In d5e1a2d5eb269df40979cda3eff3ed92c6d06d9c:

Added contrib.auth migration for refs #13147.

comment:12 Changed 7 years ago by Tim Graham <timograham@…>

In 7affb4ad58e7ee93e4cca0c350315c63035ed648:

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

comment:13 Changed 7 years ago by Loic Bistuer <loic.bistuer@…>

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 Changed 6 years ago by Tim Graham <timograham@…>

In 8047e366:

Added contrib.auth migration for refs #13147.

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