From 1eeb17cc61443b49128d3e392a599fdfe021c4c3 Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Wed, 12 Aug 2026 15:02:53 +0200 Subject: [PATCH 1/5] Fix - Scope custom dropdown option lists to their own definition --- src/Model/QuestionType/TableQuestion.php | 4 +- .../TableQuestionRenderingTest.php | 53 ++++++++++++++++++- 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/src/Model/QuestionType/TableQuestion.php b/src/Model/QuestionType/TableQuestion.php index be46ac6..a1d5e9f 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, ]; @@ -933,7 +934,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, ]; } From cec9bb6bb0cdbb32c108f9f86c5c5a61f10a430e Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Wed, 12 Aug 2026 16:40:10 +0200 Subject: [PATCH 2/5] Fix - Exclude question types with a sub-type selector --- src/Model/QuestionType/TableQuestion.php | 5 ++ .../Model/QuestionType/TableQuestionTest.php | 58 +++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/src/Model/QuestionType/TableQuestion.php b/src/Model/QuestionType/TableQuestion.php index a1d5e9f..5dffad0 100644 --- a/src/Model/QuestionType/TableQuestion.php +++ b/src/Model/QuestionType/TableQuestion.php @@ -817,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(); } diff --git a/tests/Model/QuestionType/TableQuestionTest.php b/tests/Model/QuestionType/TableQuestionTest.php index ea22748..c9b6bdf 100644 --- a/tests/Model/QuestionType/TableQuestionTest.php +++ b/tests/Model/QuestionType/TableQuestionTest.php @@ -46,6 +46,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 +161,57 @@ public function testCompatibleTypesExcludesTreeCascadeDropdown(): void $this->assertArrayNotHasKey(TreeCascadeDropdownQuestion::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(?\Glpi\Form\Question $question): string + { + return ''; + } + + #[Override] + public function renderEndUserTemplate(?\Glpi\Form\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()); From 4230f9f7e88c46fa0eb87eff55fbf0bd9a251897 Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Wed, 12 Aug 2026 16:43:37 +0200 Subject: [PATCH 3/5] Rector --- tests/Model/QuestionType/TableQuestionTest.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/Model/QuestionType/TableQuestionTest.php b/tests/Model/QuestionType/TableQuestionTest.php index c9b6bdf..17c7f22 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; @@ -181,13 +182,13 @@ public function getSubTypes(): array } #[Override] - public function renderAdministrationTemplate(?\Glpi\Form\Question $question): string + public function renderAdministrationTemplate(?Question $question): string { return ''; } #[Override] - public function renderEndUserTemplate(?\Glpi\Form\Question $question, mixed $answer = null): string + public function renderEndUserTemplate(?Question $question, mixed $answer = null): string { return ''; } From e6fc18541969ef2619ef5e026b47740255cee433 Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Wed, 12 Aug 2026 16:46:59 +0200 Subject: [PATCH 4/5] Update CHANGELOG --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) 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 From a110bf35597ba5e1d93a1eb81d56aba90a3b13aa Mon Sep 17 00:00:00 2001 From: Romain Lecouvreur <102067890+RomainLvr@users.noreply.github.com> Date: Thu, 13 Aug 2026 10:09:47 +0200 Subject: [PATCH 5/5] Update tests/Model/QuestionType/TableQuestionTest.php Co-authored-by: Romain B. <8530352+Rom1-B@users.noreply.github.com> --- tests/Model/QuestionType/TableQuestionTest.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/Model/QuestionType/TableQuestionTest.php b/tests/Model/QuestionType/TableQuestionTest.php index 17c7f22..fd3940e 100644 --- a/tests/Model/QuestionType/TableQuestionTest.php +++ b/tests/Model/QuestionType/TableQuestionTest.php @@ -162,6 +162,12 @@ 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.