Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Add related_query_name to ForeignKey/M2M. Refs #20244

  • Loading branch information...
commit 99b467f272da91b8894dc90d793d8d2c40b78d8c 1 parent e26b589
@andrewgodwin andrewgodwin authored
View
19 django/db/models/fields/related.py
@@ -139,7 +139,7 @@ def related_query_name(self):
# related object in a table-spanning query. It uses the lower-cased
# object_name by default, but this can be overridden with the
# "related_name" option.

Could add a comment here about related_query_name

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
- return self.rel.related_name or self.opts.model_name
+ return self.rel.related_query_name or self.rel.related_name or self.opts.model_name
class RenameRelatedObjectDescriptorMethods(RenameMethodsBase):
@@ -826,7 +826,7 @@ def __set__(self, instance, value):
class ForeignObjectRel(object):
def __init__(self, field, to, related_name=None, limit_choices_to=None,
- parent_link=False, on_delete=None):
+ parent_link=False, on_delete=None, related_query_name=None):
try:
to._meta
except AttributeError: # to._meta doesn't exist, so it must be RECURSIVE_RELATIONSHIP_CONSTANT
@@ -835,6 +835,7 @@ def __init__(self, field, to, related_name=None, limit_choices_to=None,
self.field = field
self.to = to
self.related_name = related_name
+ self.related_query_name = related_query_name
self.limit_choices_to = {} if limit_choices_to is None else limit_choices_to
self.multiple = True
self.parent_link = parent_link
@@ -862,10 +863,10 @@ def set_field_name(self):
class ManyToOneRel(ForeignObjectRel):
def __init__(self, field, to, field_name, related_name=None, limit_choices_to=None,
- parent_link=False, on_delete=None):
+ parent_link=False, on_delete=None, related_query_name=None):
super(ManyToOneRel, self).__init__(
field, to, related_name=related_name, limit_choices_to=limit_choices_to,
- parent_link=parent_link, on_delete=on_delete)
+ parent_link=parent_link, on_delete=on_delete, related_query_name=related_query_name)
self.field_name = field_name
def get_related_field(self):
@@ -885,21 +886,22 @@ def set_field_name(self):
class OneToOneRel(ManyToOneRel):
def __init__(self, field, to, field_name, related_name=None, limit_choices_to=None,
- parent_link=False, on_delete=None):
+ parent_link=False, on_delete=None, related_query_name=None):
super(OneToOneRel, self).__init__(field, to, field_name,
related_name=related_name, limit_choices_to=limit_choices_to,
- parent_link=parent_link, on_delete=on_delete
+ parent_link=parent_link, on_delete=on_delete, related_query_name=related_query_name,
)
self.multiple = False
class ManyToManyRel(object):
def __init__(self, to, related_name=None, limit_choices_to=None,
- symmetrical=True, through=None, db_constraint=True):
+ symmetrical=True, through=None, db_constraint=True, related_query_name=None):
if through and not db_constraint:
raise ValueError("Can't supply a through model and db_constraint=False")
self.to = to
self.related_name = related_name
+ self.related_query_name = related_query_name
if limit_choices_to is None:
limit_choices_to = {}
self.limit_choices_to = limit_choices_to
@@ -933,6 +935,7 @@ def __init__(self, to, from_fields, to_fields, **kwargs):
kwargs['rel'] = ForeignObjectRel(
self, to,
related_name=kwargs.pop('related_name', None),
+ related_query_name=kwargs.pop('related_query_name', None),
limit_choices_to=kwargs.pop('limit_choices_to', None),
parent_link=kwargs.pop('parent_link', False),
on_delete=kwargs.pop('on_delete', CASCADE),
@@ -1141,6 +1144,7 @@ def __init__(self, to, to_field=None, rel_class=ManyToOneRel,
kwargs['rel'] = rel_class(
self, to, to_field,
related_name=kwargs.pop('related_name', None),
+ related_query_name=kwargs.pop('related_query_name', None),
limit_choices_to=kwargs.pop('limit_choices_to', None),
parent_link=kwargs.pop('parent_link', False),
on_delete=kwargs.pop('on_delete', CASCADE),
@@ -1340,6 +1344,7 @@ def __init__(self, to, db_constraint=True, **kwargs):
kwargs['verbose_name'] = kwargs.get('verbose_name', None)
kwargs['rel'] = ManyToManyRel(to,
related_name=kwargs.pop('related_name', None),
+ related_query_name=kwargs.pop('related_query_name', None),
limit_choices_to=kwargs.pop('limit_choices_to', None),
symmetrical=kwargs.pop('symmetrical', to == RECURSIVE_RELATIONSHIP_CONSTANT),
through=kwargs.pop('through', None),
View
8 tests/foreign_object/models.py
@@ -150,3 +150,11 @@ class ArticleTranslation(models.Model):
class Meta:
unique_together = ('article', 'lang')
ordering = ('active_translation__title',)
+
+class ArticleTag(models.Model):
+ article = models.ForeignKey(Article, related_name="tags", related_query_name="tag")
+ name = models.CharField(max_length=255)
+
+class ArticleIdea(models.Model):
+ articles = models.ManyToManyField(Article, related_name="ideas", related_query_name="idea_things")
+ name = models.CharField(max_length=255)
View
21 tests/foreign_object/tests.py
@@ -1,9 +1,10 @@
import datetime
from operator import attrgetter
-from .models import Country, Person, Group, Membership, Friendship, Article, ArticleTranslation
+from .models import Country, Person, Group, Membership, Friendship, Article, ArticleTranslation, ArticleTag, ArticleIdea
from django.test import TestCase
from django.utils.translation import activate
+from django.core.exceptions import FieldError
from django import forms
class MultiColumnFKTests(TestCase):
@@ -321,6 +322,24 @@ def test_foreign_key_raises_informative_does_not_exist(self):
with self.assertRaisesMessage(Article.DoesNotExist, 'ArticleTranslation has no article'):
referrer.article
+ def test_foreign_key_related_query_name(self):
+ a1 = Article.objects.create(pub_date=datetime.date.today())
+ ArticleTag.objects.create(article=a1, name="foo")
+ self.assertEqual(Article.objects.filter(tag__name="foo").count(), 1)
+ self.assertEqual(Article.objects.filter(tag__name="bar").count(), 0)
+ with self.assertRaises(FieldError):
+ Article.objects.filter(tags__name="foo")
+
+ def test_many_to_many_related_query_name(self):
+ a1 = Article.objects.create(pub_date=datetime.date.today())
+ i1 = ArticleIdea.objects.create(name="idea1")
+ a1.ideas.add(i1)
+ self.assertEqual(Article.objects.filter(idea_things__name="idea1").count(), 1)
+ self.assertEqual(Article.objects.filter(idea_things__name="idea2").count(), 0)
+ with self.assertRaises(FieldError):
+ Article.objects.filter(ideas__name="idea1")
+
+
class FormsTests(TestCase):
# ForeignObjects should not have any form fields, currently the user needs
# to manually deal with the foreignobject relation.

6 comments on commit 99b467f

@mjtamlyn
Collaborator

Should this be documented?

@andrewgodwin

I'm still not sure if we want to document it or not. I guess if we're going to the trouble of adding it we probably should.

@mjtamlyn
Collaborator

It seems a pretty useful feature to me, so worth documenting.

@benjaoming

Could add a comment here about related_query_name

@benjaoming

Thanks Andrew!!

@mjtamlyn Has the need for this feature emerged before swappable models were introduced? I don't find it likely.. I would say that while being useful for fixing #1203, it is not very useful in so many other scenarios, except where there are field name and related query name clashes (which are solvable by renaming fields, anyways). I'm suggesting to leave it undocumented as to not encourage using it or clutter the documentation.

@andrewgodwin

@benjaoming It's worth documenting as it's the only way to override related_name and use old default behaviours if you want that kind of thing. It's not very much documentation, just an extra couple of paragraphs, so I don't think it's an issue.

@benjaoming

Please correct me, if I'm wrong, but isn't the usage scenario something like?

  1. Has a default behavior and uses both instance.relationname_set and objects.filter(relationname__field=...)
  2. Introduces related_name='relationname_set' and would in Django<=1.5 have to refactor all objects.filter(relationname__field=...) to objects.filter(relationname_set__field=...)
  3. In Django 1.6, uses related_query_name='relation' to maintain the old default related query name. Does not have to refactor.

...or when the model is a swappable model and gets swapped out by a differently named model, we should always either set related_name or both related_name and related_query_name (otherwise this should raise a Warning in the relation!!). The latter is useful in cases where "swappability" is introduced on relations where related_name was not set, thus depending on a default naming behavior - just as we saw with the User model. But I don't think more swappable models are in the pipeline, and for now, it's safe to introduce the Warning, because PermissionMixin has been fixed, so now it's a matter of having Django 1.6 raise Warnings...?

AFAIK, swappability is not universally implemented, but as of now, only intended for the User model.

While all this is very useful for fixing PermissionMixin, I don't think it's a very common use case that should be in the docs.. I would give more focus to preserving the readability of the docs.. so I suggest leaving it out. 1/100,000 documentation readers need it and everyone else would be better off with using related_name as the sole parameter.

...and I wouldn't know how to write the above explanation in a meaningful way for the docs :)

(Btw sry, I'm not a regular contributor, just a nit-picker on this issue, because it happened to affect me)

Please sign in to comment.
Something went wrong with that request. Please try again.