[FRONTEND][MAP] Duplicated geom on current draw's edition

[andriacap]

Version
Version de GeoNature affectée par le bug --> toutes les versions depuis que l'utilisation du composant leaflet-draw.component.ts est utilisé

Description du bug

Le bug concerne une duplication de géométrie en cours d'édition (que ce soit à la modification d'une géométrie existante ou en cours de création).

Voir captures d'écran ci dessous (cliquez pour déplier) :

Comportement attendu

Le comportement attendu est le suivant : lorsqu'on décide de modifier une géométrie existante ou que l'on décide de modifier une nouvelle géométrie en cours d'édition il faut que la géométrie précédente ne soit pas prise en compte.

Comment reproduire
Étapes à suivre pour reproduire le problème :

• Exmple sur Module OCCTAX (mais marche sur tous les endroits de GeoNature où le composant leaflet-draw.component.ts est appelé .
• Editer une géométrie (type: polygone)
• Cliquer sur l'icone de création d'un polygone dans le composant map
• Créer le polygone en le fermant et en appuyant sur "Saved"
• Cliquer sur l'icone d'édition de layers sur le composant map
• Déplacer une des coordonnées du polygone

** Historique de modifications du composant leaflet-draw.component.ts **

L'évènement "DRAWSTOP" n'était pas pris en compte jusqu'à l'intégration de cette PR : #2779.

Le commit suivant 10e905c présent dans l'historique de la PR prenait en compte la gestion de ce bug.
Après review de la PR , une partie du code proposé a été retiré .

L'idée étant donc de réintégrer ce qui avait été proposé sur le commit mentionné ci dessus. Une PR va être ouverte dans la foulée permettant de reprendre ce bout de code retiré et ledit code sera aussi refact pour une meilleur lisibilité .

Merci d'avance pour votre lecture

[camillemonchicourt]

Oui en effet, c'est un soucis qu'on a rencontré plusieurs.
J'ai l'impression qu'il n'a pas toujours été là, qu'il a été là, puis réglé puis revenu, mais je ne sais plus trop. C'est possible que cela soit là depuis longtemps et jamais solutionné, même si ça m'étonne que ça ne soit alors pas remonté plus tôt.

Dans tous les cas, tant mieux si c'est résolu durablement. :-)

[andriacap]

Ce qui est possible c'est que le bug soit apparu en ajoutant l'event "DRAWSTOP" lié à cette PR #2779 .
Mon hyptohèse c 'est que l'ensemble des cas d'usage appelant cet event n'a pas été pris en compte dans l'état final de la PR mergé . C'est pourquoi il y avait ces conditions imbriquées dans ce commit initial de la proposition de PR : 10e905c.

Et en même temps l'issue mentionnait un autre bug et la PR suffisait à résoudre l'issue #2779 .

Un peu complexe à tester tous les cas en effet et à vérifier qu'il n'y a pas d'autres bugs qui apparaissent ^^.

A terme des tests unitaire frontend permettraient de diminuer le risque d'apparition de ce genre de bugs .

[camillemonchicourt]

OK alors possible d'ajouter des tests Frontend dans cette PR pour ne pas recasser cette fonctionnalité dans le futur ?

[andriacap]

Je pense qu'il faudrait faire un socle de base solide de configuration de tests frontend unitaire avant de le faire ponctuellement sur une PR .

Parce qu'en l'état la majorité des PR proposés coté frontend ne propose pas de tests unitaires. Donc soit c'est imposé à toutes les PR avec des checks qui ne passent pas etc soit la PR est testée par les personnes qui review (ce qui est le cas actuellement) pour vérifier qu'il n'y ait pas de bugs introduits.

Dans l'idéal, dans le manuel de développeur on aurait un truc du genre :
"Si des propositions de développement frontend sont réalisés, s'appuyer sur cet exemple de tests unitaires pour pouvoir proposer une PR 'mergeable' . Avec une présentation des configurations à utiliser etc " .

Après ce n'est que mon avis et je suis dispo et à l'écoute des personnes qui souhaitent qu'on mette en place ce genre de tests :)

[camillemonchicourt]

Oui ce serait l'idéal, mais on n'en a pas actuellement la capacité ni les ressources.
On a mis en place une première série de tests frontend de base pour avoir un début et un cadre, en espérant les compléter plus tard, globalement ou au fur et à mesure, avec les contributions successives.
En se concentrant surtout sur les parties fragiles/sensibles. Et là il semble que cela en soit une et qu'elle ait cassé récemment.

On passe beaucoup beaucoup de temps pour les autres à relire et tester les PR, sans parler des releases.

Donc si les contributions externes peuvent aider et apporter même un petit test de plus, surtout quand c'est un truc qui a cassé plusieurs fois et a certainement été recassé par une autre PR récemment, c'est bienvenue.

Mais en effet pour les tests frontend, on ne peut pas l'imposer. Donc libre à chacun.

La seule chose qu'on a pu définir comme pré-requis est qu'une nouvelle PR ne fasse pas diminuer le taux de couverture de tests backend sur lesquels on s'est concentré jusqu'à présent.

[andriacap]

Oui j'entend bien que c'est difficile en terme de capacité et de ressources . Et dans mon message je ne dit pas que la relecture ou les tests ne sont pas fait loin de là .

On a mis en place une première série de tests frontend de base pour avoir un début et un cadre, en espérant les compléter plus tard, globalement ou au fur et à mesure, avec les contributions successives.
En se concentrant surtout sur les parties fragiles/sensibles

Ca se trouve à quel endroit ? Pour quelles parties du code ? Est ce qu'il y a un exemple sur lequel s'appuyer ?
Je ne parle pas des tests frontend end to end qui concernent l'import pour lesquels on a participé. Je parle de tests unitaires .

Peut être que Jacques ou un autre développeur l'a déjà implémenté quelque part et je serais bien évidemment partant pour en parler avec la personne qui l'a mis en place .

[camillemonchicourt]

Ah oui, là dans le détail je ne maitrise pas assez le sujet.
Je pensais à des tests dans la continuité de ceux existant, à priori end to end, ici : https://github.com/PnX-SI/GeoNature/tree/master/frontend/cypress