From 59a513d369f5d499474d09fae79a8d273a00c91a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laur=C3=A9line=20Gu=C3=A9rin?= <zebuline@entrouvert.com>
Date: Tue, 16 Mar 2021 11:57:05 +0100
Subject: [PATCH] geolocation: record LoggedError on address string error
 (#51773)

---
 tests/test_workflows.py | 51 +++++++++++++++++++++++++----------------
 wcs/wf/geolocate.py     | 12 ++++++----
 2 files changed, 39 insertions(+), 24 deletions(-)

diff --git a/tests/test_workflows.py b/tests/test_workflows.py
index 81d33c73..102a2261 100644
--- a/tests/test_workflows.py
+++ b/tests/test_workflows.py
@@ -3159,7 +3159,9 @@ def test_criticality(pub):
     assert formdata.get_criticality_level_object().name == 'green'
 
 
-def test_geolocate_address(pub):
+def test_geolocate_address(two_pubs):
+    if two_pubs.is_using_postgresql():
+        two_pubs.loggederror_class.wipe()
     formdef = FormDef()
     formdef.geolocations = {'base': 'bla'}
     formdef.name = 'baz'
@@ -3172,7 +3174,7 @@ def test_geolocate_address(pub):
     formdata.data = {'1': '169 rue du chateau'}
     formdata.just_created()
     formdata.store()
-    pub.substitutions.feed(formdata)
+    two_pubs.substitutions.feed(formdata)
 
     item = GeolocateWorkflowStatusItem()
     item.method = 'address_string'
@@ -3191,8 +3193,8 @@ def test_geolocate_address(pub):
         assert int(formdata.geolocations['base']['lat']) == 48
         assert int(formdata.geolocations['base']['lon']) == 2
 
-    pub.load_site_options()
-    pub.site_options.set('options', 'nominatim_key', 'KEY')
+    two_pubs.load_site_options()
+    two_pubs.site_options.set('options', 'nominatim_key', 'KEY')
     with mock.patch('wcs.wf.geolocate.http_get_page') as http_get_page:
         http_get_page.return_value = (
             None,
@@ -3207,8 +3209,8 @@ def test_geolocate_address(pub):
         assert int(formdata.geolocations['base']['lat']) == 48
         assert int(formdata.geolocations['base']['lon']) == 2
 
-    pub.load_site_options()
-    pub.site_options.set('options', 'geocoding_service_url', 'http://example.net/')
+    two_pubs.load_site_options()
+    two_pubs.site_options.set('options', 'geocoding_service_url', 'http://example.net/')
     with mock.patch('wcs.wf.geolocate.http_get_page') as http_get_page:
         http_get_page.return_value = (
             None,
@@ -3219,7 +3221,7 @@ def test_geolocate_address(pub):
         item.perform(formdata)
         assert 'http://example.net/?q=' in http_get_page.call_args[0][0]
 
-    pub.site_options.set('options', 'geocoding_service_url', 'http://example.net/?param=value')
+    two_pubs.site_options.set('options', 'geocoding_service_url', 'http://example.net/?param=value')
     with mock.patch('wcs.wf.geolocate.http_get_page') as http_get_page:
         http_get_page.return_value = (
             None,
@@ -3235,6 +3237,19 @@ def test_geolocate_address(pub):
     formdata.geolocations = None
     item.perform(formdata)
     assert formdata.geolocations == {}
+    if two_pubs.is_using_postgresql():
+        assert two_pubs.loggederror_class.count() == 1
+        logged_error = two_pubs.loggederror_class.select()[0]
+        assert (
+            logged_error.summary
+            == 'error in template for address string [syntax error in ezt template: unclosed block at line 1 and column 24]'
+        )
+        assert logged_error.formdata_id == str(formdata.id)
+        assert logged_error.exception_class == 'TemplateError'
+        assert (
+            logged_error.exception_message
+            == 'syntax error in ezt template: unclosed block at line 1 and column 24'
+        )
 
     # check for None
     item.address_string = '=None'
@@ -3242,19 +3257,8 @@ def test_geolocate_address(pub):
     item.perform(formdata)
     assert formdata.geolocations == {}
 
-    # check for nominatim server error
-    formdata.geolocations = None
-    with mock.patch('wcs.wf.geolocate.http_get_page') as http_get_page:
-        http_get_page.return_value = (
-            None,
-            500,
-            force_bytes(json.dumps([{'lat': '48.8337085', 'lon': '2.3233693'}])),
-            None,
-        )
-        item.perform(formdata)
-        assert formdata.geolocations == {}
-
     # check for nominatim returning an empty result set
+    item.address_string = '[form_var_string], paris, france'
     formdata.geolocations = None
     with mock.patch('wcs.wf.geolocate.http_get_page') as http_get_page:
         http_get_page.return_value = (None, 200, force_bytes(json.dumps([])), None)
@@ -3271,9 +3275,16 @@ def test_geolocate_address(pub):
     # check for nominatim connection error
     formdata.geolocations = None
     with mock.patch('wcs.wf.geolocate.http_get_page') as http_get_page:
-        http_get_page.side_effect = ConnectionError
+        http_get_page.side_effect = ConnectionError('some error')
         item.perform(formdata)
         assert formdata.geolocations == {}
+    if two_pubs.is_using_postgresql():
+        assert two_pubs.loggederror_class.count() == 2
+        logged_error = two_pubs.loggederror_class.select()[1]
+        assert logged_error.summary == 'error calling geocoding service [some error]'
+        assert logged_error.formdata_id == str(formdata.id)
+        assert logged_error.exception_class == 'ConnectionError'
+        assert logged_error.exception_message == 'some error'
 
 
 def test_geolocate_image(pub):
diff --git a/wcs/wf/geolocate.py b/wcs/wf/geolocate.py
index 90d7bc93..86ee2799 100644
--- a/wcs/wf/geolocate.py
+++ b/wcs/wf/geolocate.py
@@ -136,7 +136,9 @@ class GeolocateWorkflowStatusItem(WorkflowStatusItem):
             try:
                 address = self.compute(self.address_string, raises=True)
             except Exception as e:
-                get_logger().error('error in template for address string [%r]', e)
+                get_publisher().record_error(
+                    _('error in template for address string [%s]') % str(e), formdata=formdata, exception=e
+                )
                 return
         else:
             # this is when the action is being executed to prefill a map field;
@@ -158,15 +160,17 @@ class GeolocateWorkflowStatusItem(WorkflowStatusItem):
         try:
             response, status, data, auth_header = http_get_page(url, raise_on_http_errors=True)
         except ConnectionError as e:
-            get_logger().error('error calling geocoding service [%s]', str(e))
+            get_publisher().record_error(
+                _('error calling geocoding service [%s]') % str(e), formdata=formdata, exception=e
+            )
             return
         try:
             data = json.loads(force_str(data))
         except ValueError:
-            get_logger().error('non-JSON response from geocoding service')
+            get_logger().debug('non-JSON response from geocoding service')
             return
         if len(data) == 0 or isinstance(data, dict):
-            get_logger().info('error finding location')
+            get_logger().debug('error finding location')
             return
         coords = data[0]
         return normalize_geolocation({'lon': coords['lon'], 'lat': coords['lat']})
-- 
2.30.1

