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

Suggestion: Add request and response classes #139

Closed
Goldziher opened this issue Jul 31, 2020 · 2 comments
Closed

Suggestion: Add request and response classes #139

Goldziher opened this issue Jul 31, 2020 · 2 comments
Labels
enhancement New feature or request

Comments

@Goldziher
Copy link

Goldziher commented Jul 31, 2020

Hi there,

First thanks for this nice package - it works quite well and is overall very good. One issue I repeatedly need to work around though is the relying on serializers for documenting responses and requests. In a real life project quite often you do not have serializers for all of your views. In these cases I find I have to add quite a lot of code redundancy just to make sure that the api schema appears correctly. Here is an example:

class OpenIDInitView(BaseOpenIdView):
    """
        A view that returns a url for OpenID connection login / registration
        This is the first stage of the OIC "authorization flow" process
    """

    def get(self, request, *args, **kwargs):
        client = self.get_client()
        return Response(
            data={
                "login_url": client.create_login_url()
            },
            status=HTTP_200_OK,
        )

As you can see there is no serializer involved here - I use a service layer and some libraries to generate this url, and then just return this object. Obviously I can create a serializer but the only reason to do it is to make the api schema work, which is sort of silly and redundant. Nonetheless this is what I find myself doing:

class OpenIDInitView(BaseOpenIdView):
    """
        A view that returns a url for OpenID connection login / registration
        This is the first stage of the OIC "authorization flow" process
    """

   @extend_schema(
        responses={
            200: inline_serializer(
                {"login_url": CharField()}, name="OpenIDInitSerializer"
            )
        }
    )
    def get(self, request, *args, **kwargs):
        client = self.get_client()
        return Response(
            data={
                "login_url": client.create_login_url()
            },
            status=HTTP_200_OK,
        )

The code for the inline_serializer helper that is used in the above is this:

from rest_framework.serializers import Serializer

def inline_serializer(fields: dict, **kwargs) -> Serializer:
    """
        A helper function to create an inline serializer
        :param fields: dict of serializer fields
        :param kwargs: optional serializer kwargs
        :return Serializer
    """
    serializer_class = type(kwargs.pop("name", ""), (Serializer,), fields)
    return serializer_class(**kwargs)

So my suggestion is to create two classes "Request" and "Response" that can be used to directly type these where a serializer is not used. These will not rely on the serializer fields from DRF but rather on the openapi types from the package.

@tfranzel tfranzel added the enhancement New feature or request label Aug 6, 2020
@tfranzel
Copy link
Owner

tfranzel commented Aug 6, 2020

hi @Goldziher. actually that happened to me too a few times. i like the idea. will add it to my backlog. thanks

tfranzel added a commit that referenced this issue Aug 26, 2020
@tfranzel
Copy link
Owner

@Goldziher thanks for the excellent proposal. i pretty much used your implementation except that i made the name mandatory because it is required.

can you close the issue if you are happy with it?

@tfranzel tfranzel closed this as completed Sep 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants