#35742 closed Cleanup/optimization (fixed)
Avoid refering to "username" in admin templates as USERNAME_FIELD can be updated
Reported by: | antoliny0919 | Owned by: | antoliny0919 |
---|---|---|---|
Component: | contrib.auth | Version: | 5.1 |
Severity: | Normal | Keywords: | UserAdmin, USERNAME_FIELD |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Hello!! I made a ticket because I thought that UserAdmin can be improved through USERNAME_FIELD of User model and UserAdmin provided by Django.
This is a way to provide more convenient UserAdmin when django users customize their user models.
The add_fieldsetes in UserAdmin currently provided by Django use the static "username".
@admin.register(User) class UserAdmin(admin.ModelAdmin): add_fieldsets = ( ( None, { "classes": ("wide",), "fields": ("username", "usable_password", "password1", "password2"), }, ), )
Assuming that the user customizes the user model and uses email as an example of USERNAME_FIELD, UserAdmin must be customized by the user as follows.
# admin.py class CustomUserAdmin(UserAdmin): add_fieldsets = ( ( None, { "classes": ("wide",), "fields": ("email", "usable_password", "password1", "password2"), }, ), ) # add_form.html {% block form_top %} {% if not is_popup %} <p>{% translate 'First, enter a email and password. Then, you’ll be able to edit more user options.' %}</p> {% else %} <p>{% translate "Enter a email and password." %}</p> {% endif %} {% endblock %}
I think it's better to replace the static "username" with the USERNAME_FIELD dynamic value in this part.
# admin.py class CustomUserAdmin(UserAdmin): add_fieldsets = ( ( None, { "classes": ("wide",), "fields": (User.USERNAME_FIELD, "usable_password", "password1", "password2"), }, ), ) # add_form.html {% block form_top %} {% if not is_popup %} <p>{% blocktranslate %}First, enter a {{ username }} and password. Then, you’ll be able to edit more user options.{% endblocktranslate %}</p> {% else %} <p>{% blcoktranslate %}Enter a {{ username }} and password.{% endblocktranslate %}</p> {% endif %} {% endblock %}
Even if I keep the original code, there is no problem with the function, but if I replace it with a dynamic value as above, I thought it would be more convenient for the django users to customize the User model, so I suggested this part.
I would also like to know the opinions of Django members and contributors about this improvement.
Thank you for reading my improvements :)
Change History (30)
comment:2 by , 3 months ago
Description: | modified (diff) |
---|
follow-up: 7 comment:3 by , 3 months ago
Needs tests: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
Type: | New feature → Cleanup/optimization |
comment:5 by , 3 months ago
Patch needs improvement: | set |
---|
comment:6 by , 3 months ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:7 by , 3 months ago
Replying to Clifford Gama:
Hello @antoliny0919!
Thank you for submitting this ticket and the accompanying PR.
I am accepting the ticket as this seems to be a legitimate accessibility issue when USERNAME_FIELD
has been renamed, and I haven't found another ticket that deals with that specifically.
To be clear: This is about how the prompt text on the user creation form does not respect the USERNAME_FIELD
and always saysFirst, enter a username and password. Then, you’ll be able to edit more user options.
with no
way to change that.
Replying to Clifford Gama:
Thank you so much @Clifford Gama
I checked the advice you gave me in PR.
I also agree with the parts you advised and have revised the source code.
comment:9 by , 3 months ago
Triage Stage: | Accepted → Unreviewed |
---|
comment:10 by , 3 months ago
Triage Stage: | Unreviewed → Accepted |
---|
Hey antoliny, thank you for your work!
Please note that the Triage Stage should not be changed to unreviewed because that means that the ticket (as a report) needs triaging. If, instead, what needs review is your PR, you need to unset the relevant ticket flags (needs improvement, needs tests, needs docs, etc). The contributing docs provides details about this workflow: https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/submitting-patches/#patch-style
comment:11 by , 3 months ago
I've been doing some more investigation for this ticket, and while I agree this should be improved, I think there are a few more places where we need to fix the usage of the literal "username". These are:
- Password reset link on the admin login page, when password reset is enabled. Currently shows "Forgotten your password or username?"
- (Unsure) The password reset email shows "Your username, in case you’ve forgotten:". I guess this should say "your account identifier" or similar?
The PR is on the right path but needs tests for sure.
comment:12 by , 3 months ago
First of all, I'm sorry for the change in the Triage Stage. I was wondering what the Django members thought about this ticket and I didn't know if I was proceeding correctly.
As you advised, I will look for more parts that need to be fixed with USERNAME_FIELD and commit again.
And I will request it when I need a review.
Thank you for your kind answer.
comment:13 by , 3 months ago
Needs tests: | unset |
---|---|
Patch needs improvement: | unset |
comment:14 by , 3 months ago
I have updated all the .po files so that the username template variable can be translated.
Only the part that was previously used as 'username' was changed to %(username)s, but some languages were unable to change the 'username' part because it was difficult to distinguish exactly.
What should I do about these parts?
comment:15 by , 3 months ago
.po files should never be touched by non-translators except for cases where tests are affected. Translation updates follow their own workflow (generally during the last weeks before a new major release).
comment:16 by , 3 months ago
Needs tests: | set |
---|---|
Patch needs improvement: | set |
comment:17 by , 3 months ago
I updated the test code and document!!
I'd appreciate it if you could review my code.
comment:18 by , 3 months ago
Needs tests: | unset |
---|---|
Patch needs improvement: | unset |
comment:19 by , 3 months ago
Patch needs improvement: | set |
---|
comment:20 by , 3 months ago
Patch needs improvement: | unset |
---|
comment:21 by , 3 months ago
Patch needs improvement: | set |
---|
comment:22 by , 3 months ago
Component: | contrib.admin → contrib.auth |
---|---|
Description: | modified (diff) |
Keywords: | USERNAME_FIELD added; add_form_template removed |
Summary: | Apply UserAdmin add_form_template according to User model USERNAME_FIELD → Django's UserAdmin improvement with dynamic value USERNAME_FIELD |
comment:23 by , 3 months ago
Patch needs improvement: | unset |
---|
comment:24 by , 2 months ago
Patch needs improvement: | set |
---|
comment:25 by , 2 months ago
Patch needs improvement: | unset |
---|
comment:26 by , 2 months ago
Summary: | Django's UserAdmin improvement with dynamic value USERNAME_FIELD → Avoid refering to "username" in admin templates as USERNAME_FIELD can be updated |
---|
Updated the ticket description to reduce the scope overlap with #28608
comment:27 by , 2 months ago
Patch needs improvement: | set |
---|
comment:28 by , 2 months ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
Hello @antoliny0919!
Thank you for submitting this ticket and the accompanying PR.
I am accepting the ticket as this seems to be a legitimate accessibility issue when USERNAME_FIELD
has been renamed, and I haven't found another issue that deals with that specifically.
To be clear: This is about how the prompt text on the user creation form does not respect the USERNAME_FIELD
and always says
First, enter a username and password. Then, you’ll be able to edit more user options.
with noway to change that.