Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Fix 1.6 regression when GenericRelation+ForeignKey path used as admin list_filter #1913

Closed
wants to merge 3 commits into from

2 participants

Stephen McDonald Anssi Kääriäinen
Stephen McDonald

As mentioned in trac #21428

Previously in 1.5 it was possible to have a model A with a GenericRelation field to a model B that had a ForeignKey to model C, and use the B__C path in A's admin list_filter.

In 1.6 this fails in django.contrib.admin.util.get_model_from_relation where the model on the wrong side of the relationship is returned, when the correct model is available via the parent_model attribute.

Anssi Kääriäinen
Owner

As said in #21428 a separate ticket is the way to go here.

I think I want to patch L428 of django/db/models/options.py and assign f directly of f.related to the cache there. This requires also changing the direction of GenericRelation.get_path_info(). A possible problem is that there might be already code out there that relies on the new way, if so some other way will need to be invented. Just fetching parent_model if that exists (as done in this PR) seems wrong - parent_model always exists for RelatedObject, so the elif isinstance(field, models.related.RelatedObject) branch will never get executed.

Stephen McDonald

New ticket here: https://code.djangoproject.com/ticket/21431

As discussed I'm not really across how the internals of this work so I don't think I can help much more.

Anssi Kääriäinen
Owner

Fixed in 752d3d7

Anssi Kääriäinen akaariai closed this November 21, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
39  django/contrib/admin/tests.py
... ...
@@ -1,11 +1,48 @@
1 1
 import os
2 2
 
3  
-from django.test import LiveServerTestCase
  3
+from django.contrib import admin
  4
+from django.contrib.admin.validation import ModelAdminValidator
  5
+from django.contrib.contenttypes.models import ContentType
  6
+from django.contrib.contenttypes import generic
  7
+from django.core.exceptions import ImproperlyConfigured
  8
+from django.db import models
  9
+from django.test import LiveServerTestCase, TestCase
4 10
 from django.utils.module_loading import import_by_path
5 11
 from django.utils.unittest import SkipTest
6 12
 from django.utils.translation import ugettext as _
7 13
 
8 14
 
  15
+class AdminGenericRelationTestCase(TestCase):
  16
+
  17
+    def test_generic_relation_fk_list_filter(self):
  18
+        """
  19
+        Validates a model with a generic relation to a model with
  20
+        a foreign key can specify the generic+fk relationship
  21
+        path as a list_filter. See trac #21428.
  22
+        """
  23
+
  24
+        class FK(models.Model):
  25
+            pass
  26
+
  27
+        class GenericWithFK(models.Model):
  28
+            content_type = models.ForeignKey(ContentType)
  29
+            object_id = models.PositiveIntegerField()
  30
+            content_object = generic.GenericForeignKey('content_type', 'object_id')
  31
+            fk_field = models.ForeignKey(FK)
  32
+
  33
+        class HasGeneric(models.Model):
  34
+            generic = generic.GenericRelation(GenericWithFK)
  35
+
  36
+        class GenericFKAdmin(admin.ModelAdmin):
  37
+            list_filter = ('generic__fk_field',)
  38
+
  39
+        validator = ModelAdminValidator()
  40
+        try:
  41
+            validator.validate_list_filter(GenericFKAdmin, HasGeneric)
  42
+        except ImproperlyConfigured:
  43
+            self.fail("Couldn't validate a GenericRelation -> FK path in ModelAdmin.list_filter")
  44
+
  45
+
9 46
 class AdminSeleniumWebDriverTestCase(LiveServerTestCase):
10 47
 
11 48
     available_apps = [
4  django/contrib/admin/util.py
@@ -379,7 +379,9 @@ class NotRelationField(Exception):
379 379
 
380 380
 
381 381
 def get_model_from_relation(field):
382  
-    if isinstance(field, models.related.RelatedObject):
  382
+    if hasattr(field, 'parent_model'):
  383
+        return field.parent_model
  384
+    elif isinstance(field, models.related.RelatedObject):
383 385
         return field.model
384 386
     elif getattr(field, 'rel'): # or isinstance?
385 387
         return field.rel.to
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.