Conversation
|
CHANGELOG please (always update it) |
| protected static function get_label_value(string $label, mixed $value): string | ||
| { | ||
| return '<b><i>' . sprintf(__s('%1$s: %2$s'), $label . '</i></b>', $value); | ||
| } | ||
|
|
||
| protected static function get_dropdown_values(string $table, mixed $values): string | ||
| { | ||
| if (!is_array($values)) { | ||
| return Toolbox::stripTags(Dropdown::getDropdownName($table, $values)); | ||
| } | ||
|
|
||
| $names = array_filter(array_map( | ||
| static fn($value) => Toolbox::stripTags(Dropdown::getDropdownName($table, $value)), | ||
| $values, | ||
| )); | ||
|
|
||
| return implode(', ', $names); | ||
| } | ||
|
|
||
| protected static function get_group_value(CommonGLPI $item, string $field = 'groups_id'): string | ||
| { | ||
| return self::get_dropdown_values('glpi_groups', $item->fields[$field] ?? []); | ||
| } | ||
|
|
||
| protected static function get_group_column(CommonGLPI $item, string $field = 'groups_id', ?string $label = null): string | ||
| { | ||
| return self::get_label_value($label ?? __s('Group'), self::get_group_value($item, $field)); | ||
| } | ||
|
|
||
| protected static function display_group_line(PluginPdfSimplePDF $pdf, CommonGLPI $item, string $right_column): void | ||
| { | ||
| $pdf->displayLine(self::get_group_column($item), $right_column); | ||
| } | ||
|
|
There was a problem hiding this comment.
I would recommend not adding a dedicated function to manage columns, rows, or labels for groups.
As it stands, only a very small portion of the plugin would actually use it, while the vast majority still relies on $pdf->displayLine().
Regarding the handling of multi-group assignments:
Group_Item, being a CommonDBRelation, provides the getItemsAssociatedTo method. This allows us to retrieve all groups associated with a given asset.
(Note that some assets may not have multiple groups.)
Therefore, the logic should be conditional:
if (!Toolbox::hasTrait(Asset::class, AssignableItem::class)) {
Dropdown::getDropdownName(
'glpi_groups',
$item->fields['groups_id_tech']
);
} else {
$group_item = new Group_Item();
$groups = $group_item->getItemsAssociatedTo(Asset::class, $items_id);
//here filter groups depending of type (groups_id, groups_id_tech etc ...à
}After that, the line can be constructed using displayLine.
stonebuzz
left a comment
There was a problem hiding this comment.
Better 🙂
However, we now have a significant amount of duplicated code. We should refactor and consolidate these methods into a single unified implementation.
|
@stonebuzz can I send it to the client ? |
| if (!Toolbox::hasTrait($item::class, AssignableItem::class)) { | ||
| return Toolbox::stripTags(Dropdown::getDropdownName('glpi_groups', $item->fields[$field])); | ||
| } | ||
|
|
||
| $group_item = new Group_Item(); | ||
| $groups = $group_item->getItemsAssociatedTo($item::class, (int) $item->fields['id']); | ||
|
|
||
| $group_ids = []; | ||
| foreach ($groups as $group) { | ||
| if ((int) $group->fields['type'] === $group_type) { | ||
| $group_ids[] = (int) $group->fields['groups_id']; | ||
| } | ||
| } |
There was a problem hiding this comment.
Can you try this?:
| if (!Toolbox::hasTrait($item::class, AssignableItem::class)) { | |
| return Toolbox::stripTags(Dropdown::getDropdownName('glpi_groups', $item->fields[$field])); | |
| } | |
| $group_item = new Group_Item(); | |
| $groups = $group_item->getItemsAssociatedTo($item::class, (int) $item->fields['id']); | |
| $group_ids = []; | |
| foreach ($groups as $group) { | |
| if ((int) $group->fields['type'] === $group_type) { | |
| $group_ids[] = (int) $group->fields['groups_id']; | |
| } | |
| } | |
| $group_ids = (array) ($item->fields[$field] ?? 0); |
Description
groups_idandgroups_id_techwere always scalar values, while GLPI now loads these fields fromglpi_groups_itemsas arrays for assignable items. As a result, group-related fields could be empty or incorrectly rendered in generated PDFs.The classes updated are the ones impacted beacause they correspond to assets implementing the AssignableItem feature.
Screenshots :
Before fix :
After fix :