Manque de données sur certains post de bigup (disparition les liaisons de document) #4861

Closed
opened 1 month ago by marcimat · 14 comments
marcimat commented 1 month ago
Owner

En relation avec spip/medias#4872

Lorsqu’on change un document dans l’espace privé, qui est lié à plusieurs articles ou rubriques, seul 1 lien est conservé.

Il semble que le problème vienne des appels à bigup.getFormData().

https://git.spip.net/spip/bigup/src/branch/master/javascript/bigup.js#L812

Cette fonction ne prends pas en compte le cas (subtil ?) d’avoir plusieurs hiddens avec un name multiple, sans l’attribut multiple

La fonction voit bien qu’il y a N inputs

  • <input type="hidden" name="parents[]" value="article|1">
  • <input type="hidden" name="parents[]" value="rubrique|1">

Mais au final ne mettra qu’un élément dans le tableau de sortie.

À comparer peut être avec l’évolution de leur fonction chez dropzone.js f50d1828ab/src/dropzone.js (L1487) qui s’appuie sur FormData().append()

En relation avec https://git.spip.net/spip/medias/issues/4872 Lorsqu’on change un document dans l’espace privé, qui est lié à plusieurs articles ou rubriques, seul 1 lien est conservé. Il semble que le problème vienne des appels à `bigup.getFormData()`. https://git.spip.net/spip/bigup/src/branch/master/javascript/bigup.js#L812 Cette fonction ne prends pas en compte le cas (subtil ?) d’avoir plusieurs hiddens avec un name multiple, sans l’attribut `multiple` La fonction voit bien qu’il y a N inputs - `<input type="hidden" name="parents[]" value="article|1">` - `<input type="hidden" name="parents[]" value="rubrique|1">` Mais au final ne mettra qu’un élément dans le tableau de sortie. À comparer peut être avec l’évolution de leur fonction chez dropzone.js https://github.com/dropzone/dropzone/blob/f50d1828ab5df79a76be00d1306cc320e39a27f4/src/dropzone.js#L1487 qui s’appuie sur FormData().append()
Owner

on pourrait pas utiliser la fonction serialize de jQuery.forms qu'on embarque déjà et qui fait le job de façon robuste ? (mais je sais pas si elle fournit au même format les données du form...)

on pourrait pas utiliser la fonction serialize de jQuery.forms qu'on embarque déjà et qui fait le job de façon robuste ? (mais je sais pas si elle fournit au même format les données du form...)
Poster
Owner

Alors


	getFormData2: function() {
		let formData = new FormData();
		let me = this.form[0];

		for (let input of me.querySelectorAll(
			"input, textarea, select, button"
		)) {
			let inputName = input.getAttribute("name");
			let inputType = input.getAttribute("type");
			if (inputType) inputType = inputType.toLowerCase();

			// If the input doesn't have a name, we can't use it.
			if (typeof inputName === "undefined" || inputName === null) continue;

			if (input.tagName === "SELECT" && input.hasAttribute("multiple")) {
				// Possibly multiple values
				for (let option of input.options) {
					if (option.selected) {
						formData.append(inputName, option.value);
					}
				}
			} else if (
				!inputType ||
				(
					inputType !== "checkbox"
					&& inputType !== "radio"
					&& inputType !== "file"
					&& inputType !== "submit"
				) ||
				input.checked
			) {
				formData.append(inputName, input.value);
			}
		}

		return formData;
	},

Permet de remplir un FormData() effectivement, et le .append() crée bien 2 entrées pour l’input.hidden parents[]

Cela dit forcément ensuite ça ne s’utilise pas pareil qu’un bête tableau tel qu’il y avait…

Notamment on ne peut plus faire

var data = $.extend(bigup.getFormData(), {
	joindre_upload: true,
	joindre_zip: true, // les zips sont conservés zippés systématiquement.
	formulaire_action_verifier_json: true,
	bigup_reinjecter_uniquement: [description.bigup.identifiant],
});

Mais ça devrait devenir qqc comme

let data = bigup.getFormData2();
data.set('joindre_upload', true);
data.set('joindre_zip', true);
data.set('formulaire_action_verifier_json', true);
data.set('bigup_reinjecter_uniquement', [description.bigup.identifiant]);

Sauf que ce faisant on obtient, après l’upload a priori réussi du fichier

Erreur de transfert. : TypeError: 'append' called on an object that does not implement interface FormData.

Que je ne vois pas d’où ça vient

Alors ```js getFormData2: function() { let formData = new FormData(); let me = this.form[0]; for (let input of me.querySelectorAll( "input, textarea, select, button" )) { let inputName = input.getAttribute("name"); let inputType = input.getAttribute("type"); if (inputType) inputType = inputType.toLowerCase(); // If the input doesn't have a name, we can't use it. if (typeof inputName === "undefined" || inputName === null) continue; if (input.tagName === "SELECT" && input.hasAttribute("multiple")) { // Possibly multiple values for (let option of input.options) { if (option.selected) { formData.append(inputName, option.value); } } } else if ( !inputType || ( inputType !== "checkbox" && inputType !== "radio" && inputType !== "file" && inputType !== "submit" ) || input.checked ) { formData.append(inputName, input.value); } } return formData; }, ``` Permet de remplir un FormData() effectivement, et le .append() crée bien 2 entrées pour l’input.hidden `parents[]` Cela dit forcément ensuite ça ne s’utilise pas pareil qu’un bête tableau tel qu’il y avait… Notamment on ne peut plus faire ```js var data = $.extend(bigup.getFormData(), { joindre_upload: true, joindre_zip: true, // les zips sont conservés zippés systématiquement. formulaire_action_verifier_json: true, bigup_reinjecter_uniquement: [description.bigup.identifiant], }); ``` Mais ça devrait devenir qqc comme ```js let data = bigup.getFormData2(); data.set('joindre_upload', true); data.set('joindre_zip', true); data.set('formulaire_action_verifier_json', true); data.set('bigup_reinjecter_uniquement', [description.bigup.identifiant]); ``` Sauf que ce faisant on obtient, après l’upload a priori réussi du fichier ``` Erreur de transfert. : TypeError: 'append' called on an object that does not implement interface FormData. ``` Que je ne vois pas d’où ça vient
Poster
Owner

Alors pour le 'append' c’est qu’il faut passer une option à jQuery.ajax pour dire de pas toucher au formData… et on peut pas l’envoyer depuis jQuery.post, donc il faut le réécrire avec.

Ce qui donne ce diff, pour l’un des JS.

git diff -w                                                                                                                                                git:(master|✚2 
diff --git a/javascript/bigup.documents_edit.js b/javascript/bigup.documents_edit.js
index 7849e39..49a44f8 100644
--- a/javascript/bigup.documents_edit.js
+++ b/javascript/bigup.documents_edit.js
@@ -12,26 +12,42 @@ function formulaires_documents_edit_avec_bigup () {
                        var bigup = file.bigup;
                        var input = file.emplacement;
 
-                       var data = $.extend(bigup.getFormData(), {
-                               joindre_upload: true,
-                               joindre_zip: true, // les zips sont conservés zippés systématiquement.
-                               formulaire_action_verifier_json: true,
-                               bigup_reinjecter_uniquement: [description.bigup.identifiant],
-                       });
+                       let data = bigup.getFormData2();
+                       data.set('joindre_upload', true);
+                       data.set('joindre_zip', true);
+                       data.set('formulaire_action_verifier_json', true);
+                       data.set('bigup_reinjecter_uniquement', [description.bigup.identifiant]);
 
                        // verifier les champs
-                       $.post(bigup.target, data, null, 'json')
+                       $.ajax({
+                               type: "POST",
+                               url: bigup.target,
+                               data: data,
+                               processData: false,
+                               contentType: false,
+                               cache: false,
+                               dataType: 'json',
+                       })
                                .done(function(erreurs) {
                                        var erreur = erreurs[bigup.name] || erreurs.message_erreur;
                                        if (erreur) {
                                                bigup.presenter_erreur(input, erreur);
                                        } else {
-                                               delete data.formulaire_action_verifier_json;
+                                               data.delete('formulaire_action_verifier_json');
+
                                                // Faire le traitement prévu, supposant qu'il n'y aura pas d'erreur...
                                                var conteneur = bigup.form.parents('.formulaire_editer_document');
                                                conteneur.animateLoading();
                                                // Faire le traitement prévu, supposant qu'il n'y aura pas d'erreur...
-                                               $.post(bigup.target, data)
+                                               $.ajax({
+                                                       type: "POST",
+                                                       //enctype: 'multipart/form-data',
+                                                       url: bigup.target,
+                                                       data: data,
+                                                       processData: false,
+                                                       contentType: false,
+                                                       cache: false,
+                                               })
                                                        .done(function(html) {
                                                                bigup.presenter_succes(input, _T('bigup:succes_fichier_envoye'));
                                                                bigup.form.parents('.formulaire_spip').parent().html(html);
diff --git a/javascript/bigup.js b/javascript/bigup.js
index 15ff58d..ae2ad6a 100644
--- a/javascript/bigup.js
+++ b/javascript/bigup.js
@@ -845,6 +845,44 @@ Bigup.prototype = {
                return data;
        },
 
+       getFormData2: function() {
+               let formData = new FormData();
+               let me = this.form[0];
+
+               for (let input of me.querySelectorAll(
+                       "input, textarea, select, button"
+               )) {
+                       let inputName = input.getAttribute("name");
+                       let inputType = input.getAttribute("type");
+                       if (inputType) inputType = inputType.toLowerCase();
+
+                       // If the input doesn't have a name, we can't use it.
+                       if (typeof inputName === "undefined" || inputName === null) continue;
+
+                       if (input.tagName === "SELECT" && input.hasAttribute("multiple")) {
+                               // Possibly multiple values
+                               for (let option of input.options) {
+                                       if (option.selected) {
+                                               formData.append(inputName, option.value);
+                                       }
+                               }
+                       } else if (
+                               !inputType ||
+                               (
+                                       inputType !== "checkbox"
+                                       && inputType !== "radio"
+                                       && inputType !== "file"
+                                       && inputType !== "submit"
+                               ) ||
+                               input.checked
+                       ) {
+                               formData.append(inputName, input.value);
+                       }
+               }
+
+               return formData;
+       },
+
 
        /**
         * Assign one or more DOM nodes as a drop extended target.

Avec ça j’arrive à modifier un fichier de document sans que ça ne modifie ses liens avec.

Alors pour le 'append' c’est qu’il faut passer une option à `jQuery.ajax` pour dire de pas toucher au formData… et on peut pas l’envoyer depuis `jQuery.post`, donc il faut le réécrire avec. Ce qui donne ce diff, pour l’un des JS. ```diff git diff -w git:(master|✚2 diff --git a/javascript/bigup.documents_edit.js b/javascript/bigup.documents_edit.js index 7849e39..49a44f8 100644 --- a/javascript/bigup.documents_edit.js +++ b/javascript/bigup.documents_edit.js @@ -12,26 +12,42 @@ function formulaires_documents_edit_avec_bigup () { var bigup = file.bigup; var input = file.emplacement; - var data = $.extend(bigup.getFormData(), { - joindre_upload: true, - joindre_zip: true, // les zips sont conservés zippés systématiquement. - formulaire_action_verifier_json: true, - bigup_reinjecter_uniquement: [description.bigup.identifiant], - }); + let data = bigup.getFormData2(); + data.set('joindre_upload', true); + data.set('joindre_zip', true); + data.set('formulaire_action_verifier_json', true); + data.set('bigup_reinjecter_uniquement', [description.bigup.identifiant]); // verifier les champs - $.post(bigup.target, data, null, 'json') + $.ajax({ + type: "POST", + url: bigup.target, + data: data, + processData: false, + contentType: false, + cache: false, + dataType: 'json', + }) .done(function(erreurs) { var erreur = erreurs[bigup.name] || erreurs.message_erreur; if (erreur) { bigup.presenter_erreur(input, erreur); } else { - delete data.formulaire_action_verifier_json; + data.delete('formulaire_action_verifier_json'); + // Faire le traitement prévu, supposant qu'il n'y aura pas d'erreur... var conteneur = bigup.form.parents('.formulaire_editer_document'); conteneur.animateLoading(); // Faire le traitement prévu, supposant qu'il n'y aura pas d'erreur... - $.post(bigup.target, data) + $.ajax({ + type: "POST", + //enctype: 'multipart/form-data', + url: bigup.target, + data: data, + processData: false, + contentType: false, + cache: false, + }) .done(function(html) { bigup.presenter_succes(input, _T('bigup:succes_fichier_envoye')); bigup.form.parents('.formulaire_spip').parent().html(html); diff --git a/javascript/bigup.js b/javascript/bigup.js index 15ff58d..ae2ad6a 100644 --- a/javascript/bigup.js +++ b/javascript/bigup.js @@ -845,6 +845,44 @@ Bigup.prototype = { return data; }, + getFormData2: function() { + let formData = new FormData(); + let me = this.form[0]; + + for (let input of me.querySelectorAll( + "input, textarea, select, button" + )) { + let inputName = input.getAttribute("name"); + let inputType = input.getAttribute("type"); + if (inputType) inputType = inputType.toLowerCase(); + + // If the input doesn't have a name, we can't use it. + if (typeof inputName === "undefined" || inputName === null) continue; + + if (input.tagName === "SELECT" && input.hasAttribute("multiple")) { + // Possibly multiple values + for (let option of input.options) { + if (option.selected) { + formData.append(inputName, option.value); + } + } + } else if ( + !inputType || + ( + inputType !== "checkbox" + && inputType !== "radio" + && inputType !== "file" + && inputType !== "submit" + ) || + input.checked + ) { + formData.append(inputName, input.value); + } + } + + return formData; + }, + /** * Assign one or more DOM nodes as a drop extended target. ``` Avec ça j’arrive à modifier un fichier de document sans que ça ne modifie ses liens avec.
Owner

Ou donc en plus court en réutilisant les fonctions de jQuery forms:

var data = bigup.formToArray();
data.push({name:'joindre_upload', value:true});
data.push({name:'joindre_zip', value:true});
data.push({name:'formulaire_action_verifier_json', value:true});
data.push({name:'bigup_reinjecter_uniquement', value:[description.bigup.identifiant]});
data = $.param(data);

(data est alors une string, donc le delete qui vient ensuite est penible, mais on peut l'éviter en préparant un data et un data_verifier)

Ou donc en plus court en réutilisant les fonctions de jQuery forms: ``` var data = bigup.formToArray(); data.push({name:'joindre_upload', value:true}); data.push({name:'joindre_zip', value:true}); data.push({name:'formulaire_action_verifier_json', value:true}); data.push({name:'bigup_reinjecter_uniquement', value:[description.bigup.identifiant]}); data = $.param(data); ``` (data est alors une string, donc le delete qui vient ensuite est penible, mais on peut l'éviter en préparant un data et un data_verifier)
Poster
Owner

À l’heure où tout semble aller vers une sortie de jQuery pour du plus vanilla, je suis pas certain qu’utiliser une fonction de jquery.form soit le plus adapté.

Quoi qu’il en soit, la fonction de dropzone.js ignore certains champs volontairement, que j’avais complété avec les input.file et les submit. Pour tout récupérer les champs d’un formulaire, le mieux actuellement est de faire un :

const data = new FormData(form);

Ici (#4862) je propose une méthode donc identique quasimement (.buildFormData()) et une métohode pour simplifier l’ajax ensuite (.send()). Je ne l’ai appliqué pour le moment que sur un seul formulaire.

À voir ?

À l’heure où tout semble aller vers une sortie de jQuery pour du plus vanilla, je suis pas certain qu’utiliser une fonction de jquery.form soit le plus adapté. Quoi qu’il en soit, la fonction de dropzone.js ignore certains champs volontairement, que j’avais complété avec les input.file et les submit. Pour tout récupérer les champs d’un formulaire, le mieux actuellement est de faire un : ```js const data = new FormData(form); ``` Ici (https://git.spip.net/spip/bigup/pulls/4862) je propose une méthode donc identique quasimement (.buildFormData()) et une métohode pour simplifier l’ajax ensuite (.send()). Je ne l’ai appliqué pour le moment que sur un seul formulaire. À voir ?
b_b commented 1 month ago
Owner

Donc si je suis bien, #4862 permet de fixer spip/medias#4872 et donc remplacerait spip/medias#4898 ?

Donc si je suis bien, #4862 permet de fixer https://git.spip.net/spip/medias/issues/4872 et donc remplacerait https://git.spip.net/spip/medias/pulls/4898 ?
Poster
Owner

Oui @b_b.

Oui @b_b.
Poster
Owner

Du coup, je me demande s’il faudrait pas faire des branches 3.1 (pour SPIP 4.1) et 2.1 (pour SPIP 4.0) pour bigup, comme on ajoute une fonction ?

Et éventuellement une 1.1 pour SPIP 3.2 ?

Ou pas la peine ?

Du coup, je me demande s’il faudrait pas faire des branches 3.1 (pour SPIP 4.1) et 2.1 (pour SPIP 4.0) pour bigup, comme on ajoute une fonction ? Et éventuellement une 1.1 pour SPIP 3.2 ? Ou pas la peine ?
b_b commented 4 weeks ago
Owner

Je trouve "étrange" de faire une branche X.Y, autant on le fait pour SPIP, mais pour un plugin (même du core) ça me parait un peu "too much". Oui on ajoute une fonction, mais à la base c'est surtout pour corriger un bug.

D'autant plus que pour la branche concernant SPIP 3.2, pas grand monde pourra en bénéficier par SVP puisqu'il le zip n'est pas dans le dépôt de la zone mais celui du core (cf mon questionnement récent à ce sujet https://github.com/geodiversite/geodiversite/issues/29).

Je trouve "étrange" de faire une branche X.Y, autant on le fait pour SPIP, mais pour un plugin (même du core) ça me parait un peu "too much". Oui on ajoute une fonction, mais à la base c'est surtout pour corriger un bug. D'autant plus que pour la branche concernant SPIP 3.2, pas grand monde pourra en bénéficier par SVP puisqu'il le zip n'est pas dans le dépôt de la zone mais celui du core (cf mon questionnement récent à ce sujet https://github.com/geodiversite/geodiversite/issues/29).
Poster
Owner

Ok. bah ça sera plus simple :p

Ok. bah ça sera plus simple :p
Poster
Owner

Bon, je suis plutôt partie pour faire propre au moins pour les branches SPIP 4.1 (c’est fait, branche 3.1, à laquelle j’ai ajouté tes triggers #4860)
et SPIP 4.0…

Bon, je suis plutôt partie pour faire propre au moins pour les branches SPIP 4.1 (c’est fait, branche 3.1, à laquelle j’ai ajouté tes triggers #4860) et SPIP 4.0…
Poster
Owner

J’ai également fait une branche 1.1 avec ces reports, pour SPIP 3.2

J’ai également fait une branche 1.1 avec ces reports, pour SPIP 3.2
b_b commented 4 weeks ago
Owner

Donc on peut fermer le ticket ?

Donc on peut fermer le ticket ?
b_b commented 4 weeks ago
Owner

On ferme.

On ferme.
b_b closed this issue 4 weeks ago
Sign in to join this conversation.
No Milestone
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.