diff options
| -rw-r--r-- | django/contrib/admin/utils.py | 18 | ||||
| -rw-r--r-- | django/contrib/admin/views/main.py | 3 | ||||
| -rw-r--r-- | docs/releases/1.11.txt | 4 | ||||
| -rw-r--r-- | tests/admin_utils/tests.py | 1 | ||||
| -rw-r--r-- | 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 15dca40dc4..3a9a28fd3f 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. <FK>_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 <FK>_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 081488efb5..cf392e418f 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): + # <FK>_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 c94935f93a..e8cf5ae0f3 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 ecb084adbe..2e72f5a05d 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 6455b210b2..e706ba7105 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 <FK>_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'). |