[BUGFIX] Missing subcategories labels in EM 60/17660/4
authorFrancois Suter <francois@typo3.org>
Tue, 22 Jan 2013 22:16:28 +0000 (23:16 +0100)
committerChristian Kuhn <lolli@schwarzbu.ch>
Wed, 23 Jan 2013 22:19:57 +0000 (23:19 +0100)
The (new) Extension Manager correctly uses subcategories for sorting
and grouping extension configuration options (in the configuration
form), but does not display the related labels. This makes the form
rather weird and unhelpful. This information must be restored.

Resolves: #44701
Releases: 6.0, 6.1
Change-Id: I0dde1d294edc403404ad39985f91e3772ef0b710
Reviewed-on: https://review.typo3.org/17660
Reviewed-by: Wouter Wolters
Tested-by: Wouter Wolters
Reviewed-by: Christian Kuhn
Tested-by: Christian Kuhn
typo3/sysext/extensionmanager/Classes/Domain/Model/ConfigurationSubcategory.php
typo3/sysext/extensionmanager/Classes/Domain/Repository/ConfigurationItemRepository.php
typo3/sysext/extensionmanager/Resources/Private/Language/locallang.xlf
typo3/sysext/extensionmanager/Resources/Private/Templates/Configuration/ShowConfigurationForm.html
typo3/sysext/extensionmanager/Tests/Unit/Domain/Repository/ConfigurationItemRepositoryTest.php

index fabb35a..070d21a 100644 (file)
@@ -26,8 +26,13 @@ namespace TYPO3\CMS\Extensionmanager\Domain\Model;
  *
  *  This copyright notice MUST APPEAR in all copies of the script!
  ***************************************************************/
+
 /**
- * Model for configuration subcategories
+ * Model for configuration sub categories
+ *
+ * Configuration options can be structured with categories and sub categories.
+ * Categories are usually displayed as tabs and sub categories are used to
+ * group configuration items in one tab.
  *
  * @author Susanne Moog <typo3@susannemoog.de>
  */
@@ -39,6 +44,11 @@ class ConfigurationSubcategory extends \TYPO3\CMS\Extbase\DomainObject\AbstractE
        protected $name = '';
 
        /**
+        * @var string The sub category label
+        */
+       protected $label = '';
+
+       /**
         * @var \TYPO3\CMS\Extbase\Persistence\ObjectStorage<\TYPO3\CMS\Extensionmanager\Domain\Model\ConfigurationItem>
         */
        protected $items;
@@ -90,6 +100,25 @@ class ConfigurationSubcategory extends \TYPO3\CMS\Extbase\DomainObject\AbstractE
                return $this->name;
        }
 
+       /**
+        * Set sub category label
+        *
+        * @param string $label
+        * @return void
+        */
+       public function setLabel($label) {
+               $this->label = $label;
+       }
+
+       /**
+        * Get sub category label
+        *
+        * @return string
+        */
+       public function getLabel() {
+               return $this->label;
+       }
+
 }
 
 
index ab1547a..da468ff 100644 (file)
@@ -67,7 +67,9 @@ class ConfigurationItemRepository {
         * @return null|\SplObjectStorage
         */
        public function findByExtension(array $extension) {
-               $configRaw = \TYPO3\CMS\Core\Utility\GeneralUtility::getUrl(PATH_site . $extension['siteRelPath'] . '/ext_conf_template.txt');
+               $configRaw = \TYPO3\CMS\Core\Utility\GeneralUtility::getUrl(
+                       PATH_site . $extension['siteRelPath'] . '/ext_conf_template.txt'
+               );
                $configurationObjectStorage = NULL;
                if ($configRaw) {
                        $configurationArray = $this->convertRawConfigurationToArray($configRaw, $extension);
@@ -89,7 +91,10 @@ class ConfigurationItemRepository {
                $configuration = $this->mergeWithExistingConfiguration($defaultConfiguration, $extension);
                $hierarchicConfiguration = array();
                foreach ($configuration as $configurationOption) {
-                       $hierarchicConfiguration = \TYPO3\CMS\Core\Utility\GeneralUtility::array_merge_recursive_overrule($this->buildConfigurationArray($configurationOption, $extension), $hierarchicConfiguration);
+                       $hierarchicConfiguration = \TYPO3\CMS\Core\Utility\GeneralUtility::array_merge_recursive_overrule(
+                               $this->buildConfigurationArray($configurationOption, $extension),
+                               $hierarchicConfiguration
+                       );
                }
 
                // Flip category array as it was merged the other way around
@@ -182,17 +187,55 @@ class ConfigurationItemRepository {
        }
 
        /**
-        * Generate an array from the typoscript style constants
-        * Add meta data like TSConstantEditor comments
+        * Create a flat array of configuration options from
+        * incoming raw configuration file using core's typoscript parser.
         *
-        * @param string $configRaw
-        * @param array $extension
+        * Generates an array from the typoscript style constants and
+        * adds meta data like TSConstantEditor comments
+        *
+        * Result is an array, with configuration item as array keys,
+        * and item properties as key-value sub-array:
+        *
+        * array(
+        *   'fooOption' => array(
+        *     'type' => 'string',
+        *     'value' => 'foo',
+        *     ...
+        *   ),
+        *   'barOption' => array(
+        *     'type' => boolean,
+        *     'default_value' => 0,
+        *     ...
+        *   ),
+        *   ...
+        * )
+        *
+        * @param string $configRaw Raw configuration string
+        * @param array $extension Extension name
         * @return array
+        *
+        * @TODO: Code smell, this method is used in ConfigurationUtility as well.
+        *              This code should be moved elsewhere, it is not cool to have this
+        *              public method here in the repository class.
         */
        public function createArrayFromConstants($configRaw, array $extension) {
                $tsStyleConfig = $this->getT3libTsStyleConfig();
                $tsStyleConfig->doNotSortCategoriesBeforeMakingForm = TRUE;
                $theConstants = $tsStyleConfig->ext_initTSstyleConfig($configRaw, $extension['siteRelPath'], PATH_site . $extension['siteRelPath'], $GLOBALS['BACK_PATH']);
+
+               // Loop through configuration items, see if it is assigned to a sub category
+               // and add the sub category label to the item property if so.
+               foreach ($theConstants as $configurationOptionName => $configurationOption) {
+                       if (
+                               array_key_exists('subcat_name', $configurationOption)
+                               && isset($tsStyleConfig->subCategories[$configurationOption['subcat_name']])
+                               && isset($tsStyleConfig->subCategories[$configurationOption['subcat_name']][0])
+                       ) {
+                               $theConstants[$configurationOptionName]['subcat_label'] = $tsStyleConfig->subCategories[$configurationOption['subcat_name']][0];
+                       }
+               }
+
+               // Set up the additional descriptions
                if (isset($tsStyleConfig->setup['constants']['TSConstantEditor.'])) {
                        foreach ($tsStyleConfig->setup['constants']['TSConstantEditor.'] as $category => $highlights) {
                                $theConstants['__meta__'][rtrim($category, '.')]['highlightText'] = $highlights['description'];
@@ -264,6 +307,14 @@ class ConfigurationItemRepository {
                                $configurationSubcategoryObject = $this->objectManager->get('TYPO3\\CMS\\Extensionmanager\\Domain\\Model\\ConfigurationSubcategory');
                                $configurationSubcategoryObject->setName($subcatName);
                                foreach ($configurationItems as $configurationItem) {
+                                       // Set sub category label if configuration item contains a subcat label.
+                                       // The sub category label is set multiple times if there is more than one item
+                                       // in a sub category, but that is ok since all items of one sub category
+                                       // share the same label. @see createArrayFromConstants()
+                                       if (array_key_exists('subcat_label', $configurationItem)) {
+                                               $configurationSubcategoryObject->setLabel($configurationItem['subcat_label']);
+                                       }
+
                                        /** @var $configurationObject \TYPO3\CMS\Extensionmanager\Domain\Model\ConfigurationItem */
                                        $configurationObject = $this->objectManager->get('TYPO3\\CMS\\Extensionmanager\\Domain\\Model\\ConfigurationItem');
                                        if (isset($configurationItem['generic'])) {
index d94c072..df16c6b 100644 (file)
@@ -18,6 +18,9 @@
                        <trans-unit id="extConfTemplate.headline" xml:space="preserve">
                                <source>Configure Extension</source>
                        </trans-unit>
+                       <trans-unit id="extConfTemplate.other" xml:space="preserve">
+                               <source>Other</source>
+                       </trans-unit>
                        <trans-unit id="extConfTemplate.type.int+" xml:space="preserve">
                                <source>positive integer</source>
                        </trans-unit>
index bf7676f..bcf55af 100644 (file)
                                <div class="category">
                                        <f:for each="{category.subcategories}" as="subcategory">
                                                <div class="subcategory">
+                                               <f:if condition="{subcategory.label}">
+                                                       <f:then>
+                                                               <h3>{subcategory.label}</h3>
+                                                       </f:then>
+                                                       <f:else>
+                                                               <f:if condition="{category.subcategories->f:count()} > 1">
+                                                                       <h3><f:translate key="extConfTemplate.other" /></h3>
+                                                               </f:if>
+                                                       </f:else>
+                                               </f:if>
                                                        <f:form action="save" name="{subcategory.name}" class="validate">
                                                                <f:form.hidden name="extensionKey" value="{extension.key}" />
                                                                <f:for each="{subcategory.items}" as="item">
                                                                        <div>
 
                                                                                <label for="{item.name}">
-                                                                                       <h3>{item.labelHeadline} <span class="info">[{category.name}.{item.name}]</span></h3>
+                                                                                       <h4>{item.labelHeadline} <span class="info">[{category.name}.{item.name}]</span></h4>
                                                                                        {item.labelText} </label><br />
                                                                                <f:if condition="{item.highlight}">
                                                                                        <span style="background:red; padding:1px 2px; color:#fff; font-weight:bold;">{item.highlight}</span>
index 2376035..6674fd9 100644 (file)
@@ -327,7 +327,8 @@ BE.forceSalted = 0
 TSConstantEditor.advancedbackend {
   description = <span style="background:red; padding:1px 2px; color:#fff; font-weight:bold;">1</span> Install tool has hardcoded md5 hashing, enabling this setting will prevent use of a install-tool-created BE user.<br />Currently same is for changin password with user setup module unless you use pending patch!
                        1=BE.forceSalted
-}',
+}
+                               ',
                                array(
                                        'checkConfigurationFE' => array(
                                                'cat' => 'basic',
@@ -364,7 +365,8 @@ TSConstantEditor.advancedbackend {
                                                'label' => 'Frontend configuration check',
                                                'name' => 'checkConfigurationFE',
                                                'value' => '0',
-                                               'default_value' => '0'
+                                               'default_value' => '0',
+                                               'subcat_label' => 'Enable features',
                                        ),
                                        'BE.forceSalted' => array(
                                                'cat' => 'advancedbackend',
@@ -374,7 +376,7 @@ TSConstantEditor.advancedbackend {
                                                'name' => 'BE.forceSalted',
                                                'value' => '0',
                                                'default_value' => '0',
-                                               'highlight' => 1
+                                               'highlight' => 1,
                                        ),
                                        '__meta__' => array(
                                                'advancedbackend' => array(
@@ -395,19 +397,38 @@ TSConstantEditor.advancedbackend {
         * @param $setupTsConstantEditor
         * @param $expected
         * @return void
+        *
+        * @TODO: Split this test to multiple tests:
+        *              test getT3libTsStyleConfig is called with $raw
+        *              test TSConstantEditor. is considered in result
+        *              test subcat_label is correctly extracted (see method)
         */
        public function createArrayFromConstantsCreatesAnArrayWithMetaInformation($raw, $constants, $setupTsConstantEditor, $expected) {
                $tsStyleConfig = $this->getMock('TYPO3\\CMS\\Core\\TypoScript\\ConfigurationForm');
-               $configurationItemRepositoryMock = $this->getMock('TYPO3\\CMS\\Extensionmanager\\Domain\\Repository\\ConfigurationItemRepository', array('getT3libTsStyleConfig'));
-               $configurationItemRepositoryMock->expects($this->once())->method('getT3libTsStyleConfig')->will($this->returnValue($tsStyleConfig));
-               $tsStyleConfig->expects($this->once())->method('ext_initTSstyleConfig')->with($raw, $this->anything(), $this->anything(), $this->anything())->will($this->returnValue($constants));
+               $configurationItemRepositoryMock = $this->getMock(
+                       'TYPO3\\CMS\\Extensionmanager\\Domain\\Repository\\ConfigurationItemRepository',
+                       array('getT3libTsStyleConfig')
+               );
+               $configurationItemRepositoryMock
+                       ->expects($this->once())
+                       ->method('getT3libTsStyleConfig')
+                       ->will($this->returnValue($tsStyleConfig));
+               $tsStyleConfig
+                       ->expects($this->once())
+                       ->method('ext_initTSstyleConfig')
+                       ->with($raw, $this->anything(), $this->anything(), $this->anything())
+                       ->will($this->returnValue($constants));
                $tsStyleConfig->setup['constants']['TSConstantEditor.'] = $setupTsConstantEditor;
+
                $constantsResult = $configurationItemRepositoryMock->createArrayFromConstants($raw, array());
                $this->assertEquals($expected, $constantsResult);
        }
 
        /**
         * @test
+        *
+        * @TODO: This test still uses internal knowledge of createArrayFromConstants() which it shouldn't
+        *              Instead, it should mock createArrayFromConstants(), use a given result array and not the raw config string.
         */
        public function convertRawConfigurationToArrayReturnsSortedHierarchicArray() {
                $configRaw = '# cat=basic/enable/10; type=string; label=Item 1: This is the first configuration item
@@ -432,6 +453,7 @@ enableJquery = 1';
                                                'name' =>'item1',
                                                'value' => 'one',
                                                'default_value' => 'one',
+                                               'subcat_label' => 'Enable features',
                                                'labels' => array(
                                                        0 => 'Item 1',
                                                        1 => 'This is the first configuration item'
@@ -446,6 +468,7 @@ enableJquery = 1';
                                                'name' =>'integerValue',
                                                'value' => '1',
                                                'default_value' => '1',
+                                               'subcat_label' => 'Enable features',
                                                'labels' => array(
                                                        0 => 'Integer Value',
                                                        1 => 'Please insert a positive integer value'
@@ -464,6 +487,7 @@ enableJquery = 1';
                                                'name' =>'enableJquery',
                                                'value' => '1',
                                                'default_value' => '1',
+                                               'subcat_label' => 'Files',
                                                'labels' => array(
                                                        0 => 'enableJquery',
                                                        1 => 'Insert jQuery plugin'
@@ -473,7 +497,10 @@ enableJquery = 1';
                        )
                );
 
-               $this->assertSame($expectedArray, $this->configurationItemRepository->_callRef('convertRawConfigurationToArray', $configRaw, $extension));
+               $this->assertSame(
+                       $expectedArray,
+                       $this->configurationItemRepository->_callRef('convertRawConfigurationToArray', $configRaw, $extension)
+               );
        }
 
 }