From 1c4beb27d445efae45130519b2336b6c73ab0538 Mon Sep 17 00:00:00 2001
From: Nicolas ROCHE <nroche@entrouvert.com>
Date: Mon, 6 May 2019 19:56:57 +0200
Subject: [PATCH 4/4] matomo: do not abort if simulated first tracking visit
 fails (#32796)

---
 hobo/matomo/utils.py       |  8 +++-----
 hobo/matomo/views.py       | 10 ++++++++--
 tests/test_matomo_utils.py | 40 +++++++++++++++-----------------------
 tests/test_matomo_views.py | 39 ++++++++++++++++++++++++++++++-------
 4 files changed, 59 insertions(+), 38 deletions(-)

diff --git a/hobo/matomo/utils.py b/hobo/matomo/utils.py
index 1019c33..7ff0bcb 100644
--- a/hobo/matomo/utils.py
+++ b/hobo/matomo/utils.py
@@ -217,7 +217,7 @@ class MatomoWS(object):
             raise MatomoException('get_javascript_tag fails')
         return tag.text
 
-    def ping(self, id_site):
+    def create_fake_first_tracking_visit(self, id_site):
         """this function use a different matomo's webservice API"""
         url = "%s/matomo.php" % self.url_ws_base
         data = {'requests': ['?idsite=%s&action_name=ping&rec=1' % id_site]}
@@ -307,22 +307,20 @@ def compute_cnil_acknowledgment_level(tracking_js):
         return 'warning'
     return 'success'
 
-def auto_configure_matomo():
+def auto_configure_matomo(matomo):
     """main function"""
     tenant_name, site_urls = get_tenant_name_and_public_urls()
     if tenant_name is None:
         raise MatomoException("no portal-user's url available")
 
     # update matomo account
-    matomo = MatomoWS()
     id_site = upgrade_site(matomo, tenant_name, site_urls)
     logme_url = upgrade_user(matomo, tenant_name, id_site)
     tracking_js = upgrade_javascript_tag(matomo, id_site)
-    matomo.ping(id_site)
 
     # save matomo's variables
     logme_url_var = get_variable('matomo_logme_url')
     logme_url_var.value = logme_url
     logme_url_var.save()
     put_tracking_js(tracking_js)
-    return True
+    return id_site
diff --git a/hobo/matomo/views.py b/hobo/matomo/views.py
index d801def..d659be8 100644
--- a/hobo/matomo/views.py
+++ b/hobo/matomo/views.py
@@ -92,10 +92,16 @@ class EnableAutoView(FormView):
     success_url = reverse_lazy('matomo-home')
 
     def form_valid(self, form):
+        matomo = MatomoWS()
         try:
-            auto_configure_matomo()
+            id_site = auto_configure_matomo(matomo)
         except MatomoException as exc:
-            messages.error(self.request, str(exc))
+            messages.error(self.request, 'matomo: '+str(exc))
+        else:
+            try:
+                matomo.create_fake_first_tracking_visit(id_site)
+            except MatomoException as exc:
+                messages.warning(self.request, 'ping: '+str(exc))
         return super(EnableAutoView, self).form_valid(form)
 
 enable_auto = EnableAutoView.as_view()
diff --git a/tests/test_matomo_utils.py b/tests/test_matomo_utils.py
index 6976feb..69c8761 100644
--- a/tests/test_matomo_utils.py
+++ b/tests/test_matomo_utils.py
@@ -175,9 +175,8 @@ def requests_post_mocked_replies(contents):
 
         # response may be XML or JSON
         if content[0] == '{':
-            response.json = mock.MagicMock(return_value = json.loads(content))
-        else:
-            response._content = content
+            response.json = mock.MagicMock(return_value=json.loads(content))
+        response._content = content
 
         response.status_code = 200
         responses.append(response)
@@ -462,8 +461,8 @@ def test_get_javascript_tag(mocked_post):
             javascript_tag = matomo.get_javascript_tag('42')
 
 @mock.patch('requests.post')
-def test_ping(mocked_post):
-    """webservice to simulate a first tracking call"""
+def test_create_fake_first_tracking_visit(mocked_post):
+    """webservice to create a fake first tracking call"""
     with override_settings(MATOMO_SERVER=CONFIG):
         matomo = MatomoWS()
         response = Response()
@@ -473,7 +472,7 @@ def test_ping(mocked_post):
         content = PING_SUCCESS
         response.json = mock.MagicMock(return_value=json.loads(content))
         mocked_post.return_value = response
-        matomo.ping('42')
+        matomo.create_fake_first_tracking_visit('42')
         assert True
 
         # error
@@ -481,7 +480,7 @@ def test_ping(mocked_post):
         response.json = mock.MagicMock(return_value=json.loads(content))
         mocked_post.return_value = response
         with pytest.raises(MatomoError, match='ping fails'):
-            matomo.ping('42')
+            matomo.create_fake_first_tracking_visit('42')
 
         # failure (no status)
         content = PING_NOSTATUS_ERROR
@@ -489,7 +488,7 @@ def test_ping(mocked_post):
         mocked_post.return_value = response
         with pytest.raises(MatomoException,
                            match='internal error on ping \(status expected\)'):
-            matomo.ping('42')
+            matomo.create_fake_first_tracking_visit('42')
 
         # failure (no dict)
         content = PING_NODICT_ERROR
@@ -497,19 +496,19 @@ def test_ping(mocked_post):
         mocked_post.return_value = response
         with pytest.raises(MatomoException,
                            match='internal error on ping \(dict expected\)'):
-            matomo.ping('42')
+            matomo.create_fake_first_tracking_visit('42')
 
         # failure (no JSON)
         response.json = mock.MagicMock(side_effect=ValueError('not a JSON'))
         mocked_post.return_value = response
         with pytest.raises(MatomoException,
                            match='internal error on ping \(JSON expected\)'):
-            matomo.ping('42')
+            matomo.create_fake_first_tracking_visit('42')
 
         # failure (status 500)
         mocked_post.return_value.status_code = 500
         with pytest.raises(MatomoException, match='unexpected status code: 500'):
-            matomo.ping('42')
+            matomo.create_fake_first_tracking_visit('42')
 
 @mock.patch('requests.post')
 def test_upgrade_site(mocked_post):
@@ -662,7 +661,8 @@ def test_auto_configure_matomo(mocked_post):
                     DEL_UNKNOWN_USER, MATOMO_SUCCESS,
                     JAVASCRIPT_TAG, PING_SUCCESS]
         mocked_post.side_effect = requests_post_mocked_replies(contents)
-        assert auto_configure_matomo() is True
+        matomo = MatomoWS()
+        assert auto_configure_matomo(matomo) == '42'
         logme_url_var = get_variable('matomo_logme_url')
         assert logme_url_var.value != ''
         tracking_js_var = get_variable('visits_tracking_js')
@@ -677,7 +677,8 @@ def test_auto_configure_matomo_no_url():
 
     with override_settings(MATOMO_SERVER=CONFIG):
         with pytest.raises(MatomoException, match="no portal-user's url available"):
-            auto_configure_matomo()
+            matomo = MatomoWS()
+            auto_configure_matomo(matomo)
 
 @mock.patch('requests.post')
 def test_auto_configure_matomo_error(mocked_post):
@@ -694,16 +695,7 @@ def test_auto_configure_matomo_error(mocked_post):
                     JAVASCRIPT_TAG_BAD_RESPONSE]
         mocked_post.side_effect = requests_post_mocked_replies(contents)
         with pytest.raises(MatomoException, match="get_javascript_tag fails"):
-            auto_configure_matomo()
-        tracking_js_var = get_variable('visits_tracking_js')
-        assert tracking_js_var.value == 'js_code'
-
-    with override_settings(MATOMO_SERVER=CONFIG):
-        contents = [GET_NO_SITE_FROM_URL, ADD_SITE_SUCCESS,
-                    DEL_UNKNOWN_USER, MATOMO_SUCCESS,
-                    JAVASCRIPT_TAG, PING_ERROR]
-        mocked_post.side_effect = requests_post_mocked_replies(contents)
-        with pytest.raises(MatomoError, match='ping fails'):
-            auto_configure_matomo()
+            matomo = MatomoWS()
+            auto_configure_matomo(matomo)
         tracking_js_var = get_variable('visits_tracking_js')
         assert tracking_js_var.value == 'js_code'
diff --git a/tests/test_matomo_views.py b/tests/test_matomo_views.py
index 3728f44..04fd6f2 100644
--- a/tests/test_matomo_views.py
+++ b/tests/test_matomo_views.py
@@ -64,7 +64,7 @@ JAVASCRIPT_TAG = """<?xml version="1.0" encoding="utf-8" ?>
 """
 
 PING_SUCCESS = '{"status":"success","tracked":1,"invalid":0}'
-PING_ERROR = 'somethings else'
+PING_ERROR = '{"status":"not success"}'
 
 def requests_post_mocked_replies(contents):
     """buid an iterator for mock's side_effect parameter"""
@@ -74,9 +74,8 @@ def requests_post_mocked_replies(contents):
 
         # response may be XML or JSON
         if content[0] == '{':
-            response.json = mock.MagicMock(return_value = json.loads(content))
-        else:
-            response._content = content
+            response.json = mock.MagicMock(return_value=json.loads(content))
+        response._content = content
 
         response.status_code = 200
         responses.append(response)
@@ -174,13 +173,39 @@ def test_enable_auto(mocked_post, admin_user):
 
         # call utils.py::auto_configure_matomo()
         resp3 = resp2.follow()
-        print resp3.body
 
         # expect the CNIL compliance message is displayed
         assert resp3.body.find('Excellent respect of user rights') != -1
 
 @mock.patch('requests.post')
-def test_enable_auto_failure(mocked_post, admin_user):
+def test_enable_auto_warning(mocked_post, admin_user):
+    """succesfull automatic scenario having final ping failure"""
+    Combo.objects.create(base_url='https://combo.dev.publik.love',
+                         template_name='portal-user')
+    Wcs.objects.create(base_url='https://wcs.dev.publik.love')
+    Fargo.objects.create(base_url='https://fargo.dev.publik.love')
+
+    contents = [GET_NO_SITE_FROM_URL, ADD_SITE_SUCCESS,
+                DEL_UNKNOWN_USER, MATOMO_SUCCESS,
+                JAVASCRIPT_TAG, PING_ERROR]
+
+    mocked_post.side_effect = requests_post_mocked_replies(contents)
+    with override_settings(MATOMO_SERVER=CONFIG):
+        app = login(TestApp(application))
+        resp1 = app.get('/matomo/enable-auto', status=200)
+        resp2 = resp1.form.submit()
+
+        # call utils.py::auto_configure_matomo()
+        resp3 = resp2.follow()
+
+        # expect 'ping fails' warning
+        assert resp3.body.find('class="warning">ping: ping fails') != -1
+
+        # expect the CNIL compliance message is displayed
+        assert resp3.body.find('Excellent respect of user rights') != -1
+
+@mock.patch('requests.post')
+def test_enable_auto_error(mocked_post, admin_user):
     """error on automatic scenario"""
     Combo.objects.create(base_url='https://combo.dev.publik.love',
                          template_name='portal-user')
@@ -201,4 +226,4 @@ def test_enable_auto_failure(mocked_post, admin_user):
         resp3 = resp2.follow()
 
         # expect a Django error message is displayed
-        assert resp3.body.find('class="error">get_javascript_tag fails') != -1
+        assert resp3.body.find('class="error">matomo: get_javascript_tag fails') != -1
-- 
2.20.1

