sonata-project / SonataDoctrineORMAdminBundle

Integrate Doctrine ORM into the SonataAdminBundle
https://docs.sonata-project.org/projects/SonataDoctrineORMAdminBundle
MIT License
445 stars 344 forks source link

ModelManagerInterface and getParentMetadataForProperty() #1296

Closed dmaicher closed 3 years ago

dmaicher commented 3 years ago

Feature Request

Inside the AbstractTypeGuesser we are calling Sonata\AdminBundle\Model\ModelManagerInterface::getParentMetadataForProperty but this method is not part of that contract.

So we are actually expecting a Sonata\DoctrineORMAdminBundle\Model\ModelManager for it to work.

As @VincentLanglet suggested we could

Or any other ideas?

franmomu commented 3 years ago

I created https://github.com/sonata-project/SonataAdminBundle/issues/6701 for this, but I haven't tried it yet.

VincentLanglet commented 3 years ago

The getParentMetadataForProperty method was also used in the fixDescription method of our builders. @dmaicher you removed it from FormContractor, ListBuilder and ShowBuilder, but you had issues with the tests for the DatagridBuilder. With https://github.com/sonata-project/SonataDoctrineORMAdminBundle/pull/1297/files, it will be definitely removed from the builders.

So indeed, we just have to focus about the TypeGuesser usage.

$this->guesser->guessType($admin->getClass(), $fieldDescription->getName(), $admin->getModelManager())

This could be easily migrated to

$this->guesser->guessType($fieldDescription)

Because as a first step, we can change the code inside guessType to add

$admin = $fieldDescription->getAdmin();
$class = $admin->getClass();
$modelManager = $admin->getModelManager();

And keep the same code.

Note that currently, when guessType is called, we did not attach the admin It's done instead in the fixFieldDescription method: https://github.com/sonata-project/SonataDoctrineORMAdminBundle/blob/3.x/src/Builder/DatagridBuilder.php#L74 So there is something to change about this. We can move the setAdmin call from fixFieldDescription to addFilter.

I also notice that the fixFieldDescription is in every builder, but is only called in the SonataAdmin::FormMapper https://github.com/sonata-project/SonataAdminBundle/search?q=fixFieldDescription ; there is no need to make it public in other builders. A maybe better idea could be to move the addFilter and addField to the SonataAdmin code. When I look at the code I think it just require a BuilderInterface::getGuesser method and a DatagridBuilderInterface::getFilterFactory method.