entrouvert
/
passerelle
15
0
Fork 0
Code Issues Pull Requests 13 Releases Activity

toulouse-maelis : gérer l'expiration lors du paiement (#76395) #224

Merged
nroche merged 8 commits from wip/76395-parsifal-cancel-invoice into main 2023-04-21 14:10:00 +02:00
Conversation 9 Commits 8 Files Changed 7 +516 -25
nroche commented 2023-04-20 17:09:45 +02:00
Owner
Copy Link
  • Ajouter un paramètre au connecteur pour indiquer la durée de vie de la facture générée sur la validation d'un panier.
  • Cacher cette facture à Lingo si elle n'a pas encore été payé avant que le délai n'ait expiré.
  • Envoyer la demande d'annulation de la facture si le délais expiré par deux fois : pour laisser le temps à l'usager de régler sa facture dans lingo s'il s'y prend juste avant que la facture ne soit plus proposée au paiement.
* Ajouter un paramètre au connecteur pour indiquer la durée de vie de la facture générée sur la validation d'un panier. * Cacher cette facture à Lingo si elle n'a pas encore été payé avant que le délai n'ait expiré. * Envoyer la demande d'annulation de la facture si le délais expiré par deux fois : pour laisser le temps à l'usager de régler sa facture dans lingo s'il s'y prend juste avant que la facture ne soit plus proposée au paiement.
fpeters commented 2023-04-20 17:13:55 +02:00
Owner
Copy Link

3 patchs concernent les tests : ils corrigent les numéros des régies sur les tests des factures et rafraîchissent les trames SOAP des paniers avec les trames obtenues dernièrement.
(désolé pour ces diffs que j'aurais pu minimiser : je préfère repartir sur de vraies données)

Déplace plutôt ça dans un autre ticket, qui pourra être validé facilement, plutôt qu'ici où ça revient à compliquer le travail de relecture.

> 3 patchs concernent les tests : ils corrigent les numéros des régies sur les tests des factures et rafraîchissent les trames SOAP des paniers avec les trames obtenues dernièrement. (désolé pour ces diffs que j'aurais pu minimiser : je préfère repartir sur de vraies données) Déplace plutôt ça dans un autre ticket, qui pourra être validé facilement, plutôt qu'ici où ça revient à compliquer le travail de relecture.
nroche force-pushed wip/76395-parsifal-cancel-invoice from e71f00455f to 0ef473eb91 2023-04-20 17:30:01 +02:00 Compare
nroche commented 2023-04-20 17:33:03 +02:00
Author
Owner
Copy Link

Déplace plutôt ça dans un autre ticket,

https://dev.entrouvert.org/issues/76836

> Déplace plutôt ça dans un autre ticket, https://dev.entrouvert.org/issues/76836
bdauvergne approved these changes 2023-04-21 00:05:32 +02:00
bdauvergne left a comment
Owner
Copy Link

Je valide parce que la mécanique est ok mais il faudra revoir plusieurs points.

  • les nettoyages devront avoir lieu plus souvent possible, sans passer par un cron plus fréquent qu'à l'heure (le faire un peu tout le temps pendant les requêtes par exemple de lecture du panier)

  • une facture ne doit pas être payée/payable alors qu'elle va bientôt être annulé, coté combo/lingo on a le support au mieux d'une date de paiement, rien de plus précis, mais par contre on est assuré qu'avant de payer lingo fasse un get_invoice() (il vérifie que la facture existe), un paiement peut prendre jusqu'à 20 minutes, il faudrait avoir le timing suivant :

** les factures sont visibles entre le moment de validation du panier T et T + cancel_invoice_delay, en dehors de cette période ni get_invoices() ni get_invoice() ne doivent renvoyer ces factures,

** on ne doit pas annuler une facture en train d'être payé, donc on ne devrait pas les annuler avant T+30+20

Ça pourrait être intéressant d'avoir coté lingo une notification du démarrage d'un paiement, ça permettrait de ficeler tout ça un peu mieux.

Je valide parce que la mécanique est ok mais il faudra revoir plusieurs points. * les nettoyages devront avoir lieu plus souvent possible, sans passer par un cron plus fréquent qu'à l'heure (le faire un peu tout le temps pendant les requêtes par exemple de lecture du panier) * une facture ne doit pas être payée/payable alors qu'elle va bientôt être annulé, coté combo/lingo on a le support au mieux d'une date de paiement, rien de plus précis, mais par contre on est assuré qu'avant de payer lingo fasse un get_invoice() (il vérifie que la facture existe), un paiement peut prendre jusqu'à 20 minutes, il faudrait avoir le timing suivant : ** les factures sont visibles entre le moment de validation du panier T et T + cancel_invoice_delay, en dehors de cette période ni get_invoices() ni get_invoice() ne doivent renvoyer ces factures, ** on ne doit pas annuler une facture en train d'être payé, donc on ne devrait pas les annuler avant T+30+20 Ça pourrait être intéressant d'avoir coté lingo une notification du démarrage d'un paiement, ça permettrait de ficeler tout ça un peu mieux.
passerelle/contrib/toulouse_maelis/models.py Outdated
@ -287,8 +291,19 @@ class ToulouseMaelis(BaseResource, HTTPResource):
for invoice in invoices:
invoice.notify()
def cancel_basket_invoices(self):
bdauvergne commented 2023-04-20 23:54:30 +02:00
Owner
Copy Link

Je mettrai un @atomic et un select_for_update(), pendant qu'on fait ça on a pas trop envie qu'il se passe autre chose.

Je mettrai un @atomic et un select_for_update(), pendant qu'on fait ça on a pas trop envie qu'il se passe autre chose.
passerelle/contrib/toulouse_maelis/models.py Outdated
@ -290,2 +303,4 @@
def hourly(self):
self.notify_invoices_paid()
self.cancel_basket_invoices()
bdauvergne commented 2023-04-20 23:55:20 +02:00
Owner
Copy Link

Un léger doute sur le fait d'avoir un délai par défaut de 30 minutes et un cron toutes les heures :)

Un léger doute sur le fait d'avoir un délai par défaut de 30 minutes et un cron toutes les heures :)
bdauvergne commented 2023-04-21 00:35:24 +02:00
Owner
Copy Link
  • Cacher cette facture à Lingo si elle n'a pas encore été payé avant que le délai n'ait expiré.

Je ne vois pas cette partie dans le code, je vois juste le nettoyage quand le délai est expiré et que la facture n'a été payée par ailleurs, alors j'ai ouvert un ticket sur le sujet, #76854 .

> * Cacher cette facture à Lingo si elle n'a pas encore été payé avant que le délai n'ait expiré. Je ne vois pas cette partie dans le code, je vois juste le nettoyage quand le délai est expiré et que la facture n'a été payée par ailleurs, alors j'ai ouvert un ticket sur le sujet, #76854 .
bdauvergne commented 2023-04-21 00:41:59 +02:00
Owner
Copy Link
  • Cacher cette facture à Lingo si elle n'a pas encore été payé avant que le délai n'ait expiré.

Je ne vois pas cette partie dans le code, je vois juste le nettoyage quand le délai est expiré et que la facture n'a été payée par ailleurs, alors j'ai ouvert un ticket sur le sujet, #76854 .

Ok vu, tu as changé le résultat de Invoice.status() et invoices() ne retourne que les factures "created", mais ce ne sera pas suffisant il faut aussi qu'elle ne remonte pas par invoice() (sans "s") pour ne plus être payable.

> > * Cacher cette facture à Lingo si elle n'a pas encore été payé avant que le délai n'ait expiré. > > Je ne vois pas cette partie dans le code, je vois juste le nettoyage quand le délai est expiré et que la facture n'a été payée par ailleurs, alors j'ai ouvert un ticket sur le sujet, #76854 . Ok vu, tu as changé le résultat de Invoice.status() et invoices() ne retourne que les factures "created", mais ce ne sera pas suffisant il faut aussi qu'elle ne remonte pas par invoice() (sans "s") pour ne plus être payable.
bdauvergne requested changes 2023-04-21 00:42:33 +02:00
bdauvergne left a comment
Owner
Copy Link

Ajouter un test sur i.status() != 'cancelled" dans invoice().

Ajouter un test sur i.status() != 'cancelled" dans invoice().
nroche force-pushed wip/76395-parsifal-cancel-invoice from 0ef473eb91 to c45e5ae358 2023-04-21 12:09:12 +02:00 Compare
bdauvergne approved these changes 2023-04-21 12:24:04 +02:00
bdauvergne left a comment
Owner
Copy Link

C'est tout bon, j'ai rejeté les tickets que j'avais pu créer où le travail est déjà fait ici.

C'est tout bon, j'ai rejeté les tickets que j'avais pu créer où le travail est déjà fait ici.
nroche commented 2023-04-21 13:10:45 +02:00
Author
Owner
Copy Link

Ah merci beaucoup !
(du coup je pose les réponses que j'avais préparées en bloc ci-dessous)

---8<---

il faudrait avoir le timing suivant :
** les factures sont visibles entre le moment de validation du
panier T et T + cancel_invoice_delay, en dehors de cette période ni
get_invoices() ni get_invoice() ne doivent pas renvoyer ces factures,

Il y a effectivement 2 trous dans la raquette, merci :

  • je ne devrais pas renvoyer une facture annulée sur /invoice/
  • je dois permettre de payer une facture pour laquelle on n'a pas encore envoyé l'ordre d'annulation à maélis.

Sinon, oui et je complète les tests pour que ce soit plus explicite.
(Mais je n'arrive pas à écrire de test qui montre qu'une facture
annulée n'est plus affichée dans l'historique, parce que j'ai tout mis
en oeuvre dans le code pour que l'on ne puisse pas annuler une facture
déjà payée ; c'est le test test_cancel_basket_invoice_cron_keep_paid_invoices)

** on ne doit pas annuler une facture en train d'être payé, donc on ne devrait pas les annuler avant T+30+20

J'avais mis T+30*2, j'ai changé pour T+30+20.

Ça pourrait être intéressant d'avoir coté lingo une notification du démarrage d'un paiement, ça permettrait de ficeler tout ça un peu mieux.

Oui, à un moment je pensais pouvoir utiliser le Webservice de calcul des frais additionnels pour ça (mais non).

Ok vu, tu as changé le résultat de Invoice.status() et invoices() ne retourne que les factures "created", mais ce ne sera pas suffisant il faut aussi qu'elle ne remonte pas par invoice() (sans "s") pour ne plus être payable.

Oui, trou dans la raquette évoqué ci-dessus, merci.
Mais comme j'utilise invoice() dans le endpoint pay_invoice et que ce
dernier doit permettre de payer les factures qui ne sont plus
affichées mais pas encore annulée sur maélis, je préfère corriger sur
les 2 appels à invoice().

Je mettrai un @atomic et un select_for_update(), pendant qu'on fait
ça on a pas trop envie qu'il se passe autre chose.

(fait)

les nettoyages devront avoir lieu plus souvent possible, sans passer par un cron plus fréquent qu'à l'heure (le faire un peu tout le temps pendant les requêtes par exemple de lecture du panier)

Oui, j'ai fait ça, c'est malin (et ça m'arrange pour #76398).

Ah merci beaucoup ! (du coup je pose les réponses que j'avais préparées en bloc ci-dessous) ---8<--- > il faudrait avoir le timing suivant : > ** les factures sont visibles entre le moment de validation du panier T et T + cancel_invoice_delay, en dehors de cette période ni get_invoices() ni get_invoice() ne doivent pas renvoyer ces factures, Il y a effectivement 2 trous dans la raquette, merci : * je ne devrais pas renvoyer une facture annulée sur /invoice/ * je dois permettre de payer une facture pour laquelle on n'a pas encore envoyé l'ordre d'annulation à maélis. Sinon, oui et je complète les tests pour que ce soit plus explicite. (Mais je n'arrive pas à écrire de test qui montre qu'une facture annulée n'est plus affichée dans l'historique, parce que j'ai tout mis en oeuvre dans le code pour que l'on ne puisse pas annuler une facture déjà payée ; c'est le test test_cancel_basket_invoice_cron_keep_paid_invoices) > ** on ne doit pas annuler une facture en train d'être payé, donc on ne devrait pas les annuler avant T+30+20 J'avais mis T+30*2, j'ai changé pour T+30+20. > Ça pourrait être intéressant d'avoir coté lingo une notification du démarrage d'un paiement, ça permettrait de ficeler tout ça un peu mieux. Oui, à un moment je pensais pouvoir utiliser le Webservice de calcul des frais additionnels pour ça (mais non). > Ok vu, tu as changé le résultat de Invoice.status() et invoices() ne retourne que les factures "created", mais ce ne sera pas suffisant il faut aussi qu'elle ne remonte pas par invoice() (sans "s") pour ne plus être payable. Oui, trou dans la raquette évoqué ci-dessus, merci. Mais comme j'utilise invoice() dans le endpoint pay_invoice et que ce dernier doit permettre de payer les factures qui ne sont plus affichées mais pas encore annulée sur maélis, je préfère corriger sur les 2 appels à invoice(). > Je mettrai un @atomic et un select_for_update(), pendant qu'on fait ça on a pas trop envie qu'il se passe autre chose. (fait) > les nettoyages devront avoir lieu plus souvent possible, sans passer par un cron plus fréquent qu'à l'heure (le faire un peu tout le temps pendant les requêtes par exemple de lecture du panier) Oui, j'ai fait ça, c'est malin (et ça m'arrange pour #76398).
bdauvergne commented 2023-04-21 13:49:42 +02:00
Owner
Copy Link

Sinon, oui et je complète les tests pour que ce soit plus explicite.
(Mais je n'arrive pas à écrire de test qui montre qu'une facture
annulée n'est plus affichée dans l'historique, parce que j'ai tout mis
en oeuvre dans le code pour que l'on ne puisse pas annuler une facture
déjà payée ; c'est le test test_cancel_basket_invoice_cron_keep_paid_invoices)

Je n'ai pas compris ce que tu n'arrives pas à faire. Ne la paye pas, bouge freezer de 30 minutes, observe que la facture n'est plus visibles ni sur /invoices/ ni sur /invoice/ mais que /pay-invoice/ fonctionne encore.

** on ne doit pas annuler une facture en train d'être payé, donc on ne devrait pas les annuler avant T+30+20

J'avais mis T+30*2, j'ai changé pour T+30+20.

J'avais raté le *2, ok donc dans #76855 il ne restera plus qu'à remplacer le + 20 par une référence à un nouveau champ.

Ça pourrait être intéressant d'avoir coté lingo une notification du démarrage d'un paiement, ça permettrait de ficeler tout ça un peu mieux.

Oui, à un moment je pensais pouvoir utiliser le Webservice de calcul des frais additionnels pour ça (mais non).

Ouais bof non, c'est une verrue pour IMIO je préfère qu'on ne lui redonne pas une utilité.

Ok vu, tu as changé le résultat de Invoice.status() et invoices() ne retourne que les factures "created", mais ce ne sera pas suffisant il faut aussi qu'elle ne remonte pas par invoice() (sans "s") pour ne plus être payable.

Oui, trou dans la raquette évoqué ci-dessus, merci.
Mais comme j'utilise invoice() dans le endpoint pay_invoice et que ce
dernier doit permettre de payer les factures qui ne sont plus
affichées mais pas encore annulée sur maélis, je préfère corriger sur
les 2 appels à invoice().

Je crois que tu voulais dire que tu préfère ajouter le check dans invoice() et pay_invoice() plutôt que dans get_invoice(). C'est ok.

> Sinon, oui et je complète les tests pour que ce soit plus explicite. > (Mais je n'arrive pas à écrire de test qui montre qu'une facture > annulée n'est plus affichée dans l'historique, parce que j'ai tout mis > en oeuvre dans le code pour que l'on ne puisse pas annuler une facture > déjà payée ; c'est le test test_cancel_basket_invoice_cron_keep_paid_invoices) Je n'ai pas compris ce que tu n'arrives pas à faire. Ne la paye pas, bouge freezer de 30 minutes, observe que la facture n'est plus visibles ni sur /invoices/ ni sur /invoice/ mais que /pay-invoice/ fonctionne encore. > > ** on ne doit pas annuler une facture en train d'être payé, donc on ne devrait pas les annuler avant T+30+20 > > J'avais mis T+30*2, j'ai changé pour T+30+20. J'avais raté le *2, ok donc dans #76855 il ne restera plus qu'à remplacer le + 20 par une référence à un nouveau champ. > > Ça pourrait être intéressant d'avoir coté lingo une notification du démarrage d'un paiement, ça permettrait de ficeler tout ça un peu mieux. > > Oui, à un moment je pensais pouvoir utiliser le Webservice de calcul des frais additionnels pour ça (mais non). Ouais bof non, c'est une verrue pour IMIO je préfère qu'on ne lui redonne pas une utilité. > > Ok vu, tu as changé le résultat de Invoice.status() et invoices() ne retourne que les factures "created", mais ce ne sera pas suffisant il faut aussi qu'elle ne remonte pas par invoice() (sans "s") pour ne plus être payable. > > Oui, trou dans la raquette évoqué ci-dessus, merci. > Mais comme j'utilise invoice() dans le endpoint pay_invoice et que ce > dernier doit permettre de payer les factures qui ne sont plus > affichées mais pas encore annulée sur maélis, je préfère corriger sur > les 2 appels à invoice(). Je crois que tu voulais dire que tu préfère ajouter le check dans invoice() et pay_invoice() plutôt que dans get_invoice(). C'est ok.
nroche force-pushed wip/76395-parsifal-cancel-invoice from c45e5ae358 to c7d33287c5 2023-04-21 13:55:01 +02:00 Compare
nroche commented 2023-04-21 14:09:16 +02:00
Author
Owner
Copy Link

Ne la paye pas, bouge freezer de 30 minutes, observe que la facture n'est plus visibles ni sur /invoices/ ni sur /invoice/ mais que /pay-invoice/ fonctionne encore.

Oui (c'est ce que j'ai ajouté).

> Ne la paye pas, bouge freezer de 30 minutes, observe que la facture n'est plus visibles ni sur /invoices/ ni sur /invoice/ mais que /pay-invoice/ fonctionne encore. Oui (c'est ce que j'ai ajouté).
nroche merged commit c7d33287c5 into main 2023-04-21 14:10:00 +02:00
nroche deleted branch wip/76395-parsifal-cancel-invoice 2023-04-21 14:10:00 +02:00
bdauvergne commented 2023-04-21 14:32:34 +02:00
Owner
Copy Link

Ne la paye pas, bouge freezer de 30 minutes, observe que la facture n'est plus visibles ni sur /invoices/ ni sur /invoice/ mais que /pay-invoice/ fonctionne encore.

Oui (c'est ce que j'ai ajouté).

Mais donc que'est-ce que tu n'arrives pas à faire ?

> > Ne la paye pas, bouge freezer de 30 minutes, observe que la facture n'est plus visibles ni sur /invoices/ ni sur /invoice/ mais que /pay-invoice/ fonctionne encore. > > Oui (c'est ce que j'ai ajouté). Mais donc que'est-ce que tu n'arrives pas à faire ?
nroche commented 2023-04-21 16:42:54 +02:00
Author
Owner
Copy Link

Mais donc que'est-ce que tu n'arrives pas à faire ?

Tu disais que :

les factures sont visibles entre le moment de validation du
panier T et T + cancel_invoice_delay, en dehors de cette période ni
get_invoices() ni get_invoice() ne doivent pas renvoyer ces factures

Et j'ai interprété qu'il manquait un test pour mettre en évidence que les factures annulées ne seraient pas non plus affichées dans l'historique des factures.
Mais en fait ça n'a aucun sens parce qu'une facture annulée ne peut pas être payée et une facture payée ne peut pas être annulée.
Donc oui, juste ajouter des tests sur les factures en cours, ce que j'ai fait.

> Mais donc que'est-ce que tu n'arrives pas à faire ? Tu disais que : > les factures sont visibles entre le moment de validation du > panier T et T + cancel_invoice_delay, en dehors de cette période ni > get_invoices() ni get_invoice() ne doivent pas renvoyer ces factures Et j'ai interprété qu'il manquait un test pour mettre en évidence que les factures annulées ne seraient pas non plus affichées dans l'historique des factures. Mais en fait ça n'a aucun sens parce qu'une facture annulée ne peut pas être payée et une facture payée ne peut pas être annulée. Donc oui, juste ajouter des tests sur les factures en cours, ce que j'ai fait.
Sign in to join this conversation.
Reviewers
No reviewers
bdauvergne
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/passerelle#224
No description provided.
Delete Branch "wip/76395-parsifal-cancel-invoice"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?