diff --git a/CHANGELOG.md b/CHANGELOG.md index 8111ba9..29e2f87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ 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/). +## [Unreleased] + +### Fixed + +- Fix Table question column type edge cases + ## [1.3.0] - 2026-08-11 ### Changed diff --git a/src/Model/QuestionType/TableQuestion.php b/src/Model/QuestionType/TableQuestion.php index be46ac6..5dffad0 100644 --- a/src/Model/QuestionType/TableQuestion.php +++ b/src/Model/QuestionType/TableQuestion.php @@ -804,6 +804,7 @@ public function getCompatibleQuestionTypes(): array HostnameQuestion::class, HiddenQuestion::class, LdapQuestion::class, + ReservationQuestion::class, self::class, ]; @@ -816,6 +817,11 @@ public function getCompatibleQuestionTypes(): array } } + // Exclude question types with a sub-type selector (Fields plugin types) + if (!is_a($fqcn, QuestionTypeItem::class, true) && $type->getSubTypes() !== []) { + continue; + } + $types[$fqcn] = $type->getName(); } @@ -933,7 +939,8 @@ private function buildGlpiItemtypeOptions(string $itemtype): array return $options; } - $where = []; + /** @var array $where */ + $where = $itemtype::getSystemSQLCriteria(); if ($item->maybeDeleted()) { $where['is_deleted'] = 0; diff --git a/tests/Model/QuestionType/TableQuestionRenderingTest.php b/tests/Model/QuestionType/TableQuestionRenderingTest.php index 5e7098b..f030c8d 100644 --- a/tests/Model/QuestionType/TableQuestionRenderingTest.php +++ b/tests/Model/QuestionType/TableQuestionRenderingTest.php @@ -33,15 +33,18 @@ namespace GlpiPlugin\Advancedforms\Tests\Model\QuestionType; +use Dropdown; use Glpi\Application\ImportMapGenerator; use Glpi\Form\Question; use Glpi\Form\QuestionType\QuestionTypeCheckbox; use Glpi\Form\QuestionType\QuestionTypeEmail; +use Glpi\Form\QuestionType\QuestionTypeItemDropdown; use Glpi\Form\QuestionType\QuestionTypeShortText; use Glpi\Tests\FormBuilder; use GlpiPlugin\Advancedforms\Model\QuestionType\TableQuestion; use GlpiPlugin\Advancedforms\Model\QuestionType\TableQuestionConfig; use GlpiPlugin\Advancedforms\Tests\AdvancedFormsTestCase; +use Session; use Symfony\Component\DomCrawler\Crawler; use function Safe\json_decode; @@ -269,6 +272,53 @@ public function testTheImportMapVersionsTheModuleOnItsContent(): void ); } + /** + * Regression test: custom dropdown definitions all share the same database + * table (distinguished only by a foreign key to their definition), so a + * column's option list must be scoped to its own definition. Without that + * scoping, every "Item (custom dropdown)" column ends up offering entries + * from every custom dropdown definition instead of just its own. + */ + public function testEachColumnOnlyShowsItsOwnCustomDropdownEntries(): void + { + $test1_definition = $this->initDropdownDefinition('Test1'); + $test2_definition = $this->initDropdownDefinition('Test2'); + + $test1_class = $test1_definition->getDropdownClassName(); + $test2_class = $test2_definition->getDropdownClassName(); + + Dropdown::resetItemtypesStaticCache(); + + $entity_id = Session::getActiveEntity(); + + $this->createItem($test1_class, [ + 'name' => 'Item from Test1', + 'entities_id' => $entity_id, + ]); + $this->createItem($test2_class, [ + 'name' => 'Item from Test2', + 'entities_id' => $entity_id, + ]); + + $html = $this->render([ + $this->column('Col1', QuestionTypeItemDropdown::class, itemtype: $test1_class), + $this->column('Col2', QuestionTypeItemDropdown::class, itemtype: $test2_class), + ]); + + $crawler = new Crawler($html); + $selects = $crawler->filter('[data-af-table-body] [data-af-table-row] select'); + $this->assertSame(2, $selects->count()); + + $col1_options = $selects->eq(0)->filter('option')->each(fn(Crawler $n): string => $n->text()); + $col2_options = $selects->eq(1)->filter('option')->each(fn(Crawler $n): string => $n->text()); + + $this->assertContains('Item from Test1', $col1_options); + $this->assertNotContains('Item from Test2', $col1_options); + + $this->assertContains('Item from Test2', $col2_options); + $this->assertNotContains('Item from Test1', $col2_options); + } + /** * @param array $columns * @return array Decoded `data-af-pattern-cols` payload. @@ -324,12 +374,13 @@ private function column( string $fqcn, bool $required = false, string $pattern = '', + string $itemtype = '', ): array { return [ TableQuestionConfig::COL_NAME => $name, TableQuestionConfig::COL_QUESTION_TYPE => $fqcn, TableQuestionConfig::COL_REQUIRED => $required, - TableQuestionConfig::COL_ITEMTYPE => '', + TableQuestionConfig::COL_ITEMTYPE => $itemtype, TableQuestionConfig::COL_PATTERN => $pattern, ]; } diff --git a/tests/Model/QuestionType/TableQuestionTest.php b/tests/Model/QuestionType/TableQuestionTest.php index ea22748..fd3940e 100644 --- a/tests/Model/QuestionType/TableQuestionTest.php +++ b/tests/Model/QuestionType/TableQuestionTest.php @@ -33,6 +33,7 @@ namespace GlpiPlugin\Advancedforms\Tests\Model\QuestionType; +use Glpi\Form\Question; use Glpi\Form\Condition\ValueOperator; use Glpi\Form\QuestionType\QuestionTypeCheckbox; use Glpi\Form\QuestionType\QuestionTypeEmail; @@ -46,6 +47,13 @@ use GlpiPlugin\Advancedforms\Model\QuestionType\TableQuestion; use GlpiPlugin\Advancedforms\Model\QuestionType\TableQuestionConfig; use GlpiPlugin\Advancedforms\Tests\AdvancedFormsTestCase; +use Glpi\Form\QuestionType\AbstractQuestionType; +use Glpi\Form\QuestionType\QuestionTypeCategoryInterface; +use Glpi\Form\QuestionType\QuestionTypeItem; +use Glpi\Form\QuestionType\QuestionTypeItemDropdown; +use Glpi\Form\QuestionType\QuestionTypesManager; +use GlpiPlugin\Advancedforms\Model\QuestionType\AdvancedCategory; +use Override; final class TableQuestionTest extends AdvancedFormsTestCase { @@ -154,6 +162,63 @@ public function testCompatibleTypesExcludesTreeCascadeDropdown(): void $this->assertArrayNotHasKey(TreeCascadeDropdownQuestion::class, $types); } + public function testCompatibleTypesExcludesReservation(): void + { + $types = $this->type->getCompatibleQuestionTypes(); + $this->assertArrayNotHasKey(ReservationQuestion::class, $types); + } + + /** + * Regression test for types with custom sub-type selectors, which cannot + * be represented as flat table column types and thus must be excluded. + */ + public function testCompatibleTypesExcludesTypesWithSubTypes(): void + { + $fake_type = new class extends AbstractQuestionType { + #[Override] + public function getCategory(): QuestionTypeCategoryInterface + { + return new AdvancedCategory(); + } + + #[Override] + public function getSubTypes(): array + { + return ['fake' => 'Fake sub type']; + } + + #[Override] + public function renderAdministrationTemplate(?Question $question): string + { + return ''; + } + + #[Override] + public function renderEndUserTemplate(?Question $question, mixed $answer = null): string + { + return ''; + } + }; + + QuestionTypesManager::getInstance()->registerPluginQuestionType($fake_type); + + $types = $this->type->getCompatibleQuestionTypes(); + $this->assertArrayNotHasKey($fake_type::class, $types); + } + + /** + * QuestionTypeItem and QuestionTypeItemDropdown both declare a non-empty + * getSubTypes() but must stay selectable: Table + * already renders them through its own dedicated itemtype picker + * (TableQuestionConfig::COL_ITEMTYPE), independent of getSubTypes(). + */ + public function testCompatibleTypesIncludesItemAndItemDropdownDespiteSubTypes(): void + { + $types = $this->type->getCompatibleQuestionTypes(); + $this->assertArrayHasKey(QuestionTypeItem::class, $types); + $this->assertArrayHasKey(QuestionTypeItemDropdown::class, $types); + } + public function testGetConfigKey(): void { $this->assertSame('enable_question_type_table', TableQuestion::getConfigKey());