From 77e98b6fe08bc5018ed8fb02dcc9b6f331583944 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20P=C3=A9ters?= <fpeters@entrouvert.com>
Date: Sat, 28 Sep 2019 16:45:24 +0200
Subject: [PATCH] misc: add options to display/hide fields on
 validation/summary pages (#36505)

---
 tests/test_admin_pages.py         |   2 +-
 tests/test_backoffice_pages.py    |  35 ++++++----
 tests/test_form_pages.py          |  32 +++++++++
 wcs/admin/workflows.py            |   6 ++
 wcs/backoffice/data_management.py |   2 +-
 wcs/backoffice/management.py      |   6 +-
 wcs/fields.py                     | 110 ++++++++++++++++++++----------
 wcs/formdef.py                    |  11 ++-
 wcs/forms/common.py               |   9 ++-
 wcs/wf/form.py                    |   2 +-
 10 files changed, 158 insertions(+), 57 deletions(-)

diff --git a/tests/test_admin_pages.py b/tests/test_admin_pages.py
index a45bd4ad9..6ec0058e9 100644
--- a/tests/test_admin_pages.py
+++ b/tests/test_admin_pages.py
@@ -2739,7 +2739,7 @@ def test_workflows_edit_display_form_action(pub):
     resp = resp.follow()
     assert 'foobar' in resp.body
     resp = resp.click('Edit')
-    assert not 'in_listing' in resp.form.fields.keys()
+    assert not 'display_locations' in resp.form.fields.keys()
     assert 'condition$type' in resp.form.fields.keys()
     resp = resp.form.submit('cancel')
     resp = resp.follow()
diff --git a/tests/test_backoffice_pages.py b/tests/test_backoffice_pages.py
index accb8691a..1d4dbec14 100644
--- a/tests/test_backoffice_pages.py
+++ b/tests/test_backoffice_pages.py
@@ -121,12 +121,12 @@ def create_environment(pub, set_receiver=True):
 
     formdef.fields = [
             fields.StringField(id='1', label='1st field', type='string',
-                in_listing=True),
+                display_locations=['validation', 'summary', 'listings']),
             fields.ItemField(id='2', label='2nd field', type='item',
                 items=['foo', 'bar', 'baz'],
-                in_listing=True),
+                display_locations=['validation', 'summary', 'listings']),
             fields.ItemField(id='3', label='3rd field', type='item',
-                data_source=datasource, in_listing=False, varname='foo'),
+                data_source=datasource, varname='foo'),
         ]
 
     formdef.store()
@@ -568,7 +568,8 @@ def test_backoffice_image_column(pub):
     create_superuser(pub)
     create_environment(pub)
     formdef = FormDef.get_by_urlname('form-title')
-    formdef.fields.append(fields.FileField(id='4', label='file field', type='file', in_listing=True))
+    formdef.fields.append(fields.FileField(id='4', label='file field', type='file',
+                display_locations=['validation', 'summary', 'listings']))
     formdef.store()
 
     upload = PicklableUpload('test.jpeg', 'image/jpeg')
@@ -654,7 +655,8 @@ def test_backoffice_bool_filter(pub):
     create_environment(pub)
     formdef = FormDef.get_by_urlname('form-title')
     formdef.fields.append(fields.BoolField(id='4', label='4th field',
-        type='bool', in_listing=True))
+        type='bool',
+        display_locations=['validation', 'summary', 'listings']))
     formdef.store()
 
     for i, formdata in enumerate(formdef.data_class().select()):
@@ -683,7 +685,8 @@ def test_backoffice_items_filter(pub):
     create_environment(pub)
     formdef = FormDef.get_by_urlname('form-title')
     formdef.fields.append(fields.ItemsField(id='4', label='4th field', type='items',
-        items=['a', 'b', 'c', 'd'], in_listing=True))
+        items=['a', 'b', 'c', 'd'],
+        display_locations=['validation', 'summary', 'listings']))
     formdef.store()
 
     for i, formdata in enumerate(formdef.data_class().select()):
@@ -735,7 +738,7 @@ def test_backoffice_csv(pub):
     assert len(resp.body.splitlines()[0].split(',')) == 7
 
     formdef = FormDef.get_by_urlname('form-title')
-    formdef.fields[-1].in_listing = True
+    formdef.fields[-1].display_locations = ['validation', 'summary', 'listings']
     formdef.store()
     resp = app.get('/backoffice/management/form-title/')
     resp = resp.click('Export as CSV File')
@@ -877,12 +880,18 @@ def test_backoffice_ods(pub):
     assert resp.body[:2] == 'PK' # ods has a zip container
 
     formdef = FormDef.get_by_urlname('form-title')
-    formdef.fields.append(fields.FileField(id='4', label='file field', type='file', in_listing=True))
-    formdef.fields.append(fields.DateField(id='5', label='date field', type='date', in_listing=True))
-    formdef.fields.append(fields.StringField(id='6', label='number field', type='string', in_listing=True))
-    formdef.fields.append(fields.StringField(id='7', label='phone field', type='string', in_listing=True))
-    formdef.fields.append(fields.DateField(id='8', label='very old field', type='date', in_listing=True))
-    formdef.fields.append(fields.StringField(id='9', label='string field', type='string', in_listing=True))
+    formdef.fields.append(fields.FileField(id='4', label='file field', type='file',
+        display_locations=['validation', 'summary', 'listings']))
+    formdef.fields.append(fields.DateField(id='5', label='date field', type='date',
+        display_locations=['validation', 'summary', 'listings']))
+    formdef.fields.append(fields.StringField(id='6', label='number field', type='string',
+        display_locations=['validation', 'summary', 'listings']))
+    formdef.fields.append(fields.StringField(id='7', label='phone field', type='string',
+        display_locations=['validation', 'summary', 'listings']))
+    formdef.fields.append(fields.DateField(id='8', label='very old field', type='date',
+        display_locations=['validation', 'summary', 'listings']))
+    formdef.fields.append(fields.StringField(id='9', label='string field', type='string',
+        display_locations=['validation', 'summary', 'listings']))
     formdef.store()
 
     formdata = formdef.data_class().select(lambda x: x.status == 'wf-new')[0]
diff --git a/tests/test_form_pages.py b/tests/test_form_pages.py
index 0b13ade6f..3ed01097d 100644
--- a/tests/test_form_pages.py
+++ b/tests/test_form_pages.py
@@ -1205,6 +1205,38 @@ def test_form_summary_empty_pages(pub):
     assert '<h3>3rd page</h3>' in resp.body
     assert '<h3>4th page</h3>' not in resp.body
 
+def test_form_display_locations(pub):
+    formdef = create_formdef()
+    formdef.fields = [
+        fields.StringField(id='1', label='string1', display_locations=[]),
+        fields.StringField(id='2', label='string2', display_locations=['validation']),
+        fields.StringField(id='3', label='string3', display_locations=['summary']),
+        fields.CommentField(id='4', label='Bla bla bla', type='comment',
+            display_locations=['validation', 'summary']),
+    ]
+    formdef.store()
+    formdef.data_class().wipe()
+
+    resp = get_app(pub).get('/test/')
+    resp.form['f1'] = 'plop1'
+    resp.form['f2'] = 'plop2'
+    resp.form['f3'] = 'plop3'
+    resp = resp.form.submit('submit')  # -> validation
+    pq = resp.pyquery.remove_namespaces()
+    assert pq('div[style="display: none;"] [name=f1]')
+    assert not pq('div[style="display: none;"] [name=f2]')
+    assert pq('div[style="display: none;"] [name=f3]')
+    assert 'Bla bla bla' in resp.body
+
+    resp = resp.form.submit('submit').follow()  # -> submit
+    assert formdef.data_class().select()[0].data['1'] == 'plop1'
+    assert formdef.data_class().select()[0].data['2'] == 'plop2'
+    assert formdef.data_class().select()[0].data['3'] == 'plop3'
+    assert not 'plop1' in resp.body
+    assert not 'plop2' in resp.body
+    assert 'plop3' in resp.body
+    assert 'Bla bla bla' in resp.body
+
 def test_form_visit_existing(pub):
     user = create_user(pub)
     formdef = create_formdef()
diff --git a/wcs/admin/workflows.py b/wcs/admin/workflows.py
index 87c6ae984..82a03e4fc 100644
--- a/wcs/admin/workflows.py
+++ b/wcs/admin/workflows.py
@@ -839,6 +839,9 @@ class WorkflowVariablesFieldDefPage(FieldDefPage):
         form.add(WorkflowVariableWidget, 'varname', title=_('Variable'),
                 value=self.field.varname, advanced=False, required=True,
                 workflow=self.objectdef.workflow)
+        display_locations = form.get_widget('display_locations')
+        if display_locations:
+            del display_locations.widgets[0]  # validation page
         return form
 
 
@@ -849,6 +852,9 @@ class WorkflowBackofficeFieldDefPage(FieldDefPage):
     def form(self):
         form = super(WorkflowBackofficeFieldDefPage, self).form()
         form.remove('prefill')
+        display_locations = form.get_widget('display_locations')
+        if display_locations:
+            del display_locations.widgets[0]  # validation page
         return form
 
 
diff --git a/wcs/backoffice/data_management.py b/wcs/backoffice/data_management.py
index 17ebbb548..d11f83963 100644
--- a/wcs/backoffice/data_management.py
+++ b/wcs/backoffice/data_management.py
@@ -99,7 +99,7 @@ class CardPage(FormPage):
     def get_default_columns(self):
         field_ids = ['id', 'time']
         for field in self.formdef.get_all_fields():
-            if hasattr(field, 'get_view_value') and field.in_listing:
+            if hasattr(field, 'get_view_value') and field.include_in_listing:
                 field_ids.append(field.id)
         return field_ids
 
diff --git a/wcs/backoffice/management.py b/wcs/backoffice/management.py
index a41f4ff23..96cc7ec31 100644
--- a/wcs/backoffice/management.py
+++ b/wcs/backoffice/management.py
@@ -398,7 +398,7 @@ class UsersViewDirectory(Directory):
         if include_email_column:
             r += htmltext('<th data-field-sort-key="email"><span>%s</span></th>') % _('Email')
         for field in formdef.get_all_fields():
-            if field.in_listing:
+            if field.include_in_listing:
                 r += htmltext('<th data-field-sort-key="f%s"><span>%s</span></th>') % (
                         field.id, field.label)
         r += htmltext('</tr>')
@@ -412,7 +412,7 @@ class UsersViewDirectory(Directory):
             if include_email_column:
                 r += htmltext('<td>%s</td>') % (user.email or '')
             for field in formdef.get_all_fields():
-                if field.in_listing:
+                if field.include_in_listing:
                     r += htmltext('<td>%s</td>') % (user.form_data.get(field.id) or '')
             r += htmltext('</tr>')
 
@@ -1232,7 +1232,7 @@ class FormPage(Directory):
     def get_default_columns(self):
         field_ids = ['id', 'time', 'last_update_time', 'user-label']
         for field in self.formdef.get_all_fields():
-            if hasattr(field, 'get_view_value') and field.in_listing:
+            if hasattr(field, 'get_view_value') and field.include_in_listing:
                 field_ids.append(field.id)
         field_ids.append('status')
         return field_ids
diff --git a/wcs/fields.py b/wcs/fields.py
index 717cc8c65..b6591327f 100644
--- a/wcs/fields.py
+++ b/wcs/fields.py
@@ -159,7 +159,7 @@ class Field(object):
     convert_value_from_str = None
     convert_value_to_str = None
     convert_value_from_anything = None
-    in_listing = False
+    display_locations = []
     prefill = None
     store_display_value = None
     store_structured_value = None
@@ -178,6 +178,18 @@ class Field(object):
     def init(cls):
         pass
 
+    @property
+    def include_in_listing(self):
+        return 'listings' in (self.display_locations or [])
+
+    @property
+    def include_in_validation_page(self):
+        return 'validation' in (self.display_locations or [])
+
+    @property
+    def include_in_summary_page(self):
+        return 'summary' in (self.display_locations or [])
+
     @property
     def unhtmled_label(self):
         charset = get_publisher().site_charset
@@ -194,7 +206,7 @@ class Field(object):
         else:
             extra_fields = []
         for attribute in self.get_admin_attributes() + extra_fields:
-            if attribute in ('in_listing',):
+            if attribute == 'display_locations':
                 continue
             if hasattr(self, attribute) and getattr(self, attribute) is not None:
                 val = getattr(self, attribute)
@@ -392,7 +404,14 @@ class Field(object):
         pass
 
     def migrate(self):
-        return False
+        changed = False
+        if getattr(self, 'in_listing', None) is not None:
+            if self.in_listing:
+                self.display_locations = self.display_locations[:]
+                self.display_locations.append('listings')
+                changed = True
+                self.in_listing = None
+        return changed
 
     def evaluate_condition(self, dict_vars, formdef, condition):
         return PageCondition(condition, {'dict_vars': dict_vars, 'formdef': formdef}).evaluate()
@@ -436,7 +455,7 @@ class Field(object):
 class WidgetField(Field):
     hint = None
     required = True
-    in_listing = False
+    display_locations = ['validation', 'summary']
     extra_attributes = []
     prefill = {}
 
@@ -494,6 +513,11 @@ class WidgetField(Field):
             else:
                 widget.extra_css_class = self.extra_css_class
 
+    def get_display_locations_options(self):
+        return [('validation', _('Validation Page')),
+                ('summary', _('Summary Page')),
+                ('listings', _('Management Listings'))]
+
     def fill_admin_form(self, form):
         form.add(StringWidget, 'label', title = _('Label'), value = self.label,
                 required = True, size = 50)
@@ -504,8 +528,10 @@ class WidgetField(Field):
                 hint=_('This is used as suffix for variable names.'))
         form.add(TextWidget, 'hint', title = _('Hint'), value = self.hint,
                 cols=60, rows=3)
-        form.add(CheckboxWidget, 'in_listing', title = _('Display in listings'),
-                value = self.in_listing, advanced=True)
+        form.add(CheckboxesWidget, 'display_locations', title=_('Display Locations'),
+                options=self.get_display_locations_options(),
+                value=self.display_locations,
+                advanced=True)
         form.add(StringWidget, 'extra_css_class', title = _('Extra classes for CSS styling'),
                 value=self.extra_css_class, size=30, advanced=(not self.extra_css_class))
         prefill_in_advanced = (not self.prefill or self.prefill.get('type') == 'none')
@@ -531,7 +557,7 @@ class WidgetField(Field):
 
     def get_admin_attributes(self):
         return Field.get_admin_attributes(self) + ['required', 'hint',
-                'varname', 'in_listing',
+                'varname', 'display_locations',
                 'extra_css_class', 'prefill']
 
     def get_csv_heading(self):
@@ -569,6 +595,7 @@ def register_field_class(klass):
 class TitleField(Field):
     key = 'title'
     description = N_('Title')
+    display_locations = ['validation', 'summary']
 
     def add_to_form(self, form, value = None):
         extra_attributes = ' data-field-id="%s"' % self.id
@@ -579,6 +606,10 @@ class TitleField(Field):
         return widget
     add_to_view_form = add_to_form
 
+    def get_display_locations_options(self):
+        return [('validation', _('Validation Page')),
+                ('summary', _('Summary Page'))]
+
     def fill_admin_form(self, form):
         form.add(StringWidget, 'label', title = _('Label'), value = self.label,
                 required = True, size = 50)
@@ -587,9 +618,13 @@ class TitleField(Field):
         form.add(ConditionWidget, 'condition', title=_('Display Condition'), value=self.condition,
                 required=False, size=50, django_only=True,
                 advanced=not(bool(self.condition)))
+        form.add(CheckboxesWidget, 'display_locations', title=_('Display Locations'),
+                options=self.get_display_locations_options(),
+                value=self.display_locations,
+                advanced=True)
 
     def get_admin_attributes(self):
-        return Field.get_admin_attributes(self) + ['extra_css_class']
+        return Field.get_admin_attributes(self) + ['extra_css_class', 'display_locations']
 
 register_field_class(TitleField)
 
@@ -613,19 +648,24 @@ register_field_class(SubtitleField)
 class CommentField(Field):
     key = 'comment'
     description = N_('Comment')
+    display_locations = []
 
-    def add_to_form(self, form, value=None):
+    def get_text(self):
         import wcs.workflows
         label = self.get_html_content()
-        label = wcs.workflows.template_on_html_string(label)
+        return wcs.workflows.template_on_html_string(label)
+
+    def add_to_form(self, form, value=None):
         widget = CommentWidget(
-                content=label,
+                content=self.get_text(),
                 extra_css_class=self.extra_css_class)
         form.widgets.append(widget)
         return widget
 
     def add_to_view_form(self, *args):
-        pass
+        if self.include_in_validation_page:
+            return self.add_to_form(*args)
+        return None
 
     def get_html_content(self):
         if not self.label:
@@ -638,6 +678,10 @@ class CommentField(Field):
             return '<p>' + label + '</p>'
         return '<p>%s</p>' % str(htmlescape(self.label))
 
+    def get_display_locations_options(self):
+        return [('validation', _('Validation Page')),
+                ('summary', _('Summary Page'))]
+
     def fill_admin_form(self, form):
         if self.label and (self.label[0] != '<' and '[end]' in self.label):
             form.add(TextWidget, 'label', title=_('Label'), value=self.label,
@@ -650,9 +694,13 @@ class CommentField(Field):
         form.add(ConditionWidget, 'condition', title=_('Display Condition'), value=self.condition,
                 required=False, size=50, django_only=True,
                 advanced=not(bool(self.condition)))
+        form.add(CheckboxesWidget, 'display_locations', title=_('Display Locations'),
+                options=self.get_display_locations_options(),
+                value=self.display_locations,
+                advanced=True)
 
     def get_admin_attributes(self):
-        return Field.get_admin_attributes(self) + ['extra_css_class']
+        return Field.get_admin_attributes(self) + ['extra_css_class', 'display_locations']
 
 
 register_field_class(CommentField)
@@ -721,10 +769,11 @@ class StringField(WidgetField):
         return str(value)
 
     def migrate(self):
+        changed = super(StringField, self).migrate()
         if isinstance(self.validation, basestring):
             self.validation = {'type': 'regex', 'value': self.validation}
-            return True
-        return False
+            changed = True
+        return changed
 
     def init_with_xml(self, element, charset, include_id=False):
         super(StringField, self).init_with_xml(element, charset, include_id=include_id)
@@ -1036,6 +1085,7 @@ class FileField(WidgetField):
         return document_types
 
     def migrate(self):
+        changed = super(FileField, self).migrate()
         if 'file_type' in self.__dict__:
             self.document_type = {}
             if self.__dict__['file_type']:
@@ -1064,8 +1114,8 @@ class FileField(WidgetField):
                         'mimetypes': file_type,
                     }
             del self.__dict__['file_type']
-            return True
-        return False
+            changed = True
+        return changed
 
     def export_to_xml(self, charset, include_id=False):
         # convert some sub-fields to strings as export_to_xml() only supports
@@ -1784,7 +1834,6 @@ register_field_class(PageField)
 class TableField(WidgetField):
     key = 'table'
     description = N_('Table')
-    in_listing = False # no way to represent data in a single cell
 
     rows = None
     columns = None
@@ -1800,12 +1849,12 @@ class TableField(WidgetField):
         kwargs['rows'] = self.rows
         kwargs['columns'] = self.columns
 
+    def get_display_locations_options(self):
+        return [('validation', _('Validation Page')),
+                ('summary', _('Summary Page'))]
+
     def fill_admin_form(self, form):
         WidgetField.fill_admin_form(self, form)
-        try:
-            form.remove('in_listing')
-        except KeyError: # perhaps it was already removed
-            pass
         try:
             form.remove('prefill')
         except KeyError: # perhaps it was already removed
@@ -1821,10 +1870,6 @@ class TableField(WidgetField):
 
     def get_admin_attributes(self):
         t = WidgetField.get_admin_attributes(self) + ['rows', 'columns']
-        try:
-            t.remove('in_listing')
-        except ValueError:
-            pass
         try:
             t.remove('prefill')
         except ValueError:
@@ -1955,7 +2000,6 @@ register_field_class(TableSelectField)
 class TableRowsField(WidgetField):
     key = 'tablerows'
     description = N_('Table with rows')
-    in_listing = False # no way to represent data in a single cell
 
     total_row = True
     columns = None
@@ -1970,12 +2014,12 @@ class TableRowsField(WidgetField):
         kwargs['columns'] = self.columns
         kwargs['add_element_label'] = _('Add row')
 
+    def get_display_locations_options(self):
+        return [('validation', _('Validation Page')),
+                ('summary', _('Summary Page'))]
+
     def fill_admin_form(self, form):
         WidgetField.fill_admin_form(self, form)
-        try:
-            form.remove('in_listing')
-        except KeyError: # perhaps it was already removed
-            pass
         try:
             form.remove('prefill')
         except KeyError: # perhaps it was already removed
@@ -1989,10 +2033,6 @@ class TableRowsField(WidgetField):
 
     def get_admin_attributes(self):
         t = WidgetField.get_admin_attributes(self) + ['columns', 'total_row']
-        try:
-            t.remove('in_listing')
-        except ValueError:
-            pass
         try:
             t.remove('prefill')
         except ValueError:
diff --git a/wcs/formdef.py b/wcs/formdef.py
index 665e495a3..a9bb2a4d3 100644
--- a/wcs/formdef.py
+++ b/wcs/formdef.py
@@ -635,7 +635,8 @@ class FormDef(StorableObject):
                 # same text as the page.
                 continue
 
-            if field.type in ('comment', 'page'):
+            if field.type == 'comment' and not field.include_in_validation_page:
+                # don't render field that wouldn't be displayed.
                 continue
 
             if not field.is_visible(dict, self):
@@ -643,7 +644,13 @@ class FormDef(StorableObject):
 
             current_page_fields.append(field)
             value = dict.get(field.id)
-            field.add_to_view_form(form, value)
+
+            if not field.include_in_validation_page:
+                form.widgets.append(HtmlWidget(htmltext('<div style="display: none;">')))
+                field.add_to_view_form(form, value)
+                form.widgets.append(HtmlWidget(htmltext('</div>')))
+            else:
+                field.add_to_view_form(form, value)
 
         if on_page:
             form.widgets.append(HtmlWidget(htmltext('</div></div>')))
diff --git a/wcs/forms/common.py b/wcs/forms/common.py
index c2a94a5dc..57dedc6bc 100644
--- a/wcs/forms/common.py
+++ b/wcs/forms/common.py
@@ -417,13 +417,16 @@ class FormStatusPage(Directory, FormTemplateMixin):
                 # same text as the page.
                 continue
 
-            if f.type in ('title', 'subtitle'):
+            if f.type in ('title', 'subtitle', 'comment') and f.include_in_summary_page:
                 current_page_fields.append({'field': f})
                 continue
 
             if not hasattr(f, 'get_view_value'):
                 continue
 
+            if not f.include_in_summary_page:
+                continue
+
             value = get_value(f)
             if value is None and not (f.required and include_unset_required_fields):
                 continue
@@ -461,6 +464,10 @@ class FormStatusPage(Directory, FormTemplateMixin):
                 r += htmltext('<div class="subtitle %s"><h4>%s</h4></div>') % (f.extra_css_class or '', f.label)
                 continue
 
+            if f.type == 'comment':
+                r += htmltext('<div class="comment-field %s">%s</div>' % (f.extra_css_class or '', f.get_text()))
+                continue
+
             css_classes = ['field', 'field-type-%s' % f.key]
             if f.extra_css_class:
                 css_classes.append(f.extra_css_class)
diff --git a/wcs/wf/form.py b/wcs/wf/form.py
index 4f0b6074c..dfe8cee6a 100644
--- a/wcs/wf/form.py
+++ b/wcs/wf/form.py
@@ -56,7 +56,7 @@ class WorkflowFormFieldsFormDef(FormDef):
 
 class WorkflowFormFieldDefPage(FieldDefPage):
     section = 'workflows'
-    blacklisted_attributes = ['in_listing']
+    blacklisted_attributes = ['display_locations']
 
     def get_deletion_extra_warning(self):
         return None
-- 
2.23.0

