Skip to content

Feat - Inject Generic Asset - #693

Open
RomainLvr wants to merge 3 commits into
mainfrom
feat/inject-custom-assets
Open

RomainLvr wants to merge 3 commits into
mainfrom
feat/inject-custom-assets

Conversation

@RomainLvr

Copy link
Copy Markdown
Contributor

Checklist before requesting a review

  • I have performed a self-review of my code.
  • I have added tests (when available) that prove my fix is effective or that my feature works.
  • I have updated the CHANGELOG with a short functional description of the fix or new feature.
  • This change requires a documentation update.

Description

Supersedes #620.

Generic assets can now be imported with Data Injection. Every active asset definition appears in the list of types, if the user's profile is allowed to create that asset.

Supported fields:

  • Standard fields, including Type, Model, Comment and both groups. A missing type or model is created for the asset definition being imported.
  • Custom fields: text, long text, number, date, date and time, yes/no and dropdown, including custom dropdowns. A custom field can also be used as the field that finds an existing asset.
  • Linked items such as the operating system or the financial information, when the asset definition has the matching capacity.

Not supported: multiple-choice dropdowns and fields hidden for the user's profile.

All generic assets share the same database table, so an update only matches assets of the same definition.

@RomainLvr
RomainLvr requested a review from a team October 26, 2026 16:32
@RomainLvr RomainLvr self-assigned this Oct 26, 2026
@RomainLvr
RomainLvr requested review from Herafia and Lainow and removed request for a team October 26, 2026 16:32

@Lainow Lainow left a comment

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.

Asset types, models, and custom values in drop-down menus are shared across all definitions, and the kernel’s findID() function does not filter by definition (no getSystemSQLCriteria()). Thus, if two definitions both contain a type called “Server,” the injection into B binds the asset to the type from A, without this being flagged.

Comment thread src/AssetInjection.php Resolved

@Herafia Herafia left a comment •

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.

To add to what @Lainow said
The cleanest fix would be in core: add + static::getSystemSQLCriteria() to the WHERE of CommonDropdown::findID() and CommonTreeDropdown::findID(). This also affects other imports, not only datainjection

We need a test that replicates this scenario.
ex: Definition A already has the type “Server,” the model “X42,” and the value “Blue” in its custom list. We inject these same names into definition B, then verify that the asset is linked to the items in B and not to those in A

@RomainLvr

Copy link
Copy Markdown
Contributor Author

This PR now depends on the core fix. Its CI will stay red until the core fix is merged on 12.0/bugfixes

Herafia
Herafia previously approved these changes Oct 27, 2026

@Herafia Herafia left a comment

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.

LGTM with #26099

jdurand-teclib
jdurand-teclib previously approved these changes Oct 28, 2026

@Lainow Lainow left a comment

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 non-overwrite option is ignored for custom fields.

@RomainLvr
RomainLvr requested a review from Lainow October 29, 2026 14:22
@RomainLvr

Copy link
Copy Markdown
Contributor Author

This fix will only be compatible starting with GLPI 12.0.2; this should be taken into account for the next release of the plugin

@stonebuzz stonebuzz mentioned this pull request Nov 3, 2026
3 of 4 tasks
@RomainLvr
RomainLvr dismissed stale reviews from jdurand-teclib and Herafia via 8ff83e7 November 3, 2026 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants