From a188512c5b61d4e8e394f408b50fd2a8b6523ecc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laur=C3=A9line=20Gu=C3=A9rin?= <zebuline@entrouvert.com>
Date: Mon, 27 Jan 2020 16:13:43 +0100
Subject: [PATCH] lingo: return HttpResponseBadRequest instead of Exception
 (#39277)

---
 combo/apps/lingo/views.py   | 20 +++++++++++--------
 tests/test_lingo_payment.py | 40 +++++++++++++++++++++++++++++++++++--
 2 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/combo/apps/lingo/views.py b/combo/apps/lingo/views.py
index 1f75ab1..bb60d83 100644
--- a/combo/apps/lingo/views.py
+++ b/combo/apps/lingo/views.py
@@ -138,7 +138,7 @@ class AddBasketItemApiView(View):
 
         if not 'amount' in request.GET and not 'amount' in request_body and \
                 not 'amount' in extra:
-            raise Exception('missing amount parameter')
+            return HttpResponseBadRequest('missing amount parameter')
 
         item = BasketItem(amount=0)
         try:
@@ -171,7 +171,7 @@ class AddBasketItemApiView(View):
             else:
                 user = None
         except User.DoesNotExist:
-            raise Exception('unknown user')
+            return HttpResponseBadRequest('unknown user')
 
         item.user = user
         if request.GET.get('regie_id'):
@@ -240,7 +240,7 @@ class RemoveBasketItemApiView(View):
         request_body = json.loads(force_text(self.request.body))
 
         if not 'basket_item_id' in request_body:
-            raise Exception('missing basket_item_id parameter')
+            return HttpResponseBadRequest('missing basket_item_id parameter')
 
         try:
             if request.GET.get('NameId'):
@@ -250,12 +250,16 @@ class RemoveBasketItemApiView(View):
             elif request.GET.get('email'):
                 user = User.objects.get(email=request.GET.get('email'))
             else:
-                raise Exception('no user specified')
+                return HttpResponseBadRequest('no user specified')
         except User.DoesNotExist:
-            raise Exception('unknown user')
+            return HttpResponseBadRequest('unknown user')
+
+        try:
+            item = BasketItem.objects.get(id=request_body.get('basket_item_id'),
+                    user=user, cancellation_date__isnull=True)
+        except BasketItem.DoesNotExist:
+            return HttpResponseBadRequest('unknown basket item')
 
-        item = BasketItem.objects.get(id=request_body.get('basket_item_id'),
-                user=user, cancellation_date__isnull=True)
         notify_origin = bool(request_body.get('notify', 'false') == 'true')
         item.notify_cancellation(notify_origin=notify_origin)
 
@@ -525,7 +529,7 @@ class PaymentView(View):
         elif 'payment_backend_pk' in kwargs:
             payment_backend = PaymentBackend.objects.get(id=kwargs['payment_backend_pk'])
         else:
-            raise Exception("A payment backend or regie primary key must be specified")
+            return HttpResponseBadRequest("A payment backend or regie primary key must be specified")
 
         payment = get_eopayment_object(request, payment_backend)
         logger = logging.getLogger(__name__)
diff --git a/tests/test_lingo_payment.py b/tests/test_lingo_payment.py
index 4162544..ecc7a93 100644
--- a/tests/test_lingo_payment.py
+++ b/tests/test_lingo_payment.py
@@ -251,9 +251,21 @@ def test_add_amount_to_basket(app, key, regie, user):
 
     user_email = 'foo@example.com'
     User.objects.get_or_create(email=user_email)
-    amount = 42
-    data = {'amount': amount, 'display_name': 'test amount',
+
+    data = {'display_name': 'test amount',
             'url': 'http://example.com'}
+    url = '%s?email=%s&orig=wcs' % (reverse('api-add-basket-item'), user_email)
+    url = sign_url(url, key)
+    resp = app.post_json(url, params=data, status=400)
+    assert 'missing amount parameter' in resp.text
+
+    amount = 42
+    data['amount'] = amount
+    url = '%s?email=%s&orig=wcs' % (reverse('api-add-basket-item'), 'unknown@example.com')
+    url = sign_url(url, key)
+    resp = app.post_json(url, params=data, status=400)
+    assert 'unknown user' in resp.text
+
     url = '%s?email=%s&orig=wcs' % (reverse('api-add-basket-item'), user_email)
     url = sign_url(url, key)
     resp = app.post_json(url, params=data)
@@ -527,6 +539,30 @@ def test_cancel_basket_item(app, key, regie, user):
     assert BasketItem.objects.filter(amount=21, cancellation_date__isnull=True).exists()
     basket_item_id_2 = json.loads(resp.text)['id']
 
+    url = '%s?email=%s&orig=wcs' % (reverse('api-remove-basket-item'), user_email)
+    url = sign_url(url, key)
+    data = {'notify': 'true'}
+    resp = app.post_json(url, params=data, status=400)
+    assert 'missing basket_item_id parameter' in resp.text
+
+    url = '%s?email=%s&orig=wcs' % (reverse('api-remove-basket-item'), user_email)
+    url = sign_url(url, key)
+    data = {'basket_item_id': 0, 'notify': 'true'}
+    resp = app.post_json(url, params=data, status=400)
+    assert 'unknown basket item' in resp.text
+
+    url = '%s?orig=wcs' % (reverse('api-remove-basket-item'))
+    url = sign_url(url, key)
+    data = {'basket_item_id': basket_item_id, 'notify': 'true'}
+    resp = app.post_json(url, params=data, status=400)
+    assert 'no user specified' in resp.text
+
+    url = '%s?email=%s&orig=wcs' % (reverse('api-remove-basket-item'), 'unknown@example.com')
+    url = sign_url(url, key)
+    data = {'basket_item_id': basket_item_id, 'notify': 'true'}
+    resp = app.post_json(url, params=data, status=400)
+    assert 'unknown user' in resp.text
+
     with mock.patch('combo.utils.requests_wrapper.RequestsSession.request') as request:
         url = '%s?email=%s&orig=wcs' % (reverse('api-remove-basket-item'), user_email)
         url = sign_url(url, key)
-- 
2.20.1

