From 5d494f7faa43b4061f05505305b323e5c6f97cb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Cailly?= Date: Mon, 30 Mar 2026 14:55:24 +0200 Subject: [PATCH 1/3] fix(questiontype): handle potential null values in dropdown options --- CHANGELOG.md | 6 +++ inc/questiontype.class.php | 2 +- setup.php | 2 +- tests/FieldTestCase.php | 8 ++++ tests/QuestionTypeTestCase.php | 15 ++++++- .../Units/FieldQuestionTypeMigrationTest.php | 1 + tests/Units/FieldQuestionTypeTest.php | 39 ++++++++++++++++--- 7 files changed, 64 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ca4765d5..99114eea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## [1.23.4] - Unreleased + +### Fixed + +- Fix error when submitting a form with an hidden question of type `Field` + ## [1.23.3] - 2026-02-12 ### Added diff --git a/inc/questiontype.class.php b/inc/questiontype.class.php index a4558832..24b28f30 100644 --- a/inc/questiontype.class.php +++ b/inc/questiontype.class.php @@ -201,7 +201,7 @@ public function formatRawAnswer(mixed $answer, Question $question): string } $itemtype = PluginFieldsDropdown::getClassname($current_field->fields['name']); - return implode(', ', array_map(fn($opt_id) => $itemtype::getById($opt_id)->fields['name'], $answer)); + return implode(', ', array_map(fn($opt_id) => $itemtype::getById($opt_id)?->fields['name'] ?? '', $answer)); case 'yesno': return $answer ? __('Yes') : __('No'); case 'datetime': diff --git a/setup.php b/setup.php index e97c2e00..1a813e71 100644 --- a/setup.php +++ b/setup.php @@ -220,7 +220,7 @@ function plugin_init_fields() */ function plugin_fields_script_endswith($scriptname) { - return str_contains((string) $_SERVER['REQUEST_URI'], $scriptname); + return str_contains((string) ($_SERVER['REQUEST_URI'] ?? ''), $scriptname); } diff --git a/tests/FieldTestCase.php b/tests/FieldTestCase.php index 01e6ab07..0cb8e04d 100644 --- a/tests/FieldTestCase.php +++ b/tests/FieldTestCase.php @@ -31,8 +31,10 @@ namespace GlpiPlugin\Field\Tests; use DBmysql; +use Glpi\Form\Question; use PluginFieldsContainer; use PluginFieldsField; +use PluginFieldsQuestionType; trait FieldTestTrait { @@ -47,6 +49,12 @@ public function tearDownFieldTest(): void // Re-login to ensure we are logged in $this->login(); + // Clean all questions that use QuestionTypeField to avoid issues with foreign key constraints when deleting fields + array_map( + fn(Question $question) => $question->delete($question->fields, true), + iterator_to_array(Question::getSeveralFromDBByCrit(['type' => PluginFieldsQuestionType::class])), + ); + // Clean created containers array_map( fn(PluginFieldsContainer $container) => $container->delete($container->fields, true), diff --git a/tests/QuestionTypeTestCase.php b/tests/QuestionTypeTestCase.php index 87a8ee80..a4145911 100644 --- a/tests/QuestionTypeTestCase.php +++ b/tests/QuestionTypeTestCase.php @@ -48,7 +48,8 @@ abstract class QuestionTypeTestCase extends DbTestCase protected ?PluginFieldsContainer $block = null; - protected ?PluginFieldsField $field = null; + /** @var PluginFieldsField[] */ + protected array $fields = []; public function createFieldAndContainer(): void { @@ -61,23 +62,33 @@ public function createFieldAndContainer(): void 'entities_id' => $this->getTestRootEntity(true), ]); - $this->field = $this->createField([ + $this->fields['glpi_item'] = $this->createField([ 'label' => 'GLPI Item', 'type' => 'glpi_item', PluginFieldsContainer::getForeignKeyField() => $this->block->getID(), 'ranking' => 1, 'is_active' => 1, ]); + + $this->fields['dropdown'] = $this->createField([ + 'label' => 'Dropdown', + 'type' => 'dropdown', + PluginFieldsContainer::getForeignKeyField() => $this->block->getID(), + 'ranking' => 1, + 'is_active' => 1, + ]); } public function setUp(): void { $this->createFieldAndContainer(); + parent::setUp(); } public function tearDown(): void { $this->tearDownFieldTest(); + parent::tearDown(); } protected function renderFormEditor(Form $form): Crawler diff --git a/tests/Units/FieldQuestionTypeMigrationTest.php b/tests/Units/FieldQuestionTypeMigrationTest.php index 8f0b21ab..43bcaf08 100644 --- a/tests/Units/FieldQuestionTypeMigrationTest.php +++ b/tests/Units/FieldQuestionTypeMigrationTest.php @@ -39,6 +39,7 @@ final class FieldQuestionTypeMigrationTest extends QuestionTypeTestCase { + public $field; public static function setUpBeforeClass(): void { global $DB; diff --git a/tests/Units/FieldQuestionTypeTest.php b/tests/Units/FieldQuestionTypeTest.php index 8ccfb06a..b9779842 100644 --- a/tests/Units/FieldQuestionTypeTest.php +++ b/tests/Units/FieldQuestionTypeTest.php @@ -35,6 +35,7 @@ use GlpiPlugin\Field\Tests\QuestionTypeTestCase; use LogicException; use PluginFieldsContainer; +use PluginFieldsDropdown; use PluginFieldsField; use PluginFieldsQuestionType; use PluginFieldsQuestionTypeCategory; @@ -117,7 +118,7 @@ public function testFieldsQuestionEditorRendering(): void $builder->addQuestion( "My question", PluginFieldsQuestionType::class, - extra_data: json_encode($this->getFieldExtraDataConfig()), + extra_data: json_encode($this->getFieldExtraDataConfig('glpi_item')), ); $form = $this->createForm($builder); @@ -137,7 +138,7 @@ public function testFieldsQuestionHelpdeskRendering(): void $builder->addQuestion( "My question", PluginFieldsQuestionType::class, - extra_data: json_encode($this->getFieldExtraDataConfig()), + extra_data: json_encode($this->getFieldExtraDataConfig('glpi_item')), ); $form = $this->createForm($builder); @@ -148,12 +149,40 @@ public function testFieldsQuestionHelpdeskRendering(): void $this->assertNotEmpty($crawler->filter('[data-glpi-form-renderer-fields-question-type-specific-container]')); } - private function getFieldExtraDataConfig(): PluginFieldsQuestionTypeExtraDataConfig + public function testFieldsQuestionSubmitEmptyDropdown(): void { - if (!$this->block instanceof PluginFieldsContainer || !$this->field instanceof PluginFieldsField) { + $this->login(); + + /** @var CommonDBTM $dropdown_item */ + $dropdown_item = getItemForItemtype(PluginFieldsDropdown::getClassname($this->fields['dropdown']->fields['name'])); + $dropdown_ids = []; + for ($i = 1; $i <= 3; $i++) { + $dropdown_ids[] = $dropdown_item->add([ + 'name' => 'Option ' . $i, + ]); + } + + // Arrange: create form with Field question + $builder = new FormBuilder("My form"); + $builder->addQuestion( + "Dropdown field question", + PluginFieldsQuestionType::class, + extra_data: json_encode($this->getFieldExtraDataConfig('dropdown')), + ); + $form = $this->createForm($builder); + + // Act: submit form + $this->sendFormAndGetCreatedTicket($form, [ + "Dropdown field question" => '0', + ]); + } + + private function getFieldExtraDataConfig(string $field_name): PluginFieldsQuestionTypeExtraDataConfig + { + if (!$this->block instanceof PluginFieldsContainer || !$this->fields[$field_name] instanceof PluginFieldsField) { throw new LogicException("Field and container must be created before getting extra data config"); } - return new PluginFieldsQuestionTypeExtraDataConfig($this->block->getID(), $this->field->getID()); + return new PluginFieldsQuestionTypeExtraDataConfig($this->block->getID(), $this->fields[$field_name]->getID()); } } From e7747296ec30b254ad322d969984b2065d156fca Mon Sep 17 00:00:00 2001 From: "Romain B." <8530352+Rom1-B@users.noreply.github.com> Date: Wed, 1 Apr 2026 11:47:18 +0200 Subject: [PATCH 2/3] Apply suggestion from @Rom1-B --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97654e13..ec3bf208 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). -## [1.23.5] - Unreleased +## [Unreleased] ### Fixed From 02ec6b4f542f008ca91483d8ab338244a69ecba7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Cailly?= Date: Wed, 1 Apr 2026 13:33:56 +0200 Subject: [PATCH 3/3] fix: rector lint --- inc/field.class.php | 2 +- inc/inventory.class.php | 2 +- inc/migration.class.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/inc/field.class.php b/inc/field.class.php index 63d2218d..9a0da95b 100644 --- a/inc/field.class.php +++ b/inc/field.class.php @@ -973,7 +973,7 @@ public static function showForTab($params) $itemtypes = PluginFieldsContainer::getUsedItemtypes($type, true); //if no dom containers defined for this itemtype, do nothing (in_array case insensitive) - if (!in_array(strtolower((string) $item::getType()), array_map(strtolower(...), $itemtypes))) { + if (!in_array(strtolower((string) $item::getType()), array_map(strtolower(...), $itemtypes), true)) { return; } diff --git a/inc/inventory.class.php b/inc/inventory.class.php index 3c119ed8..14cdbc8d 100644 --- a/inc/inventory.class.php +++ b/inc/inventory.class.php @@ -38,7 +38,7 @@ public static function updateInventory($params = []) ) { $availaibleItemType = ['Computer', 'Printer', 'NetworkEquipment']; foreach (array_keys($params['inventory_data']) as $itemtype) { - if (in_array($itemtype, $availaibleItemType)) { + if (in_array($itemtype, $availaibleItemType, true)) { $items_id = 0; //retrieve items id switch itemtype switch ($itemtype) { diff --git a/inc/migration.class.php b/inc/migration.class.php index 5b7dffc8..0e970ee0 100644 --- a/inc/migration.class.php +++ b/inc/migration.class.php @@ -197,7 +197,7 @@ private static function getCustomFieldsInContainerTable( return array_filter( $fields, - fn(string $field) => !in_array($field, $basic_fields), + fn(string $field) => !in_array($field, $basic_fields, true), ); } }