fix: réparer l'utilisation des saisies dans un cvt multi étapes avec `#GENERER_SAISIES` #215

questionnement sur le cas qui disparait mais sinon ça a l'air bon

je me demande aussi pourquoi on gère les deux saisies_par_etapes ET _saisies_par_etapes avec et sans _ à chaque fois, pourquoi ya les deux de possibles ?

je me demande aussi pourquoi on gère les deux saisies_par_etapes ET _saisies_par_etapes avec et sans _ à chaque fois, pourquoi ya les deux de possibles ?

Je pense que c'est un facilité que l'on fournit, mais on pourrait décider de ne plus la fournir, car effectivement

Cela alourdit
C'est a priori piégeux, car on peut avoir des échappement indésirables sur les guillemets.

Je pense que c'est un facilité que l'on fournit, mais on pourrait décider de ne plus la fournir, car effectivement

Que l'on fournit à qui ? :)

Car à priori c'est un tableau d'usage interne, càd que c'est nous-mêmes qui le générons à une étape précédente du CVT, pour l'utiliser un peu plus loin dans le temps. Du coup vu que c'est nous qui le générons, bah on contrôle tout, ya donc pas à gérer plusieurs possibilités, il me semble. Donc oui toujours avec underscore pour être sûr que ce soit pas modifié (de toute façon seules les vraies valeurs des champs devraient être sans underscore, et toute autre variable doit l'avoir, c'est la règle normalement).

ok (et faudra sûrement virer "saisies_par_etapes" sans underscore plus tard donc)

Que l'on fournit à qui ? :)

Tu pointe juste. En fait je pense que j'avais mis cela dans le ticket fae06ec197 suite à une discussion qu'on aurait eu. Mais je ne me rappelle plus exactement.

Bon après ce ne me parait pas rhédibitoire. Donc je pense que l'on peut laisser ca là, quitte à se dire qu'on cassera cela plus tard ? Et surtout on documente que la seule fonctionnalité vraiment supporté c'est via formulaires_xxx_saisies et qu'après saisies s'occupe de tout ?

Heu @maieul c'est quoi ce force push avec des morceaux de var_dump dedans ? ^^

Je te laisse remettre au propre et merger si ça vous convient bien à tous les deux :)

Le force, c'est parce que j'avais envoyé une erreur sur mon commit, le var_dump c'est parce que j'étais fatigué.

Bref c'est corrigé. J'attend les retours de @rastapopoulos sur ce nouveau commit eb210ad

@maieul oui ça a l'air ok, après si c'est dans la même PR, il faudrait donc faire en parallèle le OR que je mettais ici : #215

On en est où sur cette PR ? Le patch original fixe le bug cité en objet mais il semble qu'on est bloqué sur le merge car un autre patch s'y est glissé...

@rastapopoulos je suis pas sur de comprendre à quoi tu pense exactement. Précisement mon commit supplémentaire visait à éviter de faire ce or pour pouvoir chercher _saisies directement via saisies_chercher_formulaire()

@b_b pas tout à fait, le contenu supplémentaire que j'ai ajouté vise notamment à éviter une potentiel rupture de compat liée tes modifs.

@b_b pas tout à fait, le contenu supplémentaire que j'ai ajouté vise notamment à éviter une potentiel rupture de compat liée tes modifs.

Ok, je vous laisse voir quand ça sera bon pour le merge alors :)

Des nouvelles ?

Pour moi c'est bon, mais j'attend une conformation definitive de @rastapopoulos

Oui ça a l'air ok.

Juste pour ta remarque "que dans les pipelines", non chercher_formulaire est aussi appelée ailleurs parfois :)
https://git.spip.net/spip-contrib-extensions/profils/src/branch/master/inc/profils.php#L155

Voilà ca été fusionné. Pour la release, je fais un tour des PR / tickets en attente, et s'il y a des choses simples, je les propose, sinon on releasera rapidement.

At last \o/

Reste la question d'un éventuel report dans la branche 3, je ne suis pas certain, vous en pensez quoi ?

Je pense que tant que la branche SPIP est encore officiellement maintenu, les reports de fix, s'ils ne sont pas trop galère, sont les bienvenus.

'k, je cherry-pick c970a2546c dans la branche v3 alors :)

le suivant aussi (eb210ad1d9), sinon tu risque de casser des choses :)

Je viens de le faire, sauf que $saisies = $retour_charger['_saisies'] ?? []; va poser problème en 3.2 (compat PHP 5.6).

J'ai donc adapté avec https://git.spip.net/spip-contrib-extensions/saisies/commit/fadaaab0

top merci. Pour moi c'est bon.

Et bien cette PR fusionnée pète tous les SPIP, 3 et 4, qui ont mis à jour Saisies : cf les multiples retours un peu partout, discuter et com contrib. 😢

La cause est cette nouvelle inclusion des fonctions charger() sur TOUS les formulaires (même ceux qui n'ont pas de fonctions saisies donc), "si jamais" ya "_saisies" dans le retour, et cela dans verifier() donc à un moment où ya pas forcément les mêmes choses chargées que l'appel classique des charger() par la balise FORMULAIRE.

ET à un mauvais code pas assez robuste du core et des plugins-dist, qui utilise test_formulaire_inclus_par_modele() SANS s'assurer que le fichier externe où elle se trouve est bien chargée. Rappel : quand on utilise une fonction qui est dans un autre fichier ailleurs dans SPIP on doit TOUJOURS charger le fichier car ya aucune assurance qu'il soit déjà là (sauf utils ok).

La solution rapide est de charger le PHP de "balise/formulaire_" dans Saisies et report.
La vraie solution c'est que le core et plugins-dist charge ce fichier eux-mêmes au moment d'utiliser une fonction qui est dedans !

Moi j'ai juste lu et commenté le code avec ce qui me sautait au yeux, mais je me demande du coup comment ça a pu marcher pour ceux qui ont testé la PR dans un site. Comment t'as pas pu avoir aussi ces Fatal error @maieul que rapportent tout le monde ?

Bah parce que cela ne se produit pas dans tout les formulaires : je l'ai sur les sites, mais pas sur les articles par ex.

Certains disent qu'ils l'ont dans la modif d'article et d'autres encore dans la modif de documents (en SPIP 3.2 ceux là)

Bonjour, je viens de mettre mes sites à jour et je rencontre le même problème.
Je le constate sur la modification des rubriques, articles, événements et documents. Testé sous SPIP 3.2.16.

Vu de loin cet appel systématique à charger(), pour tous les formulaires, depuis une fonction elle même appelée depuis le pipeline verifier() est une très mauvaise idée, qui potentiellement casse des workflow de formulaires

Conventionnellement charger() est appelé avant l'affichage du formulaire, pour préparer les valeurs qui seront affichées, et personne ne s'attend à ce qu'il soit sauvagement appelé pendant le verifier() et avant le traiter().

on y fait parfois un set_request('truc') ou d'autres trucs qui vont poser problème et créer des bugs. Que saisies fasse de la bouillie avec les formulaires qui utilisent saisies passe encore, mais que le plugin te foire le workflow de tous les formulaires qui ont rien demandé c'est fort de café.

Il faut revert cet appel. (Quitte à être strict et que seuls les formulaires qui déclarent une fonction _saisie soient pris en charge si c'est ça l'enjeu)

Quitte à être strict et que seuls les formulaires qui déclarent une fonction _saisie soient pris en charge si c'est ça l'enjeu

c'est effectivement cela l'enjeu. Ou plus exactement l'enjeu était : ne pas casser avec le commit de b_b les quelques formulaires qui remplissent _saisies a la place de passer par la fonction formulaire_saisie.

Mais c'est sans doute un cas marginale. On peut décider effectivement de revenir dessus, en se disant que l'enjeu est plutot faible.

Moi je suis plutôt pour que seuls les formulaires qui ont une fonction saisies() soient reconnus.

Après si des gens veulent intégrer les liste de saisies différentes et cela à plusieurs endroits différents d'un même formulaire, càd plusieurs #GENERER_SAISIES différents, bah là faudra trouver une autre solution (cas possible suivant le cas de @b_b : aller chercher les champs extras de l'objet A pour l'insérer à tel endroit du form, et les champs extras de l'objet B à un autre endroit du même form).

@rastapopoulos alors on reverte le commit (ca me va) en assumant une potentielle micro rupture de compat ? J'ai besoin d'un oui clair et écrit, parce que j'ai pas envie de me faire engueuler à nouveau après coup.

si des gens veulent intégrer les liste de saisies différentes et cela à plusieurs endroits différents d'un même formulaire, càd plusieurs #GENERER_SAISIES différents

Pour référence c'est le cas dans une des branches de dev de la fabrique (celle qui génère des formulaires avec saisies php).
Pas par choix, obligé de faire comme ça pour l'instant.

Bon après c'est peut-être l'occasion de le refaire tout en saisies php, sans jquery ui et tout.

Pour revenir sur saisies, si gérer plutôt #GENERER_SAISIES dans un même formulaire se révèle trop compliqué et lourd à gérer, ça me semblerait pas innoportun d'arrêter ce support.

 		} elseif (isset($env['_saisies_par_etapes'])) {
 			$contexte['_saisies'] = $env['_saisies_par_etapes'];
 		} else {
 			$contexte['_saisies'] = isset($env['saisies']) ? saisies_lister_par_etapes($env['saisies']) : array();

 	}
 	$saisies_par_etapes = pipeline('saisies_afficher_si_saisies', $saisies_par_etapes);
 	if ($etape = _request('_etape')) {
 	if ($etape = _request('_etape') and (_request('saisies_par_etapes') or _request('_saisies_par_etapes'))) {

fix: réparer l'utilisation des saisies dans un cvt multi étapes avec `#GENERER_SAISIES` #215

Reviewers