Add validation for OS form#22913
Open
eduardomozart wants to merge 36 commits intoglpi-project:11.0/bugfixesfrom
Open
Add validation for OS form#22913eduardomozart wants to merge 36 commits intoglpi-project:11.0/bugfixesfrom
eduardomozart wants to merge 36 commits intoglpi-project:11.0/bugfixesfrom
Conversation
Co-authored-by: eduardomozart <2974895+eduardomozart@users.noreply.github.com>
Co-authored-by: eduardomozart <2974895+eduardomozart@users.noreply.github.com>
Co-authored-by: eduardomozart <2974895+eduardomozart@users.noreply.github.com>
…lify validation logic Co-authored-by: eduardomozart <2974895+eduardomozart@users.noreply.github.com>
…s-form Prevent empty Operating System records from being created or persisted
Added checks for error messages when adding an OS with empty fields and updated object references for adding valid OS.
trasher
reviewed
Jan 30, 2026
Updated the test to check record deletion after update.
Updated comment to clarify the behavior of the update process.
Refactor method to prepare input for update and handle empty fields.
Removed tests for adding operating systems with empty values and added assertions for valid input.
Introduce testIsUsed in HasOperatingSystemCapacityTest to verify a capacity's check() returns true for a fresh asset and becomes false after creating a related item. The test initializes an asset definition with the target capacity, creates an asset, asserts the capacity is initially available, creates the related resource via the data provider target class, and finally asserts the capacity is reported as used.
Replace legacy capacity->check() assertions with capacity->isUsed($class) and update expectations accordingly. Add testGetCapacityUsageDescription to verify getCapacityUsageDescription returns the expected formatted usage strings (for 0 and 1 usages) using the provideGetCapacityUsageDescription data provider, creating test assets and related items to assert usage counts.
Update operating system validation and tests: - Change session message to advise using delete to remove an OS instead of updating with empty values. - Tighten field-check logic to only call trim() on string inputs (add is_string guard) to avoid trimming non-string values. - Update functional tests to reflect that empty OS records can be added at the model level (assertGreaterThan for add) and that update() returns true while emitting the session error message and preserving original values. Tests also assert the updated error message and add an extra assertion for the architecture field to ensure integrity. Files changed: src/Item_OperatingSystem.php, tests/functional/Item_OperatingSystemTest.php.
When updating an Operating System record, merge incoming input with the existing DB fields and reject the update only if the resulting record would be entirely empty. Tighten per-field checks to skip keys that aren't present and treat only positive integers or non-empty trimmed strings as populated. Update functional tests to expect add/update of fully empty OS data to be rejected and to assert the appropriate error behavior.
Adjust validation in Item_OperatingSystem to treat numeric values (including numeric strings) consistently: replace is_int() with is_numeric() and make the string branch exclude numeric strings. This prevents numeric strings like "0" from being treated as non-empty text and incorrectly failing validation, and also supports floats and numeric input correctly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist before requesting a review
Please delete options that are not relevant.
Description
Empty Operating System records could be created and persisted in
glpi_items_operatingsystems, causing database bloat and incorrect tab counts. This occurred when submitting the OS form with all fields left empty or set to zero.Validation logic (
src/Item_OperatingSystem.php):prepareInputForAdd(): Returnsfalsewhen all OS fields are empty, preventing record creationprepareInputForUpdate(): Deletes the record when all fields are cleared during updateareAllFieldsEmpty(): Checks 11 fields of/templates/pages/assets/operatingsystem.html.twig(OS ID, version, architecture, service pack, kernel version, edition, license number, license ID, company, owner, host ID). A record is valid if any of the 11 checked fields has a non-empty, non-zero value.Test coverage (
tests/functional/Item_OperatingSystemTest.php):Screenshots (if appropriate):