From 56b3f196057732ec441fe0f5982538431f1b069f Mon Sep 17 00:00:00 2001 From: Max Peterson Date: Fri, 3 Jul 2015 12:16:51 +0100 Subject: [PATCH 1/7] Add support for grouped choices. This also adds support for mixing single and paired choices: ``` [ ('poor', 'Poor quality'), 'medium', ('good', 'Good quality'), ] ``` --- rest_framework/fields.py | 38 +++++++++++++++++-------- tests/test_fields.py | 60 ++++++++++++++++++++++++++++++++++++++++ tests/test_validation.py | 46 ++++++++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 11 deletions(-) diff --git a/rest_framework/fields.py b/rest_framework/fields.py index 3ca7d682ed..812a35ac2c 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -5,6 +5,7 @@ import datetime import decimal import inspect +import itertools import re import uuid @@ -1098,17 +1099,8 @@ class ChoiceField(Field): } def __init__(self, choices, **kwargs): - # Allow either single or paired choices style: - # choices = [1, 2, 3] - # 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]) + flat_choices = [self.flatten_choice(c) for c in choices] + self.choices = OrderedDict(itertools.chain(*flat_choices)) # Map the string representation of choices to the underlying value. # Allows us to deal with eg. integer choices while supporting either @@ -1121,6 +1113,30 @@ def __init__(self, choices, **kwargs): super(ChoiceField, self).__init__(**kwargs) + def flatten_choice(self, choice): + """ + Convert a choices choice into a flat list of choices. + + Returns a list of choices. + """ + + # 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')] + if (not isinstance(choice, (list, tuple))): + # single choice + return [(choice, choice)] + else: + key, display_value = choice + if isinstance(display_value, (list, tuple)): + # grouped choices + sub_choices = [self.flatten_choice(c) for c in display_value] + return list(itertools.chain(*sub_choices)) + else: + # paired choice + return [(key, display_value)] + def to_internal_value(self, data): if data == '' and self.allow_blank: return '' diff --git a/tests/test_fields.py b/tests/test_fields.py index 76e6d9d60e..3bd4afc6e5 100644 --- a/tests/test_fields.py +++ b/tests/test_fields.py @@ -1091,6 +1091,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`. diff --git a/tests/test_validation.py b/tests/test_validation.py index 46e36f5d87..855ff20e01 100644 --- a/tests/test_validation.py +++ b/tests/test_validation.py @@ -141,6 +141,8 @@ def test_max_value_validation_fail(self): 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 @@ def test_choices(self): 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( From ee2afb83e2e531922a5fc10c236f8b17b30fae82 Mon Sep 17 00:00:00 2001 From: Max Peterson Date: Fri, 3 Jul 2015 13:56:49 +0100 Subject: [PATCH 2/7] Fix 1.4 tests and make flatten_choice a utility. --- rest_framework/fields.py | 56 +++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/rest_framework/fields.py b/rest_framework/fields.py index 812a35ac2c..9ef30274d2 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -108,6 +108,34 @@ def set_value(dictionary, keys, value): dictionary[keys[-1]] = value +def flatten_choice(choice): + """ + Convert a single choices choice into a flat list of choices. + + Returns a list of choices pairs. + + flatten_choice(1) -> [(1, 1)] + flatten_choice((1, '1st')) -> [(1, '1st')] + flatten_choice(('Grp', ((1, '1st'), (2, '2nd')))) -> [(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')] + if (not isinstance(choice, (list, tuple))): + # single choice + return [(choice, choice)] + else: + key, display_value = choice + if isinstance(display_value, (list, tuple)): + # grouped choices + sub_choices = [flatten_choice(c) for c in display_value] + return list(itertools.chain(*sub_choices)) + else: + # paired choice + return [(key, display_value)] + + class CreateOnlyDefault(object): """ This class may be used to provide default values that are only used @@ -1099,8 +1127,8 @@ class ChoiceField(Field): } def __init__(self, choices, **kwargs): - flat_choices = [self.flatten_choice(c) for c in choices] - self.choices = OrderedDict(itertools.chain(*flat_choices)) + flat_choices = [flatten_choice(c) for c in choices] + self.choices = OrderedDict(list(itertools.chain(*flat_choices))) # Map the string representation of choices to the underlying value. # Allows us to deal with eg. integer choices while supporting either @@ -1113,30 +1141,6 @@ def __init__(self, choices, **kwargs): super(ChoiceField, self).__init__(**kwargs) - def flatten_choice(self, choice): - """ - Convert a choices choice into a flat list of choices. - - Returns a list of choices. - """ - - # 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')] - if (not isinstance(choice, (list, tuple))): - # single choice - return [(choice, choice)] - else: - key, display_value = choice - if isinstance(display_value, (list, tuple)): - # grouped choices - sub_choices = [self.flatten_choice(c) for c in display_value] - return list(itertools.chain(*sub_choices)) - else: - # paired choice - return [(key, display_value)] - def to_internal_value(self, data): if data == '' and self.allow_blank: return '' From 27ac5a368091823f071eed198f6125eee2de8dfa Mon Sep 17 00:00:00 2001 From: Tom Christie Date: Thu, 6 Aug 2015 11:43:03 +0100 Subject: [PATCH 3/7] Support grouped choices --- rest_framework/fields.py | 87 ++++++++++++++----- .../rest_framework/horizontal/select.html | 10 ++- .../horizontal/select_multiple.html | 10 ++- .../rest_framework/inline/select.html | 11 ++- .../inline/select_multiple.html | 12 ++- .../rest_framework/vertical/select.html | 11 ++- .../vertical/select_multiple.html | 10 ++- rest_framework/utils/field_mapping.py | 4 +- tests/test_model_serializer.py | 2 +- 9 files changed, 118 insertions(+), 39 deletions(-) diff --git a/rest_framework/fields.py b/rest_framework/fields.py index 9e44a3dc3e..4aa33452fc 100644 --- a/rest_framework/fields.py +++ b/rest_framework/fields.py @@ -5,7 +5,6 @@ import datetime import decimal import inspect -import itertools import re import uuid @@ -109,32 +108,54 @@ def set_value(dictionary, keys, value): dictionary[keys[-1]] = value -def flatten_choice(choice): +def pairwise_choices(choices): """ - Convert a single choices choice into a flat list of choices. + Convert any single choices into key/value pairs instead. - Returns a list of choices pairs. - - flatten_choice(1) -> [(1, 1)] - flatten_choice((1, '1st')) -> [(1, '1st')] - flatten_choice(('Grp', ((1, '1st'), (2, '2nd')))) -> [(1, '1st'), (2, '2nd')] + pairwise_choices([1]) -> [(1, 1)] + pairwise_choices([(1, '1st')]) -> [(1, '1st')] + pairwise_choices([('Group', ((1, '1st'), 2))]) -> [(1, '1st'), (2, 2)] """ # 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')] - if (not isinstance(choice, (list, tuple))): - # single choice - return [(choice, choice)] - else: - key, display_value = choice - if isinstance(display_value, (list, tuple)): - # grouped choices - sub_choices = [flatten_choice(c) for c in display_value] - return list(itertools.chain(*sub_choices)) + ret = [] + for choice in choices: + if (not isinstance(choice, (list, tuple))): + # single choice + item = (choice, choice) + ret.append(item) + else: + key, value = choice + if isinstance(value, (list, tuple)): + # grouped choices (category, sub choices) + item = (key, pairwise_choices(value)) + ret.append(item) + else: + # paired choice (key, display value) + item = (key, value) + ret.append(item) + return ret + + +def flatten_choices(choices): + """ + Convert a group choices into a flat list of choices. + + flatten_choices([(1, '1st')]) -> [(1, '1st')] + flatten_choices([('Grp', ((1, '1st'), (2, '2nd')))]) -> [(1, '1st'), (2, '2nd')] + """ + ret = [] + for key, value in choices: + if isinstance(value, (list, tuple)): + # grouped choices (category, sub choices) + ret.extend(flatten_choices(value)) else: - # paired choice - return [(key, display_value)] + # choice (key, display value) + item = (key, value) + ret.append(item) + return ret class CreateOnlyDefault(object): @@ -1140,8 +1161,8 @@ class ChoiceField(Field): } def __init__(self, choices, **kwargs): - flat_choices = [flatten_choice(c) for c in choices] - self.choices = OrderedDict(list(itertools.chain(*flat_choices))) + self.grouped_choices = pairwise_choices(choices) + self.choices = OrderedDict(flatten_choices(self.grouped_choices)) # Map the string representation of choices to the underlying value. # Allows us to deal with eg. integer choices while supporting either @@ -1168,6 +1189,30 @@ def to_representation(self, value): return value return self.choice_strings_to_values.get(six.text_type(value), value) + def iter_options(self): + class StartOptionGroup(object): + start_option_group = True + + def __init__(self, label): + self.label = label + + class EndOptionGroup(object): + end_option_group = True + + class Option(object): + def __init__(self, value, display_text): + self.value = value + self.display_text = display_text + + for key, value in self.grouped_choices: + if isinstance(value, (list, tuple)): + yield StartOptionGroup(label=key) + for sub_key, sub_value in value: + 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 = { diff --git a/rest_framework/templates/rest_framework/horizontal/select.html b/rest_framework/templates/rest_framework/horizontal/select.html index 8957eef9f6..4aa3db3de5 100644 --- a/rest_framework/templates/rest_framework/horizontal/select.html +++ b/rest_framework/templates/rest_framework/horizontal/select.html @@ -10,8 +10,14 @@ {% if field.allow_null or field.allow_blank %} {% endif %} - {% for key, text in field.choices.items %} - + {% for select in field.iter_options %} + {% if select.start_option_group %} + + {% elif select.end_option_group %} + + {% else %} + + {% endif %} {% endfor %} diff --git a/rest_framework/templates/rest_framework/horizontal/select_multiple.html b/rest_framework/templates/rest_framework/horizontal/select_multiple.html index 407b356ff5..b1ca2f1435 100644 --- a/rest_framework/templates/rest_framework/horizontal/select_multiple.html +++ b/rest_framework/templates/rest_framework/horizontal/select_multiple.html @@ -10,8 +10,14 @@
diff --git a/rest_framework/templates/rest_framework/inline/select_multiple.html b/rest_framework/templates/rest_framework/inline/select_multiple.html index b9b8bb4ea6..3bae43eb84 100644 --- a/rest_framework/templates/rest_framework/inline/select_multiple.html +++ b/rest_framework/templates/rest_framework/inline/select_multiple.html @@ -9,9 +9,15 @@ {% endif %} diff --git a/rest_framework/templates/rest_framework/vertical/select.html b/rest_framework/templates/rest_framework/vertical/select.html index dc32d39dd5..ce30022d87 100644 --- a/rest_framework/templates/rest_framework/vertical/select.html +++ b/rest_framework/templates/rest_framework/vertical/select.html @@ -9,9 +9,14 @@ {% if field.allow_null or field.allow_blank %} {% endif %} - - {% for key, text in field.choices.items %} - + {% for select in field.iter_options %} + {% if select.start_option_group %} + + {% elif select.end_option_group %} + + {% else %} + + {% endif %} {% endfor %} diff --git a/rest_framework/templates/rest_framework/vertical/select_multiple.html b/rest_framework/templates/rest_framework/vertical/select_multiple.html index 2bb3d5ae48..cb65cec526 100644 --- a/rest_framework/templates/rest_framework/vertical/select_multiple.html +++ b/rest_framework/templates/rest_framework/vertical/select_multiple.html @@ -9,8 +9,14 @@ {% endif %}