export PDF: bloc de jours non observés

jean-io commented 4 months ago

Dans certain export PDF un bloc de jours non observé est present dans le tableau, alors qu'en réalité, les jours sont observés.

Limezy commented 4 months ago

Je n'arrive pas à reproduire

jean-io commented 4 months ago

Non moi non plus. Une utilisatrice m'a envoyé ce bug. Je vais devoir retrouver les observations en bdd pour comprendre.

mtctn commented 2 months ago

Bonjour,

Même problème pour moi, observé lors de l'export en PDF ou en CSV. En regardant le code j'ai l'impression que ça vient du fait que deux enregistrements sont présents à la même date (par exemple deux observations datées du 18/06), et l'algorithme qui prépare les données pour l'affichage ajoute des jours pour combler l'intervalle entre le 18 du mois et... le 18 du mois, qui apparaissent comme étant du 19/06 au 30/06 puis du 01/06 au 17/06.

Je pense donc qu'il y a deux problèmes indépendants :

il est possible que deux enregistrements soient faits pour la même date
lorsque c'est le cas, l'export en PDF ou en CSV ajoute des jours non observés

Merci beaucoup pour l'appli en tout cas !

jean-io commented 2 months ago

Bonjour @mtctn, merci pour l’analyse et la contribution 🙂 est-ce que vous avez réussi à reproduire le problème ?

Si le problème est confirmé, il y a deux méthodes :

supprimer l'observation dans l'interface web, il restera logiquement l'autre si l'instruction DELETE efface pas les deux.
supprimer l'une des deux observations au choix dans la base de donnée.

mtctn commented 2 months ago

Je rencontre actuellement le problème sur plusieurs cycles sur l'instance tableau.moncycle.app si vous voulez regarder les données.

Pour la solution 1, est-ce que je suis sûr que ça n'effacera qu'une seule des deux observations ?

Et pour la solution 2 il faut avoir un accès direct à la bdd ?

jean-io commented 2 months ago

Je pensais que vous utilisiez une version auto hébergé, c'est pour ca que je vous ai proposé l'option 2.

il est possible que deux enregistrements soient faits pour la même date

Comment êtes vous arrivé à cette conclusion ? en théorie c'est pas possible.

Pour l'option 1, je regarde.

mtctn commented 2 months ago

La solution 1 fonctionne mais supprime les deux lignes, il faut donc re-saisir l'observation.

Merci beaucoup !

jean-io commented 2 months ago

Top voila l'explication du fix :

Le call API s’exécute ici : https://github.com/jean-io/moncycle.app/blob/60b6ebdab2655c5e8b562a6c46374c4d670fe001/www_data/api/observation.php#L122

Le nombre de lignes supprimées est bien renvoyé vers le navigateur mais non affiché : https://github.com/jean-io/moncycle.app/blob/60b6ebdab2655c5e8b562a6c46374c4d670fe001/www_data/api/observation.php#L126

Il n'y a pas de limite du nombre de lignes supprimées dans la querie SQL : https://github.com/jean-io/moncycle.app/blob/60b6ebdab2655c5e8b562a6c46374c4d670fe001/www_data/lib/db.php#L188

Est-ce que vous pouvez ouvrir votre console est verifier le nombre de lignes supprimées?

Sinon une petit étoile sur le repo nous aide 🙂 merci.

mtctn commented 2 months ago

Je suis arrivé à cette conclusion en regardant le code qui fait l'export et les données produites. Si vous regardez les lignes J12 et J36 dans l'image qui illustre le premier post la donnée est dupliquée, et dans les cas que j'ai vus la date était la même. Après je ne suis pas certain non plus de mon interprétation.

jean-io commented 2 months ago

Je viens de regarder le PDF qui a illustré cette issue, et il y a bien cette incohérence ! bien vu !

Avez vous une idée de comment deux observations ont pu être renseigné pour la même date ? Il y avait pas de problème au niveau de l'affichage web ? C'est peut être un bug seulement au moment de l'export PDF.

mtctn commented 2 months ago

Est-ce que vous pouvez ouvrir votre console est verifier le nombre de lignes supprimées?

C'est malheureusement trop tard, on a corrigé toutes les occurrences avant de voir ce message :'( J'essaierai si ça se reproduit (mais on avait au moins 3 ou 4 occurrences...)

Limezy commented 2 months ago

@jean-io je viens de découvrir que j'avais un cas sur mon instance auto hébergée, l'épidémie semble un peu plus sévère que prévu !

Je ne touche à rien pour le moment et attends tes instructions si tu souhaites en profiter pour mener l'enquête.

J'imagine qu'une requête SQL sur la base de données de la version tableau.moncycle.app permettra de filtrer les doublons et identifier le nombre de cycles et d'utilisateurs touchés 🙃

Reste à retrouver pourquoi et comment plusieurs observations peuvent être crées pour la même date...

Limezy commented 2 months ago

Je confirme, après vérification dans ma base, que j'ai deux lignes d'observations pour la même date, ce qui semble confirmer l'analyse de @mtctn (bien joué!)

jean-io commented 2 months ago

Hum ce sont pas des bonnes nouvelles... 🤔 il y a un seul endroit dans le code où il y a une insertion d'une entrée dans la table observation:

https://github.com/jean-io/moncycle.app/blob/60b6ebdab2655c5e8b562a6c46374c4d670fe001/www_data/api/observation.php#L85

Reste à comprendre pourquoi cette condition n'est pas suffisante...

@Limezy peux-tu partager la requête SQL qui t'a permis d'identifier les doublons?

Pour moi il y a deux manières complémentaires de fixer le problème :

Améliorer la condition ci-dessus.
Rajouter une contrainte dans le schéma de la base interdisant plusieurs dates pour un même compte.

jean-io commented 2 months ago

Il y a aucun bug d'affichage web ? C'est uniquement sur les PDF ?

Limezy commented 2 months ago

Reste à comprendre pourquoi cette condition n'est pas suffisante...

J'imagine que si, pour une raison ou une autre, la mise à jour d'une observation est appelée deux ou plusieurs fois simultanément, il peut y avoir un très petit temps pendant lequel la base répond deux fois "null" ce qui permet à la fonction db_insert_observation( de s'exécuter deux fois de suite alors qu'il ne faudrait pas. Ajouter une contrainte d'unicité dans la base semble nécessaire en effet, en attendant de comprendre pourquoi cette double execution (je précise que ma femme et moi n'utilisons jamais l'application simultanément depuis deux téléphones par exemple, donc ce cas me semble hors de propos).

Un indice semble corroborer cette hypothèse : dans mon cas, pour les deux lignes, la valeur de la colonne dernier_modif était identique à la minute près, ce qui indique une mise en base des doublons proche dans le temps. Un autre indice est que le contenu des deux lignes est exactement identique.

@Limezy peux-tu partager la requête SQL qui t'a permis d'identifier les doublons?

Heu... J'ai un peu honte mais j'ai juste exporté ma base au format csv depuis Phpmyadmin, je n'ai gardé que les données de mon compte puis ai fait une mise en exergue des doublons en mise en forme conditionnelle

Il y a aucun bug d'affichage web ? C'est uniquement sur les PDF ?

Aucun problème d'affichage sur le web. Uniquement sur les pdf.

J'ai tenté d'altérer une des deux lignes dans ma base de données. C'est l'observation de valeur ID la moins élevée qui est affichée (dans mon cas doublon le plus récent, l'observation numéro 714). En cas de nettoyage de la base tu devrais donc pouvoir retirer l'observation de valeur ID plus élevée qui n'apparaît nulle part.

Pour aider au débug tu peux aussi regarder en base quelle est la première instance de ce problème, ce qui te donnera probablement une indication du commit fautif ? Dans mon cas, je n'ai eu aucun problème de doublon pendant presque 18 mois d'observations suivi par deux doublons en 3 semaines !

jean-io commented 2 months ago

Heu... J'ai un peu honte mais j'ai juste exporté ma base au format csv depuis Phpmyadmin, je n'ai gardé que les données de mon compte puis ai fait une mise en exergue des doublons en mise en forme conditionnelle

Pas de soucis, tant que ça marche! 😅

La solution est donc d'introduire des transactions MySQL pour éviter des accès concurrents + la contrainte sur la base.

Je suspecte l'auto enregistrement d'être à l'origine de requêtes ajax simultanées et de créer ce problème.

Limezy commented 2 months ago

Ça ressemble en effet à un effet secondaire de l'enregistrement auto. Les modifs citées plus haut rendront les doublons impossibles mais ne régleront malheureusement pas ce bug en tant que tel (le bloc bizarre dans les exports)...

Il faudra soit supprimer les doublons, soit altérer la routine de création du pdf pour ne prendre que la plus récente des plusieurs entrées pour une date donnée !

jean-io commented 2 months ago

La solution est donc d'introduire des transactions MySQL pour éviter des accès concurrents

A implementer : https://www.php.net/manual/en/pdo.transactions.php

Ça ressemble en effet à un effet secondaire de l'enregistrement auto. Les modifs citées plus haut rendront les doublons impossibles mais ne régleront malheureusement pas ce bug en tant que tel (le bloc bizarre dans les exports)...

Une grande partie du code part du principe que la bdd est saine. Introduire des mécanismes de verification ou de correction en cas de bdd corrompue semble très lourd au borne du projet, perso je ne suis pas fan.

Limezy commented 2 months ago

perso je ne suis pas fan.

Je comprends, mais il faudra donc passer par une phase d'assainissement de la base. Si tu parviens à le faire via un script sql, ce serait parfait de pouvoir le partager pour une exécution sur les quelques instances auto-hébergées qui auraient été touchées. Sinon il faudra demander aux admins de le faire à la main (en gros, ne perds pas de temps pour ça si ta solution n'est pas elle-même directement codée en sql)

jean-io commented 2 months ago

Voici la requête SQL que j'ai pu construire pour la recherche de doublon :

select count(no_observation) as nb_duplicate, O.no_compte, no_observation, methode, date_obs 
from observation as O
inner join compte on compte.no_compte = O.no_compte
group by date_obs, O.no_compte
having nb_duplicate>1
order by date_obs desc

@Limezy je suis preneur d'une confirmation que cette query SQL fonctionne bien sur ta base, stp 🙂

Exécuté en prod:

Quelques points clés après plus d'investigations :

il y a au total 267 dates qui ont ce problèmes
il y a 89 comptes concernés
le premier doublon est apparue le 2022-08-06
mais le phénomène a explosé autour du 2024-03-04 (on passe de 4/5 doublons par mois à 4/5 doublons par jours)
il y a que des doublons (les jours sont renseignés au maximum deux fois)
toutes les méthodes (FC, Billings, temp) sont concernées

Je suspecte l'auto enregistrement d'être à l'origine de requêtes ajax simultanées et de créer ce problème.

l'auto enregistrement (https://github.com/jean-io/moncycle.app/commit/b1427b1a65c451126b669aea1621681445e76d31) est arrivé en v8, les dates concordent pas... Je n'ai donc pas d'explication de pourquoi ce bug est d'un coup plus present.

Limezy commented 2 months ago

Merci ! Je vais tenter cette requête SQL sur ma base et je te dis.

l'auto enregistrement (b1427b1) est arrivé en v8, les dates concordent pas... Je n'ai donc pas d'explication de pourquoi ce bug est d'un coup plus présent.

Ça peut être (et ça ressemble à) un facteur indirect : il y a un bug lorsque la requête de mise en base se fait d'une certaine façon ou dans un certain timing, présent depuis longtemps, mais le changement de l'auto-enregistrement induit un comportement plus propice à ce bug. Une belle noise en tout cas !

jean-io commented 2 months ago

✅ contrainte DB OK https://github.com/jean-io/moncycle.app/commit/8580302d686a1eb78e97e8bc47851144d0b3bfd3 ✅ transaction pour les insertions en base OK https://github.com/jean-io/moncycle.app/commit/c81ec0970ababae6e6f0b49f13d3a9a6411db3a0

TODO : DELETE SQL query pour nettoyer les bases de donées

Limezy commented 2 months ago

De mon côté je confirme que la requête SQL fonctionne sur ma base 👍

jean-io commented 2 months ago

Suite au commit https://github.com/jean-io/moncycle.app/commit/57680941cc9afbcee47531f1fab9985628e0b6c9, le script de mise à jour de la base inclut bien les requêtes DELETE pour nettoyer la base de donnée. Pensez bien à faire un backup de votre base avant l’exécution du script.

Lien vers celui-ci : https://github.com/jean-io/moncycle.app/blob/master/db/migration/v13_to_v14.sql

Pour info, même si la prod n'est pas en v14, ces modifications de structure et le nettoyage ont bien appliqué sur celle-ci. Je clos donc l'issue, si vous constatez encore des problèmes, hésitez pas à les remonter ici.

jean-io commented 2 months ago

Hello, pour info, dans les logs de prod j'ai trouvé cette exception. Elle intervient après l'ajout de la contrainte en bdd mais avant la mise à jour du php :

[15-Jul-2024 06:13:19 UTC] PHP Fatal error:  Uncaught PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'xxxx-2024-07-15' for key 'unique_compte_and_date' in /var/www/html/lib/db.php:234
Stack trace:
#0 /var/www/html/lib/db.php(234): PDOStatement->execute()
#1 /var/www/html/api/observation.php(86): db_insert_observation()
#2 {main}
  thrown in /var/www/html/lib/db.php on line 234

jean-io commented 1 month ago

Hello, le bug est encore présent ... malgré la mise à jour ! heureusement la contrainte SQL fait le boulot. Avez-vous des idées des causes possibles ?

[13-Aug-2024 09:11:56 UTC] PHP Fatal error:  Uncaught PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '911-2024-08-11' for key 'unique_compte_and_date' in /var/www/html/lib/db.php:234
Stack trace:
#0 /var/www/html/lib/db.php(234): PDOStatement->execute()
#1 /var/www/html/api/observation.php(86): db_insert_observation()
#2 {main}
  thrown in /var/www/html/lib/db.php on line 234

jean-io / moncycle.app

export PDF: bloc de jours non observés #46