Opened 5 months ago

Closed 5 months ago

Last modified 5 months ago

#35584 closed Cleanup/optimization (wontfix)

Discard user full names from admin log entries

Reported by: Kamil Paduszyński Owned by:
Component: contrib.admin Version: 5.0
Severity: Normal Keywords:
Cc: Tim Graham, Josh Schneier Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: yes

Description

The get_full_name of the auth.AbstractUser model is used to indicate a user in the admin's object history:

<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:

<td>{{ action.user.get_username }}</td>

In my opinion, a username is enough to identify a user associated with a log.

Change History (4)

comment:1 by Kamil Paduszyński, 5 months ago

UI/UX: set

comment:2 by Sarah Boyce, 5 months ago

Cc: Tim Graham Josh Schneier added
Resolution: wontfix
Status: newclosed

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 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 Django forum to gain a consensus that this should be updated. 👍

comment:3 by Mariusz Felisiak, 5 months ago

Personally, I think it's rather an issue in your code. It's recommended to inherit from 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.

in reply to:  2 comment:4 by Kamil Paduszyński, 5 months ago

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:

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 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 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 Django forum to gain a consensus that this should be updated. 👍

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