Skip to content
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

Jump to bottom

Add inventoriable capacity #17807

Merged
merged 2 commits into from
Sep 17, 2024
Merged

Add inventoriable capacity #17807

merged 2 commits into from
Sep 17, 2024

Conversation

trasher
Copy link
Contributor

@trasher trasher commented Sep 6, 2024

Checklist before requesting a review

Please delete options that are not relevant.

  • I have read the CONTRIBUTING document.
  • I have performed a self-review of my code.
  • I have added tests that prove my fix is effective or that my feature works.
  • This change requires a documentation update.

Description

Add inventory for generic assets

@trasher trasher force-pushed the feature/generics-inventory branch from f4f2249 to 3563c47 Compare September 6, 2024 13:41
@trasher
Copy link
Contributor Author

trasher commented Sep 6, 2024

Currently, as you can see in tests; activated capacities are ignored. As soon as the inventory capacity is set; OS, Software, Volumes, and so on will be inventoried.
We have to define if those should be added or not if disabled for the asset; but not only from inventory (I guess issue is currently the same using API).

And we have the case of devices that are not a capacity for generic assets, but are not inventoried until related CFG_GLPI entry is properly set (done from inventoriable capacity for current PoC).

This PR has been opened as a PoC, to be discussed in our next technical meeting. ping @orthagh @cedric-anne @AdrienClairembault @flegastelois @stonebuzz

@trasher trasher force-pushed the feature/generics-inventory branch 4 times, most recently from 3b4c933 to a8becc9 Compare September 11, 2024 19:51
cedric-anne
cedric-anne reviewed Sep 12, 2024
View reviewed changes
Copy link
Member

@cedric-anne cedric-anne left a comment

Choose a reason for hiding this comment

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

All the proposed changes so far seems almost OK to me.

I think the Glpi\Asset\Asset class should use the Inventoriable trait.

I did not reviewed tests yet.

src/Glpi/Inventory/Asset/Device.php Outdated Show resolved Hide resolved
src/Glpi/Asset/Capacity/IsInventoriableCapacity.php Outdated Show resolved Hide resolved
src/Glpi/Asset/Capacity/IsInventoriableCapacity.php Outdated Show resolved Hide resolved
src/Glpi/Asset/Capacity/IsInventoriableCapacity.php Outdated Show resolved Hide resolved
src/Glpi/Asset/Capacity/IsInventoriableCapacity.php Show resolved Hide resolved
@trasher trasher force-pushed the feature/generics-inventory branch from baea9f3 to 10df957 Compare September 12, 2024 11:50
@cedric-anne cedric-anne mentioned this pull request Sep 12, 2024
Merged
3 tasks
@trasher trasher force-pushed the feature/generics-inventory branch 2 times, most recently from 7e6870c to 4132e42 Compare September 13, 2024 06:24
@trasher
Copy link
Contributor Author

trasher commented Sep 13, 2024

Failing tests on HtmlTest seems due to the way generic asset definitions currently works; I've not been able to find a workaround for that - I even do not understand why there is only 1 asset definition left.

It's easy to reproduce by running only GenericAssetInventory and Html:

./vendor/bin/phpunit phpunit/functional/Glpi/Inventory/GenericAssetInventoryTest.php phpunit/functional/HtmlTest.php

@trasher trasher force-pushed the feature/generics-inventory branch 2 times, most recently from 844d77b to a5e31e6 Compare September 16, 2024 12:11
@trasher
Copy link
Contributor Author

trasher commented Sep 16, 2024

Should be OK for review now. I do not know why phptsan is not happy (and specially why from this PR).

@trasher trasher marked this pull request as ready for review September 16, 2024 12:51
@AdrienClairembault
Copy link
Contributor

For PHPStan, I had the same issue here: https://github.com/glpi-project/glpi/pull/17809/files#diff-e59250d2d794cb4d689bceb57dd6a1fa453d1cfab6bf595af089e5ebf0294c73R386

It seems to think that DBmysqlIterator can never be empty for some unknown reason.
Maybe an invalid PHPdoc somewhere...

@trasher
Copy link
Contributor Author

trasher commented Sep 17, 2024

I've ignored the CS issue, a useless check won't break anything.

@trasher
Add inventoriable capacity
b5fb4a3 
Update DB schema with missing fields
Add configuration types for environment and process, use them in inventory
Add checks for Monitor and Peripherals
Add missing devices types in configuration
Add new tests
Fix Monitor and Printer inventory
Prevent numerous Undefined array key "glpi_currenttime"  loading bootstrap data
@trasher trasher force-pushed the feature/generics-inventory branch from 91b624c to b5fb4a3 Compare September 17, 2024 06:40
@trasher trasher mentioned this pull request Sep 17, 2024
Merged
4 tasks
@trasher trasher merged commit 50a87ef into glpi-project:main Sep 17, 2024
7 checks passed
@trasher trasher deleted the feature/generics-inventory branch September 17, 2024 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AdrienClairembault AdrienClairembault AdrienClairembault approved these changes

@orthagh orthagh orthagh approved these changes

@cedric-anne cedric-anne Awaiting requested review from cedric-anne

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@trasher @AdrienClairembault @orthagh @cedric-anne