-
-
Notifications
You must be signed in to change notification settings - Fork 7.1k
Description
Checklist
- I have verified that that issue exists against the
masterbranch of Django REST framework. - I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
- This is not a usage question. (Those should be directed to the discussion group instead.)
- This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third-party libraries where possible.)
- I have reduced the issue to the simplest possible case.
- I have included a failing test as a pull request. (If you are unable to do so we can still accept the issue.)
Steps to reproduce
- For example, we have serializer like this:
class CommunitySerializer(Serializer):
colors = ListField(allow_null=True, child=CharField(label='Colors', max_length=7), required=False)- We submit partial update encoded as form data:
test_client.patch('/communities/1/', {'colors[0]': '#ffffff', 'colors[1]': '#000000'}, 'multipart')- This will encode request body like this:
--BoUnDaRyStRiNg
Content-Disposition: form-data; name="colors[0]"
#000000
--BoUnDaRyStRiNg
Content-Disposition: form-data; name="colors[1]"
#ffffff
--BoUnDaRyStRiNg--
Expected behavior
validated_data should contain colors value parsed into a list with the respected order.
Actual behavior
validated_data is an empty dictionary.
Explanation
ListField.get_valuechecks that field is present withinstatement:
django-rest-framework/rest_framework/fields.py
Line 1639 in ed6340e
| if self.field_name not in dictionary: |
- Since
field_namedoes not match submitted keys, method checks in this is a partial update
django-rest-framework/rest_framework/fields.py
Line 1640 in ed6340e
| if getattr(self.root, 'partial', False): |
- Since this is also true, the method returns an empty marker.
django-rest-framework/rest_framework/fields.py
Line 1641 in ed6340e
| return empty |
- Field name with ordered marker does not check to present.
Alternative
We can submit the partial update with list fields unordered:
test_client.patch('/communities/1/', {'colors': ['#ffffff', '#000000']}, 'multipart')Form data will look like this
--BoUnDaRyStRiNg
Content-Disposition: form-data; name="colors"
#000000
--BoUnDaRyStRiNg
Content-Disposition: form-data; name="colors"
#ffffff
--BoUnDaRyStRiNg--
So in statement will evaluate to true, so dictionary will be processed by HTML parser.
django-rest-framework/rest_framework/fields.py
Line 1649 in ed6340e
| return html.parse_html_list(dictionary, prefix=self.field_name, default=empty) |
But this format does not guarantee items order in the list, so we can not use it for things like gradients.
Full update and create methods do not have this problem because partial check evaluates to false.
We can't use full updates because we have two file fields near. We don't want to force the user to upload two files just to change string values in the same request.
P.S.
If you think we should solve this, I'll be able to make a PR for it.