-
Notifications
You must be signed in to change notification settings - Fork 301
Add support for GenericRelations #319
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 all commits
82c81ef
ab27cd1
cfd0941
cd83e5c
0fd45e4
af61900
f9bfcc9
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 |
---|---|---|
@@ -0,0 +1,31 @@ | ||
# -*- coding: utf-8 -*- | ||
# Generated by Django 1.10.5 on 2017-02-01 08:34 | ||
from __future__ import unicode_literals | ||
|
||
from django.db import migrations, models | ||
import django.db.models.deletion | ||
|
||
|
||
class Migration(migrations.Migration): | ||
|
||
dependencies = [ | ||
('contenttypes', '0002_remove_content_type_name'), | ||
('example', '0001_initial'), | ||
] | ||
|
||
operations = [ | ||
migrations.CreateModel( | ||
name='TaggedItem', | ||
fields=[ | ||
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), | ||
('created_at', models.DateTimeField(auto_now_add=True)), | ||
('modified_at', models.DateTimeField(auto_now=True)), | ||
('tag', models.SlugField()), | ||
('object_id', models.PositiveIntegerField()), | ||
('content_type', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='contenttypes.ContentType')), | ||
], | ||
options={ | ||
'abstract': False, | ||
}, | ||
), | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,10 +30,12 @@ | |
if django.VERSION >= (1, 9): | ||
from django.db.models.fields.related_descriptors import ManyToManyDescriptor, ReverseManyToOneDescriptor | ||
ReverseManyRelatedObjectsDescriptor = type(None) | ||
from django.contrib.contenttypes.fields import ReverseGenericManyToOneDescriptor | ||
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. This method of having two mirrored imports that set one to the None type is something I found confusing. After digging through the Django contenttypes source, I see that what is happening is trying to deal with a rename of the descriptor from 1.8 to 1.9. Unfortunately, this method leads to extra branches in an already large if/elif section. How about using an import alias instead? if django.VERSION >= (1, 9):
# ... snip unmodified lines ...
from django.contrib.contenttypes.fields import ReverseGenericManyToOneDescriptor
else:
from django.contrib.contenttypes.fields import ReverseGenericRelatedObjectsDescriptor as ReverseGenericManyToOneDescriptor I believe this would clean up the code, be more efficient, and capture the fact that the descriptor was renamed. It seems this pattern was already followed for the first two imports that follow the 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! |
||
else: | ||
from django.db.models.fields.related import ManyRelatedObjectsDescriptor as ManyToManyDescriptor | ||
from django.db.models.fields.related import ForeignRelatedObjectsDescriptor as ReverseManyToOneDescriptor | ||
from django.db.models.fields.related import ReverseManyRelatedObjectsDescriptor | ||
from django.contrib.contenttypes.fields import ReverseGenericRelatedObjectsDescriptor as ReverseGenericManyToOneDescriptor | ||
|
||
|
||
def get_resource_name(context): | ||
|
@@ -210,17 +212,23 @@ def get_related_resource_type(relation): | |
else: | ||
parent_model_relation = getattr(parent_model, parent_serializer.field_name) | ||
|
||
if type(parent_model_relation) is ReverseManyToOneDescriptor: | ||
parent_model_relation_type = type(parent_model_relation) | ||
if parent_model_relation_type is ReverseManyToOneDescriptor: | ||
if django.VERSION >= (1, 9): | ||
relation_model = parent_model_relation.rel.related_model | ||
elif django.VERSION >= (1, 8): | ||
relation_model = parent_model_relation.related.related_model | ||
else: | ||
relation_model = parent_model_relation.related.model | ||
elif type(parent_model_relation) is ManyToManyDescriptor: | ||
elif parent_model_relation_type is ManyToManyDescriptor: | ||
relation_model = parent_model_relation.field.remote_field.model | ||
elif type(parent_model_relation) is ReverseManyRelatedObjectsDescriptor: | ||
elif parent_model_relation_type is ReverseManyRelatedObjectsDescriptor: | ||
relation_model = parent_model_relation.field.related.model | ||
elif parent_model_relation_type is ReverseGenericManyToOneDescriptor: | ||
if django.VERSION >= (1, 9): | ||
relation_model = parent_model_relation.rel.model | ||
else: | ||
relation_model = parent_model_relation.field.related_model | ||
else: | ||
return get_related_resource_type(parent_model_relation) | ||
|
||
|
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.
Thanks for picking up this package name change. This change has bit me on a few projects already.