Skip to content

fix(symfony): fix swagger config to support Symfony ConfigBuilders #4691

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

Merged

Conversation

alexndlm
Copy link
Contributor

@alexndlm alexndlm commented Mar 26, 2022

@stale
Copy link

stale bot commented Nov 4, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Nov 4, 2022
@alexndlm
Copy link
Contributor Author

alexndlm commented Nov 5, 2022

@soyuka would you please check this PR?

@stale stale bot removed the wontfix label Nov 5, 2022
@alexndlm
Copy link
Contributor Author

@soyuka, is any chance to merge this?

@soyuka
Copy link
Member

soyuka commented Dec 17, 2022

@vincentchalamon probably has a better idea then me of the impacts of this

@vincentchalamon
Copy link
Contributor

Hi @alexndlm,

Thanks for your PR. I've tested it in an API Platform project, but can't find any difference in the generated values comparing with the actual code, both dump me the following configuration:

array:1 [
  "foo" => array:2 [
    "name" => "Authorization"
    "type" => "header"
  ]
]

Can you explain why the use of useAttributeAsKey is necessary, and if possible add a test using it, please?

@alexndlm alexndlm force-pushed the fix/symfony-swagger-api-keys-config branch from bb88c52 to 5053f69 Compare December 19, 2022 10:27
@alexndlm
Copy link
Contributor Author

@vincentchalamon, thanks that you check this PR.

I have updated the tests and rebased PR on 2.7.

This issue is only present with Symfony PHP ConfigBuilders.

Now Symfony generates the following Config Class:
Снимок экрана 2022-12-19 в 11 17 08

My API Platform config:
Снимок экрана 2022-12-19 в 11 20 34

And it is not possible to specify the authorization name:
Снимок экрана 2022-12-19 в 11 18 20

With the fix in this PR, it will be the following:
Снимок экрана 2022-12-19 в 11 24 14

Снимок экрана 2022-12-19 в 11 23 12

Снимок экрана 2022-12-19 в 11 25 41

@alexndlm alexndlm changed the base branch from main to 2.7 December 19, 2022 10:27
@vincentchalamon
Copy link
Contributor

Thanks for the clear clarification 😃
Can you add a test with ConfigBuilder to reveal the bug and fix it, please?

@alexndlm alexndlm force-pushed the fix/symfony-swagger-api-keys-config branch from 036f83f to 955b948 Compare December 19, 2022 12:34
@alexndlm alexndlm changed the base branch from 2.7 to main December 19, 2022 12:35
@alexndlm
Copy link
Contributor Author

Done.

i-did-it-ygab1a

Copy link
Contributor

@vincentchalamon vincentchalamon left a comment

Choose a reason for hiding this comment

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

Bad commitlint (easy to fix), otherwise 👍
Good job @alexndlm!

@vincentchalamon vincentchalamon merged commit 3a845f1 into api-platform:main Dec 27, 2022
@vincentchalamon
Copy link
Contributor

Good job @alexndlm, 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.

3 participants