Merge pull request #2411 from pulzarraider/base_group_mapper_impr

Improved error messages and unit tests in BaseGroupedMapper, fixed CS

Mapper/BaseGroupedMapper.php

@@ -26,10 +26,14 @@ abstract class BaseGroupedMapper extends BaseMapper
     abstract protected function setTabs(array $tabs);
 
     /**
+     * Add new group or tab (if parameter "tab=true" is available in options)
+     *
      * @param string $name
      * @param array  $options
      *
-     * @return \Sonata\AdminBundle\Mapper\BaseGroupedMapper
+     * @return BaseGroupedMapper
+     *
+     * @throws \RuntimeException
      */
     public function with($name, array $options = array())
     {
@@ -64,16 +68,17 @@ abstract class BaseGroupedMapper extends BaseMapper
         $code = $name;
 
         // Open
-        if (array_key_exists("tab", $options) && $options["tab"]) {
+        if (array_key_exists('tab', $options) && $options['tab']) {
             $tabs = $this->getTabs();
 
             if ($this->currentTab) {
-                throw new \RuntimeException($tabs[$this->currentTab]["auto_created"] ?
-                    "New tab was added automatically when you have added field or group. You should close current tab before adding new one OR add tabs before adding groups and fields" :
-                    "You should close previous tab with end() before adding new tab"
-                );
+                if (isset($tabs[$this->currentTab]['auto_created']) && true === $tabs[$this->currentTab]['auto_created']) {
+                    throw new \RuntimeException('New tab was added automatically when you have added field or group. You should close current tab before adding new one OR add tabs before adding groups and fields.');
+                } else {
+                    throw new \RuntimeException(sprintf('You should close previous tab "%s" with end() before adding new tab "%s".', $this->currentTab, $name));
+                }
             } elseif ($this->currentGroup) {
-                throw new \RuntimeException("You should open tab before adding new groups");
+                throw new \RuntimeException(sprintf('You should open tab before adding new group "%s".', $name));
             }
 
             if (!isset($tabs[$name])) {
@@ -90,12 +95,12 @@ abstract class BaseGroupedMapper extends BaseMapper
         } else {
 
             if ($this->currentGroup) {
-                throw new \RuntimeException("You should close previous group with end() before adding new tab");
+                throw new \RuntimeException(sprintf('You should close previous group "%s" with end() before adding new tab "%s".', $this->currentGroup, $name));
             }
 
             if (!$this->currentTab) {
                 // no tab define
-                $this->with("default", array(
+                $this->with('default', array(
                     'tab'                => true,
                     'auto_created'       => true,
                     'translation_domain' => isset($options['translation_domain']) ? $options['translation_domain'] : null
@@ -103,8 +108,8 @@ abstract class BaseGroupedMapper extends BaseMapper
             }
 
             // if no tab is selected, we go the the main one named '_' ..
-            if ($this->currentTab !== "default") {
-                $code = $this->currentTab.".".$name; // groups with the same name can be on different tabs, so we prefix them in order to make unique group name
+            if ($this->currentTab !== 'default') {
+                $code = $this->currentTab.'.'.$name; // groups with the same name can be on different tabs, so we prefix them in order to make unique group name
             }
 
             $groups = $this->getGroups();
@@ -121,8 +126,8 @@ abstract class BaseGroupedMapper extends BaseMapper
             $tabs = $this->getTabs();
         }
 
-        if ($this->currentGroup && isset($tabs[$this->currentTab]) && !in_array($this->currentGroup, $tabs[$this->currentTab]["groups"])) {
-            $tabs[$this->currentTab]["groups"][] = $this->currentGroup;
+        if ($this->currentGroup && isset($tabs[$this->currentTab]) && !in_array($this->currentGroup, $tabs[$this->currentTab]['groups'])) {
+            $tabs[$this->currentTab]['groups'][] = $this->currentGroup;
         }
 
         $this->setTabs($tabs);
@@ -131,8 +136,10 @@ abstract class BaseGroupedMapper extends BaseMapper
     }
 
     /**
-     * @param       $name
-     * @param array $options
+     * Add new tab
+     *
+     * @param string $name
+     * @param array  $options
      *
      * @return BaseGroupedMapper
      */
@@ -142,7 +149,11 @@ abstract class BaseGroupedMapper extends BaseMapper
     }
 
     /**
-     * @return \Sonata\AdminBundle\Mapper\BaseGroupedMapper
+     * Close the current group or tab
+     *
+     * @return BaseGroupedMapper
+     *
+     * @throws \RuntimeException
      */
     public function end()
     {
@@ -151,7 +162,7 @@ abstract class BaseGroupedMapper extends BaseMapper
         } elseif ($this->currentTab !== null) {
             $this->currentTab = null;
         } else {
-            throw new \Exception("No open tabs or groups, you cannot use end()");
+            throw new \RuntimeException('No open tabs or groups, you cannot use end()');
         }
 
         return $this;
@@ -178,7 +189,7 @@ abstract class BaseGroupedMapper extends BaseMapper
      * Return the name of the currently selected group. The method also makes
      * sure a valid group name is currently selected
      *
-     * Note that this can have the side effect to change the "group" value
+     * Note that this can have the side effect to change the 'group' value
      * returned by the getGroup function
      *
      * @return string

Tests/Mapper/BaseGroupedMapperTest.php

@@ -26,22 +26,139 @@ class BaseGroupedMapperTest extends \PHPUnit_Framework_TestCase
      */
     protected $baseGroupedMapper;
 
+    private $tabs;
+    private $groups;
+
     public function setUp()
     {
         $admin = $this->getMock('Sonata\AdminBundle\Admin\AdminInterface');
         $builder = $this->getMock('Sonata\AdminBundle\Builder\BuilderInterface');
 
         $this->baseGroupedMapper = $this->getMockForAbstractClass('Sonata\AdminBundle\Mapper\BaseGroupedMapper', array($builder, $admin));
-        $this->baseGroupedMapper->expects($this->any())->method('getTabs')->will($this->returnValue(array()));
+
+        // php 5.3 BC
+        $object = $this;
+        $this->tabs = array();
+        $this->groups = array();
+
+        $this->baseGroupedMapper->expects($this->any())
+            ->method('getTabs')
+            ->will($this->returnCallback(function () use ($object) {
+                return $object->getTabs();
+            }));
+
+        $this->baseGroupedMapper->expects($this->any())
+            ->method('setTabs')
+            ->will($this->returnCallback(function (array $tabs) use ($object) {
+                $object->setTabs($tabs);
+            }));
+
+        $this->baseGroupedMapper->expects($this->any())
+            ->method('getGroups')
+            ->will($this->returnCallback(function () use ($object) {
+                return $object->getGroups();
+            }));
+
+        $this->baseGroupedMapper->expects($this->any())
+            ->method('setGroups')
+            ->will($this->returnCallback(function (array $groups) use ($object) {
+                $object->setGroups($groups);
+            }));
     }
 
     public function testWith()
     {
+        $this->assertCount(0, $this->tabs);
+        $this->assertCount(0, $this->groups);
         $this->assertEquals($this->baseGroupedMapper, $this->baseGroupedMapper->with('fooGroup'));
+        $this->assertCount(1, $this->tabs);
+        $this->assertCount(1, $this->groups);
     }
 
     public function testEnd()
     {
         $this->assertEquals($this->baseGroupedMapper, $this->baseGroupedMapper->with('fooGroup'));
     }
+
+    public function testTab()
+    {
+        $this->assertCount(0, $this->tabs);
+        $this->assertCount(0, $this->groups);
+        $this->assertEquals($this->baseGroupedMapper, $this->baseGroupedMapper->tab('fooTab'));
+        $this->assertCount(1, $this->tabs);
+        $this->assertCount(0, $this->groups);
+    }
+
+    public function testTab2()
+    {
+        $this->assertCount(0, $this->tabs);
+        $this->assertCount(0, $this->groups);
+        $this->assertEquals($this->baseGroupedMapper, $this->baseGroupedMapper->with('fooTab', array('tab'=>true)));
+        $this->assertCount(1, $this->tabs);
+        $this->assertCount(0, $this->groups);
+    }
+
+    public function testFluidInterface()
+    {
+        $this->assertEquals($this->baseGroupedMapper, $this->baseGroupedMapper->tab('fooTab')->with('fooGroup1')->end()->with('fooGroup2')->end()->with('fooGroup3')->end()->end()->tab('barTab')->with('barGroup1')->end()->with('barGroup2')->end()->with('barGroup3')->end()->end());
+    }
+
+    /**
+     * @expectedException        RuntimeException
+     * @expectedExceptionMessage You should close previous group "fooGroup1" with end() before adding new tab "fooGroup2".
+     */
+    public function testGroupNotClosedException()
+    {
+        $this->baseGroupedMapper->with('fooGroup1');
+        $this->baseGroupedMapper->with('fooGroup2');
+    }
+
+    /**
+     * @expectedException        RuntimeException
+     * @expectedExceptionMessage New tab was added automatically when you have added field or group. You should close current tab before adding new one OR add tabs before adding groups and fields.
+     */
+    public function testGroupInTabException()
+    {
+        $this->baseGroupedMapper->with('fooGroup');
+        $this->baseGroupedMapper->tab('fooTab');
+    }
+
+    /**
+     * @expectedException        RuntimeException
+     * @expectedExceptionMessage You should close previous tab "fooTab" with end() before adding new tab "barTab".
+     */
+    public function testTabInTabException()
+    {
+        $this->baseGroupedMapper->tab('fooTab');
+        $this->baseGroupedMapper->tab('barTab');
+    }
+
+    /**
+     * @expectedException        RuntimeException
+     * @expectedExceptionMessage No open tabs or groups, you cannot use end()
+     */
+    public function testEndException()
+    {
+        $this->baseGroupedMapper->end();
+    }
+
+    public function getTabs()
+    {
+        return $this->tabs;
+    }
+
+    public function setTabs($tabs)
+    {
+        $this->tabs = $tabs;
+    }
+
+    public function getGroups()
+    {
+        return $this->groups;
+    }
+
+    public function setGroups($groups)
+    {
+        $this->groups = $groups;
+    }
 }