Fix editable relation on list

[jordisala1991]

I am targeting this branch, because this is BC.

Closes sonata-project/SonataDoctrineORMAdminBundle#818

Changelog

### Fixed
- Fix edit choice with a relation field on admin list

To do

• Update the tests

Subject

getManagerType does not return what was expected, also if this method works it is better since it does not rely on container, but on properties that are already loaded.

This is a bugfix from my previous PR: #4930

Please, can you check if this works @mikemix ?

[mikemix]

@jordisala1991 it does.

[jordisala1991]

@greg0ire wdyt about this? We have a problem with the modelManagerInterface... It does not contain getMetadata method, even though all ModelManager from our persistence bundles implements it.

It was removed here: #736

I don't know any other way to fix the initial issue.

[greg0ire]

Fixing the initial issue would be possible by introducing an interface very much like this one and have all model managers implement it in addition to the original one. None of this would be a BC break. The only issue would be finding a good name.

[jordisala1991]

WDYT about this interface @greg0ire ?

[greg0ire]

I find it weird that it seems to be used only in tests, and I have a bad feeling about this dependency on Doctrine. Maybe it is a necessary evil

[jordisala1991]

the only alternative to me is to add those methods to the modelManager interface on next major, and use the dirty trick I had before on tests. I guess this is not the only place where this happens.

My plan if we accept this new interface is to provide PRs to the persistence bundles to implement this interface.

There is always another alternative, but implies we remove the if check that uses the metadata. If I remove that, I won’t need the interface.

None of those solutions seems perfect to me. To be honest, the dirty trick didnt seem a super bad option, because it gives some space to do it better in the future, introducing a new interface gives more constraints.

[greg0ire]

Yeah I can't think of anything better either. @sonata-project/contributors, should we accept this interface or remove the code that uses it?

[jordisala1991]

I will try what happens with and without that check first

[jordisala1991]

After some thought and testing, I think I found a solution that does not need to use the metadata class.

Can you review again @sonata-project/contributors ?

In this solution we rely on the fieldDescription to compare the targetEntity of the association with the class provided on the admin definition. If the class does not match, the list field is not configured correctly

[greg0ire]

Great solution!

[greg0ire]

Fix editable relation on list

Please amend your commit message to explain what was broken and why your changes fix the issue.

[jordisala1991]

Isn't it better to change the message when squashing on merge?

This could be the message:

`getManagerType` returns `orm` while the entity manager is `doctrine_orm`.
Also this forces us to add doctrine code on the admin-bundle, while it should
not know anything about the persistence layer (that's why we use `doctrine-orm-admin-bundle`)

The choosen solution is to not check on the metadata for the entity, but instead
look at the fieldDescription, because it contains information about the field, like the
`targetEntity` in case of a relation.

This fixed the bug and removes doctrine code from `admin-bundle`

Wdyt? can you add this message on merge?

[greg0ire]

Isn't it better to change the message when squashing on merge?

I'm fine with both solutions. I changed the subject to reflect what you did, as opposed to the side effect it had (fixing #818), and slightly altered the body. Thanks for this fix!

[jordisala1991]

Thank you @greg0ire