Skip to content

FIX / Fix mandatory fields check on updates made during item creation #1337

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

Open
Mary-Clb wants to merge 2 commits into
base: 11.0/bugfixes
Choose a base branch
from
+205 −2
Open

FIX / Fix mandatory fields check on updates made during item creation #1337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
acaacac
fix mandatory fields check on updates made during item creation
Mary-Clb Nov 6, 2026
dd0ec1d
update CHANGELOG.md
Mary-Clb Nov 6, 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 @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
### Fixed

- Fix search criterion value on multiple dropdown fields being reset after a search, which emptied the criterion when sorting or paging results
- Fix false "mandatory fields are empty" error and status not set to assigned when creating an item with an assignee and filled mandatory fields

## [1.24.7] - 2026-10-28

Expand Down
44 changes: 42 additions & 2 deletions inc/container.class.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ class PluginFieldsContainer extends CommonDBTM

public static $rightname = 'config';

/** @var WeakMap<CommonDBTM, true>|null Items being added whose values are not stored yet ("item_add" hook not called) */
private static ?WeakMap $items_being_added = null;

public static function canCreate(): bool
{
return self::canUpdate();
Expand Down Expand Up @@ -2424,6 +2427,7 @@ public static function findAllContainers($itemtype)
public static function postItemAdd(CommonDBTM $item)
{
PluginFieldsDestinationField::registerCreatedItem($item);
unset(self::$items_being_added[$item]);

if (array_key_exists('_plugin_fields_data', $item->input)) {
$container = new self();
Expand Down Expand Up @@ -2549,6 +2553,11 @@ public static function preItem(CommonDBTM $item)
return false;
}

if ($item->isNewItem()) {
self::$items_being_added ??= new WeakMap();
self::$items_being_added[$item] = true;
}

if (count($item->fields) === 0) {
$item->fields = $item->input;
}
Expand Down Expand Up @@ -2601,6 +2610,30 @@ public static function preItem(CommonDBTM $item)
return false;
}

/** @return array|null Values submitted on the add of this item for the container, null when the item is not being added */
private static function getValuesSubmittedOnAdd(CommonDBTM $item, int $c_id): ?array
{
foreach (self::$items_being_added ?? [] as $added_item => $_) {
if ($added_item::class !== $item::class || (int) $added_item->getID() !== (int) $item->getID()) {
continue;
}

if (!isset($added_item->input['_plugin_fields_data'])) {
return [];
}

foreach (self::getSubmittedContainersData($added_item) as $data) {
if ((int) $data['plugin_fields_containers_id'] === $c_id) {
return $data;
}
}

return [];
}

return null;
}

/**
* Validate a single container's mandatory fields for the given item, using
* either the values submitted in this request or, if none were submitted
Expand All @@ -2626,9 +2659,13 @@ private static function checkContainerMandatory(CommonDBTM $item, PluginFieldsCo
return $data;
}

// Update made while the item is being added (ex: status set on assignment): the "item_add" hook
// has not stored the values yet, so the ones submitted on the add are checked instead
$added_values = $item->isNewItem() ? null : self::getValuesSubmittedOnAdd($item, (int) $c_id);

//nothing submitted for this container in this request (e.g. untouched Tab)
//tab containers can't be filled before the item exists, so skip on creation
if ($item->isNewItem() && $loc_c->fields['type'] !== 'dom') {
if (($item->isNewItem() || $added_values !== null) && $loc_c->fields['type'] !== 'dom') {
return [];
}

Expand All @@ -2652,7 +2689,10 @@ private static function checkContainerMandatory(CommonDBTM $item, PluginFieldsCo
$data[$status_field_name] = $status_value;
}

if (!$item->isNewItem()) {
if ($added_values !== null) {
$data['items_id'] = $item->getID();
$data += $added_values;
} elseif (!$item->isNewItem()) {
$data['items_id'] = $item->getID();

// merge already persisted values to avoid false positives
Expand Down
162 changes: 162 additions & 0 deletions tests/Units/ContainerItemUpdateTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,19 @@
namespace GlpiPlugin\Field\Tests\Units;

use Auth;
use CommonITILObject;
use Computer;
use Glpi\Tests\DbTestCase;
use Glpi\Tests\GLPITestCase;
use GlpiPlugin\Field\Tests\FieldTestTrait;
use PluginFieldsContainer;
use PluginFieldsContainerDisplayCondition;
use PluginFieldsStatusOverride;
use Ticket;
use Entity;
use Notification;
use Problem;
use User;

require_once __DIR__ . '/../FieldTestCase.php';

Expand Down Expand Up @@ -1117,4 +1120,163 @@ public function testNotBlockedWhenMandatoryDomContainerIsHidden(): void
ERROR,
);
}

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

$container = $this->createFieldContainer([
'label' => 'Mandatory Dom 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'];

// The assignment updates the ticket status before the "item_add" hook stores the field value
$ticket = new Ticket();
$ticket_id = $ticket->add([
'name' => 'Ticket with filled mandatory field and assignee',
'content' => 'Test',
'entities_id' => 0,
'c_id' => $container->getID(),
$field_name => 'filled value',
'_actors' => [
'assign' => [
['itemtype' => User::class, 'items_id' => getItemByTypeName(User::class, 'tech', true)],
],
],
]);

$this->assertGreaterThan(0, $ticket_id);
$this->hasNoSessionMessage(ERROR);
$this->assertTrue($ticket->getFromDB($ticket_id));
$this->assertEquals(CommonITILObject::ASSIGNED, $ticket->fields['status']);

$values = getItemForItemtype(PluginFieldsContainer::getClassname(Ticket::class, $container->fields['name']));
$this->assertTrue($values->getFromDBByCrit(['items_id' => $ticket_id]));
$this->assertSame('filled value', $values->fields[$field_name]);

// Once created, the stored values are checked again on updates
$this->assertTrue($ticket->update(['id' => $ticket_id, 'name' => 'Renamed']));
}

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

$container = $this->createFieldContainer([
'label' => 'Status Override Dom Container',
'type' => 'dom',
'itemtypes' => [Ticket::class],
'is_active' => 1,
'entities_id' => 0,
'is_recursive' => 1,
]);
$field = $this->createField([
'label' => 'Mandatory When Assigned',
'type' => 'text',
PluginFieldsContainer::getForeignKeyField() => $container->getID(),
'ranking' => 1,
'is_active' => 1,
'is_readonly' => 0,
'mandatory' => 0,
]);
$field_name = $field->fields['name'];
$this->createItem(PluginFieldsStatusOverride::class, [
PluginFieldsContainer::getForeignKeyField() => $container->getID(),
'plugin_fields_fields_id' => $field->getID(),
'itemtype' => Ticket::class,
'states' => [CommonITILObject::ASSIGNED],
'is_readonly' => 0,
'mandatory' => 1,
], ['states', PluginFieldsContainer::getForeignKeyField()]);

$assign = ['assign' => [['itemtype' => User::class, 'items_id' => getItemByTypeName(User::class, 'tech', true)]]];

// Filled on creation: the ticket can be assigned
$ticket = new Ticket();
$ticket_id = $ticket->add([
'name' => 'Filled field',
'content' => 'Test',
'entities_id' => 0,
'c_id' => $container->getID(),
$field_name => 'filled value',
'_actors' => $assign,
]);
$this->assertGreaterThan(0, $ticket_id);
$this->hasNoSessionMessage(ERROR);
$this->assertTrue($ticket->getFromDB($ticket_id));
$this->assertEquals(CommonITILObject::ASSIGNED, $ticket->fields['status']);

// Left empty: the ticket is created but can't move to the status requiring the field
$ticket = new Ticket();
$ticket_id = $ticket->add([
'name' => 'Empty field',
'content' => 'Test',
'entities_id' => 0,
'c_id' => $container->getID(),
$field_name => '',
'_actors' => $assign,
]);
$this->assertGreaterThan(0, $ticket_id);
$this->hasSessionMessageThatContains(__('Some mandatory fields are empty', 'fields'), ERROR);
$this->assertTrue($ticket->getFromDB($ticket_id));
$this->assertEquals(CommonITILObject::INCOMING, $ticket->fields['status']);
}

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

$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,
]);

// A tab can't be filled before the item exists, including by the updates made during its creation
$ticket = new Ticket();
$ticket_id = $ticket->add([
'name' => 'Ticket with mandatory tab field and assignee',
'content' => 'Test',
'entities_id' => 0,
'_actors' => [
'assign' => [
['itemtype' => User::class, 'items_id' => getItemByTypeName(User::class, 'tech', true)],
],
],
]);
$this->assertGreaterThan(0, $ticket_id);
$this->hasNoSessionMessage(ERROR);
$this->assertTrue($ticket->getFromDB($ticket_id));
$this->assertEquals(CommonITILObject::ASSIGNED, $ticket->fields['status']);

// Once created, the empty tab field blocks updates again
$this->assertFalse((new Ticket())->update(['id' => $ticket_id, 'name' => 'Renamed']));
$this->hasSessionMessageThatContains(__('Some mandatory fields are empty', 'fields'), ERROR);
}
}
Loading