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

toulouse-maelis: notification asynchrone du paiement des factures (#76394) #182

Merged
nroche merged 7 commits from wip/76394-parsifal-pay-invoice-async into main 2023-04-14 17:13:12 +02:00
Conversation 2 Commits 7 Files Changed 4 +401 -73
nroche commented 2023-04-08 03:59:05 +02:00
Owner
Copy Link

Enregistrer les paiement notifiés par Lingo en base.
Appeler de façon asynchrone le WS payInvoices pour palier à des erreurs réseau.
Renvoyer à Lingo la liste des factures à payer en tenant compte que Maélis n'a peut-être pas reçu/traité la notification.

Enregistrer les paiement notifiés par Lingo en base. Appeler de façon asynchrone le WS payInvoices pour palier à des erreurs réseau. Renvoyer à Lingo la liste des factures à payer en tenant compte que Maélis n'a peut-être pas reçu/traité la notification.
fpeters requested changes 2023-04-09 19:39:05 +02:00
fpeters left a comment
Owner
Copy Link

(ça n'est pas encore une relecture totalement attentive mais c'est déjà ça)

(ça n'est pas encore une relecture totalement attentive mais c'est déjà ça)
passerelle/contrib/toulouse_maelis/models.py Outdated
@ -3783,0 +3786,4 @@
invoice.family_id = family_id
invoice.save()
else:
if invoice.regie_id != regie_id:
fpeters commented 2023-04-09 19:33:33 +02:00
Owner
Copy Link

En cas de mise à jour il ne faut pas mettre updated à jour ?

En cas de mise à jour il ne faut pas mettre updated à jour ?
nroche commented 2023-04-12 20:24:25 +02:00
Author
Owner
Copy Link

Le code a changé ici. Je teste ça dans le nouveau code.

Le code a changé ici. Je teste ça dans le nouveau code.
passerelle/contrib/toulouse_maelis/models.py Outdated
@ -3783,0 +3789,4 @@
if invoice.regie_id != regie_id:
raise APIError(
"invoice '%s' already exist on '%s' regie"
% (invoice.invoice_id, invoice.regie_id)
fpeters commented 2023-04-09 19:30:15 +02:00
Owner
Copy Link

Il me semble que des régies différentes pourraient émettre des factures avec le même numéro, que l'unicité ici devrait être sur (regie_id, invoice_id).

Il me semble que des régies différentes pourraient émettre des factures avec le même numéro, que l'unicité ici devrait être sur (regie_id, invoice_id).
bdauvergne commented 2023-04-12 10:31:21 +02:00
Owner
Copy Link

À priori non, numInvoice est une clé primaire de la table des factures, mais on devrait leur demander.

À priori non, numInvoice est une clé primaire de la table des factures, mais on devrait leur demander.
nroche commented 2023-04-12 10:40:33 +02:00
Author
Owner
Copy Link

Je pensais comme Benjamin, mais non, l'identifiant de facture est unique pour 1 régie donnée.
https://redmine.sigec.fr/issues/1573

Je pensais comme Benjamin, mais non, l'identifiant de facture est unique pour 1 régie donnée. https://redmine.sigec.fr/issues/1573
bdauvergne commented 2023-04-14 12:57:55 +02:00
Owner
Copy Link

Au final c'est bien globalement unique https://redmine.sigec.fr/issues/1573#note-21 , le contraire m'aurait étonné parc que c'est pas évident de générer des id de table non unique par rapport à une foreign key (ça demande un trigger au minimum).

C'est juste que la SIGEC ne connait pas ses propres web-service. Des fois j'ai l'impression qu'ils répondent au pif.

Au final c'est bien globalement unique https://redmine.sigec.fr/issues/1573#note-21 , le contraire m'aurait étonné parc que c'est pas évident de générer des id de table non unique par rapport à une foreign key (ça demande un trigger au minimum). C'est juste que la SIGEC ne connait pas ses propres web-service. Des fois j'ai l'impression qu'ils répondent au pif.
passerelle/contrib/toulouse_maelis/models.py Outdated
@ -3793,2 +3812,2 @@
'amount': str(float(item['amountInvoice']) - float(item['amountPaid'])),
'amount_paid': item['amountPaid'],
'total_amount': amount_invoice,
'amount': str(float(amount_invoice) - float(amount_paid)),
fpeters commented 2023-04-09 19:23:07 +02:00
Owner
Copy Link

Il ne faut jamais manipuler de l'argent avec float(). (il faut utiliser Decimal)

Il ne faut jamais manipuler de l'argent avec float(). (il faut utiliser Decimal)
nroche commented 2023-04-12 20:25:46 +02:00
Author
Owner
Copy Link

Fait.

Fait.
passerelle/contrib/toulouse_maelis/models.py Outdated
@ -3903,0 +3919,4 @@
invoice_obj.save(update_fields=['updated', 'lingo_data'])
if not invoice_obj.notify(timeout=10):
self.add_job(
'notify_invoice_paid_job',
fpeters commented 2023-04-09 19:25:06 +02:00
Owner
Copy Link

Ça me semble une complexité superflue, et 10 secondes de perdues, on peut juste tout le temps créer le job asynchrone.

Plus globalement, je serais ici pour 1/ faire le job asynchrone, puis 2/ si dans le job asynchrone ça échoue, tant pis, 3/ avoir un job hourly qui prenne les factures en attente depuis plus de 15 minutes et qui essaie de les notifier. (histoire de ne pas empiler des jobs lors d'une indispo Maélis).

(et plus tard on pourrait aussi avoir l'info des factures qui sont en attente de notification depuis plus de N heures (ou jours?), ce qui révélerait une situation anormale côté Maélis)

Ça me semble une complexité superflue, et 10 secondes de perdues, on peut juste tout le temps créer le job asynchrone. Plus globalement, je serais ici pour 1/ faire le job asynchrone, puis 2/ si dans le job asynchrone ça échoue, tant pis, 3/ avoir un job hourly qui prenne les factures en attente depuis plus de 15 minutes et qui essaie de les notifier. (histoire de ne pas empiler des jobs lors d'une indispo Maélis). (et *plus tard* on pourrait aussi avoir l'info des factures qui sont en attente de notification depuis plus de N heures (ou jours?), ce qui révélerait une situation anormale côté Maélis)
nroche commented 2023-04-12 20:26:14 +02:00
Author
Owner
Copy Link

Fait.

Fait.
passerelle/contrib/toulouse_maelis/models.py Outdated
@ -3999,0 +4028,4 @@
return 'notified'
if self.lingo_data:
return 'paid'
if self.canceled:
fpeters commented 2023-04-09 19:27:07 +02:00
Owner
Copy Link

s/canceled/cancelled/

s/canceled/cancelled/
nroche commented 2023-04-12 20:26:23 +02:00
Author
Owner
Copy Link

Fait.

Fait.
bdauvergne marked this conversation as resolved
passerelle/contrib/toulouse_maelis/models.py Outdated
@ -3999,0 +4039,4 @@
'payInvoices',
numDossier=self.maelis_data['numFamily'],
codeRegie=self.regie_id,
amount=str(float(self.maelis_data['amountInvoice']) - float(self.maelis_data['amountPaid'])),
fpeters commented 2023-04-09 19:21:43 +02:00
Owner
Copy Link

Ici aussi, ne pas manipuler de l'argent avec float(), utiliser Decimal.

Ici aussi, ne pas manipuler de l'argent avec float(), utiliser Decimal.
nroche commented 2023-04-12 20:26:35 +02:00
Author
Owner
Copy Link

Fait.

Fait.
bdauvergne commented 2023-04-09 22:57:24 +02:00
Owner
Copy Link

J'ai fait des commentaires aussi mais on ne le voit pas, je dois rater un truc.

J'ai fait des commentaires aussi mais on ne le voit pas, je dois rater un truc.
bdauvergne requested changes 2023-04-10 12:44:49 +02:00
passerelle/contrib/toulouse_maelis/models.py Outdated
@ -3789,4 +3809,3 @@
'created': item['dateInvoice'][:10],
'pay_limit_date': item['dateDeadline'][:10],
'display_id': str(item['numInvoice']),
'total_amount': item['amountInvoice'],
bdauvergne commented 2023-04-08 11:21:36 +02:00
Owner
Copy Link

Jamais de calcul de monnaie avec float, il faut utiliser decimal.Decimal. Ensuite je laisserai total_amount comme il est, le changement n'amène rien. On aurait pu avoir un objet Payment en prévision des paiements en plusieurs fois au lieu de travailler sur Invoice.status.

                paid = item['amountInvoice'] == item['amountPaid'] or invoice_obj.status() in ['paid', 'notified']
                amount_paid = item['amountInvoice'] if paid else item['amountPaid']
...
                'total_amount': item['amountInvoice'], 
                'amount': str(Decimal(item['amountInvoice']) - Decimal(amount_paid)),
                'amount_paid': amount_paid,
                'paid': paid

avec un modèle Payment(invoice_id, reference_bank, amount, notification_date), ça aurait pu donner ça :

                total_amount = Decimal(item['amountInvoice'])
                amount_locally_paid = [sum(payment.amount) for payment in self.payment_set.exclude(notification_date__isnull=False)]
                amount_paid = Decimal(item['amountPaid'])
                paid = Decimal(item['amountInvoice']) < (amount_paid + amount_locally_paid)
...
                'total_amount': item['amountInvoice'], 
                'amount': str(total_amount - amount_paid - amount_locally_paid),
                'amount_paid': str(amount_paid + amount_locally_paid)
                'paid': paid,
Jamais de calcul de monnaie avec float, il faut utiliser decimal.Decimal. Ensuite je laisserai total_amount comme il est, le changement n'amène rien. On aurait pu avoir un objet Payment en prévision des paiements en plusieurs fois au lieu de travailler sur Invoice.status. ``` paid = item['amountInvoice'] == item['amountPaid'] or invoice_obj.status() in ['paid', 'notified'] amount_paid = item['amountInvoice'] if paid else item['amountPaid'] ... 'total_amount': item['amountInvoice'], 'amount': str(Decimal(item['amountInvoice']) - Decimal(amount_paid)), 'amount_paid': amount_paid, 'paid': paid ``` avec un modèle `Payment(invoice_id, reference_bank, amount, notification_date)`, ça aurait pu donner ça : ``` total_amount = Decimal(item['amountInvoice']) amount_locally_paid = [sum(payment.amount) for payment in self.payment_set.exclude(notification_date__isnull=False)] amount_paid = Decimal(item['amountPaid']) paid = Decimal(item['amountInvoice']) < (amount_paid + amount_locally_paid) ... 'total_amount': item['amountInvoice'], 'amount': str(total_amount - amount_paid - amount_locally_paid), 'amount_paid': str(amount_paid + amount_locally_paid) 'paid': paid, ```
nroche commented 2023-04-12 20:28:32 +02:00
Author
Owner
Copy Link

J'ai fait ça, sans introduire le modèle Payment (pavé du haut).
(dans un premier temps, sauf si tu y tiens)

J'ai fait ça, sans introduire le modèle Payment (pavé du haut). (dans un premier temps, sauf si tu y tiens)
bdauvergne commented 2023-04-13 07:33:18 +02:00
Owner
Copy Link

Ok dans ce cas je pense qu'il faudrait à minima stocker le montant reçu lors de la notification par lingo, que ça représente le amount_locally_paid que j'introduis pour le modèle Payment, qu'on puisse faire un calcul honnête de amountPaid.

if self.payment_notification_date:
     amount_paid = item['amountPaid']
else:
     amount_paid = item['amountPaid'] + amount_locally_paid

Si on a pas encore notifié, on devrait avoir amountPaid=0, amount_localy_paid=amontInvoice.

Si on a notifié, on devrait avoir amountPaid=amountInvoice, amount_localy_paid=amountInvoice.

Et si la facture a été payée par ailleurs par un autre moyen, etc.. ça se verra,
et plus tard ça permettra d'introduire Payment et les paiements multiples en virant le champ amount_locally_paid et en le remplaçant par une property

@property
def amount_locally_paid(self):
    return sum(self.payments.values_list('amount', flat=True))

...

Bon voir mon commentaire suivant, je pensais sincèrement qu'amount_locally_paid était quelque chose qu'on avait.

Ok dans ce cas je pense qu'il faudrait à minima stocker le montant reçu lors de la notification par lingo, que ça représente le `amount_locally_paid` que j'introduis pour le modèle Payment, qu'on puisse faire un calcul honnête de amountPaid. if self.payment_notification_date: amount_paid = item['amountPaid'] else: amount_paid = item['amountPaid'] + amount_locally_paid Si on a pas encore notifié, on devrait avoir amountPaid=0, amount_localy_paid=amontInvoice. Si on a notifié, on devrait avoir amountPaid=amountInvoice, amount_localy_paid=amountInvoice. Et si la facture a été payée par ailleurs par un autre moyen, etc.. ça se verra, et plus tard ça permettra d'introduire Payment et les paiements multiples en virant le champ amount_locally_paid et en le remplaçant par une property @property def amount_locally_paid(self): return sum(self.payments.values_list('amount', flat=True)) ... Bon voir mon commentaire suivant, je pensais sincèrement qu'amount_locally_paid était quelque chose qu'on avait.
nroche commented 2023-04-13 09:35:42 +02:00
Author
Owner
Copy Link

Tu m'as perdu.

il faudrait à minima stocker le montant reçu lors de la notification par lingo

Donc j'attends #76572 pour avoir un modèle Payment stabilisé ?

Tu m'as perdu. > il faudrait à minima stocker le montant reçu lors de la notification par lingo Donc j'attends #76572 pour avoir un modèle Payment stabilisé ?
bdauvergne commented 2023-04-13 17:30:43 +02:00
Owner
Copy Link

Non c'est bon on verra apr

Non c'est bon on verra apr
bdauvergne marked this conversation as resolved
passerelle/contrib/toulouse_maelis/models.py Outdated
@ -3771,3 +3774,3 @@
last_update = now()
for item in result:
self.invoice_set.update_or_create(
invoice, created = self.invoice_set.update_or_create(
bdauvergne commented 2023-04-08 11:00:32 +02:00
Owner
Copy Link

Pourquoi ne pas juste rajouter ['regie_id','invoice_id'] et ['family_id', 'invoice_id'] à Invoice.Meta.unique_together si c'est cela que tu veux exprimer ? Note que tu as déjà une unicité globale pour resource_id je ne vois pas comment on pourrait avoir un doublon en base.

Ou sinon plus simple (pas la peine de repréciser `resource_id quand on travaille dans self.invoice_set):

try:
    # cas normal la facture est déjà chargée, rien à faire
    invoice = self.invoice_set.get(
        invoice_id=item['numInvoice'],
        regie_id=regie_id,
        family_id=family_id,
        maelis_data=item)
except Invoice.DoesNotExist:
    try:
        invoice, created = self.invoice_set.update_or_create(
            invoice_id=item['numInvoice'],
            regie_id=regie_id,
            family_id=family_id,
            defaults={
                'maelis_data': item
            }) # updated est mis à jour automatiquement
    except IntegrityError: # le même invoice_id existe avec une paire (regie_id, family_id) différente
        # et là je dirais qu'il faut logger une erreur mais pas tout arrêter, cette situation ne devrait juste pas arriver normalement, donc ne pas bloquer la récupération des factures pour si peu, si de fait "ça arrive"
        invoice = self.invoice_set.get(invoice_id=item['numInvoice'])
        self.logger.error('invoice "%s(regie_id=%s, family_id=%s)" already exists for (regie_id=%s, family_id=%s)',
            invoice.invoice_id, regie_id, family_id, invoice.regie_id, invoice.family_id)
Pourquoi ne pas juste rajouter `['regie_id','invoice_id']` et `['family_id', 'invoice_id']` à Invoice.Meta.unique_together si c'est cela que tu veux exprimer ? Note que tu as déjà une unicité globale pour resource_id je ne vois pas comment on pourrait avoir un doublon en base. Ou sinon plus simple (pas la peine de repréciser `resource_id quand on travaille dans self.invoice_set): ``` try: # cas normal la facture est déjà chargée, rien à faire invoice = self.invoice_set.get( invoice_id=item['numInvoice'], regie_id=regie_id, family_id=family_id, maelis_data=item) except Invoice.DoesNotExist: try: invoice, created = self.invoice_set.update_or_create( invoice_id=item['numInvoice'], regie_id=regie_id, family_id=family_id, defaults={ 'maelis_data': item }) # updated est mis à jour automatiquement except IntegrityError: # le même invoice_id existe avec une paire (regie_id, family_id) différente # et là je dirais qu'il faut logger une erreur mais pas tout arrêter, cette situation ne devrait juste pas arriver normalement, donc ne pas bloquer la récupération des factures pour si peu, si de fait "ça arrive" invoice = self.invoice_set.get(invoice_id=item['numInvoice']) self.logger.error('invoice "%s(regie_id=%s, family_id=%s)" already exists for (regie_id=%s, family_id=%s)', invoice.invoice_id, regie_id, family_id, invoice.regie_id, invoice.family_id) ```
bdauvergne commented 2023-04-08 12:01:13 +02:00
Owner
Copy Link

Aussi il faudrait logger création/mise à jour de facture (si possible en faisant un diff de maelis_data pour la mise ) au niveau info.

Aussi il faudrait logger création/mise à jour de facture (si possible en faisant un diff de maelis_data pour la mise ) au niveau info.
nroche commented 2023-04-12 23:18:13 +02:00
Author
Owner
Copy Link

Pourquoi ne pas juste rajouter ['regie_id','invoice_id'] et ['family_id', 'invoice_id'] à Invoice.Meta.unique_together si c'est cela que tu veux exprimer ?

Je voulais me prémunir d'un appel au endpoint avec de mauvais paramètres dans les tests, tout en ayant un message d'erreur intelligible.
J'ai essayé de suivre tes recommandations :

  • revoir les appels en commençant par un get
  • logger une erreur plutôt que de tout arrêter,
  • logger la création/mise à jour de facture (avec un diff)
> Pourquoi ne pas juste rajouter ['regie_id','invoice_id'] et ['family_id', 'invoice_id'] à Invoice.Meta.unique_together si c'est cela que tu veux exprimer ? Je voulais me prémunir d'un appel au endpoint avec de mauvais paramètres dans les tests, tout en ayant un message d'erreur intelligible. J'ai essayé de suivre tes recommandations : * revoir les appels en commençant par un get * logger une erreur plutôt que de tout arrêter, * logger la création/mise à jour de facture (avec un diff)
bdauvergne marked this conversation as resolved
passerelle/contrib/toulouse_maelis/models.py Outdated
@ -3802,3 +3822,3 @@
'maelis_item': item,
}
if item['amountInvoice'] == item['amountPaid']:
if amount_invoice == amount_paid:
bdauvergne commented 2023-04-08 11:21:44 +02:00
Owner
Copy Link

Remplacer par if paid et ne pas corriger amount les comptes doivent être justes ou rapportés honnêtement (si trop payé, l'afficher).

Remplacer par `if paid` et ne pas corriger `amount` les comptes doivent être justes ou rapportés honnêtement (si trop payé, l'afficher).
nroche commented 2023-04-12 23:18:56 +02:00
Author
Owner
Copy Link

Fait.

Fait.
bdauvergne marked this conversation as resolved
passerelle/contrib/toulouse_maelis/models.py Outdated
@ -3902,1 +3911,3 @@
)
invoice_obj = self.invoice_set.get(regie_id=regie_id, invoice_id=invoice['maelis_item']['numInvoice'])
if invoice_obj.status() == 'canceled':
bdauvergne commented 2023-04-08 12:00:22 +02:00
Owner
Copy Link

Je ne vois pas à quoi ça correspond, vu que rien ne pose jamais @self.canceled = True@. Comme pour les autres cas c'est bien de préférer une DateTime plutôt qu'un booléen pour tous les champs de statut, avec une méthode:

@atomic
def cancel(self):
    self.lock()
    self.canceled = now()
    self.save()

Mais je serai pour attendre de savoir si on en a besoin, je ne vois pas de notion d'annulation dans les web-services de maelis ni chez nous.

Globalement les factures ne devraient pas avoir de statut, et les paiements un seul, notifié ou pas.

Je ne vois pas à quoi ça correspond, vu que rien ne pose jamais @self.canceled = True@. Comme pour les autres cas c'est bien de préférer une DateTime plutôt qu'un booléen pour tous les champs de statut, avec une méthode: ``` @atomic def cancel(self): self.lock() self.canceled = now() self.save() ``` Mais je serai pour attendre de savoir si on en a besoin, je ne vois pas de notion d'annulation dans les web-services de maelis ni chez nous. Globalement les factures ne devraient pas avoir de statut, et les paiements un seul, notifié ou pas.
nroche commented 2023-04-12 23:24:17 +02:00
Author
Owner
Copy Link

Ça correspondra à #76394.
Si la facture est annulée, il faudrait la conserver dans le connecteur, si l'on souhaite remonter jusqu'à la demande pour l'indiquer.

Ça correspondra à #76394. Si la facture est annulée, il faudrait la conserver dans le connecteur, si l'on souhaite remonter jusqu'à la demande pour l'indiquer.
bdauvergne commented 2023-04-13 07:13:42 +02:00
Owner
Copy Link

Si tu n'as pas implémenté effectivement l'annulation des factures je préfère que tu vires ce champ et toute la logique qui y a trait parce que tu ne sais pas encore ce que tu devras faire à ce sujet; je vois déjà un besoin de deux champ, un d'annulation et un de notification de l'annulation à Maelis parce qu'il faudra le faire en asynchrone et s'assurer qu'on finit toujours par y arriver, comme pour les paiements, ça passera par un cron unique de synchro sur les factures. Pour moi ça mérite bien un ticket clair, ici on parle juste de notifier les paiements.

Si tu n'as pas implémenté effectivement l'annulation des factures je préfère que tu vires ce champ et toute la logique qui y a trait parce que tu ne sais pas encore ce que tu devras faire à ce sujet; je vois déjà un besoin de deux champ, un d'annulation et un de notification de l'annulation à Maelis parce qu'il faudra le faire en asynchrone et s'assurer qu'on finit toujours par y arriver, comme pour les paiements, ça passera par un cron unique de synchro sur les factures. Pour moi ça mérite bien un ticket clair, ici on parle juste de notifier les paiements.
nroche commented 2023-04-13 09:39:54 +02:00
Author
Owner
Copy Link

je préfère que tu vires ce champ et toute la logique qui y a trait parce que tu ne sais pas encore ce que tu devras faire à ce sujet

C'était pour indiquer aux relecteurs que les factures devraient avoir à minima un statut "annulée" dans lequel elles seraient conservées, bien qu'elles ne soient plus remontées par Maélis.
Peut-être d'abord traiter ce point, oui.

> je préfère que tu vires ce champ et toute la logique qui y a trait parce que tu ne sais pas encore ce que tu devras faire à ce sujet C'était pour indiquer aux relecteurs que les factures devraient avoir à minima un statut "annulée" dans lequel elles seraient conservées, bien qu'elles ne soient plus remontées par Maélis. Peut-être d'abord traiter ce point, oui.
nroche commented 2023-04-13 18:16:47 +02:00
Author
Owner
Copy Link

Peut-être d'abord traiter ce point, oui.

Non, ça vient après : là on peut aussi payer des factures "traditionnelles" qui n'ont rien à voir avec le panier Maélis.
Donc fait : j'ai retiré tout ce qui touche à l'annulation des factures.

> Peut-être d'abord traiter ce point, oui. Non, ça vient après : là on peut aussi payer des factures "traditionnelles" qui n'ont rien à voir avec le panier Maélis. Donc fait : j'ai retiré tout ce qui touche à l'annulation des factures.
bdauvergne marked this conversation as resolved
passerelle/contrib/toulouse_maelis/models.py Outdated
@ -3996,6 +4023,35 @@ class Invoice(models.Model):
def __repr__(self):
return '<Invoice "%s/%s">' % (self.regie_id, self.invoice_id)
def status(self):
bdauvergne commented 2023-04-08 11:34:23 +02:00
Owner
Copy Link

Préférer un champ 'notification_date', une fois que c'est notifi

Préférer un champ 'notification_date', une fois que c'est notifi
nroche commented 2023-04-12 23:25:33 +02:00
Author
Owner
Copy Link

C'était déjà un champ date, je l'ai renommé.

C'était déjà un champ date, je l'ai renommé.
bdauvergne marked this conversation as resolved
passerelle/contrib/toulouse_maelis/models.py Outdated
@ -3999,0 +4041,4 @@
codeRegie=self.regie_id,
amount=str(float(self.maelis_data['amountInvoice']) - float(self.maelis_data['amountPaid'])),
datePaiement=self.lingo_data['transaction_date'],
refTransaction=self.lingo_data['transaction_id'],
bdauvergne commented 2023-04-08 11:40:37 +02:00
Owner
Copy Link

Je crois que je l'ai déjà dit, mais la référence du paiement est dans lingo_data['bank_transaction_id'] lingo_date['transaction_id]` c

Je crois que je l'ai déjà dit, mais la référence du paiement est dans `lingo_data['bank_transaction_id']` `lingo_date['transaction_id`]` c
nroche commented 2023-04-12 09:03:54 +02:00
Author
Owner
Copy Link

(j'ai un doute là)
https://git.entrouvert.org/entrouvert/combo/src/branch/main/combo/apps/lingo/models.py#L405

(j'ai un doute là) https://git.entrouvert.org/entrouvert/combo/src/branch/main/combo/apps/lingo/models.py#L405
bdauvergne commented 2023-04-12 10:23:09 +02:00
Owner
Copy Link

Tu as raison, je me fais avoir à chaque fois parce que les deux chemins de paiement sont devenus bien différents mais ce serait bien de les faire converger, on va rester sur transaction_id pour l'instant. On verra si quelqu'un se plaint, visiblement pour Axel ça suffit, mais dans le cas du paiement d'un BasketItem Cyril avait demandé plus de données.

Tu as raison, je me fais avoir à chaque fois parce que les deux chemins de paiement sont devenus bien différents mais ce serait bien de les faire converger, on va rester sur transaction_id pour l'instant. On verra si quelqu'un se plaint, visiblement pour Axel ça suffit, mais dans le cas du paiement d'un BasketItem Cyril avait demandé plus de données.
bdauvergne marked this conversation as resolved
passerelle/contrib/toulouse_maelis/models.py Outdated
@ -3999,0 +4046,4 @@
numPerson=self.maelis_data['payer']['num'],
timeout=timeout,
)
except RequestException:
bdauvergne commented 2023-04-08 11:43:03 +02:00
Owner
Copy Link

soap_client ne lève pas de RequestException mais des passerelle.utils.soap.SOAPError.

soap_client ne lève pas de RequestException mais des `passerelle.utils.soap.SOAPError`.
nroche commented 2023-04-12 23:27:13 +02:00
Author
Owner
Copy Link

J'ai retiré ce code.

J'ai retiré ce code.
bdauvergne commented 2023-04-13 16:08:52 +02:00
Owner
Copy Link

Ok, mais c'est un bug alors il faut les corriger #76609.

Ok, mais c'est un bug alors il faut les corriger #76609.
bdauvergne marked this conversation as resolved
nroche force-pushed wip/76394-parsifal-pay-invoice-async from cd9a90a923 to d4fc07eb47 2023-04-12 20:22:59 +02:00 Compare
nroche changed title from toulouse-maelis: notification asynchrone du paiement des factures (#76394) to WIP: toulouse-maelis: notification asynchrone du paiement des factures (#76394) 2023-04-12 20:51:52 +02:00
nroche force-pushed wip/76394-parsifal-pay-invoice-async from d4fc07eb47 to c384014a0f 2023-04-12 23:18:39 +02:00 Compare
nroche changed title from WIP: toulouse-maelis: notification asynchrone du paiement des factures (#76394) to toulouse-maelis: notification asynchrone du paiement des factures (#76394) 2023-04-12 23:32:03 +02:00
nroche changed title from toulouse-maelis: notification asynchrone du paiement des factures (#76394) to WIP: toulouse-maelis: notification asynchrone du paiement des factures (#76394) 2023-04-13 07:51:18 +02:00
nroche force-pushed wip/76394-parsifal-pay-invoice-async from c384014a0f to d1e5f2815d 2023-04-13 09:19:37 +02:00 Compare
nroche force-pushed wip/76394-parsifal-pay-invoice-async from d1e5f2815d to f3e75060d6 2023-04-13 18:14:24 +02:00 Compare
nroche changed title from WIP: toulouse-maelis: notification asynchrone du paiement des factures (#76394) to toulouse-maelis: notification asynchrone du paiement des factures (#76394) 2023-04-13 18:26:59 +02:00
bdauvergne approved these changes 2023-04-13 18:43:04 +02:00
passerelle/contrib/toulouse_maelis/models.py Outdated
@ -3897,4 +3911,0 @@
amount=invoice['amount'],
datePaiement=post_data['transaction_date'],
refTransaction=post_data['transaction_id'],
numInvoices=[invoice['display_id']],
bdauvergne commented 2023-04-12 10:35:37 +02:00
Owner
Copy Link

Dans le InvoiceService.wsdl on a ça :

<xs:element maxOccurs="unbounded" name="numInvoices" type="xs:int"/>

mais aussi

<xs:element minOccurs="0" name="numInvoice" type="xs:string"/>

C'est pas cohérent, mais si c'est testé ça veut dire que zeep a converti automatiquement la chaîne en entier parce qu'au niveau XSD c'est syntaxiquement pareil, ça mériterait un petit commentaire pour plus tard.

Dans le InvoiceService.wsdl on a ça : <xs:element maxOccurs="unbounded" name="numInvoices" type="xs:int"/> mais aussi <xs:element minOccurs="0" name="numInvoice" type="xs:string"/> C'est pas cohérent, mais si c'est testé ça veut dire que zeep a converti automatiquement la chaîne en entier parce qu'au niveau XSD c'est syntaxiquement pareil, ça mériterait un petit commentaire pour plus tard.
nroche commented 2023-04-14 09:13:50 +02:00
Author
Owner
Copy Link

En regardant le wsdl qu'on a dans les sources et qui est quasi à jour :

  • la première définition concerne le paramètre de l'appel WS payInvoices.
  • la seconde définie les champs retournés pas readInvoices.

Quoi qu'il en soit j'ai toujours pas re-testé ce code (plus de facture sous la main) et je fais ça aujourd'hui.

En regardant le wsdl qu'on a dans les sources et qui est quasi à jour : * la première définition concerne le paramètre de l'appel WS payInvoices. * la seconde définie les champs retournés pas readInvoices. Quoi qu'il en soit j'ai toujours pas re-testé ce code (plus de facture sous la main) et je fais ça aujourd'hui.
nroche commented 2023-04-14 16:15:49 +02:00
Author
Owner
Copy Link

Testé en vrai.
Perso j'aimerais bien pousser, même si on n'est pas à l'abri que Sigec change tout une fois de plus.
https://redmine.sigec.fr/issues/1573#note-24

Testé en vrai. Perso j'aimerais bien pousser, même si on n'est pas à l'abri que Sigec change tout une fois de plus. https://redmine.sigec.fr/issues/1573#note-24
bdauvergne commented 2023-04-14 16:55:53 +02:00
Owner
Copy Link

Oui vas y pousse.

Oui vas y pousse.
passerelle/contrib/toulouse_maelis/models.py Outdated
@ -3894,0 +3897,4 @@
invoice.save()
complete_diff = difflib.ndiff(content_two.split('\n'), content_one.split('\n'))
diff = [x for x in complete_diff if x[0] in ['-', '+']]
self.logger.info("update %s on familly '%s': %s", repr(invoice), family_id, diff)
bdauvergne commented 2023-04-13 07:25:57 +02:00
Owner
Copy Link

Tu peux logger des messages traduits.

Tu peux logger des messages traduits.
nroche commented 2023-04-14 09:09:25 +02:00
Author
Owner
Copy Link

Fait.

Fait.
passerelle/contrib/toulouse_maelis/models.py Outdated
@ -4085,0 +4143,4 @@
numDossier=self.family_id,
codeRegie=self.regie_id,
amount=str(
Decimal(self.maelis_data['amountInvoice']) - Decimal(self.maelis_data['amountPaid'])
bdauvergne commented 2023-04-13 07:33:52 +02:00
Owner
Copy Link

Et là je me rend compte que le code idiot dans lingo ne remonte pas le montant payé, je serai franchement pour ouvrir tout de suite un ticket pour avoir ça dans la notif et ici pouvoir écrite self.lingo_data['amount'] parce que si la facture a été partiellement payée par ailleurs on ne sait quel montant a été payé.

J'ai ouvert #76572 que je vais traiter de ce pas.

Et là je me rend compte que le code idiot dans lingo ne remonte pas le montant payé, je serai franchement pour ouvrir tout de suite un ticket pour avoir ça dans la notif et ici pouvoir écrite `self.lingo_data['amount']` parce que si la facture a été partiellement payée par ailleurs on ne sait quel montant a été payé. J'ai ouvert #76572 que je vais traiter de ce pas.
nroche force-pushed wip/76394-parsifal-pay-invoice-async from f3e75060d6 to 67c94387bf 2023-04-14 09:09:07 +02:00 Compare
nroche force-pushed wip/76394-parsifal-pay-invoice-async from 67c94387bf to 29f4774d2b 2023-04-14 16:19:50 +02:00 Compare
nroche commented 2023-04-14 16:23:53 +02:00
Author
Owner
Copy Link

J'ai ajouté l'enregistrement du numéro de règlement renvoyé par maélis.
J'ai ajouté un verrou sur notify() pour départager les notifications par job et par cron.

J'ai ajouté l'enregistrement du numéro de règlement renvoyé par maélis. J'ai ajouté un verrou sur notify() pour départager les notifications par job et par cron.
nroche merged commit 29f4774d2b into main 2023-04-14 17:13:12 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
fpeters
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#182
No description provided.
Delete Branch "wip/76394-parsifal-pay-invoice-async"

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?