Skip to content

fix: MultipleChoiceField use ordered sort #9735

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions rest_framework/fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -1469,17 +1469,17 @@ def to_internal_value(self, data):
if not self.allow_empty and len(data) == 0:
self.fail('empty')

return {
# Arguments for super() are needed because of scoping inside
# comprehensions.
# Arguments for super() are needed because of scoping inside
# comprehensions.
return list(dict.fromkeys([
super(MultipleChoiceField, self).to_internal_value(item)
for item in data
}
]))

def to_representation(self, value):
return {
return list(dict.fromkeys([
self.choice_strings_to_values.get(str(item), item) for item in value
}
]))


class FilePathField(ChoiceField):
Expand Down
33 changes: 29 additions & 4 deletions tests/test_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

import pytest

from rest_framework.utils import json

try:
import pytz
except ImportError:
Expand Down Expand Up @@ -2056,16 +2058,18 @@ class TestMultipleChoiceField(FieldValues):
Valid and invalid values for `MultipleChoiceField`.
"""
valid_inputs = {
(): set(),
('aircon',): {'aircon'},
('aircon', 'manual'): {'aircon', 'manual'},
(): list(),
('aircon',): ['aircon'],
('aircon', 'manual'): ['aircon', 'manual'],
('manual', 'aircon'): ['manual', 'aircon'],
}
invalid_inputs = {
'abc': ['Expected a list of items but got type "str".'],
('aircon', 'incorrect'): ['"incorrect" is not a valid choice.']
}
outputs = [
(['aircon', 'manual', 'incorrect'], {'aircon', 'manual', 'incorrect'})
(['aircon', 'manual', 'incorrect'], ['aircon', 'manual', 'incorrect']),
(['manual', 'aircon', 'incorrect'], ['manual', 'aircon', 'incorrect']),
]
field = serializers.MultipleChoiceField(
choices=[
Expand All @@ -2082,6 +2086,27 @@ def test_against_partial_and_full_updates(self):
field.partial = True
assert field.get_value(QueryDict('')) == rest_framework.fields.empty

def test_valid_inputs_is_json_serializable(self):
for input_value, _ in get_items(self.valid_inputs):
validated = self.field.run_validation(input_value)

try:
json.dumps(validated)
except TypeError as e:
pytest.fail(f'Validated output not JSON serializable: {repr(validated)}; Error: {e}')

def test_output_is_json_serializable(self):
for output_value, _ in get_items(self.outputs):
representation = self.field.to_representation(output_value)

try:
json.dumps(representation)
except TypeError as e:
pytest.fail(
f'to_representation output not JSON serializable: '
f'{repr(representation)}; Error: {e}'
)


class TestEmptyMultipleChoiceField(FieldValues):
"""
Expand Down
4 changes: 2 additions & 2 deletions tests/test_serializer_nested.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,14 +199,14 @@ def test_nested_serializer_with_list_json(self):
serializer = self.Serializer(data=input_data)

assert serializer.is_valid()
assert serializer.validated_data['nested']['example'] == {1, 2}
assert serializer.validated_data['nested']['example'] == [1, 2]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum, that's a bit unfortunate that this data type is exposed at this level. I'm thinking of potential users testing at the serializer level might get some new test failures with this change.


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

assert serializer.is_valid()
assert serializer.validated_data['nested']['example'] == {1, 2}
assert serializer.validated_data['nested']['example'] == [1, 2]


class TestNotRequiredNestedSerializerWithMany:
Expand Down
Loading