[Django] #26823: auth signal receiver update_last_login not compatible with custom user models that have no last_login field
Django
models that have no last_login field
-------------------------------+--------------------
Reporter: hjwp | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.9
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
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...
--
Ticket URL: <https://code.djangoproject.com/ticket/26823>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
models that have no last_login field
Reporter: hjwp | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.9
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: 0
Changes (by hjwp):
* component: Uncategorized => contrib.auth
* needs_better_patch: => 0
* type: Uncategorized => Bug
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/26823#comment:1>
Django
models that have no last_login field
Reporter: hjwp | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.9
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by hjwp):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/26823#comment:2>
Django
models that have no last_login field
Reporter: hjwp | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.9
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by hjwp):
cf https://github.com/django/django/pull/6855
--
Ticket URL: <https://code.djangoproject.com/ticket/26823#comment:3>
Django
models that have no last_login field
Reporter: hjwp | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.9
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Unreviewed => Accepted
Comment:
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/26823#comment:4>
Django
models that have no last_login field
Reporter: hjwp | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* needs_better_patch: 0 => 1
Comment:
+1 to connect the signal in `ready()`
--
Ticket URL: <https://code.djangoproject.com/ticket/26823#comment:5>
Django
models that have no last_login field
Reporter: hjwp | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by apollo13):
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`
--
Ticket URL: <https://code.djangoproject.com/ticket/26823#comment:6>
Django
models that have no last_login field
Reporter: Harry Percival | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by Linus Lewandowski):
[https://github.com/django/django/pull/8557 PR].
--
Ticket URL: <https://code.djangoproject.com/ticket/26823#comment:7>
Django
models that have no last_login field
Reporter: Harry Percival | Owner: nobody
Type: Bug | Status: new
Component: contrib.auth | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/26823#comment:8>
Django
models that have no last_login field
Reporter: Harry Percival | Owner: nobody
Component: contrib.auth | Version: 1.9
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"eedc88bd4a8468c2a0daa8346c9b57183dd77782" eedc88bd]:
{{{
#!CommitTicketReference repository=""
revision="eedc88bd4a8468c2a0daa8346c9b57183dd77782"
Fixed #26823 -- Prevented update_last_login signal receiver from crashing
if User model doesn't have last_login field.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26823#comment:9>