From 9bb9f84ba170ffee9f6029a188e4a013c261b9c4 Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Wed, 17 Jun 2026 14:13:19 +0200 Subject: [PATCH 1/3] Fix - Visibility conditions not working on tree cascade dropdown questions --- public/js/modules/AfTreeCascadeDropdown.js | 2 +- .../TreeCascadeItemAsTextConditionHandler.php | 118 +++++++++++ .../TreeCascadeDropdownQuestion.php | 28 +++ ...eCascadeItemAsTextConditionHandlerTest.php | 187 ++++++++++++++++++ 4 files changed, 334 insertions(+), 1 deletion(-) create mode 100644 src/Model/ConditionHandler/TreeCascadeItemAsTextConditionHandler.php create mode 100644 tests/Model/ConditionHandler/TreeCascadeItemAsTextConditionHandlerTest.php diff --git a/public/js/modules/AfTreeCascadeDropdown.js b/public/js/modules/AfTreeCascadeDropdown.js index 543b0d3..42cb732 100644 --- a/public/js/modules/AfTreeCascadeDropdown.js +++ b/public/js/modules/AfTreeCascadeDropdown.js @@ -84,7 +84,7 @@ export class AfTreeCascadeDropdown { const value = $select.val(); const fieldName = $select.data('af-tree-field-name'); if (fieldName) { - $(`input[name="${fieldName}"]`).val(value); + $(`input[name="${fieldName}"]`).val(value).trigger('input'); } const $wrapper = $select.closest('.af-tree-level-wrapper'); diff --git a/src/Model/ConditionHandler/TreeCascadeItemAsTextConditionHandler.php b/src/Model/ConditionHandler/TreeCascadeItemAsTextConditionHandler.php new file mode 100644 index 0000000..4b70aca --- /dev/null +++ b/src/Model/ConditionHandler/TreeCascadeItemAsTextConditionHandler.php @@ -0,0 +1,118 @@ +" always fails. Using completename + * (e.g. "Parent > Child") makes the full path available for matching. + */ +final readonly class TreeCascadeItemAsTextConditionHandler implements ConditionHandlerInterface +{ + public function __construct( + private string $itemtype, + ) {} + + #[Override] + public function getSupportedValueOperators(): array + { + return [ + ValueOperator::CONTAINS, + ValueOperator::NOT_CONTAINS, + ]; + } + + #[Override] + public function getTemplate(): string + { + return '/pages/admin/form/condition_handler_templates/input.html.twig'; + } + + /** + * @return array + */ + #[Override] + public function getTemplateParameters(ConditionData $condition): array + { + return []; + } + + #[Override] + public function applyValueOperator( + mixed $a, + ValueOperator $operator, + mixed $b, + ): bool { + if (!is_array($a) || !isset($a['items_id'])) { + return false; + } + + $item = $this->itemtype::getById($a['items_id']); + if (!$item) { + return false; + } + + // Use completename for tree dropdowns so that conditions referencing + // ancestor nodes match correctly (e.g. "contains Parent" matches a child + // whose completename is "Parent > Child"). + /** @var CommonDBTM $item */ + if ($item instanceof CommonTreeDropdown) { + $completename = $item->fields['completename']; + $text = is_string($completename) ? $completename : ''; + } else { + $text = $item->getName(); + } + + $a = strtolower($text); + + $b = is_scalar($b) || $b === null ? strtolower((string) $b) : ''; + + return match ($operator) { + ValueOperator::CONTAINS => str_contains($a, $b), + ValueOperator::NOT_CONTAINS => !str_contains($a, $b), + default => false, + }; + } +} diff --git a/src/Model/QuestionType/TreeCascadeDropdownQuestion.php b/src/Model/QuestionType/TreeCascadeDropdownQuestion.php index f55f41a..2977156 100644 --- a/src/Model/QuestionType/TreeCascadeDropdownQuestion.php +++ b/src/Model/QuestionType/TreeCascadeDropdownQuestion.php @@ -36,11 +36,16 @@ use DBmysql; use CommonTreeDropdown; use Glpi\Application\View\TemplateRenderer; +use Glpi\DBAL\JsonFieldInterface; +use Glpi\Form\Condition\ConditionHandler\ItemAsTextConditionHandler; use Glpi\Form\Question; use Glpi\Form\QuestionType\QuestionTypeCategoryInterface; use Glpi\Form\QuestionType\QuestionTypeItemDropdown; +use Glpi\Form\QuestionType\QuestionTypeItemExtraDataConfig; +use GlpiPlugin\Advancedforms\Model\ConditionHandler\TreeCascadeItemAsTextConditionHandler; use GlpiPlugin\Advancedforms\Model\Config\ConfigurableItemInterface; use GlpiPlugin\Advancedforms\Model\QuestionType\AdvancedCategory; +use InvalidArgumentException; use Override; final class TreeCascadeDropdownQuestion extends QuestionTypeItemDropdown implements ConfigurableItemInterface @@ -404,6 +409,29 @@ private function getValidItemsForLevel(string $table, array $base_where, array $ return $items; } + #[Override] + public function getConditionHandlers(?JsonFieldInterface $question_config): array + { + if (!$question_config instanceof QuestionTypeItemExtraDataConfig) { + throw new InvalidArgumentException(); + } + + $handlers = parent::getConditionHandlers($question_config); + + // Replace ItemAsTextConditionHandler with the tree-aware variant that + // uses completename so conditions on ancestor nodes match correctly. + $handlers = array_filter( + $handlers, + fn($handler) => !($handler instanceof ItemAsTextConditionHandler), + ); + + if ($question_config->getItemtype()) { + $handlers[] = new TreeCascadeItemAsTextConditionHandler($question_config->getItemtype()); + } + + return array_values($handlers); + } + #[Override] public function prepareEndUserAnswer(Question $question, mixed $answer): mixed { diff --git a/tests/Model/ConditionHandler/TreeCascadeItemAsTextConditionHandlerTest.php b/tests/Model/ConditionHandler/TreeCascadeItemAsTextConditionHandlerTest.php new file mode 100644 index 0000000..90c794a --- /dev/null +++ b/tests/Model/ConditionHandler/TreeCascadeItemAsTextConditionHandlerTest.php @@ -0,0 +1,187 @@ +login(); + $this->enableConfigurableItem(TreeCascadeDropdownQuestion::class); + + $entity_id = Session::getActiveEntity(); + + $parent = $this->createItem(Location::class, [ + 'name' => 'Parent Location', + 'locations_id' => 0, + 'entities_id' => $entity_id, + ]); + + $child = $this->createItem(Location::class, [ + 'name' => 'Child Location', + 'locations_id' => $parent->getID(), + 'entities_id' => $entity_id, + ]); + + $extra_data = json_encode(new QuestionTypeItemDropdownExtraDataConfig( + itemtype: Location::class, + )); + + $form_builder = new FormBuilder("Test form"); + $form_builder->addQuestion( + name: "My location", + type: TreeCascadeDropdownQuestion::class, + extra_data: $extra_data, + ); + $form_builder->addQuestion("Dependent question", QuestionTypeShortText::class); + $form_builder->setQuestionVisibility("Dependent question", VisibilityStrategy::VISIBLE_IF, [ + [ + 'logic_operator' => LogicOperator::AND, + 'item_name' => "My location", + 'item_type' => Type::QUESTION, + 'value_operator' => ValueOperator::CONTAINS, + 'value' => 'Parent Location', + ], + ]); + + $form = $this->createForm($form_builder); + + $answers = [ + $this->getQuestionId($form, "My location") => [ + 'itemtype' => Location::class, + 'items_id' => $child->getID(), + ], + ]; + + $engine = new Engine($form, new EngineInput($answers)); + $output = $engine->computeVisibility(); + + $dependent_id = $this->getQuestionId($form, "Dependent question"); + $this->assertTrue( + $output->isQuestionVisible($dependent_id), + "Condition 'contains Parent Location' should match a child item " + . "whose completename is 'Parent Location > Child Location'", + ); + } + + /** + * Verify that NOT_CONTAINS evaluates to false when the searched text appears + * in the completename of the selected item (via an ancestor). + */ + public function testNotContainsMatchesCompletename(): void + { + $this->login(); + $this->enableConfigurableItem(TreeCascadeDropdownQuestion::class); + + $entity_id = Session::getActiveEntity(); + + $parent = $this->createItem(Location::class, [ + 'name' => 'Parent Location', + 'locations_id' => 0, + 'entities_id' => $entity_id, + ]); + + $child = $this->createItem(Location::class, [ + 'name' => 'Child Location', + 'locations_id' => $parent->getID(), + 'entities_id' => $entity_id, + ]); + + $extra_data = json_encode(new QuestionTypeItemDropdownExtraDataConfig( + itemtype: Location::class, + )); + + $form_builder = new FormBuilder("Test form not contains"); + $form_builder->addQuestion( + name: "My location", + type: TreeCascadeDropdownQuestion::class, + extra_data: $extra_data, + ); + $form_builder->addQuestion("Dependent question", QuestionTypeShortText::class); + $form_builder->setQuestionVisibility("Dependent question", VisibilityStrategy::VISIBLE_IF, [ + [ + 'logic_operator' => LogicOperator::AND, + 'item_name' => "My location", + 'item_type' => Type::QUESTION, + 'value_operator' => ValueOperator::NOT_CONTAINS, + 'value' => 'Parent Location', + ], + ]); + + $form = $this->createForm($form_builder); + + $answers = [ + $this->getQuestionId($form, "My location") => [ + 'itemtype' => Location::class, + 'items_id' => $child->getID(), + ], + ]; + + $engine = new Engine($form, new EngineInput($answers)); + $output = $engine->computeVisibility(); + + $dependent_id = $this->getQuestionId($form, "Dependent question"); + $this->assertFalse( + $output->isQuestionVisible($dependent_id), + "Condition 'not contains Parent Location' should NOT match a child item " + . "whose completename is 'Parent Location > Child Location'", + ); + } +} From 7d39e5604b66265dd07843cdade2ff0b65bf413c Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Wed, 17 Jun 2026 14:17:25 +0200 Subject: [PATCH 2/3] Update CHANGELOG --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 724738f..b319dee 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 visibility conditions on tree cascade dropdown questions + ## [1.1.1] - 2026-05-27 ### Fixed From cdaa9bf2775c7cefe4b328ac02e1d42dcc2f7967 Mon Sep 17 00:00:00 2001 From: RomainLvr Date: Fri, 26 Jun 2026 10:02:42 +0200 Subject: [PATCH 3/3] Apply suggestions --- .../TreeCascadeItemAsTextConditionHandler.php | 9 +- ...eCascadeItemAsTextConditionHandlerTest.php | 110 ++++++++++++++++++ 2 files changed, 112 insertions(+), 7 deletions(-) diff --git a/src/Model/ConditionHandler/TreeCascadeItemAsTextConditionHandler.php b/src/Model/ConditionHandler/TreeCascadeItemAsTextConditionHandler.php index 4b70aca..30519b1 100644 --- a/src/Model/ConditionHandler/TreeCascadeItemAsTextConditionHandler.php +++ b/src/Model/ConditionHandler/TreeCascadeItemAsTextConditionHandler.php @@ -34,7 +34,6 @@ namespace GlpiPlugin\Advancedforms\Model\ConditionHandler; use CommonDBTM; -use CommonTreeDropdown; use Glpi\Form\Condition\ConditionData; use Glpi\Form\Condition\ConditionHandler\ConditionHandlerInterface; use Glpi\Form\Condition\ValueOperator; @@ -98,12 +97,8 @@ public function applyValueOperator( // ancestor nodes match correctly (e.g. "contains Parent" matches a child // whose completename is "Parent > Child"). /** @var CommonDBTM $item */ - if ($item instanceof CommonTreeDropdown) { - $completename = $item->fields['completename']; - $text = is_string($completename) ? $completename : ''; - } else { - $text = $item->getName(); - } + $completename = $item->fields['completename'] ?? ''; + $text = is_string($completename) ? $completename : ''; $a = strtolower($text); diff --git a/tests/Model/ConditionHandler/TreeCascadeItemAsTextConditionHandlerTest.php b/tests/Model/ConditionHandler/TreeCascadeItemAsTextConditionHandlerTest.php index 90c794a..d4a17ca 100644 --- a/tests/Model/ConditionHandler/TreeCascadeItemAsTextConditionHandlerTest.php +++ b/tests/Model/ConditionHandler/TreeCascadeItemAsTextConditionHandlerTest.php @@ -121,6 +121,63 @@ public function testContainsMatchesCompletename(): void ); } + /** + * Verify that CONTAINS evaluates to true when the parent item itself is + * selected (completename equals the searched text exactly, not via a child). + */ + public function testContainsMatchesParentDirectly(): void + { + $this->login(); + $this->enableConfigurableItem(TreeCascadeDropdownQuestion::class); + + $entity_id = Session::getActiveEntity(); + + $parent = $this->createItem(Location::class, [ + 'name' => 'Parent Location', + 'locations_id' => 0, + 'entities_id' => $entity_id, + ]); + + $extra_data = json_encode(new QuestionTypeItemDropdownExtraDataConfig( + itemtype: Location::class, + )); + + $form_builder = new FormBuilder("Test form parent selected"); + $form_builder->addQuestion( + name: "My location", + type: TreeCascadeDropdownQuestion::class, + extra_data: $extra_data, + ); + $form_builder->addQuestion("Dependent question", QuestionTypeShortText::class); + $form_builder->setQuestionVisibility("Dependent question", VisibilityStrategy::VISIBLE_IF, [ + [ + 'logic_operator' => LogicOperator::AND, + 'item_name' => "My location", + 'item_type' => Type::QUESTION, + 'value_operator' => ValueOperator::CONTAINS, + 'value' => 'Parent Location', + ], + ]); + + $form = $this->createForm($form_builder); + + $answers = [ + $this->getQuestionId($form, "My location") => [ + 'itemtype' => Location::class, + 'items_id' => $parent->getID(), + ], + ]; + + $engine = new Engine($form, new EngineInput($answers)); + $output = $engine->computeVisibility(); + + $dependent_id = $this->getQuestionId($form, "Dependent question"); + $this->assertTrue( + $output->isQuestionVisible($dependent_id), + "Condition 'contains Parent Location' should match when the parent item itself is selected", + ); + } + /** * Verify that NOT_CONTAINS evaluates to false when the searched text appears * in the completename of the selected item (via an ancestor). @@ -184,4 +241,57 @@ public function testNotContainsMatchesCompletename(): void . "whose completename is 'Parent Location > Child Location'", ); } + + /** + * Verify that both CONTAINS and NOT_CONTAINS evaluate to false when no item + * is selected (items_id = 0), so the dependent question is hidden in both cases. + * + * This is non-obvious: a NOT_CONTAINS condition does NOT show the question + * when no selection has been made — it also evaluates to false. + */ + public function testNoSelectionHidesDependentQuestion(): void + { + $this->login(); + $this->enableConfigurableItem(TreeCascadeDropdownQuestion::class); + + $extra_data = json_encode(new QuestionTypeItemDropdownExtraDataConfig( + itemtype: Location::class, + )); + + $form_builder = new FormBuilder("Test form no selection"); + $form_builder->addQuestion( + name: "My location", + type: TreeCascadeDropdownQuestion::class, + extra_data: $extra_data, + ); + $form_builder->addQuestion("Dependent question", QuestionTypeShortText::class); + $form_builder->setQuestionVisibility("Dependent question", VisibilityStrategy::VISIBLE_IF, [ + [ + 'logic_operator' => LogicOperator::AND, + 'item_name' => "My location", + 'item_type' => Type::QUESTION, + 'value_operator' => ValueOperator::NOT_CONTAINS, + 'value' => 'some location', + ], + ]); + + $form = $this->createForm($form_builder); + + $answers = [ + $this->getQuestionId($form, "My location") => [ + 'itemtype' => Location::class, + 'items_id' => 0, + ], + ]; + + $engine = new Engine($form, new EngineInput($answers)); + $output = $engine->computeVisibility(); + + $dependent_id = $this->getQuestionId($form, "Dependent question"); + $this->assertFalse( + $output->isQuestionVisible($dependent_id), + "When no item is selected (items_id=0), NOT_CONTAINS evaluates to false " + . "and the dependent question should be hidden", + ); + } }