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
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.
3 participants
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 :