URL lookup not working with update and retrieve-CodePudding

In the url /users/*username I should be able to access each user by its username, the problem is that it isn't working how it's supposed to, doesn't matter what I write in *username it will update/delete the current logged user.

Example: I could update the user "admin", through /users/123/, even though 123 is clearly not admin or any other existing user.

Here it's the User View with the methods that are not working the way I intended:

class UserViewSet(mixins.RetrieveModelMixin,
                  mixins.UpdateModelMixin,
                  mixins.DestroyModelMixin,
                  viewsets.GenericViewSet):

    serializer_class = UserSerializer
    lookup_field = 'username'

    queryset = User.objects.all()

    def get_permissions(self):
        if self.action in ['signup', 'login',]:
            permissions = [AllowAny]
        elif self.action in ['retrieve', 'update', 'destroy']:
            permissions = [IsAuthenticated, IsSameUser]
        else:
            permissions = [IsAuthenticated]
        return [p() for p in permissions]


    def retrieve(self, request, *args, **kwargs):
        response = super(UserViewSet, self).retrieve(request, *args, **kwargs)
        data = response.data
        return Response(data=data, status=status.HTTP_200_OK)

    def update(self, request, *args, **kwargs):
        serializer = UserModelSerializer(request.user, data=request.data, partial=True)
        serializer.is_valid(raise_exception=True)
        serializer.save()
        return Response(serializer.data, status=status.HTTP_200_OK)

Users URL:

""" Users URLs """

# Django
from django.urls import include, path
# Django REST Framework
from rest_framework.routers import DefaultRouter
# Views
from users.views import UserViewSet

router = DefaultRouter()
router.register(r'users/', UserViewSet, basename='users')

urlpatterns = [
    path('', include(router.urls)),
]

CodePudding user response：

it will update/delete the current logged user.

Of course

Your update method is:

def update(self, request, *args, **kwargs):
    serializer = UserModelSerializer(request.user, data=request.data, partial=True)
    serializer.is_valid(raise_exception=True)
    serializer.save()
    return Response(serializer.data, status=status.HTTP_200_OK)

where request.user is logged in user.

Correct code is something like:

class UserViewSet(mixins.RetrieveModelMixin,
                  mixins.UpdateModelMixin,
                  mixins.DestroyModelMixin,
                  viewsets.GenericViewSet):

    serializer_class = UserSerializer
    lookup_field = 'username'

    queryset = User.objects.all()

    ...
    
    def update(self, url_username, request, *args, **kwargs):
        try:
            user_object = self.queryset.get(username=url_username)
        except User.DoesNotExist:
             Response({'error': 'not found'}, status=404)
        serializer = UserModelSerializer(user_object, data=request.data, partial=True)
        serializer.is_valid(raise_exception=True)
        serializer.save()
        return Response(serializer.data, status=status.HTTP_200_OK)

Also you need to add url_username in URL path:

urlpatterns = [
    ...,
    path('users/<str:url_username>', update_user)
]

Here

user_object is user that you want to update
request.data is a new data that you want to set to existing user
url_username is a *username part in /users/*username to get exact user

Update 1: answer on question →

I'm using DRF default router (updated the post and added what my urls.py looks like), the URL is generated taking the actions from the viewset, should I make all the URL like you did there? Like it says in the documentation all the URL patterns should be done automatically.

Oohhh... Django has million ways to implement API methods. And it is very complicated to reproduce behaviour inside my head according one piece of the code. The key of my answer is that you should use username placed in the url instead of request.user. Because request.user is logged-in user object (user that doing request).

At first, try to understand UpdateModelMixin

This Mixin provides correct and standard update() method.
You trying to add standard update() method by including mixins.UpdateModelMixin and after that creating (overriding) own update(). So include UpdateModelMixin is useless.

If you want to use standard update method, just delete own def update(): at all. And that may be solution.

If you want to create custom update method, remove UpdateModelMixin. It is unnecessary.

In this case try change update arguments like:

def update(self, request, pk, *args, **kwargs):
    try:
        user_object = self.queryset.get(username=pk)
    ...

where pk will be username in your case (because you set lookup_field = 'username')

As I said before it is hard to predict Django behaviour (actually I don't remember in details how works pk). Just try and see what happens.

If pk working as I expecting, you don't need to edit urlpatterns.

PS def retrieve(self, request, *args, **kwargs): also unnecessary if you including RetreiveModelMixin. RetreiveModelMixin adding retrieve() method for you.

CodePudding user response：

in your UserSerializer serializer, you should add this meta class:

    class Meta:
        lookup_field = 'username'