Fix UpdateModelMixin to work when no queryset is defined is defined on the view#9314
Fix UpdateModelMixin to work when no queryset is defined is defined on the view#9314browniebroke wants to merge 1 commit intoencode:masterfrom
Conversation
| if queryset._prefetch_related_lookups: | ||
| if hasattr(instance, '_prefetched_objects_cache'): |
There was a problem hiding this comment.
When only get_object is defined, I assume that folks are unlikely to have advanced prefetches in their method (although it's very much possible).
If some things were prefetched, we assume that there is a get_queryset method.
browniebroke
force-pushed
the
fix-updatemixin-queryset-error
branch
from
80e09d0 to
89c79d6
Compare
yuekui
left a comment
yuekui
left a comment
There was a problem hiding this comment.
Yeah this should fix the case that if only get_object() was defined, looks good to me
| # forcibly invalidate the prefetch cache on the instance | ||
| instance._prefetched_objects_cache = {} | ||
| prefetch_related_objects([instance], *queryset._prefetch_related_lookups) | ||
| queryset = self.filter_queryset(self.get_queryset()) |
There was a problem hiding this comment.
This is still expecting get_queryset or queryset to be defined and will raise an AssertError if it is not, which means it will still not be backwards compatabile with 3.14 correct?
There was a problem hiding this comment.
How is it possible that self has '_prefetched_objects_cache' defined without defining queryset or get_queryset?
There was a problem hiding this comment.
That's what I meant by my comment earlier, one might do:
class UserRetrieveWithoutQuerySet(generics.RetrieveUpdateAPIView):
serializer_class = UserSerializer
def get_object(self):
return User.objects.prefetch_related('groups').get(pk=self.kwargs['pk'])In such case, I would argue that it's reasonable to require users to override the get_queryset method... The error message says exactly that. Would probably need to be documented better, though.
There was a problem hiding this comment.
Probably just an extra if statement or try catch would solve all these cases.
|
should we revert the actual implementation as breaking change? and then come with a better fix? or in this case we can just go with this? #9327 |
|
Going to close this as it becoming clear that we should revert. I was only trying to help get a stable 3.15.x out, which a revert achieves... |
4 participants
Description
It seems that the fix from #8043 isn't working in all cases, especially when the users aren't setting the
querysetattribute, or overriding theget_queryset()method, and instead opt to override theget_objectmethod.Attempt to fix this edge case by delaying the call of
get_queryset, to be only if the instance that was just updated had some prefetched relations.Fix #9306