diff --git a/CHANGELOG.md b/CHANGELOG.md index 8111ba9..e3cf1ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed +- Fixed Custom Dropdown filtering in table questions to only display values from the configured dropdown definition - Fixed the `Table` question's column validation: each column is now checked against its own pattern only, values the pattern accepts are no longer rejected, the column type's own format check still applies, and errors are listed once below the table - Fix string condition operators (equals, contains, length) not being available on hidden questions diff --git a/src/Model/QuestionType/TableQuestion.php b/src/Model/QuestionType/TableQuestion.php index be46ac6..82188b4 100644 --- a/src/Model/QuestionType/TableQuestion.php +++ b/src/Model/QuestionType/TableQuestion.php @@ -526,11 +526,22 @@ private function resolveItemNames(string $itemtype, array $values): array global $DB; + $where = ['id' => $ids]; + + // GLPI 11 custom dropdown classes can share the same database table. + // Their system SQL criteria restrict rows to the current dropdown definition. + if (method_exists($itemtype, 'getSystemSQLCriteria')) { + $system_criteria = $itemtype::getSystemSQLCriteria(); + if (is_array($system_criteria) && $system_criteria !== []) { + $where[] = $system_criteria; + } + } + $map = []; foreach ($DB->request([ 'SELECT' => ['id', 'name'], 'FROM' => $item->getTable(), - 'WHERE' => ['id' => $ids], + 'WHERE' => $where, ]) as $row) { if (!is_array($row)) { continue; @@ -935,6 +946,15 @@ private function buildGlpiItemtypeOptions(string $itemtype): array $where = []; + // GLPI 11 custom dropdown classes can share the same database table. + // Their system SQL criteria restrict rows to the current dropdown definition. + if (method_exists($itemtype, 'getSystemSQLCriteria')) { + $system_criteria = $itemtype::getSystemSQLCriteria(); + if (is_array($system_criteria) && $system_criteria !== []) { + $where[] = $system_criteria; + } + } + if ($item->maybeDeleted()) { $where['is_deleted'] = 0; } diff --git a/tests/Model/QuestionType/TableQuestionTest.php b/tests/Model/QuestionType/TableQuestionTest.php index ea22748..ba88461 100644 --- a/tests/Model/QuestionType/TableQuestionTest.php +++ b/tests/Model/QuestionType/TableQuestionTest.php @@ -33,6 +33,8 @@ namespace GlpiPlugin\Advancedforms\Tests\Model\QuestionType; +use Glpi\Dropdown\DropdownDefinition; +use Glpi\Dropdown\DropdownDefinitionManager; use Glpi\Form\Condition\ValueOperator; use Glpi\Form\QuestionType\QuestionTypeCheckbox; use Glpi\Form\QuestionType\QuestionTypeEmail; @@ -46,6 +48,7 @@ use GlpiPlugin\Advancedforms\Model\QuestionType\TableQuestion; use GlpiPlugin\Advancedforms\Model\QuestionType\TableQuestionConfig; use GlpiPlugin\Advancedforms\Tests\AdvancedFormsTestCase; +use ReflectionMethod; final class TableQuestionTest extends AdvancedFormsTestCase { @@ -293,4 +296,89 @@ public function testTransformConditionValueSkipsNonArrayRows(): void $result = $this->type->transformConditionValueForComparisons($answer, null); $this->assertSame(['10.0.0.1'], $result); } + + public function testCustomDropdownOptionsAreRestrictedToConfiguredDefinition(): void + { + [$itemtype, $allowed_id, $other_id] = $this->createCustomDropdownFixture( + 'af_table_options', + ); + + $method = new ReflectionMethod( + TableQuestion::class, + 'buildGlpiItemtypeOptions', + ); + + $options = $method->invoke($this->type, $itemtype); + + $this->assertIsArray($options); + $this->assertArrayHasKey((string) $allowed_id, $options); + $this->assertSame('Allowed option', $options[(string) $allowed_id]); + $this->assertArrayNotHasKey((string) $other_id, $options); + } + + public function testCustomDropdownStoredValuesAreRestrictedToConfiguredDefinition(): void + { + [$itemtype, $allowed_id, $other_id] = $this->createCustomDropdownFixture( + 'af_table_values', + ); + + $method = new ReflectionMethod( + TableQuestion::class, + 'resolveItemNames', + ); + + $values = $method->invoke( + $this->type, + $itemtype, + [(string) $allowed_id, (string) $other_id], + ); + + $this->assertIsArray($values); + $this->assertArrayHasKey((string) $allowed_id, $values); + $this->assertSame('Allowed option', $values[(string) $allowed_id]); + $this->assertArrayNotHasKey((string) $other_id, $values); + } + + /** + * @return array{class-string, int, int} + */ + private function createCustomDropdownFixture(string $system_name): array + { + $allowed_definition = $this->createItem(DropdownDefinition::class, [ + 'system_name' => $system_name . '_allowed', + 'label' => 'Allowed dropdown', + 'is_active' => 1, + ]); + + $other_definition = $this->createItem(DropdownDefinition::class, [ + 'system_name' => $system_name . '_other', + 'label' => 'Other dropdown', + 'is_active' => 1, + ]); + + // Refresh definitions so GLPI can autoload both concrete + // Glpi\CustomDropdown classes created above. + DropdownDefinitionManager::getInstance()->bootDefinitions(); + + $allowed_itemtype = $allowed_definition->getDropdownClassName(); + $other_itemtype = $other_definition->getDropdownClassName(); + + $this->assertTrue(class_exists($allowed_itemtype)); + $this->assertTrue(class_exists($other_itemtype)); + + $allowed_item = $this->createItem($allowed_itemtype, [ + 'name' => 'Allowed option', + ]); + + $other_item = $this->createItem($other_itemtype, [ + 'name' => 'Other option', + ]); + + return [ + $allowed_itemtype, + (int) $allowed_item->getID(), + (int) $other_item->getID(), + ]; + } + }