[BUGFIX] FormEngine: Fix keepItems, addItems and removeItems handling 70/44570/4
authorMorton Jonuschat <m.jonuschat@mojocode.de>
Fri, 6 Nov 2015 10:07:46 +0000 (11:07 +0100)
committerWouter Wolters <typo3@wouterwolters.nl>
Fri, 6 Nov 2015 14:41:46 +0000 (15:41 +0100)
With the FormEngine rewrite the evaluation order and handling of the
options changed. This patch restores the previous order, adds tests
for the addItems handling as well as for the execution order of
keepItems, addItems and removeItems.

In addition the behavior of keepItems with an empty list of items has
been restored.

Resolves: #70956
Releases: master
Change-Id: I44b3036e2ba4dd824037aa689543dc2f1c653b93
Reviewed-on: https://review.typo3.org/44570
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Wouter Wolters <typo3@wouterwolters.nl>
Tested-by: Wouter Wolters <typo3@wouterwolters.nl>
typo3/sysext/backend/Classes/Form/FormDataProvider/AbstractItemProvider.php
typo3/sysext/backend/Classes/Form/FormDataProvider/TcaSelectItems.php
typo3/sysext/backend/Classes/Form/FormDataProvider/TcaSelectTreeItems.php
typo3/sysext/backend/Tests/Unit/Form/FormDataProvider/TcaSelectItemsTest.php
typo3/sysext/core/Documentation/Changelog/master/Important-70956-BehaviorOfPageTCconfigOptionsKeepItemsAddItemsAndRemoveItemsChanged.rst [new file with mode: 0644]

index 224b460..221c8a9 100644 (file)
@@ -472,12 +472,18 @@ abstract class AbstractItemProvider
     protected function removeItemsByKeepItemsPageTsConfig(array $result, $fieldName, array $items)
     {
         $table = $result['tableName'];
-        if (empty($result['pageTsConfig']['TCEFORM.'][$table . '.'][$fieldName . '.']['keepItems'])
+        if (!isset($result['pageTsConfig']['TCEFORM.'][$table . '.'][$fieldName . '.']['keepItems'])
             || !is_string($result['pageTsConfig']['TCEFORM.'][$table . '.'][$fieldName . '.']['keepItems'])
         ) {
             return $items;
         }
 
+        // If keepItems is set but is an empty list all current items get removed
+        if (empty($result['pageTsConfig']['TCEFORM.'][$table . '.'][$fieldName . '.']['keepItems'])
+            && $result['pageTsConfig']['TCEFORM.'][$table . '.'][$fieldName . '.']['keepItems'] !== '0') {
+            return [];
+        }
+
         return ArrayUtility::keepItemsInArray(
             $items,
             $result['pageTsConfig']['TCEFORM.'][$table . '.'][$fieldName . '.']['keepItems'],
index 118a2ca..a29e9e3 100644 (file)
@@ -46,7 +46,6 @@ class TcaSelectItems extends AbstractItemProvider implements FormDataProviderInt
             $fieldConfig['config']['items'] = $this->sanitizeItemArray($fieldConfig['config']['items'], $table, $fieldName);
             $fieldConfig['config']['maxitems'] = $this->sanitizeMaxItems($fieldConfig['config']['maxitems']);
 
-            $fieldConfig['config']['items'] = $this->addItemsFromPageTsConfig($result, $fieldName, $fieldConfig['config']['items']);
             $fieldConfig['config']['items'] = $this->addItemsFromSpecial($result, $fieldName, $fieldConfig['config']['items']);
             $fieldConfig['config']['items'] = $this->addItemsFromFolder($result, $fieldName, $fieldConfig['config']['items']);
             $staticItems = $fieldConfig['config']['items'];
@@ -57,7 +56,9 @@ class TcaSelectItems extends AbstractItemProvider implements FormDataProviderInt
             $removedItems = $fieldConfig['config']['items'];
 
             $fieldConfig['config']['items'] = $this->removeItemsByKeepItemsPageTsConfig($result, $fieldName, $fieldConfig['config']['items']);
+            $fieldConfig['config']['items'] = $this->addItemsFromPageTsConfig($result, $fieldName, $fieldConfig['config']['items']);
             $fieldConfig['config']['items'] = $this->removeItemsByRemoveItemsPageTsConfig($result, $fieldName, $fieldConfig['config']['items']);
+
             $fieldConfig['config']['items'] = $this->removeItemsByUserLanguageFieldRestriction($result, $fieldName, $fieldConfig['config']['items']);
             $fieldConfig['config']['items'] = $this->removeItemsByUserAuthMode($result, $fieldName, $fieldConfig['config']['items']);
             $fieldConfig['config']['items'] = $this->removeItemsByDoktypeUserRestriction($result, $fieldName, $fieldConfig['config']['items']);
index 078fa59..0af0f80 100644 (file)
@@ -49,7 +49,6 @@ class TcaSelectTreeItems extends AbstractItemProvider implements FormDataProvide
             $fieldConfig['config']['items'] = $this->sanitizeItemArray($fieldConfig['config']['items'], $table, $fieldName);
             $fieldConfig['config']['maxitems'] = $this->sanitizeMaxItems($fieldConfig['config']['maxitems']);
 
-            $fieldConfig['config']['items'] = $this->addItemsFromPageTsConfig($result, $fieldName, $fieldConfig['config']['items']);
             $fieldConfig['config']['items'] = $this->addItemsFromSpecial($result, $fieldName, $fieldConfig['config']['items']);
             $fieldConfig['config']['items'] = $this->addItemsFromFolder($result, $fieldName, $fieldConfig['config']['items']);
             $staticItems = $fieldConfig['config']['items'];
@@ -58,7 +57,9 @@ class TcaSelectTreeItems extends AbstractItemProvider implements FormDataProvide
             $dynamicItems = array_diff_key($fieldConfig['config']['items'], $staticItems);
 
             $fieldConfig['config']['items'] = $this->removeItemsByKeepItemsPageTsConfig($result, $fieldName, $fieldConfig['config']['items']);
+            $fieldConfig['config']['items'] = $this->addItemsFromPageTsConfig($result, $fieldName, $fieldConfig['config']['items']);
             $fieldConfig['config']['items'] = $this->removeItemsByRemoveItemsPageTsConfig($result, $fieldName, $fieldConfig['config']['items']);
+
             $fieldConfig['config']['items'] = $this->removeItemsByUserLanguageFieldRestriction($result, $fieldName, $fieldConfig['config']['items']);
             $fieldConfig['config']['items'] = $this->removeItemsByUserAuthMode($result, $fieldName, $fieldConfig['config']['items']);
             $fieldConfig['config']['items'] = $this->removeItemsByDoktypeUserRestriction($result, $fieldName, $fieldConfig['config']['items']);
index 9c90e41..9e4730e 100644 (file)
@@ -1154,6 +1154,124 @@ class TcaSelectItemsTest extends UnitTestCase
     }
 
     /**
+     * @test
+     */
+    public function addDataAddsItemsByAddItemsFromPageTsConfig()
+    {
+        $input = [
+            'databaseRow' => [
+                'aField' => '',
+            ],
+            'tableName' => 'aTable',
+            'processedTca' => [
+                'columns' => [
+                    'aField' => [
+                        'config' => [
+                            'type' => 'select',
+                            'renderType' => 'selectSingle',
+                            'items' => [
+                                0 => [
+                                    0 => 'keepMe',
+                                    1 => 'keep',
+                                    null,
+                                    null,
+                                ],
+                            ],
+                            'maxitems' => 1,
+                        ],
+                    ],
+                ]
+            ],
+            'pageTsConfig' => [
+                'TCEFORM.' => [
+                    'aTable.' => [
+                        'aField.' => [
+                            'addItems.' => [
+                                '1' => 'addMe'
+                            ],
+                        ],
+                    ],
+                ],
+            ],
+        ];
+
+        /** @var LanguageService|ObjectProphecy $languageService */
+        $languageService = $this->prophesize(LanguageService::class);
+        $GLOBALS['LANG'] = $languageService->reveal();
+        $languageService->sL(Argument::cetera())->willReturnArgument(0);
+
+        $expected = $input;
+        $expected['databaseRow']['aField'] = [];
+        $expected['processedTca']['columns']['aField']['config']['items'][1] = [
+            0 => 'addMe',
+            1 => '1',
+            null,
+            null,
+        ];
+
+        $this->assertEquals($expected, $this->subject->addData($input));
+    }
+
+    /**
+     * @test
+     */
+    public function addDataAddsItemsByAddItemsWithDuplicateValuesFromPageTsConfig()
+    {
+        $input = [
+            'databaseRow' => [
+                'aField' => '',
+            ],
+            'tableName' => 'aTable',
+            'processedTca' => [
+                'columns' => [
+                    'aField' => [
+                        'config' => [
+                            'type' => 'select',
+                            'renderType' => 'selectSingle',
+                            'items' => [
+                                0 => [
+                                    0 => 'keepMe',
+                                    1 => 'keep',
+                                    null,
+                                    null,
+                                ],
+                            ],
+                            'maxitems' => 1,
+                        ],
+                    ],
+                ]
+            ],
+            'pageTsConfig' => [
+                'TCEFORM.' => [
+                    'aTable.' => [
+                        'aField.' => [
+                            'addItems.' => [
+                                'keep' => 'addMe'
+                            ],
+                        ],
+                    ],
+                ],
+            ],
+        ];
+
+        /** @var LanguageService|ObjectProphecy $languageService */
+        $languageService = $this->prophesize(LanguageService::class);
+        $GLOBALS['LANG'] = $languageService->reveal();
+        $languageService->sL(Argument::cetera())->willReturnArgument(0);
+
+        $expected = $input;
+        $expected['databaseRow']['aField'] = [];
+        $expected['processedTca']['columns']['aField']['config']['items'][1] = [
+            0 => 'addMe',
+            1 => 'keep',
+            null,
+            null,
+        ];
+
+        $this->assertEquals($expected, $this->subject->addData($input));
+    }
+
+    /**
      * Data provider
      */
     public function addDataReplacesMarkersInForeignTableClauseDataProvider()
@@ -1650,6 +1768,141 @@ class TcaSelectItemsTest extends UnitTestCase
     /**
      * @test
      */
+    public function addDataRemovesAllItemsByEmptyKeepItemsPageTsConfig()
+    {
+        $input = [
+            'databaseRow' => [
+                'aField' => '',
+            ],
+            'tableName' => 'aTable',
+            'processedTca' => [
+                'columns' => [
+                    'aField' => [
+                        'config' => [
+                            'type' => 'select',
+                            'renderType' => 'selectSingle',
+                            'items' => [
+                                0 => [
+                                    0 => 'keepMe',
+                                    1 => 'keep',
+                                    null,
+                                    null,
+                                ],
+                                1 => [
+                                    0 => 'removeMe',
+                                    1 => 'remove',
+                                ],
+                            ],
+                            'maxitems' => 1,
+                        ],
+                    ],
+                ]
+            ],
+            'pageTsConfig' => [
+                'TCEFORM.' => [
+                    'aTable.' => [
+                        'aField.' => [
+                            'keepItems' => '',
+                        ],
+                    ],
+                ],
+            ],
+        ];
+
+        /** @var LanguageService|ObjectProphecy $languageService */
+        $languageService = $this->prophesize(LanguageService::class);
+        $GLOBALS['LANG'] = $languageService->reveal();
+        $languageService->sL(Argument::cetera())->willReturnArgument(0);
+
+        $expected = $input;
+        $expected['databaseRow']['aField'] = [];
+        $expected['processedTca']['columns']['aField']['config']['items'] = [];
+
+        $this->assertEquals($expected, $this->subject->addData($input));
+    }
+
+    /**
+     * @test
+     */
+    public function addDataEvaluatesKeepItemsBeforeAddItemsFromPageTsConfig()
+    {
+        $input = [
+            'databaseRow' => [
+                'aField' => '',
+            ],
+            'tableName' => 'aTable',
+            'processedTca' => [
+                'columns' => [
+                    'aField' => [
+                        'config' => [
+                            'type' => 'select',
+                            'renderType' => 'selectSingle',
+                            'items' => [
+                                0 => [
+                                    0 => 'keepMe',
+                                    1 => '1',
+                                    null,
+                                    null,
+                                ],
+                                1 => [
+                                    0 => 'removeMe',
+                                    1 => 'remove',
+                                ],
+                            ],
+                            'maxitems' => 1,
+                        ],
+                    ],
+                ]
+            ],
+            'pageTsConfig' => [
+                'TCEFORM.' => [
+                    'aTable.' => [
+                        'aField.' => [
+                            'keepItems' => '1',
+                            'addItems.' => [
+                                '1' => 'addItem #1',
+                                '12' => 'addItem #12',
+                            ],
+                        ],
+                    ],
+                ],
+            ],
+        ];
+
+        /** @var LanguageService|ObjectProphecy $languageService */
+        $languageService = $this->prophesize(LanguageService::class);
+        $GLOBALS['LANG'] = $languageService->reveal();
+        $languageService->sL(Argument::cetera())->willReturnArgument(0);
+
+        $expected = $input;
+        $expected['databaseRow']['aField'] = [];
+        $expected['processedTca']['columns']['aField']['config']['items'] = [
+            0 => [
+                0 => 'keepMe',
+                1 => '1',
+                null,
+                null,
+            ],
+            1 => [
+                0 => 'addItem #1',
+                1 => '1',
+                null,
+                null,
+            ],
+            2 => [
+                0 => 'addItem #12',
+                1 => '12',
+                null,
+                null,
+            ],
+        ];
+
+        $this->assertEquals($expected, $this->subject->addData($input));
+    }
+
+    /**
+     * @test
+     */
     public function addDataRemovesItemsByRemoveItemsPageTsConfig()
     {
         $input = [
@@ -1706,6 +1959,65 @@ class TcaSelectItemsTest extends UnitTestCase
     /**
      * @test
      */
+    public function addDataRemovesItemsAddedByAddItemsFromPageTsConfigByRemoveItemsPageTsConfig()
+    {
+        $input = [
+            'databaseRow' => [
+                'aField' => ''
+            ],
+            'tableName' => 'aTable',
+            'processedTca' => [
+                'columns' => [
+                    'aField' => [
+                        'config' => [
+                            'type' => 'select',
+                            'renderType' => 'selectSingle',
+                            'items' => [
+                                0 => [
+                                    0 => 'keepMe',
+                                    1 => 'keep',
+                                    null,
+                                    null,
+                                ],
+                                1 => [
+                                    0 => 'removeMe',
+                                    1 => 'remove',
+                                ],
+                            ],
+                            'maxitems' => 1,
+                        ],
+                    ],
+                ]
+            ],
+            'pageTsConfig' => [
+                'TCEFORM.' => [
+                    'aTable.' => [
+                        'aField.' => [
+                            'removeItems' => 'remove,add',
+                            'addItems.' => [
+                                'add' => 'addMe'
+                            ]
+                        ],
+                    ],
+                ],
+            ],
+        ];
+
+        /** @var LanguageService|ObjectProphecy $languageService */
+        $languageService = $this->prophesize(LanguageService::class);
+        $GLOBALS['LANG'] = $languageService->reveal();
+        $languageService->sL(Argument::cetera())->willReturnArgument(0);
+
+        $expected = $input;
+        $expected['databaseRow']['aField'] = [];
+        unset($expected['processedTca']['columns']['aField']['config']['items'][1]);
+
+        $this->assertEquals($expected, $this->subject->addData($input));
+    }
+
+    /**
+     * @test
+     */
     public function addDataRemovesItemsByLanguageFieldUserRestriction()
     {
         $input = [
diff --git a/typo3/sysext/core/Documentation/Changelog/master/Important-70956-BehaviorOfPageTCconfigOptionsKeepItemsAddItemsAndRemoveItemsChanged.rst b/typo3/sysext/core/Documentation/Changelog/master/Important-70956-BehaviorOfPageTCconfigOptionsKeepItemsAddItemsAndRemoveItemsChanged.rst
new file mode 100644 (file)
index 0000000..7a2631f
--- /dev/null
@@ -0,0 +1,14 @@
+=================================================================================================
+Important: #70956 - Behavior of Page TCconfig options keepItems, addItems and removeItems changed
+=================================================================================================
+
+Description
+===========
+
+The behavior of Page TSconfig options ``keepItems``, ``addItems`` and ``removeItems``
+has been restored to state of TYPO3 CMS 6.2-7.4 and the execution order of these
+options has been formalized.
+
+The first option to be evaluated is ``keepItems``, followed in turn by ``addItems``
+and ``removeItems``. All three options are evaluated after items have been added to
+the configuration by sources like folders or foreign tables.