Merge pull request #3225 from tomchristie/maxpeterson-grouped-choices-fix

Support grouped choices.

rest_framework/fields.py

@@ -108,6 +108,53 @@ def set_value(dictionary, keys, value):
     dictionary[keys[-1]] = value
 
 
+def to_choices_dict(choices):
+    """
+    Convert choices into key/value dicts.
+
+    pairwise_choices([1]) -> {1: 1}
+    pairwise_choices([(1, '1st'), (2, '2nd')]) -> {1: '1st', 2: '2nd'}
+    pairwise_choices([('Group', ((1, '1st'), 2))]) -> {'Group': {1: '1st', 2: '2nd'}}
+    """
+    # Allow single, paired or grouped choices style:
+    # choices = [1, 2, 3]
+    # choices = [(1, 'First'), (2, 'Second'), (3, 'Third')]
+    # choices = [('Category', ((1, 'First'), (2, 'Second'))), (3, 'Third')]
+    ret = OrderedDict()
+    for choice in choices:
+        if (not isinstance(choice, (list, tuple))):
+            # single choice
+            ret[choice] = choice
+        else:
+            key, value = choice
+            if isinstance(value, (list, tuple)):
+                # grouped choices (category, sub choices)
+                ret[key] = to_choices_dict(value)
+            else:
+                # paired choice (key, display value)
+                ret[key] = value
+    return ret
+
+
+def flatten_choices_dict(choices):
+    """
+    Convert a group choices dict into a flat dict of choices.
+
+    flatten_choices({1: '1st', 2: '2nd'}) -> {1: '1st', 2: '2nd'}
+    flatten_choices({'Group': {1: '1st', 2: '2nd'}}) -> {1: '1st', 2: '2nd'}
+    """
+    ret = OrderedDict()
+    for key, value in choices.items():
+        if isinstance(value, dict):
+            # grouped choices (category, sub choices)
+            for sub_key, sub_value in value.items():
+                ret[sub_key] = sub_value
+        else:
+            # choice (key, display value)
+            ret[key] = value
+    return ret
+
+
 class CreateOnlyDefault(object):
     """
     This class may be used to provide default values that are only used
@@ -1111,17 +1158,8 @@ class ChoiceField(Field):
     }
 
     def __init__(self, choices, **kwargs):
-        # Allow either single or paired choices style:
+        self.grouped_choices = to_choices_dict(choices)
-        # choices = [1, 2, 3]
+        self.choices = flatten_choices_dict(self.grouped_choices)
-        # choices = [(1, 'First'), (2, 'Second'), (3, 'Third')]
-        pairs = [
-            isinstance(item, (list, tuple)) and len(item) == 2
-            for item in choices
-        ]
-        if all(pairs):
-            self.choices = OrderedDict([(key, display_value) for key, display_value in choices])
-        else:
-            self.choices = OrderedDict([(item, item) for item in choices])
 
         # Map the string representation of choices to the underlying value.
         # Allows us to deal with eg. integer choices while supporting either
@@ -1148,6 +1186,38 @@ class ChoiceField(Field):
             return value
         return self.choice_strings_to_values.get(six.text_type(value), value)
 
+    def iter_options(self):
+        """
+        Helper method for use with templates rendering select widgets.
+        """
+        class StartOptionGroup(object):
+            start_option_group = True
+            end_option_group = False
+
+            def __init__(self, label):
+                self.label = label
+
+        class EndOptionGroup(object):
+            start_option_group = False
+            end_option_group = True
+
+        class Option(object):
+            start_option_group = False
+            end_option_group = False
+
+            def __init__(self, value, display_text):
+                self.value = value
+                self.display_text = display_text
+
+        for key, value in self.grouped_choices.items():
+            if isinstance(value, dict):
+                yield StartOptionGroup(label=key)
+                for sub_key, sub_value in value.items():
+                    yield Option(value=sub_key, display_text=sub_value)
+                yield EndOptionGroup()
+            else:
+                yield Option(value=key, display_text=value)
+
 
 class MultipleChoiceField(ChoiceField):
     default_error_messages = {

rest_framework/templates/rest_framework/horizontal/select.html

@@ -10,8 +10,14 @@
       {% if field.allow_null or field.allow_blank %}
         <option value="" {% if not field.value %}selected{% endif %}>--------</option>
       {% endif %}
-      {% for key, text in field.choices.items %}
+      {% for select in field.iter_options %}
-        <option value="{{ key }}" {% if key == field.value %}selected{% endif %}>{{ text }}</option>
+          {% if select.start_option_group %}
+            <optgroup label="{{ select.label }}">
+          {% elif select.end_option_group %}
+            </optgroup>
+          {% else %}
+            <option value="{{ select.value }}" {% if select.value == field.value %}selected{% endif %}>{{ select.display_text }}</option>
+          {% endif %}
       {% endfor %}
     </select>
 

rest_framework/templates/rest_framework/horizontal/select_multiple.html

@@ -10,8 +10,14 @@
 
   <div class="col-sm-10">
     <select multiple {{ field.choices|yesno:",disabled" }} class="form-control" name="{{ field.name }}">
-      {% for key, text in field.choices.items %}
+      {% for select in field.iter_options %}
-        <option value="{{ key }}" {% if key in field.value %}selected{% endif %}>{{ text }}</option>
+        {% if select.start_option_group %}
+          <optgroup label="{{ select.label }}">
+        {% elif select.end_option_group %}
+          </optgroup>
+        {% else %}
+          <option value="{{ select.value }}" {% if select.value == field.value %}selected{% endif %}>{{ select.display_text }}</option>
+        {% endif %}
       {% empty %}
           <option>{{ no_items }}</option>
       {% endfor %}

rest_framework/templates/rest_framework/inline/select.html

@@ -9,9 +9,14 @@
     {% if field.allow_null or field.allow_blank %}
       <option value="" {% if not field.value %}selected{% endif %}>--------</option>
     {% endif %}
-
+    {% for select in field.iter_options %}
-    {% for key, text in field.choices.items %}
+        {% if select.start_option_group %}
-      <option value="{{ key }}" {% if key == field.value %}selected{% endif %}>{{ text }}</option>
+          <optgroup label="{{ select.label }}">
+        {% elif select.end_option_group %}
+          </optgroup>
+        {% else %}
+          <option value="{{ select.value }}" {% if select.value == field.value %}selected{% endif %}>{{ select.display_text }}</option>
+        {% endif %}
     {% endfor %}
   </select>
 </div>

rest_framework/templates/rest_framework/inline/select_multiple.html

@@ -9,8 +9,14 @@
   {% endif %}
 
   <select multiple {{ field.choices|yesno:",disabled" }} class="form-control" name="{{ field.name }}">
-    {% for key, text in field.choices.items %}
+      {% for select in field.iter_options %}
-      <option value="{{ key }}" {% if key in field.value %}selected{% endif %}>{{ text }}</option>
+          {% if select.start_option_group %}
+            <optgroup label="{{ select.label }}">
+          {% elif select.end_option_group %}
+            </optgroup>
+          {% else %}
+            <option value="{{ select.value }}" {% if select.value == field.value %}selected{% endif %}>{{ select.display_text }}</option>
+          {% endif %}
       {% empty %}
       <option>{{ no_items }}</option>
     {% endfor %}

rest_framework/templates/rest_framework/vertical/select.html

@@ -9,9 +9,14 @@
     {% if field.allow_null or field.allow_blank %}
       <option value="" {% if not field.value %}selected{% endif %}>--------</option>
     {% endif %}
-
+    {% for select in field.iter_options %}
-    {% for key, text in field.choices.items %}
+        {% if select.start_option_group %}
-      <option value="{{ key }}" {% if key == field.value %}selected{% endif %}>{{ text }}</option>
+          <optgroup label="{{ select.label }}">
+        {% elif select.end_option_group %}
+          </optgroup>
+        {% else %}
+          <option value="{{ select.value }}" {% if select.value == field.value %}selected{% endif %}>{{ select.display_text }}</option>
+        {% endif %}
     {% endfor %}
   </select>
 

rest_framework/templates/rest_framework/vertical/select_multiple.html

@@ -9,8 +9,14 @@
   {% endif %}
 
   <select multiple {{ field.choices|yesno:",disabled" }} class="form-control" name="{{ field.name }}">
-    {% for key, text in field.choices.items %}
+    {% for select in field.iter_options %}
-      <option value="{{ key }}" {% if key in field.value %}selected{% endif %}>{{ text }}</option>
+        {% if select.start_option_group %}
+          <optgroup label="{{ select.label }}">
+        {% elif select.end_option_group %}
+          </optgroup>
+        {% else %}
+          <option value="{{ select.value }}" {% if select.value == field.value %}selected{% endif %}>{{ select.display_text }}</option>
+        {% endif %}
     {% empty %}
         <option>{{ no_items }}</option>
     {% endfor %}

rest_framework/utils/field_mapping.py

@@ -107,10 +107,10 @@ def get_field_kwargs(field_name, model_field):
                               isinstance(model_field, models.TextField)):
         kwargs['allow_blank'] = True
 
-    if model_field.flatchoices:
+    if model_field.choices:
         # If this model field contains choices, then return early.
         # Further keyword arguments are not valid.
-        kwargs['choices'] = model_field.flatchoices
+        kwargs['choices'] = model_field.choices
         return kwargs
 
     # Ensure that max_length is passed explicitly as a keyword arg,

tests/test_fields.py

@@ -1107,6 +1107,34 @@ class TestChoiceField(FieldValues):
         output = field.run_validation(None)
         assert output is None
 
+    def test_iter_options(self):
+        """
+        iter_options() should return a list of options and option groups.
+        """
+        field = serializers.ChoiceField(
+            choices=[
+                ('Numbers', ['integer', 'float']),
+                ('Strings', ['text', 'email', 'url']),
+                'boolean'
+            ]
+        )
+        items = list(field.iter_options())
+
+        assert items[0].start_option_group
+        assert items[0].label == 'Numbers'
+        assert items[1].value == 'integer'
+        assert items[2].value == 'float'
+        assert items[3].end_option_group
+
+        assert items[4].start_option_group
+        assert items[4].label == 'Strings'
+        assert items[5].value == 'text'
+        assert items[6].value == 'email'
+        assert items[7].value == 'url'
+        assert items[8].end_option_group
+
+        assert items[9].value == 'boolean'
+
 
 class TestChoiceFieldWithType(FieldValues):
     """
@@ -1153,6 +1181,66 @@ class TestChoiceFieldWithListChoices(FieldValues):
     field = serializers.ChoiceField(choices=('poor', 'medium', 'good'))
 
 
+class TestChoiceFieldWithGroupedChoices(FieldValues):
+    """
+    Valid and invalid values for a `Choice` field that uses a grouped list for the
+    choices, rather than a list of pairs of (`value`, `description`).
+    """
+    valid_inputs = {
+        'poor': 'poor',
+        'medium': 'medium',
+        'good': 'good',
+    }
+    invalid_inputs = {
+        'awful': ['"awful" is not a valid choice.']
+    }
+    outputs = {
+        'good': 'good'
+    }
+    field = serializers.ChoiceField(
+        choices=[
+            (
+                'Category',
+                (
+                    ('poor', 'Poor quality'),
+                    ('medium', 'Medium quality'),
+                ),
+            ),
+            ('good', 'Good quality'),
+        ]
+    )
+
+
+class TestChoiceFieldWithMixedChoices(FieldValues):
+    """
+    Valid and invalid values for a `Choice` field that uses a single paired or
+    grouped.
+    """
+    valid_inputs = {
+        'poor': 'poor',
+        'medium': 'medium',
+        'good': 'good',
+    }
+    invalid_inputs = {
+        'awful': ['"awful" is not a valid choice.']
+    }
+    outputs = {
+        'good': 'good'
+    }
+    field = serializers.ChoiceField(
+        choices=[
+            (
+                'Category',
+                (
+                    ('poor', 'Poor quality'),
+                ),
+            ),
+            'medium',
+            ('good', 'Good quality'),
+        ]
+    )
+
+
 class TestMultipleChoiceField(FieldValues):
     """
     Valid and invalid values for `MultipleChoiceField`.

tests/test_model_serializer.py

@@ -181,7 +181,7 @@ class TestRegularFieldMappings(TestCase):
                 null_field = IntegerField(allow_null=True, required=False)
                 default_field = IntegerField(required=False)
                 descriptive_field = IntegerField(help_text='Some help text', label='A label')
-                choices_field = ChoiceField(choices=[('red', 'Red'), ('blue', 'Blue'), ('green', 'Green')])
+                choices_field = ChoiceField(choices=(('red', 'Red'), ('blue', 'Blue'), ('green', 'Green')))
         """)
         if six.PY2:
             # This particular case is too awkward to resolve fully across

tests/test_validation.py

@@ -141,6 +141,8 @@ class TestMaxValueValidatorValidation(TestCase):
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
 
 
+# regression tests for issue: 1533
+
 class TestChoiceFieldChoicesValidate(TestCase):
     CHOICES = [
         (0, 'Small'),
@@ -148,6 +150,8 @@ class TestChoiceFieldChoicesValidate(TestCase):
         (2, 'Large'),
     ]
 
+    SINGLE_CHOICES = [0, 1, 2]
+
     CHOICES_NESTED = [
         ('Category', (
             (1, 'First'),
@@ -157,6 +161,15 @@ class TestChoiceFieldChoicesValidate(TestCase):
         (4, 'Fourth'),
     ]
 
+    MIXED_CHOICES = [
+        ('Category', (
+            (1, 'First'),
+            (2, 'Second'),
+        )),
+        3,
+        (4, 'Fourth'),
+    ]
+
     def test_choices(self):
         """
         Make sure a value for choices works as expected.
@@ -168,6 +181,39 @@ class TestChoiceFieldChoicesValidate(TestCase):
         except serializers.ValidationError:
             self.fail("Value %s does not validate" % str(value))
 
+    def test_single_choices(self):
+        """
+        Make sure a single value for choices works as expected.
+        """
+        f = serializers.ChoiceField(choices=self.SINGLE_CHOICES)
+        value = self.SINGLE_CHOICES[0]
+        try:
+            f.to_internal_value(value)
+        except serializers.ValidationError:
+            self.fail("Value %s does not validate" % str(value))
+
+    def test_nested_choices(self):
+        """
+        Make sure a nested value for choices works as expected.
+        """
+        f = serializers.ChoiceField(choices=self.CHOICES_NESTED)
+        value = self.CHOICES_NESTED[0][1][0][0]
+        try:
+            f.to_internal_value(value)
+        except serializers.ValidationError:
+            self.fail("Value %s does not validate" % str(value))
+
+    def test_mixed_choices(self):
+        """
+        Make sure mixed values for choices works as expected.
+        """
+        f = serializers.ChoiceField(choices=self.MIXED_CHOICES)
+        value = self.MIXED_CHOICES[1]
+        try:
+            f.to_internal_value(value)
+        except serializers.ValidationError:
+            self.fail("Value %s does not validate" % str(value))
+
 
 class RegexSerializer(serializers.Serializer):
     pin = serializers.CharField(