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

Deprecate FormMapper::create #1068

Merged
merged 1 commit into from
May 22, 2022

Conversation

VincentLanglet
Copy link
Member

Subject

I am targeting this branch, because {reason}.

See sonata-project/SonataAdminBundle#7265 (comment)

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
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member Author

@jordisala1991, is it ok for you ?

@jordisala1991
Copy link
Member

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.

@VincentLanglet VincentLanglet merged commit 66b7ca5 into sonata-project:4.x May 22, 2022
@VincentLanglet VincentLanglet deleted the formMapperCreate branch May 22, 2022 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants