encode / django-rest-framework

Web APIs for Django. 🎸
https://www.django-rest-framework.org
Other
28.19k stars 6.81k forks source link

ChoiceFields has incorrect key value mapping #8132

Closed codekaust closed 3 years ago

codekaust commented 3 years ago

https://github.com/encode/django-rest-framework/blob/c5d9144aef1144825942ddffe0a6af23102ef44a/rest_framework/fields.py#L1459

The choice field key should be mapped to the corresponding value but is mapped to the key itself.

tomchristie commented 3 years ago

I can see that it looks odd, yes. But it really does do what it's supposed to.

Choices here isn't a dictionary of {key: value}, it's a plain list of values. What we want in this portion of the code is a mapping from the string representation of the value to the actual value itself. (Eg. {"1": 1})

We need that because eg. HTML form submissions will always just be plain strings.

(Bit awkward to explain.)

james-mchugh commented 5 months ago

I know this thread is old, but I'm facing a similar issue. From my experience, it does look odd, but it also doesn't work the way I would expect. The behavior I'm expecting is that the serializer handles mapping between the choice's display name and the internal value. That does not match what I'm observing though. I'm not sure if it's just me doing something wrong, misunderstanding how this is supposed to work, or if there is an actual bug.

In my code, I have a model like so

class Example(models.Model):
    class Year(models.IntegerChoices):
        Fresh = 0, "Freshman"

    year = models.IntegerField(choices=Year)

and a serializer like so

class ExampleSerializer(serializers.ModelSerializer):

    class Meta:
        model = Example
        fields = "__all__"

When I deserialize incoming data, I would expect it to include the human readable, display name of "Freshman", but that doesn't match the observed functionality:


In [2]: s = ExampleSerializer(data={"year": "Freshman"})

In [4]: s.is_valid()
Out[4]: False

In [5]: s.errors
Out[5]: {'year': [ErrorDetail(string='"Freshman" is not a valid choice.', code='invalid_choice')]}

In [6]: s = views.ExampleSerializer(data={"year": 0})

In [7]: s.is_valid()
Out[7]: True

Similarly, when serializing data for output, I would expect it to use the display name, but that also doesn't work as expected:

In [12]: s = views.ExampleSerializer(instance=instance)

In [13]: s.data
Out[13]: {'id': 1, 'year': 0}

The serializer appears to be created correctly and uses the ChoiceField, but it does not appear to be doing any mapping between the internal value and the display name.

In [15]:ExampleSerializer()
Out[15]: 
ExampleSerializer():
    id = IntegerField(label='ID', read_only=True)
    year = ChoiceField(choices=[(0, 'Freshman')], validators=[<django.core.validators.MinValueValidator object>, <django.core.validators.MaxValueValidator object>])

Is this expected functionality? Or is something wrong either with what I'm doing or how the code is working?

james-mchugh commented 5 months ago

I think I see now. It seems the display name is only expected to be used in the frontend, which should perform the mapping of the display name to the internal value prior to sending it to the API. Is that correct?

tomchristie commented 5 months ago

Yes, the display name is only used in HTML rendering of a form control. The value is what's used by the API representations (and is also the actual value of the HTML form element - the display text is not what is submitted).