[TikiWiki-commits] [Git][tikiwiki/tiki][master] [FIX] Plugin listexecute - tracker_item_modify: split add/remove/method/assign flows

Victor Emanouilov pushed to branch master at Tiki Wiki CMS Groupware / Tiki


Commits:
252f5929 by Domeshow Emmanuel at 2026-05-13T11:02:36+00:00
[FIX] Plugin listexecute - tracker_item_modify: split add/remove/method/assign flows
---
* [ENH] Add test for tracker dropdown empty values

* [FIX] Plugin listexecute - tracker_item_modify: preserve incremental add/remove updates

* [REF] Plugin listexecute: simplify add/remove/method/assign flows

* [FIX] Plugin listexecute - Tracker item modify: preserve existing values for add/remove

* [FIX] Tracker Field: dropdown validation with leading comma values

See merge request tikiwiki/tiki!9637

- - - - -


3 changed files:

- lib/core/Search/Action/TrackerItemModify.php
- lib/core/Tracker/Field/Dropdown.php
- + lib/test/IntegrationTests/TrackerDropdownEmptyValueUpdateTest.php


Changes:

=====================================
lib/core/Search/Action/TrackerItemModify.php
=====================================
@@ -60,7 +60,7 @@ class Search_Action_TrackerItemModify implements Search_Action_Action
             }
         }
 
-        if (empty($value) && empty($calc) && empty($add) && empty($remove) && empty($method)) {
+        if (! isset($value) && empty($calc) && empty($add) && empty($remove) && empty($method)) {
             throw new Search_Action_Exception(tr('tracker_item_modify action missing value, calc, add or remove parameter.'));
         }
 
@@ -142,77 +142,64 @@ class Search_Action_TrackerItemModify implements Search_Action_Action
 
         $trklib = TikiLib::lib('trk');
 
-        $value = $this->stripNp($value);
+        if (is_string($value)) {
+            $value = $this->stripNp($value);
+        }
         $info = $trklib->get_tracker_item($object_id);
-        $definition = Tracker_Definition::get($info['trackerId']);
-
-        if (! empty($calc)) {
-            $runner = new Math_Formula_Runner(
-                [
-                    'Math_Formula_Function_' => '',
-                    'Tiki_Formula_Function_' => '',
-                ]
-            );
-            try {
-                $runner->setFormula($calc);
-                $data = ['itemId' => $object_id];
-                foreach ($runner->inspect() as $fieldName) {
-                    if (is_string($fieldName) || is_numeric($fieldName)) {
-                        $tField = $definition->getField($fieldName);
-                        if ($tField && isset($info[$tField['fieldId']])) {
-                            $data[$fieldName] = $info[$tField['fieldId']];
-                        }
-                    }
-                }
-                $item = Tracker_Item::fromInfo($info);
-                $item->prepareFieldValues($data);
-                $runner->setVariables($data);
-                $value = $runner->evaluate();
-            } catch (Math_Formula_Exception $e) {
-                throw new Search_Action_Exception(tr('Error applying tracker_item_modify calc formula to item %0: %1', $object_id, $e->getMessage()));
-            }
+        if (! $info) {
+            throw new Search_Action_Exception(tr('Tracker item %0 not found.', $object_id));
         }
+        $definition = Tracker_Definition::get($info['trackerId']);
 
         $fieldInfo = $definition->getField($field);
-        $info[$fieldInfo['fieldId']] = $value;
         $handler = $definition->getFieldFactory()->getHandler($fieldInfo, $info);
-
-        if (! empty($add)) {
+        // Ref: https://doc.tiki.org/PluginListExecute#tracker_item_modify
+        if ($add) {
             $value = $handler->addValue($add);
-        }
-
-        if (! empty($remove)) {
+        } elseif ($remove) {
             $value = $handler->removeValue($remove);
-        }
-
-        if (empty($add) && empty($remove)) {
-            if (is_scalar($value)) {
-                $value = ['ins_' . $fieldInfo['fieldId'] => $value];
+        } elseif (in_array($method, ['add', 'remove'], true)) {
+            $value = $this->normalizeAssignedValue($handler, $value, $fieldInfo['fieldId']);
+            $values = explode(',', $value);
+            $computed = $handler->getValue();
+
+            foreach ($values as $val) {
+                $currentInfo = $info;
+                $currentInfo[$fieldInfo['fieldId']] = $computed;
+                $currentHandler = $definition->getFieldFactory()->getHandler($fieldInfo, $currentInfo);
+                $computed = $method === 'add'
+                    ? $currentHandler->addValue($val)
+                    : $currentHandler->removeValue($val);
             }
-            $data = $handler->getFieldData($value);
-            $value = $data['value'];
-
-            switch ($method) {
-                case 'add':
-                    $values = explode(',', $value);
-                    $value = '';
-                    foreach ($values as $val) {
-                        $value .= $handler->addValue($val) . ',';
-                    }
-                    $values = explode(',', $value);
-                    $value = implode(',', array_unique(array_filter($values)));
-                    break;
-                case 'remove':
-                    $values = explode(',', $value);
-                    $value = '';
-                    foreach ($values as $val) {
-                        // FIXME only the last value gets removed
-                        $value = $handler->removeValue($val) . ',';
+            $value = $computed;
+        } else {
+            if (! empty($calc)) {
+                $runner = new Math_Formula_Runner(
+                    [
+                        'Math_Formula_Function_' => '',
+                        'Tiki_Formula_Function_' => '',
+                    ]
+                );
+                try {
+                    $runner->setFormula($calc);
+                    $data = ['itemId' => $object_id];
+                    foreach ($runner->inspect() as $fieldName) {
+                        if (is_string($fieldName) || is_numeric($fieldName)) {
+                            $tField = $definition->getField($fieldName);
+                            if ($tField && isset($info[$tField['fieldId']])) {
+                                $data[$fieldName] = $info[$tField['fieldId']];
+                            }
+                        }
                     }
-                    $values = explode(',', $value);
-                    $value = implode(',', array_unique(array_filter($values)));
-                    break;
+                    $item = Tracker_Item::fromInfo($info);
+                    $item->prepareFieldValues($data);
+                    $runner->setVariables($data);
+                    $value = $runner->evaluate();
+                } catch (Math_Formula_Exception $e) {
+                    throw new Search_Action_Exception(tr('Error applying tracker_item_modify calc formula to item %0: %1', $object_id, $e->getMessage()));
+                }
             }
+            $value = $this->normalizeAssignedValue($handler, $value, $fieldInfo['fieldId']);
         }
 
         $utilities = new Services_Tracker_Utilities();
@@ -233,4 +220,14 @@ class Search_Action_TrackerItemModify implements Search_Action_Action
     {
         return str_replace(['~np~', '~/np~'], '', $value);
     }
+
+    private function normalizeAssignedValue($handler, $value, int $fieldId): string
+    {
+        if (is_scalar($value)) {
+            $value = ['ins_' . $fieldId => $value];
+        }
+
+        $data = $handler->getFieldData($value);
+        return $data['value'];
+    }
 }


=====================================
lib/core/Tracker/Field/Dropdown.php
=====================================
@@ -509,7 +509,13 @@ class Tracker_Field_Dropdown extends \Tracker\Field\AbstractItemField implements
     {
         if ($this->getConfiguration('type') !== 'D') {
             $value = $this->getValue($this->getDefaultValue());
-            $allValues = $value === '' ? [] : explode(',', $value);
+            $allValues = $value === '' ? []
+                : array_filter(
+                    array_map('trim', explode(',', $value)),
+                    static function ($val) {
+                        return $val !== '';
+                    }
+                );
 
             if (! empty($allValues)) {
                 foreach ($allValues as $val) {


=====================================
lib/test/IntegrationTests/TrackerDropdownEmptyValueUpdateTest.php
=====================================
@@ -0,0 +1,151 @@
+<?php
+
+// (c) Copyright by authors of the Tiki Wiki CMS Groupware Project
+//
+// All Rights Reserved. See copyright.txt for details and a complete list of authors.
+// Licensed under the GNU LESSER GENERAL PUBLIC LICENSE. See license.txt for details.
+
+namespace Tiki\Lib\Test\IntegrationTests;
+
+use Services_Tracker_Utilities;
+use TikiLib;
+use TikiTestCase;
+use Tracker_Definition;
+
+/**
+ * @group integration
+ */
+class TrackerDropdownEmptyValueUpdateTest extends TikiTestCase
+{
+    protected static $trklib;
+    protected static $trackerId;
+    protected static $oldPrefs;
+
+    public static function setUpBeforeClass(): void
+    {
+        global $prefs;
+
+        self::$oldPrefs = $prefs;
+        $prefs['feature_trackers'] = 'y';
+
+        parent::setUpBeforeClass();
+        self::$trklib = TikiLib::lib('trk');
+        self::$trackerId = self::$trklib->replace_tracker(null, 'Dropdown Empty Value Test ' . uniqid(), '', [], 'n');
+
+        $fields = [
+            [
+                'name' => 'Title',
+                'type' => 't',
+                'isHidden' => 'n',
+                'isMandatory' => 'y',
+                'permName' => 'test_title',
+            ],
+            [
+                'name' => 'Statuses',
+                'type' => 'M',
+                'isHidden' => 'n',
+                'isMandatory' => 'n',
+                'permName' => 'test_statuses',
+                'options' => json_encode([
+                    'options' => ['=Default', '1=Active', '2=Delayed', '3=Closed'],
+                ]),
+            ],
+        ];
+
+        foreach ($fields as $i => $field) {
+            self::$trklib->replace_tracker_field(
+                self::$trackerId,
+                0,
+                $field['name'],
+                $field['type'],
+                'y',
+                'y',
+                'y',
+                'y',
+                $field['isHidden'],
+                $field['isMandatory'],
+                ($i + 1) * 10,
+                $field['options'] ?? '',
+                '',
+                '',
+                null,
+                '',
+                null,
+                null,
+                'n',
+                '',
+                '',
+                '',
+                $field['permName']
+            );
+        }
+    }
+
+    public static function tearDownAfterClass(): void
+    {
+        global $prefs;
+
+        if (! empty(self::$trackerId)) {
+            self::$trklib->remove_tracker(self::$trackerId);
+        }
+
+        $prefs = self::$oldPrefs;
+        parent::tearDownAfterClass();
+    }
+
+    public function testUpdateItemAllowsSavingValidEmptyDropdownValue(): void
+    {
+        $itemId = $this->createItem('1');
+        $definition = Tracker_Definition::get(self::$trackerId);
+        $fieldInfo = $definition->getFieldFromPermName('test_statuses');
+        $itemInfo = self::$trklib->get_tracker_item($itemId);
+        $handler = $definition->getFieldFactory()->getHandler($fieldInfo, $itemInfo);
+
+        $fieldData = $handler->getFieldData([
+            'ins_' . $fieldInfo['fieldId'] => ['', '1', '2'],
+        ]);
+
+        $this->assertSame(',1,2', $fieldData['value']);
+        $this->assertSame(['', '1', '2'], $fieldData['selected']);
+
+        $utilities = new Services_Tracker_Utilities();
+        $result = $utilities->updateItem(
+            $definition,
+            [
+                'itemId' => $itemId,
+                'status' => 'o',
+                'fields' => [
+                    'test_statuses' => $fieldData['value'],
+                ],
+                'validate' => true,
+                'notify_watchers' => null,
+            ]
+        );
+
+        $this->assertNotFalse($result);
+
+        $updatedItem = self::$trklib->get_tracker_item($itemId);
+        $this->assertSame(',1,2', $updatedItem[$fieldInfo['fieldId']]);
+
+        $updatedHandler = $definition->getFieldFactory()->getHandler($fieldInfo, $updatedItem);
+        $this->assertTrue($updatedHandler->isValid() === true);
+    }
+
+    private function createItem(string $statusesValue): int
+    {
+        $definition = Tracker_Definition::get(self::$trackerId);
+        $fields = $definition->getFields();
+
+        foreach ($fields as &$field) {
+            if ($field['permName'] === 'test_title') {
+                $field['value'] = 'Test item';
+            }
+
+            if ($field['permName'] === 'test_statuses') {
+                $field['value'] = $statusesValue;
+            }
+        }
+
+        return self::$trklib->replace_item(self::$trackerId, 0, ['data' => $fields], 'o');
+    }
+}



View it on GitLab: https://gitlab.com/tikiwiki/tiki/-/commit/252f5929091deba3fcd44597b1dee458eb1f5643

-- 
View it on GitLab: https://gitlab.com/tikiwiki/tiki/-/commit/252f5929091deba3fcd44597b1dee458eb1f5643
You're receiving this email because of your account on gitlab.com. Manage all notifications: https://gitlab.com/-/profile/notifications | Help: https://gitlab.com/help

_______________________________________________
TikiWiki-cvs mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tikiwiki-cvs