sonata-project / SonataBlockBundle

Symfony SonataBlockBundle
https://docs.sonata-project.org/projects/SonataBlockBundle
MIT License
413 stars 142 forks source link

Deprecate FormMapper::create #1068

Closed VincentLanglet closed 2 years ago

VincentLanglet commented 2 years ago

Subject

I am targeting this branch, because {reason}.

See https://github.com/sonata-project/SonataAdminBundle/pull/7265#issuecomment-1125092684

The only usage found is https://github.com/sonata-project/SonataClassificationBundle/blob/4.x/src/Block/Service/AbstractClassificationBlockService.php#L68

This can be replaced by the implementation

$formBuilder->create(...);

Changelog

### Deprecated
- FormMapper::create() method.
jordisala1991 commented 2 years ago

But the $formBuilder is not on that function of classification bundle. Can you show how to avoid deprecation / be compatible with blockbundle 5 on classification 4?

VincentLanglet commented 2 years ago

But the $formBuilder is not on that function of classification bundle. Can you show how to avoid deprecation / be compatible with blockbundle 5 on classification 4?

Implementation is here: https://github.com/sonata-project/SonataAdminBundle/blob/4.x/src/Form/FormMapper.php#L194 Since SonataAdmin will be compatible both blockBundle 4 and 5, the method won't be removed until SonataAdmin 5. So ClassificationBundle can still call the method.

Then, for ClassificationBundle to be compatible SonataAdmin 4 and 5, it will be:

    public function __construct(
        Environment $twig,
        ContextManagerInterface $contextManager, 
        ?FormBuilderInterface $formBuilder = null
) {
        parent::__construct($twig);

        $this->contextManager = $contextManager;
        $this->formBuilder = $formBuilder;
    }

    final protected function getFormAdminType(FormMapper $formMapper, AdminInterface $admin, string $formField, string $field, array $fieldOptions = [], array $adminOptions = []): FormBuilderInterface
    {
        /** @phpstan-var FieldDescriptionOptions $adminOptions */
        $adminOptions = array_merge([
            'edit' => 'list',
            'translation_domain' => 'SonataClassificationBundle',
        ], $adminOptions);

        $fieldDescription = $admin->createFieldDescription($field, $adminOptions);
        $fieldDescription->setAssociationAdmin($admin);

        $fieldOptions = array_merge([
            'sonata_field_description' => $fieldDescription,
            'class' => $admin->getClass(),
            'model_manager' => $admin->getModelManager(),
            'required' => false,
        ], $fieldOptions);

        if (null !== $this->formBuilder) {
            return $this->formBuilder->create($formField, ModelListType::class, $fieldOptions);
        } elseif (method_exists($formMapper, 'create') {
            return $formMapper->create($formField, ModelListType::class, $fieldOptions);
        } else {
            throw new \Exception('You need to inject formBuilder for Sonata 5 support');
        }
    }

But we could also look for getFormAdminType usage and see if we shouldn't deprecate this method.

VincentLanglet commented 2 years ago

@jordisala1991, is it ok for you ?

jordisala1991 commented 2 years ago

But the $formBuilder is not on that function of classification bundle. Can you show how to avoid deprecation / be compatible with blockbundle 5 on classification 4?

Implementation is here: https://github.com/sonata-project/SonataAdminBundle/blob/4.x/src/Form/FormMapper.php#L194

Since SonataAdmin will be compatible both blockBundle 4 and 5, the method won't be removed until SonataAdmin 5.

So ClassificationBundle can still call the method.

Then, for ClassificationBundle to be compatible SonataAdmin 4 and 5, it will be:


    public function __construct(

        Environment $twig,

        ContextManagerInterface $contextManager, 

        ?FormBuilderInterface $formBuilder = null

) {

        parent::__construct($twig);

        $this->contextManager = $contextManager;

        $this->formBuilder = $formBuilder;

    }

    final protected function getFormAdminType(FormMapper $formMapper, AdminInterface $admin, string $formField, string $field, array $fieldOptions = [], array $adminOptions = []): FormBuilderInterface

    {

        /** @phpstan-var FieldDescriptionOptions $adminOptions */

        $adminOptions = array_merge([

            'edit' => 'list',

            'translation_domain' => 'SonataClassificationBundle',

        ], $adminOptions);

        $fieldDescription = $admin->createFieldDescription($field, $adminOptions);

        $fieldDescription->setAssociationAdmin($admin);

        $fieldOptions = array_merge([

            'sonata_field_description' => $fieldDescription,

            'class' => $admin->getClass(),

            'model_manager' => $admin->getModelManager(),

            'required' => false,

        ], $fieldOptions);

        if (null !== $this->formBuilder) {

            return $this->formBuilder->create($formField, ModelListType::class, $fieldOptions);

        } elseif (method_exists($formMapper, 'create') {

            return $formMapper->create($formField, ModelListType::class, $fieldOptions);

        } else {

            throw new \Exception('You need to inject formBuilder for Sonata 5 support');

        }

    }

But we could also look for getFormAdminType usage and see if we shouldn't deprecate this method.

With your changes , it might be possible that $formMapper is no longer need as first argument of that function.