Code

#19394 closed Cleanup/optimization (fixed)

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

Reported by: dloewenherz 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",)

Attachments (0)

Change History (6)

comment:1 Changed 17 months ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to wontfix
  • Status changed from new to closed

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 Changed 17 months ago by dloewenherz

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 Changed 17 months ago by dloewenherz

  • Resolution wontfix deleted
  • Status changed from closed to reopened

Reopening to continue discussion

comment:4 Changed 17 months ago by russellm

  • Component changed from Forms to Documentation
  • Easy pickings set
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Bug to Cleanup/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 Changed 14 months ago by benkonrath

Just made a pull request for this:

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

comment:6 Changed 14 months ago by Preston Holmes <preston@…>

  • Resolution set to fixed
  • Status changed from reopened to closed

In d5462596478992075fd04705fedd6a3cca49fae8:

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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.