#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)
Change History (15)
comment:1 Changed 14 years ago by
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:2 Changed 13 years ago by
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 12 years ago by
| Type: | → Cleanup/optimization |
|---|
comment:4 Changed 12 years ago by
| Severity: | → Normal |
|---|
comment:5 Changed 12 years ago by
| Triage Stage: | Accepted → Design decision needed |
|---|
Marking as DDN to reflect Luke's comment.
comment:6 Changed 12 years ago by
| Easy pickings: | unset |
|---|---|
| Resolution: | → wontfix |
| Status: | new → closed |
| UI/UX: | unset |
I agree with Luke.
comment:8 Changed 9 years ago by
A real fix comes from https://github.com/django/django/pull/2521 with some cleanup.
comment:9 Changed 9 years ago by
| Resolution: | wontfix |
|---|---|
| Status: | closed → new |
| Triage Stage: | Design decision needed → Accepted |
| Version: | 1.1 → master |
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 9 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
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.