Project

General

Profile

Development #39803

Création d'une demande/resoumission : avoir accès à la demande parente

Added by Benjamin Dauvergne 3 months ago. Updated about 2 months ago.

Status:
Solution déployée
Priority:
Normal
Target version:
-
Start date:
12 Feb 2020
Due date:
% Done:

0%

Patch proposed:
Yes
Planning:
No

Description

Via form.links on a accès aux demandes filles, mais les données formdata.submission_context['orgi_formdef/formdata_id'] elles ne sont pas exposés via les variables de substitution, je propose un form.parent sur le même principe que form.parent.<varname>, ainsi une demande fille pourra référencer des données de la demande parente (exemple, une demande de pré-réservation validée pourra fournir les détails de la réservation à la demande fille permettant de remplir un dossier administratif).

0001-misc-add-parent-variable-to-lazy-formdata-39803.patch View (3.31 KB) Benjamin Dauvergne, 12 Feb 2020 07:49 PM

0002-misc-prevent-infinite-recursion-when-walking-lazy-fo.patch View (1.57 KB) Benjamin Dauvergne, 02 Mar 2020 03:16 PM

0001-misc-add-parent-variable-to-lazy-formdata-39803.patch View (3.32 KB) Benjamin Dauvergne, 02 Mar 2020 03:16 PM

Firefox_Screenshot_2020-03-31T13-39-09.307Z.png View (1.03 MB) Benjamin Dauvergne, 31 Mar 2020 03:39 PM

0002-misc-prevent-infinite-recursion-when-walking-lazy-fo.patch View (1.64 KB) Benjamin Dauvergne, 31 Mar 2020 04:22 PM

0001-misc-add-parent-variable-to-lazy-formdata-39803.patch View (3.32 KB) Benjamin Dauvergne, 31 Mar 2020 04:22 PM

0003-misc-do-not-show-form_parent-when-it-s-None-39803.patch View (798 Bytes) Benjamin Dauvergne, 31 Mar 2020 04:22 PM

Firefox_Screenshot_2020-03-31T14-25-29.215Z.png View (747 KB) Benjamin Dauvergne, 31 Mar 2020 04:25 PM

Firefox_Screenshot_2020-03-31T14-25-11.515Z.png View (451 KB) Benjamin Dauvergne, 31 Mar 2020 04:25 PM

0001-misc-prevent-infinite-recursion-when-walking-lazy-fo.patch View (1.62 KB) Benjamin Dauvergne, 31 Mar 2020 06:34 PM

0002-misc-add-parent-variable-to-lazy-formdata-39803.patch View (7.07 KB) Benjamin Dauvergne, 31 Mar 2020 06:34 PM

Firefox_Screenshot_2020-03-31T16-33-03.895Z.png View (264 KB) Benjamin Dauvergne, 31 Mar 2020 06:35 PM

Firefox_Screenshot_2020-03-31T16-32-53.662Z.png View (275 KB) Benjamin Dauvergne, 31 Mar 2020 06:35 PM

Firefox_Screenshot_2020-04-03T15-24-46.132Z.png View (10.7 KB) Benjamin Dauvergne, 03 Apr 2020 05:25 PM

Firefox_Screenshot_2020-04-03T15-24-28.537Z.png View (10.1 KB) Benjamin Dauvergne, 03 Apr 2020 05:25 PM

0001-misc-prevent-infinite-recursion-when-walking-lazy-fo.patch View (1.62 KB) Benjamin Dauvergne, 10 Apr 2020 02:17 PM

0002-misc-add-parent-variable-to-lazy-formdata-39803.patch View (5.4 KB) Benjamin Dauvergne, 10 Apr 2020 02:17 PM

Firefox_Screenshot_2020-04-10T12-22-46.393Z.png View (28.2 KB) Benjamin Dauvergne, 10 Apr 2020 02:23 PM

42853
42859
42860
42867
42868
43024
43025
43342

Associated revisions

Revision 18782e58 (diff)
Added by Benjamin Dauvergne about 2 months ago

misc: prevent infinite recursion when walking lazy formdata (#39803)

Revision 9ec51c01 (diff)
Added by Benjamin Dauvergne about 2 months ago

misc: add parent variable to lazy formdata (#39803)

History

#1 Updated by Benjamin Dauvergne 3 months ago

  • Assignee set to Benjamin Dauvergne

#2 Updated by Benjamin Dauvergne 3 months ago

#3 Updated by Benjamin Dauvergne 3 months ago

Le fait d'ajouter un accesseur dans les deux sens pose problème pour la vue /inspect (ça boucle).

#4 Updated by Thomas Noël 2 months ago

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.

#5 Updated by Frédéric Péters 2 months ago

(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.

#6 Updated by Thomas Noël 2 months ago

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.

#7 Updated by Frédéric Péters 2 months ago

#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).

#8 Updated by Benjamin Dauvergne about 2 months ago

42853

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

À 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.

J'ai vu ça mais ça déscend quand même puis ça remonte et la page inspect prend un temps dingue à s'afficher parce que même si c'est pas infini, c'est trop (ex.: form_parent_form_links_newform_form_parent_form_number_raw).

Firefox_Screenshot_2020-03-31T13-39-09.307Z.png

#9 Updated by Benjamin Dauvergne about 2 months ago

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).

#10 Updated by Benjamin Dauvergne about 2 months ago

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

#11 Updated by Benjamin Dauvergne about 2 months ago

  • Status changed from Solution proposée to En cours

Bon ça ne marche pas du tout, visiblement inspect() récupère juste un CompatibilityNamesDict() (sur form_parent et ne sait pas comment l'afficher).

            elif hasattr(v, 'inspect_keys') or isinstance(v, dict):                   
                # skip expanded                                                       
                continue 

va falloir que je réfléchisse un peu plus.

#12 Updated by Benjamin Dauvergne about 2 months ago

Avec la suggestion de Thomas et au lieu d'empêcher toute récursion je la limite à 3 (je vais mettre un screenshot) mais en gros on voit jusqu'à form_var_* (et je devrais faire pareil sur links_*).

#13 Updated by Benjamin Dauvergne about 2 months ago

42859
42860

Demande parente : Firefox_Screenshot_2020-03-31T14-25-29.215Z.png

Demande fille : Firefox_Screenshot_2020-03-31T14-25-11.515Z.png

#14 Updated by Benjamin Dauvergne about 2 months ago

Nouvelle proposition cette fois on ne voit justement qu'un lien vers la page inspect du formulaire.

#16 Updated by Frédéric Péters about 2 months ago

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).

#17 Updated by Benjamin Dauvergne about 2 months ago

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)

#18 Updated by Frédéric Péters about 2 months ago

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].

#20 Updated by Frédéric Péters about 2 months ago

(ç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 ?

#21 Updated by Benjamin Dauvergne about 2 months ago

Voilà.

PS: j'ai bougé tout le rendu dans inspect comme demandé et ça ne vise plus que les variables se terminant par 'form_parent', screenshot attaché.

#22 Updated by Benjamin Dauvergne about 2 months ago

  • File Firefox_Screenshot_2020-04-10T12-17-45.695Z.png added

#24 Updated by Benjamin Dauvergne about 2 months ago

  • File deleted (Firefox_Screenshot_2020-04-10T12-17-45.695Z.png)

#25 Updated by Thomas Noël about 2 months ago

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)

#26 Updated by Benjamin Dauvergne about 2 months ago

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.

#27 Updated by Thomas Noël about 2 months ago

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

#28 Updated by Thomas Noël about 2 months ago

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 ?

#29 Updated by Benjamin Dauvergne about 2 months ago

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.

#30 Updated by Frédéric Péters about 2 months ago

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

#31 Updated by Benjamin Dauvergne about 2 months ago

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.

#32 Updated by Thomas Noël about 2 months ago

  • Status changed from Solution proposée to Solution validée

Pour moi c'est ok ainsi en première approche, car j'ai pas d'autre idée (pour éviter la "coupure de récursion").

#33 Updated by Benjamin Dauvergne about 2 months ago

  • Status changed from Solution validée to Résolu (à déployer)
commit 9ec51c0174b89234c75cf94b9501cf82a7820e95
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Wed Feb 12 19:48:49 2020 +0100

    misc: add parent variable to lazy formdata (#39803)

commit 18782e585a615dabf26c65bbdd92650222b17f44
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Mon Mar 2 14:09:10 2020 +0100

    misc: prevent infinite recursion when walking lazy formdata (#39803)

#34 Updated by Frédéric Péters about 2 months ago

  • Status changed from Résolu (à déployer) to Solution déployée

Also available in: Atom PDF