Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fixed #22994 -- regression with generic FK + admin list_view
The reason for the regression was that the GenericForeignKey field isn't
something meta.get_field_by_name() should return. The reason is that a
couple of places in Django expects get_field_by_name() to work this way.
It could make sense to return GFKs from get_field_by_name(), but that
should likely be done as part of meta refactoring or virtual fields
refactoring patches.

Thanks to glicerinu@gmail.com for the report and to Tim for working on
the issue.
  • Loading branch information
akaariai authored and timgraham committed Jul 14, 2014
1 parent 38e001a commit 9cd5201
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 8 deletions.
4 changes: 3 additions & 1 deletion django/db/models/options.py
Expand Up @@ -441,7 +441,9 @@ def init_name_map(self):
for f, model in self.get_fields_with_model(): for f, model in self.get_fields_with_model():
cache[f.name] = cache[f.attname] = (f, model, True, False) cache[f.name] = cache[f.attname] = (f, model, True, False)
for f in self.virtual_fields: for f in self.virtual_fields:
cache[f.name] = (f, None if f.model == self.model else f.model, True, False) if hasattr(f, 'related'):
cache[f.name] = cache[f.attname] = (
f, None if f.model == self.model else f.model, True, False)
if apps.ready: if apps.ready:
self._name_map = cache self._name_map = cache
return cache return cache
Expand Down
7 changes: 6 additions & 1 deletion tests/admin_views/admin.py
Expand Up @@ -35,7 +35,7 @@
UnchangeableObject, UserMessenger, Simple, Choice, ShortMessage, Telegram, UnchangeableObject, UserMessenger, Simple, Choice, ShortMessage, Telegram,
FilteredManager, EmptyModelHidden, EmptyModelVisible, EmptyModelMixin, FilteredManager, EmptyModelHidden, EmptyModelVisible, EmptyModelMixin,
State, City, Restaurant, Worker, ParentWithDependentChildren, State, City, Restaurant, Worker, ParentWithDependentChildren,
DependentChild, StumpJoke, FieldOverridePost) DependentChild, StumpJoke, FieldOverridePost, FunkyTag)




def callable_year(dt_value): def callable_year(dt_value):
Expand Down Expand Up @@ -827,6 +827,10 @@ def get_changeform_initial_data(self, request):
return {'name': 'overridden_value'} return {'name': 'overridden_value'}




class FunkyTagAdmin(admin.ModelAdmin):
list_display = ('name', 'content_object')


site = admin.AdminSite(name="admin") site = admin.AdminSite(name="admin")
site.register(Article, ArticleAdmin) site.register(Article, ArticleAdmin)
site.register(CustomArticle, CustomArticleAdmin) site.register(CustomArticle, CustomArticleAdmin)
Expand Down Expand Up @@ -882,6 +886,7 @@ def get_changeform_initial_data(self, request):
site.register(City, CityAdmin) site.register(City, CityAdmin)
site.register(Restaurant, RestaurantAdmin) site.register(Restaurant, RestaurantAdmin)
site.register(Worker, WorkerAdmin) site.register(Worker, WorkerAdmin)
site.register(FunkyTag, FunkyTagAdmin)


# We intentionally register Promo and ChapterXtra1 but not Chapter nor ChapterXtra2. # We intentionally register Promo and ChapterXtra1 but not Chapter nor ChapterXtra2.
# That way we cover all four cases: # That way we cover all four cases:
Expand Down
17 changes: 16 additions & 1 deletion tests/admin_views/tests.py
Expand Up @@ -1718,11 +1718,26 @@ def test_generic_relations(self):
""" """
plot = Plot.objects.get(pk=3) plot = Plot.objects.get(pk=3)
FunkyTag.objects.create(content_object=plot, name='hott') FunkyTag.objects.create(content_object=plot, name='hott')
should_contain = """<li>Funky tag: hott""" should_contain = """<li>Funky tag: <a href="/test_admin/admin/admin_views/funkytag/1/">hott"""
response = self.client.get('/test_admin/admin/admin_views/plot/%s/delete/' % quote(3)) response = self.client.get('/test_admin/admin/admin_views/plot/%s/delete/' % quote(3))
self.assertContains(response, should_contain) self.assertContains(response, should_contain)




@override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',),
ROOT_URLCONF="admin_views.urls")
class TestGenericRelations(TestCase):
fixtures = ['admin-views-users.xml', 'deleted-objects.xml']

def setUp(self):
self.client.login(username='super', password='secret')

def test_generic_content_object_in_list_display(self):
plot = Plot.objects.get(pk=3)
FunkyTag.objects.create(content_object=plot, name='hott')
response = self.client.get('/test_admin/admin/admin_views/funkytag/')
self.assertContains(response, "%s</td>" % plot)


@override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',), @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',),
ROOT_URLCONF="admin_views.urls") ROOT_URLCONF="admin_views.urls")
class AdminViewStringPrimaryKeyTest(TestCase): class AdminViewStringPrimaryKeyTest(TestCase):
Expand Down
15 changes: 10 additions & 5 deletions tests/model_meta/test.py
@@ -1,7 +1,7 @@
from django import test from django import test


from django.db.models.fields import related, CharField, Field from django.db.models.fields import related, CharField, Field, FieldDoesNotExist
from django.contrib.contenttypes.fields import GenericForeignKey from django.contrib.contenttypes.fields import GenericRelation


from .models import ( from .models import (
AbstractPerson, BasePerson, Person, Relating, Relation AbstractPerson, BasePerson, Person, Relating, Relation
Expand Down Expand Up @@ -650,7 +650,12 @@ def test_get_related_m2m(self):
self.assertEqual(field_info[1:], (None, False, True)) self.assertEqual(field_info[1:], (None, False, True))
self.assertIsInstance(field_info[0], related.RelatedObject) self.assertIsInstance(field_info[0], related.RelatedObject)


def test_get_virtual_field(self): def test_get_generic_foreign_key(self):
field_info = Person._meta.get_field_by_name('content_object_base') # For historic reasons generic foreign keys aren't available.
with self.assertRaises(FieldDoesNotExist):
Person._meta.get_field_by_name('content_object_base')

def test_get_generic_relation(self):
field_info = Person._meta.get_field_by_name('generic_relation_base')
self.assertEqual(field_info[1:], (None, True, False)) self.assertEqual(field_info[1:], (None, True, False))
self.assertIsInstance(field_info[0], GenericForeignKey) self.assertIsInstance(field_info[0], GenericRelation)

0 comments on commit 9cd5201

Please sign in to comment.