From 774b0fb0e28736cf5629639a1c016379c453436b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20P=C3=A9ters?= <fpeters@entrouvert.com>
Date: Sat, 15 Jan 2022 14:04:19 +0100
Subject: [PATCH] workflows: report labels of deleted fields in create
 form/card action (#60696)

---
 tests/admin_pages/test_workflow.py | 49 ++++++++++++++++++++++++++++++
 tests/workflow/test_formdata.py    | 17 ++++++++++-
 wcs/wf/create_formdata.py          | 26 ++++++++++++++--
 3 files changed, 88 insertions(+), 4 deletions(-)

diff --git a/tests/admin_pages/test_workflow.py b/tests/admin_pages/test_workflow.py
index 4f3471ae8..fcd689d53 100644
--- a/tests/admin_pages/test_workflow.py
+++ b/tests/admin_pages/test_workflow.py
@@ -2508,6 +2508,55 @@ def test_workflows_create_formdata_action_config(pub):
     assert "syntax error: Could not parse the remainder: '{{' from '{{'" in resp
 
 
+def test_workflows_create_formdata_deleted_field(pub):
+    create_superuser(pub)
+
+    FormDef.wipe()
+    target_formdef = FormDef()
+    target_formdef.name = 'target form'
+    target_formdef.enable_tracking_codes = True
+    target_formdef.fields = [
+        fields.StringField(id='0', label='string1', varname='foo'),
+        fields.StringField(id='1', label='string2', varname='bar'),
+    ]
+    target_formdef.store()
+
+    Workflow.wipe()
+    wf = Workflow(name='create-formdata')
+
+    st2 = wf.add_status('Resubmit')
+
+    create_formdata = CreateFormdataWorkflowStatusItem()
+    create_formdata.id = '_create_formdata'
+    create_formdata.formdef_slug = target_formdef.url_name
+    create_formdata.mappings = [
+        Mapping(field_id='0', expression='{{ "a" }}'),
+        Mapping(field_id='1', expression='{{ "b" }}'),
+    ]
+    create_formdata.parent = st2
+    st2.items.append(create_formdata)
+
+    wf.store()
+
+    app = login(get_app(pub))
+    # edit/submit to get field labels into cache
+    resp = app.get('/backoffice/workflows/%s/status/%s/items/_create_formdata/' % (wf.id, st2.id))
+    resp = resp.form.submit(name='submit')
+
+    # remove field
+    target_formdef.fields = [
+        fields.StringField(id='1', label='string2', varname='bar'),
+    ]
+    target_formdef.store()
+
+    resp = app.get('/backoffice/workflows/%s/status/%s/items/_create_formdata/' % (wf.id, st2.id))
+    assert resp.form['mappings$element1$field_id'].options == [
+        ('', False, '---'),
+        ('1', False, 'string2'),
+        ('0', True, '❗ string1 (deleted field)'),
+    ]
+
+
 def test_workflows_create_carddata_action_config(pub):
     create_superuser(pub)
 
diff --git a/tests/workflow/test_formdata.py b/tests/workflow/test_formdata.py
index ce9a66964..101c84c0f 100644
--- a/tests/workflow/test_formdata.py
+++ b/tests/workflow/test_formdata.py
@@ -106,10 +106,25 @@ def test_create_formdata(two_pubs):
         errors = two_pubs.loggederror_class.select()
         assert len(errors) == 2
         assert 'form_var_toto_string' in errors[0].exception_message
-        assert 'Missing field' in errors[1].summary
+        assert errors[1].summary == 'Missing field: unknown (1), unknown (2)'
         assert errors[0].formdata_id == str(target_formdef.data_class().select()[0].id)
         assert errors[1].formdata_id == str(target_formdef.data_class().select()[0].id)
 
+    if two_pubs.is_using_postgresql():
+        # add field labels cache
+        two_pubs.loggederror_class.wipe()
+        target_formdef.data_class().wipe()
+        create.formdef_slug = target_formdef.url_name
+        create.cached_field_labels = {'0': 'field0', '1': 'field1', '2': 'field2'}
+        wf.store()
+        del source_formdef._workflow
+        formdata.perform_workflow()
+        assert target_formdef.data_class().count() == 1
+
+        errors = two_pubs.loggederror_class.select()
+        assert len(errors) == 2
+        assert errors[1].summary == 'Missing field: field1, field2'
+
     # no tracking code has been created
     created_formdata = target_formdef.data_class().select()[0]
     assert created_formdata.tracking_code is None
diff --git a/wcs/wf/create_formdata.py b/wcs/wf/create_formdata.py
index b6f9e8375..f7faa4b16 100644
--- a/wcs/wf/create_formdata.py
+++ b/wcs/wf/create_formdata.py
@@ -22,7 +22,7 @@ from quixote import get_publisher, get_request, get_session
 from quixote.html import htmltext
 
 from wcs.formdef import FormDef
-from wcs.qommon import _
+from wcs.qommon import _, ngettext
 from wcs.qommon.form import (
     CheckboxWidget,
     CompositeWidget,
@@ -48,13 +48,17 @@ class Mapping:
 
 
 class MappingWidget(CompositeWidget):
-    def __init__(self, name, value=None, to_formdef=None, **kwargs):
+    def __init__(self, name, value=None, to_formdef=None, cached_field_labels=None, **kwargs):
         value = value or Mapping(None, '')
         self.accept_empty_value = kwargs.pop('accept_empty_value', False)
         super().__init__(name, value, **kwargs)
 
         to_fields = self._fields_to_options(to_formdef)
 
+        if value and value.field_id not in [x[0] for x in to_fields]:
+            old_label = (cached_field_labels or {}).get(value.field_id) or _('Unknown')
+            error_option = '❗ %s (%s)' % (old_label, _('deleted field'))
+            to_fields.append((value.field_id, error_option, value.field_id))
         self.add(
             SingleSelectWidget, name='field_id', title=_('Field'), value=value.field_id, options=to_fields
         )
@@ -107,6 +111,7 @@ class MappingsWidget(WidgetListAsTable):
             element_kwargs={
                 'to_formdef': to_formdef,
                 'accept_empty_value': accept_empty_value,
+                'cached_field_labels': kwargs.pop('cached_field_labels', None),
             },
             **kwargs,
         )
@@ -304,6 +309,7 @@ class CreateFormdataWorkflowStatusItem(WorkflowStatusItem):
     varname = None
     map_fields_by_varname = False
     attach_to_history = False
+    cached_field_labels = None
 
     def migrate(self):
         changed = super().migrate()
@@ -402,6 +408,7 @@ class CreateFormdataWorkflowStatusItem(WorkflowStatusItem):
                 accept_empty_value=self.accept_empty_value,
                 to_formdef=formdef,
                 value=self.mappings,
+                cached_field_labels=self.cached_field_labels,
             )
             if form.is_submitted() and get_request().form.get('map_fields_by_varname') != 'yes':
                 # do not validate form if formdef is changed and there is no mappings
@@ -452,6 +459,15 @@ class CreateFormdataWorkflowStatusItem(WorkflowStatusItem):
         else:
             form.ERROR_NOTICE = Form.ERROR_NOTICE
 
+    def submit_admin_form(self, form):
+        super().submit_admin_form(form)
+        # keep a cache of field labels, to be used in case of errors if fields are removed
+        mapped_field_ids = [x.field_id for x in self.mappings or []]
+        if self.formdef:
+            self.cached_field_labels = {
+                x.id: x.label for x in self.formdef.get_widget_fields() if x.id in mapped_field_ids
+            }
+
     def get_mappings_parameter_view_value(self):
         to_id_fields = {str(field.id): field for field in self.formdef.get_widget_fields()}
         result = []
@@ -654,7 +670,11 @@ class CreateFormdataWorkflowStatusItem(WorkflowStatusItem):
                 )
 
         if missing_fields:
-            summary = _('Missing field %r') % missing_fields
+            labels = [(self.cached_field_labels or {}).get(x, 'unknown (%s)' % x) for x in missing_fields]
+            summary = '%s %s' % (
+                ngettext('Missing field:', 'Missing fields:', len(labels)),
+                ', '.join(sorted(labels)),
+            )
             get_publisher().record_error(summary, formdata=src, status_item=self)
 
     def _set_value(self, formdata, field, value):
-- 
2.34.1

