Opened 7 years ago

Closed 7 years ago

#28089 closed Cleanup/optimization (fixed)

Remove requirement of get_short_name() and get_full_name() for subclasses of AbstractBaseUser

Reported by: Josh Schneier Owned by: Josh Schneier
Component: contrib.auth Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Since USERNAME_FIELD became required and the corresponding get_username now always exists it doesn't make much sense to me to require the implementation of these 2 methods. They are only used once each in fairly inconsequential parts of the admin (history in addition to username for get_full_name and the greetings in the header block for get_short_name). All other methods are concrete and generally applicable. I do think we should still leave them in AbstractUser.

Change History (5)

comment:1 by Tim Graham, 7 years ago

Type: UncategorizedCleanup/optimization

Do you have any code changes is mind or would this be a documentation clarification? I'm not sure what "removing the requirement" looks like.

comment:2 by Josh Schneier, 7 years ago

Currently in AbstractBaseUser they raise NotImplementedError, I would remove those definitions.

Actually what do you think of simply removing get_short_name and get_full_name altogether? They feel like legacy warts to me given the custom user model. We can just use the __str__ of the user where appropriate (currently get_username() everywhere).

comment:3 by Tim Graham, 7 years ago

Summary: Remove requirement of get_short_name and get_full_name for subclasses of AbstractBaseUserRemove requirement of get_short_name() and get_full_name() for subclasses of AbstractBaseUser
Triage Stage: UnreviewedAccepted

Removing the definitions in AbstractBaseUser that raise NotImplementedError might be reasonable. As far as I see, the admin would work without them:

{% firstof user.get_short_name user.get_username %}
(from contrib/admin/templates/admin/base.html)

<td>{{ action.user.get_username }}{% if action.user.get_full_name %} ({{ action.user.get_full_name }}){% endif %}</td>
(from contrib/admin/templates/admin/object_history.html)

Currently, if a subclass doesn't want to use those methods, it has to define them to return an empty value to suppress the NotImplementedError exceptions while rendering those admin templates.

Documentation updates as well as a mention in the release notes for third-party apps that might expect those methods to exist are also required.

comment:4 by Josh Schneier, 7 years ago

Has patch: set
Owner: changed from nobody to Josh Schneier
Status: newassigned

comment:5 by Tim Graham <timograham@…>, 7 years ago

Resolution: fixed
Status: assignedclosed

In 5df0ff4:

Fixed #28089 -- Removed requirement to implement get_short_name() and get_full_name() in AbstractBaseUser subclasses.

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