Skip to content

fix(container.class): Prevent item creation with null value for mandatory fields #1229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Rom1-B merged 3 commits into from
Jul 22, 2026
Merged

fix(container.class): Prevent item creation with null value for mandatory fields #1229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f407141
fix(container.class): Prevent item creation with null value for manda…
TarekRemo Jul 21, 2026
9af89bb
changelog
TarekRemo Jul 21, 2026
30f4d72
add status override
TarekRemo Jul 22, 2026
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

### Fixed

- Fix item creation with null value for mandatory fields
- Fix search crash when two containers share a dropdown field with the same name.

## [1.24.2] - 2026-06-30
Expand Down
16 changes: 16 additions & 0 deletions inc/container.class.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1970,6 +1970,22 @@ public static function preItem(CommonDBTM $item)
return true;
}

//call validateValues() with a minimal data array to check for missing mandatory fields
//in case populateData() fails
if ($item->isNewItem() && $loc_c->fields['type'] === 'dom') {
$status_field_name = PluginFieldsStatusOverride::getStatusFieldName($item::getType());
$data = ['plugin_fields_containers_id' => $c_id];
if (array_key_exists($status_field_name, $item->input) && $item->input[$status_field_name] !== '') {
$data[$status_field_name] = (int) $item->input[$status_field_name];
} elseif (array_key_exists($status_field_name, $item->fields) && $item->fields[$status_field_name] !== '') {
$data[$status_field_name] = (int) $item->fields[$status_field_name];
}

if (self::validateValues($data, $item::getType(), isset($_REQUEST['massiveaction'])) === false) {
$item->input = [];
}
}
Comment on lines +1975 to +1987

@Rom1-B Rom1-B Jul 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The minimal $data array built for this new validation path omits the status field. populateData()'s normal path always sets $data[$status_field_name] from $item->input/$item->fields (lines 2029-2034), and validateValues() resolves $status_value = $data[$status_field_name] ?? null to apply PluginFieldsStatusOverride mandatory/readonly relaxations (line 1673). Since this new branch never sets that key, any status override waiving a field's mandatory flag for a given creation status is silently ignored here, so a legitimate creation (API/CLI, dom container, status override in place) gets incorrectly blocked.

Suggested change
if ($item->isNewItem() && $loc_c->fields['type'] === 'dom') {
$data = ['plugin_fields_containers_id' => $c_id];
if (self::validateValues($data, $item::getType(), isset($_REQUEST['massiveaction'])) === false) {
$item->input = [];
return false;
}
}
if ($item->isNewItem() && $loc_c->fields['type'] === 'dom') {
$status_field_name = PluginFieldsStatusOverride::getStatusFieldName($item::getType());
$data = ['plugin_fields_containers_id' => $c_id];
if (array_key_exists($status_field_name, $item->input) && $item->input[$status_field_name] !== '') {
$data[$status_field_name] = (int) $item->input[$status_field_name];
} elseif (array_key_exists($status_field_name, $item->fields) && $item->fields[$status_field_name] !== '') {
$data[$status_field_name] = (int) $item->fields[$status_field_name];
}
if (self::validateValues($data, $item::getType(), isset($_REQUEST['massiveaction'])) === false) {
$item->input = [];
return false;
}
}

@TarekRemo TarekRemo Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


return false;
}

Expand Down
118 changes: 118 additions & 0 deletions tests/Units/ContainerItemUpdateTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,124 @@ public function testCreateTicketInApiLikeContext(): void
$this->assertSame('created via api', $plugin_row[$field_name]);
}

public function testCreateIsBlockedWhenMandatoryDomFieldIsMissing(): void
{
$this->login();

$container = $this->createFieldContainer([
'label' => 'Mandatory Create Container',
'type' => 'dom',
'itemtypes' => [Ticket::class],
'is_active' => 1,
'entities_id' => 0,
'is_recursive' => 1,
]);

$field = $this->createField([
'label' => 'Mandatory Field',
'type' => 'text',
PluginFieldsContainer::getForeignKeyField() => $container->getID(),
'ranking' => 1,
'is_active' => 1,
'is_readonly' => 0,
'mandatory' => 1,
]);
$field_name = $field->fields['name'];

$this->simulateApiBoot();

// Creation with the mandatory field omitted must be rejected.
$ticket = new Ticket();
$ticket_id = $ticket->add([
'name' => 'Ticket without mandatory field',
'content' => 'Test creation',
'entities_id' => 0,
]);
$this->assertFalse($ticket_id, 'Creation must be blocked when a mandatory dom field is missing.');
$this->hasSessionMessageThatContains(
__('Some mandatory fields are empty', 'fields'),
ERROR,
);

// Creation with the mandatory field filled must succeed.
$ticket = new Ticket();
$ticket_id = $ticket->add([
'name' => 'Ticket with mandatory field',
'content' => 'Test creation',
'entities_id' => 0,
$field_name => 'filled value',
]);
$this->assertGreaterThan(0, $ticket_id);

$plugin_row = $this->getPluginFieldValues(Ticket::class, $ticket_id, $container->getID());
$this->assertNotFalse($plugin_row);
$this->assertSame('filled value', $plugin_row[$field_name]);
}

public function testCreateIsNotBlockedWhenMandatoryTabOrDomtabFieldIsMissing(): void
{
$this->login();

// TAB container with a mandatory field.
$tab_container = $this->createFieldContainer([
'label' => 'Mandatory Tab Container',
'type' => 'tab',
'itemtypes' => [Ticket::class],
'is_active' => 1,
'entities_id' => 0,
'is_recursive' => 1,
]);
$this->createField([
'label' => 'Mandatory Tab Field',
'type' => 'text',
PluginFieldsContainer::getForeignKeyField() => $tab_container->getID(),
'ranking' => 1,
'is_active' => 1,
'is_readonly' => 0,
'mandatory' => 1,
]);

// DOMTAB container with a mandatory field, attached to a specific tab.
$domtab_container = $this->createFieldContainer([
'label' => 'Mandatory Domtab Container',
'type' => 'domtab',
'subtype' => Ticket::class . '$1',
'itemtypes' => [Ticket::class],
'is_active' => 1,
'entities_id' => 0,
'is_recursive' => 1,
]);
$this->createField([
'label' => 'Mandatory Domtab Field',
'type' => 'text',
PluginFieldsContainer::getForeignKeyField() => $domtab_container->getID(),
'ranking' => 1,
'is_active' => 1,
'is_readonly' => 0,
'mandatory' => 1,
]);

// Creation with no plugin field must not block the creation.
$ticket = new Ticket();
$ticket_id = $ticket->add([
'name' => 'Ticket with mandatory tab field missing',
'content' => 'Test creation',
'entities_id' => 0,
]);
$this->assertGreaterThan(0, $ticket_id, 'A mandatory tab field must not block creation.');

// Creation forcing the "domtab" container resolution via explicit c_id:
// it must not block the creation either.
$ticket = new Ticket();
$ticket_id = $ticket->add([
'name' => 'Ticket with mandatory domtab field missing',
'content' => 'Test creation',
'entities_id' => 0,
'c_id' => $domtab_container->getID(),
]);
$this->assertGreaterThan(0, $ticket_id, 'A mandatory domtab field must not block creation.');
}

/**
* Update a ticket with explicit c_id through an API-like context.
*/
Expand Down