From 034633a020d793194dc3e6f8bc933d7865b7b822 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20P=C3=A9ters?= <fpeters@entrouvert.com>
Date: Tue, 27 Oct 2015 14:07:50 +0100
Subject: [PATCH] fields: review item field to get back to previous behaviour
 (#8785)

In recent commits we tried to handle optional item field with an empty value
prepended to the list of options but while this allows the user to leave
optional fields uncompleted this is not the same behaviour we add.

Return to the previous behaviour.  The only way for an item field to be skipped
is now to be displayed as a <select> field with the hint put as the first
element.
---
 tests/test_datasource.py |   2 +-
 tests/test_fields.py     | 151 ++++++++++++++++++++++++++++++---------
 tests/test_form_pages.py |   2 +-
 wcs/fields.py            |  17 ++---
 wcs/qommon/form.py       |   2 +-
 5 files changed, 127 insertions(+), 47 deletions(-)

diff --git a/tests/test_datasource.py b/tests/test_datasource.py
index eaa406826..4a39ff65c 100644
--- a/tests/test_datasource.py
+++ b/tests/test_datasource.py
@@ -211,4 +211,4 @@ def test_optional_item_field_with_data_source():
     field.add_to_form(form)
     widget = form.get_widget('f1')
     assert widget is not None
-    assert widget.options == [(None, '', 'None'), ('1', 'un', '1'), ('2', 'deux', '2')]
+    assert widget.options == [('1', 'un', '1'), ('2', 'deux', '2')]
diff --git a/tests/test_fields.py b/tests/test_fields.py
index e8e5984d4..1f4c18a34 100644
--- a/tests/test_fields.py
+++ b/tests/test_fields.py
@@ -148,40 +148,127 @@ def test_map():
     assert fields.MapField().get_json_value('foobar') == None
 
 def test_item_render():
-    field = fields.ItemField(id='1', label='Foobar', items=['a', 'b', 'c'])
-    form = Form()
-    field.add_to_form(form)
-    assert str(form.render()).count('<option') == 3
+    items_kwargs = []
+    items_kwargs.append({'items': ['a', 'b', 'c']})
+    items_kwargs.append({'data_source': {
+        'type': 'formula',
+        'value': '''['a', 'b', 'c']'''}})
 
-    field = fields.ItemField(id='1', label='Foobar', items=['a', 'b', 'c'],
-            required=False)
-    form = Form()
-    field.add_to_form(form)
-    assert str(form.render()).count('<option selected="selected" value="None"></option>') == 1 # None
-    assert str(form.render()).count('<option') == 4 # 3 + None as first item
+    for item_kwargs in items_kwargs:
+        field = fields.ItemField(id='1', label='Foobar', **item_kwargs)
+        form = Form()
+        field.add_to_form(form)
+        assert str(form.render()).count('<option') == 3
 
-    field = fields.ItemField(id='1', label='Foobar', items=['a', 'b', 'c'],
-            required=False, hint='Bla bla bla')
-    form = Form()
-    field.add_to_form(form)
-    assert str(form.render()).count('<option value="">Bla bla bla</option>') == 1 # ---
-    assert str(form.render()).count('<option') == 4
+        field = fields.ItemField(id='1', label='Foobar', required=False,
+                **item_kwargs)
+        form = Form()
+        field.add_to_form(form)
+        assert str(form.render()).count('<option') == 3
 
-    field = fields.ItemField(id='1', label='Foobar', items=['a', 'b', 'c'],
-            required=True, hint='Bla bla bla')
-    form = Form()
-    field.add_to_form(form)
-    assert str(form.render()).count('<option value="">Bla bla bla</option>') == 1 # ---
-    assert str(form.render()).count('<option') == 4
+        field = fields.ItemField(id='1', label='Foobar',
+                required=False, hint='Bla bla bla', **item_kwargs)
+        form = Form()
+        field.add_to_form(form)
+        assert str(form.render()).count('<option value="">Bla bla bla</option>') == 1 # ---
+        assert str(form.render()).count('<option') == 4
 
-    # without any item
-    field = fields.ItemField(id='1', label='Foobar', items=[])
-    form = Form()
-    field.add_to_form(form)
-    assert str(form.render()).count('<option') == 1
+        field = fields.ItemField(id='1', label='Foobar',
+                required=True, hint='Bla bla bla', **item_kwargs)
+        form = Form()
+        field.add_to_form(form)
+        assert str(form.render()).count('<option value="">Bla bla bla</option>') == 1 # ---
+        assert str(form.render()).count('<option') == 4
 
-    # without any item and not being required
-    field = fields.ItemField(id='1', label='Foobar', items=[], required=False)
-    form = Form()
-    field.add_to_form(form)
-    assert str(form.render()).count('<option') == 1
+    items_kwargs = []
+    items_kwargs.append({'items': None})
+    items_kwargs.append({'items': []})
+    items_kwargs.append({'data_source': {
+        'type': 'formula',
+        'value': '''[]'''}})
+    for item_kwargs in items_kwargs:
+        field = fields.ItemField(id='1', label='Foobar', **item_kwargs)
+        form = Form()
+        field.add_to_form(form)
+        assert str(form.render()).count('<option') == 1
+
+        field = fields.ItemField(id='1', label='Foobar', required=False,
+                **item_kwargs)
+        form = Form()
+        field.add_to_form(form)
+        assert str(form.render()).count('<option') == 1
+
+        field = fields.ItemField(id='1', label='Foobar',
+                required=False, hint='Bla bla bla', **item_kwargs)
+        form = Form()
+        field.add_to_form(form)
+        assert str(form.render()).count('<option value="">Bla bla bla</option>') == 1 # ---
+        assert str(form.render()).count('<option') == 1
+
+        field = fields.ItemField(id='1', label='Foobar',
+                required=True, hint='Bla bla bla', **item_kwargs)
+        form = Form()
+        field.add_to_form(form)
+        assert str(form.render()).count('<option value="">Bla bla bla</option>') == 1 # ---
+        assert str(form.render()).count('<option') == 1
+
+def test_item_render_as_radio():
+    items_kwargs = []
+    items_kwargs.append({'items': ['a', 'b', 'c']})
+    items_kwargs.append({'data_source': {
+        'type': 'formula',
+        'value': '''['a', 'b', 'c']'''}})
+
+    for item_kwargs in items_kwargs:
+        field = fields.ItemField(id='1', label='Foobar', show_as_radio=True, **item_kwargs)
+        form = Form()
+        field.add_to_form(form)
+        assert str(form.render()).count('"radio"') == 3
+
+        field = fields.ItemField(id='1', label='Foobar', required=False,
+                show_as_radio=True, **item_kwargs)
+        form = Form()
+        field.add_to_form(form)
+        assert str(form.render()).count('"radio"') == 3
+
+        field = fields.ItemField(id='1', label='Foobar',
+                show_as_radio=True, required=False, hint='Bla bla bla', **item_kwargs)
+        form = Form()
+        field.add_to_form(form)
+        assert str(form.render()).count('"radio"') == 3
+
+        field = fields.ItemField(id='1', label='Foobar',
+                show_as_radio=True, required=True, hint='Bla bla bla', **item_kwargs)
+        form = Form()
+        field.add_to_form(form)
+        assert str(form.render()).count('"radio"') == 3
+
+    items_kwargs = []
+    items_kwargs.append({'items': None})
+    items_kwargs.append({'items': []})
+    items_kwargs.append({'data_source': {
+        'type': 'formula',
+        'value': '''[]'''}})
+    for item_kwargs in items_kwargs:
+        field = fields.ItemField(id='1', label='Foobar', show_as_radio=True, **item_kwargs)
+        form = Form()
+        field.add_to_form(form)
+        assert str(form.render()).count('"radio"') == 1
+
+        field = fields.ItemField(id='1', label='Foobar', required=False,
+                show_as_radio=True, **item_kwargs)
+        form = Form()
+        field.add_to_form(form)
+        assert str(form.render()).count('"radio"') == 1
+
+        field = fields.ItemField(id='1', label='Foobar',
+                show_as_radio=True, required=False, hint='Bla bla bla', **item_kwargs)
+        form = Form()
+        field.add_to_form(form)
+        assert str(form.render()).count('"radio"') == 1
+
+        field = fields.ItemField(id='1', label='Foobar',
+                show_as_radio=True, required=True, hint='Bla bla bla', **item_kwargs)
+        form = Form()
+        field.add_to_form(form)
+        assert str(form.render()).count('"radio"') == 1
diff --git a/tests/test_form_pages.py b/tests/test_form_pages.py
index f5d294eb4..af6228fdc 100644
--- a/tests/test_form_pages.py
+++ b/tests/test_form_pages.py
@@ -307,7 +307,7 @@ def test_form_multi_page_condition_select(pub):
     resp = get_app(pub).get('/test/')
     assert not '2nd page' in resp.body
     assert not '3rd page' in resp.body
-    resp.forms[0]['f1'] = 'Foo'
+    assert resp.forms[0]['f1'].value == 'Foo' # preset
     resp = resp.forms[0].submit('submit')
     assert '2nd page' in resp.body
     assert not '3rd page' in resp.body
diff --git a/wcs/fields.py b/wcs/fields.py
index b90aebe76..f4b827108 100644
--- a/wcs/fields.py
+++ b/wcs/fields.py
@@ -857,18 +857,11 @@ class ItemField(WidgetField):
         WidgetField.__init__(self, **kwargs)
 
     def get_options(self):
-        if not self.data_source:
-            options = self.items[:]
-        else:
-            options = data_sources.get_items(self.data_source)
-        if options and not self.required:
-            if type(options[0]) is str:
-                options[:0] = [None]
-            elif len(options[0]) == 2:
-                options[:0] = [(None, '')]
-            elif len(options[0]) == 3:
-                options[:0] = [(None, '', None)]
-        return options
+        if self.data_source:
+            return data_sources.get_items(self.data_source)
+        if self.items:
+            return self.items[:]
+        return []
 
     def perform_more_widget_changes(self, form, kwargs, edit = True):
         real_data_source = data_sources.get_real(self.data_source)
diff --git a/wcs/qommon/form.py b/wcs/qommon/form.py
index fa8446819..f4bb350a4 100644
--- a/wcs/qommon/form.py
+++ b/wcs/qommon/form.py
@@ -1366,7 +1366,7 @@ class SingleSelectHintWidget(SingleSelectWidget):
         if not self.separate_hint() and self.hint:
             r = htmltag('option', value='', selected=None)
             tags.append(r + htmlescape(self.hint) + htmltext('</option>'))
-            if not self.required:
+            if self.options[0][0] is None:
                 # hint has been put as first element, skip the default empty
                 # value.
                 options = self.options[1:]