Opened 11 days ago

Last modified 39 hours ago

#35742 assigned Cleanup/optimization

Apply UserAdmin add_form_template according to User model USERNAME_FIELD

Reported by: antoliny0919 Owned by: antoliny0919
Component: contrib.admin Version: 5.1
Severity: Normal Keywords: UserAdmin, add_form_template
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by antoliny0919)

Hello!! I made a ticket because there was something ambiguous on the Add page of the UserAdmin model.

The UserAdmin model customizes the ModelAdmin Add page with add_form_template variable.
And the template used as the value for that variable is admin/auth/user/add_form.html.

I felt this part was ambiguous in the add_form template.

...
{% block form_top %}
  {% if not is_popup %}
    <p>{% translate 'First, enter a username and password. Then, you’ll be able to edit more user options.' %}</p>
  {% else %}
    <p>{% translate "Enter a username and password." %}</p>
  {% endif %}
{% endblock %}
...

In the UserAdmin model, when a user uses a custom user model, USERNAME_FIELD allows a field other than username to be used.

However, in the above template, <p>{%translate 'First, enter a username and password. Then, you'll be able to edit more user options.'%}</p> This part is target at the default user provided by Django, so it is awkward because "username" is also seen when customizing USERNAME_FIELD using custom users.

Is this part using a static value because it is a ModelAdmin based on the User model provided by Django?

I added the method to the UserAdmin model and modified add_form.html to show the tag using the USERNAME_FIELD value.

I respect the great code from the Django maintainer and contributors. But I made a ticket out of my personal opinion that the code would be more curious and in a better way.

Thank you for reading it.

Change History (18)

comment:1 by antoliny0919, 11 days ago

Has patch: set
Last edited 9 days ago by antoliny0919 (previous) (diff)

comment:2 by antoliny0919, 11 days ago

Description: modified (diff)

comment:3 by Clifford Gama, 11 days ago

Needs tests: set
Triage Stage: UnreviewedAccepted
Type: New featureCleanup/optimization

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 says First, enter a username and password. Then, you’ll be able to edit more user options. with no
way to change that.

Last edited 11 days ago by Clifford Gama (previous) (diff)

comment:5 by Clifford Gama, 10 days ago

Patch needs improvement: set

comment:6 by Clifford Gama, 10 days ago

Owner: set to antoliny0919
Status: newassigned

in reply to:  3 comment:7 by antoliny0919, 10 days 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 says First, 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:8 by antoliny0919, 9 days ago

I created PR through good advice!

PR -> https://github.com/django/django/pull/18552

Last edited 9 days ago by antoliny0919 (previous) (diff)

comment:9 by antoliny0919, 7 days ago

Triage Stage: AcceptedUnreviewed

comment:10 by Natalia Bidart, 7 days ago

Triage Stage: UnreviewedAccepted

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 Natalia Bidart, 7 days 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 antoliny0919, 6 days 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.

Last edited 6 days ago by antoliny0919 (previous) (diff)

comment:13 by antoliny0919, 5 days ago

Needs tests: unset
Patch needs improvement: unset

comment:14 by antoliny0919, 4 days 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 Claude Paroz, 4 days 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 Natalia Bidart, 3 days ago

Needs tests: set
Patch needs improvement: set

comment:17 by antoliny0919, 2 days ago

I updated the test code and document!!
I'd appreciate it if you could review my code.

comment:18 by antoliny0919, 39 hours ago

Needs tests: unset
Patch needs improvement: unset
Note: See TracTickets for help on using tickets.
Back to Top