Produits Entr'ouvert » w.c.s.

Il faut peut-être passer un peu de temps, en parallèle au code, à discuter du vocabulaire.

Le problème c'est que cette action peut servir à tout et n'importe quoi, sur tout et n'importe quel objet, difficile de la caractériser. Et c'est pour ça que j'en suis à "Action générique" pour l'instant, mais ça pourrait aussi être "Action sur mesure" ou "Action paramétrable".

Et il faudrait sans doute aussi renommer la liste dans laquelle cette action va apparaître :

Agir sur la demande => Agir sur une demande / fiche

Et finalement comme cette action concernera toujours une action globale dans un workflow différent du workflow en cours, mon nom préféré est "Workflow externe".

Je n'aime pas du tout workflow externe, ça fait deux mots creux. "Émettre/déclencher/envoyer un signal/un évènement/une notification/une action/un message/" ?

Aussi je trouverai bien que les cibles soient déduites des formulaires du workflow (qu'on y trouve automatiquement tous les champs liés à des fiches ou des formulaires (formulaire parent et formulaire fils via créer une demande).

Tu ne fabriques pas de workflows.

"Workflow" dans le contexte de Publik est un mot extrêmement précis et compris de tous. "Externe", j'ai expliqué sur la liste :

C'est moins correct que "tiers" puisque ça laisse la place à
une interprétation "externe à la plate-forme", mais c'est plus immédiatement
compréhensible concernant le fait qu'on parle d'un truc qui vient de l'extérieur
du workflow sur lequel on travaille.

(commentaire sur la capture)

On n'a généralement pas d'indentation comme ça des champs.

On aurait plutôt une case à cocher, et dessous, si décochée, la possibilité d'entrer une référence manuelle,

 Automatically get object from linked data
   [ ]
 Reference to object
   [ ......... ]

Mais il me semble qu'on a le code d'affichage dynamique du second champ uniquement sur base d'un <select>, il y aurait du coup un peu de modif dans les termes pour faire passer ça ainsi; et magie ça retombe sur la description du ticket.

      Objet concerné :
        [ automatique    |v]  (ou expression)

_~~

class TargetObject(object):

Le (object) n'est pas nécessaire (on est en python 3). Mais plus fondamentalement, et oui je sais que j'ai laissé passer ça dans l'action de création d'une demande; je préférerais vraiment avoir un dictionnaire, notamment pour pouvoir se reposer sur la logique existante de sérialisation, plutôt qu'avoir à manuellement la gérer.

_~~

        trigger = self.get_trigger(objectdef.workflow)
        if not trigger:
            LoggedError.record('Could not find trigger "%s"' % self.event,

Il faut garder un seul nom, ça me semble compliqué d'avoir parfois ça appelé trigger, parfois ça appelé event.

La question des mots est sensible, pour être sûr d'être sur un ensemble cohérent, il serait peut-être bon d'exposer, en français, les différents endroits.

_~~

    def perform(self, formdata):

La recherche de l'objet associé mérite sans doute sa propre méthode (ses?).

_~~

Quid de plusieurs demandes du même type liées ?

_~~

Quid d'un objet qui est référencé via un champ liste de la demande ? (qui me semble un cas bien plus commun).

_~~

        try:
            perform_items(trigger.parent.items, target_data)
        except KeyError as e:
            LoggedError.record('Linked object "%s" does not exist' % target_data.get_display_name(),
                               formdata=formdata, exception=e)

Ça me semble embrasser un KeyError trop large.

_~~

    def get_parameters(self):
        return ('slug', 'target_object', 'event')

manque condition.

Frédéric Péters a écrit :

Mais il me semble qu'on a le code d'affichage dynamique du second champ uniquement sur base d'un <select>, il y aurait du coup un peu de modif dans les termes pour faire passer ça ainsi; et magie ça retombe sur la description du ticket.

J'ai découvert data-dynamic-display-, merci.

Le (object) n'est pas nécessaire (on est en python 3). Mais plus fondamentalement, et oui je sais que j'ai laissé passer ça dans l'action de création d'une demande; je préférerais vraiment avoir un dictionnaire, notamment pour pouvoir se reposer sur la logique existante de sérialisation, plutôt qu'avoir à manuellement la gérer.

Yep.

Il faut garder un seul nom, ça me semble compliqué d'avoir parfois ça appelé trigger, parfois ça appelé event.

Effectivement je fais un micmac, alors que le message doit être que le trigger corresponant à l'événement n'est pas trouvé.

La recherche de l'objet associé mérite sans doute sa propre méthode (ses?).

Je penche pour générateur qui retourne tous les target_data et ainsi cela prendra en compte plusieurs demandes/fiches liées.

Quid d'un objet qui est référencé via un champ liste de la demande ? (qui me semble un cas bien plus commun).

Je pensais couvrir ce cas avec en choisissant le type de l'objet et en définissant son idéntifiant.

Ça me semble embrasser un KeyError trop large.

Oui, cela comprend la demande qui n'existe plus (c'est ce que je vise ici) mais aussi le formdef qui a été supprimé. Mais comme la vérification de l'existence du formdef est vérifié en amont, seule l'absence de la demande peut lever cette exception il me semble.

seule l'absence de la demande peut lever cette exception il me semble.

KeyError peut être levé par tout le code exécuté dans perform_items, c'est-à-dire vraiment beaucoup de code.

J'écrivais :

La question des mots est sensible, pour être sûr d'être sur un ensemble cohérent, il serait peut-être bon d'exposer, en français, les différents endroits.

_~~

Quid d'un objet qui est référencé via un champ liste de la demande ? (qui me semble un cas bien plus commun).

Je pensais couvrir ce cas avec en choisissant le type de l'objet et en définissant son idéntifiant.

Le cas commun devrait être couvert de manière automatique, ça ferait des vacances à tout le monde.

La question des mots est sensible, pour être sûr d'être sur un ensemble cohérent, il serait peut-être bon d'exposer, en français, les différents endroits.

Ce que je voulais dire ici, c'est qu'il serait utile (je pense) de lister les différentes chaines parlant de tout ça, en français par facilité, mais anglais/français très bien aussi; vite fait en me basant sur _() :

• Event (libellé de champ dans l'action), en français ?
• event « %s » on %s (description de l'action), français ?
• External workflow (by event "%s") (description déclencheur configuré), français ?
• Event name (configuration déclencheur), français ?

Frédéric Péters a écrit :

Ce que je voulais dire ici, c'est qu'il serait utile (je pense) de lister les différentes chaines parlant de tout ça, en français par facilité, mais anglais/français très bien aussi; vite fait en me basant sur _() :

• Event (libellé de champ dans l'action), en français ?

Je vais renommer en "Trigger", la traduction existe déjà.

• event « %s » on %s (description de l'action), français ?

Cela va dévenir "trigger « %s » on %s ", et "déclencheur « %s »" en français/

• External workflow (by event "%s") (description déclencheur configuré), français ?

=> "External workflow (by name « %s »)". Français: "Workflow externe (par le nom « %s »)"

• Event name (configuration déclencheur), français ?

=> "Name". Traduction déjà existante.

(l'objectif est de faire un truc clair pas de réutiliser au maximum les traductions existantes)

Tout à fait.

Je préfère appeler le déclencheur par son nom et pas utiliser "event".

Pas encore relu mais pour faciliter l'intégration, il y aurait moyen que ça demande activation via site_option, histoire de s'offrir un temps de chauffe avec une utilisation contrôlée par les CPF ?

Ça a été abordé quelque part mais être rangé dans "agir sur la demande" est bizarre vu que c'est justement fait pour agir sur une demande différente. C'est #40204#note-5, prendre ça en compte.

Après la sélection d'un type d'objet, on se trouve envoyé vers l'index des actions, faut retourner dedans pour choisir l'objet cible. Le même genre de comportement a été adapté dans l'action de création d'une demande, #39732, il faudrait un truc similaire ici. (en gros, rester sur la page du formulaire tant les trois champs n'ont pas été complétés.

Une fois ça en place, ça me va que quelqu'un relise/valide demain et que ça soit ensuite traduit/taggué/déployé.

Tu peux juste mettre required=True sur les 3 champs dans ton cas.

• wcs/carddef.py : tu peux faire l'import de Workflow dans la ligne précédente, en fait
• wcs/wf/external_workflow.py :
  • passer le copyright à 2020
  • ('auto', _('Automatic'), 'auto'), : plutôt "All linked objets" que "Automatic" qu'on ne comprends pas
  • self.widgets[1].extra_css_class = 'grid-3-4' mais juste au dessus tu fais extra_css_class='grid-3-4', je peux que tu peux virer cette dernière ligne ... mais en fait ça me semble un peu casse-gueule de jouer une grille ici, parce qu'il y a de l'affichage conditionné. Autant supprimer tout affaire de grille selon moi, si l'affichage ne le nécessite pas (de fait on met jamais vraiment de grid dans le code de nos applis)

... bon ... je m'arrête ici, relus 2 fois et je ne comprends pas le but de tout ces codes (et le ticket ne le dit pas, j'ai suivi ce projet de trop loin et j'ai pas de temps d'archéologie). Je passe la main.

Il n'y a pas d'urgence particulière ici (à ma connaissance), il est plutôt important d'avoir quelque chose de clair. C'est déjà dans cette optique que je demandais que ça soit visible uniquement après avoir activé une option, sur l'idée de prendre le temps et les ajustements pour clarifier (l'action et/ou sa compréhension).

Concernant l'archéologie, sur le nom de l'action, il y a le sous-fil "nommage de l'action «action tiers»" que tu inities le 3 mars et sur l'action elle-même le fil "Fiches: édition et suppression" initié par Serghei le 21 février.

Thomas Noël a écrit :

... bon ... je m'arrête ici, relus 2 fois et je ne comprends pas le but de tout ces codes (et le ticket ne le dit pas, j'ai suivi ce projet de trop loin et j'ai pas de temps d'archéologie). Je passe la main.

• soit automatiquement
  • toute les fiches référencés parmi les champs Item du formulaire,
  • touls formulaires lié via l'action création demande (il manque la possibilité de la demande parente),
• soit "manuellement" via une expression qui doit renvoyer l'id d'une fiche ou d'une demande.

3. en dernier lieu on choisit le nom du trigger en question, à choisir parmi ceux disponibles dans le workflow des fiches ou formulaires visés en 1

Je note juste ici que si ça rend la configuration facile dans le cas fiche, dans le cas action sur le formulaire parent ou fils (avec une action de création de demande) ça rend la chose quasi impossible de manière générique (le workflow n'est pas réutilisable sur plusieurs type de formulaire, soit pour accéder au parent dont le type peut varier, soit pour accéder aux enfants dans le cas création du même type); ce sera l'occasion d'un autre ticket.

tout ces codes

Ce bout est peut-être à clarifier, je ne suis pas bien sûr de ce à quoi ça fait référence.

Il y a un "code" qui est le libellé pour identifier un "déclencheur", similaire au "déclencheur" sur un saut automaituqe ou à l'identifiant qui se définit dans le "déclencheur" "webservice" d'une action globale. (l'attribut "name" de WorkflowGlobalActionExternalWorkflowTrigger).

Il y a aussi au niveau de l'action de manière éditable une "expression" pour faire référence à un objet; ça manque clairement d'information sur ce qui se met là-dedans. (c'est l'id du formdata/carddata du type renseigné dans le premier champ, et je suis pas fan) (dans le second fil de mails que j'évoque c'est ma question "comment faire référence à un objet tiers ?").

Mais "code" pointe peut-être autre chose.

Thomas Noël a écrit :

• ('auto', _('Automatic'), 'auto'), : plutôt "All linked objets" que "Automatic" qu'on ne comprends pas

Dans ce cas plutôt "All" avec hint = "Automatic linked objects, or expression" ?

• self.widgets[1].extra_css_class = 'grid-3-4' mais juste au dessus tu fais @extra_css_class='grid-3-4',

Yep. extra_css_class='grid-3-4' ne sert à rien, viré.

mais en fait ça me semble un peu casse-gueule de jouer une grille ici, parce qu'il y a de l'affichage conditionné. Autant supprimer tout affaire de grille selon moi, si l'affichage ne le nécessite pas (de fait on met jamais vraiment de grid dans le code de nos applis)

Tu as raison, c'est fragile, même si je trouve le rendu des 2 widgets sur la même ligne plutôt sympa.

Ok, j'ai ajouté du liquide de refroidissement et ça va mieux.

Mais "code" pointe peut-être autre chose.

C'est le mode "auto" qui m'a perdu et m'a fait un peu couler en me disant qu'on allait dans le mur (quelles images) :

+            # search linked objects in data sources
+            for field in formdata.get_formdef().fields:
+                if field.data_source and field.data_source['type'] == self.slug:
+                    data_ids.append(formdata.data.get(field.id))
+            # search in evolution
+            for part in formdata.iter_evolution_parts():
+                if isinstance(part, LinkedFormdataEvolutionPart) and part.formdef_class == objectdef.__class__:
+                    data_ids.append(part.formdata_id)

Ça ramasse large, alors que le terme "Automatic" me donnait une impression d'intelligence... mais non, on va "tout rafler". Et c'est bien ainsi, mais il faut le rendre plus explicite. Donc je propose "All linked forms/card of this family" au lieu de "Automatic" (et peut-être "all" au lieu de "auto" dans le cpde).

Et quitte à rafler : « for field in formdata.get_formdef().get_all_fields(): » pour attraper aussi les données de traitement.

Je ne comprends pas pourquoi tu laisses ('all', _('All'), 'all'), c'est encore pire que "Automatic". Afficher "Automatique" ou "Tous", ça veut rien dire, on comprends pas. Ce que je propose c'est vraiment de mettre "Tous les objets liés", on se débarrasse d'un hint inutile et assez incompréhensible "tous les objects liés ou une expression", ça me semble juste confusant.

J'en arrive à me dire qu'il faut juste dégager le mode "manuel", ce qui du coup retire le besoin de trouver d'autres libellés, ça laisse juste à expliquer correctement quelque part la manière dont l'action trouve l'objet concerné.

Aussi, enregistrer le trigger_id c'est pas bon (...) à la place du name, ait un identifiant (identifier, comme pour WorkflowGlobalActionWebserviceTrigger). Et c'est cet identifiant qui remplacera le "id".

À ce compte, n'ajoutons pas de nouveau type de déclencheur, renommons l'existant "Webservice" en "Appel externe", qui couvrira aussi bien les appels par webservice que le déclenchement "intra-w.c.s." par cette nouvelle action.

Cependant ça serait sans doute bien de dès maintenant stocker "action:xxx" dans le trigger_id.

Et pour le stockage, oui, ça deviendrait "action:xxx", en anticipation d'une gestion de "jump:xxx" qui couvrirait les sauts.

Dès qu'on parle de libellé, j'essaie de comprendre et là j'y arrive pas, je pourrai réagir uniquement quand je verrai le truc, sorry.

Une chose toutefois : "objet" ça veut rien dire pour nous autres administrateurs fonctionnels.

L'idée de l"Appel externe" j'aime bien en revanche.

Un doute (alimenté par Steph) m'habite, est-ce que cette action va bien permettre de modifier une fiche ? C'est quand même sa principale raison d'être au départ.

Or, comme vous avez choisi de la faire fonctionner avec des actions globales plutôt qu'avec des statuts, et qu'il n'y pas d'action globale d'édition (pas d'actions globales interactives), je vois pas comment on va faire.

Frédéric Péters a écrit :

Je ne comprends pas pourquoi tu laisses ('all', _('All'), 'all'), c'est encore pire que "Automatic". Afficher "Automatique" ou "Tous", ça veut rien dire, on comprends pas. Ce que je propose c'est vraiment de mettre "Tous les objets liés", on se débarrasse d'un hint inutile et assez incompréhensible "tous les objects liés ou une expression", ça me semble juste confusant.

J'en arrive à me dire qu'il faut juste dégager le mode "manuel", ce qui du coup retire le besoin de trouver d'autres libellés, ça laisse juste à expliquer correctement quelque part la manière dont l'action trouve l'objet concerné.

• on a par exemple la liste des Item(s)Field avec avec varname en champs ou champs de traitement (tous formulaires liés au workflow courant confondus), valeur target=('field', field.varname, carddef.url_name)
• les actions create-formdata avec varname pour le workflow en cours (dont on connaît le type actuellement, mais pas forcément si le ticket "Même formulaire" est fait), target=('link', link_item.varname)
• le formulaire parent (dont on connaît pas le type par contre :/), target=('parent', None).

• soit de laisser une saisie libre pour les cas limites,
• soit de proposer tous les déclencheurs tout workflow confondu mais liés à un formulaire si on ne peut pas faire mieux, cas d'un formulaire de type indéterminé (formulaire parent).

Je pense que dans l'idée on veut aussi ne pas avoir des milliers de déclencheurs avec des noms différents, ce serait du « design smell ».

Et juste une question: quel moyen y-a-t-il pour la demande qui déclenche l'évènement de communiquer de l'information à la demande/fiche qui le reçoit ? (certainement ça a été évoqué mais j'avoue ne pas avoir bien suivi)

Si ça repart dans un tour de discussions, ça peut être sur la liste, plus pratique que l'historique linéaire du ticket ?

(ma prochaine participation à ce ticket c'est le rejeter quand il atteindra 50 commentaires).

Thomas Noël a écrit :

Ce que je propose c'est vraiment de mettre "Tous les objets liés", on se débarrasse d'un hint inutile et assez incompréhensible "tous les objects liés ou une expression", ça me semble juste confusant.

Tu as raison. Corrigé.

Mais on peut laisser filer ça pour l'instant car j'ai pas vraiment d'idée pour remplacer "expression". En tout cas si on reste sur le numéro, il faut plutôt appeler cela "Object number" ; tu peux mettre ça dans ton patch.

Je préfère laisser l'option pour couvrir le cas d'usage (qui sera très probablement rare) ou on souhaite pointer un objet précis.

Dans mon idée initiale il faut agir sur les objets distants via des trigger (sauts automatiques). Mais passons, ça pourra être une évolution. Cependant ça serait sans doute bien de dès maintenant stocker "action:xxx" dans le trigger_id.

Yep, done.

Pire, ça pourra être une autre action qui n'a rien à voir. Il faut donc que WorkflowGlobalActionExternalWorkflowTrigger, à la place du name, ait un identifiant (identifier, comme pour WorkflowGlobalActionWebserviceTrigger). Et c'est cet identifiant qui remplacera le "id".

Yep.

Détail pour faire joli : dans add_parameters_widgets quand on ne trouve aucun type d'objet capable d'être déclenché, on devrait juste afficher un message au lieu de proposer une liste vide. "Aucun workflow du site ne contient d'action globale déclenchable par workflow externe" et return.

Bien vu.

Sur for objectdef in wf.formdefs() + wf.carddefs(): je pense que tu peux utiliser lightweight=True (pour ne pas charger les fields dont tu n'as pas besoin ici, ça bouffe un temps fou).

J'ai appris une chose, merci.

Aussi, jamais de guillemets français dans le code 'trigger « %s » on %s' → 'trigger "%s" on %s' --- et d'ailleurs c'est 'Global Action "%s" on %s'.

Même chose dans wcs/workflows.py : 'External workflow (by name « %s »)' → 'External workflow (by name "%s")'

Corrigé.

Comme suggère Frédéric dans le commentaire #42, je me base désormais sur le déclencheur "webservice" en la rénommant au préalable (et ça vire du code du patch initial).

Pour revenir sur les libellés, je vais reprendre la discussion sur la liste.

Serghei Mihai a écrit :

Thomas Noël a écrit :

Ce que je propose c'est vraiment de mettre "Tous les objets liés", on se débarrasse d'un hint inutile et assez incompréhensible "tous les objects liés ou une expression", ça me semble juste confusant.

Tu as raison. Corrigé.

Idéalement au lieu de "objet" il faudrait pouvoir indiquer "demande" ou "fiche" en fonction de ce qui a été sélectionné au dessus.

Mais on peut laisser filer ça pour l'instant car j'ai pas vraiment d'idée pour remplacer "expression". En tout cas si on reste sur le numéro, il faut plutôt appeler cela "Object number" ; tu peux mettre ça dans ton patch.

Je préfère laisser l'option pour couvrir le cas d'usage (qui sera très probablement rare) ou on souhaite pointer un objet précis.

Je suis de l'avis de Frédéric, de laisser tomber ce cas, puisqu'il n'arrivera jamais (et donc ne sera pas facile à déboguer). On verra s'il se présente un jour. Ca a un énorme avantage : ça éviter de chercher un label :)

Comme suggère Frédéric dans le commentaire #42, je me base désormais sur le déclencheur "webservice" en la rénommant au préalable (et ça vire du code du patch initial).

Ok.

Thomas Noël a écrit :

Je suis de l'avis de Frédéric, de laisser tomber ce cas, puisqu'il n'arrivera jamais (et donc ne sera pas facile à déboguer). On verra s'il se présente un jour. Ca a un énorme avantage : ça éviter de chercher un label :)

Argument béton.

+import time
+import xml.etree.ElementTree as ET
...

time, ET, quixote.get_request, htmltext, cached_property, ComputedExpressionWidget sont importés et pas utilisés.

    def get_workflow_webservice_triggers(self, workflow):
        triggers = []

triggers est défini et pas utilisé.

    def get_parameters(self):
        return ('slug',  'trigger_id', 'condition')

Il y a deux espaces devant 'trigger_id'.

Ces commentaires sont offerts par flake™.

_~~

        try:
            return object_class.get_by_urlname(slug)
        except Exception as e:
            pass

except trop large, ça devrait être uniquement KeyError. (et le as e n'est pas exploité).

            if len(objects) == 1:
                form.add_global_errors((_('No workflow with external triggerable global action '),))

Pas un bug mais ça sera plus lisible avec un liste qu'un tuple.

                    triggers_names += [(trigger_id, trigger.parent.name, trigger_id)
                    ]

Retour à la ligne farfelu pour ce ].

Aussi, pas un bug mais plutôt utiliser .append().

                        objects += [(object_slug, objectdef.name, object_slug)]

même commentaire ici.

            return _('action "%s" on %s' % (trigger.parent.name, objectdef.name))

Tu passes ainsi une chaine assemblée à gettext, qui ne la trouvera pas dans son catalogue.

→ _('action "%s" on s') % (...) mais non plus, parce qu'avec plusieurs substitutions, il ~faut utiliser une substitution par keyword plutôt que par position. → _('action "(trigger_name)s" on %(form_name)s') % {...}.

        error_msg = 'Could not find linked "%s" object' % objectdef.name
...
                LoggedError.record(error_msg, formdata=formdata, exception=e)
...
            LoggedError.record('Could not find trigger "%s"' % self.trigger_id,

Les messages d'erreur exposés ainsi doivent être traduits. (je viens à ce sujet de poser #42102).

Toujours,

            return _('action "%(trigger_name)s" on %(form_name)s' % {
                'trigger_name': trigger.parent.name,
                'form_name': objectdef.name})

et autres occurences.

gettext ne fonctionne pas ainsi, la chaine doit lui être donnée avant substitutions.

Comme noté plus haut, ça doit être _('action "(trigger_name)s" on %(form_name)s') % {...}.

_('la chaine') % {les substitutions}

pas

_('la chaine' % {les substitutions})

                data_ids.append(part.formdata_id)
        error_msg = _('Could not find linked "%(object_name)s" object') % {'object_name': objectdef.name}

        for target_id in data_ids:
            try:
                yield objectdef.data_class().get(target_id)
            except KeyError as e:
                # use custom error message depending on target type
                LoggedError.record(error_msg, formdata=formdata, exception=e)

C'est bizarre d'avoir le error_msg créé à cet endroit, surtout que ça va l'amener unique et si jamais il y avait plusieurs data_ids on ne saura pas lequel a pu foirer (je me disais qu'au pire on l'aurait peut-être dans la trace mais non il n'y a pas de production automatique de trace).

_~~

+        trigger = self.get_trigger(objectdef.workflow)
+        if not trigger:
+            LoggedError.record(_('Could not find trigger "%(trigger_name)s"') % {
+                'trigger_name': self.trigger_id}, formdata=formdata)

on récupère un trigger sur base d'un workflow, mais si on ne le trouve pas on loggue via une substitution parlant de nom de trigger une variable évoquant un identifiant de trigger, c'est quand même tout bizarre.

_~~

(je zappe d'autres commentaires du même niveau et reprendrai plus tard pour valider whatever).

A la base je souhaitais faire apparaître le nom du trigger dans la ligne de log, mais on peut la zapper.

Ce que je voulais pointer c'était la confusion générale au niveau des variables, si tu veux faire apparaitre un nom très bien mais que la variable qui est affichée contienne "name", pas "id", parce qu'on s'y perd.