diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f42bf7..1a14412 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Fix image (field : description) in to pdf - Fix massive action PDF export redirecting to item list instead of generating the PDF +- Fix PDF export for assignable asset groups by properly handling multiple groups ## [4.1.2] - 2026-01-08 diff --git a/composer.json b/composer.json index d3e2efd..982e2db 100644 --- a/composer.json +++ b/composer.json @@ -18,7 +18,8 @@ }, "autoload-dev": { "psr-4": { - "Glpi\\Tools\\": "../../tools/src/" + "Glpi\\Tools\\": "../../tools/src/", + "GlpiPlugin\\Pdf\\Tests\\": "tests/" } } } diff --git a/inc/common.class.php b/inc/common.class.php index a883614..85ed404 100644 --- a/inc/common.class.php +++ b/inc/common.class.php @@ -37,6 +37,21 @@ abstract class PluginPdfCommon extends CommonGLPI public static $rightname = 'plugin_pdf'; + protected static function getGroupName(CommonDBTM $item, int $group_type = Group_Item::GROUP_TYPE_NORMAL): string + { + $field = $group_type === Group_Item::GROUP_TYPE_TECH ? 'groups_id_tech' : 'groups_id'; + + // $item->fields[$field] is already an array of group ids for items using the + // AssignableItem trait (many-to-many relation via glpi_groups_items), and a scalar + // group id for items using a direct one-to-one column (e.g. glpi_itilcategories). + $group_ids = (array) ($item->fields[$field] ?? 0); + + return implode(', ', array_filter(array_map( + static fn($group_id) => Toolbox::stripTags(Dropdown::getDropdownName('glpi_groups', $group_id)), + $group_ids, + ))); + } + /** * Constructor, should intialize $this->obj property **/ @@ -509,10 +524,7 @@ public static function mainLine(PluginPdfSimplePDF $pdf, $item, $field) '' . sprintf( __s('%1$s: %2$s'), __s('Group in charge of the hardware') . '', - Dropdown::getDropdownName( - 'glpi_groups', - $item->fields['groups_id_tech'], - ), + self::getGroupName($item, Group_Item::GROUP_TYPE_TECH), ), '' . sprintf( __s('%1$s: %2$s'), diff --git a/inc/computer.class.php b/inc/computer.class.php index c8851cf..6861ff8 100644 --- a/inc/computer.class.php +++ b/inc/computer.class.php @@ -89,12 +89,13 @@ public static function pdfMain(PluginPdfSimplePDF $pdf, Computer $computer) '' . sprintf( __('%1$s: %2$s'), __('Group') . '', - Dropdown::getDropdownName( - 'glpi_groups', - $computer->fields['groups_id'], - ), + self::getGroupName($computer), + ), + '' . sprintf( + __('%1$s: %2$s'), + __('UUID') . '', + $computer->fields['uuid'], ), - '' . sprintf(__('%1$s: %2$s'), __('UUID') . '', $computer->fields['uuid']), ); $pdf->displayLine( diff --git a/inc/monitor.class.php b/inc/monitor.class.php index 5d7ccc7..5a8516d 100644 --- a/inc/monitor.class.php +++ b/inc/monitor.class.php @@ -61,11 +61,7 @@ public static function pdfMain(PluginPdfSimplePDF $pdf, Monitor $item) PluginPdfCommon::mainLine($pdf, $item, 'user-management'); $pdf->displayLine( - '' . sprintf( - __s('%1$s: %2$s'), - __s('Group') . '', - Dropdown::getDropdownName('glpi_groups', $item->fields['groups_id']), - ), + '' . sprintf(__s('%1$s: %2$s'), __s('Group') . '', self::getGroupName($item)), '' . sprintf( __s('%1$s: %2$s'), __s('Size') . '', diff --git a/inc/networkequipment.class.php b/inc/networkequipment.class.php index cbd1155..993d199 100644 --- a/inc/networkequipment.class.php +++ b/inc/networkequipment.class.php @@ -83,11 +83,7 @@ public static function pdfMain(PluginPdfSimplePDF $pdf, NetworkEquipment $item) ); $pdf->displayLine( - '' . sprintf( - __s('%1$s: %2$s'), - __s('Group') . '', - Dropdown::getDropdownName('glpi_groups', $item->fields['groups_id']), - ), + '' . sprintf(__s('%1$s: %2$s'), __s('Group') . '', self::getGroupName($item)), '' . __s('The MAC address and the IP of the equipment are included in an aggregated network port'), '' . sprintf( __s('%1$s: %2$s'), diff --git a/inc/peripheral.class.php b/inc/peripheral.class.php index 9f38312..e5ab9f2 100644 --- a/inc/peripheral.class.php +++ b/inc/peripheral.class.php @@ -63,11 +63,7 @@ public static function pdfMain(PluginPdfSimplePDF $pdf, Peripheral $item) PluginPdfCommon::mainLine($pdf, $item, 'user-management'); $pdf->displayLine( - '' . sprintf( - __s('%1$s: %2$s'), - __s('Group') . '', - Dropdown::getDropdownName('glpi_groups', $item->fields['groups_id']), - ), + '' . sprintf(__s('%1$s: %2$s'), __s('Group') . '', self::getGroupName($item)), '' . sprintf(__s('%1$s: %2$s'), __s('Brand') . '', $item->fields['brand']), ); diff --git a/inc/phone.class.php b/inc/phone.class.php index e35cf92..187fe79 100644 --- a/inc/phone.class.php +++ b/inc/phone.class.php @@ -64,16 +64,8 @@ public static function pdfMain(PluginPdfSimplePDF $pdf, Phone $item) $pdf->displayLine( - '' . sprintf( - __s('%1$s: %2$s'), - __s('Group') . '', - Dropdown::getDropdownName('glpi_groups', $item->fields['groups_id']), - ), - '' . sprintf( - __s('%1$s: %2$s'), - __s('UUID') . '', - $item->fields['uuid'], - ), + '' . sprintf(__s('%1$s: %2$s'), __s('Group') . '', self::getGroupName($item)), + '' . sprintf(__s('%1$s: %2$s'), __s('UUID') . '', $item->fields['uuid']), ); $pdf->displayLine( diff --git a/inc/printer.class.php b/inc/printer.class.php index d493fc0..b3ab619 100644 --- a/inc/printer.class.php +++ b/inc/printer.class.php @@ -96,16 +96,8 @@ public static function pdfMain(PluginPdfSimplePDF $pdf, Printer $printer) ); $pdf->displayLine( - '' . sprintf( - __s('%1$s: %2$s'), - __s('Group') . '', - Dropdown::getDropdownName('glpi_groups', $printer->fields['groups_id']), - ), - '' . sprintf( - __s('%1$s: %2$s'), - __s('UUID') . '', - $printer->fields['uuid'], - ), + '' . sprintf(__s('%1$s: %2$s'), __s('Group') . '', self::getGroupName($printer)), + '' . sprintf(__s('%1$s: %2$s'), __s('UUID') . '', $printer->fields['uuid']), ); diff --git a/inc/software.class.php b/inc/software.class.php index 21a3526..c48fee3 100644 --- a/inc/software.class.php +++ b/inc/software.class.php @@ -93,10 +93,7 @@ public static function pdfMain(PluginPdfSimplePDF $pdf, Software $software) '' . sprintf( __s('%1$s: %2$s'), __s('Group in charge of the hardware') . '', - Dropdown::getDropdownName( - 'glpi_groups', - $software->fields['groups_id_tech'], - ), + self::getGroupName($software, Group_Item::GROUP_TYPE_TECH), ), '' . sprintf( __s('%1$s: %2$s'), @@ -109,7 +106,7 @@ public static function pdfMain(PluginPdfSimplePDF $pdf, Software $software) '' . sprintf( __s('%1$s: %2$s'), __s('Group') . '', - Dropdown::getDropdownName('glpi_groups', $software->fields['groups_id']), + self::getGroupName($software), ), ); diff --git a/phpunit.xml b/phpunit.xml new file mode 100644 index 0000000..3023d8c --- /dev/null +++ b/phpunit.xml @@ -0,0 +1,7 @@ + + + + tests + + + diff --git a/tests/Units/GetGroupNameTest.php b/tests/Units/GetGroupNameTest.php new file mode 100644 index 0000000..597d603 --- /dev/null +++ b/tests/Units/GetGroupNameTest.php @@ -0,0 +1,133 @@ +. + * + * @author Nelly Mahu-Lasson, Remi Collet, Teclib + * @copyright Copyright (c) 2009-2022 PDF plugin team + * @license AGPL License 3.0 or (at your option) any later version + * @link https://github.com/pluginsGLPI/pdf/ + * @link http://www.glpi-project.org/ + * @package pdf + * @since 2009 + * http://www.gnu.org/licenses/agpl-3.0-standalone.html + * -------------------------------------------------------------------------- + */ + +namespace GlpiPlugin\Pdf\Tests\Units; + +use Computer; +use Glpi\Tests\DbTestCase; +use Group; +use Group_Item; +use ITILCategory; +use PluginPdfCommon; +use ReflectionMethod; + +final class GetGroupNameTest extends DbTestCase +{ + private function getGroupName(\CommonDBTM $item, int $group_type = Group_Item::GROUP_TYPE_NORMAL): string + { + $method = new ReflectionMethod(PluginPdfCommon::class, 'getGroupName'); + $method->setAccessible(true); + + return $method->invoke(null, $item, $group_type); + } + + /** + * One-to-one relation: the group id is stored directly in the `groups_id` column + * (e.g. glpi_itilcategories), as a scalar value. + */ + public function testScalarGroupOnOneToOneRelation(): void + { + $group = $this->createItem(Group::class, [ + 'name' => 'One-to-one group', + 'entities_id' => $this->getTestRootEntity(true), + ]); + + $category = $this->createItem(ITILCategory::class, [ + 'name' => 'Category with a group', + 'entities_id' => $this->getTestRootEntity(true), + 'groups_id' => $group->getID(), + ]); + $category->getFromDB($category->getID()); + + $this->assertSame($group->fields['name'], $this->getGroupName($category)); + } + + public function testNoGroupOnOneToOneRelation(): void + { + $category = $this->createItem(ITILCategory::class, [ + 'name' => 'Category without a group', + 'entities_id' => $this->getTestRootEntity(true), + ]); + $category->getFromDB($category->getID()); + + $this->assertSame('', $this->getGroupName($category)); + } + + /** + * Many-to-many relation: groups are stored in the `glpi_groups_items` pivot table + * (AssignableItem trait, e.g. glpi_computers), as an array of ids. + */ + public function testArrayGroupsOnManyToManyRelation(): void + { + $group1 = $this->createItem(Group::class, [ + 'name' => 'Computer group 1', + 'entities_id' => $this->getTestRootEntity(true), + ]); + $group2 = $this->createItem(Group::class, [ + 'name' => 'Computer group 2', + 'entities_id' => $this->getTestRootEntity(true), + ]); + $tech_group = $this->createItem(Group::class, [ + 'name' => 'Computer tech group', + 'entities_id' => $this->getTestRootEntity(true), + ]); + + $computer = $this->createItem(Computer::class, [ + 'name' => 'Computer with groups', + 'entities_id' => $this->getTestRootEntity(true), + 'groups_id' => [$group1->getID(), $group2->getID()], + 'groups_id_tech' => [$tech_group->getID()], + ]); + $computer->getFromDB($computer->getID()); + + $this->assertSame( + implode(', ', [$group1->fields['name'], $group2->fields['name']]), + $this->getGroupName($computer) + ); + $this->assertSame( + $tech_group->fields['name'], + $this->getGroupName($computer, Group_Item::GROUP_TYPE_TECH) + ); + } + + public function testNoGroupsOnManyToManyRelation(): void + { + $computer = $this->createItem(Computer::class, [ + 'name' => 'Computer without groups', + 'entities_id' => $this->getTestRootEntity(true), + ]); + $computer->getFromDB($computer->getID()); + + $this->assertSame('', $this->getGroupName($computer)); + $this->assertSame('', $this->getGroupName($computer, Group_Item::GROUP_TYPE_TECH)); + } +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php new file mode 100644 index 0000000..6fc9112 --- /dev/null +++ b/tests/bootstrap.php @@ -0,0 +1,37 @@ +. + * + * @author Nelly Mahu-Lasson, Remi Collet, Teclib + * @copyright Copyright (c) 2009-2022 PDF plugin team + * @license AGPL License 3.0 or (at your option) any later version + * @link https://github.com/pluginsGLPI/pdf/ + * @link http://www.glpi-project.org/ + * @package pdf + * @since 2009 + * http://www.gnu.org/licenses/agpl-3.0-standalone.html + * -------------------------------------------------------------------------- + */ + +require __DIR__ . '/../../../tests/bootstrap.php'; + +if (!Plugin::isPluginActive('pdf')) { + throw new RuntimeException('Plugin pdf is not active in the test database'); +}