From 6249e6e5197f5545519cb071193560e640573a58 Mon Sep 17 00:00:00 2001
From: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date: Fri, 30 Jul 2021 11:11:19 +0200
Subject: [PATCH 3/3] lingo: make BasketItem creation idempotent for paid
 invoices

Using get_or_create() based on the remote_item_id field inside a
transaction is idempotent, the @self.items.add(...)@ operation also (if
the item is already linked it does nothing).

The new behaviour garantee that event if we cannot notify the regie's
web-service, a BasketItem is created to show the user its payment has
been recorded.
---
 combo/apps/lingo/models.py  | 23 ++++++++++++++---------
 tests/test_lingo_payment.py |  6 ++++++
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/combo/apps/lingo/models.py b/combo/apps/lingo/models.py
index fe248409..de459d93 100644
--- a/combo/apps/lingo/models.py
+++ b/combo/apps/lingo/models.py
@@ -857,6 +857,8 @@ class Transaction(models.Model):
         for item_id in items.split(','):
             try:
                 remote_item = regie.get_invoice(user=self.user, invoice_id=item_id, raise_4xx=True)
+                with atomic(savepoint=False):
+                    self.items.add(self.create_paid_invoice_basket_item(item_id, remote_item))
                 regie.pay_invoice(item_id, self.order_id, self.bank_transaction_date or self.end_date)
             except ObjectDoesNotExist:
                 # 4xx error
@@ -879,21 +881,24 @@ class Transaction(models.Model):
                 )
             else:
                 logger.info('notified payment for remote item %s from transaction %s', item_id, self)
-                self.items.add(self.create_paid_invoice_basket_item(remote_item))
 
         self.to_be_paid_remote_items = ','.join(to_be_paid_remote_items) or None
         self.save(update_fields=['to_be_paid_remote_items'])
 
-    def create_paid_invoice_basket_item(self, remote_item):
+    def create_paid_invoice_basket_item(self, item_id, remote_item):
         subject = _('Invoice #%s') % remote_item.display_id
-        return BasketItem.objects.create(
-            user=self.user,
-            regie=remote_item.regie,
-            source_url='',
-            subject=subject,
-            amount=remote_item.amount,
-            payment_date=self.end_date,
+        basket_item, created = BasketItem.objects.get_or_create(
+            remote_item_id=item_id,
+            defaults=dict(
+                user=self.user,
+                regie=remote_item.regie,
+                source_url='',
+                subject=subject,
+                amount=remote_item.amount,
+                payment_date=self.end_date,
+            ),
         )
+        return basket_item
 
     def handle_backend_response(self, response, callback=True):
         logger.debug('lingo: regie "%s" handling response for transaction "%%s"' % self.regie, self.order_id)
diff --git a/tests/test_lingo_payment.py b/tests/test_lingo_payment.py
index 712f8500..c8b92901 100644
--- a/tests/test_lingo_payment.py
+++ b/tests/test_lingo_payment.py
@@ -1120,6 +1120,8 @@ def test_transaction_retry_failure(mock_request, remote_regie):
     mock_err = mock.Mock(status_code=200)
     mock_err.json.return_url = {'err': 1}
 
+    assert transaction.items.count() == 0
+
     # error on get invoice
     mock_request.side_effect = [
         ConnectionError('where is my hostname?'),  # get invoice 42
@@ -1128,6 +1130,8 @@ def test_transaction_retry_failure(mock_request, remote_regie):
     ]
     appconfig.update_transactions()
     transaction.refresh_from_db()
+    assert transaction.items.count() == 1  # only 35 was found
+    assert set(transaction.items.values_list('remote_item_id', flat=True)) == {'35'}
     assert transaction.to_be_paid_remote_items == '42'  # retry for first one
 
     # error on pay invoice
@@ -1140,6 +1144,8 @@ def test_transaction_retry_failure(mock_request, remote_regie):
     ]
     appconfig.update_transactions()
     transaction.refresh_from_db()
+    assert transaction.items.count() == 2  # both were updated now that get_invoice worked for 42
+    assert set(transaction.items.values_list('remote_item_id', flat=True)) == {'35', '42'}
     assert transaction.to_be_paid_remote_items == '42'  # retry for first one
 
     # unknown error on get invoice
-- 
2.32.0.rc0