Permettre aux adhérent·e·s de s'inscrire/désinscrire seul·e·s aux listes mail de l'association.
.
Cela se veut une première version basique. Plusieurs améliorations sont possibles ; je les ai pour l'instant regroupées dans ce ticket https://code.ffdn.org/FFDN/coin/issues/195
Je suis bien friand de retours, pour vérifier que ça marche, me donner votre avis sur mon code, améliorer la doc… etc.
Ce qui me reste à faire (mais je pense que la review peut se faire ou a minima commencer sans) :
finir d'écrire les tests unitaires
bien border les cas où la commande de synchronisation échoue
si synchro lancée par une modif de la fiche adhérent·e : ne pas bloquer coin pour autant, mais afficher un message d'erreur explicite à l'admin : Trop compliqué à gérer, donc je laisse comme ça : le code plantera (erreur 500) dans ce cas.
si synchro lancée explicitement dans l'admin (via les admin actions), ne pas planter coin, mais afficher des messages pour indiquer quelles listes ont réussi et quelles listes ont échoué.
si synchro lancée par l'inscription d'un·e adhérent·e de manière autonome, l'avertir si la synchro a échoué (et ne pas enregistrer son adhésion en base).
Permettre aux adhérent·e·s de s'inscrire/désinscrire seul·e·s aux listes mail de l'association.
.
Cela se veut une première version basique. Plusieurs améliorations sont possibles ; je les ai pour l'instant regroupées dans ce ticket https://code.ffdn.org/FFDN/coin/issues/195
Je suis bien friand de retours, pour vérifier que ça marche, me donner votre avis sur mon code, améliorer la doc… etc.
Ce qui me reste à faire (mais je pense que la review peut se faire ou a minima commencer sans) :
- [x] finir d'écrire les tests unitaires
- [x] bien border les cas où la commande de synchronisation échoue
- <strike>si synchro lancée par une modif de la fiche adhérent·e : ne pas bloquer coin pour autant, mais afficher un message d'erreur explicite à l'admin</strike> : Trop compliqué à gérer, donc je laisse comme ça : le code plantera (erreur 500) dans ce cas.
- [x] si synchro lancée explicitement dans l'admin (via les admin actions), ne pas planter coin, mais afficher des messages pour indiquer quelles listes ont réussi et quelles listes ont échoué.
- [x] si synchro lancée par l'inscription d'un·e adhérent·e de manière autonome, l'avertir si la synchro a échoué (et ne pas enregistrer son adhésion en base).
à la vue très rapide du code je pense qu'il y a un risque d'accès concurrent : rien ne semble empêcher l'exécution de deux commande de synchronisation en même temps.
Vu les soucis, cas d'erreurs, blocages évoqués dans ta PR je me dis que l'utilisation de taches asynchrones en arrière plan auraient une bonne place pour résoudre ces soucis
à la vue très rapide du code je pense qu'il y a un risque d'accès concurrent : rien ne semble empêcher l'exécution de deux commande de synchronisation en même temps.
Vu les soucis, cas d'erreurs, blocages évoqués dans ta PR je me dis que l'utilisation de taches asynchrones en arrière plan auraient une bonne place pour résoudre ces soucis
à la vue très rapide du code je pense qu'il y a un risque d'accès concurrent : rien ne semble empêcher l'exécution de deux commande de synchronisation en même temps.
Non, c'est vrai. Dans la plupart des cas, cela ne posera pas souci majeur (à mon avis) vu que la liste est régénérée en entier à chaque modif.
Vu les soucis, cas d'erreurs, blocages évoqués dans ta PR je me dis que l'utilisation de taches asynchrones en arrière plan auraient une bonne place pour résoudre ces soucis
Dans l'idée oui, ça serait mieux. J'ai choisi de ne pas utiliser des tâches asynchrones (type celery) pour l'instant :
complexité (l'autre nom de la complexité est « flemme »)
on reste sur des tâches pas très longues non plus (donc la latence n'était pas un critère énorme à mon sens, mais ça reste à voir…)
plus complexe de produire un retour utilisateur (passe par un système de messages/alertes asynchrones)
Bref, j'ai l'impression qu'utiliser un système de tâches asynchrones pourrait être une solution mais demanderait un travail un peu trop conséquent et repousserait pas mal cette fonctionnalité :-/.
Hey, merci :-)
> à la vue très rapide du code je pense qu'il y a un risque d'accès concurrent : rien ne semble empêcher l'exécution de deux commande de synchronisation en même temps.
Non, c'est vrai. Dans la plupart des cas, cela ne posera pas souci majeur (à mon avis) vu que la liste est régénérée en entier à chaque modif.
> Vu les soucis, cas d'erreurs, blocages évoqués dans ta PR je me dis que l'utilisation de taches asynchrones en arrière plan auraient une bonne place pour résoudre ces soucis
Dans l'idée oui, ça serait mieux. J'ai choisi de ne pas utiliser des tâches asynchrones (type celery) pour l'instant :
- complexité (l'autre nom de la complexité est « flemme »)
- on reste sur des tâches pas très longues non plus (donc la latence n'était pas un critère énorme à mon sens, mais ça reste à voir…)
- plus complexe de produire un retour utilisateur (passe par un système de messages/alertes asynchrones)
Bref, j'ai l'impression qu'utiliser un système de tâches asynchrones pourrait être une solution mais demanderait un travail un peu trop conséquent et repousserait pas mal cette fonctionnalité :-/.
(je ne sais pas comment tu as fait pour citer une portion de message, gogs est une énigme)
Oui ca rajoute de la complexité tant sur le flow de traitement que sur la mise en place des environnements. Après j'ai pas suivi depuis un certain temps les développements de Coin mais si il y a d'autres besoins de ce type ca peut valoir le coup (générations des factures, relances sans passer par du cron etc, ...)
Dans l'absolu je rejoins ton avis, au pire ca sera pété jusqu'à ce qu'un gentil membre change ses abonnements
(je ne sais pas comment tu as fait pour citer une portion de message, gogs est une énigme)
Oui ca rajoute de la complexité tant sur le flow de traitement que sur la mise en place des environnements. Après j'ai pas suivi depuis un certain temps les développements de Coin mais si il y a d'autres besoins de ce type ca peut valoir le coup (générations des factures, relances sans passer par du cron etc, ...)
Dans l'absolu je rejoins ton avis, au pire ca sera pété jusqu'à ce qu'un gentil membre change ses abonnements
(je ne sais pas comment tu as fait pour citer une portion de message, gogs est une énigme)
markdown + copié-collé
Oui ca rajoute de la complexité tant sur le flow de traitement que sur la mise en place des environnements. Après j'ai pas suivi depuis un certain temps les développements de Coin mais si il y a d'autres besoins de ce type ca peut valoir le coup (générations des factures, relances sans passer par du cron etc, ...)
Yep, je pense que ça pourrait valoir le coup pour ce genre de cas dans l'absolu. Juste le rapport temps passé / bénéfice n'est pas très favorable à mon avis. Si on fait un dévcamp et que quelqu'un·e veut jouer avec celery, ça peut être un truc rigolo en revanche. À voir.
Dans l'absolu je rejoins ton avis, au pire ca sera pété jusqu'à ce qu'un gentil membre change ses abonnements
Sachant que les admins ont la possibilité de déclencher manuellement la synchro.
> (je ne sais pas comment tu as fait pour citer une portion de message, gogs est une énigme)
markdown + copié-collé
> Oui ca rajoute de la complexité tant sur le flow de traitement que sur la mise en place des environnements. Après j'ai pas suivi depuis un certain temps les développements de Coin mais si il y a d'autres besoins de ce type ca peut valoir le coup (générations des factures, relances sans passer par du cron etc, ...)
Yep, je pense que ça pourrait valoir le coup pour ce genre de cas dans l'absolu. Juste le rapport temps passé / bénéfice n'est pas très favorable à mon avis. Si on fait un dévcamp et que quelqu'un·e veut jouer avec celery, ça peut être un truc rigolo en revanche. À voir.
> Dans l'absolu je rejoins ton avis, au pire ca sera pété jusqu'à ce qu'un gentil membre change ses abonnements
Sachant que les admins ont la possibilité de déclencher manuellement la synchro.
Le truc qui reste nul c'est que si la synchro plante :
on peut éditer ses abo depuis l'interface user (et il y a un warning) donc c'est bien
par contre pas depuis l'interface d'admin
et même pire ça empêche l'édition de l'adresse mail (puisqu'elle déclenche la synchro)
Pour les deux derniers points on pourrait rattraper l'erreur mais pas afficher de "message" parce qu'on n'a pas accès à la "request" dans les signaux ...
Je propose de merger en l'état après relecture.
On a fait quelques modifs.
Le truc qui reste nul c'est que si la synchro plante :
- on peut éditer ses abo depuis l'interface user (et il y a un warning) donc c'est bien
- par contre pas depuis l'interface d'admin
- et même pire ça empêche l'édition de l'adresse mail (puisqu'elle déclenche la synchro)
Pour les deux derniers points on pourrait rattraper l'erreur mais pas afficher de "message" parce qu'on n'a pas accès à la "request" dans les signaux ...
Je propose de merger en l'état après relecture.
OK pour le compromis sur la gestion d'erreur, c'est comme ça :-)
Relu, testé, validé !
… Et je n'ai pas pu m'empêcher, j'ai apporté quelques peaufinements d'interfaces (cf dernier commit dont le message est détaillé).
@sim Vu que c'est des changements mineurs, je merge dans la foulée, qu'on en finisse :-)
Youhou !
OK pour le compromis sur la gestion d'erreur, c'est comme ça :-)
Relu, testé, validé !
… Et je n'ai pas pu m'empêcher, j'ai apporté quelques peaufinements d'interfaces (cf dernier commit dont le message est détaillé).
@sim Vu que c'est des changements mineurs, je merge dans la foulée, qu'on en finisse :-)
Youhou !
Sim
Permettre aux adhérent·e·s de s'inscrire/désinscrire seul·e·s aux listes mail de l'association.
Cela se veut une première version basique. Plusieurs améliorations sont possibles ; je les ai pour l'instant regroupées dans ce ticket https://code.ffdn.org/FFDN/coin/issues/195
Je suis bien friand de retours, pour vérifier que ça marche, me donner votre avis sur mon code, améliorer la doc… etc.
Ce qui me reste à faire (mais je pense que la review peut se faire ou a minima commencer sans) :
si synchro lancée par une modif de la fiche adhérent·e : ne pas bloquer coin pour autant, mais afficher un message d'erreur explicite à l'admin: Trop compliqué à gérer, donc je laisse comme ça : le code plantera (erreur 500) dans ce cas.j'ai écrit les tests.
J'ai géré les cas d'erreur au mieux.
à la vue très rapide du code je pense qu'il y a un risque d'accès concurrent : rien ne semble empêcher l'exécution de deux commande de synchronisation en même temps.
Vu les soucis, cas d'erreurs, blocages évoqués dans ta PR je me dis que l'utilisation de taches asynchrones en arrière plan auraient une bonne place pour résoudre ces soucis
Hey, merci :-)
Non, c'est vrai. Dans la plupart des cas, cela ne posera pas souci majeur (à mon avis) vu que la liste est régénérée en entier à chaque modif.
Dans l'idée oui, ça serait mieux. J'ai choisi de ne pas utiliser des tâches asynchrones (type celery) pour l'instant :
Bref, j'ai l'impression qu'utiliser un système de tâches asynchrones pourrait être une solution mais demanderait un travail un peu trop conséquent et repousserait pas mal cette fonctionnalité :-/.
(je ne sais pas comment tu as fait pour citer une portion de message, gogs est une énigme)
Oui ca rajoute de la complexité tant sur le flow de traitement que sur la mise en place des environnements. Après j'ai pas suivi depuis un certain temps les développements de Coin mais si il y a d'autres besoins de ce type ca peut valoir le coup (générations des factures, relances sans passer par du cron etc, ...)
Dans l'absolu je rejoins ton avis, au pire ca sera pété jusqu'à ce qu'un gentil membre change ses abonnements
markdown + copié-collé
Yep, je pense que ça pourrait valoir le coup pour ce genre de cas dans l'absolu. Juste le rapport temps passé / bénéfice n'est pas très favorable à mon avis. Si on fait un dévcamp et que quelqu'un·e veut jouer avec celery, ça peut être un truc rigolo en revanche. À voir.
Sachant que les admins ont la possibilité de déclencher manuellement la synchro.
ARN ne compte pas utiliser ce module pour l'instant (on met en place un forum discourse pour remplacer nos ML) Donc on vous laisse gérer cette MR.
On a fait quelques modifs.
Le truc qui reste nul c'est que si la synchro plante :
Pour les deux derniers points on pourrait rattraper l'erreur mais pas afficher de "message" parce qu'on n'a pas accès à la "request" dans les signaux ...
Je propose de merger en l'état après relecture.
OK pour le compromis sur la gestion d'erreur, c'est comme ça :-)
Relu, testé, validé !
… Et je n'ai pas pu m'empêcher, j'ai apporté quelques peaufinements d'interfaces (cf dernier commit dont le message est détaillé).
@sim Vu que c'est des changements mineurs, je merge dans la foulée, qu'on en finisse :-)
Youhou !
Et j'ai documenté un petit warning également pour éviter d'être surpris par les 200 appels à SSH.