[Django] #35584: Discard user full names from admin log entries

12 views
Skip to first unread message

Django

unread,
Jul 8, 2024, 7:53:13 AM7/8/24
to django-...@googlegroups.com
#35584: Discard user full names from admin log entries
-------------------------------------+-------------------------------------
Reporter: Kamil Paduszyński | Type:
| Cleanup/optimization
Status: new | Component:
| contrib.admin
Version: 5.0 | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
The `get_full_name` of the `auth.AbstractUser` model is used to indicate a
user in the admin's object history:

{{{
#!default
<td>{{ action.user.get_username }}{% if action.user.get_full_name %} ({{
action.user.get_full_name }}){% endif %}</td>
}}}

In some circumstances, personal data like `first_name` or `last_name`
(used in the mentioned method) are generally handled by a different model,
where the `AUTH_USER_MODEL` is for the authentication data and permissions
only.

Given I discard `first_name` and `last_name` from my user model and mark
the respective methods as raising `NotImplementedError`, I have to
remember to override the entire huge `content` block just to update a
single line of code... In the case of the `get_short_name` method used in
the `admin/base.html` template, the issue is much easier to resolve due to
a better template organization (there, an update in a single block
`welcome-msg` is needed).

Therefore, I think it would be better to remove the reference to
`get_full_name` from the template, simply by replacing it by:

{{{
#!default
<td>{{ action.user.get_username }}</td>
}}}

In my opinion, a username is enough to identify a user associated with a
log.
--
Ticket URL: <https://code.djangoproject.com/ticket/35584>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jul 8, 2024, 7:54:02 AM7/8/24
to django-...@googlegroups.com
#35584: Discard user full names from admin log entries
-------------------------------------+-------------------------------------
Reporter: Kamil Paduszyński | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Changes (by Kamil Paduszyński):

* ui_ux: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35584#comment:1>

Django

unread,
Jul 8, 2024, 12:43:58 PM7/8/24
to django-...@googlegroups.com
#35584: Discard user full names from admin log entries
-------------------------------------+-------------------------------------
Reporter: Kamil Paduszyński | Owner: (none)
Type: | Status: closed
Cleanup/optimization |
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* cc: Tim Graham, Josh Schneier (added)
* resolution: => wontfix
* status: new => closed

Comment:

Hi Kamil, can you not overwrite `get_full_name` to return `""` (an empty
string, which is False-y and should achieve the same thing as overwriting
the template)?

This feels slightly related to #28089. I think the docs around
[https://docs.djangoproject.com/en/5.0/topics/auth/customizing/#django.contrib.auth.models.CustomUser.get_full_name
CustomUser.get_full_name()] might want an update to add that the default
is a concatenation of the first and last name rather than "If implemented,
this appears alongside the username in an object’s history" (which doesn't
feel 100% accurate).

The request to completely drop `get_full_name` from the admin log entries
would need a wider discussion, as so I will close this as wontfix for now,
please raise this on the [https://forum.djangoproject.com/c/internals/5
Django forum] to gain a consensus that this should be updated. 👍
--
Ticket URL: <https://code.djangoproject.com/ticket/35584#comment:2>

Django

unread,
Jul 8, 2024, 1:16:05 PM7/8/24
to django-...@googlegroups.com
#35584: Discard user full names from admin log entries
-------------------------------------+-------------------------------------
Reporter: Kamil Paduszyński | Owner: (none)
Type: | Status: closed
Cleanup/optimization |
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):

Personally, I think it's rather an issue in your code. It's recommended to
inherit from
[https://docs.djangoproject.com/en/5.0/topics/auth/customizing/#specifying-a
-custom-user-model AbstractBaseUser] when you want to specify a custom
user model. `AbstractBaseUser` doesn't have the `get_full_name()`, so you
wouldn't have anything to override.
--
Ticket URL: <https://code.djangoproject.com/ticket/35584#comment:3>

Django

unread,
Jul 8, 2024, 4:50:16 PM7/8/24
to django-...@googlegroups.com
#35584: Discard user full names from admin log entries
-------------------------------------+-------------------------------------
Reporter: Kamil Paduszyński | Owner: (none)
Type: | Status: closed
Cleanup/optimization |
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (by Kamil Paduszyński):

Sarah, Mariusz - thanks for responding so quickly.

Indeed, I don't have to override the `get_full_name`. The point is I don't
want to keep the `first_name` and `last_name` fields in the model. I
simply don't find them useful in the `auth` app in general - this app
intends to handle authentication and authorization, not personal data. The
best place to add the latter would be the custom `AbstractUser` (not
`AbstractBaseUser`) subclasses (it's my personal opinion though).

Going with:

{{{
#!python
class CustomUser(AbstractBaseUser):
...
}}}

suggested by Mariusz results in permissions unhandled (yeah, I know I can
add `PermissionMixin` 🙂 - but it's just another point to remember) and
some important fields (`email`, `is_staff`, `is_active`) missing. I think
most of the projects would like to have those functionalities in their
custom user models...

Anyway, I respect your decision, and thank you guys for the discussion 👍

Replying to [comment:2 Sarah Boyce]:
> Hi Kamil, can you not overwrite `get_full_name` to return `""` (an empty
string, which is False-y and should achieve the same thing as overwriting
the template)?
>
> This feels slightly related to #28089. I think the docs around
[https://docs.djangoproject.com/en/5.0/topics/auth/customizing/#django.contrib.auth.models.CustomUser.get_full_name
CustomUser.get_full_name()] might want an update to add that the default
is a concatenation of the first and last name rather than "If implemented,
this appears alongside the username in an object’s history" (which doesn't
feel 100% accurate).
>
> The request to completely drop `get_full_name` from the admin log
entries would need a wider discussion, as so I will close this as wontfix
for now, please raise this on the
[https://forum.djangoproject.com/c/internals/5 Django forum] to gain a
consensus that this should be updated. 👍
--
Ticket URL: <https://code.djangoproject.com/ticket/35584#comment:4>
Reply all
Reply to author
Forward
0 new messages