Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion src/Model/QuestionType/TableQuestion.php
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,7 @@ public function getCompatibleQuestionTypes(): array
HostnameQuestion::class,
HiddenQuestion::class,
LdapQuestion::class,
ReservationQuestion::class,
self::class,
];

Expand All @@ -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();
}

Expand Down Expand Up @@ -933,7 +939,8 @@ private function buildGlpiItemtypeOptions(string $itemtype): array
return $options;
}

$where = [];
/** @var array<string, mixed> $where */
$where = $itemtype::getSystemSQLCriteria();

if ($item->maybeDeleted()) {
$where['is_deleted'] = 0;
Expand Down
53 changes: 52 additions & 1 deletion tests/Model/QuestionType/TableQuestionRenderingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<array{name: string, question_type: string, required: bool, itemtype: string, pattern: string}> $columns
* @return array<string, string> Decoded `data-af-pattern-cols` payload.
Expand Down Expand Up @@ -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,
];
}
Expand Down
59 changes: 59 additions & 0 deletions tests/Model/QuestionType/TableQuestionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -46,6 +47,13 @@
use GlpiPlugin\Advancedforms\Model\QuestionType\TableQuestion;
use GlpiPlugin\Advancedforms\Model\QuestionType\TableQuestionConfig;
Comment thread
RomainLvr marked this conversation as resolved.
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
{
Expand Down Expand Up @@ -154,6 +162,57 @@ public function testCompatibleTypesExcludesTreeCascadeDropdown(): void
$this->assertArrayNotHasKey(TreeCascadeDropdownQuestion::class, $types);
}

Comment thread
RomainLvr marked this conversation as resolved.
/**
* 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());
Expand Down