Skip to content
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

Set child instance to an instance instead of QuerySet. #2720

Closed
wants to merge 1 commit into from

Conversation

lukashambsch
Copy link

I came across this issue using drf-bulk to do a bulk update on a model with the unique_together validator on it. Looking into it further I came across the following issues that mirrored my problem:

#2575
miki725/django-rest-framework-bulk#30

This should allow validation to be run on each instance in the ListSerializer.

@tomchristie
Copy link
Member

This pull request would assume that the representation includes an 'id' primary key.
We can't handle a general case for this, because there isn't one.

@kevin-brown
Copy link
Member

Just to copy the comment from the other pull request

Instead of handling querysets in the validation, we should ensure that the validator is applied per item, not at the list serialiser level.

There are deeper issues here which need to be looked into before a fix is made.

@paulcollinsiii
Copy link

so as I understand it the list serializer AND the child serializer are instantiated at the same time and get copies of almost all the initialization attributes. It looks like the child serializer is actually doing validation per item, but it's poking at self.instance and expecting it to be a single instance. If the ListSerializer was adjusted to take all the attributes of the item passed in, and do a .get() based on that (assuming such a query would produce a unique result and is secure) to give to the self.child.instance that has kind of the same effect while being slightly more generic right?

Deeper issue wise I need to do some more reading into how the many_init class method works, but it seems the main issue from this and the other PR is even touching the self.instance queryset or have I misunderstood? It appears that most of the problems are caused by how the queryset is being shared between both the ListSerializer AND the child serializer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants