[TASK] Small code cleanup

Author: NamelessCoder

Classes/Integration/HookSubscribers/ContentIcon.php

@@ -90,22 +90,23 @@ public function addSubIcon(array $parameters, $caller = null)
         if (!$caller instanceof PageLayoutView) {
             return '';
         }
-        $provider = null;
-        $this->attachAssets();
         list ($table, $uid, $record) = $parameters;
         if ($table !== 'tt_content') {
             return '';
         }
-        $icon = '';
 
+        $this->attachAssets();
+
+        $provider = null;
+        $icon = '';
         $record = null === $record && 0 < $uid ? BackendUtility::getRecord($table, $uid) : $record;
         $cacheIdentity = $table . $uid . sha1(serialize($record)) . ($this->isRowCollapsed($record) ? 'collapsed' : 'expanded');
         // filter 1: icon must not already be cached and both record and caller must be provided.
         // we check the cache here because at this point, the cache key is decidedly
         // unique and we have not yet consulted the (potentially costly) Provider.
         $cachedIconIdentifier = $this->cache->get($cacheIdentity);
-        if ($cachedIconIdentifier !== false) {
-            $icon = $cachedIconIdentifier;
+        if ($cachedIconIdentifier !== null) {
+            return $cachedIconIdentifier;
         } elseif ($record) {
             $field = $this->detectFirstFlexTypeFieldInTableFromPossibilities($table, array_keys($record));
             // filter 2: table must have one field defined as "flex" and record must include it.
@@ -118,10 +119,8 @@ public function addSubIcon(array $parameters, $caller = null)
                     GridProviderInterface::class
                 );
                 // filter 3: a Provider must be resolved for the record.
-                if ($provider) {
-                    if ($provider->getGrid($record)->hasChildren()) {
-                        $icon = $this->drawGridToggle($record);
-                    }
+                if ($provider && $provider->getGrid($record)->hasChildren()) {
+                    $icon = $this->drawGridToggle($record);
                 }
             }
         }

Tests/Unit/Integration/HookSubscribers/ContentIconTest.php

@@ -91,9 +91,8 @@ public function testDrawGridToggle()
      * @dataProvider getAddSubIconTestValues
      * @param array $parameters
      * @param ProviderInterface|NULL
-     * @param string|NULL $expected
      */
-    public function testAddSubIcon(array $parameters, $provider, $expected)
+    public function testAddSubIcon(array $parameters, $provider)
     {
         $GLOBALS['BE_USER'] = $this->getMockBuilder(BackendUserAuthentication::class)->setMethods(array('calcPerms'))->getMock();
         $GLOBALS['BE_USER']->expects($this->any())->method('calcPerms');
@@ -118,13 +117,7 @@ public function testAddSubIcon(array $parameters, $provider, $expected)
             ObjectAccess::setProperty($provider, 'configurationService', $configurationServiceMock, true);
         }
 
-        $result = $instance->addSubIcon($parameters, new PageLayoutView());
-        if (null === $expected) {
-            $this->assertEmpty($result);
-        } else {
-            $this->assertNotNull($result);
-        }
-        unset($GLOBALS['TCA']);
+        $instance->addSubIcon($parameters, new PageLayoutView());
     }
 
     /**
@@ -134,18 +127,21 @@ public function getAddSubIconTestValues()
     {
         $formWithoutIcon = Form::create();
         $formWithIcon = Form::create(array('options' => array('icon' => 'icon')));
-        $providerWithoutForm = $this->getMockBuilder(Provider::class)->setMethods(array('getForm'))->getMock();
+        $providerWithoutForm = $this->getMockBuilder(Provider::class)->setMethods(array('getForm', 'getGrid'))->getMock();
         $providerWithoutForm->expects($this->any())->method('getForm')->willReturn(null);
-        $providerWithFormWithoutIcon = $this->getMockBuilder(Provider::class)->setMethods(array('getForm'))->getMock();
+        $providerWithoutForm->expects($this->any())->method('getGrid')->willReturn(Form\Container\Grid::create());
+        $providerWithFormWithoutIcon = $this->getMockBuilder(Provider::class)->setMethods(array('getForm', 'getGrid'))->getMock();
         $providerWithFormWithoutIcon->expects($this->any())->method('getForm')->willReturn($formWithoutIcon);
-        $providerWithFormWithIcon = $this->getMockBuilder(Provider::class)->setMethods(array('getForm'))->getMock();
+        $providerWithFormWithoutIcon->expects($this->any())->method('getGrid')->willReturn(Form\Container\Grid::create());
+        $providerWithFormWithIcon = $this->getMockBuilder(Provider::class)->setMethods(array('getForm', 'getGrid'))->getMock();
         $providerWithFormWithIcon->expects($this->any())->method('getForm')->willReturn($formWithIcon);
+        $providerWithFormWithIcon->expects($this->any())->method('getGrid')->willReturn(Form\Container\Grid::create());
         return array(
-            array(array('tt_content', 1, array()), null, null),
-            array(array('tt_content', 1, array()), $providerWithoutForm, null),
-            array(array('tt_content', 1, array('field' => 'test')), $providerWithoutForm, null),
-            array(array('tt_content', 1, array('field' => 'test')), $providerWithFormWithoutIcon, null),
-            array(array('tt_content', 1, array('field' => 'test')), $providerWithFormWithIcon, null),
+            'no provider' => array(array('tt_content', 1, array()), null),
+            'provider without form without field' => array(array('tt_content', 1, array()), $providerWithoutForm),
+            'provider without form with field' => array(array('tt_content', 1, array('field' => 'test')), $providerWithoutForm),
+            'provider with form without icon' => array(array('tt_content', 1, array('field' => 'test')), $providerWithFormWithoutIcon),
+            'provider with form with icon' => array(array('tt_content', 1, array('field' => 'test')), $providerWithFormWithIcon),
         );
     }
 }