From acaacac0d500af1cf361a8a38fb0735877d334e6 Mon Sep 17 00:00:00 2001 From: Marie Bressy Date: Fri, 6 Nov 2026 13:24:45 +0100 Subject: [PATCH 1/2] fix mandatory fields check on updates made during item creation --- inc/container.class.php | 44 ++++++- tests/Units/ContainerItemUpdateTest.php | 162 ++++++++++++++++++++++++ 2 files changed, 204 insertions(+), 2 deletions(-) diff --git a/inc/container.class.php b/inc/container.class.php index a5a78af4..d0fbc1f8 100644 --- a/inc/container.class.php +++ b/inc/container.class.php @@ -38,6 +38,9 @@ class PluginFieldsContainer extends CommonDBTM public static $rightname = 'config'; + /** @var WeakMap|null Items being added whose values are not stored yet ("item_add" hook not called) */ + private static ?WeakMap $items_being_added = null; + public static function canCreate(): bool { return self::canUpdate(); @@ -2424,6 +2427,7 @@ public static function findAllContainers($itemtype) public static function postItemAdd(CommonDBTM $item) { PluginFieldsDestinationField::registerCreatedItem($item); + unset(self::$items_being_added[$item]); if (array_key_exists('_plugin_fields_data', $item->input)) { $container = new self(); @@ -2549,6 +2553,11 @@ public static function preItem(CommonDBTM $item) return false; } + if ($item->isNewItem()) { + self::$items_being_added ??= new WeakMap(); + self::$items_being_added[$item] = true; + } + if (count($item->fields) === 0) { $item->fields = $item->input; } @@ -2601,6 +2610,30 @@ public static function preItem(CommonDBTM $item) return false; } + /** @return array|null Values submitted on the add of this item for the container, null when the item is not being added */ + private static function getValuesSubmittedOnAdd(CommonDBTM $item, int $c_id): ?array + { + foreach (self::$items_being_added ?? [] as $added_item => $_) { + if ($added_item::class !== $item::class || (int) $added_item->getID() !== (int) $item->getID()) { + continue; + } + + if (!isset($added_item->input['_plugin_fields_data'])) { + return []; + } + + foreach (self::getSubmittedContainersData($added_item) as $data) { + if ((int) $data['plugin_fields_containers_id'] === $c_id) { + return $data; + } + } + + return []; + } + + return null; + } + /** * Validate a single container's mandatory fields for the given item, using * either the values submitted in this request or, if none were submitted @@ -2626,9 +2659,13 @@ private static function checkContainerMandatory(CommonDBTM $item, PluginFieldsCo return $data; } + // Update made while the item is being added (ex: status set on assignment): the "item_add" hook + // has not stored the values yet, so the ones submitted on the add are checked instead + $added_values = $item->isNewItem() ? null : self::getValuesSubmittedOnAdd($item, (int) $c_id); + //nothing submitted for this container in this request (e.g. untouched Tab) //tab containers can't be filled before the item exists, so skip on creation - if ($item->isNewItem() && $loc_c->fields['type'] !== 'dom') { + if (($item->isNewItem() || $added_values !== null) && $loc_c->fields['type'] !== 'dom') { return []; } @@ -2652,7 +2689,10 @@ private static function checkContainerMandatory(CommonDBTM $item, PluginFieldsCo $data[$status_field_name] = $status_value; } - if (!$item->isNewItem()) { + if ($added_values !== null) { + $data['items_id'] = $item->getID(); + $data += $added_values; + } elseif (!$item->isNewItem()) { $data['items_id'] = $item->getID(); // merge already persisted values to avoid false positives diff --git a/tests/Units/ContainerItemUpdateTest.php b/tests/Units/ContainerItemUpdateTest.php index 4888a513..8d9f8d39 100644 --- a/tests/Units/ContainerItemUpdateTest.php +++ b/tests/Units/ContainerItemUpdateTest.php @@ -31,16 +31,19 @@ namespace GlpiPlugin\Field\Tests\Units; use Auth; +use CommonITILObject; use Computer; use Glpi\Tests\DbTestCase; use Glpi\Tests\GLPITestCase; use GlpiPlugin\Field\Tests\FieldTestTrait; use PluginFieldsContainer; use PluginFieldsContainerDisplayCondition; +use PluginFieldsStatusOverride; use Ticket; use Entity; use Notification; use Problem; +use User; require_once __DIR__ . '/../FieldTestCase.php'; @@ -1117,4 +1120,163 @@ public function testNotBlockedWhenMandatoryDomContainerIsHidden(): void ERROR, ); } + + public function testCreateWithAssigneeIsNotBlockedByFilledMandatoryField(): void + { + $this->login(); + + $container = $this->createFieldContainer([ + 'label' => 'Mandatory Dom Container', + 'type' => 'dom', + 'itemtypes' => [Ticket::class], + 'is_active' => 1, + 'entities_id' => 0, + 'is_recursive' => 1, + ]); + $field = $this->createField([ + 'label' => 'Mandatory Field', + 'type' => 'text', + PluginFieldsContainer::getForeignKeyField() => $container->getID(), + 'ranking' => 1, + 'is_active' => 1, + 'is_readonly' => 0, + 'mandatory' => 1, + ]); + $field_name = $field->fields['name']; + + // The assignment updates the ticket status before the "item_add" hook stores the field value + $ticket = new Ticket(); + $ticket_id = $ticket->add([ + 'name' => 'Ticket with filled mandatory field and assignee', + 'content' => 'Test', + 'entities_id' => 0, + 'c_id' => $container->getID(), + $field_name => 'filled value', + '_actors' => [ + 'assign' => [ + ['itemtype' => User::class, 'items_id' => getItemByTypeName(User::class, 'tech', true)], + ], + ], + ]); + + $this->assertGreaterThan(0, $ticket_id); + $this->hasNoSessionMessage(ERROR); + $this->assertTrue($ticket->getFromDB($ticket_id)); + $this->assertEquals(CommonITILObject::ASSIGNED, $ticket->fields['status']); + + $values = getItemForItemtype(PluginFieldsContainer::getClassname(Ticket::class, $container->fields['name'])); + $this->assertTrue($values->getFromDBByCrit(['items_id' => $ticket_id])); + $this->assertSame('filled value', $values->fields[$field_name]); + + // Once created, the stored values are checked again on updates + $this->assertTrue($ticket->update(['id' => $ticket_id, 'name' => 'Renamed'])); + } + + public function testCreateWithAssigneeChecksStatusOverrideAgainstSubmittedValues(): void + { + $this->login(); + + $container = $this->createFieldContainer([ + 'label' => 'Status Override Dom Container', + 'type' => 'dom', + 'itemtypes' => [Ticket::class], + 'is_active' => 1, + 'entities_id' => 0, + 'is_recursive' => 1, + ]); + $field = $this->createField([ + 'label' => 'Mandatory When Assigned', + 'type' => 'text', + PluginFieldsContainer::getForeignKeyField() => $container->getID(), + 'ranking' => 1, + 'is_active' => 1, + 'is_readonly' => 0, + 'mandatory' => 0, + ]); + $field_name = $field->fields['name']; + $this->createItem(PluginFieldsStatusOverride::class, [ + PluginFieldsContainer::getForeignKeyField() => $container->getID(), + 'plugin_fields_fields_id' => $field->getID(), + 'itemtype' => Ticket::class, + 'states' => [CommonITILObject::ASSIGNED], + 'is_readonly' => 0, + 'mandatory' => 1, + ], ['states', PluginFieldsContainer::getForeignKeyField()]); + + $assign = ['assign' => [['itemtype' => User::class, 'items_id' => getItemByTypeName(User::class, 'tech', true)]]]; + + // Filled on creation: the ticket can be assigned + $ticket = new Ticket(); + $ticket_id = $ticket->add([ + 'name' => 'Filled field', + 'content' => 'Test', + 'entities_id' => 0, + 'c_id' => $container->getID(), + $field_name => 'filled value', + '_actors' => $assign, + ]); + $this->assertGreaterThan(0, $ticket_id); + $this->hasNoSessionMessage(ERROR); + $this->assertTrue($ticket->getFromDB($ticket_id)); + $this->assertEquals(CommonITILObject::ASSIGNED, $ticket->fields['status']); + + // Left empty: the ticket is created but can't move to the status requiring the field + $ticket = new Ticket(); + $ticket_id = $ticket->add([ + 'name' => 'Empty field', + 'content' => 'Test', + 'entities_id' => 0, + 'c_id' => $container->getID(), + $field_name => '', + '_actors' => $assign, + ]); + $this->assertGreaterThan(0, $ticket_id); + $this->hasSessionMessageThatContains(__('Some mandatory fields are empty', 'fields'), ERROR); + $this->assertTrue($ticket->getFromDB($ticket_id)); + $this->assertEquals(CommonITILObject::INCOMING, $ticket->fields['status']); + } + + public function testCreateWithAssigneeIsNotBlockedByMandatoryTabField(): void + { + $this->login(); + + $tab_container = $this->createFieldContainer([ + 'label' => 'Mandatory Tab Container', + 'type' => 'tab', + 'itemtypes' => [Ticket::class], + 'is_active' => 1, + 'entities_id' => 0, + 'is_recursive' => 1, + ]); + $this->createField([ + 'label' => 'Mandatory Tab Field', + 'type' => 'text', + PluginFieldsContainer::getForeignKeyField() => $tab_container->getID(), + 'ranking' => 1, + 'is_active' => 1, + 'is_readonly' => 0, + 'mandatory' => 1, + ]); + + // A tab can't be filled before the item exists, including by the updates made during its creation + $ticket = new Ticket(); + $ticket_id = $ticket->add([ + 'name' => 'Ticket with mandatory tab field and assignee', + 'content' => 'Test', + 'entities_id' => 0, + '_actors' => [ + 'assign' => [ + ['itemtype' => User::class, 'items_id' => getItemByTypeName(User::class, 'tech', true)], + ], + ], + ]); + $this->assertGreaterThan(0, $ticket_id); + $this->hasNoSessionMessage(ERROR); + $this->assertTrue($ticket->getFromDB($ticket_id)); + $this->assertEquals(CommonITILObject::ASSIGNED, $ticket->fields['status']); + + // Once created, the empty tab field blocks updates again + $this->assertFalse((new Ticket())->update(['id' => $ticket_id, 'name' => 'Renamed'])); + $this->hasSessionMessageThatContains(__('Some mandatory fields are empty', 'fields'), ERROR); + } } From dd0ec1d7579563ce9013cf488f6fca433fe4b88f Mon Sep 17 00:00:00 2001 From: Marie Bressy Date: Fri, 6 Nov 2026 13:26:41 +0100 Subject: [PATCH 2/2] update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9779874c..750c9c8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed - Fix search criterion value on multiple dropdown fields being reset after a search, which emptied the criterion when sorting or paging results +- Fix false "mandatory fields are empty" error and status not set to assigned when creating an item with an assignee and filled mandatory fields ## [1.24.7] - 2026-10-28