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

fix: class_alias with preloading #6293

Open
wants to merge 2 commits into
base: 2.7
Choose a base branch
from

Conversation

sylfabre
Copy link

@sylfabre sylfabre commented Apr 5, 2024

Q A
Branch? 2.7
Tickets Closes similar issue to https://github.com/api-platform/core/pull/5523/files
License MIT
Doc PR N/A

https://github.com/api-platform/core/blob/2.7/src/Symfony/Validator/EventListener/ValidationExceptionListener.php suffers from the same issue as https://github.com/api-platform/core/pull/5523/files

=> This PR wraps all calls to class_alias() in an if condition to check first if the alias already exists

This is important to us because we want to upgrade first to 2.7 before moving forward to 3.0: we cannot do it right now because preloading breaks with 2.7

This PR follows up on #6217 which was a disaster. This new version has been tested internally within my company and we plan to release it on week 15 (April 8th, 2024)

* Make sure class aliases are not defined twice when preloading is used
@sylfabre
Copy link
Author

sylfabre commented Apr 5, 2024

Hello @dpawelec @markom-werbeagentur @ywarnier

Here is a new version that passed tests with preloading and classloading.

Would you please give it a try before asking @soyuka or @dunglas?
Thanks a lot

@sylfabre
Copy link
Author

FYI, we have been using our fork in production for about a week without any bug report

@sylfabre sylfabre changed the title [DRAFT - DO NOT MERGE] fix: class_alias with preloading fix: class_alias with preloading Apr 13, 2024
@markom-werbeagentur
Copy link

Seems to work for me, thank you very much for your work.

@soyuka
Copy link
Member

soyuka commented Apr 15, 2024

should I merge and tag this then @sylfabre ?

@sylfabre
Copy link
Author

Hello @soyuka

So far so good here at AssoConnect.
So I think you can merge safely.

Waiting a bit more for more feedbacks is also a viable option

Up to you!

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.

None yet

3 participants