Projet

Général

Profil

Development #45135

Donnée de traitement à None dans un firstof avec default en chaine de caractère vide

Ajouté par Emmanuel Cazenave il y a plus de 3 ans. Mis à jour il y a plus de 3 ans.

Statut:
Fermé
Priorité:
Normal
Assigné à:
Version cible:
-
Début:
15 juillet 2020
Echéance:
% réalisé:

0%

Temps estimé:
Patch proposed:
Oui
Planning:
Non

Description

Une donnée de traitement toto qui doit prendre pour valeur {% firstof form_var_foo form_var_bar "" %}

Si form_var_foo et form_var_bar ne sont pas définis, toto prend la valeur None.

On s'attendrait plutôt à une chaîne de caractère vide ne serait-ce que parce qu'on utilise un template django.


Fichiers

Révisions associées

Révision 19a1ad6c (diff)
Ajouté par Thomas Noël il y a plus de 3 ans

workflows: set bo fields to empty value only if value before compute is empty (#45135)

Révision 1861bb36 (diff)
Ajouté par Thomas Noël il y a plus de 3 ans

workflows: set bo fields to empty value only if value before compute is empty (#45135)

Historique

#2

Mis à jour par Frédéric Péters il y a plus de 3 ans

Si form_var_foo et form_var_bar ne sont pas définis

Ces "ne sont pas définis" ne me sont jamais clairs; sur le formdata, le .data contient quoi ?

#3

Mis à jour par Thomas Noël il y a plus de 3 ans

Quand l'évaluation d'une donnée de traitement renvoie une chaine vide, on stocke None (ie on l'efface). C'est nouveau, cf #44310

+            if new_value == '':
+                # assign empty strings as None, as that will work for all field
+                # types
+                new_value = None
+
#4

Mis à jour par Emmanuel Cazenave il y a plus de 3 ans

Dans mon formulaire il n'y pas de variable avec l'identifiant bar ni avec l'identifiant foo, c'est pas une histoire de champs remplis ou pas en fonction d'une condition, c'est vraiment variables inconnues au bataillon à tous les coups.

(réponse joker après 15 min à ne pas trouver où aller mettre un point d'arrêt pour inspecter formdata.data)

#6

Mis à jour par Emmanuel Cazenave il y a plus de 3 ans

Et donc pour revenir à l'intention initiale de de #44310, ce serait possible de vider la donnée de traitement uniquement quand le gabarit est vide, mais pas quand le résultat de l'évaluation du gabarit est vide ?

#7

Mis à jour par Thomas Noël il y a plus de 3 ans

  • Assigné à mis à Thomas Noël
#8

Mis à jour par Thomas Noël il y a plus de 3 ans

#9

Mis à jour par Frédéric Péters il y a plus de 3 ans

Ok je comprends je pense l'intention mais ça ne me semble pas bon et je ne vois pas comment ça interviendrait de toute façon sur la situation d'Emmanuel où il dit que ses noms de variables sont fantaisistes et ne correspondent pas à des vrais champs.

Et même, on se trouverait une situation différente avant le passage par cette action, alors qu'il me semble opportun de toujours considérer de la même manière un champ sans contenu (que le champ vienne du formulaire ou des données de traitement).

#10

Mis à jour par Thomas Noël il y a plus de 3 ans

Sur l'action "donnée de traitemen", il devrait y avoir une différence entre un champ dont le contenu est vide (le widget affiche "Leaving the field blank will empty the value.") et un champ dont le contenu après calcul est une chaîne vide.

Dans le cas d'Emmanuel il y a une donné de traitement «form_var_foo» qui est calculée avec le gabarit {% firstof truc_qui_n_existe_pas et_celui_la_non_plus "" %}. Le rendu de ce gabarit donne une string vide, et donc #44310 attribue «None» à form_var_foo (alors qu'avant #44310 on attribuait bien la chaine vide). Résultat : l'utilisation de {{form_var_foo}} donne "None" au lieu d'une chaine vide, et ça fait planter la suite de l'aventure (il faut ajouter des |default:"" un peu partout pour s'en sortir).

Mon patch ici propose d'effacer une donnée de traitement si et seulement si le widget a bien été laissé vide, i.e. avec le message "Leaving the field blank will empty the value.". On retire une magie qui m'avait semblé pertinente à la relecture de #44310, mais en fait, non, cette magie n'est pas bienvenue.

(on a déjà eu le même soucis sur des données de traitement devenues "None" au lieu de "", cf #44916)

#11

Mis à jour par Frédéric Péters il y a plus de 3 ans

Mmm, ok, "Mon patch ici propose d'effacer une donnée de traitement si et seulement si le widget a bien été laissé vide", le patch n'était du coup pas bien clair pour moi. (et à le taper dans un terminal c'est pas mal dû à la structure générale, j'ai l'impression).

Reste quand même que,

    item.fields = [{'field_id': 'bo1', 'value': '=""'}]

je préfixerais ça par un commentaire pour dire ce qui est testé.

# check a value computed as the empty string is stored as an empty string, not None

et de là alors également, un test avec :

    item.fields = [{'field_id': 'bo1', 'value': '{{empty}}'}]

qui devrait a priori marcher aussi.

Aussi,

            if field['value'] == '':

je me demande si plutôt pas if not field['value'], voire if not field.get('value'), pour ne pas avoir à s'assurer que jamais ça ne se trouverait être None là-dedans.

#12

Mis à jour par Thomas Noël il y a plus de 3 ans

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

Mmm, ok, "Mon patch ici propose d'effacer une donnée de traitement si et seulement si le widget a bien été laissé vide", le patch n'était du coup pas bien clair pour moi. (et à le taper dans un terminal c'est pas mal dû à la structure générale, j'ai l'impression).

Oui, le patch n'était pas joli.

Reste quand même que,
je préfixerais ça par un commentaire pour dire ce qui est testé.
et de là alors également, un test avec :
qui devrait a priori marcher aussi.

Ajoutés.

Aussi, je me demande si plutôt pas if not field['value'], voire if not field.get('value'), pour ne pas avoir à s'assurer que jamais ça ne se trouverait être None là-dedans.

J'ai mis le if not field.get('value') auquel j'avais pensé initialement aussi. Ca demande une modification d'un test qui a mon avis était assez bidon de toute façon :

     # invalid values => do nothing
     assert LoggedError.count() == 0
-    for value in ('plop', {}, []):
+    for value in ('plop', '={}', '=[]'):

parce qu'avoir {} ou [] directement dans value ça ne me semble juste pas possible... n'est-ce pas ?

#13

Mis à jour par Frédéric Péters il y a plus de 3 ans

parce qu'avoir {} ou [] directement dans value ça ne me semble juste pas possible... n'est-ce pas ?

Quoique je ne prendrais pas ce risque, if field.get('value') in (None, ""): peut-être alors ?

#14

Mis à jour par Frédéric Péters il y a plus de 3 ans

  • Statut changé de Solution proposée à Solution validée

Bon je viens de regarder mieux le contexte et clairement non ça ne peut pas arriver, juste du zèle au niveau du test j'imagine.

#15

Mis à jour par Thomas Noël il y a plus de 3 ans

  • Statut changé de Solution validée à Résolu (à déployer)
commit 19a1ad6ce4b9e0d29a25e62b0e73e2685a69df83
Author: Thomas NOËL <tnoel@entrouvert.com>
Date:   Wed Jul 15 17:30:15 2020 +0200

    workflows: set bo fields to empty value only if value before compute is empty (#45135)

#17

Mis à jour par Thomas Noël il y a plus de 3 ans

hotfix sur v5.24 en cours

#19

Mis à jour par Thomas Noël il y a plus de 3 ans

Thomas Noël a écrit :

hotfix sur v5.24 en cours

hotfix publié.

#20

Mis à jour par Emmanuel Cazenave il y a plus de 3 ans

Merci.

#21

Mis à jour par Frédéric Péters il y a plus de 3 ans

  • Statut changé de Résolu (à déployer) à Solution déployée

Formats disponibles : Atom PDF