Merge pull request #3771 from jordisala1991/fix-admin-helper

Fixed detection of properties with underscores on nested forms

Admin/AdminHelper.php

@@ -19,6 +19,7 @@ use Sonata\AdminBundle\Util\FormViewIterator;
 use Symfony\Component\Form\FormBuilderInterface;
 use Symfony\Component\Form\FormView;
 use Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException;
+use Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException;
 use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
 
 /**
@@ -254,53 +255,28 @@ class AdminHelper
     {
         $propertyAccessor = $this->pool->getPropertyAccessor();
 
-        $idWithoutUniqueIdentifier = implode('_', explode('_', substr($elementId, strpos($elementId, '_') + 1)));
+        $idWithoutIdentifier = preg_replace('/^[^_]*_/', '', $elementId);
+        $initialPath = preg_replace('#(_(\d+)_)#', '[$2]_', $idWithoutIdentifier);
 
-        //array access of id converted to format which PropertyAccessor understands
-        $initialPath = preg_replace('#(_(\d+)_)#', '[$2]', $idWithoutUniqueIdentifier);
+        $parts = explode('_', $initialPath);
+        $totalPath = '';
+        $currentPath = '';
 
-        $parts = preg_split('#\[\d+\]#', $initialPath);
+        foreach ($parts as $part) {
+            $currentPath .= empty($currentPath) ? $part : '_'.$part;
+            $separator = empty($totalPath) ? '' : '.';
 
-        $partReturnValue = $returnValue = '';
-        $currentEntity = $entity;
-
-        foreach ($parts as $key => $value) {
-            $subParts = explode('_', $value);
-            $id = '';
-            $dot = '';
-
-            foreach ($subParts as $subValue) {
-                $id .= ($id) ? '_'.$subValue : $subValue;
-
-                if ($this->pathExists($propertyAccessor, $currentEntity, $partReturnValue.$dot.$id)) {
-                    $partReturnValue .= $dot.$id;
-                    $dot = '.';
-                    $id = '';
-                } else {
-                    $dot = '';
-                }
+            if ($this->pathExists($propertyAccessor, $entity, $totalPath.$separator.$currentPath)) {
+                $totalPath .= $separator.$currentPath;
+                $currentPath = '';
             }
+        }
 
-            if ($dot !== '.') {
-                throw new \Exception(sprintf('Could not get element id from %s Failing part: %s', $elementId, $subValue));
-            }
-
-            //check if array access was in this location originally
-            preg_match("#$value\[(\d+)#", $initialPath, $matches);
-
-            if (isset($matches[1])) {
-                $partReturnValue .= '['.$matches[1].']';
-            }
-
-            $returnValue .= $returnValue ? '.'.$partReturnValue : $partReturnValue;
-            $partReturnValue = '';
-
-            if (isset($parts[$key + 1])) {
-                $currentEntity = $propertyAccessor->getValue($entity, $returnValue);
-            }
+        if (!empty($currentPath)) {
+            throw new \Exception(sprintf('Could not get element id from %s Failing part: %s', $elementId, $currentPath));
         }
 
-        return $returnValue;
+        return $totalPath;
     }
 
     /**
@@ -346,6 +322,8 @@ class AdminHelper
             return true;
         } catch (NoSuchPropertyException $e) {
             return false;
+        } catch (UnexpectedTypeException $e) {
+            return false;
         }
     }
 }

CHANGELOG.md

@@ -6,6 +6,9 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 ### Added
 - Added AbstractAdmin, replacing Admin
 
+### Fixed
+- Fix detection of path when using nested properties with underscores in `AdminHelper:getElementAccessPath` method
+
 ### Changed
 - Updated AdminLTE theme to version 2.3.3
 

Tests/Admin/AdminHelperTest.php

@@ -18,13 +18,21 @@ use Symfony\Component\Form\FormView;
 
 class AdminHelperTest extends \PHPUnit_Framework_TestCase
 {
-    public function testGetChildFormBuilder()
+    /**
+     * @var AdminHelper
+     */
+    protected $helper;
+
+    public function setUp()
     {
         $container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface');
 
         $pool = new Pool($container, 'title', 'logo.png');
-        $helper = new AdminHelper($pool);
+        $this->helper = new AdminHelper($pool);
+    }
 
+    public function testGetChildFormBuilder()
+    {
         $formFactory = $this->getMock('Symfony\Component\Form\FormFactoryInterface');
         $eventDispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface');
 
@@ -33,33 +41,23 @@ class AdminHelperTest extends \PHPUnit_Framework_TestCase
         $childFormBuilder = new FormBuilder('elementId', 'stdClass', $eventDispatcher, $formFactory);
         $formBuilder->add($childFormBuilder);
 
-        $this->assertNull($helper->getChildFormBuilder($formBuilder, 'foo'));
-        $this->isInstanceOf('Symfony\Component\Form\FormBuilder', $helper->getChildFormBuilder($formBuilder, 'test_elementId'));
+        $this->assertNull($this->helper->getChildFormBuilder($formBuilder, 'foo'));
+        $this->isInstanceOf('Symfony\Component\Form\FormBuilder', $this->helper->getChildFormBuilder($formBuilder, 'test_elementId'));
     }
 
     public function testGetChildFormView()
     {
-        $container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface');
-
-        $pool = new Pool($container, 'title', 'logo.png');
-        $helper = new AdminHelper($pool);
-
         $formView = new FormView();
         $formView->vars['id'] = 'test';
         $child = new FormView($formView);
         $child->vars['id'] = 'test_elementId';
 
-        $this->assertNull($helper->getChildFormView($formView, 'foo'));
-        $this->isInstanceOf('Symfony\Component\Form\FormView', $helper->getChildFormView($formView, 'test_elementId'));
+        $this->assertNull($this->helper->getChildFormView($formView, 'foo'));
+        $this->isInstanceOf('Symfony\Component\Form\FormView', $this->helper->getChildFormView($formView, 'test_elementId'));
     }
 
     public function testAddNewInstance()
     {
-        $container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface');
-
-        $pool = new Pool($container, 'title', 'logo.png');
-        $helper = new AdminHelper($pool);
-
         $admin = $this->getMock('Sonata\AdminBundle\Admin\AdminInterface');
         $admin->expects($this->once())->method('getNewInstance')->will($this->returnValue(new \stdClass()));
 
@@ -70,16 +68,11 @@ class AdminHelperTest extends \PHPUnit_Framework_TestCase
         $object = $this->getMock('sdtClass', array('addFooBar'));
         $object->expects($this->once())->method('addFooBar');
 
-        $helper->addNewInstance($object, $fieldDescription);
+        $this->helper->addNewInstance($object, $fieldDescription);
     }
 
     public function testAddNewInstancePlural()
     {
-        $container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface');
-
-        $pool = new Pool($container, 'title', 'logo.png');
-        $helper = new AdminHelper($pool);
-
         $admin = $this->getMock('Sonata\AdminBundle\Admin\AdminInterface');
         $admin->expects($this->once())->method('getNewInstance')->will($this->returnValue(new \stdClass()));
 
@@ -90,16 +83,11 @@ class AdminHelperTest extends \PHPUnit_Framework_TestCase
         $object = $this->getMock('sdtClass', array('addFooBar'));
         $object->expects($this->once())->method('addFooBar');
 
-        $helper->addNewInstance($object, $fieldDescription);
+        $this->helper->addNewInstance($object, $fieldDescription);
     }
 
     public function testAddNewInstanceInflector()
     {
-        $container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface');
-
-        $pool = new Pool($container, 'title', 'logo.png');
-        $helper = new AdminHelper($pool);
-
         $admin = $this->getMock('Sonata\AdminBundle\Admin\AdminInterface');
         $admin->expects($this->once())->method('getNewInstance')->will($this->returnValue(new \stdClass()));
 
@@ -110,41 +98,40 @@ class AdminHelperTest extends \PHPUnit_Framework_TestCase
         $object = $this->getMock('sdtClass', array('addEntry'));
         $object->expects($this->once())->method('addEntry');
 
-        $helper->addNewInstance($object, $fieldDescription);
+        $this->helper->addNewInstance($object, $fieldDescription);
     }
 
     public function testGetElementAccessPath()
     {
-        $container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface');
+        $object = $this->getMock('stdClass', array('getPathToObject'));
+        $subObject = $this->getMock('stdClass', array('getAnother'));
+        $sub2Object = $this->getMock('stdClass', array('getMoreThings'));
 
-        $pool = new Pool($container, 'title', 'logo.png');
-        $helper = new AdminHelper($pool);
+        $object->expects($this->atLeastOnce())->method('getPathToObject')->will($this->returnValue(array($subObject)));
+        $subObject->expects($this->atLeastOnce())->method('getAnother')->will($this->returnValue($sub2Object));
+        $sub2Object->expects($this->atLeastOnce())->method('getMoreThings')->will($this->returnValue('Value'));
+
+        $path = $this->helper->getElementAccessPath('uniquePartOfId_path_to_object_0_another_more_things', $object);
+
+        $this->assertSame('path_to_object[0].another.more_things', $path);
+    }
 
+    public function testItThrowsExceptionWhenDoesNotFindTheFullPath()
+    {
+        $path = 'uniquePartOfId_path_to_object_0_more_calls';
         $object = $this->getMock('stdClass', array('getPathToObject'));
-        $subObject = $this->getMock('stdClass', array('getAnd'));
-        $sub2Object = $this->getMock('stdClass', array('getMore'));
+        $subObject = $this->getMock('stdClass', array('getMore'));
 
         $object->expects($this->atLeastOnce())->method('getPathToObject')->will($this->returnValue(array($subObject)));
-        $subObject->expects($this->atLeastOnce())->method('getAnd')->will($this->returnValue($sub2Object));
-        $sub2Object->expects($this->atLeastOnce())->method('getMore')->will($this->returnValue('Value'));
+        $subObject->expects($this->atLeastOnce())->method('getMore')->will($this->returnValue('Value'));
 
-        $path = $helper->getElementAccessPath('uniquePartOfId_path_to_object_0_and_more', $object);
+        $this->setExpectedException('Exception', 'Could not get element id from '.$path.' Failing part: calls');
 
-        $this->assertSame('path_to_object[0].and.more', $path);
+        $this->helper->getElementAccessPath($path, $object);
     }
 
-    /**
-     * tests only so far that actual value/object is retrieved.
-     *
-     * @expectedException        Exception
-     * @expectedExceptionCode    0
-     * @expectedExceptionMessage unknown collection class
-     */
     public function testAppendFormFieldElementNested()
     {
-        $container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface');
-        $pool = new Pool($container, 'title', 'logo.png');
-        $helper = new AdminHelper($pool);
         $admin = $this->getMock('Sonata\AdminBundle\Admin\AdminInterface');
         $object = $this->getMock('stdClass', array('getSubObject'));
         $simpleObject = $this->getMock('stdClass', array('getSubObject'));
@@ -169,6 +156,8 @@ class AdminHelperTest extends \PHPUnit_Framework_TestCase
         $admin->expects($this->once())->method('getFormBuilder')->will($this->returnValue($formBuilder));
         $admin->expects($this->once())->method('getSubject')->will($this->returnValue($object));
 
-        $helper->appendFormFieldElement($admin, $simpleObject, 'uniquePartOfId_sub_object_0_and_more_0_final_data');
+        $this->setExpectedException('Exception', 'unknown collection class');
+
+        $this->helper->appendFormFieldElement($admin, $simpleObject, 'uniquePartOfId_sub_object_0_and_more_0_final_data');
     }
 }