Skip to content
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

Added ability for username to act as identifier for API user detail endpoint in addition to primary key (/api/users/<userID> OR /api/users/<username>) #321

Merged
merged 9 commits into from
Dec 4, 2023
2 changes: 1 addition & 1 deletion src/chigame/api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
path("lobbies/<int:pk>/", views.LobbyDetailView.as_view(), name="api-lobby-detail"),
# USER API URLS
path("users/", views.UserListView.as_view(), name="api-user-list"),
path("users/<int:pk>/", views.UserDetailView.as_view(), name="api-user-detail"),
path("users/<slug:slug>/", views.UserDetailView.as_view(), name="api-user-detail"),
path("users/<int:pk>/friends/", views.UserFriendsAPIView.as_view(), name="api-user-friends"),
# CHAT API URLS
path("tournaments/chat/", views.MessageView.as_view(), name="api-chat-list"),
Expand Down
14 changes: 13 additions & 1 deletion src/chigame/api/views.py
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if a user goes by the name "1234"? That could cause an issue with searching for the correct user. A better implementation could have you search through user name first and store usernames as "User#user_id" That way you can query id numbers by using the #, and we can exclude # from potential username characters.

Copy link
Contributor Author

@abrahmasandra abrahmasandra Nov 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just made a commit that validates the User's username, and ensures that it cannot be fully numeric. This way, we will not have this problem, as a given URL parameter can only be interpreted as either a username OR an id but not BOTH.

NOTE: This commit is in PR #291, because in that PR, I created the username field for the User model, so it is only consistent that I put restrictions on the username field in that PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You must reach out to the users team before I will approve any changes to the User model.

Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# from django.shortcuts import render
from django.shortcuts import get_object_or_404
from django_filters.rest_framework import DjangoFilterBackend
from rest_framework import generics, status
from rest_framework.pagination import PageNumberPagination
Expand Down Expand Up @@ -35,7 +36,7 @@ class UserFriendsAPIView(generics.RetrieveAPIView):

def get_queryset(self):
user_id = self.kwargs["pk"]
user_profile = UserProfile.objects.get(user=user_id)
user_profile = get_object_or_404(UserProfile, user=user_id)
return user_profile.friends.all()


Expand All @@ -60,6 +61,17 @@ class UserListView(generics.ListCreateAPIView):
class UserDetailView(generics.RetrieveUpdateDestroyAPIView):
queryset = User.objects.all()
serializer_class = UserSerializer
lookup_field = "slug"

def get_object(self):
lookup_value = self.kwargs.get(self.lookup_field)

# If the lookup_value is an integer, use the id field
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good code comment!

if lookup_value.isdigit():
return get_object_or_404(User, pk=lookup_value)
else:
# Otherwise, use the slug field
return get_object_or_404(User, username=lookup_value)


class MessageView(generics.CreateAPIView):
Expand Down