-
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 5 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 |
---|---|---|
|
@@ -3,6 +3,9 @@ | |
|
||
from django.db import models | ||
from django.utils.encoding import python_2_unicode_compatible | ||
from django.contrib.contenttypes.models import ContentType | ||
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. After reviewing this package's code, imports are usually alphabetically sorted. Could you move these |
||
from django.contrib.contenttypes.fields import GenericForeignKey | ||
from django.contrib.contenttypes.fields import GenericRelation | ||
|
||
|
||
class BaseModel(models.Model): | ||
|
@@ -16,10 +19,21 @@ class Meta: | |
abstract = True | ||
|
||
|
||
class TaggedItem(BaseModel): | ||
tag = models.SlugField() | ||
content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE) | ||
object_id = models.PositiveIntegerField() | ||
content_object = GenericForeignKey('content_type', 'object_id') | ||
|
||
def __str__(self): | ||
return self.tag | ||
|
||
|
||
@python_2_unicode_compatible | ||
class Blog(BaseModel): | ||
name = models.CharField(max_length=100) | ||
tagline = models.TextField() | ||
tags = GenericRelation(TaggedItem) | ||
|
||
def __str__(self): | ||
return self.name | ||
|
@@ -54,6 +68,7 @@ class Entry(BaseModel): | |
n_comments = models.IntegerField(default=0) | ||
n_pingbacks = models.IntegerField(default=0) | ||
rating = models.IntegerField(default=0) | ||
tags = GenericRelation(TaggedItem) | ||
|
||
def __str__(self): | ||
return self.headline | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,6 @@ | |
pytest>=2.9.0,<3.0 | ||
pytest-django | ||
pytest-factoryboy | ||
fake-factory | ||
Faker | ||
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. Thanks for picking up this package name change. This change has bit me on a few projects already. |
||
tox | ||
mock |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,11 +30,14 @@ | |
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! |
||
ReverseGenericRelatedObjectsDescriptor = type(None) | ||
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 | ||
ReverseGenericManyToOneDescriptor = type(None) | ||
|
||
def get_resource_name(context): | ||
""" | ||
|
@@ -203,17 +206,22 @@ 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: | ||
relation_model = parent_model_relation.rel.model | ||
elif parent_model_relation_type is ReverseGenericRelatedObjectsDescriptor: | ||
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.
Could you call this
TaggedItemFactory
instead ofEntryTaggedItemFactory
? That seems more in line with the other factory naming conventions here.