Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
c0822fa
Initial plan
Copilot Jan 30, 2026
59b0986
Add validation to prevent empty OS records
Copilot Jan 30, 2026
0816264
Fix prepareInputForUpdate to use current record data
Copilot Jan 30, 2026
7ba5d3d
Add company, owner, and hostid to validation fields
Copilot Jan 30, 2026
0d19d21
Address code review feedback: improve separation of concerns and simp…
Copilot Jan 30, 2026
e22aee5
Merge pull request #7 from eduardomozart/copilot/add-validation-for-o…
eduardomozart Jan 30, 2026
1469227
Update parameter type hint in areAllFieldsEmpty
eduardomozart Jan 30, 2026
56ec06c
Enhance OS addition tests with error message checks
eduardomozart Jan 30, 2026
252e5c0
Fix error message assertions in tests
eduardomozart Jan 30, 2026
514f884
Fix string quotes in error messages for consistency
eduardomozart Jan 30, 2026
91cec23
Fix operating systems ID reference in tests
eduardomozart Jan 30, 2026
91b922e
Fix string quotes in Item_OperatingSystemTest
eduardomozart Jan 30, 2026
df0e7b1
Fix references for operating system and architecture
eduardomozart Jan 30, 2026
022dc95
Update Item_OperatingSystem.php
eduardomozart Jan 30, 2026
bdab75c
Update item_operatingsystem.form.php
eduardomozart Jan 30, 2026
17c28e8
Change translation function from __() to __s()
eduardomozart Jan 30, 2026
c4a63b3
Update Item_OperatingSystem.php
eduardomozart Jan 30, 2026
5201d41
Replace prepareInputForUpdate to post_updateItem
eduardomozart Jan 30, 2026
083ce8e
Change test to verify record deletion on update
eduardomozart Jan 30, 2026
5786a3c
Clarify comment regarding update behavior in tests
eduardomozart Jan 30, 2026
8d87b50
Trim input fields before validation check
eduardomozart Jan 30, 2026
9519de7
Rename post_updateItem to prepareInputForUpdate
eduardomozart Jan 30, 2026
c84b32a
Refactor OS test messages for consistency
eduardomozart Jan 30, 2026
5428131
Refactor OS add tests for better validation
eduardomozart Jan 30, 2026
dad745e
Fix session message for empty operating system update
eduardomozart Jan 30, 2026
9d6a549
Improve error message clarity in OS tests
eduardomozart Jan 30, 2026
eeff193
Refactor testUpdateOSToEmptyIsRejected method
eduardomozart Jan 30, 2026
aefa813
Fix formatting in Item_OperatingSystemTest.php
eduardomozart Jan 30, 2026
0c98e19
Add test for capacity isUsed behavior
eduardomozart Jan 31, 2026
9bcdc05
Use isUsed() and add capacity usage test
eduardomozart Jan 31, 2026
eb5fc63
Adjust OS validation, messages, and tests
eduardomozart Jan 31, 2026
7e53981
Prevent empty operating-system updates
eduardomozart Jan 31, 2026
23fd0d6
Update Item_OperatingSystemTest.php
eduardomozart Jan 31, 2026
ca51ee6
Update HasOperatingSystemCapacityTest.php
eduardomozart Jan 31, 2026
218cc0e
Fix numeric string validation in OS item
eduardomozart Jan 31, 2026
602f613
Fix logical condition in input validation
eduardomozart Jan 31, 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
70 changes: 70 additions & 0 deletions src/Item_OperatingSystem.php
Original file line number Diff line number Diff line change
Expand Up @@ -662,9 +662,79 @@ public function prepareInputForAdd($input)
$item->getFromDB($input['items_id']);
$input['entities_id'] = $item->fields['entities_id'];
$input['is_recursive'] = $item->fields['is_recursive'];

// Check if all OS fields are empty
if ($this->areAllFieldsEmpty($input)) {
Session::addMessageAfterRedirect(
__s("Cannot add an empty operating system. At least one field must be filled."),
false,
ERROR
);
return false;
}

return $input;
}

public function prepareInputForUpdate($input)
{
// Check if all OS fields are empty
if (isset($input['id']) && $this->getFromDB($input['id'])) {
$merged = array_merge($this->fields, $input);

if ($this->areAllFieldsEmpty($merged)) {
Session::addMessageAfterRedirect(
__s("Cannot update operating system with empty values. To remove the operating system, use the delete action instead."),
false,
ERROR
);
return false;
}
}

return $input;
}

/**
* Check if all relevant OS fields are empty
*
* @param array<string, mixed> $input Input data
*
* @return bool True if all fields are empty
*/
private function areAllFieldsEmpty(array $input): bool
{
$fields_to_check = [
'operatingsystems_id',
'operatingsystemversions_id',
'operatingsystemservicepacks_id',
'operatingsystemarchitectures_id',
'operatingsystemkernelversions_id',
'operatingsystemeditions_id',
'license_number',
'licenseid',
'company',
'owner',
'hostid',
];

foreach ($fields_to_check as $field) {
if (!array_key_exists($field, $input)) {
continue;
}

$value = $input[$field];

if (
(is_numeric($value) && (int) $value > 0)
|| (is_string($value) && trim($value) !== '' && !is_numeric($value))
) {
return false;
}
}

return true;
}

public static function getIcon()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,78 @@ public function testCloneAsset()
);
}

/**
* @dataProvider provideIsUsed
*/
public function testIsUsed(string $target_classname): void
{
$definition = $this->initAssetDefinition(
capacities: [new Capacity($this->getTargetCapacity())]
);
$class = $definition->getAssetClassName();

$asset = $this->createItem($class, [
'name' => 'Test asset',
'entities_id' => $this->getTestRootEntity(true),
]);

$capacity_class = $this->getTargetCapacity();
$capacity = new $capacity_class();
$this->assertFalse($capacity->isUsed($class));

// An OS capacity is used only if a meaningful OS exists
$os = $this->createItem('OperatingSystem', [
'name' => 'Test OS',
]);

$input = [
'itemtype' => $asset::getType(),
'items_id' => $asset->getID(),
'operatingsystems_id' => $os->getID(),
];
$this->createItem($target_classname, $input);

$this->assertTrue($capacity->isUsed($class));
}

/**
* @dataProvider provideGetCapacityUsageDescription
*/
public function testGetCapacityUsageDescription(string $target_classname, string $expected): void
{
$definition = $this->initAssetDefinition(
capacities: [new Capacity($this->getTargetCapacity())]
);
$class = $definition->getAssetClassName();
$capacity = new ($this->getTargetCapacity());

$this->assertEquals(
sprintf($expected, 0, 0),
$capacity->getCapacityUsageDescription($class)
);

$asset = $this->createItem($class, [
'name' => 'Test asset',
'entities_id' => $this->getTestRootEntity(true),
]);

// An OS capacity is used only if a meaningful OS exists
$os = $this->createItem('OperatingSystem', [
'name' => 'Test OS',
]);

$this->createItem($target_classname, [
'itemtype' => $class,
'items_id' => $asset->getID(),
'operatingsystems_id' => $os->getID(),
]);

$this->assertEquals(
sprintf($expected, 1, 1),
$capacity->getCapacityUsageDescription($class)
);
}

public static function provideIsUsed(): iterable
{
yield [
Expand Down
103 changes: 103 additions & 0 deletions tests/functional/Item_OperatingSystemTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -314,4 +314,107 @@ public function testEntityAccess()
$this->setEntity('_test_child_2', true);
$this->assertTrue($ios->can($ios->getID(), READ));
}

public function testPreventEmptyOSAdd()
{
$this->login();
$computer = getItemByTypeName('Computer', '_test_pc01');

$ios = new Item_OperatingSystem();

// Test adding an OS with all empty fields - should fail
$input = [
'itemtype' => $computer->getType(),
'items_id' => $computer->getID(),
'operatingsystems_id' => 0,
'operatingsystemarchitectures_id' => 0,
'operatingsystemversions_id' => 0,
'operatingsystemkernelversions_id' => 0,
'operatingsystemeditions_id' => 0,
'operatingsystemservicepacks_id' => 0,
'licenseid' => '',
'license_number' => '',
];

// GLPI currently allows empty OS records
$this->assertFalse(
$ios->add($input),
"Should not be able to add an OS with all empty fields.",
);

// Check for the error message
$this->hasSessionMessages(ERROR, [
"Cannot add an empty operating system. At least one field must be filled.",
]);

$this->assertSame(
0,
Item_OperatingSystem::countForItem($computer),
"No OS should be linked after failed add."
);
}

public function testUpdateOSToEmptyIsRejected()
{
$this->login();
$computer = getItemByTypeName('Computer', '_test_pc01');
$objects = $this->createDdObjects();
$ios = new Item_OperatingSystem();

// First add a valid OS record
$input = [
'itemtype' => $computer->getType(),
'items_id' => $computer->getID(),
'entities_id' => $_SESSION['glpiactive_entity'],
'operatingsystems_id' => $objects['']->getID(),
'operatingsystemarchitectures_id' => $objects['Architecture']->getID(),
];

$id = $ios->add($input);
$this->assertGreaterThan(0, $id);
$this->assertSame(
1,
Item_OperatingSystem::countForItem($computer),
"Count should be 1 after add.",
);

// Reload and ensure record still exists
$this->assertTrue($ios->getFromDB($id));
// Snapshot original state
$original = $ios->fields;

// Attempt to update with empty values
$result = $ios->update([
'id' => $id,
'operatingsystems_id' => 0,
'operatingsystemarchitectures_id' => 0,
]);

// Update must be rejected
$this->assertFalse(
$result,
"Updating OS to empty values should be rejected.",
);

// Consume the session message so tearDown doesn't fail
$this->hasSessionMessages(ERROR, [
"Cannot update operating system with empty values. To remove the operating system, use the delete action instead.",
]);

// Verify data integrity - record must still exist and be unchanged
$this->assertTrue(
$ios->getFromDB($id),
"OS record should still exist."
);

// Asserting against real DB columns to ensure no changes were made
$this->assertSame(
$original['operatingsystems_id'],
$ios->fields['operatingsystems_id'],
);
$this->assertSame(
$original['operatingsystemarchitectures_id'],
$ios->fields['operatingsystemarchitectures_id'],
);
}
}