Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions src/Model/QuestionType/LdapQuestion.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
44 changes: 44 additions & 0 deletions src/Model/QuestionType/TableQuestion.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)) {
Expand Down Expand Up @@ -331,6 +335,46 @@ private function rowHasValue(array $row): bool
return false;
}

/**
* @param array<array-key, mixed> $rows
* @return array<array-key, mixed>
*/
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
{
Expand Down
29 changes: 29 additions & 0 deletions tests/Model/QuestionType/LdapQuestionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand Down
45 changes: 45 additions & 0 deletions tests/Model/QuestionType/TableQuestionValidationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<array{name: string, question_type: string, required: bool, itemtype: string}> $columns
*/
Expand Down