Membership in months or by default

[manuelh78]

pour validation
Si OK sur le principe, je me pencherais sur les tests.

Fonctionne via les plugins paypal et stripe.

https://bugs.galette.eu/issues/1438

[trasher]

Merci pour la PR ; je m'y penche dès que possible :)

[trasher]

J'ai regardé d'un peu plus près, je suis totalement d'accord sur le principe - merci. Je n'ai pas vraiment testé encore.

Je me suis permis de corriger le script de migration PostgreSQL ainsi que les erreurs de CS dans le PHP.

Je craignais que les tests existants ne passent pas ; mais il n'y a pas de problème : c'est un bon point, comme tu le soulevais, on utilise assez peu la valeur modifiée finalement :)

Je vais peut-être avoir quelques remarques techniques à faire, mais rien de bien important, on verra ça plus tard.

[trasher]

J'ai de nouveau fait quelques changements ; plutôt que de faire des remarques :)

Je pense que les modifications dans la classe Contributions gagneraient certainement à être scindées en deux méthodes distinctes ; mais je pense qu'il faut ajouter des tests avant ça.

Je pense que tout le reste est bon sinon.

[gagnieray]

Pour ma part tout me semble OK.

Cependant, je ne comprends pas le résultat des tests unitaires dans Github avec PHP 8.3 et PostgreSQL. De mon côté quand je les lance en local (avec PHP 8.3.3 et PostgreSQL 15.6) aucun n'échoue 😲

Sinon, je me pose la question de l'incidence de ces changements sur les rappels d'échéances.

Actuellement ils ne fonctionnent toujours pas correctement (c.f. #375). Donc je ne pense pas que ça change quoi que ce soit pour le moment 😅

Mais dans le cas d'une extension d'adhésion courte (1 mois principalement), n'y a-t-il pas un risque de "flooder" les adhérents de rappels à peine leur adhésion enregistrée (il est normalement censé pouvoir y avoir un rappel à 30 jours puis un autre à 7 jours de la date d'échéance) 🤔

[trasher]

Pour ma part tout me semble OK.

Merci :)

Cependant, je ne comprends pas le résultat des tests unitaires dans Github avec PHP 8.3 et PostgreSQL. De mon côté quand je les lance en local (avec PHP 8.3.3 et PostgreSQL 15.6) aucun n'échoue 😲

Ha... Étrange, j'ai pu reproduire de mon côté, j'ai corrigé.

Sinon, je me pose la question de l'incidence de ces changements sur les rappels d'échéances.

Actuellement ils ne fonctionnent toujours pas correctement (c.f. #375). Donc je ne pense pas que ça change quoi que ce soit pour le moment 😅

Alors, c'est une bonne question, en effet ; merci de l'avoir posée 🤣
Je n'y avais pas du tout songé... Comme tu le fais remarquer, ce n'est toujours pas totalement fonctionnel, mais une partie fonctionne quand même... Donc des rappels peuvent partir.
Sans parler spécifiquement de "flood", les durées entre deux rappels devraient peut-être prendre en compte la durée initiale de l'adhésion - parce qu'en effet ce ne serait pas spécialement cohérent sinon.

Je vais voir à ajouter un ou deux tests avec un type de contribution ayant une échéance "courte" dans la partie des rappels pour voir comment ça se comporte actuellement ; ce sera dans tous les cas utiles pour la suite.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 34.37500% with 21 lines in your changes are missing coverage. Please review.

Project coverage is 45.76%. Comparing base (3856ce3) to head (5c583a7).
Report is 145 commits behind head on develop.

❗ Current head 5c583a7 differs from pull request most recent head 8b6db0d. Consider uploading reports for the commit 8b6db0d to get more accurate results

Files	Patch %	Lines
galette/lib/Galette/Entity/Contribution.php	22.72%	17 Missing ⚠️
galette/lib/Galette/Entity/ContributionsTypes.php	66.66%	3 Missing ⚠️
.../Controllers/Crud/ContributionsTypesController.php	0.00%	1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@              Coverage Diff              @@
##             develop     #448      +/-   ##
=============================================
- Coverage      45.79%   45.76%   -0.03%     
+ Complexity      6024     6020       -4     
=============================================
  Files            146      146              
  Lines          24414    24416       +2     
=============================================
- Hits           11180    11174       -6     
- Misses         13234    13242       +8     

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

[manuelh78]

Github indique qu'il y a un commit 'unmerged' en conflit.
Manuel

[trasher]

Heu... Il y a bien des conflits sur cette PR (cf capture ci-dessous), mais je ne vois mentions nulle part de "commit unmerged". Aurais-tu ajouté un comit sans avoir préalablement récupéré les modifications que j'ai faites sur la branche ?

[manuelh78]

Slt,
Si je ferme le PR, j'ai un "commit unmerged", si je l'ouvre à nouveau, il disparait.
Je n'ai pas les droits de résoudre les conflits.

[trasher]

Les conflits, il vaut mieux les résoudre en local, en faisant un rebase sur la branche develop (et après un backup de ta branche si tu n'est pas sûr de ton coup).
Mais normalement ce n'est pas non plus gênant, les conflits empêchent de merger la PR, ce sera à voir quand elle sera terminée.

Évidemment que si tu ferme la PR, github va te dire que ce n'est pas mergé, simplement parce que c'est le cas. Mais... Pourquoi vouloir fermer ?

[trasher]

Salut,

J'ai créé une autre PR (#498) depuis ta branche, pour tester avec un code à jour sans avoir à intervenir sur ton fork.

Je me suis par ailleurs rendu compte que certains tests n'étaient lancés : finalement cette PR casse un des tests existants :/

Il faut absolument ajouter des tests pour que cette fonctionnalité soit intégrable - je ne suis pas spécialement le mieux placé pour ça : j'ai tendance à penser qu'il manque du traitement sur des cas particuliers, et je ne suis pas certain de savoir ce qui est attendu dans les différents cas de figure.

[manuelh78]

Salut,
Je n'ai pas eu le temps de regarder.
J'imagine qu'il y a un test qui vérifie la date d'échéance juste après l'ajout d'une contribution ?

A la suite, il faudrait ajouter un nouveau type de contribution d'une durée spécifique, par exemple 3 mois
Puis ajouter une contribution de ce type
puis contrôler que la date d'échéance a été augmentée de 3 mois

Normalement, année civile ou pas, ça ne devrait pas avoir d'impact, puisqu'en passant en mois, il n'y a plus vraiment de notion "d'année".

Manu

[trasher]

(autant continuer la discussion ici ;))

Salut, Je n'ai pas eu le temps de regarder. J'imagine qu'il y a un test qui vérifie la date d'échéance juste après l'ajout d'une contribution ?

Oui, c'est d'ailleurs ce test là qui ne passe plus :/

[...]
Normalement, année civile ou pas, ça ne devrait pas avoir d'impact, puisqu'en passant en mois, il n'y a plus vraiment de notion "d'année".

OK, dans ce cas l'implémentation actuelle n'est pas bonne, puisque ça ne va fonctionner que si pref_beg_membership est défini - il faut revoir les modification apportées à Entity\Contribution.
Cela fait justement partie des choses que je ne comprenais pas.

[manuelh78]

OK, dans ce cas l'implémentation actuelle n'est pas bonne, puisque ça ne va fonctionner que si pref_beg_membership est défini - il faut revoir les modification apportées à Entity\Contribution.

Il faut proposer le fonctionnement normal sur 365j ou par année civile, et à n'importe quel moment pouvoir passer en abonnement mensuel.

Il s'agit de fidéliser l'adhérent.

Ce type de fonctionnement est courant dans les "grosses" associations.
Par exemple :

Paypal et Stripe implémentent les abonnements.

[trasher]

OK, dans ce cas l'implémentation actuelle n'est pas bonne, puisque ça ne va fonctionner que si pref_beg_membership est défini - il faut revoir les modification apportées à Entity\Contribution.

Il faut proposer le fonctionnement normal sur 365j ou par année civile, et à n'importe quel moment pouvoir passer en abonnement mensuel.

Il s'agit de fidéliser l'adhérent.

Heu... C'est l'implémentation qui est incorrecte. La PR telle que proposée au début ne pouvait juste pas fonctionner correctement.

[trasher]

J'ai revu, corrigé et ajouté des tests ; voir #498.