From 585b9606bd19b85ae73da600c01e338e06df604e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20P=C3=A9ters?= <fpeters@entrouvert.com>
Date: Thu, 7 Jun 2018 15:19:43 +0200
Subject: [PATCH] forms: only include pages with actual data in summary
 (#24048)

---
 tests/test_form_pages.py | 65 ++++++++++++++++++++++++++++++
 wcs/forms/common.py      | 87 +++++++++++++++++++++++-----------------
 2 files changed, 115 insertions(+), 37 deletions(-)

diff --git a/tests/test_form_pages.py b/tests/test_form_pages.py
index 165ca11b..6091303b 100644
--- a/tests/test_form_pages.py
+++ b/tests/test_form_pages.py
@@ -1041,12 +1041,77 @@ def test_form_titles(pub):
     assert '<h3>1st page</h3>' in resp.body
     assert '<h4>subtitle of 1st page</h4>' in resp.body
     assert '<h3>title of second page</h3>' in resp.body
+    resp.form['f3'] = 'foo'
     resp = resp.form.submit('submit').follow()  # -> submit
     assert '<h3>1st page</h3>' in resp.body
     assert not '<div class="title "><h3>1st page</h3></div>' in resp.body
     assert '<div class="subtitle "><h4>subtitle of 1st page</h4></div>' in resp.body
     assert '<div class="title "><h3>title of second page</h3></div>' in resp.body
 
+def test_form_summary_empty_pages(pub):
+    user = create_user(pub)
+    formdef = create_formdef()
+    formdef.fields = [
+        fields.PageField(id='0', label='1st page', type='page'),
+        fields.StringField(id='1', label='string', varname='toto'),
+        fields.PageField(id='2', label='2nd page', type='page',
+            condition={'type': 'python', 'value': 'form_var_toto == "foo"'}),
+        fields.TitleField(id='6', label='title in second page', type='title'),
+        fields.StringField(id='3', label='string'),
+        fields.PageField(id='4', label='3rd page', type='page'),
+        fields.StringField(id='5', label='string'),
+        fields.PageField(id='7', label='4th page', type='page'),
+        fields.CommentField(id='8', label='Bla bla bla', type='comment'),
+    ]
+    formdef.store()
+    formdef.data_class().wipe()
+
+    app = login(get_app(pub), username='foo', password='foo')
+
+    resp = app.get('/test/')  # -> 1st page
+    resp.form['f1'] = 'foo'
+    resp = resp.form.submit('submit')  # -> 2nd page
+    resp.form['f3'] = 'bar'
+    resp = resp.form.submit('submit')  # -> 3rd page
+    resp.form['f5'] = 'baz'
+    resp = resp.form.submit('submit')  # -> 4th page
+    resp = resp.form.submit('submit')  # -> validation
+    resp = resp.form.submit('submit')
+    formdata_id = resp.location.split('/')[-2]
+    resp = resp.follow()  # -> submit
+    assert '<h3>1st page</h3>' in resp.body
+    assert '<h3>2nd page</h3>' in resp.body
+    assert '<h3>3rd page</h3>' in resp.body
+    assert '<h3>4th page</h3>' not in resp.body
+
+    resp = app.get('/test/')  # -> 1st page
+    resp.form['f1'] = 'foo'
+    resp = resp.form.submit('submit')  # -> 2nd page
+    resp.form['f3'] = 'bar'
+    resp = resp.form.submit('previous')  # -> 1st page
+    resp.form['f1'] = 'baz'
+    resp = resp.form.submit('submit')  # -> 3rd page
+    resp.form['f5'] = 'baz'
+    resp = resp.form.submit('submit')  # -> 4th page
+    resp = resp.form.submit('submit')  # -> validation
+    resp = resp.form.submit('submit').follow()  # -> submit
+    assert '<h3>1st page</h3>' in resp.body
+    assert '<h3>2nd page</h3>' not in resp.body
+    assert '<h3>3rd page</h3>' in resp.body
+    assert '<h3>4th page</h3>' not in resp.body
+
+    # change condition to have second page never displayed
+    formdef.fields[2].condition['value'] = False
+    formdef.store()
+    formdata = formdef.data_class().get(formdata_id)
+    resp = app.get(formdata.get_url())
+    # it was filled by user, it should still appear (conditions should not be
+    # replayed)
+    assert '<h3>1st page</h3>' in resp.body
+    assert '<h3>2nd page</h3>' in resp.body
+    assert '<h3>3rd page</h3>' in resp.body
+    assert '<h3>4th page</h3>' not in resp.body
+
 def test_form_visit_existing(pub):
     user = create_user(pub)
     formdef = create_formdef()
diff --git a/wcs/forms/common.py b/wcs/forms/common.py
index 920a8c68..93808e62 100644
--- a/wcs/forms/common.py
+++ b/wcs/forms/common.py
@@ -332,26 +332,60 @@ class FormStatusPage(Directory, FormTemplateMixin):
         r = TemplateIO(html=True)
         on_page = False
         on_disabled_page = False
+        pages = []
+        current_page_fields = []
+
+        def get_value(f):
+            if not self.filled.data.has_key(f.id):
+                value = None
+            else:
+                if f.store_display_value and ('%s_display' % f.id) in self.filled.data:
+                    value = self.filled.data['%s_display' % f.id]
+                else:
+                    value = self.filled.data[f.id]
+
+                if value is None or value == '':
+                    value = None
+            return value
+
         for i, f in enumerate(fields):
             if f.type == 'page':
-                on_disabled_page = False
-                if not f.is_visible(self.filled.data, self.formdef):
-                    on_disabled_page = True
-
-                form_field = False
-                for f1 in self.formdef.fields[self.formdef.fields.index(f)+1:]:
-                    if f1.key == 'page':
-                        break
-                    if isinstance(f1, WidgetField):
-                        form_field = True
-                        break
-                if form_field is False:
-                    on_disabled_page = True
-
-            if on_disabled_page:
+                on_page = f
+                current_page_fields = []
+                pages.append({'page': f, 'fields': current_page_fields})
+                continue
+
+            if f.type == 'title' and on_page and fields[i-1] is on_page and on_page.label == f.label:
+                # don't include first title of a page if that title has the
+                # same text as the page.
                 continue
 
+            if f.type in ('title', 'subtitle'):
+                current_page_fields.append({'field': f})
+                continue
+
+            if not hasattr(f, 'get_view_value'):
+                continue
+
+            value = get_value(f)
+            if value is None and not (f.required and include_unset_required_fields):
+                continue
 
+            current_page_fields.append({'field': f, 'value': value})
+
+        if not pages:
+            fields = [x['field'] for x in current_page_fields]
+        else:
+            # ignore empty pages
+            fields = []
+            for page in pages:
+                if not any([x.has_key('value') for x in page['fields']]):
+                    continue
+                fields.append(page['page'])
+                fields.extend([x['field'] for x in page['fields']])
+
+        on_page = None
+        for f in fields:
             if f.type == 'page':
                 if on_page:
                     r += htmltext('</div>')
@@ -362,11 +396,6 @@ class FormStatusPage(Directory, FormTemplateMixin):
                 on_page = f
                 continue
 
-            if f.type == 'title' and on_page and fields[i-1] is on_page and on_page.label == f.label:
-                # don't include first title of a page if that title has the
-                # same text as the page.
-                continue
-
             if f.type == 'title':
                 r += htmltext('<div class="title %s"><h3>%s</h3></div>') % (f.extra_css_class or '', f.label)
                 continue
@@ -375,28 +404,12 @@ class FormStatusPage(Directory, FormTemplateMixin):
                 r += htmltext('<div class="subtitle %s"><h4>%s</h4></div>') % (f.extra_css_class or '', f.label)
                 continue
 
-            if not hasattr(f, str('get_view_value')):
-                continue
-
-            if not self.filled.data.has_key(f.id):
-                value = None
-            else:
-                if f.store_display_value and ('%s_display' % f.id) in self.filled.data:
-                    value = self.filled.data['%s_display' % f.id]
-                else:
-                    value = self.filled.data[f.id]
-
-                if value is None or value == '':
-                    value = None
-
-            if value is None and not (f.required and include_unset_required_fields):
-                continue
-
             css_classes = ['field', 'field-type-%s' % f.key]
             if f.extra_css_class:
                 css_classes.append(f.extra_css_class)
             r += htmltext('<div class="%s">' % ' '.join(css_classes))
             r += htmltext('<span class="label">%s</span> ') % f.label
+            value = get_value(f)
             if value is None:
                 r += htmltext('<div class="value"><i>%s</i></div>') % _('Not set')
             else:
-- 
2.17.1