[BUGFIX] Checkboxes with itemProcFunc are not saved 50/48450/2
authorAnja Leichsenring <aleichsenring@ab-softlab.de>
Wed, 11 May 2016 14:54:51 +0000 (16:54 +0200)
committerMarkus Klein <markus.klein@typo3.org>
Sat, 4 Jun 2016 21:55:01 +0000 (23:55 +0200)
Retrieve items added by an itemProcFunc before validating the current
set of selected checkboxes.

This fixes the bug that a item set consisting of mixed predefined and
dynamically added items always stored all predefined checkboxes as
selected (and no dynamic items).

It also fixes the case that a checkbox list consisting of purely
dynamically added items would never store a selection in the database,
showing all checkboxes as unselected.

Resolves: #76147
Releases: master, 7.6
Change-Id: I254a2936974f5bb5fbb6800a17667b66e1a86ca2
Reviewed-on: https://review.typo3.org/48450
Reviewed-by: Markus Klein <markus.klein@typo3.org>
Tested-by: Markus Klein <markus.klein@typo3.org>
typo3/sysext/core/Classes/DataHandling/DataHandler.php
typo3/sysext/core/Tests/Functional/DataHandling/Regular/CheckValueTestForCheckboxes.php [new file with mode: 0644]
typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_datahandler/Classes/Tca/CheckboxElementItems.php [new file with mode: 0644]
typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_datahandler/ext_tables.php
typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_datahandler/ext_tables.sql
typo3/sysext/core/Tests/Unit/DataHandling/DataHandlerTest.php

index cda90b9..cfeec42 100644 (file)
@@ -1918,16 +1918,31 @@ class DataHandler
      */
     protected function checkValueForCheck($res, $value, $tcaFieldConf, $table, $id, $realPid, $field)
     {
-        $itemC = count($tcaFieldConf['items']);
+        $items = $tcaFieldConf['items'];
+        if ($tcaFieldConf['itemsProcFunc']) {
+            /** @var ItemProcessingService $processingService */
+            $processingService = GeneralUtility::makeInstance(ItemProcessingService::class);
+            $items = $processingService->getProcessingItems($table, $realPid, $field,
+                $this->checkValue_currentRecord,
+                $tcaFieldConf, $tcaFieldConf['items']);
+        }
+
+        $itemC = count($items);
         if (!$itemC) {
             $itemC = 1;
         }
         $maxV = pow(2, $itemC) - 1;
         if ($value < 0) {
+            // @todo: throw LogicException here? Negative values for checkbox items do not make sense and indicate a coding error.
             $value = 0;
         }
         if ($value > $maxV) {
-            $value = $maxV;
+            // @todo: This case is pretty ugly: If there is an itemsProcFunc registered, and if it returns a dynamic,
+            // @todo: changing list of items, then it may happen that a value is transformed and vanished checkboxes
+            // @todo: are permanently removed from the value.
+            // @todo: Suggestion: Throw an exception instead? Maybe a specific, catchable exception that generates a
+            // @todo: error message to the user - dynamic item sets via itemProcFunc on check would be a bad idea anyway.
+            $value = $value & $maxV;
         }
         if ($field && $realPid >= 0 && $value > 0 && !empty($tcaFieldConf['eval'])) {
             $evalCodesArray = GeneralUtility::trimExplode(',', $tcaFieldConf['eval'], true);
diff --git a/typo3/sysext/core/Tests/Functional/DataHandling/Regular/CheckValueTestForCheckboxes.php b/typo3/sysext/core/Tests/Functional/DataHandling/Regular/CheckValueTestForCheckboxes.php
new file mode 100644 (file)
index 0000000..7dd1d53
--- /dev/null
@@ -0,0 +1,70 @@
+<?php
+declare (strict_types = 1);
+namespace TYPO3\CMS\Core\Tests\Functional\DataHandling\Regular;
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+use TYPO3\CMS\Backend\Utility\BackendUtility;
+use TYPO3\CMS\Core\Tests\Functional\DataHandling\AbstractDataHandlerActionTestCase;
+
+/**
+ * Functional Test for DataHandlen::checkValue() concerning checkboxes
+ */
+class CheckValueTestForCheckboxes extends AbstractDataHandlerActionTestCase
+{
+
+    /**
+     * @var string
+     */
+    protected $scenarioDataSetDirectory = 'typo3/sysext/core/Tests/Functional/DataHandling/Regular/DataSet/';
+
+    protected function setUp()
+    {
+        $this->testExtensionsToLoad[] = 'typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_datahandler';
+
+        parent::setUp();
+        $this->importScenarioDataSet('LiveDefaultPages');
+    }
+
+    /**
+     * @test
+     */
+    public function checkBoxValueMustBeDefinedInTcaItems()
+    {
+        // pid 88 comes from LiveDefaultPages
+        $result = $this->actionService->createNewRecord('tt_content', 88, array(
+            'tx_testdatahandler_checkbox' => '1'
+        ));
+        $recordUid = $result['tt_content'][0];
+
+        $record = BackendUtility::getRecord('tt_content', $recordUid);
+
+        $this->assertEquals(1, $record['tx_testdatahandler_checkbox']);
+    }
+
+    /**
+     * @test
+     */
+    public function checkBoxValueMustComeFromItemsProcFuncIfNotDefinedInTcaItems()
+    {
+        // pid 88 comes from LiveDefaultPages
+        $result = $this->actionService->createNewRecord('tt_content', 88, array(
+            'tx_testdatahandler_checkbox' => '2'
+        ));
+        $recordUid = $result['tt_content'][0];
+
+        $record = BackendUtility::getRecord('tt_content', $recordUid);
+
+        $this->assertEquals(2, $record['tx_testdatahandler_checkbox']);
+    }
+}
diff --git a/typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_datahandler/Classes/Tca/CheckboxElementItems.php b/typo3/sysext/core/Tests/Functional/Fixtures/Extensions/test_datahandler/Classes/Tca/CheckboxElementItems.php
new file mode 100644 (file)
index 0000000..3880af9
--- /dev/null
@@ -0,0 +1,16 @@
+<?php
+namespace TYPO3\TestDatahandler\Classes\Tca;
+
+/**
+ * Items processor for radio buttons for the functional tests of DataHandler::checkValue()
+ */
+class CheckboxElementItems
+{
+    /**
+     * @return array
+     */
+    public function getItems($params)
+    {
+        $params['items'][] = array('processed label', '');
+    }
+}
index 32a34bb..28642b0 100644 (file)
@@ -44,6 +44,18 @@ defined('TYPO3_MODE') or die();
                 'default' => '',
             ),
         ),
+         'tx_testdatahandler_checkbox' => array(
+             'exclude' => 1,
+             'label' => 'DataHandler Test Checkbox',
+             'config' => array(
+                 'type' => 'check',
+                 'items' => array(
+                     array('predefined label', 'predefined value')
+                 ),
+                 'itemsProcFunc' => 'TYPO3\TestDatahandler\Classes\Tca\CheckboxElementItems->getItems',
+                 'default' => '',
+             ),
+         ),
     )
 );
 
index 17033ea..171ce07 100644 (file)
@@ -4,7 +4,8 @@
 CREATE TABLE tt_content (
     tx_testdatahandler_select text,
     tx_testdatahandler_group text,
-    tx_testdatahandler_radio text
+    tx_testdatahandler_radio text,
+    tx_testdatahandler_checkbox text
 );
 
 #
index 6b42c06..805017d 100644 (file)
@@ -818,10 +818,14 @@ class DataHandlerTest extends \TYPO3\CMS\Core\Tests\UnitTestCase
                 3,
                 3
             ),
-            'Value is higher than allowed' => array(
+            'Value is higher than allowed (all checkboxes checked)' => array(
                 15,
                 7
             ),
+            'Value is higher than allowed (some checkboxes checked)' => array(
+                11,
+                3
+            ),
             'Negative value' => array(
                 -5,
                 0