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

feat: add is_list option to view action method #331

Closed
wants to merge 4 commits into from

Conversation

codetalks-new
Copy link

I struggled a day try to add drf-yasg for a exists project.
First I found drf-yasg generate unintented schema.
our TopicViewSet as follows:( one Topic have many Posts)

class TopicViewSet(viewsets.ModelViewSet):
    queryset = Topic.objects.all()
    serializer_class = TopicSerializer
    permission_classes = (DjangoModelPermissionsOrAnonReadOnly,)

    @action(detail=True)
    def posts(self, request:HttpRequest, pk) -> Response:
        pits = PostInTopic.objects.filter(topic_id=pk).all()
        page = self.paginate_queryset(pits)
        if page is not None:
            serializer = PostInTopicSerializer(page, many=True)
            return self.get_paginated_response(serializer.data)

        serializer = PostInTopicSerializer(pits, many=True)
        return Response(serializer.data)

in order to make drf-yasg generate right schema for posts actions. I have debug a lot and read drf-yasg's source code.
and I found:

  1. detail=True can't set to False because We need the pk parameter.
  2. if detail=True then we can't return paginated response schema.

My finally code was as below:

class TopicViewSet(viewsets.ModelViewSet):
    queryset = Topic.objects.all()
    serializer_class = TopicSerializer
    permission_classes = (DjangoModelPermissionsOrAnonReadOnly,)

    @action(detail=True)
    def posts(self, request: HttpRequest, pk: int) -> Response:
        pits = PostInTopic.objects.filter(topic_id=pk).all()
        page = self.paginate_queryset(pits)
        if page is not None:
            serializer = self.get_serializer(page, many=True)
            return self.get_paginated_response(serializer.data)

        serializer = self.get_serializer(pits, many=True)
        return Response(serializer.data)

    posts.is_list = True # we do return list response

    def get_serializer_class(self):
        #  drf_yasf  will call get_serializer()
        if self.action == "posts":
            return PostInTopicSerializer
        return super().get_serializer_class()

and I have to midify the drf-yasg source code, I think it would be very useful to add optional is_list flag for view action method.

I've checked our project,there are many places, I need to use is_list option to make drf-yasg work correctly.

@axnsan12
Copy link
Owner

Hello! Thanks for the pull request and ideas.

The solution proposed here is really inconsistent with the rest of the drf-yasg API. All other overrides are done via decorators, so there's no reason we should start doing them by method attributes now.

You can solve the paginator problem by selecting the paginator class in the paginator method instead of the paginate_queryset method. Both DRF and drf-yasg will then see the same paginator class and behave in the same way.

For the is_list problem, I would much rather add an overridable method in SwaggerAutoSchema for maximum flexibility. The presented use case is fairly uncommon and I don't believe it warrants a declarative way to override.

@axnsan12
Copy link
Owner

The proposed hooks should suffice to satisfy this need.

@axnsan12 axnsan12 closed this Mar 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants