toulouse-maelis: add a for_payment parameter to reduce cancellation delay (#76856) #227

Merged
nroche merged 2 commits from wip/76856-parsifal-param-for-invoice-cancel-delay into main 2023-04-28 07:53:53 +02:00
Owner

Sur réception de ce paramètre on update Invoice.start_payment_date à now(), ça permettra de réduire le délai avant annulation à min(Invoice.created + cancel_invoice_delay, Invoice.start_payment_date + max_payment_delay).

Sur réception de ce paramètre on update Invoice.start_payment_date à now(), ça permettra de réduire le délai avant annulation à min(Invoice.created + cancel_invoice_delay, Invoice.start_payment_date + max_payment_delay).
Author
Owner

Je note ici qu'un effet de bord de l'implémentation est qu'après un appel à /invoice/?for_payment la facture ne sera plus affiché à l'usager.
Autrement dit, il ne la verra plus s'il revient de Lingo sans l'avoir payée et devra recommencer à zéro la démarche d'inscription s'il (re-)change à nouveau d'avis.
(Je ne sais pas si le parcours était imaginé ainsi ou non.)

Je n'ai pas cherché à optimiser la requête du cron (en vrai je ne saurais pas faire).

Je note ici qu'un effet de bord de l'implémentation est qu'après un appel à /invoice/?for_payment la facture ne sera plus affiché à l'usager. Autrement dit, il ne la verra plus s'il revient de Lingo sans l'avoir payée et devra recommencer à zéro la démarche d'inscription s'il (re-)change à nouveau d'avis. (Je ne sais pas si le parcours était imaginé ainsi ou non.) Je n'ai pas cherché à optimiser la requête du cron (en vrai je ne saurais pas faire).
Owner

Je note ici qu'un effet de bord de l'implémentation est qu'après un appel à /invoice/?for_payment la facture ne sera plus affiché à l'usager.
Autrement dit, il ne la verra plus s'il revient de Lingo sans l'avoir payée et devra recommencer à zéro la démarche d'inscription s'il (re-)change à nouveau d'avis.
(Je ne sais pas si le parcours était imaginé ainsi ou non.)

Je n'ai pas cherché à optimiser la requête du cron (en vrai je ne saurais pas faire).

Je ne vois pas pourquoi ça ferait ça, tu peux expliquer ? Il peut y avoir un bloquage des factures pendant leur paiement pour éviter les doubles transactions mais elle est toujours visible.

> Je note ici qu'un effet de bord de l'implémentation est qu'après un appel à /invoice/?for_payment la facture ne sera plus affiché à l'usager. > Autrement dit, il ne la verra plus s'il revient de Lingo sans l'avoir payée et devra recommencer à zéro la démarche d'inscription s'il (re-)change à nouveau d'avis. > (Je ne sais pas si le parcours était imaginé ainsi ou non.) > > Je n'ai pas cherché à optimiser la requête du cron (en vrai je ne saurais pas faire). Je ne vois pas pourquoi ça ferait ça, tu peux expliquer ? Il peut y avoir un bloquage des factures pendant leur paiement pour éviter les doubles transactions mais elle est toujours visible.
Author
Owner

Ce patch a pour but de réduire le temps d'attente d'envoie de l'ordre d'annulation de la facture en supprimant le delai d'attente cancel_invoice_delay.

Sur invoices/ et /invoice il ne faut plus remonter les factures dès lors qu'elle sont sur le point d'être annulées (cancel_invoice_delay dépassé) dans max_payment_delay minutes.

Ici on a bien une facture qui est sur le point d'être annulée dans max_payment_delay minutes.
Ce serait contradictoire à mon avis de toujours l'afficher, parce qu'ensuite l'usager devrait la payer en moins de max_payment_delay minutes.

S'il faut encore afficher la facture pendant le paiement, alors

  • je ne l'affiche que si /?for_payment est à nouveau passé ?
  • et je remet le compteur (start_payment_date) à now ?
Ce patch a pour but de réduire le temps d'attente d'envoie de l'ordre d'annulation de la facture en supprimant le delai d'attente cancel_invoice_delay. Sur invoices/ et /invoice il ne faut plus remonter les factures dès lors qu'elle sont sur le point d'être annulées (cancel_invoice_delay dépassé) dans max_payment_delay minutes. Ici on a bien une facture qui est sur le point d'être annulée dans max_payment_delay minutes. Ce serait contradictoire à mon avis de toujours l'afficher, parce qu'ensuite l'usager devrait la payer en moins de max_payment_delay minutes. S'il faut encore afficher la facture pendant le paiement, alors * je ne l'affiche que si /?for_payment est à nouveau passé ? * et je remet le compteur (start_payment_date) à now ?
Owner

S'il faut encore afficher la facture pendant le paiement, alors

  • je ne l'affiche que si /?for_payment est à nouveau passé ?
  • et je remet le compteur (start_payment_date) à now ?

Je dirai qu'effectivement c'est un peu bizarre de cacher la facture pendant qu'on la paie, mais les délais sont déjà bien long. Les pages de paiement ne laisse pas 20 minutes aux gens pour valider leur paiement, c'est plus 5 minutes. 20 minutes c'est la borne haute pour recevoir la notification. Je dirai que après T+cancel_invoice_delay si on a une valeur start_payment_date, on affiche la facture avec un statut non payable en cours de paiement/expiration.

L'alternative ce serait d'avoir un cancel_invoice_delay très court mais qu'on prolonge de max_payment_delay à chaque fois qu'une personne démarre un paiement.

> S'il faut encore afficher la facture pendant le paiement, alors > * je ne l'affiche que si /?for_payment est à nouveau passé ? > * et je remet le compteur (start_payment_date) à now ? > Je dirai qu'effectivement c'est un peu bizarre de cacher la facture pendant qu'on la paie, mais les délais sont déjà bien long. Les pages de paiement ne laisse pas 20 minutes aux gens pour valider leur paiement, c'est plus 5 minutes. 20 minutes c'est la borne haute pour recevoir la notification. Je dirai que après T+cancel_invoice_delay si on a une valeur start_payment_date, on affiche la facture avec un statut non payable en cours de paiement/expiration. L'alternative ce serait d'avoir un cancel_invoice_delay très court mais qu'on prolonge de max_payment_delay à chaque fois qu'une personne démarre un paiement.
Owner

Je propose ne pas cherche trop la règle parfaite, on a les deux paramètres qu'il nous faut max_payment_delay et cancel_invoice_delay, on pourra toujours ajuste les règles.

Je propose ne pas cherche trop la règle parfaite, on a les deux paramètres qu'il nous faut max_payment_delay et cancel_invoice_delay, on pourra toujours ajuste les règles.
bdauvergne approved these changes 2023-04-24 11:00:34 +02:00
@ -301,4 +301,2 @@
basket_generation_date__isnull=False,
maelis_cancel_notification_date__isnull=True,
created__lte=now()
- datetime.timedelta(minutes=(self.cancel_invoice_delay + self.max_payment_delay)),
Owner

Garde created__lte=now() - datetime.timedelta(minutes=self.cancel_invoice_delay) mais à ajoute :

  .filter(Q(start_payment_date__isnull=True) | Q(start_payment_date__lte=now() -  datetime.timedelta(minutes=self.max_payment_delay))

faut prendre le max (my bad) pas le min. Désolé.

Garde `created__lte=now() - datetime.timedelta(minutes=self.cancel_invoice_delay)` mais à ajoute : ``` .filter(Q(start_payment_date__isnull=True) | Q(start_payment_date__lte=now() - datetime.timedelta(minutes=self.max_payment_delay)) ``` faut prendre le max (my bad) pas le min. Désolé.
Author
Owner

faut prendre le max (my bad) pas le min. Désolé.

oui (si j'ai bien interprété), donc on change rien ici.
---8<---

Actuellement on a :

  • cancel_invoice_delay=30 : Délai de conservation des factures issues d'un panier
  • max_payment_delay=20 : Délai maximum pour payer une facture via Lingo
  • à T la fature est crée
  • à T+30, la facture n'est plus affichée dans Lingo, mais reste payable
  • à T+30+20, on envoie l'ordre d'annulation de la facture à Maélis

Le but de ce ticket c'est :

  • à T'>T, on commence le paiement (/invoice/xxx/?for_payment)
  • à T'+20, on envoie l'ordre d'annulation de la facture à Maélis

Ce qui est résumé dans la description du ticket par :
delais avant annulation = min(T+30, T'+20)

Si l'on réduit le delais avant annulation (pas d'affichage lingo) à min(T+30, T'+20),
alors on doit annuler (ordre qui est passé à maélis) les factures qui sont dans ces deux cas (OU logique)
soit (T <= t - (30+20) ) OU (T' <= t - 20).

> faut prendre le max (my bad) pas le min. Désolé. oui (si j'ai bien interprété), donc on change rien ici. ---8<--- Actuellement on a : * cancel_invoice_delay=30 : Délai de conservation des factures issues d'un panier * max_payment_delay=20 : Délai maximum pour payer une facture via Lingo * à T la fature est crée * à T+30, la facture n'est plus affichée dans Lingo, mais reste payable * à T+30+20, on envoie l'ordre d'annulation de la facture à Maélis Le but de ce ticket c'est : * à T'>T, on commence le paiement (/invoice/xxx/?for_payment) * à T'+20, on envoie l'ordre d'annulation de la facture à Maélis Ce qui est résumé dans la description du ticket par : delais avant annulation = min(T+30, T'+20) Si l'on réduit le delais avant annulation (pas d'affichage lingo) à min(T+30, T'+20), alors on doit annuler (ordre qui est passé à maélis) les factures qui sont dans ces deux cas (OU logique) soit (T <= t - (30+20) ) OU (T' <= t - 20).
@ -305,3 +303,3 @@
)
for invoice in invoices:
invoice.cancel()
if (
Owner

La condition ne sera plus nécessaire.

La condition ne sera plus nécessaire.
Author
Owner

Merci de m'avoir donné la formule (que j'avais demandé), je la comprends, mais en fait je ne souhaite pas optimiser dès à présent (pour moi c'est encore trop embrouillé).

Merci de m'avoir donné la formule (que j'avais demandé), je la comprends, mais en fait je ne souhaite pas optimiser dès à présent (pour moi c'est encore trop embrouillé).
@ -4044,3 +4052,3 @@
if invoice.status() == 'cancelled':
raise APIError('Invoice cancelled')
return {'data': invoice.format_content()}
if invoice.basket_generation_date and for_payment is not None:
Owner

Je pense que tu peux mettre à jour start_payment_date sur n'importe quel type de facture ça ne mange pas de pain et ça nous donne une information dans tous les cas.

Je pense que tu peux mettre à jour start_payment_date sur n'importe quel type de facture ça ne mange pas de pain et ça nous donne une information dans tous les cas.
Author
Owner

Fait.

Fait.
@ -4047,0 +4055,4 @@
invoice.start_payment_date = now()
invoice.save()
return {
'has_invoice_for_payment': bool(invoice.start_payment_date is not None),
Owner

Non c'est sur invoices qu'il faut ajouter has_invoice_for_payment et c'est toujours True.

Non c'est sur invoices qu'il faut ajouter has_invoice_for_payment et c'est toujours True.
Owner

Ce point est important, sans ça ça ne marchera pas.

Ce point est important, sans ça ça ne marchera pas.
Author
Owner

Oups, j'ai compris tout de travers.
Il s'agit ici d'ajouter une indication comme quoi le connecteur gère bien le paramètre '?for_payment'.

J'ai corrigé : je le renvoie toujours à True sur /invoies et il n'est pas renvoyé sur invoices/history.

Oups, j'ai compris tout de travers. Il s'agit ici d'ajouter une indication comme quoi le connecteur gère bien le paramètre '?for_payment'. J'ai corrigé : je le renvoie toujours à True sur `/invoies` et il n'est pas renvoyé sur `invoices/history`.
Author
Owner

Je dirai qu'effectivement c'est un peu bizarre de cacher la facture
pendant qu'on la paie

J'avais mal interprété la description du tiket.
Pour moi on réduisait le temps d'envoie de l'ordre d'annulation
en supprimant le premier temps où la facture n'est plus affichée.
Je réalise qu'en fait le second scénario s'insère de manière plus
complexe : réduire le temps d'envoie de l'ordre d'annulation
en ayant la facture toujours affichée mais pas payable.

Je dirai que après T+cancel_invoice_delay
si on a une valeur start_payment_date,
on affiche la facture avec un statut non payable en cours de paiement/expiration.

Ok, j'ajoute des états pour détailler l'état "annulé" :

  • cancelling : n'est plus affichée dans lingo (T+30)
  • for_payment: est encore affiché dans Lingo mais n'est plus proposé au paiement
  • cancelled : ordre d'annulation envoyé à maélis

L'alternative ce serait d'avoir un cancel_invoice_delay
très court mais qu'on prolonge de max_payment_delay à chaque fois
qu'une personne démarre un paiement.

Bof, là tu as introduis un future second scénario (avec ?for_payment) donc
je serais d'avis de conserver le premier en l'état (sans ?for_payment).

J'ai ajouté 3 commits pour faciliter la relecture, que je pense squasher ensuite.

> Je dirai qu'effectivement c'est un peu bizarre de cacher la facture > pendant qu'on la paie J'avais mal interprété la description du tiket. Pour moi on réduisait le temps d'envoie de l'ordre d'annulation en supprimant le premier temps où la facture n'est plus affichée. Je réalise qu'en fait le second scénario s'insère de manière plus complexe : réduire le temps d'envoie de l'ordre d'annulation en ayant la facture toujours affichée mais pas payable. > Je dirai que après T+cancel_invoice_delay > si on a une valeur start_payment_date, > on affiche la facture avec un statut non payable en cours de paiement/expiration. Ok, j'ajoute des états pour détailler l'état "annulé" : * cancelling : n'est plus affichée dans lingo (T+30) * for_payment: est encore affiché dans Lingo mais n'est plus proposé au paiement * cancelled : ordre d'annulation envoyé à maélis > L'alternative ce serait d'avoir un cancel_invoice_delay > très court mais qu'on prolonge de max_payment_delay à chaque fois > qu'une personne démarre un paiement. Bof, là tu as introduis un future second scénario (avec ?for_payment) donc je serais d'avis de conserver le premier en l'état (sans ?for_payment). J'ai ajouté 3 commits pour faciliter la relecture, que je pense squasher ensuite.
nroche force-pushed wip/76856-parsifal-param-for-invoice-cancel-delay from 2716b72748 to 06b640731f 2023-04-26 11:44:33 +02:00 Compare
nroche merged commit 06b640731f into main 2023-04-28 07:53:53 +02:00
nroche deleted branch wip/76856-parsifal-param-for-invoice-cancel-delay 2023-04-28 07:53:53 +02:00
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No Assignees
2 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#227
No description provided.