Fix mapping for choice values #8968
Conversation
saadullahaleem
changed the title
rest_framework/fields.py
Outdated
| @@ -1398,6 +1399,9 @@ def to_internal_value(self, data): | |||
| if data == '' and self.allow_blank: | |||
| return '' | |||
|
|
|||
| if isinstance(data, IntegerChoices) and str(data) != str(data.value): | |||
There was a problem hiding this comment.
i'm happy with the handling of IntegerChoices but I would also love to see TextChoices and other buiilt in django enum choices compats as well
There was a problem hiding this comment.
@auvipy I've added TextChoices here too but should we have Enum too?
There was a problem hiding this comment.
It is also possible to make use of the Enum Functional API with the caveat that labels are automatically generated -- its is from the django docs so we can consider that. may be on the original PR?
| @@ -1824,6 +1826,31 @@ def test_edit_choices(self): | |||
| field.run_validation(2) | |||
| assert exc_info.value.detail == ['"2" is not a valid choice.'] | |||
|
|
|||
| def test_enum_choices(self): | |||
| class ChoiceCase(IntegerChoices): | |||
There was a problem hiding this comment.
we should add test for TextChoices too
There was a problem hiding this comment.
@auvipy added
| @@ -1406,6 +1411,11 @@ def to_internal_value(self, data): | |||
| def to_representation(self, value): | |||
| if value in ('', None): | |||
| return value | |||
|
|
|||
| if isinstance(value, (IntegerChoices, TextChoices)) and str(value) != \ | |||
There was a problem hiding this comment.
I have one concerns/question. should we consider both Int and text value as string?
There was a problem hiding this comment.
I've typecasted both the __str__ representation of value and value.value for the check here.
|
congrats for you first contribution and welcome aboard |
Description
This pull request fixes the mapping between the human-labels and the database-values for the
ChoiceFieldclass. This PR has been mutated from 8955Discussion 8965