[Feature] Allow multiple "Insertion in the form" (dom) blocks per entity by JeremieMercier · Pull Request #941 · pluginsGLPI/fields

JeremieMercier · 2025-04-04T11:17:57Z

This PR provides the ability to define multiple "Insertion in the form" blocks for the same item based on its entity

Created a new method findContainers() (based on findContainer()) that returns all 'dom' containers for an item based on its entity (with parent entity handling via getAncestorsOf()).
Adapted hooks (pre_item_add, pre_item_update, post_item_add, post_item_update) to manage multiple containers using the _plugin_fields_data_multi array.
Updated the populateData() function to extract input values by stripping the prefix, ensuring that data is saved into the correct columns of the injection table.
Modified the container.form.php file to "clean" the form data (by removing the prefix) before calling updateFieldsValues(), thereby enabling the saving of domtab containers.

Please test these modifications and verify that everything works as expected for all object types supported by the Fields plugin. Note that my tests were successful, although I focused exclusively on 'Ticket' type objects for my use case.

Checklist before requesting a review

Please delete options that are not relevant.

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.

Issues : #744, #789, #834

stonebuzz · 2025-04-09T09:37:16Z

Hi @JeremieMercier

I am questioning the plugin's ability to reconcile the correct container during API calls, an issue we have encountered in the past. For example, when updating a ticket and including the "fields" in the payload, could you verify this point?

JeremieMercier · 2025-04-09T10:40:05Z

Hi @stonebuzz,

I retrieve the block row ID like this:

Then, I update the field using the following method:

So I haven’t encountered any issue with the API — unless I misunderstood how you're updating the fields via the API?

stonebuzz · 2025-04-09T11:03:44Z

It is also possible to update fields from the fields plugin during ticket update.

PUT {{URL_GLPI}} /Ticket/5

JeremieMercier · 2025-04-09T11:16:49Z

Yes, it works

Just include the block ID in the field name, like:
plugin_fields_15_bloconechamponefield

JeremieMercier · 2025-04-16T13:41:49Z

Hello, do you have time to review the PR ?

JeremieMercier · 2025-04-16T16:08:43Z

I applied the changes for PHPStan 🤞

JeremieMercier · 2025-04-17T12:55:42Z

I don't understand @trasher

With php 7.4 :

JeremieMercier · 2025-04-17T13:20:42Z

A syntax error, it was slipped in after applying the suggestion 😅

JeremieMercier · 2025-04-17T13:36:18Z

Are we good this time ? 😅🤞

JeremieMercier · 2025-04-23T12:36:39Z

Hi, what's the next step ?

stonebuzz · 2025-04-24T07:54:38Z

Hi @JeremieMercier

The feature you are proposing is substantial and introduces significant changes. To ensure its smooth integration and functionality, we must be extremely vigilant. This means we need to "lock down" our testing to guarantee that everything works as expected.

It is important to acknowledge that the plugin is already complex to maintain, especially with the new features introduced in GLPI. Given this complexity, we must approach any additional features with caution to ensure they do not further complicate maintenance and support.

To ensure the proper functioning of this PR, here is a concise list of unit tests that should be added:

Unit Tests for `findContainers`

Retrieve Containers for Specific Itemtype and Entity
- Verify that findContainers returns the correct containers for a given itemtype and entity.
Retrieve Containers with Subtype
- Verify that findContainers returns the correct containers when a subtype is specified.
Retrieve Containers with Recursive Entity
- Verify that findContainers considers recursive entities.

Unit Tests for Hooks

preItemAdd with Multiple Containers
- Verify that the preItemAdd hook correctly handles data for multiple containers.
postItemAdd with Multiple Containers
- Verify that the postItemAdd hook correctly updates field values for multiple containers.
preItemUpdate with Multiple Containers
- Verify that the preItemUpdate hook correctly handles data for multiple containers during item update.

Unit Tests for `populateData`

populateData with Prefix
- Verify that populateData correctly extracts field values by removing the prefix.
populateData with Multiple Selection Fields
- Verify that populateData correctly handles fields allowing multiple selections.

Unit Tests for `showForTab`

showForTab with Multiple Containers
- Verify that showForTab correctly displays containers for a given item.
showForTab with Insufficient Rights
- Verify that showForTab returns nothing if the user does not have sufficient rights on the containers.

Unit Tests for `API`

Ticket input update
Container update

Unit Tests for `rights`

JeremieMercier · 2025-05-07T23:38:10Z

Hi @stonebuzz Thanks for the message

I fully understand the concerns and the importance of thorough testing.
I'm currently working on implementing the required tests and will make sure all the listed cases are properly covered.

While working on the tests already partially in place, I’ve been able to identify and fix some issues especially related to "dropdown" field handling and container restriction management.

I’ll keep updating here as I progress.

akstis-typer · 2025-05-23T06:34:55Z

Hi, @JeremieMercier.
What tool you are using to send HTTP requests?
I'm newby in WEB development so I'm not entirely shure what tool is that.
Thank you :)

Edit: I've done some research and found Postman, as a tool to send HTTP requests and creating API tests

JeremieMercier · 2025-05-25T21:32:29Z

I’ve added all the requested unit tests (except the API ones):

findContainers : basic cases, sub-types, entity recursion
Hooks: preItem, postItemAdd, preItemUpdate with multiple containers
populateData: prefix handling and multi-selection fields
showForTab: displaying multiple containers and checking rights

Please note I’m not a testing expert, so the tests may not follow every usual best practice. Feel free to share any feedback so I can improve.

The API tests still need to be written by whoever is willing to tackle them.

CorentinS6 · 2025-07-18T11:43:08Z

Hello,
Tank you very much @JeremieMercier for this work. That's a realy needed feat for us !
Any followup of this PR ?
Best regards

JeremieMercier · 2025-07-21T15:50:46Z

Hello, Tank you very much @JeremieMercier for this work. That's a realy needed feat for us ! Any followup of this PR ? Best regards

Hello @CorentinS6, From my side everything seems good, it remains to be seen if a developer of the project can finalize the tests

CorentinS6 · 2025-08-26T07:52:16Z

And code generated by AI ?

JeremieMercier · 2025-08-28T08:22:45Z

And code generated by AI ?

Hello, All the logic is 100% mine, however for the tests, it's another story, I used AI to learn and understand the use of the tests in addition to videos like this one from Grafikart, but as I said, I'm not an expert..., the tests can also be redone by a more competent person if necessary knowing that basically, there were no tests set up on the plugin.

CorentinS6 · 2025-08-28T09:29:15Z

@stonebuzz , is this work good enough for the merge now?

Created a new method findContainers() (based on findContainer()) that returns all 'dom' containers for an item based on its entity (with parent entity handling via getAncestorsOf()). Adapted hooks (pre_item_add, pre_item_update, post_item_add, post_item_update) to manage multiple containers using the _plugin_fields_data_multi array. Updated the populateData() function to extract input values by stripping the prefix, ensuring that data is saved into the correct columns of the injection table. Modified the container.form.php file to "clean" the form data (by removing the prefix) before calling updateFieldsValues(), thereby enabling the saving of domtab containers. This PR provides the ability to define multiple "Insertion in the form" blocks for the same item based on its entity by leveraging the new findContainers() method and adapting the save process.

…oving the prefix) before calling updateFieldsValues(), thereby enabling the saving of domtab containers.

@var

- Added missing @var annotation for global $DB - Replaced empty() on $entityIds with count() - Removed redundant is_array() check on $entityRestriction - Simplified isset() condition by removing unnecessary !== null check

Note: PHPStan still reports a type error on DB::request() (expects array<string>|string), but this issue already exists elsewhere in the file and was not introduced by this change.

update: showForTab method fix: dropdown field

• New phpunit.xml and bootstrap for the Fields plugin • PluginFieldsContainerTest covers: - Creating / reading containers - Filtering containers by entity, subtype, recursion - Hooks: preItem, postItemAdd, preItemUpdate - populateData (single + multi-dropdown) - showForTab rendering and rights checks

Remove phpunit and require_once unnecessary

…ion'

JuanPabloToniolo-Udesa · 2025-12-12T11:30:11Z

This will be very useful, now I can only add one block to all entities, I need one per entity and this seems to solve the problem, is there something I can help with?

killerwanee · 2026-02-18T11:29:15Z

Hi,
I have some needs for that type of thing.
Like i have an entity that needs different informations on tickets than the other, but even tho my fields are created seperately on the entities, i'm blocked because it'es for the same "type".
Is there a way to do this in glpi 11 ?
Thank you

Rom1-B · 2026-07-22T07:04:27Z

+        $containerID = $_POST['plugin_fields_containers_id'];
+        $data = [];
+        foreach ($_REQUEST as $key => $value) {
+            // if key starts with plugin_fields_<containerID>_ remove the prefix
+            if (strpos($key, "plugin_fields_{$containerID}_") === 0) {
+                $new_key = substr($key, strlen("plugin_fields_{$containerID}_"));
+                $data[$new_key] = $value;
+            } else {
+                $data[$key] = $value;
+            }
+        }
        $container->updateFieldsValues($_REQUEST, $_REQUEST['itemtype'], false);


$data is computed but never used?!

Rom1-B · 2026-07-22T07:07:26Z

+            if (false !== ($data = self::populateData($c_id, $item))) {
+                if (!self::validateValues($data, $item->getType(), isset($_REQUEST['massiveaction']))) {
+                    // if validation fails, we need to remove the data from the item input
+                    $item->input = [];

-            return true;
+                    return false;
+                }
+                $all_data[] = $data;
+            }


When an item has several applicable containers, a validation failure on any single container wipes $item->input and returns immediately, discarding the data already accumulated in $all_data for the other, valid containers. In the single-container world this was the existing behaviour (nothing else to lose), but with multiple containers per item it means one invalid field in container B now also blocks saving valid data in container A. Please confirm whether this all-or-nothing semantics is intentional; if not, previously-validated containers' data should still make it into _plugin_fields_data_multi.

Rom1-B · 2026-07-22T07:09:08Z

+        $fieldTypeName = '_plugin_fields_type_' . $id;
+        $fieldSubTypeName = '_plugin_fields_subtype_' . $id;
+
        echo Html::hidden('_plugin_fields_type', ['value' => $type]);
        echo Html::hidden('_plugin_fields_subtype', ['value' => $subtype]);
+        echo Html::hidden($fieldTypeName, ['value' => $type]);
+        echo Html::hidden($fieldSubTypeName, ['value' => $subtype]);


_plugin_fields_type_{id} / _plugin_fields_subtype_{id} are added alongside the existing non-suffixed hidden fields, but no other code (PHP, Twig, or JS) in the plugin reads them back. Either wire them up (presumably needed to disambiguate type/subtype per container once several dom containers coexist on one page) or drop them.

Rom1-B · 2026-07-22T07:09:58Z

The diff downgrades sebastian/diff from 6.0.2 (PHP >=8.2, dev-requires phpunit/phpunit ^11.0) to 4.0.6 (PHP >=7.3, dev-requires phpunit/phpunit ^9.3), alongside an unrelated phpstan/phpstan patch bump. This looks like stale lockfile drift from an out-of-sync branch rather than an intentional change for this feature. Suggest regenerating the lockfile with a clean composer update against current composer.json before merge.

Rom1-B · 2026-07-22T07:10:57Z

+            'entities_id'                               => 0,
+        ]);
+
+        // 3. Injecter les données comme si elles venaient du formulaire


Should be in English per the project's coding conventions.

stonebuzz requested review from Rom1-B and stonebuzz April 7, 2025 07:05

stonebuzz added enhancement feature suggestion labels Apr 7, 2025

JeremieMercier mentioned this pull request Apr 11, 2025

[Feature] Notification target ticket #943

Open

4 tasks

trasher reviewed Apr 17, 2025

View reviewed changes

Comment thread inc/container.class.php Outdated

Rom1-B reviewed Apr 17, 2025

View reviewed changes

Comment thread inc/container.class.php Outdated

stonebuzz assigned JeremieMercier May 20, 2025

stonebuzz added the Need unit tests label May 20, 2025

JeremieMercier force-pushed the feature/multiple-dom-blocks branch 2 times, most recently from 051fcd8 to cf0f71f Compare May 29, 2025 16:46

JeremieMercier force-pushed the feature/multiple-dom-blocks branch from af76515 to 1aa2594 Compare October 7, 2025 19:35

Rom1-B requested changes Oct 9, 2025

View reviewed changes

Comment thread tests/bootstrap.php Outdated

Comment thread composer.json Outdated

Comment thread phpunit.xml Outdated

JeremieMercier requested a review from Rom1-B October 9, 2025 09:16

JeremieMercier added 17 commits October 10, 2025 22:14

Modified the container.form.php file to "clean" the form data (by rem…

67b011b

…oving the prefix) before calling updateFieldsValues(), thereby enabling the saving of domtab containers.

fix: Undefined array key "entities_id"

13ea826

fix: PHPStan warnings in findContainers()

519a5bd

- Added missing @var annotation for global $DB - Replaced empty() on $entityIds with count() - Removed redundant is_array() check on $entityRestriction - Simplified isset() condition by removing unnecessary !== null check

update: use GLPI DBIterator

eb05cad

Note: PHPStan still reports a type error on DB::request() (expects array<string>|string), but this issue already exists elsewhere in the file and was not introduced by this change.

update: container.class.php

2d442f9

fix: last change

ab8560c

update for phpstan

58f8e34

update: findContainers method to use entityId instead of itemId

621bfe2

update: showForTab method fix: dropdown field

composer lock generate

3aedb41

chore(composer): sync lock (phpunit + phpstan bump)

9fa7eb3

composer update

660a6d4

Simplify bootstrap and standardize phpunit config

d685bc5

Remove phpunit and require_once unnecessary

fix(container): correct entity ID and add 'use Glpi\DBAL\QueryExpress…

246a764

…ion'

php cs fixer

829e78a

Add licence header

aaf0f14

JeremieMercier force-pushed the feature/multiple-dom-blocks branch from 7ee0d82 to aaf0f14 Compare October 10, 2025 20:29

fix fusion

9f6178f

Rom1-B requested changes Jul 22, 2026

View reviewed changes

Uh oh!

Conversation

JeremieMercier commented Apr 4, 2025

Checklist before requesting a review

Uh oh!

stonebuzz commented Apr 9, 2025

Uh oh!

JeremieMercier commented Apr 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stonebuzz commented Apr 9, 2025

Uh oh!

JeremieMercier commented Apr 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JeremieMercier commented Apr 16, 2025

Uh oh!

JeremieMercier commented Apr 16, 2025

Uh oh!

Uh oh!

JeremieMercier commented Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

JeremieMercier commented Apr 17, 2025

Uh oh!

JeremieMercier commented Apr 17, 2025

Uh oh!

JeremieMercier commented Apr 23, 2025

Uh oh!

stonebuzz commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Tests for findContainers

Unit Tests for Hooks

Unit Tests for populateData

Unit Tests for showForTab

Unit Tests for API

Unit Tests for rights

Uh oh!

JeremieMercier commented May 7, 2025

Uh oh!

akstis-typer commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JeremieMercier commented May 25, 2025

Uh oh!

CorentinS6 commented Jul 18, 2025

Uh oh!

JeremieMercier commented Jul 21, 2025

Uh oh!

CorentinS6 commented Aug 26, 2025

Uh oh!

JeremieMercier commented Aug 28, 2025

Uh oh!

CorentinS6 commented Aug 28, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JuanPabloToniolo-Udesa commented Dec 12, 2025

Uh oh!

killerwanee commented Feb 18, 2026

Uh oh!

Rom1-B Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

Rom1-B Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

Rom1-B Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

Rom1-B Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

Rom1-B Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

JeremieMercier commented Apr 9, 2025 •

edited

Loading

JeremieMercier commented Apr 9, 2025 •

edited

Loading

JeremieMercier commented Apr 17, 2025 •

edited

Loading

stonebuzz commented Apr 24, 2025 •

edited

Loading

Unit Tests for `findContainers`

Unit Tests for `populateData`

Unit Tests for `showForTab`

Unit Tests for `API`

Unit Tests for `rights`

akstis-typer commented May 23, 2025 •

edited

Loading