Produits Entr'ouvert » w.c.s.

Voici un test qui permet de reproduire l'erreur.
Il reprend test_field_live_select_content afin de tester #27173 non plus sur le formulaire principale, mais sur un second formulaire affiché au cours du workflow.

def test_field_live_select_content_on_display_form(pub):
    create_user(pub)    
    wf = Workflow(name='wf-title')
    st1 = wf.add_status('Status1', 'st1')

    data_from_data_source = [{'id': '1', 'text': 'un'}, {'id': '2', 'text': 'deux'}]

    # form displayed into workflow
    display_form = FormWorkflowStatusItem()
    display_form.id = '_x'
    display_form.by = ['_submitter']
    display_form.varname = 'xxx'
    display_form.formdef = WorkflowFormFieldsFormDef(item=display_form)
    display_form.formdef.fields = [
        fields.StringField(type='string', id='1', label='Foo', varname='foo'),
        fields.ItemField(type='item', id='2', label='Bar', varname='bar',
                         data_source={
                             'type': 'json',
                             'value': "https://api.example.com/json?foo=[xxx_var_foo]"}),
      ]

    st1.items.append(display_form)
    display_form.parent = st1
    wf.store()

    # initial empty 'test' form
    formdef = create_formdef()
    formdef.fields = []
    formdef.confirmation = False
    formdef.workflow_id = wf.id
    formdef.store()
    formdef.data_class().wipe()

    # submit initial empty form
    app = get_app(pub)
    resp = login(app, username='foo', password='foo').get('/test/')
    resp = resp.form.submit('submit').follow()

    # provide a value for 'f1' field
    assert resp.html.find('div', {'data-field-id': '1'}).attrs['data-live-source'] == 'true'
    assert resp.html.find('div', {'data-field-id': '2'}).find('select')
    resp.form['f1'].value = 'foo'
    live_resp = app.post('/test/1/live?modified_field_id=1', params=resp.form.submit_fields())
    assert live_resp.json['result']['1']['visible']
    assert live_resp.json['result']['2']['visible']

    # simulate javascript filling the <select>
    resp.form['f2'].options = []
    for item in data_from_data_source:
        resp.form['f2'].options.append((item['id'], False, item['text']))
    resp.form['f2'] = '2'

    # submit the form
    resp = resp.form.submit('submit')  # <- hang here ...
    # ... fails to retrieve the simulate javascript filled options for the select

    resp = resp.follow()
    assert 'The form has been recorded' in resp.body

Le test échoue dans quixote/form/widget.py::SelectWidget:_parse_single_selection() ...

for value, description, key in self.options:
    ...
else:
    if self.verify_selection:
        self.error = self.SELECTION_ERROR
        return default

(Pdb) self.options
[(None, '---', '')]

également ici : #35874

Pour les ItemField le contenu de la datasource est mise en cache dès le premier appel à ItemField.get_options() dans self._cached_data_source, je suppute un lien mais je vais lire le code de FormWorkflowStatusItem pour en être sûr...
Pas le bon code.

Donc non, ce serait plutôt dans wcs/forms/common.py:FormStatusPage au niveau de get_workflow_form() :

    def get_workflow_form(self, user):
        submitted_fields = []
        form = self.filled.get_workflow_form(user, displayed_fields=submitted_fields)
        if form:
            form.attrs['data-live-url'] = self.filled.get_url() + 'live'
        if form and form.is_submitted():
            with get_publisher().substitutions.temporary_feed(self.filled, force_mode='lazy'):
                # remove fields that could be required but are not visible
                self.filled.evaluate_live_workflow_form(user, form)
                get_publisher().substitutions.feed(self.filled)
                for field in submitted_fields:
                    if not field.is_visible(self.filled.data, self.formdef) and 'f%s' % field.id in form._names:
                        del form._names['f%s' % field.id]
        return form

on voit bien le feed des données actuelle du formulaire mais pas le feed des données qui viennent d'être soumises comme je pense on doit l'avoir lors de la soumission d'un formulaire "classique" avec les histoires de get_transient_formdata, comme là :

            submitted_fields = []
            transient_formdata = self.get_transient_formdata()
            with get_publisher().substitutions.temporary_feed(
                    transient_formdata, force_mode='lazy'):
                form = self.create_form(page=page,
                        displayed_fields=submitted_fields,
                        transient_formdata=transient_formdata)

Voilà je pense que c'est cerné, Nicolas je pense qu'il te faudra la science de Fred pour avancer plus loin.

Mauvaise analyse, les données sont bien ajoutées aux substitutions par :

                self.filled.evaluate_live_workflow_form(user, form)

mais comme on est dans le contexte d'un temporary_feed c'est défait, et donc quand on arrive là :

279     def check_submitted_form(self, form):
280         if form and form.is_submitted() and not form.has_errors():
281             url = self.submit(form)

le form.has_errors() renvoie True puisqu'on a plus les valeurs, il faudrait ajouter le même code dans check_submitted_form, comme ceci :

     def check_submitted_form(self, form):
         if form and form.is_submitted():
             with get_publisher().substitutions.temporary_feed(self.filled, force_mode='lazy'):
                 # remove fields that could be required but are not visible
                 self.filled.evaluate_live_workflow_form(user, form)
                 get_publisher().substitutions.feed(self.filled)
                 for field in submitted_fields:
                     if not field.is_visible(self.filled.data, self.formdef) and 'f%s' % field.id in form._names:
                         del form._names['f%s' % field.id]

                 if not form.has_errors():
                     url = self.submit(form)
                     if url is None:
                         url = get_request().get_frontoffice_url()
                     response = get_response()
                     response.set_status(303)
                     response.headers[str('location')] = url
                     response.content_type = 'text/plain'
                     return "Your browser should redirect you" 

ou alors refactoriser un peu tout ce code la pour limiter la duplication.

Ma fois je n'y comprends rien du tout, t'as juste sorti le code dans le with, hors du with, je suppose que s'il y a un contexte il doit y avoir une raison.

Je vais passer la main pour la relecture, ça ne ressemble pas à ce que je pensais être la correction et je n'ai pas particulièrement d'idée sur ce qui serait juste. Juste renommer submitted_fields en display_fields me semble gratuit.

C'est cinq itérations sur un même patch, attache un seul patch.

Et les affaires sont suffisamment compliquées comme ça, comme écrit Benjamin, ne viens pas mêler des changements cosmétiques.

Nicolas Roche a écrit :

oui, j'en conviens, désolé.
(mauvais patch ajouté ci-dessus)

À mon avis le commentaire de Fred sur #36728 s'applique aussi :

Commentaire sur le patch vu dans la branche ça simplifie pas du tout ça modifie tout le comportement de w.c.s. qui suit le bout de code, sur des situations ultra-particulières qui ne se produisent sans doute pas dans les tests. (genre des combinaisons de pages conditionnelles, utilisant du python, avec des champs conditionnels et du django et des sources de données elles-mêmes utilisant l'un ou l'autre, etc.).

Dans le code suivant :

    def get_workflow_form(self, user):
        form, submitted_fields = self.evaluate_workflow_form(user)
        if form:
            form.attrs['data-live-url'] = self.filled.get_url() + 'live'
        if form and form.is_submitted():
            with get_publisher().substitutions.temporary_feed(self.filled, force_mode='lazy'):
                # remove fields that could be required but are not visible
                self.filled.evaluate_live_workflow_form(user, form)
                get_publisher().substitutions.feed(self.filled)
                for field in submitted_fields:
                    if not field.is_visible(self.filled.data, self.formdef) and 'f%s' % field.id in form._names:
                        del form._names['f%s' % field.id]
        return form

le with protège le code qui vient après d'être perturbé par ces deux lignes qui modifie le contexte des variables de substitutions :

                self.filled.evaluate_live_workflow_form(user, form)
                get_publisher().substitutions.feed(self.filled)

+        self.filled.evaluate_live_workflow_form(user, form)
+        get_publisher().substitutions.unfeed(lambda x: x is self.filled)
+        get_publisher().substitutions.feed(self.filled)

Le problème n'est même pas là, il est sur ces lignes :

289     def check_submitted_form(self, form):
290         if form and form.is_submitted() and not form.has_errors():

c'est dans form.has_errors() que quixote/form/widget.py::SelectWidget::_parse_single_selection() dont tu parles plus haut est appelé, c'est autour de cette ligne qu'il faut changer des choses.

En fait c'est vraiment super d'avoir le test pour reproduire le problème et ça va vraiment aider mais la correction en elle-même, parce qu'elle doit prendre en compte des dangers invisibles, je pense maintenant que ça doit être pour ma pomme.

Merci Fred !
(Je regarde si je peux pas fournir un second test).
Au cas où un dernier petit laïus sur le sujet.

---8<---
Oui, j'ai bien has_error() à True, mais je ne vois pas comment l'exploiter.
Voici la pile des appels :

...
  /home/nroche/src/wcs/wcs/forms/common.py(242)_q_index()
-> form = self.get_workflow_form(user)
  /home/nroche/src/wcs/wcs/forms/common.py(271)get_workflow_form()
-> self.filled.evaluate_live_workflow_form(user, form)
  /home/nroche/src/wcs/wcs/formdata.py(576)evaluate_live_workflow_form()
-> wf_status.evaluate_live_form(form, self, user)
  /home/nroche/src/wcs/wcs/workflows.py(1467)evaluate_live_form()
-> item.evaluate_live_form(form, filled, user)
  /home/nroche/src/wcs/wcs/wf/form.py(199)evaluate_live_form()
-> self.formdef.fields, self.formdef.get_data(form),
  /home/nroche/src/wcs/wcs/formdef.py(710)get_data()
-> d.update(self.get_field_data(field, widget))
  /home/nroche/src/wcs/wcs/formdef.py(686)get_field_data()
-> d[field.id] = widget.parse()
  /home/nroche/envs/publik-env/local/lib/python2.7/site-packages/quixote/form/widget.py(125)parse()
-> self._parse(request)
  /home/nroche/envs/publik-env/local/lib/python2.7/site-packages/quixote/form/widget.py(466)_parse()
-> self.value = self._parse_single_selection(parsed_key)
> /home/nroche/envs/publik-env/local/lib/python2.7/site-packages/quixote/form/widget.py(413)_parse_single_selection()
-> return default

Par exemple, dans get_data j'ai tenté de positionner widget.verify_selection à False sur les liste afin d'éviter l'erreur. Cela permet de valider le formulaire (de passer sans encombre dans check_submitted_form comme Benjamin le propose dans sa seconde analyse), mais ensuite on ne récupère pas les bonnes valeurs.

formdef.py:

    def get_data(self, form):
        d = {}
        for field in self.fields:
            widget = form.get_widget('f%s' % field.id)
            if widget:
                d.update(self.get_field_data(field, widget))
        return d

Au début je pensais intervenir sur evaluate_live_form car c'est ici que l'on observe que la valeur du camps 1 (qui permet de récupérer les options du select du champ 2) est restée la même que celle fourni par le précédent formulaire validé (formulaire de démarche ou formulaire de workflow validé précédemment). Mais je n'ai pas réussi :

wf/form.py:

    def evaluate_live_form(self, form, formdata, user):
++      print "avant: %s" % formdata.workflow_data
        workflow_data = {}
        for k, v in get_dict_with_varnames(
                        self.formdef.fields, self.formdef.get_data(form),
                        varnames_only=True).items():
            workflow_data['%s_%s' % (self.varname, k)] = v
        formdata.update_workflow_data(workflow_data)
++      print "apres: %s" % formdata.workflow_data

A vrai dire le test fourni ne reproduit pas cela (il me faudrait le rajouter à la fin du test test_backoffice_workflow_form_with_live_data_source de #35363). Le test fourni permet cependant de montrer que les variables de workflow sont mises à jour à ce moment.

avant: None
apres: {'xxx_var_bar2': 'plop', 'xxx_var_foo_raw': None, 'xxx_var_foo': None, 'xxx_var_bar': 'hello'}

Or les options du select sont récupérées en utilisant la variable de workflow du champ 1 par la méthode formdata.py::FormData::get_workflow_form() appellée (ici) avant l'instruction qui met à jour cette variable (là) :

common.py:

   def get_workflow_form(self, user):
        submitted_fields = []
        form = self.filled.get_workflow_form(user, displayed_fields=submitted_fields)  # <- *ici*
        if form:
            form.attrs['data-live-url'] = self.filled.get_url() + 'live'
        if form and form.is_submitted():
            with get_publisher().substitutions.temporary_feed(self.filled, force_mode='lazy'):
                # remove fields that could be required but are not visible
                self.filled.evaluate_live_workflow_form(user, form)  # <- *là*
        ...

Le test reproduit la même erreur ('valeur choisie invalide') parce que le champs 1 n'est pas encore valué dans formdata lors de la récupération des options du select.

Pour moi se serait plutôt à ce moment ici qu'il faudrait réévaluer les variables de workflow (ou inversement là, ne pas les valider tant que l'on ne les a pas évaluées). Ce qui est fait par #34564 (reevaluate workflow form according to possible new content) en faisant deux passes : la première passe est fausse (et l'erreur est ignorée) mais les 2 passes ont le mérite de rester cohérentes.

Y a ça qui est perdu au passage quand on recrée le form il me semble (surtout en cas d'erreur de validation, si la soumission passe c'est moins grave):

        if form:
            form.attrs['data-live-url'] = self.filled.get_url() + 'live'

idem, il y a aussi

submitted_fields = []

Sérieux, on peut ne pas tous travailler sur la même chose ?

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

Sérieux, on peut ne pas tous travailler sur la même chose ?

Je signale juste que la solution apportée ne marche pas et je me demande si ça marche en front et comment; je trouve bien qu'on soit plusieurs à comprendre, ma branche n'apporte pas la solution, elle montre les problèmes qui restent.

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

ma branche n'apporte pas la solution

Je modifie le ticket pour refléter ça, et confirmer que je m'occupe de ce ticket, merci.

Ah oui pardon, j'ai fait un git sub j'ai oublié de dire non à la question de prendre le ticket, pardon, j'ai supprimé ma branche aussi pour ne pas polluer.

Le client demande si ce sera corrigé d'ici fin octobre.

Si le cas précis correspond au patch, ça peut aller, modulo modification pour reprendre l'attribut data-live-url; ça dépend de l'acceptation d'un patch qui se limiterait à ça. S'il s'agit d'un changement plus large, je n'ai pas regardé.

De mon coté je vais ouvrir un autre ticket et rebaser ma branche avec le problème à long terme, j'essayerai de voir si ça affecte le front aussi.