Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat : flask admin view - Tableau de bord #39

Merged
merged 10 commits into from
Apr 20, 2023

Conversation

andriacap
Copy link
Contributor

@andriacap andriacap commented Mar 15, 2023

Dans le cadre de discussions avec le Parc National des Ecrins , et discuté notamment avec @camillemonchicourt il s'agissait de proposer un tableau de bord pour les tables de log générées avec GN2PG .
Du coup, la solution proposée est d'utiliser Flask-Admin pour généré les vues de ces tables.

Exemple de vues :

home_gn2pg_dashboard

Le travail prévu pour cette PR :

  • Créer application avec Flask et Flask-admin
  • Les vues (tables de log en bdd download_log , erorr_log et increment_log ) doivent être en readonly sur flask-admin
  • L'app GN2PG avec flask et flask-adlmin (tableau de bord) doit être déployable avec l'utilisation de service et configuration apache
  • L'accès à l'app GN2PG doit être protégé avec un htaccess et htpasswd
  • Une documentation est rédigée pour que l'utilisateur puisse installer l'applicaiton de manière autonome

    - Add config for variable environment (with python-dotenv)
    - Add model DownloadModel
    - Add view ReadOnlyView and DownloadView

Reviewed-by: andriac
@andriacap andriacap force-pushed the feat/flask-admin-view branch 3 times, most recently from 057165a to 69619f9 Compare March 15, 2023 11:23
- Add test for config.py (install pip pytest-env and config pytest.ini)
- Add test for model (WIP)

Reviewed-by: andriac
Reviewed-by:andriac
Change configtest (default)

reviewed-by:andriac
- Add test for config.py (install pip pytest-env and config pytest.ini)
- Add test for model (WIP)

Reviewed-by: andriac
@andriacap andriacap force-pushed the feat/flask-admin-view branch from 69619f9 to c9163a9 Compare March 15, 2023 14:23
Based on settings , conf of GeoNature community (TaxHub,UsersHub)

Reviewed-by:andriac
@lpofredc
Copy link
Member

Merci pour ces devs.

Il me semblerait plus pertinent de séparer concrètement l'application GN2PG de cet outil de suivi et de visualisation. Il parait dommage d'alourdir l'installation actuelle de GN2PG pour un outil qui ne servira pas systématiquement. D'autant plus que cela n'apporte concrètement aucune nouvelle fonctionnalité à son objet principal (récupération de données depuis le module d'export vers une base de donnée tierce). N'était-il pas question d'en faire un module GeoNature mais avec uniquement la partie BackOffice ? @camillemonchicourt ?

Pour l'installation, si on reste bien en "standalone", je séparerai l'installation de l'application de l'installation des config serveur (service, apache, etc.). Pour laisser la possibilité de configurer soit-même son serveur.

Il serait mieux de privilégier de l'utilisation des variables d'environnement plutôt que l'utilisation stricte des fichiers settings.ini ou .env, notamment pour la dockerisation qui est aujourd'hui souvent privilégié à une installation en dur. python-decouple permet l'utilisation mixte des variables d'environnement ou du .env.

Lors de la visualisation des logs, le contenu des champs Item (item issu de l'export) et Error (Error provenant de la bdd de destination) du model 'ErrorLog' sont souvent très importants. Je verrai bien une surcouche du template natif list.html en tronquant automatiquement le contenu des champs dépassant un certain nombre de caractères (200 ?) et en ajoutant une modal de visualisation détaillée du contenu du champ pour ces cas de figure.

Un formattage des données du champ Item (JSON) sera aussi bienvenu. cf. GeoNature-citizen (capture qui suit).

Un petit passage à Bootstrap4 en pleine largeur (class CSS container-fluid) ne serait pas du luxe ;)

Screenshot 2023-03-16 at 10-34-27 Enquêtes - 3a - Formulaires dynamiques - GN-Citizen Backoffice d'administration (version 1 0 0 )

Screenshot 2023-03-16 at 10-27-02 Error Log - GN2PG

@camillemonchicourt
Copy link
Contributor

Merci @lpofredc pour ces retours.
On n'a pas souhaité faire de cet outil un module GeoNature pour faire simple et garder GN2PG autonome et indépendant de GeoNature.

Je pense que c'est bien de garder l'admin dans GN2PG et dans le même dépôt, mais en effet pour séparer son installation dans un script dédiée pour qu'elle soit optionnelle.

Formatter for long text and json
Add boostrap tempalte and custom css

Reviewed: andriac
@andriacap
Copy link
Contributor Author

Hello,

Merci à vous @lpofredc et @camillemonchicourt pour vos retours !

Sur la base de tes retours @lpofredc j'ai ajouté , dans le dernier commit #0e1ffe des formatters pour :

  • formater le json comme c'est le cas dans GNCitizen
  • formatter les lignes du tableau pour forcer une hauteur max et rendre scrollable la cellule qui contient le texte/json trop long .

J'ai également rajouté le template bootstrap et ajouté un custom.css pour mettre en 100% width.

Concernant la partie installation , ce qui est proposé dans cette PR permet tout à fait d'installer où non le coté "applicatif" pour visualiser le tableau de bord. Je pourrais rajouter dans la documentation une précision "si vous souhaitez installer l'application pour avoir un visuel des logs alors lancez le script d'installation install_app.sh qui permet de généré tous les fichiers apache et de service." Et à cette documentation il serait possible d'ajouter des images d'exemple de l'application pour que l'utilisateur puisse se faire une idée de ce qu'il va installer.

Merci de m'informer si cette option peut convenir ou si vous pensiez à autre chose concernant "la séparation de l'installation"

Bonne journée ! :)

Add script to install with possibility to choose user for htpsswd andto
install conf with virtualhost and new server_name or juste create conf
to include in an existent virtualhost with an existent server_name

Reviewed-by:andriac
@lpofredc
Copy link
Member

Je pense que c'est bien de garder l'admin dans GN2PG et dans le même dépôt, mais en effet pour séparer son installation dans un script dédiée pour qu'elle soit optionnelle.

De mon point de vue, l'intégration actuelle de cette application de dashboard dans gn2pg pose problème du moment ou elles peuvent parfaitement fonctionner chacune de leur côté.

Cela alourdit significativement les dépendances de GN2PG par des dépendances non utilisées par gn2pg_cli et pour un outil non requis et qui ne sera pas toujours utilisé. Ce n'est pas non plus sans conséquences en terme de maintenance et de mises à jour de ces dépendances.

+flask = "^2.2.3"
+flask-sqlalchemy = "^3.0.3"
+python-dotenv = "^1.0.0"
+flask-admin = "^1.6.1"
+gunicorn = "^20.1.0"

Je maintient mon avis sur la nécessité de séparer ces deux applications sur deux dépôts différents en notant toutefois dans citant chacune dans leurs docs respectives.

@camillemonchicourt
Copy link
Contributor

On voit donc 2 possibilités :

On préfère la deuxième solution pour ne pas se compliquer avec 2 dépôts / 2 outils à gérer, à mettre en cohérence, etc...

- Add poetry --group dashboard
- Add documentation install dashboard gn2pg
- Change config.py to use settings [db] from config.toml used for
  gn2pg_cli

Reviewed-by: andriac
@andriacap andriacap marked this pull request as ready for review March 23, 2023 18:01
@andriacap
Copy link
Contributor Author

andriacap commented Mar 23, 2023

Hello,

Suite à nos discussions j'ai apporté les modifications suivantes :

  • Ajout de la documentation (pour le moment elle est placée ici : docs/app_dashboard.rst

  • Modifications du fichier de config.py pour utiliser les configurations de la db issues du fichier utilisé par gnpg_cli

     db_host = "localhost"
     db_port = 5432
     db_user = "<dbUser>"
     db_password = "<dbPassword>"
     db_name = "<dbName>"
     db_schema_import = "gn2pg_import"

N'hésitez pas si vous avez des questions .
Bonne soirée

Correction documentation and add into index.rst

Reviewed-by: andriac
@lpofredc lpofredc merged commit 23ca52c into lpoaura:dev Apr 20, 2023
@lpofredc lpofredc mentioned this pull request Apr 20, 2023
@lpofredc
Copy link
Member

PR mergée dans la branche Dev et PR d'intégration officielle à la branche main en attente (#42) car checks échouent.

@adricap, Les tests CI échouent à cause d'erreurs PyLint à corriger.

Bien penser à installer pre-commit pour vérifier les commit.

poetry run pre-commit install

Pour lancer manuellement pylint, il faut éxécuter make pylint à la racine du projet.

Je reste assez mitigé sur le déploiement du dashbord tel que présenté dans la doc et les scripts d'install (à partir du code source tandis que gn2pg s'installe simplement avec un pip install...). Notamment avec le fichier settings.ini dont l'usage nécessaire est très limité...

J'ai commencé à travailler sur une version avec intégration dans le CLI de GN2PG pour une plus simple utilisation gn2pg_cli --dashboard <fichierdeconfig> ici: dev...feat-dashbord_cli_integration

@andriacap
Copy link
Contributor Author

andriacap commented Apr 21, 2023

Salut @lpofredc ,

Ok merci pour ces retours . Je voir pour le pylint . Il faut corriger toutes les erreurs remontées par Pylint pour que la merge passe c'est ça ?
Pour l'execution du dashboard en ligne de commande , je vois pas trop comment aider pour le moment. On pourra voir pour en discuter pour savoir ce que tu aimerais mettre en place.
Encore merci pour ces retours !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants