api-platform / core

The server component of API Platform: hypermedia and GraphQL APIs in minutes
https://api-platform.com
MIT License
2.45k stars 877 forks source link

feat(openapi): make open_api_override_responses act on default 404 response generation #6551

Closed monitaurus closed 2 months ago

monitaurus commented 2 months ago
Q A
Branch? 3.4
Tickets
License MIT
Doc PR

6221 introduces options open_api_override_responses to toggle default responses generation.

This fix include 404 response generation into open_api_override_responses reach.

soyuka commented 2 months ago

feature should target 3.4, I think that we should also consider renaming this option to addDefaultResponses as suggest in the related issue, but then it's weird to add a default response although the user specified not to?

monitaurus commented 2 months ago

@soyuka

feature should target 3.4

Done, target is now 3.4

I think that we should also consider renaming this option to addDefaultResponses as suggest in the related issue

Sure, I can change the name, but wouldn't it be a breaking change for those who use this already?

But, as this is an option to add on the operation level, I fear that addDefaultResponses may be ambigous by not mentioning OpenAPI. Something like addDefaultOpenApiResponses, or generateOpenApiDefaultResponses may be more self explainatory.

but then it's weird to add a default response although the user specified not to?

Sorry I didn't understand the question 😅

In the case I faced, I've got a DELETE endpoint that for some reason don't generates 404. So i tried to use the open_api_override_responses to prevent the 404 OpenAPI generation.

I was thinking that open_api_override_responses generates default responses such as: METHOD Status
GET 200, 404
POST 201, 400, 422
PATCH, PUT 200, 400, 404, 422
DELETE 204, 404

But found out that 404 is handle in a different block of code. Hence the PR to be consistent with to toggle default response as a whole or not at all.

soyuka commented 2 months ago

Thanks!