From 68607c6e10ba3e73c56bf06eb38b21abae9fcfff Mon Sep 17 00:00:00 2001 From: Samuel Launay <107540223+Lainow@users.noreply.github.com> Date: Mon, 9 Nov 2026 10:22:03 +0100 Subject: [PATCH] BACKPORT-CONFLICT --- CHANGELOG.md | 9 ++ inc/questiontype.class.php | 10 +- tests/Units/FieldQuestionTypeTest.php | 200 ++++++++++++++++++++++++++ 3 files changed, 218 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c281f39..25e17a35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,11 +5,20 @@ 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/). +<<<<<<< HEAD ## [Unreleased] ### Fixed - Remove redundant variable initialization in the question type AJAX controller +======= +## [UNRELEASED] + +### Fixed + +- Fix form submission error when a multiple dropdown field question is left empty +- Fix search criterion value on multiple dropdown fields being reset after a search, which emptied the criterion when sorting or paging results +>>>>>>> be73dd9 (Fix form submission error when a multiple dropdown field question is empty (#1323)) ## [1.25.1] - 2026-10-28 diff --git a/inc/questiontype.class.php b/inc/questiontype.class.php index 72882f23..44ca33e0 100644 --- a/inc/questiontype.class.php +++ b/inc/questiontype.class.php @@ -203,10 +203,18 @@ public function formatRawAnswer(mixed $answer, Question $question): string case 'date': return (string) $answer; case 'dropdown': +<<<<<<< HEAD $answer = self::extractDropdownAnswerIds($answer); +======= + // An empty multiple dropdown is submitted as an empty string + $ids = array_filter( + array_map(intval(...), (array) ($answer['items_id'] ?? [])), + fn(int $id) => $id > 0, + ); +>>>>>>> be73dd9 (Fix form submission error when a multiple dropdown field question is empty (#1323)) $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(int $opt_id) => $itemtype::getById($opt_id)?->fields['name'] ?? '', $ids)); case 'yesno': return $answer ? __('Yes') : __('No'); case 'datetime': diff --git a/tests/Units/FieldQuestionTypeTest.php b/tests/Units/FieldQuestionTypeTest.php index 8014c118..a5f2089e 100644 --- a/tests/Units/FieldQuestionTypeTest.php +++ b/tests/Units/FieldQuestionTypeTest.php @@ -190,6 +190,206 @@ public function testFormatRawAnswerSupportsLegacyItemsIdKey(): void { $this->login(); +<<<<<<< HEAD +======= + // Arrange: create a multiple "dropdown-Location" field + $this->fields['locations'] = $this->createField([ + 'label' => 'Locations', + 'type' => 'dropdown-' . Location::class, + 'multiple' => 1, + 'default_value' => [], + PluginFieldsContainer::getForeignKeyField() => $this->block->getID(), + 'ranking' => 2, + 'is_active' => 1, + ], ['default_value']); + + $location1 = $this->createItem(Location::class, [ + 'name' => __FUNCTION__ . ' 1', + 'entities_id' => $this->getTestRootEntity(true), + ]); + $location2 = $this->createItem(Location::class, [ + 'name' => __FUNCTION__ . ' 2', + 'entities_id' => $this->getTestRootEntity(true), + ]); + + $builder = new FormBuilder("My form"); + $builder->addQuestion( + "Locations question", + PluginFieldsQuestionType::class, + extra_data: json_encode($this->getFieldExtraDataConfig('locations')), + ); + $form = $this->createForm($builder); + $question = Question::getById($this->getQuestionId($form, "Locations question")); + + try { + // Act: format the answer as submitted by the end user template + $formatted = (new PluginFieldsQuestionType())->formatRawAnswer([ + 'itemtype' => Location::class, + 'items_id' => [(string) $location1->getID(), (string) $location2->getID()], + ], $question); + + // Assert: both location names are displayed + $this->assertEquals( + $location1->fields['name'] . ', ' . $location2->fields['name'], + $formatted, + ); + } finally { + // delete locations for another run + $location1->delete($location1->fields, true); + $location2->delete($location2->fields, true); + } + } + + public function testFormatRawAnswerForMultipleDropdown(): void + { + $this->login(); + + // Arrange: create a multiple plugin dropdown field + $this->fields['multiple_dropdown'] = $this->createField([ + 'label' => 'Multiple dropdown', + 'type' => 'dropdown', + 'multiple' => 1, + 'default_value' => [], + PluginFieldsContainer::getForeignKeyField() => $this->block->getID(), + 'ranking' => 2, + 'is_active' => 1, + ], ['default_value']); + + $dropdown_class = PluginFieldsDropdown::getClassname($this->fields['multiple_dropdown']->fields['name']); + $value1 = $this->createItem($dropdown_class, ['name' => 'Value 1', 'entities_id' => 0]); + $value2 = $this->createItem($dropdown_class, ['name' => 'Value 2', 'entities_id' => 0]); + + $builder = new FormBuilder("My form"); + $builder->addQuestion( + "Multiple dropdown question", + PluginFieldsQuestionType::class, + extra_data: json_encode($this->getFieldExtraDataConfig('multiple_dropdown')), + ); + $form = $this->createForm($builder); + $question = Question::getById($this->getQuestionId($form, "Multiple dropdown question")); + $question_type = new PluginFieldsQuestionType(); + + // Act & Assert: an empty selection is submitted as an empty string + $this->assertEquals('', $question_type->formatRawAnswer([ + 'itemtype' => $dropdown_class, + 'items_id' => '', + ], $question)); + + // Act & Assert: selected values are displayed + $this->assertEquals('Value 1, Value 2', $question_type->formatRawAnswer([ + 'itemtype' => $dropdown_class, + 'items_id' => [(string) $value1->getID(), (string) $value2->getID()], + ], $question)); + } + + public function testFieldsQuestionEditorRenderingForMultipleDropdownWithStringKeyedDefaultValue(): void + { + $this->login(); + + // Arrange: create a multiple "dropdown-Location" field + $this->fields['locations'] = $this->createField([ + 'label' => 'Locations', + 'type' => 'dropdown-' . Location::class, + 'multiple' => 1, + 'default_value' => [], + PluginFieldsContainer::getForeignKeyField() => $this->block->getID(), + 'ranking' => 2, + 'is_active' => 1, + ], ['default_value']); + + $location1 = $this->createItem(Location::class, [ + 'name' => __FUNCTION__ . ' 1', + 'entities_id' => $this->getTestRootEntity(true), + ]); + $location2 = $this->createItem(Location::class, [ + 'name' => __FUNCTION__ . ' 2', + 'entities_id' => $this->getTestRootEntity(true), + ]); + + // Default value with string keys, as they would be passed as named parameters if not reindexed + $builder = new FormBuilder("My form"); + $builder->addQuestion( + "Locations question", + PluginFieldsQuestionType::class, + default_value: [ + 'itemtype' => Location::class, + 'items_id' => [ + 'first' => $location1->getID(), + 'second' => $location2->getID(), + ], + ], + extra_data: json_encode($this->getFieldExtraDataConfig('locations')), + ); + $form = $this->createForm($builder); + + try { + // Act: render form editor + $crawler = $this->renderFormEditor($form); + + // Assert: both locations are rendered as the default value + $selected_values = $crawler + ->filter('.form-editor-container [data-glpi-form-editor-question] input[name^="default_value[items_id]"]') + ->each(fn($input) => (int) $input->attr('value')); + $this->assertEqualsCanonicalizing( + [$location1->getID(), $location2->getID()], + $selected_values, + ); + } finally { + // delete locations for another run + $location1->delete($location1->fields, true); + $location2->delete($location2->fields, true); + } + } + + public static function cloneDefaultValueProvider(): iterable + { + yield 'text' => ['text', 'hello']; + yield 'textarea' => ['textarea', "first line\nsecond line"]; + yield 'number' => ['number', '42']; + yield 'url' => ['url', 'https://glpi-project.org']; + yield 'date' => ['date', '2026-10-15']; + yield 'json-like text' => ['text', '[1,2]']; + } + + #[DataProvider('cloneDefaultValueProvider')] + public function testCloneKeepsScalarDefaultValue(string $type, string $default_value): void + { + $this->login(); + + // Arrange: create a form with a question using a scalar field + $this->fields['scalar'] = $this->createField([ + 'label' => 'Scalar field', + 'type' => $type, + PluginFieldsContainer::getForeignKeyField() => $this->block->getID(), + 'ranking' => 2, + 'is_active' => 1, + ]); + $builder = new FormBuilder("My form"); + $builder->addQuestion( + "Scalar question", + PluginFieldsQuestionType::class, + default_value: $default_value, + extra_data: json_encode($this->getFieldExtraDataConfig('scalar')), + ); + $form = $this->createForm($builder); + + // Act: clone the form, then clone the clone + $clone = Form::getById($form->clone()); + $clone_of_clone = Form::getById($clone->clone()); + + // Assert: the default value is encoded only once in each form + foreach ([$form, $clone, $clone_of_clone] as $current_form) { + $question = Question::getById($this->getQuestionId($current_form, "Scalar question")); + $this->assertSame(json_encode($default_value), $question->fields['default_value']); + } + } + + public function testCloneKeepsDropdownDefaultValue(): void + { + $this->login(); + + // Arrange: create a form with a dropdown question having a default value +>>>>>>> be73dd9 (Fix form submission error when a multiple dropdown field question is empty (#1323)) $itemtype = PluginFieldsDropdown::getClassname($this->fields['dropdown']->fields['name']); $item = $this->createItem($itemtype, ['name' => 'Legacy Option']);