Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Fix included resources not being included on list #307

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

Merged
mblayman merged 5 commits into django-json-api:develop from rpatterson:develop
Feb 22, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions example/serializers.py
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ class Meta:
'authors', 'comments', 'featured', 'suggested',)
meta_fields = ('body_format',)

class JSONAPIMeta:
included_resources = ['comments']


class AuthorBioSerializer(serializers.ModelSerializer):

Expand Down
7 changes: 0 additions & 7 deletions example/tests/integration/test_includes.py
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,9 @@

from example.tests.utils import load_json

try:
from unittest import mock
except ImportError:
import mock

pytestmark = pytest.mark.django_db


@mock.patch('rest_framework_json_api.utils.get_default_included_resources_from_serializer', new=lambda s: ['comments'])
def test_default_included_data_on_list(multiple_entries, client):
return test_included_data_on_list(multiple_entries=multiple_entries, client=client, query='?page_size=5')

Expand All @@ -28,7 +22,6 @@ def test_included_data_on_list(multiple_entries, client, query='?include=comment
assert comment_count == expected_comment_count, 'List comment count is incorrect'


@mock.patch('rest_framework_json_api.utils.get_default_included_resources_from_serializer', new=lambda s: ['comments'])
def test_default_included_data_on_detail(single_entry, client):
return test_included_data_on_detail(single_entry=single_entry, client=client, query='')

Expand Down
9 changes: 9 additions & 0 deletions example/tests/integration/test_non_paginated_responses.py
View file Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
from django.core.urlresolvers import reverse
from django.conf import settings

try:
from unittest import mock
except ImportError:
import mock

import pytest

from example.views import EntryViewSet
Expand All @@ -12,6 +17,10 @@


# rf == request_factory
@mock.patch(
'rest_framework_json_api.utils'
'.get_default_included_resources_from_serializer',
new=lambda s: [])
def test_multiple_entries_no_pagination(multiple_entries, rf):

expected = {
Expand Down
10 changes: 9 additions & 1 deletion example/tests/integration/test_pagination.py
View file Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
from django.core.urlresolvers import reverse

try:
from unittest import mock
except ImportError:
import mock

import pytest
from example.tests.utils import dump_json, redump_json

pytestmark = pytest.mark.django_db


@mock.patch(
'rest_framework_json_api.utils'
'.get_default_included_resources_from_serializer',
new=lambda s: [])
def test_pagination_with_single_entry(single_entry, client):

expected = {
Expand Down
15 changes: 11 additions & 4 deletions rest_framework_json_api/utils.py
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,13 @@ def get_related_resource_type(relation):
relation_model = relation.model
elif hasattr(relation, 'get_queryset') and relation.get_queryset() is not None:
relation_model = relation.get_queryset().model
elif (
getattr(relation, 'many', False) and
hasattr(relation.child, 'Meta') and
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if these hasattr checks are a bit too much. A serializer doesn't have to be a ModelSerializer so there is no requirement for Meta to exist, but isn't model a required attribute of Meta for a ModelSerializer? It feels slightly too defensive to me to check that model exists.

Copy link
Contributor Author

@rpatterson rpatterson Feb 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand. If the serializer may not be a ModelSerializer, then shouldn't we not assume that Meta has a model?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you're right. I thought a vanilla serializer made no use of Meta, but I can see from the source that that is not correct. By all means, please keep model. 😊

hasattr(relation.child.Meta, 'model')):
# For ManyToMany relationships, get the model from the child
# serializer of the list serializer
relation_model = relation.child.Meta.model
else:
parent_serializer = relation.parent
parent_model = None
Expand Down Expand Up @@ -266,10 +273,10 @@ def get_included_resources(request, serializer=None):


def get_default_included_resources_from_serializer(serializer):
try:
return list(serializer.JSONAPIMeta.included_resources)
except AttributeError:
return []
meta = getattr(serializer, 'JSONAPIMeta', None)
if meta is None and getattr(serializer, 'many', False):
meta = getattr(serializer.child, 'JSONAPIMeta', None)
return list(getattr(meta, 'included_resources', []))


def get_included_serializers(serializer):
Expand Down

AltStyle によって変換されたページ (->オリジナル) /