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(openapi): make open_api_override_responses act on default 404 response generation #6551

Merged
merged 1 commit into from
Aug 30, 2024

Conversation

monitaurus
Copy link
Contributor

@monitaurus monitaurus commented Aug 28, 2024

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.

@monitaurus monitaurus marked this pull request as ready for review August 28, 2024 10:50
@soyuka
Copy link
Member

soyuka commented Aug 29, 2024

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 monitaurus changed the base branch from 3.3 to 3.4 August 30, 2024 06:07
@monitaurus
Copy link
Contributor Author

monitaurus commented Aug 30, 2024

@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 soyuka merged commit db1241c into api-platform:3.4 Aug 30, 2024
73 of 77 checks passed
@soyuka
Copy link
Member

soyuka commented Aug 30, 2024

Thanks!

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.

2 participants