Opened 11 years ago

Closed 11 years ago

#19394 closed Cleanup/optimization (fixed)

Forms in django.contrib.auth have "username" as a hardcoded field value

Reported by: Dan Loewenherz Owned by: nobody
Component: Documentation Version: 1.5-beta-1
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

This is a barrier to users who want to make use of these forms after the 1.5 upgrade with custom user field names.

E.g.

class UserCreationForm(forms.ModelForm):
    """
    A form that creates a user, with no privileges, from the given username and
    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"),
        widget=forms.PasswordInput,
        help_text=_("Enter the same password as above, for verification."))

    class Meta:
        model = User
        fields = ("username",)

Change History (6)

comment:1 by Russell Keith-Magee, 11 years ago

Resolution: wontfix
Status: newclosed

I completely agree. However, in this case, we're stuck between a rock and a hard place.

These forms are tightly bound to the specific user model, so it's very hard to make a truly generic form that will work with all user models. I made a design decision to not try and be too smart with this -- I kept the UserCreationForm etc simple and username-specific, and documented the fact that end-users will need to define their own forms. Given that the form logic isn't that complicated (if you look at UserCreationForm, for example, most of the code is actually setting up error messages and regexes for parsing username), I didn't think it was *that* onerous.

The one exception to this is the AuthenticationForm -- in that case we're using the conceit of calling the "unique identifier field" username, regardless of what it is actually called. This makes form processing for logins much easier.

So - absent of a specific proposal to fix the problem -- e.g., showing how the form could be refactored -- I'm going to mark this wontfix. If anyone can propose a specific way to address this that doesn't result in hideously complex code, or an complex class heirarchy that stills require a bunch of code to be written, feel free to reopen.

comment:2 by Dan Loewenherz, 11 years ago

Hmm, I see the difficulty here.

Given the situation, I would push to add a little snippet in the docs that note that the forms are only usable with the built in user model. See https://docs.djangoproject.com/en/dev/topics/auth/#module-django.contrib.auth.forms

Maybe add a little note after this paragraph?

"If you don't want to use the built-in views, but want the convenience of not having to write forms for this functionality, the authentication system provides several built-in forms located in django.contrib.auth.forms:"

I can submit the patch as well if that's ok.

comment:3 by Dan Loewenherz, 11 years ago

Resolution: wontfix
Status: closedreopened

Reopening to continue discussion

comment:4 by Russell Keith-Magee, 11 years ago

Component: FormsDocumentation
Easy pickings: set
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

Sure - there's certinaly some room for improvement there. If you go down to the docs on swappable users, there's a detailed description of the limitations of the individual forms; a not linking from the point you've highlighted to the docs on those limitations would certainly be appropriate.

comment:5 by Ben Kornrath, 11 years ago

Just made a pull request for this:

https://github.com/django/django/pull/827

comment:6 by Preston Holmes <preston@…>, 11 years ago

Resolution: fixed
Status: reopenedclosed

In d5462596478992075fd04705fedd6a3cca49fae8:

Fixed #19394 --Added note about auth forms and custom user models.

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