From 9cd5201abd24ba2632753029ee1957947cdc32f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anssi=20K=C3=A4=C3=A4ri=C3=A4inen?= Date: Mon, 14 Jul 2014 14:23:34 +0300 Subject: [PATCH] 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. --- django/db/models/options.py | 4 +++- tests/admin_views/admin.py | 7 ++++++- tests/admin_views/tests.py | 17 ++++++++++++++++- tests/model_meta/test.py | 15 ++++++++++----- 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/django/db/models/options.py b/django/db/models/options.py index 39f0ab6f19..22fa8faf10 100644 --- a/django/db/models/options.py +++ b/django/db/models/options.py @@ -441,7 +441,9 @@ class Options(object): for f, model in self.get_fields_with_model(): cache[f.name] = cache[f.attname] = (f, model, True, False) 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: self._name_map = cache return cache diff --git a/tests/admin_views/admin.py b/tests/admin_views/admin.py index 22ff4536fa..e00cc69f52 100644 --- a/tests/admin_views/admin.py +++ b/tests/admin_views/admin.py @@ -35,7 +35,7 @@ from .models import (Article, Chapter, Child, Parent, Picture, Widget, UnchangeableObject, UserMessenger, Simple, Choice, ShortMessage, Telegram, FilteredManager, EmptyModelHidden, EmptyModelVisible, EmptyModelMixin, State, City, Restaurant, Worker, ParentWithDependentChildren, - DependentChild, StumpJoke, FieldOverridePost) + DependentChild, StumpJoke, FieldOverridePost, FunkyTag) def callable_year(dt_value): @@ -827,6 +827,10 @@ class RestaurantAdmin(admin.ModelAdmin): return {'name': 'overridden_value'} +class FunkyTagAdmin(admin.ModelAdmin): + list_display = ('name', 'content_object') + + site = admin.AdminSite(name="admin") site.register(Article, ArticleAdmin) site.register(CustomArticle, CustomArticleAdmin) @@ -882,6 +886,7 @@ site.register(State, StateAdmin) site.register(City, CityAdmin) site.register(Restaurant, RestaurantAdmin) site.register(Worker, WorkerAdmin) +site.register(FunkyTag, FunkyTagAdmin) # We intentionally register Promo and ChapterXtra1 but not Chapter nor ChapterXtra2. # That way we cover all four cases: diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index b2c554da0f..6e5f2cb085 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -1718,11 +1718,26 @@ class AdminViewDeletedObjectsTest(TestCase): """ plot = Plot.objects.get(pk=3) FunkyTag.objects.create(content_object=plot, name='hott') - should_contain = """
  • Funky tag: hott""" + should_contain = """
  • Funky tag: hott""" response = self.client.get('/test_admin/admin/admin_views/plot/%s/delete/' % quote(3)) 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" % plot) + + @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',), ROOT_URLCONF="admin_views.urls") class AdminViewStringPrimaryKeyTest(TestCase): diff --git a/tests/model_meta/test.py b/tests/model_meta/test.py index 89a5575665..8fd0a6bb33 100644 --- a/tests/model_meta/test.py +++ b/tests/model_meta/test.py @@ -1,7 +1,7 @@ from django import test -from django.db.models.fields import related, CharField, Field -from django.contrib.contenttypes.fields import GenericForeignKey +from django.db.models.fields import related, CharField, Field, FieldDoesNotExist +from django.contrib.contenttypes.fields import GenericRelation from .models import ( AbstractPerson, BasePerson, Person, Relating, Relation @@ -650,7 +650,12 @@ class GetFieldByNameTests(OptionsBaseTests): self.assertEqual(field_info[1:], (None, False, True)) self.assertIsInstance(field_info[0], related.RelatedObject) - def test_get_virtual_field(self): - field_info = Person._meta.get_field_by_name('content_object_base') + def test_get_generic_foreign_key(self): + # 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.assertIsInstance(field_info[0], GenericForeignKey) + self.assertIsInstance(field_info[0], GenericRelation)