drf-yasg
drf-yasg copied to clipboard
Wrong handeling of combination "@swagger_auto_schema(... responses={... XSerializer(many=True)})" and "@action(detail=True, ...)"
Bug Report
Description
In our project, I was trying to write the definition of a new API endpoint that combines Serializer(many=True)
and detail=True
.
I was able to write the expected definition for drf_spectacular
but drf_yasg
has a different result.
Response for drf_spectacular
{
"count": 123,
"next": "http://api.example.org/accounts/?offset=400&limit=100",
"previous": "http://api.example.org/accounts/?offset=200&limit=100",
"results": [
{
"model": "string",
"id": 0,
"name": "string"
}
]
}
Response for drf_yasg
[
{
"model": "string",
"id": 0,
"name": "string"
}
]
Is it a bug or did I miss something in the documentation?
Is this a regression?
I don't know, I'm writing a new implementation
Minimal Reproduction
class DeletePreviewModelMixin:
@extend_schema(
methods=['GET'],
responses={status.HTTP_200_OK: serializers.DeletePreviewSerializer(many=True)}
)
@swagger_auto_schema(
method='get',
responses={status.HTTP_200_OK: serializers.DeletePreviewSerializer(many=True)}
)
@action(detail=True, methods=["get"], filter_backends=[])
def delete_preview(self, request, pk=None):
...
...
class DeletePreviewSerializer(serializers.Serializer):
model = serializers.CharField(read_only=True)
id = serializers.IntegerField(read_only=True, allow_null=True)
name = serializers.CharField(read_only=True)
Your Environment
Django==3.2.12
djangorestframework==3.13.1
django-filter==21.1
drf_yasg==1.20.0
Full context
https://github.com/DefectDojo/django-DefectDojo/pull/5612
-
detail=True
only determines where the endpoint is mounted. irrelevant for the endpoint schema itself. - spectacular response suggests there is a
pagination_class
present on that view/action. The pagination logic gets activated by doingSerializer(many=True)
. - we make no guarantees about correctness when using both
spectacular
andyasg
decorators at the same time. your mileage may vary.
TL;DR:
Check 'default'
and suffix
@swagger_auto_schema(
method='get',
responses={'default': serializers.DeletePreviewSerializer(many=True)}
)
@action(detail=True, methods=["get"], filter_backends=[], suffix='List')
Long version
Hi @tfranzel,
- I disagree with this statement:
detail=True
only determines where the endpoint is mounted. irrelevant for the endpoint schema itself. (please check the analysis below) - I can agree with
pagination_class
- It is a little bit sad but Ok.
Analysis
I did a quite deep dive. I was trying to find out, why yasg
doesn't evaluate my definition as I expect.
- I knew that response is already array/list. Originally, I thought that behavior is different because the condition of these lines is
False
https://github.com/axnsan12/drf-yasg/blob/d9700dbf8cc80d725a7db485c2d4446c19c29840/src/drf_yasg/inspectors/view.py#L216-L217 Actually, I found out, that these lines are not even executed. Moreover, the whole functionget_default_responses
is not executed. - So I started to debug
get_response_serializers
(which callsget_default_responses
). I found out thatget_default_responses
is not called because I usedstatus.HTTP_200_OK
inresponses
(which is one of the "success responses"). https://github.com/axnsan12/drf-yasg/blob/d9700dbf8cc80d725a7db485c2d4446c19c29840/src/drf_yasg/inspectors/view.py#L230-L235 So I used'default'
as HTTP response code. - Perfect,
get_default_responses
is executed right now. But my feeling from point 1. is right,-
self.should_page
is returningFalse
because it callsself.has_list_response
and it is returningFalse
as well. -
self.has_list_response
is returningFalse
because it callsself.is_list_view
and it is returningFalse
as well. -
self.is_list_view
is returningFalse
because it callsutils.is_list_view
and it is returningFalse
as well.
-
- Now, when we are deep enough, let's find the real reason why
https://github.com/axnsan12/drf-yasg/blob/d9700dbf8cc80d725a7db485c2d4446c19c29840/src/drf_yasg/utils.py#L219-L229
This implementation expects that if the user defines
@action(default=True)
, the result shouldn't be listed. 😢
a detail action is surely not a list route
So, because I'm not able to change the action
name and detail
has to be True
, I decided to use a "dirty hack" and set suffix = 'List'
. Thankfully there is not any side effect.
@kiblik I can't comment on the other things but detail
works 100% like I said, trust me :smiley:
/x/ # list
/x/{id}/ # retrieve
/x/action # action with detail=False
/x/{id}/action # action with detail=True
DRF does not specify what comes out of the action endpoint, i.e. if it is a list or not. Spectacular defaults to non-list with actions and you need to provide many=True
if you want your action to be a list.