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

IBX-8470: Fixed outstanding issues after Symfony 6 upgrade #130

Merged
merged 16 commits into from
Feb 7, 2025

Conversation

tomaszszopinski
Copy link
Contributor

@tomaszszopinski tomaszszopinski commented Feb 5, 2025

🎫 Issue IBX-8470

Description:

Issue 1: RetryChoiceListFactory

RetryChoiceListFactory decorates form.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, however choice_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 with PropertyAccessDecorator.

This happens only when Behat bundle is enabled, otherwise it performs a pass through CachingFactoryDecorator and PropertyAccessDecorator using string 'languageCode' as 2nd argument of createListFrom* 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-value

Issue 2: UnauthenticatedRedirectController

UnauthenticatedRedirectController had injected @service_container directly along with performAccessCheck call. The method in the background tried to access security.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.

@alongosz alongosz changed the title Test after symfony 6 upgrade IBX-8470: Fixed outstanding issues after Symfony 6 upgrade Feb 5, 2025
@alongosz alongosz requested a review from a team February 5, 2025 13:11
Copy link

sonarqubecloud bot commented Feb 6, 2025

@tomaszszopinski
Copy link
Contributor Author

tomaszszopinski commented Feb 6, 2025

https://github.com/ibexa/oss/actions/runs/13154896672 - failure seem unrelated behat bundle regarding symfony6 upgrade.

@tomaszszopinski
Copy link
Contributor Author

🟢 light to merge from me.

@adamwojs adamwojs merged commit cec9ad5 into main Feb 7, 2025
11 of 12 checks passed
@adamwojs adamwojs deleted the symfony6-upgrade-test branch February 7, 2025 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants