From f6681393d3f53a67b4e0645e8d02f95579d8ae2d Mon Sep 17 00:00:00 2001 From: krishbharadwaj Date: Sat, 23 Apr 2016 23:05:18 +0530 Subject: [PATCH] Fixing #26524 -- Made a foreign key id reference in ModelAdmin.list_display display the id. --- django/contrib/admin/utils.py | 18 ++++++++++++++++-- django/contrib/admin/views/main.py | 3 +++ docs/releases/1.11.txt | 4 ++++ tests/admin_utils/tests.py | 1 + tests/admin_views/tests.py | 12 ++++++++++++ 5 files changed, 36 insertions(+), 2 deletions(-) diff --git a/django/contrib/admin/utils.py b/django/contrib/admin/utils.py index 15dca40dc4a..3a9a28fd3f2 100644 --- a/django/contrib/admin/utils.py +++ b/django/contrib/admin/utils.py @@ -19,6 +19,11 @@ from django.utils.text import capfirst from django.utils.translation import ungettext +class FieldIsAForeignKeyColumnName(Exception): + """A field is a foreign key attname, i.e. _id.""" + pass + + def lookup_needs_distinct(opts, lookup_path): """ Returns True if 'distinct()' should be used to query the given lookup path. @@ -272,7 +277,7 @@ def lookup_field(name, obj, model_admin=None): opts = obj._meta try: f = _get_non_gfk_field(opts, name) - except FieldDoesNotExist: + except (FieldDoesNotExist, FieldIsAForeignKeyColumnName): # For non-field values, the value is either a method, property or # returned via a callable. if callable(name): @@ -310,6 +315,11 @@ def _get_non_gfk_field(opts, name): # Generic foreign keys OR reverse relations ((field.many_to_one and not field.related_model) or field.one_to_many)): raise FieldDoesNotExist() + + # Avoid coercing _id fields to FK + if field.is_relation and hasattr(field, 'attname') and field.attname == name: + raise FieldIsAForeignKeyColumnName() + return field @@ -362,6 +372,10 @@ def label_for_field(name, model, model_admin=None, return_attr=False): label = pretty_name(attr.__name__) else: label = pretty_name(name) + except FieldIsAForeignKeyColumnName: + label = pretty_name(name) + attr = name + if return_attr: return (label, attr) else: @@ -372,7 +386,7 @@ def help_text_for_field(name, model): help_text = "" try: field = _get_non_gfk_field(model._meta, name) - except FieldDoesNotExist: + except (FieldDoesNotExist, FieldIsAForeignKeyColumnName): pass else: if hasattr(field, 'help_text'): diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index 081488efb5d..cf392e418f9 100644 --- a/django/contrib/admin/views/main.py +++ b/django/contrib/admin/views/main.py @@ -371,6 +371,9 @@ class ChangeList(object): pass else: if isinstance(field.remote_field, models.ManyToOneRel): + # _id field names don't require a join. + if field_name == field.get_attname(): + continue return True return False diff --git a/docs/releases/1.11.txt b/docs/releases/1.11.txt index c94935f93ae..e8cf5ae0f34 100644 --- a/docs/releases/1.11.txt +++ b/docs/releases/1.11.txt @@ -253,6 +253,10 @@ Miscellaneous * CSRF failures are logged to the ``django.security.csrf ``` logger instead of ``django.request``. +* Using a foreign key's id (e.g. ``'field_id'``) in ``ModelAdmin.list_display`` + displays the related object's ID instead of ``repr(object)``. Remove the + ``_id`` suffix if you want the ``repr()``. + .. _deprecated-features-1.11: Features deprecated in 1.11 diff --git a/tests/admin_utils/tests.py b/tests/admin_utils/tests.py index ecb084adbec..2e72f5a05df 100644 --- a/tests/admin_utils/tests.py +++ b/tests/admin_utils/tests.py @@ -259,6 +259,7 @@ class UtilsTests(SimpleTestCase): label_for_field(lambda x: "nothing", Article), "--" ) + self.assertEqual(label_for_field('site_id', Article), 'Site id') class MockModelAdmin(object): def test_from_model(self, obj): diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index 6455b210b28..e706ba71052 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -538,6 +538,18 @@ class AdminViewBasicTest(AdminViewBasicTestCase): self.assertContentBefore(response, 'The First Item', 'The Middle Item') self.assertContentBefore(response, 'The Middle Item', 'The Last Item') + def test_has_related_field_in_list_display(self): + """Joins shouldn't be performed for _id fields in list display.""" + state = State.objects.create(name='Karnataka') + City.objects.create(state=state, name='Bangalore') + response = self.client.get(reverse('admin:admin_views_city_changelist'), {}) + + response.context['cl'].list_display = ['id', 'name', 'state'] + self.assertEqual(response.context['cl'].has_related_field_in_list_display(), True) + + response.context['cl'].list_display = ['id', 'name', 'state_id'] + self.assertEqual(response.context['cl'].has_related_field_in_list_display(), False) + def test_limited_filter(self): """Ensure admin changelist filters do not contain objects excluded via limit_choices_to. This also tests relation-spanning filters (e.g. 'color__value').