From b0b8c7363a375020ecc80a95d946c0d4b34af41e Mon Sep 17 00:00:00 2001 From: Lainow Date: Wed, 14 Oct 2026 13:52:11 +0200 Subject: [PATCH 1/3] Fix rendering error 'unknown named parameters' for duplicated form --- CHANGELOG.md | 1 + inc/field.class.php | 2 +- inc/questiontype.class.php | 35 +++++++++++++++++++++++++++++------ 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a45cf583..c96ff463 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed - Fix mandatory fields of a block hidden by its display conditions blocking the save of an item +- Fix rendering error `unknown named parameters` for duplicated form ## [1.24.6] - 2026-10-06 diff --git a/inc/field.class.php b/inc/field.class.php index c7ab2bce..252be560 100644 --- a/inc/field.class.php +++ b/inc/field.class.php @@ -1357,7 +1357,7 @@ public static function prepareHtmlFields( $value = array_values(array_filter( array_merge(...array_map( static fn($v) => is_array($v) ? array_values($v) : [$v], - $value, + array_values($value), )), is_scalar(...), )); diff --git a/inc/questiontype.class.php b/inc/questiontype.class.php index 919d1d87..da2985cf 100644 --- a/inc/questiontype.class.php +++ b/inc/questiontype.class.php @@ -82,6 +82,11 @@ public function getSubTypeDefaultValue(?Question $question): string #[Override] public function formatDefaultValueForDB(mixed $value): string { + // For cloned questions, the value may already be encoded, do not re-encode it. + if (is_string($value) && is_array(\json_decode($value, true))) { + return $value; + } + return json_encode($value); } @@ -130,8 +135,8 @@ public function renderAdministrationTemplate(?Question $question): string // Compute default value for the field $default_value = null; - if ($question instanceof Question && !empty($question->fields['default_value'])) { - $default_value = json_decode($question->fields['default_value'], true); + if ($question instanceof Question) { + $default_value = $this->decodeDefaultValue($question); } $twig = TemplateRenderer::getInstance(); @@ -145,6 +150,27 @@ public function renderAdministrationTemplate(?Question $question): string ]); } + /** + * Decode the question default value. + * Handles values that were JSON encoded twice by previous form clones. + */ + private function decodeDefaultValue(Question $question): mixed + { + if (empty($question->fields['default_value'])) { + return null; + } + + $default_value = json_decode($question->fields['default_value'], true); + if (is_string($default_value)) { + $decoded = \json_decode($default_value, true); + if (is_array($decoded)) { + $default_value = $decoded; + } + } + + return $default_value; + } + #[Override] public function renderEndUserTemplate(Question $question): string { @@ -165,10 +191,7 @@ public function renderEndUserTemplate(Question $question): string $current_field = PluginFieldsField::getById($current_field_id); // Compute default value for the field - $default_value = null; - if (!empty($question->fields['default_value'])) { - $default_value = json_decode($question->fields['default_value'], true); - } + $default_value = $this->decodeDefaultValue($question); $itemtype = null; if (str_starts_with((string) $current_field->fields['type'], 'dropdown')) { From 7ae80007dea4477696bf2abdfe9a03f2b88d5d84 Mon Sep 17 00:00:00 2001 From: Lainow <107540223+Lainow@users.noreply.github.com> Date: Thu, 15 Oct 2026 17:10:55 +0200 Subject: [PATCH 2/3] Review --- hook.php | 1 + inc/questiontype.class.php | 94 ++++++++++++++---- setup.php | 5 + tests/Units/FieldQuestionTypeTest.php | 133 ++++++++++++++++++++++++++ 4 files changed, 216 insertions(+), 17 deletions(-) diff --git a/hook.php b/hook.php index d9cbb497..bc3a3526 100644 --- a/hook.php +++ b/hook.php @@ -71,6 +71,7 @@ function plugin_fields_install() PluginFieldsField::class, PluginFieldsLabelTranslation::class, PluginFieldsProfile::class, + PluginFieldsQuestionType::class, PluginFieldsStatusOverride::class, ]; diff --git a/inc/questiontype.class.php b/inc/questiontype.class.php index da2985cf..9bb49f67 100644 --- a/inc/questiontype.class.php +++ b/inc/questiontype.class.php @@ -37,6 +37,7 @@ use Glpi\Form\Question; use Glpi\Form\QuestionType\AbstractQuestionType; use Glpi\Form\QuestionType\QuestionTypeCategoryInterface; +use Safe\Exceptions\JsonException; use function Safe\json_decode; use function Safe\json_encode; @@ -82,12 +83,83 @@ public function getSubTypeDefaultValue(?Question $question): string #[Override] public function formatDefaultValueForDB(mixed $value): string { - // For cloned questions, the value may already be encoded, do not re-encode it. - if (is_string($value) && is_array(\json_decode($value, true))) { - return $value; + return json_encode($value); + } + + /** + * Decode the stored default value of a cloned question, as it is passed + * again to `formatDefaultValueForDB()` when the clone is added. + */ + public static function preQuestionAdd(Question $question): void + { + if ( + !($question->input['clone'] ?? false) + || ($question->input['type'] ?? null) !== self::class + || !is_string($question->input['default_value'] ?? null) + ) { + return; } - return json_encode($value); + try { + $question->input['default_value'] = json_decode($question->input['default_value'], true); + } catch (JsonException) { + // Not a JSON value, keep it as is + } + } + + /** + * Repair default values that were JSON encoded several times by previous form clones. + */ + public static function installBaseData(Migration $migration, $version): void + { + /** @var DBmysql $DB */ + global $DB; + + $question_table = Question::getTable(); + if (!$DB->tableExists($question_table)) { + return; + } + + $migration->displayMessage(__('Repair default values of cloned form questions', 'fields')); + + $questions = $DB->request([ + 'SELECT' => ['id', 'default_value', 'extra_data'], + 'FROM' => $question_table, + 'WHERE' => ['type' => self::class], + ]); + foreach ($questions as $question) { + if (!is_string($question['default_value']) || !is_string($question['extra_data'])) { + continue; + } + + $extra_data = \json_decode($question['extra_data'], true); + $field = new PluginFieldsField(); + if (!is_array($extra_data) || !$field->getFromDB((int) ($extra_data['field_id'] ?? 0))) { + continue; + } + + // Dropdowns and GLPI items store an array, other fields store a scalar value + $is_array_value = $field->fields['type'] === 'glpi_item' + || str_starts_with((string) $field->fields['type'], 'dropdown'); + + $default_value = $question['default_value']; + while (is_string($decoded = \json_decode($default_value, true))) { + // Unwrap while the value is still a JSON encoded string (or array for array values) + $inner = \json_decode($decoded, true); + if (!is_string($inner) && !($is_array_value && is_array($inner))) { + break; + } + $default_value = $decoded; + } + + if ($default_value !== $question['default_value']) { + $DB->update( + $question_table, + ['default_value' => $default_value], + ['id' => $question['id']], + ); + } + } } #[Override] @@ -150,25 +222,13 @@ public function renderAdministrationTemplate(?Question $question): string ]); } - /** - * Decode the question default value. - * Handles values that were JSON encoded twice by previous form clones. - */ private function decodeDefaultValue(Question $question): mixed { if (empty($question->fields['default_value'])) { return null; } - $default_value = json_decode($question->fields['default_value'], true); - if (is_string($default_value)) { - $decoded = \json_decode($default_value, true); - if (is_array($decoded)) { - $default_value = $decoded; - } - } - - return $default_value; + return json_decode($question->fields['default_value'], true); } #[Override] diff --git a/setup.php b/setup.php index 5ed4c552..d467557b 100644 --- a/setup.php +++ b/setup.php @@ -75,6 +75,7 @@ use Glpi\Form\Destination\FormDestinationProblem; use Glpi\Form\Destination\FormDestinationTicket; use Glpi\Form\Migration\TypesConversionMapper; +use Glpi\Form\Question; use Glpi\Form\QuestionType\QuestionTypesManager; use Glpi\Plugin\Hooks; use Symfony\Component\Yaml\Yaml; @@ -134,6 +135,10 @@ function plugin_init_fields() // Register fields question type plugin_fields_register_plugin_types(); + $PLUGIN_HOOKS['pre_item_add']['fields'][Question::class] = [ + PluginFieldsQuestionType::class, + 'preQuestionAdd', + ]; if (Session::getLoginUserID() || isCommandLine()) { // Init hook about itemtype(s) for plugin fields diff --git a/tests/Units/FieldQuestionTypeTest.php b/tests/Units/FieldQuestionTypeTest.php index 08a4c6c5..0cc0d637 100644 --- a/tests/Units/FieldQuestionTypeTest.php +++ b/tests/Units/FieldQuestionTypeTest.php @@ -30,6 +30,7 @@ namespace GlpiPlugin\Field\Tests\Units; +use DBmysql; use Glpi\Form\Condition\ConditionHandler\ItemAsTextConditionHandler; use Glpi\Form\Condition\ConditionHandler\ItemConditionHandler; use Glpi\Form\Condition\Engine; @@ -38,6 +39,7 @@ use Glpi\Form\Condition\Type; use Glpi\Form\Condition\ValueOperator; use Glpi\Form\Condition\VisibilityStrategy; +use Glpi\Form\Form; use Glpi\Form\Question; use Glpi\Form\QuestionType\QuestionTypeShortText; use Glpi\Form\QuestionType\QuestionTypesManager; @@ -45,6 +47,8 @@ use GlpiPlugin\Field\Tests\QuestionTypeTestCase; use Location; use LogicException; +use Migration; +use PHPUnit\Framework\Attributes\DataProvider; use PluginFieldsContainer; use PluginFieldsDropdown; use PluginFieldsField; @@ -52,6 +56,7 @@ use PluginFieldsQuestionTypeCategory; use PluginFieldsQuestionTypeExtraDataConfig; +use function Safe\json_decode; use function Safe\json_encode; final class FieldQuestionTypeTest extends QuestionTypeTestCase @@ -242,6 +247,134 @@ public function testFormatRawAnswerForMultipleGlpiItemDropdown(): void } } + 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 + $itemtype = PluginFieldsDropdown::getClassname($this->fields['dropdown']->fields['name']); + $option = $this->createItem($itemtype, ['name' => 'Default option']); + $default_value = ['itemtype' => $itemtype, 'items_id' => $option->getID()]; + + $builder = new FormBuilder("My form"); + $builder->addQuestion( + "Dropdown question", + PluginFieldsQuestionType::class, + default_value: $default_value, + extra_data: json_encode($this->getFieldExtraDataConfig('dropdown')), + ); + $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 kept as an array in each form + foreach ([$form, $clone, $clone_of_clone] as $current_form) { + $question = Question::getById($this->getQuestionId($current_form, "Dropdown question")); + $this->assertSame($default_value, json_decode($question->fields['default_value'], true)); + } + } + + public function testInstallRepairsDefaultValuesEncodedByClones(): void + { + /** @var DBmysql $DB */ + global $DB; + + $this->login(); + + // Arrange: create questions whose default values were encoded by successive clones + $this->fields['text'] = $this->createField([ + 'label' => 'Text field', + 'type' => 'text', + PluginFieldsContainer::getForeignKeyField() => $this->block->getID(), + 'ranking' => 2, + 'is_active' => 1, + ]); + $itemtype = PluginFieldsDropdown::getClassname($this->fields['dropdown']->fields['name']); + $dropdown_value = ['itemtype' => $itemtype, 'items_id' => 1]; + + $cases = [ + // question name => [field, stored default value, expected default value] + 'Text cloned twice' => ['text', json_encode(json_encode(json_encode('hello'))), json_encode('hello')], + 'Text not cloned' => ['text', json_encode('hello'), json_encode('hello')], + 'JSON-like text' => ['text', json_encode('[1,2]'), json_encode('[1,2]')], + 'Dropdown cloned twice' => ['dropdown', json_encode(json_encode(json_encode($dropdown_value))), json_encode($dropdown_value)], + 'Dropdown not cloned' => ['dropdown', json_encode($dropdown_value), json_encode($dropdown_value)], + ]; + + $builder = new FormBuilder("My form"); + foreach ($cases as $name => [$field_name]) { + $builder->addQuestion( + $name, + PluginFieldsQuestionType::class, + extra_data: json_encode($this->getFieldExtraDataConfig($field_name)), + ); + } + $form = $this->createForm($builder); + foreach ($cases as $name => [, $stored_value]) { + $DB->update( + Question::getTable(), + ['default_value' => $stored_value], + ['id' => $this->getQuestionId($form, $name)], + ); + } + + // Act: run the plugin migration + ob_start(); + PluginFieldsQuestionType::installBaseData(new Migration(PLUGIN_FIELDS_VERSION), PLUGIN_FIELDS_VERSION); + ob_end_clean(); + + // Assert: default values are encoded only once + foreach ($cases as $name => [, , $expected_value]) { + $question = Question::getById($this->getQuestionId($form, $name)); + $this->assertSame($expected_value, $question->fields['default_value'], $name); + } + } + public function testFieldDeletionWhenUsedInForm(): void { $this->login(); From 661971cd32fc5436b8b81dfa850084c83219fa81 Mon Sep 17 00:00:00 2001 From: Lainow <107540223+Lainow@users.noreply.github.com> Date: Thu, 15 Oct 2026 17:36:04 +0200 Subject: [PATCH 3/3] Rector --- inc/questiontype.class.php | 3 ++- setup.php | 5 +---- tests/Units/FieldQuestionTypeTest.php | 1 + 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/inc/questiontype.class.php b/inc/questiontype.class.php index 9bb49f67..99697e92 100644 --- a/inc/questiontype.class.php +++ b/inc/questiontype.class.php @@ -146,9 +146,10 @@ public static function installBaseData(Migration $migration, $version): void while (is_string($decoded = \json_decode($default_value, true))) { // Unwrap while the value is still a JSON encoded string (or array for array values) $inner = \json_decode($decoded, true); - if (!is_string($inner) && !($is_array_value && is_array($inner))) { + if (!is_string($inner) && (!$is_array_value || !is_array($inner))) { break; } + $default_value = $decoded; } diff --git a/setup.php b/setup.php index d467557b..9c4f7464 100644 --- a/setup.php +++ b/setup.php @@ -135,10 +135,7 @@ function plugin_init_fields() // Register fields question type plugin_fields_register_plugin_types(); - $PLUGIN_HOOKS['pre_item_add']['fields'][Question::class] = [ - PluginFieldsQuestionType::class, - 'preQuestionAdd', - ]; + $PLUGIN_HOOKS['pre_item_add']['fields'][Question::class] = PluginFieldsQuestionType::preQuestionAdd(...); if (Session::getLoginUserID() || isCommandLine()) { // Init hook about itemtype(s) for plugin fields diff --git a/tests/Units/FieldQuestionTypeTest.php b/tests/Units/FieldQuestionTypeTest.php index 0cc0d637..e10636a3 100644 --- a/tests/Units/FieldQuestionTypeTest.php +++ b/tests/Units/FieldQuestionTypeTest.php @@ -354,6 +354,7 @@ public function testInstallRepairsDefaultValuesEncodedByClones(): void extra_data: json_encode($this->getFieldExtraDataConfig($field_name)), ); } + $form = $this->createForm($builder); foreach ($cases as $name => [, $stored_value]) { $DB->update(