-
Notifications
You must be signed in to change notification settings - Fork 670
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
user_logged_in instead of update_last_login #752
base: master
Are you sure you want to change the base?
Conversation
By pure coincidence, I just tried the library django-axes and without this code, it raises:
So this code seems to fix this issue as well. I don't understand the reason for calling the signals directly anyway! |
Fixed compatibility with `django-axes` error. ``` TypeError: AxesProxyHandler.user_logged_in() missing 1 required positional argument: 'request' ``` According to `django-axes`: ``` Authenticating users¶ Axes needs a request attribute to be supplied to the stock Django authenticate method in the django.contrib.auth module in order to function correctly. ``` https://django-axes.readthedocs.io/en/latest/3_usage.html
for more information, see https://pre-commit.ci
The new commit fixes the django-axes issue mentioned above. |
self.__class__ is more accurate.
Also, the 2 I think This must be updated. Example:
|
One could (or even should) also add In which case, I don't know if any of the signals also applies to any of the rest of the serializers (TokenRefreshSerializer, TokenRefreshSlidingSerializer, TokenVerifySerializer). |
Let me know if this is going anywhere or not. |
Hi please rebase with master |
I did not see a reason to call
update_last_login
directly.This allows code like the following to function properly.
I also thought of providing the
data
variable as a kwarg to allow even more flexibility.user_logged_in.send(sender=self.user.__class__, user=self.user, **data)
oruser_logged_in.send(sender=self.user.__class__, user=self.user, data=data)