Produits Entr'ouvert » w.c.s.

• Mozilla/5.0 (Windows NT 6.1; WOW64; rv:42.0) Gecko/20100101 Firefox/42.0 12:26:15
• Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/47.0.2526.111 Safari/537.36

• aller sur la page
• oublier un champ
• valider
• l'URL devient alors et pour toujours /agoraplus/integration/address/communes/Alfortville/types-of-streets/Rue/ parce que les [var_....] sont remplacées par les variables du formulaires, désormais existantes

Dans qommon/form.py

1704         if '[' in self.url:
1705             vars = get_publisher().substitutions.get_context_variables()
1706             # skip variables that were not set (None)
1707             vars = dict((x, y) for x, y in vars.items() if y is not None)
1708             url = misc.get_variadic_url(self.url, vars, encode_query=False)

il faut aussi ignorer (skip) toutes les variables qui sont sur la page actuelle

• un formulaire avec une page contenant une condition (c'est ça qui ajoute les var_ dans les variables du contexte)
• un premier champ liste avec source de données JSON, varname "x"
• un second champ liste avec une source de données JSONP dont l'URL contient [var_x]
  En cas de page non valide, var_x se retrouve valué dans les variables du contexte, et l'URL du JSONP n'est plus dynamique ([var_x] y est remplacé par la valeur une fois pour toute).

C'est ce que corrige le patch ci-dessus, en remplaçant toujours les var_ de façon dynamique.

La correction ne me va pas trop, on n'a pas envie de maintenir deux jeux de variables et d'avoir à expliquer que parfois c'est var_ et parfois c'est form_var et quand exactement on ne sait pas trop.

Selon moi ce qu'il faut c'est pouvoir, dans le cas précis du jsonp, dire qu'on ne veut pas les variables de la page courante pas encore validée. Ce que je verrais c'est donc un paramètre exclude supplémentaire à get_context_variables, par lequel on pourrait passer une liste des sources de données à exclure; d'ensuite marquer ConditionVars (du evaluate de la page de condition) avec un identifiant et de passer celui-ci dans le nouveau paramètre exclude.

Les conditions étant établies c'est maintenant normalement facile d'ajouter un test.

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

La correction ne me va pas trop, on n'a pas envie de maintenir deux jeux de variables et d'avoir à expliquer que parfois c'est var_ et parfois c'est form_var et quand exactement on ne sait pas trop.

Selon moi ce qu'il faut c'est pouvoir, dans le cas précis du jsonp, dire qu'on ne veut pas les variables de la page courante pas encore validée. Ce que je verrais c'est donc un paramètre exclude supplémentaire à get_context_variables, par lequel on pourrait passer une liste des sources de données à exclure; d'ensuite marquer ConditionVars (du evaluate de la page de condition) avec un identifiant et de passer celui-ci dans le nouveau paramètre exclude.

Exclure ConditionVars ne va pas exclure les variables de la page courante non validé, mais toutes les var_xx. Ce qui est une bonne chose, puisqu'elles n'étaient pas toujours là (il fallait être dans une page avec une condition).

Donc oui, si on peut nettoyer au lieu de tenter comme moi de conserver un fonctionnement tordu, c'est mieux. Je regarde à faire ça.

Exclure ConditionVars ne va pas exclure les variables de la page courante non validé, mais toutes les var_xx. Ce qui est une bonne chose, puisqu'elles n'étaient pas toujours là (il fallait être dans une page avec une condition).

À d'autres endroits ça ne devrait de toute façon pas être utilisé, c'était seulement explicitement mis dans l'évaluation des conditions pour compabilité avec des versions antérieures.

L'autre endroit où des [var_...] apparaissent, c'est bien le jsonp mais même là ça devrait pouvoir être remplacé par des [form_var_...], ou alors par un nom tout autre, qui n'apportera pas confusion. Tiens, ça peut d'ailleurs peut-être être une autre piste pour ce ticket, ajouter au jsonp paramétrique la prise en compte d'un autre préfixe (en supplément du var_ à garder pour compatibilité), mais c'est un peu foutraque javascript, plus difficile à tester.

Après avoir dormi dessus, je me dis que ton approche initiale peut aller, et même de manière plus franche, dans cet esprit :

--- a/wcs/qommon/form.py
+++ b/wcs/qommon/form.py
@@ -1716,8 +1716,11 @@ $("#form_%(id)s").select2({

         if '[' in self.url:
             vars = get_publisher().substitutions.get_context_variables()
-            # skip variables that were not set (None)
-            vars = dict((x, y) for x, y in vars.items() if y is not None)
+            # skip variables that were not set (None) and backward-compatiblity
+            # variables (starting with var_) that conflicts with the variables
+            # that can be dynamically set by javascript on field changes.
+            vars = dict((x, y) for x, y in vars.items() if
+                    y is not None and not x.startswith('var_'))
             url = misc.get_variadic_url(self.url, vars, encode_query=False)
         else:
             url = self.url

(et peu importe l'option retenue, elle doit être mise en place aussi dans AutocompleteStringWidget)

Autre approche qui me semble très bien, ce serait de reprendre le evaluate_condition() du champ Page et de ne pas insérer dans ConditionVars les variables de compatibilité (var_), quelque chose comme ça : (en actualisant les commentaires)

--- a/wcs/fields.py
+++ b/wcs/fields.py
@@ -1346,18 +1346,19 @@ class PageField(Field):
         # and add them as form_var_xxx.
         from formdata import get_dict_with_varnames
         live_data = get_dict_with_varnames(formdef.fields, dict)
-        for k, v in live_data.items():
-            live_data['form_' + k] = v
+        form_live_data = dict(('form_' + x, y) for x, y in live_data.items())

         # and feed those new variables in the global substitution system, they
         # will shadow formdata context variables with their new "live" value,
         # this may be useful when evaluating data sources.
         class ConditionVars(object):
             def get_substitution_variables(self):
-                return live_data
+                return form_live_data

         get_publisher().substitutions.feed(ConditionVars())
         data = get_publisher().substitutions.get_context_variables()
+        data.update(live_data)
+        data.update(form_live_data)

         try:
             if eval(condition, get_publisher().get_global_eval_dict(), data):

Cette dernière solution ; très bien vue !

Thomas Noël a écrit :

Cette dernière solution ; très bien vue !

Et finalement à reprendre ce patch, je n'en suis plus aussi sûr : ça m'ennuie un peu de changer le comportement, même «bizarre», du calcul des conditions. En fait, je n'arrive pas à percevoir les effets exacts de ne plus mettre les var_ dans le get_substitution_variables().

Et donc je pencherai plutôt le patch du commentaire #12 qui nettoie les var_

L'apparition des var_… dans les variables de substitution c'est une conséquence de #8272 et personne n'en dépend (je laisse le soin au lecteur de vérifier ça); je préfère vraiment l'option de corriger le problème à la source, qui est l'ajout de ces var_, plutôt qu'au moment où attention elles sont là mais il ne faut pas les consommer.

Ack (quoique y'a un petit # qui traine dans un commentaire, c'est un scandale)