Opened 8 years ago
Closed 8 years ago
#26823 closed Bug (fixed)
auth signal receiver update_last_login not compatible with custom user models that have no last_login field
Reported by: | Harry Percival | Owned by: | nobody |
---|---|---|---|
Component: | contrib.auth | Version: | 1.9 |
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
In my experiments with building a minimal custom user model, I've managed to build one that has just one field, email. Which I think is brilliant of course. One feature in the auth
module is stopping it from working though, the update_last_login
signal receiver from contrib.auth.models, which is wired up in auth.signals.user_logged_in
.
My workaround is to do a
from django.contrib import auth auth.signals.user_logged_in.disconnect(auth.models.update_last_login)
But we could make the signal handler more resilient. Something like this?
def update_last_login(sender, user, **kwargs): if not hasattr(user, 'last_login'): return
PR to follow...
Change History (9)
comment:1 by , 8 years ago
Component: | Uncategorized → contrib.auth |
---|---|
Type: | Uncategorized → Bug |
comment:2 by , 8 years ago
Has patch: | set |
---|
comment:3 by , 8 years ago
comment:4 by , 8 years ago
Triage Stage: | Unreviewed → Accepted |
---|
An alternative implementation I thought of would be to introspect the user model and see if it has a last_login
field and avoid connecting the signal if not. I'm not sure it's feasible with user_logged_in.connect(update_last_login)
at the module level since get_user_model()
can't be called there, but maybe if the signal connecting happened in AuthConfig.ready()
that could work?
comment:6 by , 8 years ago
Signal connection in ready()
should not be done lightly, this method should be idempotent -- either just import a module from there or make sure to set dispatch_uid
comment:8 by , 8 years ago
Patch needs improvement: | unset |
---|
cf https://github.com/django/django/pull/6855