From 4ff75ab35faf5a7409e3363a45ba10fc94b7899a Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Mon, 31 Aug 2026 15:13:06 +0200 Subject: [PATCH 1/2] Fix - Filter the '0' empty-selection sentinel in Table and LDAP select questions --- src/Model/QuestionType/LdapQuestion.php | 10 +++++ src/Model/QuestionType/TableQuestion.php | 44 ++++++++++++++++++ tests/Model/QuestionType/LdapQuestionTest.php | 29 ++++++++++++ .../TableQuestionValidationTest.php | 45 +++++++++++++++++++ 4 files changed, 128 insertions(+) diff --git a/src/Model/QuestionType/LdapQuestion.php b/src/Model/QuestionType/LdapQuestion.php index 81d1e1d..11ba636 100644 --- a/src/Model/QuestionType/LdapQuestion.php +++ b/src/Model/QuestionType/LdapQuestion.php @@ -130,6 +130,16 @@ public function getExtraDataConfigClass(): string return LdapQuestionConfig::class; } + #[Override] + public function prepareEndUserAnswer(Question $question, mixed $answer): mixed + { + if ($answer === '0' || $answer === 0) { + return ''; + } + + return $answer; + } + #[Override] public function renderEndUserTemplate(Question|null $question): string { diff --git a/src/Model/QuestionType/TableQuestion.php b/src/Model/QuestionType/TableQuestion.php index d4cc892..cf26c2b 100644 --- a/src/Model/QuestionType/TableQuestion.php +++ b/src/Model/QuestionType/TableQuestion.php @@ -182,6 +182,8 @@ public function prepareEndUserAnswer(Question $question, mixed $answer): mixed return []; } + $answer = $this->normalizeDropdownCells($answer, $question); + // Drop empty rows; required columns are enforced by validateAnswer(). $result = []; foreach ($answer as $row) { @@ -206,6 +208,8 @@ public function validateAnswer(Question $question, mixed $answer): ValidationRes return $result; } + $answer = $this->normalizeDropdownCells($answer, $question); + $row_number = 0; foreach ($answer as $row) { if (!is_array($row)) { @@ -331,6 +335,46 @@ private function rowHasValue(array $row): bool return false; } + /** + * @param array $rows + * @return array + */ + private function normalizeDropdownCells(array $rows, Question $question): array + { + $dropdown_indexes = []; + foreach ($this->loadConfig($question)->getColumns() as $index => $col) { + $fqcn = $col[TableQuestionConfig::COL_QUESTION_TYPE]; + $itemtype = $col[TableQuestionConfig::COL_ITEMTYPE] ?? ''; + if ( + is_a($fqcn, AbstractQuestionTypeActors::class, true) + || (is_a($fqcn, QuestionTypeItem::class, true) && $itemtype !== '' && class_exists($itemtype)) + ) { + $dropdown_indexes[] = $index; + } + } + + if ($dropdown_indexes === []) { + return $rows; + } + + foreach ($rows as &$row) { + if (!is_array($row)) { + continue; + } + + foreach ($dropdown_indexes as $index) { + $key = 'col_' . $index; + if (($row[$key] ?? null) === '0' || ($row[$key] ?? null) === 0) { + $row[$key] = ''; + } + } + } + + unset($row); + + return $rows; + } + #[Override] public function transformConditionValueForComparisons(mixed $value, ?JsonFieldInterface $question_config): string|array { diff --git a/tests/Model/QuestionType/LdapQuestionTest.php b/tests/Model/QuestionType/LdapQuestionTest.php index ef6af35..5d842ab 100644 --- a/tests/Model/QuestionType/LdapQuestionTest.php +++ b/tests/Model/QuestionType/LdapQuestionTest.php @@ -33,7 +33,9 @@ namespace GlpiPlugin\Advancedforms\Tests\Model\QuestionType; +use Glpi\Form\Question; use Glpi\Form\QuestionType\QuestionTypeInterface; +use Glpi\Tests\FormBuilder; use Glpi\Tests\FormTesterTrait; use GlpiPlugin\Advancedforms\Model\Config\ConfigurableItemInterface; use GlpiPlugin\Advancedforms\Model\QuestionType\LdapQuestion; @@ -51,6 +53,33 @@ protected function getTestedQuestionType(): QuestionTypeInterface&ConfigurableIt return new LdapQuestion(); } + /** + * Dropdown::show(), which renders this question's field, defaults an + * unselected value to "0" instead of an empty string. Without a guard, + * that sentinel gets saved and displayed as if it were a real answer. + */ + public function testPrepareEndUserAnswerFiltersEmptySelectionSentinel(): void + { + $type = new LdapQuestion(); + + $this->enableConfigurableItem($type); + $builder = new FormBuilder("My form"); + $builder->addQuestion( + "My question", + LdapQuestion::class, + extra_data: json_encode(['authldap_id' => 1]), + ); + + $form = $this->createForm($builder); + $questions_id = $this->getQuestionId($form, "My question"); + $question = new Question(); + $this->assertTrue($question->getFromDB($questions_id)); + + $this->assertSame('', $type->prepareEndUserAnswer($question, '0')); + $this->assertSame('', $type->prepareEndUserAnswer($question, 0)); + $this->assertSame('jdoe@example.com', $type->prepareEndUserAnswer($question, 'jdoe@example.com')); + } + #[Override] protected function validateEditorRenderingWhenEnabled( Crawler $html, diff --git a/tests/Model/QuestionType/TableQuestionValidationTest.php b/tests/Model/QuestionType/TableQuestionValidationTest.php index 80e7178..abf9fc5 100644 --- a/tests/Model/QuestionType/TableQuestionValidationTest.php +++ b/tests/Model/QuestionType/TableQuestionValidationTest.php @@ -35,6 +35,7 @@ use Glpi\Form\AnswersHandler\AnswersHandler; use Glpi\Form\Question; +use Glpi\Form\QuestionType\QuestionTypeAssignee; use Glpi\Form\QuestionType\QuestionTypeCheckbox; use Glpi\Form\QuestionType\QuestionTypeEmail; use Glpi\Form\QuestionType\QuestionTypeNumber; @@ -767,6 +768,50 @@ public function testCellForAnUnknownColumnIsIgnored(): void $this->assertTrue($result->isValid()); } + public function testRequiredActorColumnWithEmptySentinelProducesError(): void + { + $question = $this->makeTableQuestion([ + $this->column('Assignee', QuestionTypeAssignee::class, required: true), + $this->column('Comment', QuestionTypeShortText::class, required: false), + ]); + + $result = $this->type->validateAnswer($question, [ + ['col_0' => '0', 'col_1' => 'a comment'], + ]); + + $this->assertFalse($result->isValid()); + $this->assertCount(1, $result->getErrors()); + } + + public function testPrepareEndUserAnswerDropsRowWhoseOnlyValueIsActorEmptySentinel(): void + { + $question = $this->makeTableQuestion([ + $this->column('Assignee', QuestionTypeAssignee::class, required: false), + ]); + + $result = $this->type->prepareEndUserAnswer($question, [ + ['col_0' => '0'], + ]); + + $this->assertSame([], $result); + } + + public function testPrepareEndUserAnswerNormalizesActorEmptySentinelToEmptyString(): void + { + $question = $this->makeTableQuestion([ + $this->column('Assignee', QuestionTypeAssignee::class, required: false), + $this->column('Comment', QuestionTypeShortText::class, required: false), + ]); + + $result = $this->type->prepareEndUserAnswer($question, [ + ['col_0' => '0', 'col_1' => 'a comment'], + ]); + + $this->assertSame([ + ['col_0' => '', 'col_1' => 'a comment'], + ], $result); + } + /** * @param array $columns */ From 4aae5b7a39dbd39acfb8b36a6986d6e1b1652398 Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Mon, 31 Aug 2026 15:13:46 +0200 Subject: [PATCH 2/2] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 29e2f87..825e679 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed +- Filter the '0' empty-selection sentinel in Table and LDAP select questions - Fix Table question column type edge cases ## [1.3.0] - 2026-08-11