Skip to content

Commit a450422

Browse files
fbozhangauvipybrowniebroke
authored
Preserve ordering in MultipleChoiceField (#9735)
* fix: MultipleChoiceField use ordered sort (cherry picked from commit 8436483) * test: fix unit tests (cherry picked from commit 6428ac4) * test: test TestMultipleChoiceField can json serializable (cherry picked from commit 12908b1) * test: fix unit test (cherry picked from commit 73a709c) * minor: rest old formatting * fix: using pytest.fail to test * Update test_fields.py * Update test_fields.py * Update test_fields.py * test: add test cases * docs: update docs * Update docs/api-guide/fields.md * Skip inner list allocation * Fix punctuation --------- Co-authored-by: Asif Saif Uddin {"Auvi":"অভি"} <auvipy@gmail.com> Co-authored-by: Bruno Alla <browniebroke@users.noreply.github.com> Co-authored-by: Bruno Alla <alla.brunoo@gmail.com>
1 parent a3f939f commit a450422

File tree

4 files changed

+47
-16
lines changed

4 files changed

+47
-16
lines changed

docs/api-guide/fields.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,7 @@ Both the `allow_blank` and `allow_null` are valid options on `ChoiceField`, alth
417417

418418
## MultipleChoiceField
419419

420-
A field that can accept a set of zero, one or many values, chosen from a limited set of choices. Takes a single mandatory argument. `to_internal_value` returns a `set` containing the selected values.
420+
A field that can accept a list of zero, one or many values, chosen from a limited set of choices. Takes a single mandatory argument. `to_internal_value` returns a `list` containing the selected values, deduplicated.
421421

422422
**Signature:** `MultipleChoiceField(choices)`
423423

rest_framework/fields.py

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1523,17 +1523,22 @@ def to_internal_value(self, data):
15231523
if not self.allow_empty and len(data) == 0:
15241524
self.fail('empty')
15251525

1526-
return {
1527-
# Arguments for super() are needed because of scoping inside
1528-
# comprehensions.
1529-
super(MultipleChoiceField, self).to_internal_value(item)
1530-
for item in data
1531-
}
1526+
# Arguments for super() are needed because of scoping inside
1527+
# comprehensions.
1528+
return list(
1529+
dict.fromkeys(
1530+
super(MultipleChoiceField, self).to_internal_value(item)
1531+
for item in data
1532+
)
1533+
)
15321534

15331535
def to_representation(self, value):
1534-
return {
1535-
self.choice_strings_to_values.get(str(item), item) for item in value
1536-
}
1536+
return list(
1537+
dict.fromkeys(
1538+
self.choice_strings_to_values.get(str(item), item)
1539+
for item in value
1540+
)
1541+
)
15371542

15381543

15391544
class FilePathField(ChoiceField):

tests/test_fields.py

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
BuiltinSignatureError, DjangoImageField, SkipField, empty,
2525
is_simple_callable
2626
)
27+
from rest_framework.utils import json
2728
from tests.models import UUIDForeignKeyTarget
2829

2930
utc = datetime.timezone.utc
@@ -2178,16 +2179,20 @@ class TestMultipleChoiceField(FieldValues):
21782179
Valid and invalid values for `MultipleChoiceField`.
21792180
"""
21802181
valid_inputs = {
2181-
(): set(),
2182-
('aircon',): {'aircon'},
2183-
('aircon', 'manual'): {'aircon', 'manual'},
2182+
(): list(),
2183+
('aircon',): ['aircon'],
2184+
('aircon', 'aircon'): ['aircon'],
2185+
('aircon', 'manual'): ['aircon', 'manual'],
2186+
('manual', 'aircon'): ['manual', 'aircon'],
21842187
}
21852188
invalid_inputs = {
21862189
'abc': ['Expected a list of items but got type "str".'],
21872190
('aircon', 'incorrect'): ['"incorrect" is not a valid choice.']
21882191
}
21892192
outputs = [
2190-
(['aircon', 'manual', 'incorrect'], {'aircon', 'manual', 'incorrect'})
2193+
(['aircon', 'manual', 'incorrect'], ['aircon', 'manual', 'incorrect']),
2194+
(['manual', 'aircon', 'incorrect'], ['manual', 'aircon', 'incorrect']),
2195+
(['aircon', 'manual', 'aircon'], ['aircon', 'manual']),
21912196
]
21922197
field = serializers.MultipleChoiceField(
21932198
choices=[
@@ -2204,6 +2209,27 @@ def test_against_partial_and_full_updates(self):
22042209
field.partial = True
22052210
assert field.get_value(QueryDict('')) == rest_framework.fields.empty
22062211

2212+
def test_valid_inputs_is_json_serializable(self):
2213+
for input_value, _ in get_items(self.valid_inputs):
2214+
validated = self.field.run_validation(input_value)
2215+
2216+
try:
2217+
json.dumps(validated)
2218+
except TypeError as e:
2219+
pytest.fail(f'Validated output not JSON serializable: {repr(validated)}; Error: {e}')
2220+
2221+
def test_output_is_json_serializable(self):
2222+
for output_value, _ in get_items(self.outputs):
2223+
representation = self.field.to_representation(output_value)
2224+
2225+
try:
2226+
json.dumps(representation)
2227+
except TypeError as e:
2228+
pytest.fail(
2229+
f'to_representation output not JSON serializable: '
2230+
f'{repr(representation)}; Error: {e}'
2231+
)
2232+
22072233

22082234
class TestEmptyMultipleChoiceField(FieldValues):
22092235
"""

tests/test_serializer_nested.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,14 +199,14 @@ def test_nested_serializer_with_list_json(self):
199199
serializer = self.Serializer(data=input_data)
200200

201201
assert serializer.is_valid()
202-
assert serializer.validated_data['nested']['example'] == {1, 2}
202+
assert serializer.validated_data['nested']['example'] == [1, 2]
203203

204204
def test_nested_serializer_with_list_multipart(self):
205205
input_data = QueryDict('nested.example=1&nested.example=2')
206206
serializer = self.Serializer(data=input_data)
207207

208208
assert serializer.is_valid()
209-
assert serializer.validated_data['nested']['example'] == {1, 2}
209+
assert serializer.validated_data['nested']['example'] == [1, 2]
210210

211211

212212
class TestNotRequiredNestedSerializerWithMany:

0 commit comments

Comments
 (0)