Skip to content

fix: multiple formats for custom "api_doc" route example#2122

Merged
soyuka merged 1 commit intoapi-platform:3.4from
phansys:api_doc_formats
Feb 12, 2025
Merged

fix: multiple formats for custom "api_doc" route example#2122
soyuka merged 1 commit intoapi-platform:3.4from
phansys:api_doc_formats

Conversation

@phansys
Copy link
Contributor

@phansys phansys commented Jan 8, 2025

Without the _format option, the resulting route is not able to expose other paths like /api_documentation.jsonopenapi.

alanpoulain
alanpoulain reviewed Jan 9, 2025
View reviewed changes
core/openapi.md
Comment on lines +458 to +519
methods: ['HEAD', 'GET']
defaults:
_format: html
Copy link
Member

@alanpoulain alanpoulain Jan 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these necessary as well?

Copy link
Contributor Author

@phansys phansys Jan 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so. Without the methods definition, the route will accept any HTTP verb.

This condition was added in version 3.3.7.

The default format is required for the routing to match the /api_documentation.{_format} pattern. Otherwise, a request to the /api_documentation path results in 404, as the pseudo-extension (.html in this case) is missing.

@phansys phansys requested a review from alanpoulain January 9, 2025 14:31
@soyuka
Copy link
Member

soyuka commented Jan 13, 2025

shouldn't we merge this on a 3.x branch?

@phansys
Copy link
Contributor Author

phansys commented Jan 13, 2025 •
edited
Loading

shouldn't we merge this on a 3.x branch?

I've chosen the 2.7 branch, as this branch is the older one where the route is defined. The version 3.3.7 was the first one introducing these improvements / fixes in the default route definition, although I guess this situation is not a blocker to provide these changes from the branch where the route was introduced.

Please, let me know if you agree. Otherwise, I can update the target branch to 3.7 based on your answer.
Thank you.

@vinceAmstoutz
Copy link
Member

vinceAmstoutz commented Jan 14, 2025

shouldn't we merge this on a 3.x branch?

I've chosen the 2.7 branch, as this branch is the older one where the route is defined. The version 3.3.7 was the first one introducing these improvements / fixes in the default route definition, although I guess this situation is not a blocker to provide these changes from the branch where the route was introduced.

Please, let me know if you agree. Otherwise, I can update the target branch to 3.7 based on your answer. Thank you.

@phansys Yes, you're right but we currently support >= 3.3 branches. Please target the 3.4 branch for this one. For more details check out our Maintenance documentation.

@phansys phansys changed the base branch from 2.7 to 3.4 January 14, 2025 12:02
@vinceAmstoutz vinceAmstoutz requested a review from soyuka January 14, 2025 13:48
@phansys
Copy link
Contributor Author

phansys commented Jan 18, 2025

The target branch was changed to 3.4. Thanks.

@soyuka soyuka merged commit 607c080 into api-platform:3.4 Feb 12, 2025
1 check passed
@soyuka
Copy link
Member

soyuka commented Feb 12, 2025

thanks!

@phansys phansys deleted the api_doc_formats branch February 12, 2025 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alanpoulain alanpoulain Awaiting requested review from alanpoulain

@soyuka soyuka Awaiting requested review from soyuka

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@phansys @soyuka @vinceAmstoutz @alanpoulain