Fixing #26524 -- Made a foreign key id reference in ModelAdmin.list_display display the id.

This commit is contained in:
krishbharadwaj 2016-04-23 23:05:18 +05:30 committed by Tim Graham
parent c8d2120b06
commit f6681393d3
5 changed files with 36 additions and 2 deletions

View File

@ -19,6 +19,11 @@ from django.utils.text import capfirst
from django.utils.translation import ungettext 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): def lookup_needs_distinct(opts, lookup_path):
""" """
Returns True if 'distinct()' should be used to query the given 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 opts = obj._meta
try: try:
f = _get_non_gfk_field(opts, name) f = _get_non_gfk_field(opts, name)
except FieldDoesNotExist: except (FieldDoesNotExist, FieldIsAForeignKeyColumnName):
# For non-field values, the value is either a method, property or # For non-field values, the value is either a method, property or
# returned via a callable. # returned via a callable.
if callable(name): if callable(name):
@ -310,6 +315,11 @@ def _get_non_gfk_field(opts, name):
# Generic foreign keys OR reverse relations # Generic foreign keys OR reverse relations
((field.many_to_one and not field.related_model) or field.one_to_many)): ((field.many_to_one and not field.related_model) or field.one_to_many)):
raise FieldDoesNotExist() 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 return field
@ -362,6 +372,10 @@ def label_for_field(name, model, model_admin=None, return_attr=False):
label = pretty_name(attr.__name__) label = pretty_name(attr.__name__)
else: else:
label = pretty_name(name) label = pretty_name(name)
except FieldIsAForeignKeyColumnName:
label = pretty_name(name)
attr = name
if return_attr: if return_attr:
return (label, attr) return (label, attr)
else: else:
@ -372,7 +386,7 @@ def help_text_for_field(name, model):
help_text = "" help_text = ""
try: try:
field = _get_non_gfk_field(model._meta, name) field = _get_non_gfk_field(model._meta, name)
except FieldDoesNotExist: except (FieldDoesNotExist, FieldIsAForeignKeyColumnName):
pass pass
else: else:
if hasattr(field, 'help_text'): if hasattr(field, 'help_text'):

View File

@ -371,6 +371,9 @@ class ChangeList(object):
pass pass
else: else:
if isinstance(field.remote_field, models.ManyToOneRel): 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 True
return False return False

View File

@ -253,6 +253,10 @@ Miscellaneous
* CSRF failures are logged to the ``django.security.csrf ``` logger instead of * CSRF failures are logged to the ``django.security.csrf ``` logger instead of
``django.request``. ``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: .. _deprecated-features-1.11:
Features deprecated in 1.11 Features deprecated in 1.11

View File

@ -259,6 +259,7 @@ class UtilsTests(SimpleTestCase):
label_for_field(lambda x: "nothing", Article), label_for_field(lambda x: "nothing", Article),
"--" "--"
) )
self.assertEqual(label_for_field('site_id', Article), 'Site id')
class MockModelAdmin(object): class MockModelAdmin(object):
def test_from_model(self, obj): def test_from_model(self, obj):

View File

@ -538,6 +538,18 @@ class AdminViewBasicTest(AdminViewBasicTestCase):
self.assertContentBefore(response, 'The First Item', 'The Middle Item') self.assertContentBefore(response, 'The First Item', 'The Middle Item')
self.assertContentBefore(response, 'The Middle Item', 'The Last 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): def test_limited_filter(self):
"""Ensure admin changelist filters do not contain objects excluded via limit_choices_to. """Ensure admin changelist filters do not contain objects excluded via limit_choices_to.
This also tests relation-spanning filters (e.g. 'color__value'). This also tests relation-spanning filters (e.g. 'color__value').