update_last_login signal handler shouldn't be connected if User.last_login attribute isn't a field

[Mikhail Porokhovnichenko]

Please note! It's not a #26823 duplicate, and it's just an issue related this one.

In the current implementation, a signal handler connects with a user model when this model has a last_login field.

if hasattr(get_user_model(), 'last_login'):
    from .models import update_last_login
    user_logged_in.connect(update_last_login, dispatch_uid='update_last_login')

And it would work when there isn't a last_login attribute in the custom user model. But what if I've inherited my custom model from AbstractUser and dropped the last_login by setting it to None?

class User(AbstractBaseUser):
    last_login = None     # Drop ``last_login`` field off

Technically, the model has a last_login attribute, but it's not a field!

Suggesting change the check condition something like that:

last_login = getattr(get_user_model(), 'last_login', None)
if last_login is not None:
    # ...

or even

if isisintance(last_login, models.Field):
    # ...

[Tim Graham]

I was thinking that isinstance(... Field) might be incorrect if last_login was a settable property, but in that case, I guess user.save(update_fields['last_login']) probably wouldn't be desired.

[Mikhail Porokhovnichenko]

Sorry, I didn't get it. What approach you'd like to use here? Need I create a patch?

[Tim Graham]

Try the isinstance() approach. You're welcome to create a patch. Most of the work will be writing a test.

[Mikhail Porokhovnichenko]

Ok, will do it ASAP

[Mikhail Porokhovnichenko]

Seems to be done. Can be reviewed ​here

[Mikhail Porokhovnichenko]

Please check ​PR#9712.

[Tim Graham <timograham@…>]

In 14e34dcf:

Fixed #29132 -- Avoided connecting update_last_login() handler if User.last_login isn't a field.