From 770b7844cae717ce186fba189a711c19c0ce28f3 Mon Sep 17 00:00:00 2001 From: Julien Durand Date: Thu, 1 Oct 2026 11:43:26 +0200 Subject: [PATCH 1/3] fix(46642): Wrong groups names displaying in PDF --- CHANGELOG.md | 2 + inc/appliance.class.php | 10 +- inc/cartridgeitem.class.php | 5 +- inc/common.class.php | 20 ++- inc/computer.class.php | 5 +- inc/consumableitem.class.php | 5 +- inc/domain_item.class.php | 28 ++++- inc/monitor.class.php | 2 +- inc/networkequipment.class.php | 2 +- inc/peripheral.class.php | 2 +- inc/phone.class.php | 2 +- inc/printer.class.php | 2 +- inc/software.class.php | 7 +- phpunit.xml | 11 ++ tests/bootstrap.php | 50 ++++++++ tests/fixtures/RecordingSimplePDF.php | 62 ++++++++++ tests/units/GroupsDisplayTest.php | 168 ++++++++++++++++++++++++++ 17 files changed, 347 insertions(+), 36 deletions(-) create mode 100644 phpunit.xml create mode 100644 tests/bootstrap.php create mode 100644 tests/fixtures/RecordingSimplePDF.php create mode 100644 tests/units/GroupsDisplayTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index ee6b5eed..1011cd5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Dependencies conflicts with core - Internal server error when generating appliance asset pdf +- Display all groups and groups in charge in asset PDF exports +- Fix warning on the Domains tab of PDF exports ## [4.1.5] - 2026-09-01 diff --git a/inc/appliance.class.php b/inc/appliance.class.php index 62852ef8..7deff978 100644 --- a/inc/appliance.class.php +++ b/inc/appliance.class.php @@ -154,10 +154,7 @@ public static function pdfMain(PluginPdfSimplePDF $pdf, Appliance $item) sprintf( __s('%1$s: %2$s'), '' . __s('Group in charge of the hardware') . '', - Toolbox::stripTags(Dropdown::getDropdownName( - 'glpi_groups', - $item->fields['groups_id_tech'], - )), + PluginPdfCommon::getGroupNames($item->fields['groups_id_tech']), ), ); @@ -183,10 +180,7 @@ public static function pdfMain(PluginPdfSimplePDF $pdf, Appliance $item) sprintf( __s('%1$s: %2$s'), '' . __s('Group') . '', - Toolbox::stripTags(Dropdown::getDropdownName( - 'glpi_groups', - $item->fields['groups_id'], - )), + PluginPdfCommon::getGroupNames($item->fields['groups_id']), ), ); diff --git a/inc/cartridgeitem.class.php b/inc/cartridgeitem.class.php index 63a09db5..47cad788 100644 --- a/inc/cartridgeitem.class.php +++ b/inc/cartridgeitem.class.php @@ -95,10 +95,7 @@ public static function pdfMain(PluginPdfSimplePDF $pdf, CartridgeItem $cartitem) '' . sprintf( __s('%1$s: %2$s'), __s('Group in charge of the hardware') . '', - Dropdown::getDropdownName( - 'glpi_groups', - $cartitem->fields['groups_id_tech'], - ), + PluginPdfCommon::getGroupNames($cartitem->fields['groups_id_tech']), ), ); diff --git a/inc/common.class.php b/inc/common.class.php index 49509e94..e7f5fa93 100644 --- a/inc/common.class.php +++ b/inc/common.class.php @@ -431,6 +431,21 @@ final public function generatePDF($tab_id, $tabs, $page = 0, $render = true) } } + /** + * Get the names of groups, sorted and separated by commas, ready to be displayed in a PDF cell + * + * @param array $groups_ids + * + * @return string + */ + public static function getGroupNames(array $groups_ids): string + { + $names = Dropdown::getDropdownArrayNames('glpi_groups', $groups_ids); + natcasesort($names); + + return Toolbox::stripTags(implode(', ', $names)); + } + public static function mainTitle(PluginPdfSimplePDF $pdf, $item) { $pdf->setColumnsSize(50, 50); @@ -520,10 +535,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::getGroupNames($item->fields['groups_id_tech']), ), '' . sprintf( __s('%1$s: %2$s'), diff --git a/inc/computer.class.php b/inc/computer.class.php index 0b610cf3..79c46d2e 100644 --- a/inc/computer.class.php +++ b/inc/computer.class.php @@ -91,10 +91,7 @@ public static function pdfMain(PluginPdfSimplePDF $pdf, Computer $computer) '' . sprintf( __('%1$s: %2$s'), __('Group') . '', - Dropdown::getDropdownName( - 'glpi_groups', - $computer->fields['groups_id'], - ), + PluginPdfCommon::getGroupNames($computer->fields['groups_id']), ), '' . sprintf(__('%1$s: %2$s'), __('UUID') . '', $computer->fields['uuid']), ); diff --git a/inc/consumableitem.class.php b/inc/consumableitem.class.php index 991a641f..77454720 100644 --- a/inc/consumableitem.class.php +++ b/inc/consumableitem.class.php @@ -84,10 +84,7 @@ public static function pdfMain(PluginPdfSimplePDF $pdf, ConsumableItem $consitem '' . sprintf( __s('%1$s: %2$s'), __s('Group in charge of the hardware') . '', - Dropdown::getDropdownName( - 'glpi_groups', - $consitem->fields['groups_id_tech'], - ), + PluginPdfCommon::getGroupNames($consitem->fields['groups_id_tech']), ), ); diff --git a/inc/domain_item.class.php b/inc/domain_item.class.php index 22e54562..448df899 100644 --- a/inc/domain_item.class.php +++ b/inc/domain_item.class.php @@ -84,11 +84,35 @@ public static function pdfForItem(PluginPdfSimplePDF $pdf, CommonDBTM $item) __s('Expiration date'), ); - foreach ($result as $data) { + $domains = iterator_to_array($result, false); + $tech_groups = []; + $groups = $DB->request([ + 'SELECT' => ['glpi_groups_items.items_id', 'glpi_groups.completename'], + 'FROM' => Group_Item::getTable(), + 'INNER JOIN' => [ + 'glpi_groups' => [ + 'FKEY' => [ + 'glpi_groups_items' => 'groups_id', + 'glpi_groups' => 'id', + ], + ], + ], + 'WHERE' => [ + 'glpi_groups_items.itemtype' => Domain::class, + 'glpi_groups_items.items_id' => array_column($domains, 'id'), + 'glpi_groups_items.type' => Group_Item::GROUP_TYPE_TECH, + ], + 'ORDER' => 'glpi_groups.completename', + ]); + foreach ($groups as $group) { + $tech_groups[$group['items_id']][] = $group['completename']; + } + + foreach ($domains as $data) { $pdf->displayLine( $data['name'], Dropdown::getDropdownName('glpi_entities', $data['entities_id']), - Dropdown::getDropdownName('glpi_groups', $data['groups_id_tech']), + Toolbox::stripTags(implode(', ', $tech_groups[$data['id']] ?? [])), getUserName($data['users_id_tech']), Dropdown::getDropdownName('glpi_domaintypes', $data['domaintypes_id']), Dropdown::getDropdownName('glpi_domainrelations', $data['domainrelations_id']), diff --git a/inc/monitor.class.php b/inc/monitor.class.php index 5d7ccc7e..52ffbb4a 100644 --- a/inc/monitor.class.php +++ b/inc/monitor.class.php @@ -64,7 +64,7 @@ public static function pdfMain(PluginPdfSimplePDF $pdf, Monitor $item) '' . sprintf( __s('%1$s: %2$s'), __s('Group') . '', - Dropdown::getDropdownName('glpi_groups', $item->fields['groups_id']), + PluginPdfCommon::getGroupNames($item->fields['groups_id']), ), '' . sprintf( __s('%1$s: %2$s'), diff --git a/inc/networkequipment.class.php b/inc/networkequipment.class.php index cbd11557..9c684296 100644 --- a/inc/networkequipment.class.php +++ b/inc/networkequipment.class.php @@ -86,7 +86,7 @@ public static function pdfMain(PluginPdfSimplePDF $pdf, NetworkEquipment $item) '' . sprintf( __s('%1$s: %2$s'), __s('Group') . '', - Dropdown::getDropdownName('glpi_groups', $item->fields['groups_id']), + PluginPdfCommon::getGroupNames($item->fields['groups_id']), ), '' . __s('The MAC address and the IP of the equipment are included in an aggregated network port'), '' . sprintf( diff --git a/inc/peripheral.class.php b/inc/peripheral.class.php index 9f383122..6b712d9b 100644 --- a/inc/peripheral.class.php +++ b/inc/peripheral.class.php @@ -66,7 +66,7 @@ public static function pdfMain(PluginPdfSimplePDF $pdf, Peripheral $item) '' . sprintf( __s('%1$s: %2$s'), __s('Group') . '', - Dropdown::getDropdownName('glpi_groups', $item->fields['groups_id']), + PluginPdfCommon::getGroupNames($item->fields['groups_id']), ), '' . sprintf(__s('%1$s: %2$s'), __s('Brand') . '', $item->fields['brand']), ); diff --git a/inc/phone.class.php b/inc/phone.class.php index e35cf925..7fbfa9c9 100644 --- a/inc/phone.class.php +++ b/inc/phone.class.php @@ -67,7 +67,7 @@ public static function pdfMain(PluginPdfSimplePDF $pdf, Phone $item) '' . sprintf( __s('%1$s: %2$s'), __s('Group') . '', - Dropdown::getDropdownName('glpi_groups', $item->fields['groups_id']), + PluginPdfCommon::getGroupNames($item->fields['groups_id']), ), '' . sprintf( __s('%1$s: %2$s'), diff --git a/inc/printer.class.php b/inc/printer.class.php index d493fc01..e695603b 100644 --- a/inc/printer.class.php +++ b/inc/printer.class.php @@ -99,7 +99,7 @@ public static function pdfMain(PluginPdfSimplePDF $pdf, Printer $printer) '' . sprintf( __s('%1$s: %2$s'), __s('Group') . '', - Dropdown::getDropdownName('glpi_groups', $printer->fields['groups_id']), + PluginPdfCommon::getGroupNames($printer->fields['groups_id']), ), '' . sprintf( __s('%1$s: %2$s'), diff --git a/inc/software.class.php b/inc/software.class.php index 21a3526e..756fc691 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'], - ), + PluginPdfCommon::getGroupNames($software->fields['groups_id_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']), + PluginPdfCommon::getGroupNames($software->fields['groups_id']), ), ); diff --git a/phpunit.xml b/phpunit.xml new file mode 100644 index 00000000..e66703dc --- /dev/null +++ b/phpunit.xml @@ -0,0 +1,11 @@ + + + + tests/units + + + diff --git a/tests/bootstrap.php b/tests/bootstrap.php new file mode 100644 index 00000000..57830034 --- /dev/null +++ b/tests/bootstrap.php @@ -0,0 +1,50 @@ +. + * + * @author Nelly Mahu-Lasson, Remi Collet, Teclib + * @copyright Copyright (c) 2009-2022 PDF plugin team + * @copyright 2015-2024 Teclib' and contributors. + * @copyright 2003-2014 by the INDEPNET Development Team. + * @licence https://www.gnu.org/licenses/gpl-3.0.html + * @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'; + +$plugin = new Plugin(); +$plugin->checkPluginState('pdf'); +$plugin->getFromDBbyDir('pdf'); + +if (!$plugin->isInstalled('pdf')) { + $plugin->install($plugin->getID()); +} + +if (!$plugin->isActivated('pdf')) { + $plugin->activate($plugin->getID()); +} + +require_once __DIR__ . '/fixtures/RecordingSimplePDF.php'; diff --git a/tests/fixtures/RecordingSimplePDF.php b/tests/fixtures/RecordingSimplePDF.php new file mode 100644 index 00000000..54df30dd --- /dev/null +++ b/tests/fixtures/RecordingSimplePDF.php @@ -0,0 +1,62 @@ +. + * + * @author Nelly Mahu-Lasson, Remi Collet, Teclib + * @copyright Copyright (c) 2009-2022 PDF plugin team + * @copyright 2015-2024 Teclib' and contributors. + * @copyright 2003-2014 by the INDEPNET Development Team. + * @licence https://www.gnu.org/licenses/gpl-3.0.html + * @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; + +use PluginPdfSimplePDF; + +class RecordingSimplePDF extends PluginPdfSimplePDF +{ + /** @var string[] */ + public array $cells = []; + + public function __construct() + { + parent::__construct(); + $this->newPage(); + } + + public function displayTitle() + { + array_push($this->cells, ...func_get_args()); + parent::displayTitle(...func_get_args()); + } + + public function displayLine() + { + array_push($this->cells, ...func_get_args()); + parent::displayLine(...func_get_args()); + } +} diff --git a/tests/units/GroupsDisplayTest.php b/tests/units/GroupsDisplayTest.php new file mode 100644 index 00000000..36190a46 --- /dev/null +++ b/tests/units/GroupsDisplayTest.php @@ -0,0 +1,168 @@ +. + * + * @author Nelly Mahu-Lasson, Remi Collet, Teclib + * @copyright Copyright (c) 2009-2022 PDF plugin team + * @copyright 2015-2024 Teclib' and contributors. + * @copyright 2003-2014 by the INDEPNET Development Team. + * @licence https://www.gnu.org/licenses/gpl-3.0.html + * @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 Appliance; +use CartridgeItem; +use Computer; +use ConsumableItem; +use Domain; +use Domain_Item; +use Glpi\Tests\DbTestCase; +use GlpiPlugin\Pdf\Tests\RecordingSimplePDF; +use Group; +use Monitor; +use NetworkEquipment; +use Peripheral; +use Phone; +use PHPUnit\Framework\Attributes\DataProvider; +use PluginPdfAppliance; +use PluginPdfCartridgeItem; +use PluginPdfComputer; +use PluginPdfConsumableItem; +use PluginPdfDomain_Item; +use PluginPdfMonitor; +use PluginPdfNetworkEquipment; +use PluginPdfPeripheral; +use PluginPdfPhone; +use PluginPdfPrinter; +use PluginPdfSoftware; +use Printer; +use Software; + +class GroupsDisplayTest extends DbTestCase +{ + private const GROUP_CELL = 'Group: %s'; + private const TECH_GROUP_CELL = 'Group in charge of the hardware: %s'; + + public static function pdfMainProvider(): iterable + { + $assets = [ + 'Computer' => [Computer::class, PluginPdfComputer::class], + 'Monitor' => [Monitor::class, PluginPdfMonitor::class], + 'NetworkEquipment' => [NetworkEquipment::class, PluginPdfNetworkEquipment::class], + 'Peripheral' => [Peripheral::class, PluginPdfPeripheral::class], + 'Phone' => [Phone::class, PluginPdfPhone::class], + 'Printer' => [Printer::class, PluginPdfPrinter::class], + 'Software' => [Software::class, PluginPdfSoftware::class], + 'Appliance' => [Appliance::class, PluginPdfAppliance::class], + ]; + foreach ($assets as $label => [$itemtype, $pdf_class]) { + yield "$label groups" => [ + 'itemtype' => $itemtype, + 'pdf_class' => $pdf_class, + 'field' => 'groups_id', + 'template' => self::GROUP_CELL, + ]; + yield "$label tech groups" => [ + 'itemtype' => $itemtype, + 'pdf_class' => $pdf_class, + 'field' => 'groups_id_tech', + 'template' => self::TECH_GROUP_CELL, + ]; + } + yield 'CartridgeItem tech groups' => [ + 'itemtype' => CartridgeItem::class, + 'pdf_class' => PluginPdfCartridgeItem::class, + 'field' => 'groups_id_tech', + 'template' => self::TECH_GROUP_CELL, + ]; + yield 'ConsumableItem tech groups' => [ + 'itemtype' => ConsumableItem::class, + 'pdf_class' => PluginPdfConsumableItem::class, + 'field' => 'groups_id_tech', + 'template' => self::TECH_GROUP_CELL, + ]; + } + + #[DataProvider('pdfMainProvider')] + public function testPdfMainDisplaysAllGroups(string $itemtype, string $pdf_class, string $field, string $template): void + { + $this->login(); + $group_b = $this->createItem(Group::class, ['name' => 'PDF group B', 'entities_id' => 0]); + $group_a = $this->createItem(Group::class, ['name' => 'PDF group A', 'entities_id' => 0]); + + $item = $this->createItem($itemtype, [ + 'name' => 'PDF item', + 'entities_id' => 0, + $field => [$group_a->getID(), $group_b->getID()], + ]); + + $pdf = new RecordingSimplePDF(); + $pdf_class::pdfMain($pdf, $item); + + $this->assertContains(sprintf($template, 'PDF group A, PDF group B'), $pdf->cells); + } + + #[DataProvider('pdfMainProvider')] + public function testPdfMainDisplaysNoGroup(string $itemtype, string $pdf_class, string $field, string $template): void + { + $this->login(); + $item = $this->createItem($itemtype, [ + 'name' => 'PDF item', + 'entities_id' => 0, + ]); + + $pdf = new RecordingSimplePDF(); + $pdf_class::pdfMain($pdf, $item); + + $this->assertContains(sprintf($template, ''), $pdf->cells); + } + + public function testDomainItemDisplaysAllTechGroups(): void + { + $this->login(); + $group_b = $this->createItem(Group::class, ['name' => 'PDF group B', 'entities_id' => 0]); + $group_a = $this->createItem(Group::class, ['name' => 'PDF group A', 'entities_id' => 0]); + + $computer = $this->createItem(Computer::class, ['name' => 'PDF computer', 'entities_id' => 0]); + $domain = $this->createItem(Domain::class, [ + 'name' => 'pdf.example.com', + 'entities_id' => 0, + 'groups_id_tech' => [$group_a->getID(), $group_b->getID()], + ]); + $this->createItem(Domain_Item::class, [ + 'domains_id' => $domain->getID(), + 'itemtype' => Computer::class, + 'items_id' => $computer->getID(), + ]); + + $pdf = new RecordingSimplePDF(); + PluginPdfDomain_Item::pdfForItem($pdf, $computer); + + $this->assertContains('PDF group A, PDF group B', $pdf->cells); + } +} From 23773b8c9afdfd4ade2514c98fb463439ec39797 Mon Sep 17 00:00:00 2001 From: Julien Durand Date: Thu, 1 Oct 2026 11:44:37 +0200 Subject: [PATCH 2/3] chore(46642): ignore phpunit result cache --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 57872d0f..19f18f7d 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ /vendor/ +.phpunit.result.cache \ No newline at end of file From d3ef24fc7540a902f913b72a4a8c0d274660a166 Mon Sep 17 00:00:00 2001 From: Julien Durand Date: Wed, 7 Oct 2026 15:16:28 +0200 Subject: [PATCH 3/3] trigger CI