Open PhilipGarnero opened 6 years ago
provide both many=True and instance=YourModel.objects.all()
I don't think that's supported usage. instance should be an instance, not a queryset.
Documentation suggests it should be: http://www.django-rest-framework.org/api-guide/serializers/#customizing-multiple-update
Note that the best way to address this would be to declare the fields explicitly, so that you don't end up with an auto-generated uniqueness validator (Ideally just use a fully explicit Serializer class rather than leaning on ModelSerializer)
The quick and dirty fix on my side was to put validators = ()
on the serializer for things to be working correctly.
I would find it a bit weird to have to define a Serializer instead of a ModelSerialier to serialize models
Moreover, it would be nearly the same as my actual model serializer
I think the usage right now is fine. We just need to prevent the crash from occuring or to warn people about it in the docs. As this is a pretty advanced usage, not many of us are going to encounter this issue anyways.
thinking about this, the unique validator will have some nasty side effects with many anyway. Like sending a value twice in the set. If it's not already in the db that would probably pass the validation and yet fail the DB insert. One thing I had to overcome a couple of years ago now that I think of it.
@PhilipGarnero Yup I think you've got exactly the right assessment there.
We just need to prevent the crash from occuring or to warn people about it in the docs.
I'd suggest we raise a RuntimeError in this case with an explicit message, and note in the docs that unique_together validation cannot automatically work on bulk updates, and that users should override validators
on any ModelSerializer, and handle the validation explicitly.
agreed
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
Checklist
master
branch of Django REST framework.When using a custom ListSerializer to update data and providing the instance as a queryset, if the model has some unique together constraint, the is_valid call crashes
Steps to reproduce
Create a model with a unique together constraint Create a serializer for this model (Probably not needed) Create a custom list serializer with the
update
method overwritten and provide it to your model serializer withlist_serializer_class
Now use the model serializer to serialize a list of data and provide bothmany=True
andinstance=YourModel.objects.all()
to your serializer__init__
Callserializer.is_valid(raise_exception=True)
Expected behavior
It should pass
is_valid
or at least not crashActual behavior
It crashes in the unique together validator because
instance
was not popped in the many_init so the child will receive a queryset instead of a model instanceHow to fix
Popping
instance
from the kwargs in many_init should do the trick but I'm not sure this won't introduce unforeseen problems Moreover, the validators will probably fail because they will consider the call as a create rather than an update so here is that