-
Notifications
You must be signed in to change notification settings - Fork 301
Check resource name on included serializer in to_internal_value #306
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 5 commits
484adc2
1c8d835
0700202
76caa82
4053901
b0f0ed1
1001fa5
50ff418
f454894
fec7e0a
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,9 +1,10 @@ | ||
import pytest | ||
from django.core.urlresolvers import reverse | ||
from example import models, serializers, views | ||
from example.tests.utils import dump_json, load_json | ||
from rest_framework import status | ||
|
||
from example.tests.utils import load_json | ||
from django.core.urlresolvers import reverse | ||
|
||
from example import models, serializers, views | ||
pytestmark = pytest.mark.django_db | ||
|
||
|
||
|
@@ -37,6 +38,24 @@ def _check_relationship_and_included_comment_type_are_the_same(django_client, ur | |
@pytest.mark.usefixtures("single_entry") | ||
class TestModelResourceName: | ||
|
||
create_data = { | ||
'data': { | ||
'type': 'resource_name_from_JSONAPIMeta', | ||
'id': None, | ||
'attributes': { | ||
'body': 'example', | ||
}, | ||
'relationships': { | ||
'entry': { | ||
'data': { | ||
'type': 'resource_name_from_JSONAPIMeta', | ||
'id': 1 | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
def test_model_resource_name_on_list(self, client): | ||
models.Comment.__bases__ += (_PatchedModel,) | ||
response = client.get(reverse("comment-list")) | ||
|
@@ -46,7 +65,7 @@ def test_model_resource_name_on_list(self, client): | |
'resource_name from model incorrect on list') | ||
|
||
# Precedence tests | ||
def test_resource_name_precendence(self, client): | ||
def test_resource_name_precendence(self, client, monkeypatch): | ||
# default | ||
response = client.get(reverse("comment-list")) | ||
data = load_json(response.content)['data'][0] | ||
|
@@ -61,29 +80,43 @@ def test_resource_name_precendence(self, client): | |
'resource_name from model incorrect on list') | ||
|
||
# serializer > model | ||
serializers.CommentSerializer.Meta.resource_name = "resource_name_from_serializer" | ||
monkeypatch.setattr(serializers.CommentSerializer.Meta, 'resource_name', 'resource_name_from_serializer', False) | ||
response = client.get(reverse("comment-list")) | ||
data = load_json(response.content)['data'][0] | ||
assert (data.get('type') == 'resource_name_from_serializer'), ( | ||
'resource_name from serializer incorrect on list') | ||
|
||
# view > serializer > model | ||
views.CommentViewSet.resource_name = 'resource_name_from_view' | ||
monkeypatch.setattr(views.CommentViewSet, 'resource_name', 'resource_name_from_view', False) | ||
response = client.get(reverse("comment-list")) | ||
data = load_json(response.content)['data'][0] | ||
assert (data.get('type') == 'resource_name_from_view'), ( | ||
'resource_name from view incorrect on list') | ||
|
||
def test_model_resource_name_create(self, client): | ||
models.Comment.__bases__ += (_PatchedModel,) | ||
models.Entry.__bases__ += (_PatchedModel,) | ||
response = client.post(reverse("comment-list"), | ||
dump_json(self.create_data), | ||
content_type='application/vnd.api+json') | ||
|
||
assert response.status_code == status.HTTP_201_CREATED | ||
|
||
def test_serializer_resource_name_create(self, client, monkeypatch): | ||
monkeypatch.setattr(serializers.CommentSerializer.Meta, 'resource_name', 'renamed_comments', False) | ||
monkeypatch.setattr(serializers.EntrySerializer.Meta, 'resource_name', 'renamed_entries', False) | ||
self.create_data['data']['type'] = 'renamed_comments' | ||
self.create_data['data']['relationships']['entry']['data']['type'] = 'renamed_entries' | ||
|
||
response = client.post(reverse("comment-list"), | ||
dump_json(self.create_data), | ||
content_type='application/vnd.api+json') | ||
|
||
assert response.status_code == status.HTTP_201_CREATED | ||
|
||
def teardown_method(self, method): | ||
models.Comment.__bases__ = (models.Comment.__bases__[0],) | ||
try: | ||
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'm not super familiar with these tests. Would you mind explaining why this tear down code is not needed? 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. |
||
delattr(serializers.CommentSerializer.Meta, "resource_name") | ||
except AttributeError: | ||
pass | ||
try: | ||
delattr(views.CommentViewSet, "resource_name") | ||
except AttributeError: | ||
pass | ||
models.Entry.__bases__ = (models.Entry.__bases__[0],) | ||
|
||
|
||
@pytest.mark.usefixtures("single_entry") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import collections | ||
import inflection | ||
import json | ||
|
||
from rest_framework.fields import MISSING_ERROR_MESSAGE, SerializerMethodField | ||
|
@@ -123,7 +124,12 @@ def to_internal_value(self, data): | |
self.fail('incorrect_type', data_type=type(data).__name__) | ||
if not isinstance(data, dict): | ||
self.fail('incorrect_type', data_type=type(data).__name__) | ||
|
||
expected_relation_type = get_resource_type_from_queryset(self.queryset) | ||
serializer_resource_type = self.get_resource_type_from_included_serializer() | ||
|
||
if serializer_resource_type is not None: | ||
expected_relation_type = serializer_resource_type | ||
|
||
if 'type' not in data: | ||
self.fail('missing_type') | ||
|
@@ -142,19 +148,44 @@ def to_representation(self, value): | |
else: | ||
pk = value.pk | ||
|
||
# check to see if this resource has a different resource_name when | ||
# included and use that name | ||
resource_type = None | ||
root = getattr(self.parent, 'parent', self.parent) | ||
field_name = self.field_name if self.field_name else self.parent.field_name | ||
if getattr(root, 'included_serializers', None) is not None: | ||
includes = get_included_serializers(root) | ||
if field_name in includes.keys(): | ||
resource_type = get_resource_type_from_serializer(includes[field_name]) | ||
resource_type = self.get_resource_type_from_included_serializer() | ||
if resource_type is None: | ||
resource_type = get_resource_type_from_instance(value) | ||
|
||
resource_type = resource_type if resource_type else get_resource_type_from_instance(value) | ||
return OrderedDict([('type', resource_type), ('id', str(pk))]) | ||
|
||
def get_resource_type_from_included_serializer(self): | ||
""" | ||
Check to see it this resource has a different resource_name when | ||
included and return that name, or None | ||
""" | ||
field_name = self.field_name or self.parent.field_name | ||
root = self.get_root_serializer() | ||
|
||
if root is not None: | ||
# accept both singular and plural versions of field_name | ||
field_names = [ | ||
inflection.singularize(field_name), | ||
inflection.pluralize(field_name) | ||
] | ||
includes = get_included_serializers(root) | ||
for field in field_names: | ||
if field in includes.keys(): | ||
return get_resource_type_from_serializer(includes[field]) | ||
|
||
return None | ||
|
||
def get_root_serializer(self): | ||
if hasattr(self.parent, 'parent') and self.is_serializer(self.parent.parent): | ||
return self.parent.parent | ||
elif self.is_serializer(self.parent): | ||
return self.parent | ||
|
||
return None | ||
|
||
def is_serializer(self, candidate): | ||
return hasattr(candidate, 'included_serializers') | ||
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 this a true test of whether something is a serializer? A serializer could be somethign that donesn't have 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. You're right - i just verified that |
||
|
||
def get_choices(self, cutoff=None): | ||
queryset = self.get_queryset() | ||
if queryset is None: | ||
|
@@ -219,4 +250,4 @@ def to_representation(self, value): | |
if isinstance(value, collections.Iterable): | ||
base = super(SerializerMethodResourceRelatedField, self) | ||
return [base.to_representation(x) for x in value] | ||
return super(SerializerMethodResourceRelatedField, self).to_representation(value) | ||
return super(SerializerMethodResourceRelatedField, self).to_representation(value) |
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.
I think you should probably deepcopy
self.create_data
. Otherwise, I think you're modifying the class state for all tests.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.
Agreed!