From d1524fcf8e49468d87d58d348d620c4ea0512094 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Anne?= Date: Wed, 22 May 2024 22:14:38 +0200 Subject: [PATCH] simplify tests + add missing trait / SO --- src/Api/HL/Controller/AssetController.php | 1 - src/Cable.php | 52 ++++++++ src/CommonDBTM.php | 2 +- src/PDU.php | 48 +++++++- src/PassiveDCEquipment.php | 46 ++++++- src/Search/Provider/SQLProvider.php | 4 +- tests/functional/Search.php | 7 -- tests/units/Glpi/Features/AssignableItem.php | 122 +++++++++++-------- 8 files changed, 214 insertions(+), 68 deletions(-) diff --git a/src/Api/HL/Controller/AssetController.php b/src/Api/HL/Controller/AssetController.php index fd18e4433de..b3e44fc40b7 100644 --- a/src/Api/HL/Controller/AssetController.php +++ b/src/Api/HL/Controller/AssetController.php @@ -42,7 +42,6 @@ use Glpi\Api\HL\Middleware\ResultFormatterMiddleware; use Glpi\Api\HL\Route; use Glpi\Api\HL\Search; -use Glpi\Features\AssignableItem; use Glpi\Http\JSONResponse; use Glpi\Http\Request; use Glpi\Http\Response; diff --git a/src/Cable.php b/src/Cable.php index 8a28130dce8..470bb17fcac 100644 --- a/src/Cable.php +++ b/src/Cable.php @@ -40,6 +40,7 @@ /// Class Cable class Cable extends CommonDBTM { + use Glpi\Features\AssignableItem; use Glpi\Features\Clonable; use Glpi\Features\State; @@ -271,6 +272,35 @@ public function rawSearchOptions() 'datatype' => 'text' ]; + $tab[] = [ + 'id' => '70', + 'table' => 'glpi_users', + 'field' => 'name', + 'name' => User::getTypeName(1), + 'datatype' => 'dropdown', + 'right' => 'all' + ]; + + $tab[] = [ + 'id' => '71', + 'table' => 'glpi_groups', + 'field' => 'completename', + 'name' => Group::getTypeName(1), + 'condition' => ['is_itemgroup' => 1], + 'joinparams' => [ + 'beforejoin' => [ + 'table' => 'glpi_groups_items', + 'joinparams' => [ + 'jointype' => 'itemtype_item', + 'condition' => ['NEWTABLE.type' => Group_Item::GROUP_TYPE_NORMAL] + ] + ] + ], + 'forcegroupby' => true, + 'massiveaction' => false, + 'datatype' => 'dropdown' + ]; + $tab[] = [ 'id' => '19', 'table' => $this->getTable(), @@ -286,6 +316,28 @@ public function rawSearchOptions() 'field' => 'name', 'linkfield' => 'users_id_tech', 'name' => __('Technician in charge'), + 'datatype' => 'dropdown', + 'right' => 'own_ticket' + ]; + + $tab[] = [ + 'id' => '49', + 'table' => 'glpi_groups', + 'field' => 'completename', + 'linkfield' => 'groups_id', + 'name' => __('Group in charge'), + 'condition' => ['is_assign' => 1], + 'joinparams' => [ + 'beforejoin' => [ + 'table' => 'glpi_groups_items', + 'joinparams' => [ + 'jointype' => 'itemtype_item', + 'condition' => ['NEWTABLE.type' => Group_Item::GROUP_TYPE_TECH] + ] + ] + ], + 'forcegroupby' => true, + 'massiveaction' => false, 'datatype' => 'dropdown' ]; diff --git a/src/CommonDBTM.php b/src/CommonDBTM.php index 85f73aef1c7..5a729fde2a1 100644 --- a/src/CommonDBTM.php +++ b/src/CommonDBTM.php @@ -1006,7 +1006,7 @@ public function cleanRelationTable() */ global $CFG_GLPI, $DB; - if (Toolbox::hasTrait(static::class, AssignableItem::class)) { + if (in_array(static::class, $CFG_GLPI['assignable_types'], true)) { $group_item = new Group_Item(); $group_item->deleteByCriteria( [ diff --git a/src/PDU.php b/src/PDU.php index 9b83e2f79fe..ff952fd20c6 100644 --- a/src/PDU.php +++ b/src/PDU.php @@ -38,6 +38,9 @@ **/ class PDU extends CommonDBTM { + use Glpi\Features\AssignableItem { + prepareInputForAdd as prepareInputForAddAssignableItem; + } use Glpi\Features\DCBreadcrumb; use Glpi\Features\Clonable; use Glpi\Features\State; @@ -127,6 +130,35 @@ public function rawSearchOptions() 'datatype' => 'text' ]; + $tab[] = [ + 'id' => '70', + 'table' => 'glpi_users', + 'field' => 'name', + 'name' => User::getTypeName(1), + 'datatype' => 'dropdown', + 'right' => 'all' + ]; + + $tab[] = [ + 'id' => '71', + 'table' => 'glpi_groups', + 'field' => 'completename', + 'name' => Group::getTypeName(1), + 'condition' => ['is_itemgroup' => 1], + 'joinparams' => [ + 'beforejoin' => [ + 'table' => 'glpi_groups_items', + 'joinparams' => [ + 'jointype' => 'itemtype_item', + 'condition' => ['NEWTABLE.type' => Group_Item::GROUP_TYPE_NORMAL] + ] + ] + ], + 'forcegroupby' => true, + 'massiveaction' => false, + 'datatype' => 'dropdown' + ]; + $tab[] = [ 'id' => '19', 'table' => $this->getTable(), @@ -175,9 +207,20 @@ public function rawSearchOptions() 'id' => '49', 'table' => 'glpi_groups', 'field' => 'completename', - 'linkfield' => 'groups_id_tech', + 'linkfield' => 'groups_id', 'name' => __('Group in charge'), 'condition' => ['is_assign' => 1], + 'joinparams' => [ + 'beforejoin' => [ + 'table' => 'glpi_groups_items', + 'joinparams' => [ + 'jointype' => 'itemtype_item', + 'condition' => ['NEWTABLE.type' => Group_Item::GROUP_TYPE_TECH] + ] + ] + ], + 'forcegroupby' => true, + 'massiveaction' => false, 'datatype' => 'dropdown' ]; @@ -244,6 +287,9 @@ public function prepareInputForAdd($input) } unset($input['id']); unset($input['withtemplate']); + + $input = $this->prepareInputForAddAssignableItem($input); + return $input; } } diff --git a/src/PassiveDCEquipment.php b/src/PassiveDCEquipment.php index 017cad90865..a4c9cd62f04 100644 --- a/src/PassiveDCEquipment.php +++ b/src/PassiveDCEquipment.php @@ -33,7 +33,6 @@ * --------------------------------------------------------------------- */ -use Glpi\Features\Clonable; use Glpi\Socket; /** @@ -41,7 +40,8 @@ **/ class PassiveDCEquipment extends CommonDBTM { - use Clonable; + use Glpi\Features\AssignableItem; + use Glpi\Features\Clonable; use Glpi\Features\DCBreadcrumb; use Glpi\Features\State; @@ -124,6 +124,35 @@ public function rawSearchOptions() $tab = array_merge($tab, Location::rawSearchOptionsToAdd()); + $tab[] = [ + 'id' => '70', + 'table' => 'glpi_users', + 'field' => 'name', + 'name' => User::getTypeName(1), + 'datatype' => 'dropdown', + 'right' => 'all' + ]; + + $tab[] = [ + 'id' => '71', + 'table' => 'glpi_groups', + 'field' => 'completename', + 'name' => Group::getTypeName(1), + 'condition' => ['is_itemgroup' => 1], + 'joinparams' => [ + 'beforejoin' => [ + 'table' => 'glpi_groups_items', + 'joinparams' => [ + 'jointype' => 'itemtype_item', + 'condition' => ['NEWTABLE.type' => Group_Item::GROUP_TYPE_NORMAL] + ] + ] + ], + 'forcegroupby' => true, + 'massiveaction' => false, + 'datatype' => 'dropdown' + ]; + $tab[] = [ 'id' => '19', 'table' => $this->getTable(), @@ -172,9 +201,20 @@ public function rawSearchOptions() 'id' => '49', 'table' => 'glpi_groups', 'field' => 'completename', - 'linkfield' => 'groups_id_tech', + 'linkfield' => 'groups_id', 'name' => __('Group in charge'), 'condition' => ['is_assign' => 1], + 'joinparams' => [ + 'beforejoin' => [ + 'table' => 'glpi_groups_items', + 'joinparams' => [ + 'jointype' => 'itemtype_item', + 'condition' => ['NEWTABLE.type' => Group_Item::GROUP_TYPE_TECH] + ] + ] + ], + 'forcegroupby' => true, + 'massiveaction' => false, 'datatype' => 'dropdown' ]; diff --git a/src/Search/Provider/SQLProvider.php b/src/Search/Provider/SQLProvider.php index 206fb0ab888..459cc9d901f 100644 --- a/src/Search/Provider/SQLProvider.php +++ b/src/Search/Provider/SQLProvider.php @@ -3044,7 +3044,7 @@ public static function getMetaLeftJoinCriteria(string $from_type, string $to_typ return $joins; } - if ($to_type === 'Group' && Toolbox::hasTrait($from_referencetype, AssignableItem::class)) { + if ($to_type === 'Group' && in_array($from_referencetype, $CFG_GLPI['assignable_types'], true)) { $relation_table_alias = 'glpi_groups_items' . $alias_suffix; if (!in_array($relation_table_alias, $already_link_tables2, true)) { $already_link_tables2[] = $relation_table_alias; @@ -3076,7 +3076,7 @@ public static function getMetaLeftJoinCriteria(string $from_type, string $to_typ return $joins; } - if ($from_referencetype === 'Group' && Toolbox::hasTrait($to_type, AssignableItem::class)) { + if ($from_referencetype === 'Group' && in_array($to_type, $CFG_GLPI['assignable_types'], true)) { $relation_table_alias = 'glpi_groups_items' . $alias_suffix; if (!in_array($relation_table_alias, $already_link_tables2, true)) { $already_link_tables2[] = $relation_table_alias; diff --git a/tests/functional/Search.php b/tests/functional/Search.php index f3beaaf99f6..5dd34b22c74 100644 --- a/tests/functional/Search.php +++ b/tests/functional/Search.php @@ -43,7 +43,6 @@ use Document; use Document_Item; use Glpi\Asset\Capacity\HasDocumentsCapacity; -use Glpi\Features\AssignableItem; use Group_User; use Psr\Log\LogLevel; use Ticket; @@ -1901,10 +1900,8 @@ public function testAllAssetsFields() 'users_id', 'contact', 'contact_num', - 'groups_id', 'date_mod', 'manufacturers_id', - 'groups_id_tech', 'entities_id', ]; @@ -1912,10 +1909,6 @@ public function testAllAssetsFields() $table = getTableForItemType($itemtype); foreach ($needed_fields as $field) { - if (\Toolbox::hasTrait($itemtype, AssignableItem::class) && str_starts_with($field, 'groups_id')) { - // The group fields don't exist in the main table for these assets - continue; - } $this->boolean($DB->fieldExists($table, $field)) ->isTrue("$table.$field is missing"); } diff --git a/tests/units/Glpi/Features/AssignableItem.php b/tests/units/Glpi/Features/AssignableItem.php index 3ef1c79ce0d..168fb54fb85 100644 --- a/tests/units/Glpi/Features/AssignableItem.php +++ b/tests/units/Glpi/Features/AssignableItem.php @@ -47,7 +47,7 @@ protected function itemtypeProvider(): iterable global $CFG_GLPI; foreach ($CFG_GLPI['assignable_types'] as $itemtype) { - yield[ + yield $itemtype => [ 'class' => $itemtype, ]; } @@ -58,7 +58,7 @@ protected function itemtypeProvider(): iterable */ public function testClassUsesTrait(string $class): void { - $this->boolean(in_array(\Glpi\Features\AssignableItem::class, class_uses($class, true))); + $this->boolean(in_array(\Glpi\Features\AssignableItem::class, class_uses($class), true))->isTrue(); } protected function groupAssignableItemtypeProvider(): iterable @@ -69,12 +69,12 @@ protected function groupAssignableItemtypeProvider(): iterable global $CFG_GLPI; foreach ($CFG_GLPI['assignable_types'] as $itemtype) { - yield[ + yield $itemtype . '-normal' => [ 'class' => $itemtype, 'type' => Group_Item::GROUP_TYPE_NORMAL, ]; - yield[ + yield $itemtype . '-normal' => [ 'class' => $itemtype, 'type' => Group_Item::GROUP_TYPE_TECH, ]; @@ -85,87 +85,98 @@ protected function groupAssignableItemtypeProvider(): iterable * Test adding an item with the groups_id/groups_id_tech field as an array and null. * Test updating an item with the groups_id/groups_id_tech field as an array and null. * - * @dataProvider groupAssignableItemtypeProvider + * @dataProvider itemtypeProvider */ - public function testAddAndUpdateMultipleGroups(string $class, int $type): void + public function testAddAndUpdateMultipleGroups(string $class): void { $this->login(); // login to bypass some rights checks (e.g. on domain records) $input = $this->getMinimalCreationInput($class); - $field = match ($type) { - Group_Item::GROUP_TYPE_NORMAL => 'groups_id', - Group_Item::GROUP_TYPE_TECH => 'groups_id_tech', - }; - $item_1 = $this->createItem( $class, $input + [ $class::getNameField() => __FUNCTION__ . ' 1', - $field => [1, 2], + 'groups_id' => [1, 2], + 'groups_id_tech' => [3], ] ); - $this->array($item_1->fields[$field])->isEqualTo([1, 2]); + $this->array($item_1->fields['groups_id'])->isEqualTo([1, 2]); + $this->array($item_1->fields['groups_id_tech'])->isEqualTo([3]); $item_2 = $this->createItem( $class, $input + [ $class::getNameField() => __FUNCTION__ . ' 2', - $field => null, + 'groups_id' => null, + 'groups_id_tech' => null, ] ); - $this->array($item_2->fields[$field])->isEmpty(); + $this->array($item_2->fields['groups_id'])->isEmpty(); + $this->array($item_2->fields['groups_id_tech'])->isEmpty(); // Update both items. Asset 1 will have the groups set to null and item 2 will have the groups set to an array. - $this->boolean($item_1->update(['id' => $item_1->getID(), $field => null]))->isTrue(); - $this->array($item_1->fields[$field])->isEmpty(); + $updated = $item_1->update(['id' => $item_1->getID(), 'groups_id' => null, 'groups_id_tech' => null]); + $this->boolean($updated)->isTrue(); + $this->array($item_1->fields['groups_id'])->isEmpty(); + $this->array($item_1->fields['groups_id_tech'])->isEmpty(); - $this->boolean($item_2->update(['id' => $item_2->getID(),$field => [5, 6]]))->isTrue(); - $this->array($item_2->fields[$field])->isEqualTo([5, 6]); + $updated = $item_2->update(['id' => $item_2->getID(), 'groups_id' => [5, 6], 'groups_id_tech' => [7]]); + $this->boolean($updated)->isTrue(); + $this->array($item_2->fields['groups_id'])->isEqualTo([5, 6]); + $this->array($item_2->fields['groups_id_tech'])->isEqualTo([7]); // Test updating array to array - $this->boolean($item_2->update(['id' => $item_2->getID(), $field => [1, 2]]))->isTrue(); - $this->array($item_2->fields[$field])->isEqualTo([1, 2]); + $updated = $item_2->update(['id' => $item_2->getID(), 'groups_id' => [1, 2], 'groups_id_tech' => [4, 5]]); + $this->boolean($updated)->isTrue(); + $this->array($item_2->fields['groups_id'])->isEqualTo([1, 2]); + $this->array($item_2->fields['groups_id_tech'])->isEqualTo([4, 5]); } /** * Test the loading item which still have integer values for groups_id/groups_id_tech (0 for no group). * The value should be automatically normalized to an array. If the group was '0', the array should be empty. * - * @dataProvider groupAssignableItemtypeProvider + * @dataProvider itemtypeProvider */ - public function testLoadGroupsFromDb(string $class, int $type): void + public function testLoadGroupsFromDb(string $class): void { /** @var \DBmysql $DB */ global $DB; $input = $this->getMinimalCreationInput($class); - $field = match ($type) { - Group_Item::GROUP_TYPE_NORMAL => 'groups_id', - Group_Item::GROUP_TYPE_TECH => 'groups_id_tech', - }; - $item = $this->createItem( $class, $input + [ $class::getNameField() => __FUNCTION__, ] ); - $this->array($item->fields[$field])->isEmpty(); + $this->array($item->fields['groups_id'])->isEmpty(); + $this->array($item->fields['groups_id_tech'])->isEmpty(); + $DB->insert( + 'glpi_groups_items', + [ + 'itemtype' => $class, + 'items_id' => $item->getID(), + 'groups_id' => 1, + 'type' => Group_Item::GROUP_TYPE_NORMAL, + ], + ); $DB->insert( 'glpi_groups_items', [ 'itemtype' => $class, 'items_id' => $item->getID(), 'groups_id' => 2, - 'type' => $type, + 'type' => Group_Item::GROUP_TYPE_TECH, ], ); $this->boolean($item->getFromDB($item->getID()))->isTrue(); - $this->array($item->fields[$field])->isEqualTo([2]); + $this->array($item->fields['groups_id'])->isEqualTo([1]); + $this->array($item->fields['groups_id_tech'])->isEqualTo([2]); $DB->insert( 'glpi_groups_items', @@ -173,57 +184,62 @@ public function testLoadGroupsFromDb(string $class, int $type): void 'itemtype' => $class, 'items_id' => $item->getID(), 'groups_id' => 3, - 'type' => $type, + 'type' => Group_Item::GROUP_TYPE_NORMAL, + ], + ); + $DB->insert( + 'glpi_groups_items', + [ + 'itemtype' => $class, + 'items_id' => $item->getID(), + 'groups_id' => 4, + 'type' => Group_Item::GROUP_TYPE_TECH, ], ); $this->boolean($item->getFromDB($item->getID()))->isTrue(); - $this->array($item->fields[$field])->isEqualTo([2, 3]); + $this->array($item->fields['groups_id'])->isEqualTo([1, 3]); + $this->array($item->fields['groups_id_tech'])->isEqualTo([2, 4]); } /** * An empty item should have the groups_id/groups_id_tech fields initialized as an empty array. * - * @dataProvider groupAssignableItemtypeProvider + * @dataProvider itemtypeProvider */ - public function testGetEmpty(string $class, int $type): void + public function testGetEmpty(string $class): void { - $field = match ($type) { - Group_Item::GROUP_TYPE_NORMAL => 'groups_id', - Group_Item::GROUP_TYPE_TECH => 'groups_id_tech', - }; - $item = new $class(); $this->boolean($item->getEmpty())->isTrue(); - $this->array($item->fields[$field])->isEmpty(); + $this->array($item->fields['groups_id'])->isEmpty(); + $this->array($item->fields['groups_id_tech'])->isEmpty(); } /** * Check that adding and updating an item with groups_id/groups_id_tech as an integer still works (minor BC, mainly for API scripts). * - * @dataProvider groupAssignableItemtypeProvider + * @dataProvider itemtypeProvider */ - public function testAddUpdateWithIntGroups(string $class, int $type): void + public function testAddUpdateWithIntGroups(string $class): void { $this->login(); // login to bypass some rights checks (e.g. on domain records) $input = $this->getMinimalCreationInput($class); - $field = match ($type) { - Group_Item::GROUP_TYPE_NORMAL => 'groups_id', - Group_Item::GROUP_TYPE_TECH => 'groups_id_tech', - }; - $item = $this->createItem( $class, $input + [ $class::getNameField() => __FUNCTION__, - $field => 1, + 'groups_id' => 1, + 'groups_id_tech' => 2, ], - [$field] // ignore the field as it will be transformed to an array + ['groups_id', 'groups_id_tech'] // ignore the fields as it will be transformed to an array ); - $this->array($item->fields[$field])->isEqualTo([1]); + $this->array($item->fields['groups_id'])->isEqualTo([1]); + $this->array($item->fields['groups_id_tech'])->isEqualTo([2]); - $this->boolean($item->update(['id' => $item->getID(), $field => 2]))->isTrue(); - $this->array($item->fields[$field])->isEqualTo([2]); + $updated = $item->update(['id' => $item->getID(), 'groups_id' => 3, 'groups_id_tech' => 4]); + $this->boolean($updated)->isTrue(); + $this->array($item->fields['groups_id'])->isEqualTo([3]); + $this->array($item->fields['groups_id_tech'])->isEqualTo([4]); } }