From 19075ba7323e1ba482f0ef7028384b2e3ab760e1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laur=C3=A9line=20Gu=C3=A9rin?= <zebuline@entrouvert.com>
Date: Thu, 18 Feb 2021 17:13:17 +0100
Subject: [PATCH] api: APIError handling (#51181)

---
 chrono/api/utils.py | 30 ++++++++++++++++++
 chrono/api/views.py | 75 ++++++++++++++++++---------------------------
 chrono/settings.py  |  2 ++
 tests/settings.py   |  1 +
 tests/test_api.py   | 24 +++++++++++++++
 5 files changed, 87 insertions(+), 45 deletions(-)

diff --git a/chrono/api/utils.py b/chrono/api/utils.py
index f474636..0ddba26 100644
--- a/chrono/api/utils.py
+++ b/chrono/api/utils.py
@@ -15,7 +15,11 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+
+from django.utils.encoding import force_text
+
 from rest_framework.response import Response as DRFResponse
+from rest_framework.views import exception_handler as DRF_exception_handler
 
 
 class Response(DRFResponse):
@@ -24,3 +28,29 @@ class Response(DRFResponse):
         if data is not None and 'err_class' in data:
             data['reason'] = data['err_class']
         super(Response, self).__init__(data=data, *args, **kwargs)
+
+
+class APIError(Exception):
+    err = 1
+    http_status = 200
+
+    def __init__(self, *args, **kwargs):
+        self.__dict__.update(kwargs)
+        super(APIError, self).__init__(*args)
+
+    def to_response(self):
+        data = {
+            'err': self.err,
+            'err_class': self.err_class,
+            'err_desc': force_text(self),
+        }
+        if hasattr(self, 'errors'):
+            data['errors'] = self.errors
+        return Response(data, status=self.http_status)
+
+
+def exception_handler(exc, context):
+    if isinstance(exc, APIError):
+        return exc.to_response()
+
+    return DRF_exception_handler(exc, context)
diff --git a/chrono/api/views.py b/chrono/api/views.py
index 9a35085..b24c3a0 100644
--- a/chrono/api/views.py
+++ b/chrono/api/views.py
@@ -36,30 +36,11 @@ from rest_framework import permissions, serializers, status
 
 from rest_framework.views import APIView
 
-from chrono.api.utils import Response
+from chrono.api.utils import Response, APIError
 from ..agendas.models import Agenda, Event, Booking, MeetingType, TimePeriodException, Desk, BookingColor
 from ..interval import IntervalSet
 
 
-class APIError(Exception):
-    err = 1
-    http_status = 200
-
-    def __init__(self, *args, **kwargs):
-        self.__dict__.update(kwargs)
-        super(APIError, self).__init__(*args)
-
-    def to_response(self):
-        data = {
-            'err': self.err,
-            'err_class': self.err_class,
-            'err_desc': force_text(self),
-        }
-        if hasattr(self, 'errors'):
-            data['errors'] = self.errors
-        return Response(data, status=self.http_status)
-
-
 def format_response_datetime(dt):
     return localtime(dt).strftime('%Y-%m-%d %H:%M:%S')
 
@@ -552,9 +533,25 @@ class Datetimes(APIView):
 
         date_start, date_end = request.GET.get('date_start'), request.GET.get('date_end')
         if date_start:
-            date_start = make_aware(datetime.datetime.combine(parse_date(date_start), datetime.time(0, 0)))
+            try:
+                date_start = make_aware(
+                    datetime.datetime.combine(parse_date(date_start), datetime.time(0, 0))
+                )
+            except TypeError:
+                raise APIError(
+                    _('date_start format must be YYYY-MM-DD'),
+                    err_class='date_start format must be YYYY-MM-DD',
+                    http_status=status.HTTP_400_BAD_REQUEST,
+                )
         if date_end:
-            date_end = make_aware(datetime.datetime.combine(parse_date(date_end), datetime.time(0, 0)))
+            try:
+                date_end = make_aware(datetime.datetime.combine(parse_date(date_end), datetime.time(0, 0)))
+            except TypeError:
+                raise APIError(
+                    _('date_end format must be YYYY-MM-DD'),
+                    err_class='date_end format must be YYYY-MM-DD',
+                    http_status=status.HTTP_400_BAD_REQUEST,
+                )
 
         entries = agenda.get_open_events(annotate_queryset=True, min_start=date_start, max_start=date_end)
 
@@ -586,10 +583,7 @@ class MeetingDatetimes(APIView):
 
         now_datetime = now()
 
-        try:
-            resources = get_resources_from_request(request, agenda)
-        except APIError as e:
-            return e.to_response()
+        resources = get_resources_from_request(request, agenda)
 
         start_datetime = None
         if 'date_start' in request.GET:
@@ -597,7 +591,7 @@ class MeetingDatetimes(APIView):
                 start_datetime = make_aware(
                     datetime.datetime.combine(parse_date(request.GET['date_start']), datetime.time(0, 0))
                 )
-            except TypeError as e:
+            except TypeError:
                 raise APIError(
                     _('date_start format must be YYYY-MM-DD'),
                     err_class='date_start format must be YYYY-MM-DD',
@@ -610,7 +604,7 @@ class MeetingDatetimes(APIView):
                 end_datetime = make_aware(
                     datetime.datetime.combine(parse_date(request.GET['date_end']), datetime.time(0, 0))
                 )
-            except TypeError as e:
+            except TypeError:
                 raise APIError(
                     _('date_end format must be YYYY-MM-DD'),
                     err_class='date_end format must be YYYY-MM-DD',
@@ -836,10 +830,7 @@ class Fillslots(APIView):
     serializer_class = SlotsSerializer
 
     def post(self, request, agenda_identifier=None, event_identifier=None, format=None):
-        try:
-            return self.fillslot(request=request, agenda_identifier=agenda_identifier, format=format)
-        except APIError as e:
-            return e.to_response()
+        return self.fillslot(request=request, agenda_identifier=agenda_identifier, format=format)
 
     def fillslot(self, request, agenda_identifier=None, slots=[], format=None):
         multiple_booking = bool(not slots)
@@ -961,10 +952,7 @@ class Fillslots(APIView):
                         http_status=status.HTTP_400_BAD_REQUEST,
                     )
 
-            try:
-                resources = get_resources_from_request(request, agenda)
-            except APIError as e:
-                return e.to_response()
+            resources = get_resources_from_request(request, agenda)
 
             # get all free slots and separate them by desk
             try:
@@ -1224,15 +1212,12 @@ class Fillslot(Fillslots):
     serializer_class = SlotSerializer
 
     def post(self, request, agenda_identifier=None, event_identifier=None, format=None):
-        try:
-            return self.fillslot(
-                request=request,
-                agenda_identifier=agenda_identifier,
-                slots=[event_identifier],  # fill a "list on one slot"
-                format=format,
-            )
-        except APIError as e:
-            return e.to_response()
+        return self.fillslot(
+            request=request,
+            agenda_identifier=agenda_identifier,
+            slots=[event_identifier],  # fill a "list on one slot"
+            format=format,
+        )
 
 
 fillslot = Fillslot.as_view()
diff --git a/chrono/settings.py b/chrono/settings.py
index 68d2531..cf4c8b6 100644
--- a/chrono/settings.py
+++ b/chrono/settings.py
@@ -184,6 +184,8 @@ TEMPLATE_VARS = {}
 SMS_URL = ''
 SMS_SENDER = ''
 
+REST_FRAMEWORK = {'EXCEPTION_HANDLER': 'chrono.api.utils.exception_handler'}
+
 local_settings_file = os.environ.get(
     'CHRONO_SETTINGS_FILE', os.path.join(os.path.dirname(__file__), 'local_settings.py')
 )
diff --git a/tests/settings.py b/tests/settings.py
index ebe744c..18996a8 100644
--- a/tests/settings.py
+++ b/tests/settings.py
@@ -5,6 +5,7 @@ LANGUAGE_CODE = 'en-us'
 
 REST_FRAMEWORK = {
     'DEFAULT_AUTHENTICATION_CLASSES': ['rest_framework.authentication.BasicAuthentication'],
+    'EXCEPTION_HANDLER': 'chrono.api.utils.exception_handler',
 }
 
 DATABASES = {
diff --git a/tests/test_api.py b/tests/test_api.py
index 3c31d07..a321df5 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -3176,6 +3176,18 @@ def test_agenda_api_date_range(app, some_data):
             event_dt = datetime.datetime.combine(day, datetime.datetime.strptime(event, '%H:%M').time())
             Event.objects.create(agenda=agenda2, start_datetime=make_aware(event_dt), places=2)
 
+    params = {'date_start': 'foo'}
+    resp = app.get('/api/agenda/%s/datetimes/' % agenda2.slug, params=params, status=400)
+    assert resp.json['err'] == 1
+    assert resp.json['err_class'] == 'date_start format must be YYYY-MM-DD'
+    assert resp.json['err_desc'] == 'date_start format must be YYYY-MM-DD'
+
+    params = {'date_end': 'foo'}
+    resp = app.get('/api/agenda/%s/datetimes/' % agenda2.slug, params=params, status=400)
+    assert resp.json['err'] == 1
+    assert resp.json['err_class'] == 'date_end format must be YYYY-MM-DD'
+    assert resp.json['err_desc'] == 'date_end format must be YYYY-MM-DD'
+
     params = {'date_start': base_date.isoformat()}
     resp = app.get('/api/agenda/%s/datetimes/' % agenda2.slug, params=params)
     assert len(resp.json['data']) == 6
@@ -4977,6 +4989,18 @@ def test_meetings_and_virtual_datetimes_date_filter(app):
     resp = app.get(virtual_api_url)
     assert len(resp.json['data']) == 24
 
+    params = {'date_start': 'foo'}
+    resp = app.get(foo_api_url, params=params, status=400)
+    assert resp.json['err'] == 1
+    assert resp.json['err_class'] == 'date_start format must be YYYY-MM-DD'
+    assert resp.json['err_desc'] == 'date_start format must be YYYY-MM-DD'
+
+    params = {'date_end': 'foo'}
+    resp = app.get(foo_api_url, params=params, status=400)
+    assert resp.json['err'] == 1
+    assert resp.json['err_class'] == 'date_end format must be YYYY-MM-DD'
+    assert resp.json['err_desc'] == 'date_end format must be YYYY-MM-DD'
+
     # exclude weekday1 through date_start, 4 slots each day * 5 days
     params = {'date_start': (localtime(now()) + datetime.timedelta(days=2)).date().isoformat()}
     resp = app.get(foo_api_url, params=params)
-- 
2.29.2

