Produits Entr'ouvert » w.c.s.

J'ai compris les tests, un peu moins le reste :)
C'est ok pour moi, mais si une autre personne veut bien relire et valider je préfère.
Si personne ne s'y colle je validerai :)

Je vais faire une passe aussi (dans la journée) [edit : et non, je n'ai pas encore trouvé le temps...]

J'ai un problème avec le save de ComputedField où on voit :

        if not self.varname:
            self.varname = misc.simplify(self.label, space='_')

la création d'identifiant en doublon me semble être un truc qui va se produire souvent. Mais je vois aussi qu'on a un required=True dans le form... et je me demande donc ce morceau de code dans le save() n'est pas juste là "pour se rassurer" ...?

Sur :

value = WorkflowStatusItem.compute(field.value_template, raises=True, allow_complex=True)

on va stocker "n'importe quoi" dans un champ calculé. C'est bien, c'est souple, mais peut-être trop : je m'interroge sur la compréhension par nos utilisateurs. Et m'inquiète un peu des objets bizarres qu'on pourrait se retrouver stocker, genre des résultats de requêtes...

Idéalement j'aurai préféré que dans ComputedField on demande de défniir un "target_type" parmi string/text/bool/file/date. Et après le compute on ferait un convert_value_from_anything

Bien sûr ça limiterait peut-être quelques usages, mais la magie et le support s'opposent.

Pour le reste, ça m'a l'air pas mal. Je dois cependant essayer de comprendre encore comme ça s'imbrique avec la notion de "live" (si tu l'as envisagé ou pas)

la création d'identifiant en doublon me semble être un truc qui va se produire souvent

Parce que souvent les gens vont mettre le même libellé ? (Il n'y a de toute façon pas d'interdiction d'identifiant en doublon, de manière générale sur les champs.)

Ça rejoint la demande de taper automatiquement un identifiant sur les champs (#41324) mais comme ici le nom de variable est obligatoire, c'est nécessaire.

Ta préférence serait à ce qu'il y ait regard sur les champs existants pour suffixer d'un compteur si jamais l'identifiant se trouvait utilisé ?

_~~

Idéalement j'aurai préféré que dans ComputedField on demande de défniir un "target_type"

J'imaginais pas exemple qu'un champ pourrait avoir comme contenu le résultat d'un appel web, qui ne rentrerait pas dans ces cases.

S'il fallait limiter je serais pour limiter aux chaines, sans notion de type de données.

_~~

Pour le reste, ça m'a l'air pas mal. Je dois cependant essayer de comprendre encore comme ça s'imbrique avec la notion de "live" (si tu l'as envisagé ou pas)

Non pour le moment l'évaluation est bornée à l'affichage et submit de page; j'imagine possible d'étendre ça pour également faire ça début/fin d'évaluation live mais je rangeais ça pour plus tard.

Non pour le moment l'évaluation est bornée à l'affichage et submit de page; j'imagine possible d'étendre ça pour également faire ça début/fin d'évaluation live mais je rangeais ça pour plus tard.

En fait j'écrivais ça sans trop réfléchir; en réfléchissant un peu j'arrive à me dire qu'il n'y a rien à faire, que le live se fait sur l'évaluation de la page et aura les champs et que ceux-ci ne changeront pas, mais que j'ai peut-être oublié de justement mettre à disposition ces variables lors de l'évaluation du live. (je vais ajouter un test pour vérifier/corriger).

À un moment j'ai été à me dire qu'il pourrait y avoir d'une manière ou d'une autre recalcul et qu'une valeur changerait et que ça serait utile de l'avoir mais sans arriver à continuer l'idée vers une situation concrète, donc je rangerais vraiment cette part de côté.

(je vais ajouter un test pour vérifier/corriger).

Voilà j'ai ajouté test_computed_field_usage_in_live_data qui vérifie qu'un commentaire dont le contenu est réévalué par le live a bien la valeur du champ calculé, et ça a marché sans modif.

Reste un petit détail, dans la liste de choix du type de champ, le "Computed Data" apparait dans la section titre/sous-titre/commentaire/page (cf copie d'écran jointe). C'est voulu ? J'en ferais plutôt une section à part, comme pour les blocs.

Oui c'était voulu mais j'avais noté ça dans ma liste de points à reprendre en questions et j'ai oublié; je viens de pousser une branche qui l'isole.

@@ -3459,6 +3459,8 @@ def get_field_types():
 def get_field_options(blacklisted_types):
     widgets, non_widgets = [], []
     for klass in field_classes:
+        if klass is ComputedField:
+            continue
         if klass.key in blacklisted_types:
             continue
         if issubclass(klass, WidgetField):
@@ -3466,6 +3468,10 @@ def get_field_options(blacklisted_types):
         else:
             non_widgets.append((klass.key, _(klass.description), klass.key))
     options = widgets + [('', '—', '')] + non_widgets
+
+    # add computed field in its own "section" 
+    options.extend([('', '—', ''), (ComputedField.key, _(ComputedField.description), ComputedField.key)])
+
     if get_publisher().has_site_option('fields-blocks') and (
         not blacklisted_types or 'blocks' not in blacklisted_types
     ):

Suite de mes tests, en pré-visualisation d'un formulaire avec un champ calculé j'ai ce message : « Erreur à l’affichage en prévisualisation du champ. ('NoneType' object is not callable) » parce que « add_to_form = None » pour un ComputedField.

Et donc peut-être faire :

--- a/wcs/admin/forms.py
+++ b/wcs/admin/forms.py
@@ -1141,7 +1141,7 @@ class FormDefPage(Directory):
         on_page = 0
         for i, field in enumerate(self.formdef.fields):
             field.id = i
-            if hasattr(field, str('add_to_form')):
+            if getattr(field, str('add_to_form'), None):
                 try:
                     field.add_to_form(form)
                 except Exception as e:

if getattr(field, str('add_to_form'), None): et nettoyer ça pour le str() devenu inutile il y a longtemps.

Branche poussée ainsi. Mais ça peut ouvrir la question de se demander si ces champs devraient être d'une manière particulière quand même affiché dans cette "prévisualisation", là j'ai pris l'option de non rien afficher.