datasources: add the template type (#78777) #695

Merged
ecazenave merged 3 commits from wip/78777-template-data-source into main 2023-10-31 14:03:23 +01:00
Owner
No description provided.
ecazenave added 1 commit 2023-09-19 14:54:16 +02:00
gitea/wcs/pipeline/head There was a failure building this commit Details
ae1d1d77f9
datasources: add the template type (#78777)
fpeters reviewed 2023-09-19 15:04:51 +02:00
fpeters left a comment
Owner

À part ça dans ce qu'il était noté dans le ticket, il y avait :

(il y aurait alors également ce filtre |json_dumps à coder)

À part ça dans ce qu'il était noté dans le ticket, il y avait : > (il y aurait alors également ce filtre |json_dumps à coder)
@ -574,0 +600,4 @@
value = Template(data_source.get('value')).render(context=variables)
except TemplateSyntaxError:
get_publisher().record_error(
'Python data source (%r) gave a template syntax error' % data_source.get('value'),
Owner

Pas Python.

Pas Python.
ecazenave force-pushed wip/78777-template-data-source from ae1d1d77f9 to 300273aa55 2023-09-19 15:54:05 +02:00 Compare
ecazenave force-pushed wip/78777-template-data-source from 300273aa55 to 5a2ddba54c 2023-09-19 17:15:57 +02:00 Compare
ecazenave force-pushed wip/78777-template-data-source from 5a2ddba54c to 1406c2db4b 2023-09-19 17:34:07 +02:00 Compare
ecazenave force-pushed wip/78777-template-data-source from 1406c2db4b to bd15f02d5c 2023-09-21 16:23:34 +02:00 Compare
ecazenave added 1 commit 2023-09-21 17:19:46 +02:00
gitea/wcs/pipeline/head This commit looks good Details
7a5806bb5f
misc: add json_dumps filter (#78777)
ecazenave force-pushed wip/78777-template-data-source from 7a5806bb5f to d535e49bfc 2023-09-21 17:54:23 +02:00 Compare
Author
Owner

À part ça dans ce qu'il était noté dans le ticket, il y avait :

Je sais bien c'est pour ça que cette PR était encore en 'wip' quand tu l'as commenté.

A part ça depuis ta relecture j'ai changé les choses : pas de nouveau type de data source, plutôt que celles de type jsonvalue acceptent un gabarit.

L'introduction d'un nouveau type m'a furieusement fait penser à la distinction texte/gabarit qu' on a finit par faire disparaître je ne sais plus où.

Aussi ça me semble plus lisible pour les utilisateurs : les sources de données de type jsonvalue, soit on y écrit directement du json, soit un gabarit qui va donner du json (cas d'usage de Steph), soit un gabarit avec une seule variable qu'on transforme en json via |json_dumps.

Tout ça me semble bien raccord.

> À part ça dans ce qu'il était noté dans le ticket, il y avait : Je sais bien c'est pour ça que cette PR était encore en 'wip' quand tu l'as commenté. A part ça depuis ta relecture j'ai changé les choses : pas de nouveau type de data source, plutôt que celles de type jsonvalue acceptent un gabarit. L'introduction d'un nouveau type m'a furieusement fait penser à la distinction texte/gabarit qu' on a finit par faire disparaître je ne sais plus où. Aussi ça me semble plus lisible pour les utilisateurs : les sources de données de type jsonvalue, soit on y écrit directement du json, soit un gabarit qui va donner du json (cas d'usage de Steph), soit un gabarit avec une seule variable qu'on transforme en json via |json_dumps. Tout ça me semble bien raccord.
ecazenave force-pushed wip/78777-template-data-source from d535e49bfc to 0bb9984aca 2023-09-21 18:24:17 +02:00 Compare
Author
Owner

Il manque encore des choses pour le rafraîchissement automatique si un champ qui précède sur la page change et si le gabarit de la source de donnée fait usage de ce champ.

Je regarde ça, peut-être dans un ticket à part (aucune idée de la complexité).

Il manque encore des choses pour le rafraîchissement automatique si un champ qui précède sur la page change et si le gabarit de la source de donnée fait usage de ce champ. Je regarde ça, peut-être dans un ticket à part (aucune idée de la complexité).
ecazenave changed title from WIP: datasources: add the template type (#78777) to datasources: add the template type (#78777) 2023-09-22 09:58:41 +02:00
ecazenave force-pushed wip/78777-template-data-source from 0bb9984aca to fd507a983f 2023-09-25 17:34:35 +02:00 Compare
ecazenave force-pushed wip/78777-template-data-source from fd507a983f to 3a33d08b53 2023-09-25 18:08:42 +02:00 Compare
Author
Owner

Je regarde ça, peut-être dans un ticket à part (aucune idée de la complexité).

Un commit de plus pour gérer le cas simple : {{% if form_var_toto %}} ...

Par contre pour pour la version {{form_vat_toto_agendas|json_dumps}} ça ne fonctionne pas du tout. Sur ce gabarit Field.get_referenced_varnames ne renvoie rien.

Quand bien même je lui passerais juste {{form_var_toto_agendas}} il me renverrait toto_agendas alors que je voudrais juste toto.

Et j'imagine qu'on veut pas complexifier l'expression régulière avec un postfix spécifique 'agendas' .... bref pas de piste.

> Je regarde ça, peut-être dans un ticket à part (aucune idée de la complexité). Un commit de plus pour gérer le cas simple : `{{% if form_var_toto %}} ...` Par contre pour pour la version `{{form_vat_toto_agendas|json_dumps}}` ça ne fonctionne pas du tout. Sur ce gabarit `Field.get_referenced_varnames` ne renvoie rien. Quand bien même je lui passerais juste `{{form_var_toto_agendas}}` il me renverrait `toto_agendas` alors que je voudrais juste `toto`. Et j'imagine qu'on veut pas complexifier l'expression régulière avec un postfix spécifique 'agendas' .... bref pas de piste.
ecazenave force-pushed wip/78777-template-data-source from 3a33d08b53 to 5cab2eb7ea 2023-09-29 17:38:59 +02:00 Compare
Author
Owner

Par contre pour pour la version {{form_vat_toto_agendas|json_dumps}} ça ne fonctionne pas du tout. Sur ce gabarit Field.get_referenced_varnames ne renvoie rien.

Je me suis aventuré là dedans en commençant par un le patch ci-joint que je met ici pour mémoire. Mais ça ne suffit pas, j'ai passé un moment à creuser ça, je finis à tort ou à raison dans qommon.form.js et là je renonce.

Bilan des courses, avec les commits inclus ici la forme {% if form_var_toto %}} ...écrire du json {% endif %} fonctionne sur une même page et sur deux pages différentes, la forme {form_vat_toto_agendas|json_dumps}} fonctionne sur deux pages différentes mais pas sur une même page.

Et je vais m'arrêter là (en attendant relecture) sur ce ticket.

> Par contre pour pour la version `{{form_vat_toto_agendas|json_dumps}}` ça ne fonctionne pas du tout. Sur ce gabarit `Field.get_referenced_varnames` ne renvoie rien. Je me suis aventuré là dedans en commençant par un le patch ci-joint que je met ici pour mémoire. Mais ça ne suffit pas, j'ai passé un moment à creuser ça, je finis à tort ou à raison dans qommon.form.js et là je renonce. Bilan des courses, avec les commits inclus ici la forme `{% if form_var_toto %}} ...écrire du json {% endif %}` fonctionne sur une même page et sur deux pages différentes, la forme {form_vat_toto_agendas|json_dumps}} fonctionne sur deux pages différentes mais pas sur une même page. Et je vais m'arrêter là (en attendant relecture) sur ce ticket.
Owner

Et je vais m'arrêter là (en attendant relecture) sur ce ticket.

J'ai essayé de faire le lien entre les tests et la description du ticket mais je ne suis pas bien sûr, et le dernier commentaire semble dire que ce qui était imaginé dans le ticket, le {{form_var_foo_agendas|json_dumps}} ne marcherait pas s'il est mis sur la même page que le champ donnant "form_var_foo" ?

Je serais pour avoir dans les tests quelque chose de plus proche du besoin exprimé dans le ticket. Et éventuellement le test en xfail ou dans un commit séparé qui rappellerait qu'il faut faire marcher ça sur une seule page.

> Et je vais m'arrêter là (en attendant relecture) sur ce ticket. J'ai essayé de faire le lien entre les tests et la description du ticket mais je ne suis pas bien sûr, et le dernier commentaire semble dire que ce qui était imaginé dans le ticket, le {{form_var_foo_agendas|json_dumps}} ne marcherait pas s'il est mis sur la même page que le champ donnant "form_var_foo" ? Je serais pour avoir dans les tests quelque chose de plus proche du besoin exprimé dans le ticket. Et éventuellement le test en xfail ou dans un commit séparé qui rappellerait qu'il faut faire marcher ça sur une seule page.
Owner

Et je vais m'arrêter là (en attendant relecture) sur ce ticket.

J'ai essayé de faire le lien entre les tests et la description du ticket mais je ne suis pas bien sûr, et le dernier commentaire semble dire que ce qui était imaginé dans le ticket, le {{form_var_foo_agendas|json_dumps}} ne marcherait pas s'il est mis sur la même page que le champ donnant "form_var_foo" ?

Je serais pour avoir dans les tests quelque chose de plus proche du besoin exprimé dans le ticket. Et éventuellement le test en xfail ou dans un commit séparé qui rappellerait qu'il faut faire marcher ça sur une seule page.

Je relis le diff en fichier attaché et je comprends que la difficulté serait sur la détection de la variable, en fait cette partie est totalement accessoire, et quand j'écris {{form_var_foo_agendas|json_dumps}} il peut tout à fait être lu

{% if form_var_foo %}{{form_var_foo_agendas|json_dumps}}{% else %}[]{% endif %}

(sans trop savoir quelle nécessité au else, c'est évidemment mieux si ça peut marcher sans, qu'un résultat vide soit ok.

(reste que dans les tests je souhaiterais quand même quelque chose s'approchant davantage du besoin originel, c'est-à-dire la combinaison d'un champ avec une source de données allant chercher dans chrono une info et dessous un champ qui alimente sa liste de choix par un extrait de ce qui a été choisi dans le premier).

> > Et je vais m'arrêter là (en attendant relecture) sur ce ticket. > > J'ai essayé de faire le lien entre les tests et la description du ticket mais je ne suis pas bien sûr, et le dernier commentaire semble dire que ce qui était imaginé dans le ticket, le {{form_var_foo_agendas|json_dumps}} ne marcherait pas s'il est mis sur la même page que le champ donnant "form_var_foo" ? > > Je serais pour avoir dans les tests quelque chose de plus proche du besoin exprimé dans le ticket. Et éventuellement le test en xfail ou dans un commit séparé qui rappellerait qu'il faut faire marcher ça sur une seule page. Je relis le diff en fichier attaché et je comprends que la difficulté serait sur la détection de la variable, en fait cette partie est totalement accessoire, et quand j'écris {{form_var_foo_agendas|json_dumps}} il peut tout à fait être lu ``` {% if form_var_foo %}{{form_var_foo_agendas|json_dumps}}{% else %}[]{% endif %} ``` (sans trop savoir quelle nécessité au else, c'est évidemment mieux si ça peut marcher sans, qu'un résultat vide soit ok. (reste que dans les tests je souhaiterais quand même quelque chose s'approchant davantage du besoin originel, c'est-à-dire la combinaison d'un champ avec une source de données allant chercher dans chrono une info et dessous un champ qui alimente sa liste de choix par un extrait de ce qui a été choisi dans le premier).
fpeters requested changes 2023-10-03 12:49:49 +02:00
fpeters left a comment
Owner

(cf demande de test spécifique ci-dessus)

(cf demande de test spécifique ci-dessus)
ecazenave force-pushed wip/78777-template-data-source from 5cab2eb7ea to 598f4ab9d4 2023-10-05 16:07:26 +02:00 Compare
Author
Owner

Voilà avec deux tests qui reprennent le cas d'usage chrono, un premier avec {% if form_var_rdv %}{{ form_var_rdv_agendas|json_dumps}}{% endif %} qui passe.

Un deuxième marqué xfail avec {{ form_var_rdv_agendas|json_dumps}}.

Dans le premier, pour simuler le comportement du js je fais live_resp = app.post('/foo/live?modified_field_id=1', params=resp.form.submit_fields()).

Malheureusement notre js ne passe pas de paramètre modified_field_id dans la requête, donc pour l'instant ça ne marche pas "en vrai" : https://dev.entrouvert.org/issues/82042

Voilà avec deux tests qui reprennent le cas d'usage chrono, un premier avec `{% if form_var_rdv %}{{ form_var_rdv_agendas|json_dumps}}{% endif %}` qui passe. Un deuxième marqué xfail avec `{{ form_var_rdv_agendas|json_dumps}}`. Dans le premier, pour simuler le comportement du js je fais `live_resp = app.post('/foo/live?modified_field_id=1', params=resp.form.submit_fields())`. Malheureusement notre js ne passe pas de paramètre modified_field_id dans la requête, donc pour l'instant ça ne marche pas "en vrai" : https://dev.entrouvert.org/issues/82042
Owner

Un deuxième marqué xfail avec {{ form_var_rdv_agendas|json_dumps}}.

Je serais à dire qu'il pourrait être dégagé, c'est trop annexe et sans tout l'historique/commentaire on ne retrouvera pas pourquoi il a été ajouté.

> Un deuxième marqué xfail avec {{ form_var_rdv_agendas|json_dumps}}. Je serais à dire qu'il pourrait être dégagé, c'est trop annexe et sans tout l'historique/commentaire on ne retrouvera pas pourquoi il a été ajouté.
ecazenave force-pushed wip/78777-template-data-source from 598f4ab9d4 to 9fb1dd12d0 2023-10-31 12:22:31 +01:00 Compare
Author
Owner

Viré.

Viré.
ecazenave requested review from fpeters 2023-10-31 13:20:28 +01:00
fpeters approved these changes 2023-10-31 13:58:31 +01:00
ecazenave force-pushed wip/78777-template-data-source from 9fb1dd12d0 to 941ad44c51 2023-10-31 14:03:05 +01:00 Compare
ecazenave merged commit 941ad44c51 into main 2023-10-31 14:03:23 +01:00
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/wcs#695
No description provided.