merged branch Burgov/fix_entity_choice_list (PR #2942)

Commits
-------

c60f036 fixed typo
231e79c fixed entity choice list BC break

Discussion
----------

fixed EntityChoiceList BC break

Bug fix: yes
Feature addition: no
Backwards compatibility break: fixes a BC break
Symfony2 tests pass: yes

This PR resolves a serious BC break introduced in commit b919d92
Prior to this commit, it was possible to use the entity shorthand notation in the EntityChoiceList constructor, but it broke because the EntityChoiceList now expects the second argument to be the actual class name

There is another issue at hand here, but I'm not sure how to fix it:

The EntityChoiceManager expects an Doctrine\Common\Persistence\ObjectManager instance, then the ClassMetadata is fetched from it and the method getIdentifierFieldNames is called on it. Yet, according to the docblock, getClassMetadata of the ObjectManager returns an instance of Doctrine\Common\Persistence\Mapping\ClassMetadata, which doesn't have a getIdentifierFieldNames() method.

So either the EntityChoiceList should expect an instance of EntityManager, or it should be rewritten to not use getIdentifierFieldNames() anymore.

Any ideas?

---------------------------------------------------------------------------

by fabpot at 2011/12/22 03:48:49 -0800

ping @beberlei

---------------------------------------------------------------------------

by beberlei at 2011/12/22 04:02:30 -0800

The fix is valid

---------------------------------------------------------------------------

by stof at 2011/12/22 04:54:53 -0800

@beberlei getIdentifierFieldNames should probably be added in the interface too. Currently, we only have isIdentifier in it.

This methods needs to be implemented in the MongoDB ODM though (simply returning ``array($this->getIdentifier())``

src/Symfony/Bridge/Doctrine/Form/ChoiceList/EntityChoiceList.php

@@ -103,7 +103,7 @@ public function __construct(ObjectManager $manager, $class, $property = null, En
         // displaying entities as strings
         if ($property) {
             $this->propertyPath = new PropertyPath($property);
-        } elseif (!method_exists($this->class, '__toString')) {
+        } elseif (!method_exists($this->classMetadata->getName(), '__toString')) {
             // Otherwise expect a __toString() method in the entity
             throw new FormException('Entities passed to the choice field must have a "__toString()" method defined (or you can also override the "property" option).');
         }

tests/Symfony/Tests/Bridge/Doctrine/Form/ChoiceList/EntityChoiceListTest.php

@@ -196,4 +196,24 @@ public function testGroupByInvalidPropertyPathReturnsFlatChoices()
             2 => 'Bar'
         ), $choiceList->getChoices('choices'));
     }
+
+    public function testPossibleToProvideShorthandEntityName()
+    {
+        $shorthandName = 'FooBarBundle:Bar';
+
+        $metadata = $this->getMockBuilder('Doctrine\ORM\Mapping\ClassMetadata')->disableOriginalConstructor()->getMock();
+        $metadata->expects($this->any())->method('getName')->will($this->returnValue('Symfony\Tests\Bridge\Doctrine\Fixtures\SingleIdentEntity'));
+
+        $em = $this->getMock('Doctrine\Common\Persistence\ObjectManager');
+        $em->expects($this->once())->method('getClassMetaData')->with($shorthandName)->will($this->returnValue($metadata));
+
+        $choiceList = new EntityChoiceList(
+            $em,
+            $shorthandName,
+            null,
+            null,
+            null,
+            null
+        );
+    }
 }