Produits Entr'ouvert » w.c.s.

Je me suis posé la question d'utilise form_orig parce que c'est ainsi que c'est codé (orig_formdef_id / orig_formdata_id) mais parent c'est joli.

Rien à dire sur 0001, sauf que l'inspecteur va afficher un form_parent à None sur toutes les demandes qui n'ont pas de parent. C'est un peu dommage je trouve, on pourrait l'éviter avec :

--- a/wcs/variables.py
+++ b/wcs/variables.py
@@ -213,7 +213,8 @@ class LazyFormData(LazyFormDef):
             if key[0] == '_' or key in blacklist:
                 continue
             if key == 'parent':
-                yield key, False  # = recurse
+                if self.parent:
+                    yield key, False  # = recurse
             else:
                 yield key

Si le "self.parent" va un peu trop loin parce qu'il faut un formdata.get_substitution_variables inutile ici, on pourrait décomposer avec un "has_parent(self)" et un "parent(self):" dans LazyFormData, le second utilisant le premier. Et on pourrait dnoc faire un "if self.has_parent():" à la place de "if self.parent:" dans le inspect_keys ci-dessus.

(Mais je suis ouvert à la remarque qui serait "c'est bien d'avoir un form_parent = None", ça veut dire explicitement que cette demande n'a pas de parent)

Sur 0002 pour moi ça manque de commentaire un peu plus explicite. Sur le « yield key, False # = recurse » j'aurai préféré une phrase qui explique qu'on renvoie ici un message spécifique (tuple) pour faire comprendre à get_flat_keys qu'il ne doit pas aller plus loin. Et dans get_flat_keys rappeler ce mécanisme au niveau du "if not isinstance(sub_key, str):". Je dis ça parce que ça m'a pris un peu de temps de piger le truc.

(Mais je suis ouvert à la remarque qui serait "c'est bien d'avoir un form_parent = None", ça veut dire explicitement que cette demande n'a pas de parent)

Là-dessus pas d'avis majeur de mon côté, instinctivement je serais pour ne pas afficher. (on n'affiche pas un form_attachments qui n'existe pas, bien qu'on voudrait peut-être, #21107).

À noter aussi que la récursion infinie que cela essaie d'éviter est déjà évitée par #39803. Ça n'empêche pas de se dire qu'on veut que parent_... ne soit pas développé/déplié mais je me demande quand même ce que ça affiche dans le /inspect.

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

(Mais je suis ouvert à la remarque qui serait "c'est bien d'avoir un form_parent = None", ça veut dire explicitement que cette demande n'a pas de parent)

Là-dessus pas d'avis majeur de mon côté, instinctivement je serais pour ne pas afficher. (on n'affiche pas un form_attachments qui n'existe pas, bien qu'on voudrait peut-être, #21107).
À noter aussi que la récursion infinie que cela essaie d'éviter est déjà évitée par #39803

#39803 c'est ce ticket lui-même, tu voulais parler d'un autre ?

Ça n'empêche pas de se dire qu'on veut que parent_... ne soit pas développé/déplié mais je me demande quand même ce que ça affiche dans le /inspect

Effectivement, comme form_link ça va ajouter beaucoup (beaucoup) de choses dans l'inspect qui ne sont pas liées à la demande elle-même. On pourrait effectivement voir à afficher les form_links et form_parent dans une section à part, comme on prévoit de le faire pour form_attachments. Ca aurait ma préférence.

#39803 c'est ce ticket lui-même, tu voulais parler d'un autre ?

yep, #40429...

ça va ajouter beaucoup (beaucoup) de choses dans l'inspect qui ne sont pas liées à la demande elle-même. (...)

Mon idée (mais ça quitte se ticket), c'était que tout soit affiché dans la même liste mais certaines entrées seraient "pliées" par défaut; on aurait form_attachments_… [déplier]. (j'étais parti pour le faire au moment où j'ai tapé l'utilisation des lazy pour le /inspect mais ça mêlait trop de choses).

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

#39803 c'est ce ticket lui-même, tu voulais parler d'un autre ?

yep, #40429...

ça va ajouter beaucoup (beaucoup) de choses dans l'inspect qui ne sont pas liées à la demande elle-même. (...)

Mon idée (mais ça quitte se ticket), c'était que tout soit affiché dans la même liste mais certaines entrées seraient "pliées" par défaut; on aurait form_attachments_… [déplier]. (j'étais parti pour le faire au moment où j'ai tapé l'utilisation des lazy pour le /inspect mais ça mêlait trop de choses).

De mon coté plutôt que de partir sur une mécanique un peu complexe, j'aurai plutôt passer du temps pour voir comment en faire des liens vers les vues inspect des demandes concernées (et des fiches aussi).

Le screenshot est obtenu en faisant un revert du patch 0002.

Pas vraiment fan, ça n'aide pas à capter les noms de variable qui peuvent être utilisés.

(note sur le côté, j'ai retiré les affichages directs des images pour laisser uniquement les vignettes).

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

Pas vraiment fan, ça n'aide pas à capter les noms de variable qui peuvent être utilisés.

(note sur le côté, j'ai retiré les affichages directs des images pour laisser uniquement les vignettes).

Donc tu préfères le patch d'avant ? (#39803-13)

Ma préférence déjà notée ce serait « certaines entrées seraient "pliées" par défaut; on aurait form_attachments_… [déplier]. ».

Mais j'ai également noté que ce serait pour ailleurs; à défaut, ici, donc je serais pour

- form_parent     <lien vers>
+ form_parent_…   /variables de la demande parente (<lien vers>)/

qui est "explicite" sur le fait qu'on peut avoir des variables commençant par form_parent_; et sur le détail ça attendra l'ajout d'un bouton [déplier].

(ça reste moche ce truc de coupage de récursion et à mon avis ça sera à défaire au moment d'étendre la vue pour pouvoir déplier mais passons)

Vu les tests ça fait aussi partir form_links_resubmitted_form_var_foo_string, ça pourrait concerner uniquement parent_ ? (a priori ne pas avoir la modif à create_formdata.py)

Sans doute un peu de répercussion ici elif hasattr(v, 'inspect_url') and getattr(v, '_formdata', None) != self.filled: pour que ça soit moins générique, que ça vise juste la variable "parent"; mais sans doute ça rajoute une autre couche pas terrible, donc passons tout ça, coupons aussi la récurision de "resubmitted", juste quand même ajouter un commentaire à cet endroit, genre "variable representing another form, subvariables are not included, a link to its inspect page is used instead"). (question ici sur les fiches, qui vont être prises aussi dans ce jeu, ok passons).

inspect_url pourrait être get_inspect_url retourner une URL, plutôt qu'un bout d'HTML ?

A mon tour, allez.

• tu peux supprimer le from quixote.html import htmltext de wcs/variables.py
• htmltext(_('<var>/variables from parent\'s request/</var>')) → htmltext('<var>%s</var>') % _('variables from parent\'s request') pour avoir un .po plus propre

Pour moi on peut se passer du get_inspect_url et faire un 'inspect_url': v['form'].backoffice_url() + 'inspect',

(Bon, aussi, je n'arrive pas bien à voir ce qui va se passer si v['form'] pointe vers un formulaire qui n'existe plus, du mal à suivre le fil)

Thomas Noël a écrit :

A mon tour, allez.

• tu peux supprimer le from quixote.html import htmltext de wcs/variables.py

ok.

• htmltext(_('<var>/variables from parent\'s request/</var>')) → htmltext('<var>%s</var>') % _('variables from parent\'s request') pour avoir un .po plus propre

ok.

Pour moi on peut se passer du get_inspect_url et faire un 'inspect_url': v['form'].backoffice_url() + 'inspect',

ok.

(Bon, aussi, je n'arrive pas bien à voir ce qui va se passer si v['form'] pointe vers un formulaire qui n'existe plus, du mal à suivre le fil)

LazyFormData.parent va renvoyer None, et donc isinstance(v, CompatibilityNamesDict) va renvoyer False, et donc ça on ne passera pas dans cette branche du elif.

PS: branche à jour.

Je pige pas trop les "/" dans "/variables from parent\'s request/" (ça fait un peu moche, mais bon, goûts, couleurs, tout ça)

Thomas Noël a écrit :

Je pige pas trop les "/" dans "/variables from parent\'s request/" (ça fait un peu moche, mais bon, goûts, couleurs, tout ça)

En dehors de ça j'ai rien d'autre à dire... Frédéric, en dernière passe, c'est ok ?

Thomas Noël a écrit :

Je pige pas trop les "/" dans "/variables from parent\'s request/" (ça fait un peu moche, mais bon, goûts, couleurs, tout ça)

C'est une demande explicite de Fred.

Je retombe pas sur mon commentaire mais ça symbolisait de l'italique.

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

Je retombe pas sur mon commentaire mais ça symbolisait de l'italique.

Ok je vire les barres obliques.

PS: branche à jour.