From a67cb5ce3eb58ba31b2ee8f32e20294743d4e1fa Mon Sep 17 00:00:00 2001 From: MyuTsu Date: Fri, 25 Sep 2026 15:51:29 +0200 Subject: [PATCH 1/3] fix(fields): don't enforce mandatory fields on automated item creation --- CHANGELOG.md | 1 + inc/container.class.php | 14 ++++++++++++-- tests/Units/ContainerItemUpdateTest.php | 9 +++++++++ tests/Units/ContainerTest.php | 19 ++++++------------- 4 files changed, 28 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e35fb6e4..cb4e6597 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Fix administrators losing access to a block's configuration after setting a profile to "no access" on that block. - Fix dependency conflict with GLPI core by no longer vendoring symfony/deprecation-contracts and symfony/polyfill-ctype. - Fix default field values not being applied when fields are empty on creation +- Fix mandatory fields blocking automated item creation ## [1.24.5] - 2026-09-11 diff --git a/inc/container.class.php b/inc/container.class.php index 350541f1..eee45fc5 100644 --- a/inc/container.class.php +++ b/inc/container.class.php @@ -1739,9 +1739,9 @@ public static function validateValues($data, $itemtype, $massiveaction) $field['itemtype'] = PluginFieldsField::getType(); $field['label'] = PluginFieldsLabelTranslation::getLabelFor($field); - // Check mandatory fields if ( - $field['mandatory'] == 1 + !isCommandLine() && !Session::isCron() && !($data['_auto_import'] ?? false) + && $field['mandatory'] == 1 && ( empty($value) || (($field['type'] === 'dropdown' || preg_match('/^dropdown-.+/i', (string) $field['type'])) && $value == 0) @@ -2028,6 +2028,10 @@ private static function checkContainerMandatory(CommonDBTM $item, PluginFieldsCo $status_field_name = PluginFieldsStatusOverride::getStatusFieldName($item::getType()); $data = ['plugin_fields_containers_id' => $c_id]; + if ($item->input['_auto_import'] ?? false) { + $data['_auto_import'] = true; + } + if (array_key_exists($status_field_name, $item->input) && $item->input[$status_field_name] !== '') { $data[$status_field_name] = (int) $item->input[$status_field_name]; } elseif (array_key_exists($status_field_name, $item->fields) && $item->fields[$status_field_name] !== '') { @@ -2094,6 +2098,12 @@ private static function populateData($c_id, CommonDBTM $item) $data['items_id'] = $item->getID(); } + // Carry over the "automated import" marker so mandatory fields can be relaxed + // for items created without a human filling a form. + if ($item->input['_auto_import'] ?? false) { + $data['_auto_import'] = true; + } + // Add status so it can be used with status overrides $status_field_name = PluginFieldsStatusOverride::getStatusFieldName($item->getType()); $data[$status_field_name] = null; diff --git a/tests/Units/ContainerItemUpdateTest.php b/tests/Units/ContainerItemUpdateTest.php index e381a97e..eb8d2085 100644 --- a/tests/Units/ContainerItemUpdateTest.php +++ b/tests/Units/ContainerItemUpdateTest.php @@ -340,6 +340,8 @@ public function testCreateIsBlockedWhenMandatoryDomFieldIsMissing(): void $this->simulateApiBoot(); + $GLOBALS['GLPI_IS_COMMAND_LINE'] = false; + // Creation with the mandatory field omitted must be rejected. $ticket = new Ticket(); $ticket_id = $ticket->add([ @@ -353,6 +355,8 @@ public function testCreateIsBlockedWhenMandatoryDomFieldIsMissing(): void ERROR, ); + unset($GLOBALS['GLPI_IS_COMMAND_LINE']); + // Creation with the mandatory field filled must succeed. $ticket = new Ticket(); $ticket_id = $ticket->add([ @@ -468,11 +472,16 @@ public function testUpdateIsBlockedWhenMandatoryTabFieldWasNeverFilled(): void 'entities_id' => 0, ]); + $GLOBALS['GLPI_IS_COMMAND_LINE'] = false; + // Update the main form only, without ever opening the Tab. $updated = $problem->update([ 'id' => $problem->getID(), 'name' => 'Renamed while tab field still empty', ]); + + unset($GLOBALS['GLPI_IS_COMMAND_LINE']); + $this->assertFalse($updated, 'Update must be blocked while a mandatory tab field is empty.'); $this->hasSessionMessageThatContains( __('Some mandatory fields are empty', 'fields'), diff --git a/tests/Units/ContainerTest.php b/tests/Units/ContainerTest.php index bd85899d..c269b363 100644 --- a/tests/Units/ContainerTest.php +++ b/tests/Units/ContainerTest.php @@ -245,17 +245,13 @@ public function testMailCollectorImportRespectsMandatoryFieldDefaultValue( 'content' => 'This is a test email imported via the mail collector.', ]); - // No default value on the mandatory field + $GLOBALS['GLPI_IS_COMMAND_LINE'] = false; + $tkt = $collector->buildTicket(1, $message, ['mailgates_id' => $collector->getID(), 'play_rules' => false]); $tkt['entities_id'] = 0; + $this->createItem(Ticket::class, $tkt, ['users_id', 'itemtype']); - $ticket = new Ticket(); - $ticket_id = $ticket->add($tkt); - $this->assertFalse($ticket_id, sprintf('Import must be blocked when the mandatory %s field has no value and no default.', $type)); - $this->hasSessionMessageThatContains( - __('Some mandatory fields are empty', 'fields'), - (string) ERROR, - ); + unset($GLOBALS['GLPI_IS_COMMAND_LINE']); $this->updateItem( PluginFieldsField::class, @@ -266,16 +262,13 @@ public function testMailCollectorImportRespectsMandatoryFieldDefaultValue( $tkt = $collector->buildTicket(2, $message, ['mailgates_id' => $collector->getID(), 'play_rules' => false]); $tkt['entities_id'] = 0; - - $ticket = new Ticket(); - $ticket_id = $ticket->add($tkt); - $this->assertGreaterThan(0, $ticket_id, sprintf('Import must succeed once the mandatory %s field has a default value.', $type)); + $ticket = $this->createItem(Ticket::class, $tkt, ['users_id', 'itemtype']); $classname = PluginFieldsContainer::getClassname(Ticket::class, $container->fields['name']); $obj = getItemForItemtype($classname); $obj->getFromDBByCrit([ 'plugin_fields_containers_id' => $container->getID(), - 'items_id' => $ticket_id, + 'items_id' => $ticket->getID(), ]); $container_ticket_fields_value = $obj->fields; $stored_value = $multiple ? json_decode((string) $container_ticket_fields_value[$row_key], true) From 3f82ede5c00dae6efc87ea9eeb34efcb2a0cf563 Mon Sep 17 00:00:00 2001 From: MyuTsu Date: Mon, 28 Sep 2026 10:39:44 +0200 Subject: [PATCH 2/3] review --- inc/container.class.php | 23 ++++++++++++++++++++--- tests/Units/ContainerItemUpdateTest.php | 12 ++++-------- tests/Units/ContainerTest.php | 8 ++++---- 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/inc/container.class.php b/inc/container.class.php index eee45fc5..45afb6ed 100644 --- a/inc/container.class.php +++ b/inc/container.class.php @@ -1653,6 +1653,15 @@ public static function constructHistory( } } + private static function isMandatoryCheckBypassed(array $data): bool + { + return isCommandLine() + || Session::isCron() + || isAPI() + || !empty($data['_auto_import']) + || !empty($data['is_dynamic']); + } + /** * check data inserted * display a message when not ok @@ -1740,7 +1749,7 @@ public static function validateValues($data, $itemtype, $massiveaction) $field['label'] = PluginFieldsLabelTranslation::getLabelFor($field); if ( - !isCommandLine() && !Session::isCron() && !($data['_auto_import'] ?? false) + !self::isMandatoryCheckBypassed($data) && $field['mandatory'] == 1 && ( empty($value) @@ -2032,6 +2041,10 @@ private static function checkContainerMandatory(CommonDBTM $item, PluginFieldsCo $data['_auto_import'] = true; } + if (!empty($item->input['is_dynamic'])) { + $data['is_dynamic'] = true; + } + if (array_key_exists($status_field_name, $item->input) && $item->input[$status_field_name] !== '') { $data[$status_field_name] = (int) $item->input[$status_field_name]; } elseif (array_key_exists($status_field_name, $item->fields) && $item->fields[$status_field_name] !== '') { @@ -2098,12 +2111,16 @@ private static function populateData($c_id, CommonDBTM $item) $data['items_id'] = $item->getID(); } - // Carry over the "automated import" marker so mandatory fields can be relaxed + // Carry over the "automated import" markers so mandatory fields can be relaxed // for items created without a human filling a form. - if ($item->input['_auto_import'] ?? false) { + if (!empty($item->input['_auto_import'])) { $data['_auto_import'] = true; } + if (!empty($item->input['is_dynamic'])) { + $data['is_dynamic'] = true; + } + // Add status so it can be used with status overrides $status_field_name = PluginFieldsStatusOverride::getStatusFieldName($item->getType()); $data[$status_field_name] = null; diff --git a/tests/Units/ContainerItemUpdateTest.php b/tests/Units/ContainerItemUpdateTest.php index eb8d2085..6c019e97 100644 --- a/tests/Units/ContainerItemUpdateTest.php +++ b/tests/Units/ContainerItemUpdateTest.php @@ -65,10 +65,14 @@ final class ContainerItemUpdateTest extends DbTestCase public function setUp(): void { GLPITestCase::setUp(); + + $GLOBALS['GLPI_IS_COMMAND_LINE'] = false; } public function tearDown(): void { + unset($GLOBALS['GLPI_IS_COMMAND_LINE']); + global $DB; $DB->setMustUnsanitizeData(false); // Be sure to switch back to disabled unsanitization. @@ -340,8 +344,6 @@ public function testCreateIsBlockedWhenMandatoryDomFieldIsMissing(): void $this->simulateApiBoot(); - $GLOBALS['GLPI_IS_COMMAND_LINE'] = false; - // Creation with the mandatory field omitted must be rejected. $ticket = new Ticket(); $ticket_id = $ticket->add([ @@ -355,8 +357,6 @@ public function testCreateIsBlockedWhenMandatoryDomFieldIsMissing(): void ERROR, ); - unset($GLOBALS['GLPI_IS_COMMAND_LINE']); - // Creation with the mandatory field filled must succeed. $ticket = new Ticket(); $ticket_id = $ticket->add([ @@ -472,16 +472,12 @@ public function testUpdateIsBlockedWhenMandatoryTabFieldWasNeverFilled(): void 'entities_id' => 0, ]); - $GLOBALS['GLPI_IS_COMMAND_LINE'] = false; - // Update the main form only, without ever opening the Tab. $updated = $problem->update([ 'id' => $problem->getID(), 'name' => 'Renamed while tab field still empty', ]); - unset($GLOBALS['GLPI_IS_COMMAND_LINE']); - $this->assertFalse($updated, 'Update must be blocked while a mandatory tab field is empty.'); $this->hasSessionMessageThatContains( __('Some mandatory fields are empty', 'fields'), diff --git a/tests/Units/ContainerTest.php b/tests/Units/ContainerTest.php index c269b363..b00c6dae 100644 --- a/tests/Units/ContainerTest.php +++ b/tests/Units/ContainerTest.php @@ -56,10 +56,14 @@ public function setUp(): void { GLPITestCase::setUp(); $this->login(); + + $GLOBALS['GLPI_IS_COMMAND_LINE'] = false; } public function tearDown(): void { + unset($GLOBALS['GLPI_IS_COMMAND_LINE']); + $this->tearDownFieldTest(); GLPITestCase::tearDown(); } @@ -245,14 +249,10 @@ public function testMailCollectorImportRespectsMandatoryFieldDefaultValue( 'content' => 'This is a test email imported via the mail collector.', ]); - $GLOBALS['GLPI_IS_COMMAND_LINE'] = false; - $tkt = $collector->buildTicket(1, $message, ['mailgates_id' => $collector->getID(), 'play_rules' => false]); $tkt['entities_id'] = 0; $this->createItem(Ticket::class, $tkt, ['users_id', 'itemtype']); - unset($GLOBALS['GLPI_IS_COMMAND_LINE']); - $this->updateItem( PluginFieldsField::class, $field->getID(), From d229b6901e4b8a7a5ef5e0c1710b90b895a46b1f Mon Sep 17 00:00:00 2001 From: MyuTsu Date: Tue, 29 Sep 2026 12:05:18 +0200 Subject: [PATCH 3/3] add tests --- tests/Units/ContainerItemUpdateTest.php | 78 ++++++++++++++++++++++--- 1 file changed, 70 insertions(+), 8 deletions(-) diff --git a/tests/Units/ContainerItemUpdateTest.php b/tests/Units/ContainerItemUpdateTest.php index 6c019e97..760ce33e 100644 --- a/tests/Units/ContainerItemUpdateTest.php +++ b/tests/Units/ContainerItemUpdateTest.php @@ -30,6 +30,7 @@ namespace GlpiPlugin\Field\Tests\Units; +use Computer; use Glpi\Tests\DbTestCase; use Glpi\Tests\GLPITestCase; use GlpiPlugin\Field\Tests\FieldTestTrait; @@ -304,18 +305,16 @@ public function testCreateTicketInApiLikeContext(): void $this->simulateApiBoot(); - $ticket = new Ticket(); - $ticket_id = $ticket->add([ - 'name' => 'API created ticket', + $ticket = $this->createItem(Ticket::class, [ + 'name' => 'Ticket created via API', 'content' => 'Test creation', 'entities_id' => 0, - $field_name => 'created via api', - ]); - $this->assertGreaterThan(0, $ticket_id); + $field_name => 'api create value', + ], [$field_name]); - $plugin_row = $this->getPluginFieldValues(Ticket::class, $ticket_id, $container->getID()); + $plugin_row = $this->getPluginFieldValues(Ticket::class, $ticket->getID(), $container->getID()); $this->assertNotFalse($plugin_row, 'Plugin fields row must exist after API-like creation.'); - $this->assertSame('created via api', $plugin_row[$field_name]); + $this->assertSame('api create value', $plugin_row[$field_name]); } public function testCreateIsBlockedWhenMandatoryDomFieldIsMissing(): void @@ -372,6 +371,69 @@ public function testCreateIsBlockedWhenMandatoryDomFieldIsMissing(): void $this->assertSame('filled value', $plugin_row[$field_name]); } + public function testCreateIsNotBlockedForInventoryCreatedItem(): void + { + $this->login(); + + $container = $this->createFieldContainer([ + 'label' => 'Mandatory Inventory Container', + 'type' => 'dom', + 'itemtypes' => [Computer::class], + 'is_active' => 1, + 'entities_id' => 0, + 'is_recursive' => 1, + ]); + $this->createField([ + 'label' => 'Mandatory Field', + 'type' => 'text', + PluginFieldsContainer::getForeignKeyField() => $container->getID(), + 'ranking' => 1, + 'is_active' => 1, + 'is_readonly' => 0, + 'mandatory' => 1, + ]); + + $this->createItem(Computer::class, [ + 'name' => 'Computer created by the inventory agent', + 'entities_id' => 0, + 'is_dynamic' => 1, + ], ['is_dynamic']); + } + + public function testCreateIsNotBlockedInApiContext(): void + { + $this->login(); + + $container = $this->createFieldContainer([ + 'label' => 'Mandatory Api Container', + 'type' => 'dom', + 'itemtypes' => [Ticket::class], + 'is_active' => 1, + 'entities_id' => 0, + 'is_recursive' => 1, + ]); + $this->createField([ + 'label' => 'Mandatory Field', + 'type' => 'text', + PluginFieldsContainer::getForeignKeyField() => $container->getID(), + 'ranking' => 1, + 'is_active' => 1, + 'is_readonly' => 0, + 'mandatory' => 1, + ]); + + $this->simulateApiBoot(); + + $_SERVER['REQUEST_URI'] = '/apirest.php/Ticket'; + $this->assertTrue(isAPI()); + + $this->createItem(Ticket::class, [ + 'name' => 'Ticket created via the REST API', + 'content' => 'Test creation', + 'entities_id' => 0, + ]); + } + public function testCreateIsNotBlockedWhenMandatoryTabOrDomtabFieldIsMissing(): void { $this->login();