fix(commoninjectionlib.class.php): normalize group assignment fields for assignable items only#631
fix(commoninjectionlib.class.php): normalize group assignment fields for assignable items only#631TarekRemo wants to merge 4 commits into
Conversation
…for assignable items only
|
I would appreciate having a unit test covering this part in order to definitively address the issue, as it has come up several times. This would also provide a good starting point for building our unit test coverage.Ideally, this should be validated with a real test involving the injection of a CSV file. |
I added unit tests. Didn't use a real CSV file but the tests simulate the starting point when a real CSV is injected. |
|
|
||
| final class GroupInjectionTest extends DbTestCase | ||
| { | ||
| public static function AssigneGroupToInjectedAssignableItemProvider(): array |
There was a problem hiding this comment.
| public static function AssigneGroupToInjectedAssignableItemProvider(): array | |
| public static function assignGroupToInjectedAssignableItemProvider(): array |
|
|
||
| /** | ||
| * | ||
| * @dataProvider assigneGroupToInjectedAssignableItemProvider |
There was a problem hiding this comment.
| * @dataProvider assigneGroupToInjectedAssignableItemProvider | |
| * @dataProvider assignGroupToInjectedAssignableItemProvider |
|
|
||
| use function Safe\realpath; | ||
|
|
||
| $current_plugin_folder = basename(realpath(__DIR__ . '/../')); |
| /** | ||
| * Updating an existing non assignable item via injection must also persist groups_id. | ||
| */ | ||
| public function testGroupIsUpdatedOnExistingNonAssugnableItem(): void |
There was a problem hiding this comment.
| public function testGroupIsUpdatedOnExistingNonAssugnableItem(): void | |
| public function testGroupIsUpdatedOnExistingNonAssignableItem(): void |
| // Normalize group assignment fields to GLPI relation payload keys. | ||
| // Some search options do not expose a stable joinparams shape, but GLPI | ||
| // still expects _groups_id/_groups_id_tech arrays when saving equipment groups. | ||
| // Only apply to items using the AssignableItem trait. |
There was a problem hiding this comment.
| // Only apply to items using the AssignableItem trait. | |
| // Only apply to items using the AssignableItem trait. | |
| // Non-AssignableItem types store groups_id as a plain FK; skip array normalisation for them. |
|
Awaiting customer approval |
2 participants
Checklist before requesting a review
Please delete options that are not relevant.
Description
groups_id → _groups_id(array)) was triggered for any item with a search option pointing toglpi_groups. Some items (example:ITILCategory) storegroups_idas a direct integer FK, not viaGroup_Item. For these items, the_groups_idarray this block produced was silently ignored, leavinggroups_id = 0.AssignableItemtrait to the guard condition. The rewrite now only happens for items likeComputerthat actually has this trait and expect group assignments throughGroup_Itemrelations.