saisies_verifier() et les dates nulles #79

Closed
opened 2 years ago by maieul · 11 comments
maieul commented 2 years ago
Owner

J'ai voulu me pencher un peu sur cextras pour mutualiser toute la vérification avec saisies.

Le point où on se heurte est le suivant. Dans cextras_pipelines, on le commentaire suivant

// [FIXME] exceptions connues de vérifications (pour les dates entre autres)
// en attendant une meilleure solution !
//
// Lorsque le champ n'est pas rempli dans le formulaire
// alors qu'une normalisation est demandée,
// verifier() sort sans indiquer d'erreur (c'est normal).
//
// Sauf que la donnée alors soumise à SQL sera une chaine vide,
// ce qui ne correspond pas toujours à ce qui est attendu.

En gros: si un champ date a été envoyé en '', il faut automatiquement le convertir en '0000:00:00'. Or la normalisation n'est appelé que si le champ !=''. Du coup pas de normalisation...et cextras contourner cela.

Pareil que pour #78, il faudrait que ce soit géré automatiquement par saisies, et pareil, uniquement sur les formulaires editer_truc.

J'en viens à me demande du coup s'il faudrait pas créer une fonction saisies_normaliser_pour_sql() qui

  1. Serait appelée à la toute fin de saisies_verifier() si l'on est dans un formulaire editer_xxx
  2. Ferait les deux choses suivantes :
    a. Pour les saisies notées comme afficher_si_masque dans la globale, ferait _set_request('lasaisie', '')
    b. Pour les saisies 'date' avec un 'normaliser' en sql, transformerait '' en 0000-00-00. A moins que l'on considère que ce soit du rôle du core ?
J'ai voulu me pencher un peu sur cextras pour mutualiser toute la vérification avec saisies. Le point où on se heurte est le suivant. Dans cextras_pipelines, on le commentaire suivant // [FIXME] exceptions connues de vérifications (pour les dates entre autres) // en attendant une meilleure solution ! // // Lorsque le champ n'est pas rempli dans le formulaire // alors qu'une normalisation est demandée, // verifier() sort sans indiquer d'erreur (c'est normal). // // Sauf que la donnée alors soumise à SQL sera une chaine vide, // ce qui ne correspond pas toujours à ce qui est attendu. En gros: si un champ date a été envoyé en `''`, il faut automatiquement le convertir en `'0000:00:00'`. Or la normalisation n'est appelé que si le champ `!=''`. Du coup pas de normalisation...et cextras contourner cela. Pareil que pour #78, il faudrait que ce soit géré automatiquement par saisies, et pareil, uniquement sur les formulaires `editer_truc`. J'en viens à me demande du coup s'il faudrait pas créer une fonction `saisies_normaliser_pour_sql()` qui 1. Serait appelée à la toute fin de `saisies_verifier()` si l'on est dans un formulaire `editer_xxx` 2. Ferait les deux choses suivantes : a. Pour les saisies notées comme `afficher_si_masque` dans la globale, ferait `_set_request('lasaisie', '')` b. Pour les saisies 'date' avec un 'normaliser' en sql, transformerait `''` en `0000-00-00`. A moins que l'on considère que ce soit du rôle du core ?
Poster
Owner

Hier soir, @chankalan a rapporté un bug sur cextras lié à la fois à #79 et #78.

En gros :

  • un champ extra date conditionnée par afficher_si
  • si jamais la condition n'est pas rempli, champ extra tente de mettre la valeur du champ à ''
  • du coup ca échoue en mysql

Le bug ne se produisait pas par le passé, car avant champ_extra n'ajoutait à pre_edition que les champs qui n'était pas masqué par afficher_si. Si bien que masquer un champ faisait que l'ancienne valeur était conservée au lieu d'être mise à '', 0 ou 0000-00-00.

Du coup

  1. Cela confirme qu'il faudrait en pre_edition de saisies mettre les _request masquées par afficher_si à l'une des valeurs suivantes :
    a. En priorité, la valeur DEFAUT du sql indiquée dans la définition de saisie
    b. A defaut, la valeur DEFAUT de la description SQL en base
    c. A defaut, à ''
  2. Cela résoudrait le bug #78 et #79 en même temps, a priori.
Hier soir, @chankalan a rapporté un bug sur cextras lié à la fois à #79 et #78. En gros : - un champ extra date conditionnée par afficher_si - si jamais la condition n'est pas rempli, champ extra tente de mettre la valeur du champ à '' - du coup ca échoue en mysql Le bug ne se produisait pas par le passé, car avant champ_extra n'ajoutait à pre_edition que les champs qui n'était pas masqué par afficher_si. Si bien que masquer un champ faisait que l'ancienne valeur était conservée au lieu d'être mise à `''`, `0` ou `0000-00-00`. Du coup 1. Cela confirme qu'il faudrait en pre_edition de saisies mettre les `_request` masquées par `afficher_si` à l'une des valeurs suivantes : a. En priorité, la valeur DEFAUT du sql indiquée dans la définition de saisie b. A defaut, la valeur DEFAUT de la description SQL en base c. A defaut, à '' 2. Cela résoudrait le bug #78 et #79 en même temps, a priori.

Je suis très (très) dubitatif de cela pour le moment. Saisies est une API uniquement de génération des formulaires : pas de leur traitement.

Les problèmes de SQL, sont parfaitement propre à UNE utilisation précise de génération de formulaire : pour enregistrer le résultat dans une table SQL. Et ça c'est donc propre à ce besoin, entre autre Champs Extras.

Donc je trouve plutôt très logique que ce soit là où yen a besoin en tant que traitement, que ce soit fait.

Je suis très (très) dubitatif de cela pour le moment. Saisies est une API uniquement de génération des formulaires : pas de leur traitement. Les problèmes de SQL, sont parfaitement propre à UNE utilisation précise de génération de formulaire : pour enregistrer le résultat dans une table SQL. Et ça c'est donc propre à ce besoin, entre autre Champs Extras. Donc je trouve plutôt très logique que ce soit là où yen a besoin en tant que traitement, que ce soit fait.
Poster
Owner

oui, oki, ca se tient. Mais il n'empeche dans ce cas qu'il faut d'une manière ou d'une autre que cextras sache au moment du traitzement quels sont les saisies masquées par afficher_si pour décider le cas échéant de de mettre au choix un 0, un 0000-00-00 ou une chaine vide.

Et comme le problème pourrait se poser avec d'autres formulaires qui utilisent les saisies et enregistre en table SQL il ne parait pas aberrant que saisies fournissent une fonction pour gérer ce genre de cas, histoire de mutualiser le code (quitte à ce que cette fonction soit appelée à chaque besoin)

oui, oki, ca se tient. Mais il n'empeche dans ce cas qu'il faut d'une manière ou d'une autre que cextras sache au moment du traitzement quels sont les saisies masquées par afficher_si pour décider le cas échéant de de mettre au choix un 0, un 0000-00-00 ou une chaine vide. Et comme le problème pourrait se poser avec d'autres formulaires qui utilisent les saisies et enregistre en table SQL il ne parait pas aberrant que saisies fournissent une fonction pour gérer ce genre de cas, histoire de mutualiser le code (quitte à ce que cette fonction soit appelée à chaque besoin)

c'est aussi à cause du mélange entre verifier et normaliser, qui, ça a déjà été débattu, devrait sûrement être une API dédié aussi qu'on décrit à part, et qui s'applique même s'il n'y a pas de vérification… mais c'est un débat/chantier plus gros (et qui pèterait tout)

c'est aussi à cause du mélange entre verifier et normaliser, qui, ça a déjà été débattu, devrait sûrement être une API dédié aussi qu'on décrit à part, et qui s'applique même s'il n'y a pas de vérification… mais c'est un débat/chantier plus gros (et qui pèterait tout)

La proposition super simple d'hier soir qui corrige en 10 caractères :

  • c'est à faire dans verifier() car c'est ce plugin qui s'occupe pour l'instant des normalisations
  • actuellement les valeurs ne passent pas du tout dans les vérifs si la valeur est vide
  • c'est logique si ya que des vérif, mais depuis que les normalisations ont été ajoutées, la logique c'est que le plugin doit quand même lancer la vérif (qui normalise) SI ya une normalisation demandée

Donc là https://git.spip.net/spip-contrib-extensions/verifier/src/branch/master/inc/verifier.php#L35

Il faut modifier avec un truc du genre :

if (
	(is_null($valeur) or (is_string($valeur) and $valeur == ''))
	and empty($options['normaliser'])
)

et alors dès qu'il y a une demande de normalisation, même si la valeur est vide, ça va quand même lancer

Ça ne résout pas le 78, mais on a trouvé un moyen que je vais noter dans l'autre.

La proposition super simple d'hier soir qui corrige en 10 caractères : - c'est à faire dans `verifier()` car c'est ce plugin qui s'occupe pour l'instant des normalisations - actuellement les valeurs ne passent pas du tout dans les vérifs si la valeur est vide - c'est logique si ya que des vérif, mais depuis que les normalisations ont été ajoutées, la logique c'est que le plugin doit *quand même* lancer la vérif (qui normalise) SI ya une normalisation demandée Donc là https://git.spip.net/spip-contrib-extensions/verifier/src/branch/master/inc/verifier.php#L35 Il faut modifier avec un truc du genre : ``` php if ( (is_null($valeur) or (is_string($valeur) and $valeur == '')) and empty($options['normaliser']) ) ``` et alors dès qu'il y a une demande de normalisation, même si la valeur est vide, ça va quand même lancer Ça ne résout pas le 78, mais on a trouvé un moyen que je vais noter dans l'autre.

En fait on peut sûrement faire plus fin pour ce point.

Si c'est vide, et qu'il y a une demande de normaliser, alors il ne faudrait faire QUE la normalisation. Il y a deux manières de faire :

  • soit on vérifie et/ou améliore la manière dont les normalisations sont appelées, pour être sûr que ce soit normé justement, ce qui permet alors à coup sûr d'avoir une "vraie" API normalisation à l'intérieur du plugin, càd que même si pour l'instant ça reste obligatoirement dans Vérifier, on s'assure de pouvoir les appeler seules aussi (et donc on le fait dans ce cas)
  • soit on laisse comme c'est, quelque soit l'architecture, mais pour ces valeurs vides où y a une demande quand même de normalisation, on appelle quand même la fonction dédié de vérif/normalisation, mais pour là on ne récupère pas l'erreur, mais uniquement le $normaliser

Ainsi ça serait directement à la source que ce serait bien fait.

Avec ça en place, alors dans le #78, pour l'étape 3, on peut simplifier et ya même pas à savoir lesquelles sont masquées ! En effet :

  • seules celles non masquées (fonction déjà utilisée) passent dans le test d'obligation
  • mais on fait passer l'ensemble (donc y compris celles masquées qui sont désormais en chaine vide) dans l'API Vérifier (au lieu d'écraser $formulaire et utiliser que ça, il suffit de distinguer $formulaire_complet et $formulaire_non_masquees en gros pour dire vite)
En fait on peut sûrement faire plus fin pour ce point. Si c'est vide, et qu'il y a une demande de normaliser, alors il ne faudrait faire QUE la normalisation. Il y a deux manières de faire : - soit on vérifie et/ou améliore la manière dont les normalisations sont appelées, pour être sûr que ce soit normé justement, ce qui permet alors à coup sûr d'avoir une "vraie" API normalisation à l'intérieur du plugin, càd que même si pour l'instant ça reste obligatoirement dans Vérifier, on s'assure de pouvoir les appeler seules aussi (et donc on le fait dans ce cas) - soit on laisse comme c'est, quelque soit l'architecture, mais pour ces valeurs vides où y a une demande quand même de normalisation, on appelle quand même la fonction dédié de vérif/normalisation, mais pour là on ne récupère pas l'erreur, mais uniquement le $normaliser Ainsi ça serait directement à la source que ce serait bien fait. Avec ça en place, alors dans le #78, pour l'étape 3, on peut simplifier et ya même pas à savoir lesquelles sont masquées ! En effet : - seules celles non masquées (fonction déjà utilisée) passent dans le test d'obligation - mais on fait passer l'ensemble (donc y compris celles masquées qui sont désormais en chaine vide) dans l'API Vérifier (au lieu d'écraser $formulaire et utiliser que ça, il suffit de distinguer $formulaire_complet et $formulaire_non_masquees en gros pour dire vite)
Collaborator

Attention, les dates en "0000-00-00" ça peut être casse gueule.

En fonction du sql_mode (variable de config dans my.cnf), elles peuvent être rejetées s'il contient NO_ZERO_DATE, ce qui est le cas par défaut à partir de Mysql 5.7.8

https://dev.mysql.com/doc/refman/5.7/en/sql-mode.html

Attention, les dates en "0000-00-00" ça peut être casse gueule. En fonction du sql_mode (variable de config dans my.cnf), elles peuvent être rejetées s'il contient NO_ZERO_DATE, ce qui est le cas par défaut à partir de Mysql 5.7.8 https://dev.mysql.com/doc/refman/5.7/en/sql-mode.html
Poster
Owner

Attention, les dates en "0000-00-00" ça peut être casse gueule.

En fonction du sql_mode (variable de config dans my.cnf), elles peuvent être rejetées s'il contient NO_ZERO_DATE, ce qui est le cas par défaut à partir de Mysql 5.7.8

tout à fait, mais pour le coup on suit la norme SPIPienne. Il y a un ticket core sur le sujet je crois.

> Attention, les dates en "0000-00-00" ça peut être casse gueule. > En fonction du sql_mode (variable de config dans my.cnf), elles peuvent être rejetées s'il contient NO_ZERO_DATE, ce qui est le cas par défaut à partir de Mysql 5.7.8 tout à fait, mais pour le coup on suit la norme SPIPienne. Il y a un ticket core sur le sujet je crois.
Poster
Owner

Et sinon pas d'opinion pour l'instant entre les 2 options concernant le rapport verif/normalisation.

Et sinon pas d'opinion pour l'instant entre les 2 options concernant le rapport verif/normalisation.
Collaborator

Plus précisément, ça dépend aussi du mode SQL strict :
https://dev.mysql.com/doc/refman/5.7/en/sql-mode.html#sqlmode_no_zero_date
mais j'ai déjà été confronté à ça.

Plus précisément, ça dépend aussi du mode SQL strict : https://dev.mysql.com/doc/refman/5.7/en/sql-mode.html#sqlmode_no_zero_date mais j'ai déjà été confronté à ça.

Effectivement, mais donc SPIP utilise ça absolument 100% partout sur tous les objets du monde, et dans tous les plugins objets du monde. Donc avec saisies ou sans, avec vérifier ou sans. Du coup s'il y a une chose à résoudre pour ce point, ça sera forcément globalement, comprenant le core, plugins-dist, etc.

Tant qu'à faire le lien du ticket dédié : https://core.spip.net/issues/4364

Effectivement, mais donc SPIP utilise ça absolument 100% partout sur tous les objets du monde, et dans tous les plugins objets du monde. Donc avec saisies ou sans, avec vérifier ou sans. Du coup s'il y a une chose à résoudre pour ce point, ça sera forcément globalement, comprenant le core, plugins-dist, etc. Tant qu'à faire le lien du ticket dédié : https://core.spip.net/issues/4364
maieul closed this issue 2 years 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.