[SPIP Zone] [Spip-zone-commit] r109247 - in _plugins_/roles_documents/trunk

Le vendredi 02 mars 2018 à 23:35 +0100, spip-zone-commit@rezo.net a
écrit :

Author: tcharlss@bravecassine.com
Date: 2018-03-02 23:35:03 +0100 (Fri, 02 Mar 2018)
New Revision: 109247

Modified:
_plugins_/roles_documents/trunk/action/iconifier_logo.php
_plugins_/roles_documents/trunk/action/supprimer_logo.php
_plugins_/roles_documents/trunk/inc/chercher_logo.php
Log:
Refaire fonctionner les boutons d'action pour supprimer un vieux logo
ou le convertir en document. chercher_logo() renvoie maintenant en
priorité un document, donc logo_supprimer() ne retrouvait plus le
vieux logo, et on ne peut pas surcharger la fonction. Donc petit hack
un peu vilain dans chercher_logo() pour savoir si l'appel vient de
logo_supprimer() (et dans ce cas on force la recherche de vieux
logo).

Cool de voir que ça bouge bien du côté de roles_documents ! J'ai
branché logos_roles pour suivre ton travail et tester un peu tout ça,
dans l'idée de laisser la main à roles_documents pour s'occuper de tout
ce qui ne concerne pas les nouveaux types de logos.

Chemin faisant, je me questionne à propos du « petit hack un peu
vilain ». Je comprends que la fonction logo_supprimer ne pourras jamais
voir les logos enregistrés comme documents. Elle devra alors être
dépréciée à terme ?

Je pense que ça serait dommage de sacrifier cette API. L'API actuelle
du core est assez simple et facile à surcharger :

- la fonction inc/chercher_logo pour trouver un logo
- logo_modifier (dans action/editer_logo) pour modifier/créer
- logo_supprimer (dans action/editer_logo) pour supprimer

Je ne vois pas de raison de ne pas réutiliser cette structure. Ces
fonctions devraient s'occuper de tout sans qu'on ait besoin de savoir
ce qui se passe en coulisse (logo historique ou document). Et le reste
du code devrait les utiliser sans se soucier du système de stockage des
logos.
C'est ce qui est fait dans logos_roles, et ça simplifie bien des
choses, en plus de garantir la compatibilité avec les développements
existants.
Mais là, si je veux garder cette cohérence avec la version actuelle de
roles_documents, je suis obligé de surcharger inc/chercher_logo pour
virer ce petit hack, et c'est dommage…

--
bystrano

Le 04/03/2018 à 11:02, Michel Bystranowski a écrit :

Je pense que ça serait dommage de sacrifier cette API. L'API actuelle
du core est assez simple et facile à surcharger :

- la fonction inc/chercher_logo pour trouver un logo
- logo_modifier (dans action/editer_logo) pour modifier/créer
- logo_supprimer (dans action/editer_logo) pour supprimer

+1

On peut vouloir créer des objets autrement qu'avec les formulaires du privé (import en masse, formulaire public...) et avoir cette API sous la main est très pratique.

--
nicod_

Glop,

Le 04/03/2018 à 11:02, Michel Bystranowski a écrit :

Cool de voir que ça bouge bien du côté de roles_documents ! J'ai
branché logos_roles pour suivre ton travail et tester un peu tout ça,
dans l'idée de laisser la main à roles_documents pour s'occuper de tout
ce qui ne concerne pas les nouveaux types de logos.

Merci ! La publication de la doc a motivé pour finir ce qui avait été laissé de côté depuis le début. Avec le recul on aurait dû attendre un peu le temps de bien finaliser avant de publier, mais bon... C'est fait.

Au niveau des fonctionnalités, je pense que le formulaire pour migrer les vieux logos pourrait aussi être déplacé dans rôles de documents.
J'avais pensé à ajouter une option pour proposer de sauvegarder les vieux logos, histoire de pouvoir éventuellement les récupérer en cas de désinstallation du plugin (avec la mise en garde de mise sur l'espace disque occupé). Mais si on part là-dessus, ça pose d'autres questions : est-ce qu'il faut aussi convertir les nouveaux logos documents en vieux logos ? Ça ferait partir sur un truc bien compliqué (vérifier les doublons, les utilisations, etc.).

Le critère {logo} pourrait aussi être déplacé dans rôles de documents, en faisant une adaptation pour la perf (cf. remarque de cedric) : une jointure sur le document lié avec le rôle adéquat. Par contre je ne sais pas s'il faut garder la compat avec les vieux logos pour ce critère, si ça impacte trop la perf ?

Chemin faisant, je me questionne à propos du « petit hack un peu
vilain ». Je comprends que la fonction logo_supprimer ne pourras jamais
voir les logos enregistrés comme documents. Elle devra alors être
dépréciée à terme ?

Je pense que ça serait dommage de sacrifier cette API. L'API actuelle
du core est assez simple et facile à surcharger :

Ah oui, my bad !
En voyant que ce n'était pas des fonctions surchargeables (pas de suffixe _dist), je m'étais arrêté là, mais on peut surcharger tout le fichier effectivement.
Je vais faire le nécessaire.

Le lundi 05 mars 2018 à 00:56 +0100, Charles Razack a écrit :

Au niveau des fonctionnalités, je pense que le formulaire pour
migrer
les vieux logos pourrait aussi être déplacé dans rôles de documents.
J'avais pensé à ajouter une option pour proposer de sauvegarder les
vieux logos, histoire de pouvoir éventuellement les récupérer en cas
de
désinstallation du plugin (avec la mise en garde de mise sur
l'espace
disque occupé). Mais si on part là-dessus, ça pose d'autres questions
:
est-ce qu'il faut aussi convertir les nouveaux logos documents en
vieux
logos ? Ça ferait partir sur un truc bien compliqué (vérifier les
doublons, les utilisations, etc.).

Sers-toi si tu veux le reprendre ! Après je ne l'ai pas trop testé, c'est une contribution de David, un spipien de la zone.

Après je trouve que c'est une demi-solution… Au final la vraie bonne chose à faire serait de les migrer tous automatiquement à l'install du plugin, mais tant que c'est pas inclus dans le core c'est problématique. Et au final, tant que les API's retournent les bons logos, on se fiche de savoir comment ils sont stockés, l'important c'est de stocker les nouveaux avec la bonne méthode.

Les cas où ça vaut vraiment la peine de migrer tout de suite sont rares, mais ça peut arriver, comme c'était le cas pour David justement.
Il avait tellement de logos dans son site que ça posait des problèmes de performances (trop de fichiers dans le même dossier). Il a alors écrit ce script pour migrer tous ces logos en documents. En combinant ça avec le plugin Hash Documents (Hash documents - Plugins SPIP), les problèmes de perf étaient réglés.

Le critère {logo} pourrait aussi être déplacé dans rôles de
documents,
en faisant une adaptation pour la perf (cf. remarque de cedric) :
une
jointure sur le document lié avec le rôle adéquat. Par contre je ne
sais
pas s'il faut garder la compat avec les vieux logos pour ce critère,
si
ça impacte trop la perf ?

Tant qu'on a pas migré les anciens logos il faudra continuer à les prendre en compte dans ce critère, sous peine de casser le squelettes existants, non ?

> Chemin faisant, je me questionne à propos du « petit hack un peu
> vilain ». Je comprends que la fonction logo_supprimer ne pourras
> jamais
> voir les logos enregistrés comme documents. Elle devra alors être
> dépréciée à terme ?
>
> Je pense que ça serait dommage de sacrifier cette API. L'API
> actuelle
> du core est assez simple et facile à surcharger :
Ah oui, my bad !
En voyant que ce n'était pas des fonctions surchargeables (pas de
suffixe _dist), je m'étais arrêté là, mais on peut surcharger tout
le
fichier effectivement.
Je vais faire le nécessaire.

Top, merci !

--
bystrano

Le 05/03/2018 à 00:56, Charles Razack a écrit :

J'avais pensé à ajouter une option pour proposer de sauvegarder les vieux logos, histoire de pouvoir éventuellement les récupérer en cas de désinstallation du plugin (avec la mise en garde de mise sur l'espace disque occupé). Mais si on part là-dessus, ça pose d'autres questions : est-ce qu'il faut aussi convertir les nouveaux logos documents en vieux logos ? Ça ferait partir sur un truc bien compliqué (vérifier les doublons, les utilisations, etc.).

Les sauvegarder non, trop compliqué.
Peut être qu'à la désinstallation, ça pourrait juste recopier les documents avec role logo comme "vieux" logos ?
En même temps, on parle de quelque chose qui aurait vocation à intégrer le core, je ne suis pas sûr qu'il faille y passer trop de temps.

--
nicod_