Feat/Synthese Export: add group3 inpn into synthese view export

[andriacap]

Bonjour,

Dans le cadre d'une prestation pour l'Agence Régionale de la Biodiversité en île de France, une demande a été faite pour avoir la possibilité d'afficher la colonne group3_inpn dans les exports de synthese notamment sur les exports csv d'observations et de taxons .

Etant donné que ces téléchargements csv se basent sur les vues v_synthese_for_export et v_synthese_taxon_for_export_view , j'ai suggéré d'ajouter une revision qui modifierait ces vues , ainsi qu'un changement dans la synthese config pour avoir par défaut la colonne group3_inpn dans l'export.

Pour la révision je m'appuie sur des scripts sql plutot que de l'éxécution de "statement" avec alembic . Si la norme veut que l'on fasse autrement , je peux apporter les modifications nécessaires .

Je met la PR en draft en attendant vos retours , avis. D'ailleurs cette PR, bien que indépendante au niveau code, appartient au même "scope" que la PR de Maxime (voir #2621)

Merci

[andriacap]

Bonjour,

Je ne comprend pas pourquoi les check failed .
J'ai bien récupéré les dev de la branche develop de Geonature et j'ai rebase la branche feat/add-group3-inpn-to-synthese-export-view par rapport à la branche develop.

J'ai également éxécuté les commandes git submodule init , git submodule update et geonature db autoupgrade  . Tout est ok en local , je comprends pas les failed de ces checks ..

En attendant que j'effectue quelques recherches plus approfondies, sii quelqu'un peut m"éclairer afin que les checks passent , ça serait avec plaisir

Merci

[mvergez]

Il s'avère que la migration ne passe pas car elle dépend de la migration du schéma taxonomie ajoutant la colonne group3_inpn. Il faut donc que celle ci soit une dépendance de la migration créée dans cette PR

[andriacap]

Merci Maxime pour ton retour !

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a3bde03) 78.44% compared to head (f41151d) 68.56%.
Report is 6 commits behind head on feat/groupinpn3.

❗ Current head f41151d differs from pull request most recent head 64aff5c. Consider uploading reports for the commit 64aff5c to get more accurate results

Additional details and impacted files

@@                 Coverage Diff                 @@
##           feat/groupinpn3    #2637      +/-   ##
===================================================
- Coverage            78.44%   68.56%   -9.88%     
===================================================
  Files                   89       86       -3     
  Lines                 7213     7435     +222     
===================================================
- Hits                  5658     5098     -560     
- Misses                1555     2337     +782     

Flag	Coverage Δ
pytest	68.56% <ø> (-9.88%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[camillemonchicourt]

L'idée de créer un fichier SQL pour la création ou modification des vues et d'uniquement exécuter ce SQL dans la migration me plait bien car elle permet plus de lisibilité sur ce qu'on fera ensuite évoluer dans cette vue, avec un diff plus lisible que si on recréé toute la vue dans la migration Alembic.

Cependant on ne fait comme ça jusqu'à présent où l'on recréé justement toute la vue dans la migration quand on fait évoluer une vue ou fonction.

Pourquoi pas partir sur des SQL à part mais à homogénéiser, généraliser et valider ce fonctionnement avant ?
Un fichier par vue ? Un fichier avec toutes les vues ? Un fichier avec toutes les vues et les fonctions ?

---

Aussi, pour l'ajout du group3_inpn dans la vue, je pense qu'il faut l'ajouter juste après l'ajout des groupes INPN 1 et 2 pour plus de lisibilité.

[andriacap]

L'idée de créer un fichier SQL pour la création ou modification des vues et d'uniquement exécuter ce SQL dans la migration me plait bien car elle permet plus de lisibilité sur ce qu'on fera ensuite évoluer dans cette vue, avec un diff plus lisible que si on recréé toute la vue dans la migration Alembic.

Cependant on ne fait comme ça jusqu'à présent où l'on recréé justement toute la vue dans la migration quand on fait évoluer une vue ou fonction.

Pourquoi pas partir sur des SQL à part mais à homogénéiser, généraliser et valider ce fonctionnement avant ? Un fichier par vue ? Un fichier avec toutes les vues ? Un fichier avec toutes les vues et les fonctions ?

Aussi, pour l'ajout du group3_inpn dans la vue, je pense qu'il faut l'ajouter juste après l'ajout des groupes INPN 1 et 2 pour plus de lisibilité.

Salut @camillemonchicourt , j'ai changé l'ordre d'ajout des colonnes group3_inpn .
Concernant l'architecture d'alembic , il peut être intéressant de ré organiser les fichiers sql dans des sous dossiers également ? (comme j'ai pu le faire dans le dernier commit que je viens de push)

Pour une réflexion plus élargie sur alembic j'ai également vu cette article qui présente des choses qui peuvent être intéressantes à mettre en place --> https://medium.com/alan/making-alembic-migrations-more-team-friendly-e92997f60eb2

[andriacap]

Pour le lint frontend , j'ai bien appliqué npm run format et le lint Prettier me dit que tous mes fichiers sont "OK" . Je ne comprend pas pourquoi ça ne passe pas .. J'investigue le problème..

[camillemonchicourt]

OK, oui intéressant.
Mais, en l'état, ce qui m'embête c'est que le dossier "/backend/geonature/migrations/data/core/" contient les SQL de création des schéma de GN dans l'état en version 2.7, avant de passer à Alembic et n'évoluent plus, mais on commencerait à y ajouter aussi des fichiers SQL qui évoluent, dans un sous-dossier "gn_synthese" avec une partie qui évolue.
Et un fichier SQL de toute la synthèse qui lui n'est pas dans le dossier "gn_synthese" et n'évolue plus (synthese.sql).

Donc si on part sur cette solution, il faudrait trouver un moyen que cela soit plus clair et pas mélanger les anciens trucs qui bougent plus et les SQL des vues et fonctions.

Il y a aussi ce sujet qui reste ouvert sur le fait qu'on a perdu en lisibilité sur la structure de la BDD et de son évolution avec la mise en place d'Alembic (même si on y a beaucoup gagné par ailleurs) - #1574

Dans Geotrek-admin, c'est assez intéressant et clair comme c'est fait :

• Un dossier par module avec les migrations (ordonnées ça aide) concentrés uniquement sur les champs des tables - https://github.com/GeotrekCE/Geotrek-admin/tree/master/geotrek/trekking/migrations
• Un dossier par module avec des scripts SQL à jouer automatiquement avant ou après chaque migration, qui suppriment et recréé tout ce qui est moins bien géré par les migrations (vues, fonctions, triggers) : https://github.com/GeotrekCE/Geotrek-admin/tree/master/geotrek/trekking/templates/trekking/sql
  Cela fait que les vues, fonctions et triggers sont tous supprimés et recréés à chaque migration, mais c'est pas un soucis.
  Et un système permet aussi de se créer des fichiers SQL maison qui sont aussi lancé avant ou après chaque migration - https://geotrek.readthedocs.io/en/latest/install/advanced-configuration.html#custom-sql