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.

Sign up for GitHub

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

Jump to bottom

Issues with the api documentation override #14553

Open
Nek- opened this issue Nov 17, 2022 · 3 comments
Open

Issues with the api documentation override #14553

Nek- opened this issue Nov 17, 2022 · 3 comments
Assignees
@NoResponseMate
Labels
Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). Maintenance CI configurations, READMEs, releases, etc.

Comments

@Nek-
Copy link
Contributor

Nek- commented Nov 17, 2022

Problem

Because ApiPlatform uses final classes, you had to override entirely the documentation controller of the API. It's working. But last versions of ApiPlatform use a different way to get the documentation and they changed a lot their own controller.

Basically, their version conflicts with how you manage the documentation. It leads to issues in OpenApi output.

Solution

First, I think you can add your experimental flag to the documentation without overriding the whole controller since ApiPlatform team added a way to change the assets.

To support ApiPlatform 2.7 as well as keep compatibility with 2.5, it seems to be required to have DocumentationNormalizers as well as OpenApiFactory...

To make things better here, we need many changes in the code base, some code duplication and... work.

I wanted to know your feeling about all of this before working on some pull requests to fix this doc-related issue.
@jakubtobiasz jakubtobiasz added Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). Maintenance CI configurations, READMEs, releases, etc. labels Nov 21, 2022
@NoResponseMate
Copy link
Contributor

I'm all for it.
The controller has been overridden ages ago and is horribly out of date so using the supported way of modifying/extending docs would be a great improvement.

@rimas-kudelis
Copy link
Contributor

Should you decide to work on normalizers, I believe you could reuse what I did in #13385. This PR was merged but later reverted in #13459 due to the lack of tests and it being considered buggy, but I remember @lchrusciel telling me it could be brought back if it worked and proper tests were added.

I suppose it may have seemed buggy/not working because API Platform added a api_platform.openapi.backward_compatibility_layer preference at some point which defaults to true and (I think) affects which of the two documentation decoration patterns are used.

@Nek-
Copy link
Contributor Author

Nek- commented Dec 30, 2022

I tried some things to make a PR and the topic is indeed a bit tricky. 😬

@coldic3 coldic3 mentioned this issue Jul 11, 2023
Merged
jakubtobiasz added a commit that referenced this issue Jul 30, 2023
@jakubtobiasz
refactor #15158 [API] Replace Swagger with OpenAPI (coldic3)
97945ca 
This PR was merged into the 1.13 branch.

Discussion
----------

| Q               | A                                                            |
|-----------------|--------------------------------------------------------------|
| Branch?         | 1.13 |
| Bug fix?        | yes                                                       |
| New feature?    | yes                                                       |
| BC breaks?      | no                                                       |
| Deprecations?   | no |
| Related tickets | fixes #14877, #14553 |
| License         | MIT                                                          |

In this PR I do several things:
1. Remove `SwaggerUiAction` that overrides API Platform's one. It was used (if I see correctly) only to add a custom twig template. Thanks to the removal of overwritten action, we use OpenAPI instead of Swagger. I don't know how to override twig template in a bundle, so I do it `templates/bundles`. I think no one uses `ApiBundle` separately ;)
2. Remove normalizers located in `Sylius\Bundle\ApiBundle\Swagger` and decorate `api_platform.openapi.factory`. `DocumentationModifierInterface` do the same what normalizers were doing.

Commits
-------
  [API] Replace Swagger with OpenAPI
GSadee pushed a commit to Sylius/SyliusApiBundle that referenced this issue Aug 11, 2023
@jakubtobiasz
refactor #15158 [API] Replace Swagger with OpenAPI (coldic3)
21154c8 
This PR was merged into the 1.13 branch.

Discussion
----------

| Q               | A                                                            |
|-----------------|--------------------------------------------------------------|
| Branch?         | 1.13 |
| Bug fix?        | yes                                                       |
| New feature?    | yes                                                       |
| BC breaks?      | no                                                       |
| Deprecations?   | no |
| Related tickets | fixes Sylius/Sylius#14877, Sylius/Sylius#14553 |
| License         | MIT                                                          |

In this PR I do several things:
1. Remove `SwaggerUiAction` that overrides API Platform's one. It was used (if I see correctly) only to add a custom twig template. Thanks to the removal of overwritten action, we use OpenAPI instead of Swagger. I don't know how to override twig template in a bundle, so I do it `templates/bundles`. I think no one uses `ApiBundle` separately ;)
2. Remove normalizers located in `Sylius\Bundle\ApiBundle\Swagger` and decorate `api_platform.openapi.factory`. `DocumentationModifierInterface` do the same what normalizers were doing.

Commits
-------
  [API] Replace Swagger with OpenAPI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@NoResponseMate NoResponseMate

Labels
Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). Maintenance CI configurations, READMEs, releases, etc.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@Nek- @rimas-kudelis @NoResponseMate @jakubtobiasz