Skip to content

Commit 62116eb

Browse files
authored
fix(questiontype): handle potential null values in dropdown options (#1173)
1 parent b3d4f91 commit 62116eb

File tree

9 files changed

+68
-12
lines changed

9 files changed

+68
-12
lines changed

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
55
The format is based on [Keep a Changelog](http://keepachangelog.com/)
66
and this project adheres to [Semantic Versioning](http://semver.org/).
77

8+
## [Unreleased]
9+
10+
### Fixed
11+
12+
- Fix error when submitting a form with an hidden question of type `Field`
13+
814
## [1.23.4] - 2026-03-26
915

1016
### Fixed

inc/field.class.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -973,7 +973,7 @@ public static function showForTab($params)
973973
$itemtypes = PluginFieldsContainer::getUsedItemtypes($type, true);
974974

975975
//if no dom containers defined for this itemtype, do nothing (in_array case insensitive)
976-
if (!in_array(strtolower((string) $item::getType()), array_map(strtolower(...), $itemtypes))) {
976+
if (!in_array(strtolower((string) $item::getType()), array_map(strtolower(...), $itemtypes), true)) {
977977
return;
978978
}
979979

inc/inventory.class.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public static function updateInventory($params = [])
3838
) {
3939
$availaibleItemType = ['Computer', 'Printer', 'NetworkEquipment'];
4040
foreach (array_keys($params['inventory_data']) as $itemtype) {
41-
if (in_array($itemtype, $availaibleItemType)) {
41+
if (in_array($itemtype, $availaibleItemType, true)) {
4242
$items_id = 0;
4343
//retrieve items id switch itemtype
4444
switch ($itemtype) {

inc/migration.class.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ private static function getCustomFieldsInContainerTable(
197197

198198
return array_filter(
199199
$fields,
200-
fn(string $field) => !in_array($field, $basic_fields),
200+
fn(string $field) => !in_array($field, $basic_fields, true),
201201
);
202202
}
203203
}

inc/questiontype.class.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ public function formatRawAnswer(mixed $answer, Question $question): string
201201
}
202202

203203
$itemtype = PluginFieldsDropdown::getClassname($current_field->fields['name']);
204-
return implode(', ', array_map(fn($opt_id) => $itemtype::getById($opt_id)->fields['name'], $answer));
204+
return implode(', ', array_map(fn($opt_id) => $itemtype::getById($opt_id)?->fields['name'] ?? '', $answer));
205205
case 'yesno':
206206
return $answer ? __('Yes') : __('No');
207207
case 'datetime':

tests/FieldTestCase.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,10 @@
3131
namespace GlpiPlugin\Field\Tests;
3232

3333
use DBmysql;
34+
use Glpi\Form\Question;
3435
use PluginFieldsContainer;
3536
use PluginFieldsField;
37+
use PluginFieldsQuestionType;
3638

3739
trait FieldTestTrait
3840
{
@@ -47,6 +49,12 @@ public function tearDownFieldTest(): void
4749
// Re-login to ensure we are logged in
4850
$this->login();
4951

52+
// Clean all questions that use QuestionTypeField to avoid issues with foreign key constraints when deleting fields
53+
array_map(
54+
fn(Question $question) => $question->delete($question->fields, true),
55+
iterator_to_array(Question::getSeveralFromDBByCrit(['type' => PluginFieldsQuestionType::class])),
56+
);
57+
5058
// Clean created containers
5159
array_map(
5260
fn(PluginFieldsContainer $container) => $container->delete($container->fields, true),

tests/QuestionTypeTestCase.php

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ abstract class QuestionTypeTestCase extends DbTestCase
4848

4949
protected ?PluginFieldsContainer $block = null;
5050

51-
protected ?PluginFieldsField $field = null;
51+
/** @var PluginFieldsField[] */
52+
protected array $fields = [];
5253

5354
public function createFieldAndContainer(): void
5455
{
@@ -61,23 +62,33 @@ public function createFieldAndContainer(): void
6162
'entities_id' => $this->getTestRootEntity(true),
6263
]);
6364

64-
$this->field = $this->createField([
65+
$this->fields['glpi_item'] = $this->createField([
6566
'label' => 'GLPI Item',
6667
'type' => 'glpi_item',
6768
PluginFieldsContainer::getForeignKeyField() => $this->block->getID(),
6869
'ranking' => 1,
6970
'is_active' => 1,
7071
]);
72+
73+
$this->fields['dropdown'] = $this->createField([
74+
'label' => 'Dropdown',
75+
'type' => 'dropdown',
76+
PluginFieldsContainer::getForeignKeyField() => $this->block->getID(),
77+
'ranking' => 1,
78+
'is_active' => 1,
79+
]);
7180
}
7281

7382
public function setUp(): void
7483
{
7584
$this->createFieldAndContainer();
85+
parent::setUp();
7686
}
7787

7888
public function tearDown(): void
7989
{
8090
$this->tearDownFieldTest();
91+
parent::tearDown();
8192
}
8293

8394
protected function renderFormEditor(Form $form): Crawler

tests/Units/FieldQuestionTypeMigrationTest.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@
3939

4040
final class FieldQuestionTypeMigrationTest extends QuestionTypeTestCase
4141
{
42+
public array $fields;
43+
4244
public static function setUpBeforeClass(): void
4345
{
4446
global $DB;
@@ -104,7 +106,7 @@ public function testFieldsQuestionIsMigrated(): void
104106
'row' => 0,
105107
'col' => 0,
106108
'values' => json_encode([
107-
'dropdown_fields_field' => $this->field->fields['name'],
109+
'dropdown_fields_field' => $this->fields['glpi_item']->fields['name'],
108110
'blocks_field' => $this->block->getID(),
109111
]),
110112
],

tests/Units/FieldQuestionTypeTest.php

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
use GlpiPlugin\Field\Tests\QuestionTypeTestCase;
3636
use LogicException;
3737
use PluginFieldsContainer;
38+
use PluginFieldsDropdown;
3839
use PluginFieldsField;
3940
use PluginFieldsQuestionType;
4041
use PluginFieldsQuestionTypeCategory;
@@ -117,7 +118,7 @@ public function testFieldsQuestionEditorRendering(): void
117118
$builder->addQuestion(
118119
"My question",
119120
PluginFieldsQuestionType::class,
120-
extra_data: json_encode($this->getFieldExtraDataConfig()),
121+
extra_data: json_encode($this->getFieldExtraDataConfig('glpi_item')),
121122
);
122123
$form = $this->createForm($builder);
123124

@@ -137,7 +138,7 @@ public function testFieldsQuestionHelpdeskRendering(): void
137138
$builder->addQuestion(
138139
"My question",
139140
PluginFieldsQuestionType::class,
140-
extra_data: json_encode($this->getFieldExtraDataConfig()),
141+
extra_data: json_encode($this->getFieldExtraDataConfig('glpi_item')),
141142
);
142143
$form = $this->createForm($builder);
143144

@@ -148,12 +149,40 @@ public function testFieldsQuestionHelpdeskRendering(): void
148149
$this->assertNotEmpty($crawler->filter('[data-glpi-form-renderer-fields-question-type-specific-container]'));
149150
}
150151

151-
private function getFieldExtraDataConfig(): PluginFieldsQuestionTypeExtraDataConfig
152+
public function testFieldsQuestionSubmitEmptyDropdown(): void
152153
{
153-
if (!$this->block instanceof PluginFieldsContainer || !$this->field instanceof PluginFieldsField) {
154+
$this->login();
155+
156+
/** @var CommonDBTM $dropdown_item */
157+
$dropdown_item = getItemForItemtype(PluginFieldsDropdown::getClassname($this->fields['dropdown']->fields['name']));
158+
$dropdown_ids = [];
159+
for ($i = 1; $i <= 3; $i++) {
160+
$dropdown_ids[] = $dropdown_item->add([
161+
'name' => 'Option ' . $i,
162+
]);
163+
}
164+
165+
// Arrange: create form with Field question
166+
$builder = new FormBuilder("My form");
167+
$builder->addQuestion(
168+
"Dropdown field question",
169+
PluginFieldsQuestionType::class,
170+
extra_data: json_encode($this->getFieldExtraDataConfig('dropdown')),
171+
);
172+
$form = $this->createForm($builder);
173+
174+
// Act: submit form
175+
$this->sendFormAndGetCreatedTicket($form, [
176+
"Dropdown field question" => '0',
177+
]);
178+
}
179+
180+
private function getFieldExtraDataConfig(string $field_name): PluginFieldsQuestionTypeExtraDataConfig
181+
{
182+
if (!$this->block instanceof PluginFieldsContainer || !$this->fields[$field_name] instanceof PluginFieldsField) {
154183
throw new LogicException("Field and container must be created before getting extra data config");
155184
}
156185

157-
return new PluginFieldsQuestionTypeExtraDataConfig($this->block->getID(), $this->field->getID());
186+
return new PluginFieldsQuestionTypeExtraDataConfig($this->block->getID(), $this->fields[$field_name]->getID());
158187
}
159188
}

0 commit comments

Comments
 (0)