From 1f1f47c24ca64dc9b4fc3c92b52717456845bde3 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       | 13 ++++++++++---
 hobo/matomo/views.py       |  7 +++++--
 tests/test_matomo_utils.py | 40 +++++++++++++++++++++++++-------------
 tests/test_matomo_views.py | 38 ++++++++++++++++++++++++++++++------
 4 files changed, 73 insertions(+), 25 deletions(-)

diff --git a/hobo/matomo/utils.py b/hobo/matomo/utils.py
index 96f9fcb..10ae0f7 100644
--- a/hobo/matomo/utils.py
+++ b/hobo/matomo/utils.py
@@ -307,7 +307,9 @@ def compute_cnil_acknowledgment_level(tracking_js):
     return 'success'
 
 def auto_configure_matomo():
-    """main function"""
+    """main function
+    may returns MatomoException raised by ping"""
+    warning = None
     tenant_name, site_urls = get_tenant_name_and_public_urls()
     if tenant_name is None:
         raise MatomoException("no portal-user's url available")
@@ -317,11 +319,16 @@ def auto_configure_matomo():
     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)
+
+    # simulate a first tracking visit (no matter if it fails)
+    try:
+        matomo.ping(id_site)
+    except MatomoException as exc:
+        warning = exc
 
     # 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 warning
diff --git a/hobo/matomo/views.py b/hobo/matomo/views.py
index d801def..f393d31 100644
--- a/hobo/matomo/views.py
+++ b/hobo/matomo/views.py
@@ -92,10 +92,13 @@ class EnableAutoView(FormView):
     success_url = reverse_lazy('matomo-home')
 
     def form_valid(self, form):
+        warning = None
         try:
-            auto_configure_matomo()
+            warning = auto_configure_matomo()
         except MatomoException as exc:
-            messages.error(self.request, str(exc))
+            messages.error(self.request, 'matomo: '+str(exc))
+        if warning:
+            messages.warning(self.request, 'ping: '+str(warning))
         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..1a44b7d 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)
@@ -662,7 +661,30 @@ 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
+        assert auto_configure_matomo() is None
+        logme_url_var = get_variable('matomo_logme_url')
+        assert logme_url_var.value != ''
+        tracking_js_var = get_variable('visits_tracking_js')
+        assert tracking_js_var.value == ''
+        tracking_js2_var = get_variable('cnil_compliant_visits_tracking_js')
+        assert tracking_js2_var.value != ''
+
+@mock.patch('requests.post')
+def test_auto_configure_matomo_warning(mocked_post):
+    tracking_js_var = get_variable('visits_tracking_js', 'js_code')
+
+    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')
+
+    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)
+        warning = auto_configure_matomo()
+        assert 'ping fails' in str(warning)
         logme_url_var = get_variable('matomo_logme_url')
         assert logme_url_var.value != ''
         tracking_js_var = get_variable('visits_tracking_js')
@@ -697,13 +719,3 @@ def test_auto_configure_matomo_error(mocked_post):
             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()
-        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..9afc923 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)
@@ -166,6 +165,30 @@ def test_enable_auto(mocked_post, admin_user):
                 DEL_UNKNOWN_USER, MATOMO_SUCCESS,
                 JAVASCRIPT_TAG, PING_SUCCESS]
 
+    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 the CNIL compliance message is displayed
+        assert resp3.body.find('Excellent respect of user rights') != -1
+
+@mock.patch('requests.post')
+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))
@@ -176,11 +199,14 @@ def test_enable_auto(mocked_post, admin_user):
         resp3 = resp2.follow()
         print resp3.body
 
+        # 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_failure(mocked_post, admin_user):
+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 +227,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

