-
Notifications
You must be signed in to change notification settings - Fork 698
feat: Add custom get user queryset method (fixes #903) #961
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| from typing import Optional, TypeVar | ||
| from typing import Any, Optional, TypeVar | ||
|
|
||
| from django.contrib.auth import get_user_model | ||
| from django.contrib.auth.models import AbstractBaseUser | ||
|
|
@@ -121,6 +121,9 @@ def get_validated_token(self, raw_token: bytes) -> Token: | |
| } | ||
| ) | ||
|
|
||
| def get_user_queryset(self, user_id: Any = None) -> AuthUser | None: | ||
| return self.user_model.objects.get(**{api_settings.USER_ID_FIELD: user_id}) | ||
|
|
||
| def get_user(self, validated_token: Token) -> AuthUser: | ||
| """ | ||
| Attempts to find and return a user using the given validated token. | ||
|
|
@@ -133,7 +136,9 @@ def get_user(self, validated_token: Token) -> AuthUser: | |
| ) from e | ||
|
|
||
| try: | ||
| user = self.user_model.objects.get(**{api_settings.USER_ID_FIELD: user_id}) | ||
| user = self.get_user_queryset(user_id) | ||
| if not user: | ||
|
Comment on lines
+139
to
+140
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DRF has a
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should be ensuring that only a single object is returned, otherwise we can raise a dev internal error for multiple objects returned or an iterable is returned as a helpful guide. |
||
| raise AuthenticationFailed(_("User not found"), code="user_not_found") | ||
| except self.user_model.DoesNotExist as e: | ||
| raise AuthenticationFailed( | ||
| _("User not found"), code="user_not_found" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's a bit nitty, but would the better method name here be
get_user_object?It would immediately be clear from the name that an object is fetched, and not a queryset, and it would be similar to DRF method: https://www.django-rest-framework.org/api-guide/generic-views/#get_objectself