From 0ded4ae599d4c0b51a9a523ebc09ee7b14fc11ec Mon Sep 17 00:00:00 2001
From: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date: Fri, 12 Jun 2020 10:35:26 +0200
Subject: [PATCH] lingo: report errors on invoices retrieval (#43967)

---
 combo/apps/lingo/models.py                    | 41 +++++++++++++++----
 .../lingo/templates/lingo/combo/items.html    |  9 +++-
 tests/test_lingo_remote_regie.py              | 11 +++++
 3 files changed, 53 insertions(+), 8 deletions(-)

diff --git a/combo/apps/lingo/models.py b/combo/apps/lingo/models.py
index 4a78e392..3998266a 100644
--- a/combo/apps/lingo/models.py
+++ b/combo/apps/lingo/models.py
@@ -25,6 +25,7 @@ import re
 from dateutil import parser
 import eopayment
 from jsonfield import JSONField
+from requests import RequestException
 
 from django import template
 from django.conf import settings
@@ -118,6 +119,10 @@ class PaymentBackend(models.Model):
         return eopayment.Payment(self.service, options)
 
 
+class RegieException(Exception):
+    pass
+
+
 @python_2_unicode_compatible
 class Regie(models.Model):
     label = models.CharField(verbose_name=_('Label'), max_length=64)
@@ -176,7 +181,20 @@ class Regie(models.Model):
             url = self.webservice_url + '/invoices/'
             if history:
                 url += 'history/'
-            items = requests.get(url, user=user, remote_service='auto', cache_duration=0).json()
+
+            regie_exc_msg = _('Regie "%(label)s" is unavailable, please retry later.') % {
+                'label': self.label,
+            }
+
+            try:
+                response = requests.get(url, user=user, remote_service='auto', cache_duration=0)
+                response.raise_for_status()
+            except RequestException as e:
+                raise RegieException(regie_exc_msg) from e
+            try:
+                items = response.json()
+            except ValueError as e:
+                raise RegieException(regie_exc_msg) from e
             if items.get('data'):
                 return [build_remote_item(item, self) for item in items.get('data')]
             return []
@@ -693,7 +711,7 @@ class Items(CellBase):
         return Regie.objects.all()
 
     def get_invoices(self, user):
-        return []
+        return [], []
 
     def get_cell_extra_context(self, context):
         ctx = super(Items, self).get_cell_extra_context(context)
@@ -701,11 +719,12 @@ class Items(CellBase):
             # don't call webservices when we're just looking for placeholders
             return ctx
         ctx.update({'title': self.title, 'text': self.text})
-        items = self.get_invoices(user=context['user'])
+        items, errors = self.get_invoices(user=context['user'])
         none_date = datetime.datetime(1900, 1, 1)  # to avoid None-None comparison errors
         items.sort(key=lambda i: i.creation_date or none_date, reverse=True)
         ctx.update({
             'items': items,
+            'errors': errors,
             'with_payment_limit_date': any(i.payment_limit_date for i in items),
             'with_amount_paid': any(getattr(i, 'amount_paid', None) for i in items),
         })
@@ -726,9 +745,13 @@ class ItemsHistory(Items):
 
     def get_invoices(self, user):
         items = []
+        errors = []
         for r in self.get_regies():
-            items.extend(r.get_invoices(user, history=True))
-        return items
+            try:
+                items.extend(r.get_invoices(user, history=True))
+            except RegieException as e:
+                errors.append(e)
+        return items, errors
 
 
 @register_cell_class
@@ -740,9 +763,13 @@ class ActiveItems(Items):
 
     def get_invoices(self, user):
         items = []
+        errors = []
         for r in self.get_regies():
-            items.extend(r.get_invoices(user))
-        return items
+            try:
+                items.extend(r.get_invoices(user))
+            except RegieException as e:
+                errors.append(e)
+        return items, errors
 
 
 @register_cell_class
diff --git a/combo/apps/lingo/templates/lingo/combo/items.html b/combo/apps/lingo/templates/lingo/combo/items.html
index 4c270f88..d7d87104 100644
--- a/combo/apps/lingo/templates/lingo/combo/items.html
+++ b/combo/apps/lingo/templates/lingo/combo/items.html
@@ -1,9 +1,16 @@
 {% load i18n %}
 {% block cell-content %}
-{% if items or not cell.hide_if_empty %}
+{% if errors or items or not cell.hide_if_empty %}
 {% if title %}<h2>{{ title|safe }}</h2>{% endif %}
 <div>
 {% if text %}{{ text|safe }}{% endif %}
+{% if errors %}
+  <ul class="errorlist">
+    {% for error in errors %}
+      <li>{{ error }}</li>
+    {% endfor %}
+  </ul>
+{% endif %}
 {% if items %}
 <table class="invoices">
   <thead>
diff --git a/tests/test_lingo_remote_regie.py b/tests/test_lingo_remote_regie.py
index e11ac578..1d207cf3 100644
--- a/tests/test_lingo_remote_regie.py
+++ b/tests/test_lingo_remote_regie.py
@@ -129,6 +129,12 @@ def test_remote_regie_active_invoices_cell(mock_send, remote_regie):
     content = cell.render(context)
     assert 'No items yet' in content
 
+    # regie is down
+    mock_send.side_effect = ConnectionError
+    content = cell.render(context)
+    assert 'Regie &quot;Remote&quot; is unavailable, please retry later.' in content
+
+
 @mock.patch('combo.utils.requests_wrapper.RequestsSession.send')
 def test_remote_regie_past_invoices_cell(mock_send, remote_regie):
     assert remote_regie.is_remote() == True
@@ -215,6 +221,11 @@ def test_remote_regie_past_invoices_cell(mock_send, remote_regie):
     content = cell.render(context)
     assert content.strip() == ''
 
+    # regie is down
+    mock_send.side_effect = ConnectionError
+    content = cell.render(context)
+    assert 'Regie &quot;Remote&quot; is unavailable, please retry later.' in content
+
 
 @mock.patch('combo.apps.lingo.models.Regie.pay_invoice')
 @mock.patch('combo.apps.lingo.models.requests.get')