From 663bc40c0154c82f490944eedabd67e24d9d772f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laur=C3=A9line=20Gu=C3=A9rin?= <zebuline@entrouvert.com>
Date: Fri, 18 Sep 2020 10:45:56 +0200
Subject: [PATCH 1/2] workflow: wscall item configuration to notify and record
 errors (#19662)

---
 tests/test_backoffice_pages.py      | 26 ++++++++++++++++++-----
 tests/test_workflows_admin_pages.py | 33 +++++++++++++++++++++++++++++
 tests/test_wscall.py                |  2 +-
 wcs/wf/wscall.py                    | 17 ++++++++++-----
 wcs/wscalls.py                      |  2 +-
 5 files changed, 68 insertions(+), 12 deletions(-)

diff --git a/tests/test_backoffice_pages.py b/tests/test_backoffice_pages.py
index 42e6ec3a..b2a33754 100644
--- a/tests/test_backoffice_pages.py
+++ b/tests/test_backoffice_pages.py
@@ -3561,11 +3561,13 @@ def test_backoffice_wscall_failure_display(http_requests, pub):
     assert not 'Error during webservice call' in resp.text
 
 
-def test_backoffice_wscall_error_email(http_requests, pub, emails):
+@pytest.mark.parametrize('notify_on_errors', [True, False])
+@pytest.mark.parametrize('record_on_errors', [True, False])
+def test_backoffice_wscall_on_error(http_requests, pub, emails, notify_on_errors, record_on_errors):
     pub.cfg['debug'] = {'error_email': 'errors@localhost.invalid'}
     pub.write_cfg()
 
-    user = create_user(pub)
+    create_user(pub)
     create_environment(pub)
     formdef = FormDef.get_by_urlname('form-title')
     form_class = formdef.data_class()
@@ -3581,6 +3583,8 @@ def test_backoffice_wscall_error_email(http_requests, pub, emails):
     wscall.varname = 'xxx'
     wscall.url = 'http://remote.example.net/xml'
     wscall.action_on_bad_data = ':stop'
+    wscall.notify_on_errors = notify_on_errors
+    wscall.record_on_errors = record_on_errors
     wscall.record_errors = True
     st1.items.append(wscall)
     wscall.parent = st1
@@ -3608,9 +3612,21 @@ def test_backoffice_wscall_error_email(http_requests, pub, emails):
     assert 'Error during webservice call' in resp.text
 
     # check email box
-    error_email = emails.get('[ERROR] [WSCALL] json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)')
-    assert '/form-title/%s/' % number31.id in error_email['payload']
-    assert error_email['msg']['References']
+    if notify_on_errors:
+        assert emails.count() == 1
+        error_email = emails.get('[ERROR] [WSCALL] json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)')
+        assert '/form-title/%s/' % number31.id in error_email['payload']
+        if record_on_errors:
+            assert error_email['msg']['References']
+    else:
+        assert emails.count() == 0
+
+    # check LoggedError
+    if record_on_errors:
+        assert LoggedError.count() == 1
+        LoggedError.wipe()
+    else:
+        assert LoggedError.count() == 0
 
 
 def test_backoffice_wscall_attachment(http_requests, pub):
diff --git a/tests/test_workflows_admin_pages.py b/tests/test_workflows_admin_pages.py
index 53863911..ef9e4a13 100644
--- a/tests/test_workflows_admin_pages.py
+++ b/tests/test_workflows_admin_pages.py
@@ -1592,6 +1592,39 @@ def test_workflows_wscall_label(pub):
     assert 'Webservice (foowscallbar)' in resp.text
 
 
+@pytest.mark.parametrize('value', [True, False])
+def test_workflows_wscall_options(pub, value):
+    create_superuser(pub)
+    create_role()
+
+    Workflow.wipe()
+    workflow = Workflow(name='foo')
+    baz_status = workflow.add_status(name='baz')
+    wscall = WebserviceCallStatusItem()
+    wscall.parent = baz_status
+    baz_status.items.append(wscall)
+    workflow.store()
+
+    app = login(get_app(pub))
+    resp = app.get('/backoffice/workflows/%s/status/%s/items/1/' % (workflow.id, baz_status.id))
+    assert resp.form['notify_on_errors'].value is None
+    assert resp.form['record_on_errors'].value == 'yes'
+    resp.form['notify_on_errors'] = value
+    resp.form['record_on_errors'] = value
+    resp = resp.form.submit('submit').follow()
+
+    resp = app.get('/backoffice/workflows/%s/status/%s/items/1/' % (workflow.id, baz_status.id))
+    assert resp.form['notify_on_errors'].value == ('yes' if value else None)
+    assert resp.form['record_on_errors'].value == ('yes' if value else None)
+    resp.form['notify_on_errors'] = not value
+    resp.form['record_on_errors'] = not value
+    resp = resp.form.submit('submit').follow()
+
+    resp = app.get('/backoffice/workflows/%s/status/%s/items/1/' % (workflow.id, baz_status.id))
+    assert resp.form['notify_on_errors'].value == ('yes' if not value else None)
+    assert resp.form['record_on_errors'].value == ('yes' if not value else None)
+
+
 def test_workflows_inspect_view(pub):
     from wcs.workflows import WorkflowVariablesFieldsFormDef
     from wcs.wf.form import FormWorkflowStatusItem, WorkflowFormFieldsFormDef
diff --git a/tests/test_wscall.py b/tests/test_wscall.py
index ae0ed4ee..a33cea6c 100644
--- a/tests/test_wscall.py
+++ b/tests/test_wscall.py
@@ -212,7 +212,7 @@ def test_webservice_on_error(http_requests, pub, emails, notify_on_errors, recor
         assert 'Foo Bar ' in resp.text
         if notify_on_errors:
             assert emails.count() == 1
-            assert "[ERROR] ['WSCALL'] Exception: %s whatever" % status_code in emails.emails
+            assert "[ERROR] [WSCALL] Exception: %s whatever" % status_code in emails.emails
             emails.empty()
         else:
             assert emails.count() == 0
diff --git a/wcs/wf/wscall.py b/wcs/wf/wscall.py
index e77fa8ad..7c81fa97 100644
--- a/wcs/wf/wscall.py
+++ b/wcs/wf/wscall.py
@@ -117,7 +117,8 @@ class WebserviceCallStatusItem(WorkflowStatusItem):
     action_on_5xx = ':stop'
     action_on_bad_data = ':pass'
     action_on_network_errors = ':stop'
-    notify_on_errors = True
+    notify_on_errors = False
+    record_on_errors = True
     record_errors = False
 
     @property
@@ -152,7 +153,7 @@ class WebserviceCallStatusItem(WorkflowStatusItem):
                 'response_type', 'varname', 'backoffice_filefield_id',
                 'action_on_app_error', 'action_on_4xx', 'action_on_5xx', 'action_on_bad_data',
                 'action_on_network_errors',
-                'notify_on_errors', 'record_errors',
+                'notify_on_errors', 'record_on_errors', 'record_errors',
                 'condition')
 
     def add_parameters_widgets(self, form, parameters, prefix='', formdef=None):
@@ -273,6 +274,11 @@ class WebserviceCallStatusItem(WorkflowStatusItem):
                     title=_('Notify on errors'),
                     value=self.notify_on_errors)
 
+        if 'record_on_errors' in parameters:
+            form.add(CheckboxWidget, '%srecord_on_errors' % prefix,
+                     title=_('Record on errors'),
+                     value=self.record_on_errors)
+
         if 'record_errors' in parameters:
             form.add(CheckboxWidget, '%srecord_errors' % prefix,
                     title=_('Record errors in the log'),
@@ -389,7 +395,7 @@ class WebserviceCallStatusItem(WorkflowStatusItem):
             formdata.evolution[-1].add_part(attachment)
 
     def action_on_error(self, action, formdata, response=None, data=None, exc_info=None):
-        if action in (':pass', ':stop') and (self.notify_on_errors or self.record_errors):
+        if action in (':pass', ':stop') and (self.notify_on_errors or self.record_on_errors or self.record_errors):
             if exc_info:
                 summary = traceback.format_exception_only(exc_info[0], exc_info[1])[-1]
             else:
@@ -401,8 +407,9 @@ class WebserviceCallStatusItem(WorkflowStatusItem):
                 except Exception as e:
                     exc_info = sys.exc_info()
 
-            if self.notify_on_errors:
-                get_publisher().notify_of_exception(exc_info, context='[WSCALL]')
+            if self.notify_on_errors or self.record_on_errors:
+                get_publisher().notify_of_exception(
+                    exc_info, context='[WSCALL]', notify=self.notify_on_errors, record=self.record_on_errors)
             if self.record_errors and formdata.evolution:
                 formdata.evolution[-1].add_part(
                         JournalWsCallErrorPart(summary, self.label, data))
diff --git a/wcs/wscalls.py b/wcs/wscalls.py
index 447e410e..400f505b 100644
--- a/wcs/wscalls.py
+++ b/wcs/wscalls.py
@@ -155,7 +155,7 @@ def call_webservice(
         except Exception:
             exc_info = sys.exc_info()
         get_publisher().notify_of_exception(
-            exc_info, context=['WSCALL'], notify=notify_on_errors, record=record_on_errors)
+            exc_info, context='[WSCALL]', notify=notify_on_errors, record=record_on_errors)
 
     return (response, status, data)
 
-- 
2.28.0

