[TASK] Disallow empty values for multi select fields 39/43339/3
authorAlexander Stehlik <alexander.stehlik@gmail.com>
Tue, 15 Sep 2015 15:22:06 +0000 (17:22 +0200)
committerAnja Leichsenring <aleichsenring@ab-softlab.de>
Tue, 15 Sep 2015 15:46:59 +0000 (17:46 +0200)
The TcaSelectItems provider strips out empty values from the
value array if more than one item can be selected.

Resolves: #69834
Releases: master
Change-Id: I31fba0180d3ec9be66058fa65dbdcb8a17d904da
Reviewed-on: http://review.typo3.org/43339
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
typo3/sysext/backend/Classes/Form/FormDataProvider/TcaSelectItems.php
typo3/sysext/backend/Tests/Unit/Form/FormDataProvider/TcaSelectItemsTest.php

index 1bc9f2e..364a24f 100644 (file)
@@ -57,6 +57,17 @@ class TcaSelectItems extends AbstractItemProvider implements FormDataProviderInt
                        if (!is_array($fieldConfig['config']['items'])) {
                                $fieldConfig['config']['items'] = [];
                        }
+
+                       // Make sure maxitems is always filled with a valid integer value.
+                       if (
+                               !empty($fieldConfig['config']['maxitems'])
+                               && (int)$fieldConfig['config']['maxitems'] > 1
+                       ) {
+                               $fieldConfig['config']['maxitems'] = (int)$fieldConfig['config']['maxitems'];
+                       } else {
+                               $fieldConfig['config']['maxitems'] = 1;
+                       }
+
                        foreach ($fieldConfig['config']['items'] as $item) {
                                if (!is_array($item)) {
                                        throw new \UnexpectedValueException(
@@ -984,13 +995,17 @@ class TcaSelectItems extends AbstractItemProvider implements FormDataProviderInt
 
                // For single select fields we just keep the current value because the renderer
                // will take care of showing the "Invalid value" text.
+               // For maxitems=1 select fields is is also possible to select empty values.
                // @todo: move handling of invalid values to this data provider.
-               if (!isset($fieldConfig['config']['maxitems']) || (int)$fieldConfig['config']['maxitems'] <= 1) {
+               if ($fieldConfig['config']['maxitems'] === 1) {
                        return array($result['databaseRow'][$fieldName]);
                }
 
                $currentDatabaseValues = array_key_exists($fieldName, $result['databaseRow']) ? $result['databaseRow'][$fieldName] : '';
-               $currentDatabaseValuesArray = GeneralUtility::trimExplode(',', $currentDatabaseValues);
+               // Selecting empty values does not make sense for fields that can contain more than one item
+               // because it is impossible to determine if the empty value or nothing is selected.
+               // This is why empty values will be removed for multi value fields.
+               $currentDatabaseValuesArray = GeneralUtility::trimExplode(',', $currentDatabaseValues, TRUE);
                $newDatabaseValueArray = [];
 
                // Add all values that were defined by static methods and do not come from the relation
index 7168431..ab4a7ab 100644 (file)
@@ -139,6 +139,7 @@ class TcaSelectItemsTest extends UnitTestCase {
                                                                        1 => 'aValue',
                                                                ],
                                                        ],
+                                                       'maxitems' => 1
                                                ],
                                        ],
                                ],
@@ -182,6 +183,7 @@ class TcaSelectItemsTest extends UnitTestCase {
                                                                        3 => NULL,
                                                                ],
                                                        ],
+                                                       'maxitems' => 1,
                                                ],
                                        ],
                                ],
@@ -237,6 +239,7 @@ class TcaSelectItemsTest extends UnitTestCase {
                                                'config' => [
                                                        'type' => 'select',
                                                        'special' => 'tables',
+                                                       'maxitems' => 1,
                                                ],
                                        ],
                                ],
@@ -295,6 +298,7 @@ class TcaSelectItemsTest extends UnitTestCase {
                                                        'type' => 'select',
                                                        'special' => 'pagetypes',
                                                        'items' => [],
+                                                       'maxitems' => 1,
                                                ],
                                        ],
                                ],
@@ -1392,6 +1396,7 @@ class TcaSelectItemsTest extends UnitTestCase {
                                                                        3 => NULL,
                                                                ],
                                                        ],
+                                                       'maxitems' => 1,
                                                ],
                                        ],
                                ]
@@ -1454,6 +1459,7 @@ class TcaSelectItemsTest extends UnitTestCase {
                                                        'foreign_table' => 'fTable',
                                                        'foreign_table_prefix' => 'aPrefix',
                                                        'items' => [],
+                                                       'maxitems' => 1,
                                                ],
                                        ],
                                ]
@@ -1539,6 +1545,7 @@ class TcaSelectItemsTest extends UnitTestCase {
                                                                        1 => 'remove',
                                                                ],
                                                        ],
+                                                       'maxitems' => 1,
                                                ],
                                        ],
                                ]
@@ -1592,6 +1599,7 @@ class TcaSelectItemsTest extends UnitTestCase {
                                                                        1 => 'remove',
                                                                ],
                                                        ],
+                                                       'maxitems' => 1,
                                                ],
                                        ],
                                ]
@@ -1648,6 +1656,7 @@ class TcaSelectItemsTest extends UnitTestCase {
                                                                        1 => 'remove',
                                                                ],
                                                        ],
+                                                       'maxitems' => 1,
                                                ],
                                        ],
                                ]
@@ -1699,6 +1708,7 @@ class TcaSelectItemsTest extends UnitTestCase {
                                                                        1 => 'remove',
                                                                ],
                                                        ],
+                                                       'maxitems' => 1,
                                                ],
                                        ],
                                ]
@@ -1745,6 +1755,7 @@ class TcaSelectItemsTest extends UnitTestCase {
                                                                        NULL,
                                                                ],
                                                        ],
+                                                       'maxitems' => 1,
                                                ],
                                        ],
                                ],
@@ -1793,6 +1804,7 @@ class TcaSelectItemsTest extends UnitTestCase {
                                                                        1 => 'remove',
                                                                ],
                                                        ],
+                                                       'maxitems' => 1,
                                                ],
                                        ],
                                ],
@@ -1867,6 +1879,7 @@ class TcaSelectItemsTest extends UnitTestCase {
                                        3 => NULL,
                                ],
                        ],
+                       'maxitems' => 1,
                ];
 
                $this->assertSame($expected, $this->subject->addData($input));
@@ -2020,6 +2033,7 @@ class TcaSelectItemsTest extends UnitTestCase {
                                                                        NULL,
                                                                ],
                                                        ],
+                                                       'maxitems' => 1,
                                                ],
                                        ],
                                ],
@@ -2250,6 +2264,79 @@ class TcaSelectItemsTest extends UnitTestCase {
        /**
         * @test
         */
+       public function processSelectFieldValueReturnsEmptyValueForSingleSelect() {
+               $languageService = $this->prophesize(LanguageService::class);
+               $GLOBALS['LANG'] = $languageService->reveal();
+               $languageService->sL(Argument::cetera())->willReturnArgument(0);
+
+               $input = [
+                       'tableName' => 'aTable',
+                       'databaseRow' => [
+                               'aField' => '',
+                       ],
+                       'processedTca' => [
+                               'columns' => [
+                                       'aField' => [
+                                               'config' => [
+                                                       'type' => 'select',
+                                                       'maxitems' => 1,
+                                                       'items' => [],
+                                               ],
+                                       ],
+                               ],
+                       ],
+               ];
+
+               $expected = $input;
+               $expected['databaseRow']['aField'] = [
+                       '',
+               ];
+
+               $this->assertEquals($expected, $this->subject->addData($input));
+       }
+
+       /**
+        * @test
+        */
+       public function processSelectFieldValueTrimsEmptyValueForMultiValueSelect() {
+               $languageService = $this->prophesize(LanguageService::class);
+               $GLOBALS['LANG'] = $languageService->reveal();
+               $languageService->sL(Argument::cetera())->willReturnArgument(0);
+
+               $input = [
+                       'tableName' => 'aTable',
+                       'databaseRow' => [
+                               'aField' => 'b,,c',
+                       ],
+                       'processedTca' => [
+                               'columns' => [
+                                       'aField' => [
+                                               'config' => [
+                                                       'type' => 'select',
+                                                       'maxitems' => 999,
+                                                       'items' => [
+                                                               ['a', '', NULL, NULL],
+                                                               ['b', 'b', NULL, NULL],
+                                                               ['c', 'c', NULL, NULL],
+                                                       ],
+                                               ],
+                                       ],
+                               ],
+                       ],
+               ];
+
+               $expected = $input;
+               $expected['databaseRow']['aField'] = [
+                       'b',
+                       'c',
+               ];
+
+               $this->assertEquals($expected, $this->subject->addData($input));
+       }
+
+       /**
+        * @test
+        */
        public function processSelectFieldValueDoesNotCallRelationManagerForStaticOnlyItems() {
                $languageService = $this->prophesize(LanguageService::class);
                $GLOBALS['LANG'] = $languageService->reveal();