workflows: allow setting backoffice fields to an empty value (#44310)

tests/test_widgets.py

@@ -541,6 +541,21 @@ def test_computed_expression_widget():
     assert widget.parse() == None
     assert not widget.has_error()
 
+    # python value left empty
+    widget = ComputedExpressionWidget('test')
+    mock_form_submission(req, widget, {'test$value_python': '', 'test$type': ['python']})
+    assert not widget.has_error()
+
+    widget = ComputedExpressionWidget('test')
+    mock_form_submission(req, widget, {'test$type': ['python']})
+    assert not widget.has_error()
+
+    # but field marqued as required
+    widget = ComputedExpressionWidget('test', required=True)
+    mock_form_submission(req, widget, {'test$value_python': '', 'test$type': ['python']})
+    assert widget.has_error()
+
+    # invalid values
     widget = ComputedExpressionWidget('test')
     mock_form_submission(req, widget, {'test$value_python': 'hello world', 'test$type': ['python']})
     assert widget.has_error()
@@ -551,16 +566,6 @@ def test_computed_expression_widget():
     assert widget.has_error()
     assert widget.get_error().startswith('syntax error')
 
-    widget = ComputedExpressionWidget('test')
-    mock_form_submission(req, widget, {'test$value_python': '', 'test$type': ['python']})
-    assert widget.has_error()
-    assert widget.get_error().startswith('syntax error')
-
-    widget = ComputedExpressionWidget('test')
-    mock_form_submission(req, widget, {'test$type': ['python']})
-    assert widget.has_error()
-    assert widget.get_error().startswith('syntax error')
-
     widget = ComputedExpressionWidget('test')
     mock_form_submission(req, widget, {'test$value_python': '{{form_var_foo}}', 'test$type': ['python']})
     assert widget.has_error()

tests/test_workflows.py

@@ -3761,6 +3761,16 @@ def test_set_backoffice_field(http_requests, two_pubs):
     formdata = formdef.data_class().get(formdata.id)
     assert formdata.data['bo1'] == 'X PLOP'
 
+    item.fields = [{'field_id': 'bo1', 'value': None}]
+    item.perform(formdata)
+    formdata = formdef.data_class().get(formdata.id)
+    assert formdata.data['bo1'] is None
+
+    item.fields = [{'field_id': 'bo1', 'value': ''}]
+    item.perform(formdata)
+    formdata = formdef.data_class().get(formdata.id)
+    assert formdata.data['bo1'] is None
+
     assert LoggedError.count() == 0
 
     item.fields = [{'field_id': 'bo1', 'value': '= ~ invalid python ~'}]
@@ -3922,6 +3932,17 @@ def test_set_backoffice_field_file(http_requests, two_pubs):
     assert formdata.data['bo1'].base_filename == 'hello.txt'
     assert formdata.data['bo1'].get_content() == b'hello world'
 
+    # check resetting a value
+    for value in ('', None):
+        item = SetBackofficeFieldsWorkflowStatusItem()
+        item.parent = st1
+        item.fields = [{'field_id': 'bo1', 'value': value}]
+        item.perform(formdata)
+
+        formdata = formdef.data_class().get(formdata.id)
+        assert formdata.data['bo1'] is None
+
+    # set from base64 content
     item = SetBackofficeFieldsWorkflowStatusItem()
     item.parent = st1
     item.fields = [{'field_id': 'bo1',
@@ -3933,8 +3954,8 @@ def test_set_backoffice_field_file(http_requests, two_pubs):
     assert formdata.data['bo1'].get_content() == b'HELLO WORLD'
 
     hello_world = formdata.data['bo1']
-    # check wrong value, or None (no file)
-    for value in ('="HELLO"', '', 'BAD', '={}', '=[]', None):
+    # check wrong value
+    for value in ('="HELLO"', 'BAD', '={}', '=[]'):
         formdata.data['bo1'] = hello_world
         formdata.store()
 
@@ -3946,17 +3967,13 @@ def test_set_backoffice_field_file(http_requests, two_pubs):
         item.perform(formdata)
 
         formdata = formdef.data_class().get(formdata.id)
-        if value is not None:  # wrong value : do nothing
-            assert formdata.data['bo1'].base_filename == 'hello.txt'
-            assert formdata.data['bo1'].get_content() == b'HELLO WORLD'
-            assert LoggedError.count() == 1
-            logged_error = LoggedError.select()[0]
-            assert logged_error.summary.startswith('Failed to convert')
-            assert logged_error.formdata_id == str(formdata.id)
-            assert logged_error.exception_class == 'ValueError'
-        else:  # empty value : remove field
-            assert formdata.data['bo1'] is None
-            assert LoggedError.count() == 0
+        assert formdata.data['bo1'].base_filename == 'hello.txt'
+        assert formdata.data['bo1'].get_content() == b'HELLO WORLD'
+        assert LoggedError.count() == 1
+        logged_error = LoggedError.select()[0]
+        assert logged_error.summary.startswith('Failed to convert')
+        assert logged_error.formdata_id == str(formdata.id)
+        assert logged_error.exception_class == 'ValueError'
 
     # check wrong field
     item = SetBackofficeFieldsWorkflowStatusItem()
@@ -3965,7 +3982,6 @@ def test_set_backoffice_field_file(http_requests, two_pubs):
     item.perform(formdata)
 
     formdata = formdef.data_class().get(formdata.id)
-    assert formdata.data.get('bo1') is None
     assert formdata.data.get('bo3') is None
 
 

wcs/qommon/form.py

@@ -2418,6 +2418,8 @@ class ComputedExpressionWidget(CompositeWidget):
         else:
             from wcs.workflows import WorkflowStatusItem
             value = WorkflowStatusItem.get_expression(value)
+
+        value_placeholder = kwargs.pop('value_placeholder', None)
         CompositeWidget.__init__(self, name, value, **kwargs)
 
         options = [
@@ -2427,13 +2429,16 @@ class ComputedExpressionWidget(CompositeWidget):
         ]
 
         self.add(StringWidget, 'value_text', size=80,
-                 value=value.get('value') if value.get('type') == 'text' else None)
+                 value=value.get('value') if value.get('type') == 'text' else None,
+                 placeholder=value_placeholder)
 
         self.add(StringWidget, 'value_template', size=80,
-                 value=value.get('value') if value.get('type') == 'template' else None)
+                 value=value.get('value') if value.get('type') == 'template' else None,
+                 placeholder=value_placeholder)
 
         self.add(StringWidget, 'value_python', size=80,
-                 value=value.get('value') if value.get('type') == 'python' else None)
+                 value=value.get('value') if value.get('type') == 'python' else None,
+                 placeholder=value_placeholder)
 
         self.add(SingleSelectWidget, 'type', options=options,
                  value=value.get('type'),
@@ -2501,7 +2506,7 @@ class ComputedExpressionWidget(CompositeWidget):
             return
         value_type = self.get('type')
         value_content = self.get('value_%s' % value_type)
-        if value_type == 'python':
+        if value_type == 'python' and value_content:
             self.value = '=' + (value_content or '')
         else:
             self.value = value_content

wcs/wf/backoffice_fields.py

@@ -40,10 +40,11 @@ class SetBackofficeFieldRowWidget(CompositeWidget):
                 value=value.get('field_id'),
                 options=fields, **kwargs)
         self.add(ComputedExpressionWidget, name='value', title=_('Value'),
-                value=value.get('value'))
+                value=value.get('value'),
+                value_placeholder=_('Leaving the field blank will empty the value.'))
 
     def _parse(self, request):
-        if self.get('value') and self.get('field_id'):
+        if self.get('field_id'):
             self.value = {
                 'value': self.get('value'),
                 'field_id': self.get('field_id')
@@ -112,6 +113,11 @@ class SetBackofficeFieldsWorkflowStatusItem(WorkflowStatusItem):
             except:
                 continue
 
+            if new_value == '':
+                # assign empty strings as None, as that will work for all field
+                # types
+                new_value = None
+
             if formdef_field.convert_value_from_anything:
                 try:
                     new_value = formdef_field.convert_value_from_anything(new_value)