-
Notifications
You must be signed in to change notification settings - Fork 299
Add support for related links using parent view and its permissions #451
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
Changes from 11 commits
ffe61c6
196d8ba
fc52dc4
73d51a5
7677042
bde21ec
9e78e67
d9b5f08
9decc1f
77ac0b2
6e0b47c
a210e63
c148507
c0f0dab
228b1e8
8191d6d
b8a902f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
from collections import Iterable | ||
|
||
from django.core.exceptions import ImproperlyConfigured | ||
from django.db.models import Model | ||
from django.db.models.fields.related_descriptors import ( | ||
|
@@ -9,6 +11,7 @@ | |
from django.db.models.manager import Manager | ||
from django.db.models.query import QuerySet | ||
from django.urls import NoReverseMatch | ||
from django.utils.module_loading import import_string as import_class_from_dotted_path | ||
from rest_framework import generics, viewsets | ||
from rest_framework.exceptions import MethodNotAllowed, NotFound | ||
from rest_framework.response import Response | ||
|
@@ -98,12 +101,87 @@ def get_queryset(self, *args, **kwargs): | |
return qs | ||
|
||
|
||
class ModelViewSet(AutoPrefetchMixin, PrefetchForIncludesHelperMixin, viewsets.ModelViewSet): | ||
class RelatedMixin(object): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any reason we shouldn't directly derive There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea. That was my initial PR, so I made things as simple as possible. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think so too so let's extend There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great. |
||
""" | ||
This mixin handles all related entities, whose Serializers are declared in "related_serializers" | ||
""" | ||
related_serializers = {} | ||
|
||
related_field_mapping = {} | ||
|
||
def retrieve_related(self, request, *args, **kwargs): | ||
serializer_kwargs = {} | ||
instance = self.get_related_instance() | ||
|
||
if hasattr(instance, 'all'): | ||
instance = instance.all() | ||
|
||
if callable(instance): | ||
instance = instance() | ||
|
||
if instance is None: | ||
return Response(data=None) | ||
|
||
if isinstance(instance, Iterable): | ||
serializer_kwargs['many'] = True | ||
|
||
serializer = self.get_serializer(instance, **serializer_kwargs) | ||
return Response(serializer.data) | ||
|
||
def get_serializer_class(self): | ||
parent_serializer_class = super(RelatedMixin, self).get_serializer_class() | ||
|
||
if 'related_field' in self.kwargs: | ||
field_name = self.kwargs['related_field'] | ||
|
||
assert hasattr(parent_serializer_class, 'included_serializers')\ | ||
or self.related_serializers,\ | ||
|
||
'Either "included_serializers" or ' \ | ||
'"related_serializers" should be configured' | ||
|
||
# Try get the class from related_serializers | ||
class_str = self.related_serializers.get(field_name, None) | ||
|
||
if class_str is None: | ||
# Class was not found in related_serializers, look for it in included_serializers | ||
class_str = getattr(self, 'included_serializers', {}).get(field_name, None) | ||
|
||
if class_str is None: | ||
raise NotFound | ||
return import_class_from_dotted_path(class_str) | ||
|
||
return parent_serializer_class | ||
|
||
def get_related_field_name(self): | ||
field_name = self.kwargs['related_field'] | ||
if field_name in self.related_field_mapping: | ||
return self.related_field_mapping[field_name] | ||
return field_name | ||
|
||
def get_related_instance(self): | ||
parent_obj = self.get_object() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice! Really like it. I think we should now remove the We can still leave the method |
||
parent_serializer = self.serializer_class(parent_obj) | ||
field_name = self.get_related_field_name() | ||
field = parent_serializer.fields.get(field_name, None) | ||
|
||
if field is not None: | ||
return field.get_attribute(parent_obj) | ||
else: | ||
try: | ||
return getattr(parent_obj, field_name) | ||
except AttributeError: | ||
raise NotFound | ||
|
||
|
||
class ModelViewSet(AutoPrefetchMixin, | ||
PrefetchForIncludesHelperMixin, | ||
RelatedMixin, | ||
viewsets.ModelViewSet): | ||
pass | ||
|
||
|
||
class ReadOnlyModelViewSet(AutoPrefetchMixin, | ||
PrefetchForIncludesHelperMixin, | ||
RelatedMixin, | ||
viewsets.ReadOnlyModelViewSet): | ||
pass | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have more insight into generation of this links. Can you briefly explain why this is needed? In what case would this if match?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah!
The
if
would match in any case you didn't pass the argument when declaring a field, or the argument value equals 'pk'.When
related_link_url_kwarg == 'pk'
we can suppose the url was declared like this:and the route has 2 parameters:
pk
andrelated_field
. Exactly likeself_link
. That is why we do:related_kwargs = self_kwargs
in the next line.In any other cases (
related_link_url_kwarg != 'pk'
) we assume that we have "old-style" urls declaration like this:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense. What do you think about adding a small doc string explaining this?
Also do you think this could break backwards compatibility in case someone used
pk
in its related link? Although documentation states differently I guess.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your added docstring looks good. Thanks.