Permettre la gestion de plusieurs items pour les actions massives

En passant les items actifs à la fenêtre de choix des actions, on peut filtrer plus précisémment les actions à effectuer.

Par exemple, pour les liens élément<->composant, la modification 'update' n'est pas utile sur certains type de devices. Ainsi, si on ne coche que des Item_DeviceCase, on n'affiche que le "delete" et le "unaffect" dans la fenêtre modale.
De plus, lorsque l'on coche plusieurs types de devices, le update est spécifique alors que la suppression ou la désaffectation est commune. Par exemple, si on coche des processeurs et des mémoires, on pourrait choisir comment action :
  • Désaffectation des composants ;
  • Suppression des composants ;
  • Mise à jours de la mémoire ;
  • Mise à jours du processeur.

Cela serait beaucoup plus simple pour gérer la suite de l'affichage. En effet, pour la mise à jours, on a besoin de savoir exactement sur quel type d'objet on travail, pour connaître les champs disponibles. Alors que pour les deux autres actions, on peut l'appliquer à tous les types d'items.

Webmyster: personnellement, je trouve que la classe CommonDBTM est énorme (près de 5000 lignes de code). Je galère toujours pour retrouver ce que je recherche dedans. Donc, je propose de transférer tous les éléments génériques concernant les massives action dans une classe MassiveAction (showMassiveActionsParameters, executeMassiveActions, doMassiveActions, getAllMassiveActions ...). Après tout, on a besoin d'un objet item dans les méthodes de gestion des MassiveActions. Mais ce n'est pas spécifique à un item particulier. Les méthodes spécifiques (showSpecificMassiveActionsParameters, doSpecificMassiveActions, getForbiddenStandardMassiveAction, getSpecificMassiveActions ...) resteraient dans la classe CommonDBTM.

Pour dissocier les actions "communes" des actions "spécifiques", je propose d'ajouter une méthode MassiveAction::getCommonActions qui renverrait un tableau des actions qui sont communes. Par définition, les actions "communes" sont au moins définies dans la classe MassiveAction. Donc, il me semble plus simple de les indiquer plutôt que d'indiquer celles qui sont spécifiques. Cela est invalidé par la section Comment gérer les actions spécifiques ?' ci-dessous.

Suppression de itemtype ?

Après réflexion (et quelques tests), je me rend compte que le champs itemtype, défini dans Html::showMassiveActions() n'est plus utile dans tout le traitement des actions massives. En effet, ce champs itemtype serait dynamique avec la liste des champs choisis. Et certaines actions spécifiques, ou non, n'en ont l'utilité qu'au dernier moment.

Ce champs me semble même problèmatique : quel itemtype définir si nous proposons les actions massives pour une recherche globale (front/allassets.php) avec des Printer, des Computer ... dans le résultat ?

MoYo : Il me semble qu'on peut effectivement le virer en basculant sur la nouvelle architecture.

Web : la solution proposée dans le patch ci-dessous (du 10 juillet) utilisait la variable $CFG_GLPI['massive_action_itemtype']. Mais Moyo propose, à juste titre, de directement ajouter la classe qui traîte l'action dans le nom de l'action. Par exemple, add_document_item deviendrait Document:add_item. Le ':' semble la meilleure solution. Mais on peut prendre un autre caractère. J'ai peur que le '/' pose problème, notamment si on le transfert les requêtes HTTP. Je propose d'utiliser plutôt le ':'. Ainsi add_document_item deviendrait Document:add_item.

MoYo : ou le / était là pour l'exemple :) mais le : est plus adapté

Qu'en est-il des actions qui en ont besoin ?

Certains actions ont besoin de savoir à quel type d'item elles doivent s'appliquer, notamment update (pour savoir quels sont les champs modifiables). Lorsque l'utilisateur a coché des cases de nature différente, l'action peut afficher un dropdown de choix de l'itemtype (affichage centralisé dans la classe MassiveActions). Dès que l'itemtype est défini ou si l'utilisateur n'a coché qu'un seul type d'item, elle peut afficher ses informations spécifiques (liste des champs à modifier pour l'action update).

MoYo : Là j'ai quelques doutes. On peut aussi simplement proposer les entrées "Mise à jours de la mémoire / Mise à jours du processeur." comme tu l'indiques au début ?

Comment gérer les actions spécifiques ?

Lorsqu'il s'agit d'actions spécifiques (ie. non gérées par la classe MassiveActions), nous pourrions associer une classe à chaque action. Cette classe contiendrait les méthodes showSpecificMassiveActionsParameters et doSpecificMassiveActions.
Par exemple, les actions add_document et remove_document renverraient la classe Document, toutes les actions unlock_* (réunies sous la même appellation unlock) concerneraient la classe Lock. On voit que certaines actions définies dans les méthodes communes showMassiveActionsParameters et executeMassiveActions peuvent en sortir pour intégrer d'autres classes plus appropriées.
Au passage, on pourrait unifier le mécanisme pour les classes du coeur et celles des plugins (avec un Plugin::registerClass). Nous pouvons conserver le mécanisme actuel en parallèle de ce nouveau mécanisme.

MoYo ; je crois qu'au final je ne vois vraiment pas ce que tu proposes. Tu pourrai proposer une maquette même simpliste de ce que tu proposes ?
MoYo : une idée peut-être fausse de ce que je comprend. Au lieu de mettre add_document tu veux mettre comme clé d'action Document/add par exemple (action massive add de Document) ?
Web : J'ai ajouté un diff (pas à jour par rapport au tronc commun car j'ai commencé ce prototype la semaine dernière). Dans ce proto, seules les actions de mise à jours et les actions liées au lock sont actives. Mais l'idée générale est là.

aparté: le plugin example me semble invalide car il déclare une action do_nothing qui n'est pas interprétée comme l'action d'un plugin (cf. test ligne 3566 de inc/commondbtm.class.php).

MoYo : normal on est pas aller jusqu'à proposer une action complète (c'est pour ca quelle s'appelle do_nothing d'ailleurs)

Après avoir essayer de gratter un peu, je me suis rendu que dans certaines classes, certaines actions n'étaient de que des proxy vers les vraies classes. Par exemple, la vraie classe gérant connect et disconnect est Computer_Item, les autres classes (Phone, Printer ...) n'ont défini doSpecificMassiveActions que pour faire un proxy vers Computer_Item.

MoYo : oui dans l'architecture actuelle cela permettait simplement de centraliser le code. Car dans les moteurs de recherche on a pas accès à l'item Computer_Item. Cf. remarque plus haut sur l'intérêt d'une maquette je pense.

Passage des méthodes en static ?

Les méthodes de la classe MassiveActions doivent être static. Ne pourrions-nous pas faire pareille pour les méthodes spécifiques (showSpecificMassiveActionsParameters et doSpecificMassiveActions) dans les différentes classes ?

MoYo : pourquoi pas. A voir avec une proposition concrète.

Est-il possible de supprimer l'utilisation de checkitem ?

Avec le mécanisme proposé, je sens qu'il est possible de se passer de checkitem. En fait, j'ai du mal à comprendre son utilisation ...

MoYo : si on peut le supprimer tant mieux. L'intérêt est le suivant : pouvoir contrôler un autre objet pour savoir si on peut réaliser l'action.
Par exemple sur les NetworkPort, si on contrôle le droit standard on peut avoir le droit de modifier les ports réseaux globalement et on aura donc les modifs massives proposés pour tous les itemtypes qui ont des ports réseaux; Hors on peut avoir un itemtype pour lequel on a que le droit de lecture (et donc pas de modifs massives permises).

Web : le checkitem, préventif, me semble incomplet. Par exemple, nous pourrions avoir une page front/networkport.php (à l'image de front/networkname.php) qui recense l'ensemble des ports réseaux. Dans ce cas, les ports réseaux pourraient être attachés à tout type d'item, ce qui pose problème par rapport à l'unicité du checkitem. Pourquoi ne pas laisser le processus aller jusqu'au bout et ne faire aucun test. La seule gène sera un message indiquant l'impossibilité de faire l'action en question pour des raisons de droit.

MoYo : dans le cas que tu exposes tu pourrais laisser les actions sans mettre le checkitem par exemple. Mais ne pas afficher un élément dont on sait pertinemment qu'il ne retournera qu'une erreur me semble quand beaucoup plus simple et efficace pour l'utilisateur. Après il faut regarder les cas précis et voir si un contrôle a priori sur l'affichage ou non des modifs massives ne suffirai pas.

Qu'en est-il de specific_action ?

Est-ce encore utile de trasnférer le champs specific_action jusqu'au traitement par les méthodes ?

MoYo : pour des cas particulier je pense que ca a son intérêt. Exemple les cartouches ou consommables ou suivant l'état on n'a pas les même action (donner ou retour dans le stock).

Proposition d'action massive "générique" pour MassiveAction:update

Pour le moment, l'action MassiveAction:update ne gère qu'un unique itemtype. Pour cela, on affiche une dropdown pour choisir le type d'item lorsque plusieurs types ont été sélectionnés. Mais, certaines modifications peuvent concerner plusieurs types en même temps. Par exemple, les infocoms peuvent concerner des PCs et des switchs qui ont été achetés dans une même commande.
Nous pourrions afficher deux dropdown exclusives entre elles : l'une permet de choisir le type des items pour les champs qui ne concernent qu'un seul type ; l'autre permet de sélectionner les champs qui concernent tous les types (champs transversaux). Selon ce que l'on choisit, on respectivement, les champs disponibles pour l'item choisit ou le formulaire de saisie de la valeur lorsque c'est un champs transversal. Le reste du processus reste le même.

svn.diff Magnifier - Fichier SVN contenant un premier prototype relativement fonctionnel (88.5 KB) webmyster, 07/10/2013 02:17 PM