-
Notifications
You must be signed in to change notification settings - Fork 0
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
IBX-8470: Fixed outstanding issues after Symfony 6 upgrade #130
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ecorator RetryChoiceListFactory decorates CachingFactoryDecorator, which decorates PropertyAccessDecorator
adamwojs
approved these changes
Feb 6, 2025
tbialcz
approved these changes
Feb 6, 2025
|
https://github.com/ibexa/oss/actions/runs/13154896672 - failure seem unrelated behat bundle regarding symfony6 upgrade. |
🟢 light to merge from me. |
konradoboza
approved these changes
Feb 6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
Issue 1:
RetryChoiceListFactory
RetryChoiceListFactory
decoratesform.choice_list_factory
Symfony Forms service, which in fact is an instance of\Symfony\Component\Form\ChoiceList\Factory\CachingFactoryDecorator
decorating\Symfony\Component\Form\ChoiceList\Factory\PropertyAccessDecorator
.The issue was that the method signatures of those classes are not aligned with the implementing interface, with broader scope of
mixed
type instead of?callable
. Not sure if this is the proper direction, howeverchoice_value
configured via\Ibexa\AdminUi\Form\Type\Language\AbstractLanguageChoiceType::configureOptions
is of scalar type, therefore causing a fatal error when trying to pass it as?callable
argument.Instead, I've aligned
RetryChoiceListFactory
method signatures withPropertyAccessDecorator
.This happens only when Behat bundle is enabled, otherwise it performs a pass through
CachingFactoryDecorator
andPropertyAccessDecorator
usingstring 'languageCode'
as 2nd argument ofcreateListFrom*
methods.Should we change
choice_value
to be a callable instead? Seems both types are okay: https://symfony.com/doc/6.4/reference/forms/types/choice.html#choice-valueIssue 2:
UnauthenticatedRedirectController
UnauthenticatedRedirectController
had injected@service_container
directly along withperformAccessCheck
call. The method in the background tried to accesssecurity.authorization_checker
service, which in Symfony 6 is private.Per symfony/symfony#44391 conclusions and recommendations, I've changed the service to be auto-configured (when there's
calls
entry, it's not, despite_defaults
) and I've refactored the controller to rather perform access check on specific action than when building service. It worked by accident due to the fact that the controller was being built on demand, instead of when building the whole container.Kudos to @Steveb-p for the help with this one.