From 2547801a6af10db2ec5b632ff1a2463820549303 Mon Sep 17 00:00:00 2001
From: Serghei Mihai <smihai@entrouvert.com>
Date: Thu, 29 Dec 2016 14:13:22 +0100
Subject: [PATCH] clicrdv: replace urllib2 by connector's requests (#7898)

---
 passerelle/apps/clicrdv/models.py |  54 ++++++--------
 tests/test_clicrdv.py             | 120 ++++++++++++++++++++++--------
 2 files changed, 110 insertions(+), 64 deletions(-)

diff --git a/passerelle/apps/clicrdv/models.py b/passerelle/apps/clicrdv/models.py
index feef9c8d..4bdc228d 100644
--- a/passerelle/apps/clicrdv/models.py
+++ b/passerelle/apps/clicrdv/models.py
@@ -7,7 +7,7 @@ It is a gateway to http://developers.clicrdv.com/fr/rest-api.html
 import base64
 import datetime
 import json
-import urllib2
+import requests
 
 from django.conf import settings
 from django.core.urlresolvers import reverse
@@ -15,6 +15,7 @@ from django.db import models
 from django.utils.dateformat import format as date_format
 from django.utils.dateformat import time_format
 from django.utils.translation import ugettext_lazy as _
+from django.utils.http import urlquote
 
 from passerelle.base.models import BaseResource
 from passerelle.utils.api import endpoint
@@ -48,25 +49,19 @@ class ClicRdv(BaseResource):
     def get_verbose_name(cls):
         return cls._meta.verbose_name
 
-    def get_request(self, uri):
+    def request(self, uri, method='get', **kwargs):
         url = 'https://%s/api/v1/groups/%s/%s' % (self.server, self.group_id, uri)
         if '?' in url:
            url = url + '&apikey=%s&format=json' % self.apikey
         else:
            url = url + '?apikey=%s&format=json' % self.apikey
-        req = urllib2.Request(url)
-        authheader = 'Basic ' + \
-            base64.encodestring('%s:%s' % (self.username, self.password))[:-1]
-        req.add_header('Authorization', authheader)
-        return req
-
-    def get_json(self, uri):
-        req = self.get_request(uri)
-        return json.load(urllib2.urlopen(req))
+        basic_auth = requests.auth.HTTPBasicAuth(self.username, self.password)
+        return self.requests.request(method, url, auth=basic_auth, **kwargs)
 
     @endpoint(name='interventionsets')
     def get_interventionsets(self, request, **kwargs):
-        records = self.get_json('interventionsets').get('records')
+        response = self.request('interventionsets')
+        records = response.json().get('records')
         records.sort(lambda x,y: cmp(x['sort'], y['sort']))
         ret = []
         for record in records:
@@ -77,7 +72,8 @@ class ClicRdv(BaseResource):
     @endpoint(name='interventionsets', pattern='(?P<set>\d+)/')
     def get_interventions(self, request, set, **kwargs):
         ret = []
-        records = self.get_json('interventions?interventionset_id=%s' % set).get('records')
+        response = self.request('interventions?interventionset_id=%s' % set)
+        records = response.json().get('records')
         records.sort(lambda x,y: cmp(x['sort'], y['sort']))
         for record in records:
             if record.get('publicname'):
@@ -93,10 +89,10 @@ class ClicRdv(BaseResource):
         if date_end is None:
             date_end = (datetime.datetime.today() + datetime.timedelta(366)).strftime('%Y-%m-%d')
         if date_start:
-            request_uri = request_uri + '&start=%s' % urllib2.quote(date_start)
+            request_uri = request_uri + '&start=%s' % urlquote(date_start)
         if date_end:
-            request_uri = request_uri + '&end=%s' % urllib2.quote(date_end)
-        for timeslot in self.get_json(request_uri).get('availabletimeslots'):
+            request_uri = request_uri + '&end=%s' % urlquote(date_end)
+        for timeslot in self.request(request_uri).json().get('availabletimeslots'):
             timeslots.append(timeslot.get('start'))
         timeslots.sort()
         return timeslots
@@ -137,19 +133,15 @@ class ClicRdv(BaseResource):
         times.sort(lambda x,y: cmp(x.get('id'), y.get('id')))
         return times
 
-    def cancel(self, id, **kwargs):
-        appointment_id = int(id)
-        req = self.get_request('appointments/%s' % appointment_id)
-        req.get_method = (lambda: 'DELETE')
+    def cancel(self, appointment_id, **kwargs):
         try:
-            fd = urllib2.urlopen(req)
-            none = fd.read()
-        except urllib2.HTTPError as e:
+            r = self.request('DELETE', 'appointments/%s' % appointment_id)
+            r.raise_for_status()
+        except requests.exceptions.HTTPError as e:
             # clicrdv will return a "Bad Request" (HTTP 400) response
             # when it's not possible to remove an appointment
             # (for example because it's too late)
-            response = e.read()
-            response = json.loads(response)
+            response = json.loads(str(e))
             return {'success': False, 'error': response}
         return {'success': True}
 
@@ -190,14 +182,12 @@ class ClicRdv(BaseResource):
         for fieldname in (fields.keys() + extra.keys() + data.keys()):
             if fieldname.startswith('clicrdv_fiche_'):
                 appointment['fiche'][fieldname[14:]] = get_data(fieldname) or ''
-        req = self.get_request('appointments')
-        req.add_data(json.dumps({'appointment': appointment}))
-        req.add_header('Content-Type', 'application/json')
         try:
-            fd = urllib2.urlopen(req)
-        except urllib2.HTTPError, e:
+            r = self.request('appointments', 'post', json=appointment)
+            r.raise_for_status()
+        except requests.exceptions.HTTPError as e:
             try:
-                error = json.load(e.fp)[0].get('error')
+                error = json.loads(str(e))[0].get('error')
             except:
                 error = 'Unknown error (Passerelle)'
             return {
@@ -206,7 +196,7 @@ class ClicRdv(BaseResource):
             }
         else:
             success = True
-            response = json.load(fd)
+            response = r.json()
             appointment_id = response.get('records')[0].get('id')
             return {
                 'success': True,
diff --git a/tests/test_clicrdv.py b/tests/test_clicrdv.py
index 5b49a834..70ca85eb 100644
--- a/tests/test_clicrdv.py
+++ b/tests/test_clicrdv.py
@@ -1,7 +1,9 @@
 import mock
 import pytest
 import urlparse
-from StringIO import StringIO
+from requests.exceptions import HTTPError
+
+from django.contrib.contenttypes.models import ContentType
 
 from passerelle.base.models import ApiUser, AccessRight
 from passerelle.apps.clicrdv.models import ClicRdv
@@ -13,18 +15,19 @@ def connector(db):
                         password='test')
 
 
-@mock.patch('urllib2.urlopen')
-def test_urlopen_call(urlopen, app, connector):
-    urlopen.return_value = StringIO('"foo"')
-    rjson = connector.get_json('bar')
-    assert urlopen.call_count == 1
-    req = urlopen.call_args[0][0]
-    assert req.get_full_url().startswith('https://sandbox.clicrdv.com/api/v1/groups/5242/bar')
+@mock.patch('passerelle.utils.Request.request')
+def test_request_call(mocked_request, app, connector):
+    mocked_request.json.return_value = "foo"
+    connector.request('bar')
+    assert mocked_request.call_count == 1
+    req = mocked_request.call_args[0][1]
+    assert req.startswith('https://sandbox.clicrdv.com/api/v1/groups/5242/bar')
 
 
-@mock.patch('passerelle.apps.clicrdv.models.ClicRdv.get_json')
-def test_interventionsets(mocked_get, app, connector):
-    mocked_get.return_value = {
+@mock.patch('passerelle.utils.Request.request')
+def test_interventionsets(mocked_request, app, connector):
+    response = mock.Mock()
+    response.json.return_value = {
             "totalRecords": 2,
             "records": [
                 {
@@ -42,14 +45,16 @@ def test_interventionsets(mocked_get, app, connector):
                     "group_id": 5242,
                 },
             ]}
+    mocked_request.return_value = response
     resp = app.get('/clicrdv/test/interventionsets/')
     assert len(resp.json.get('data')) == 2
     assert resp.json.get('data')[0]['text'] == 'Une Demande de Passeport'
 
 
-@mock.patch('passerelle.apps.clicrdv.models.ClicRdv.get_json')
-def test_interventionsets_details(mocked_get, app, connector):
-    mocked_get.return_value = {
+@mock.patch('passerelle.utils.Request.request')
+def test_interventionsets_details(mocked_request, app, connector):
+    response = mock.Mock()
+    response.json.return_value = {
             "totalRecords": 2,
             "records": [
                 {
@@ -73,19 +78,21 @@ def test_interventionsets_details(mocked_get, app, connector):
                     "abbr": "2 demandes"
                 },
             ]}
-
+    mocked_request.return_value = response
     resp = app.get('/clicrdv/test/interventionsets/7032/')
     assert len(resp.json.get('data')) == 2
     assert resp.json.get('data')[0]['text'] == 'pour une personne'
 
-@mock.patch('urllib2.urlopen')
-def test_interventions_get_datetimes(urlopen, app, connector):
-    urlopen.return_value = StringIO('{"availabletimeslots": []}')
+@mock.patch('passerelle.utils.Request.request')
+def test_interventions_get_datetimes(mocked_request, app, connector):
+    response = mock.Mock()
+    response.json.return_value = {"availabletimeslots": []}
+    mocked_request.return_value = response
     resp = app.get('/clicrdv/test/interventions/63258/dates/')
     assert resp.json.get('data') == []
     assert resp.json.get('err') == 0
-    assert urlopen.call_count == 1
-    url = urlopen.call_args[0][0].get_full_url()
+    assert mocked_request.call_count == 1
+    url = mocked_request.call_args[0][1]
     # https://sandbox.clicrdv.com/api/v1/groups/5242/availabletimeslots?
     #   intervention_ids[]=63258&start=2016-09-21&end=2017-09-22&apikey=test&format=json
     scheme, netloc, path, params, query, fragment = urlparse.urlparse(url)
@@ -101,35 +108,38 @@ def test_interventions_get_datetimes(urlopen, app, connector):
     assert query['apikey'] == ['test']
     assert query['format'] == ['json']
 
-    urlopen.return_value = StringIO('''{"availabletimeslots": [
-            { "start": "2016-09-21 12:34:56" },
-            { "start": "2016-09-22 11:22:33" }
-        ]}''')
+    response.json.return_value = {"availabletimeslots": [
+        { "start": "2016-09-21 12:34:56" },
+        { "start": "2016-09-22 11:22:33" }
+    ]}
+    mocked_request.return_value = response
     resp = app.get('/clicrdv/test/interventions/63258/dates/').json
-    assert urlopen.call_count == 2
+    assert mocked_request.call_count == 2
     assert resp.get('err') == 0
     assert len(resp.get('data')) == 2
     assert resp['data'][0] == {'id': '2016-09-21', 'text': '21 September 2016'}
     assert resp['data'][1] == {'id': '2016-09-22', 'text': '22 September 2016'}
 
-    urlopen.return_value = StringIO('''{"availabletimeslots": [
+    response.json.return_value = {"availabletimeslots": [
             { "start": "2016-09-22 11:22:33" },
             { "start": "2016-09-21 12:34:56" }
-        ]}''') # will be sorted
+    ]} # will be sorted
+    mocked_request.return_value = response
     resp = app.get('/clicrdv/test/interventions/63258/datetimes/').json
-    assert urlopen.call_count == 3
+    assert mocked_request.call_count == 3
     assert resp.get('err') == 0
     assert len(resp.get('data')) == 2
     assert resp['data'][0] == {'id': '2016-09-21-12:34:56', 'text': '21 September 2016 12:34'}
     assert resp['data'][1] == {'id': '2016-09-22-11:22:33', 'text': '22 September 2016 11:22'}
 
-    urlopen.return_value = StringIO('''{"availabletimeslots": [
+    response.json.return_value = {"availabletimeslots": [
             { "start": "2016-09-21 12:34:56" },
             { "start": "2016-09-21 11:22:33" }
-        ]}''') # will be sorted
+    ]} # will be sorted
+    mocked_request.return_value = response
     resp = app.get('/clicrdv/test/interventions/63258/2016-09-21/times').json
-    assert urlopen.call_count == 4
-    url = urlopen.call_args[0][0].get_full_url()
+    assert mocked_request.call_count == 4
+    url = mocked_request.call_args[0][1]
     scheme, netloc, path, params, query, fragment = urlparse.urlparse(url)
     query = urlparse.parse_qs(query, keep_blank_values=True)
     assert query['start'] == ['2016-09-21 00:00:00']
@@ -138,3 +148,49 @@ def test_interventions_get_datetimes(urlopen, app, connector):
     assert len(resp.get('data')) == 2
     assert resp['data'][0] == {'id': '11:22:33', 'text': '11:22'}
     assert resp['data'][1] == {'id': '12:34:56', 'text': '12:34'}
+
+@mock.patch('passerelle.utils.Request.request')
+def test_cancel_appointment(mocked_request, app, connector):
+    obj_type = ContentType.objects.get_for_model(ClicRdv)
+    apiuser = ApiUser.objects.create(username='apiuser', keytype='API',
+                                     key='apiuser')
+    AccessRight.objects.create(codename='can_manage_appointment',
+                resource_type=obj_type, resource_pk=connector.pk,
+                apiuser=apiuser)
+
+    resp = app.get('/clicrdv/test/63258/cancel?apikey=apiuser').json
+    assert mocked_request.call_count == 1
+    assert resp['data']['success']
+
+@mock.patch('passerelle.utils.Request.request',
+            side_effect=HTTPError('{"msg": "cancel failed"}'))
+def test_failed_cancel_appointment(mocked_request, app, connector):
+    obj_type = ContentType.objects.get_for_model(ClicRdv)
+    apiuser = ApiUser.objects.create(username='apiuser', keytype='API',
+                                key='apiuser')
+    AccessRight.objects.create(codename='can_manage_appointment',
+                resource_type=obj_type, resource_pk=connector.pk,
+                apiuser=apiuser)
+    resp = app.get('/clicrdv/test/63258/cancel?apikey=apiuser').json
+    assert mocked_request.call_count == 1
+    assert resp.get('err') == 0
+    assert resp['data']
+    assert resp['data']['error'] == {'msg': 'cancel failed'}
+
+
+@mock.patch('passerelle.utils.Request.request',
+            side_effect=HTTPError('[{"error": "creation failed"}]'))
+def test_failed_appointment_creation(mocked_request, app, connector):
+    obj_type = ContentType.objects.get_for_model(ClicRdv)
+    apiuser = ApiUser.objects.create(username='apiuser', keytype='API',
+                                key='apiuser')
+    AccessRight.objects.create(codename='can_manage_appointment',
+                resource_type=obj_type, resource_pk=connector.pk,
+                apiuser=apiuser)
+
+    data = {'fields': {'clicrdv_date_raw': '2017-01-01' , 'clicrdv_time_raw': '12:00:00',
+                       'firstname': 'Foo', 'lastname': 'Bar', 'email': 'foobar@example.com'}}
+    resp = app.post_json('/clicrdv/test/interventions/63258/create?apikey=apiuser', params=data).json
+    assert resp['data']
+    assert not resp['data']['success']
+    assert resp['data']['error'] == 'creation failed'
-- 
2.25.0

