Skip to content

Commit a689ba9

Browse files
brosnerbrosner
committed
Fixed #12455 -- corrected an oversight in result_headers not honoring admin_order_field
This finishes some slightly refactoring that went into the admin_list template tags. Thanks to kegan for discovering the oversight. git-svn-id: http://code.djangoproject.com/svn/django/trunk@12157 bcc190cf-cafb-0310-a4f2-bffc1f526a37
1 parent 6e756a3 commit a689ba9

File tree

4 files changed

+138
-31
lines changed

4 files changed

+138
-31
lines changed

‎django/contrib/admin/templatetags/admin_list.py

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -76,17 +76,17 @@ def result_headers(cl):
7676
lookup_opts = cl.lookup_opts
7777

7878
for i, field_name in enumerate(cl.list_display):
79-
attr = None
80-
try:
81-
f = lookup_opts.get_field(field_name)
82-
admin_order_field = None
83-
header = f.verbose_name
84-
except models.FieldDoesNotExist:
85-
header = label_for_field(field_name, cl.model, cl.model_admin)
79+
header, attr = label_for_field(field_name, cl.model,
80+
model_admin = cl.model_admin,
81+
return_attr = True
82+
)
83+
if attr:
8684
# if the field is the action checkbox: no sorting and special class
8785
if field_name == 'action_checkbox':
88-
yield {"text": header,
89-
"class_attrib": mark_safe(' class="action-checkbox-column"')}
86+
yield {
87+
"text": header,
88+
"class_attrib": mark_safe(' class="action-checkbox-column"')
89+
}
9090
continue
9191
header = pretty_name(header)
9292

@@ -98,6 +98,8 @@ def result_headers(cl):
9898

9999
# So this _is_ a sortable non-field. Go to the yield
100100
# after the else clause.
101+
else:
102+
admin_order_field = None
101103

102104
th_classes = []
103105
new_order_type = 'asc'

‎django/contrib/admin/util.py

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -250,32 +250,41 @@ def lookup_field(name, obj, model_admin=None):
250250
value = getattr(obj, f.attname)
251251
return f, attr, value
252252

253-
def label_for_field(name, model, model_admin):
253+
def label_for_field(name, model, model_admin=None, return_attr=False):
254+
attr = None
254255
try:
255-
return model._meta.get_field_by_name(name)[0].verbose_name
256+
label = model._meta.get_field_by_name(name)[0].verbose_name
256257
except models.FieldDoesNotExist:
257258
if name == "__unicode__":
258-
return force_unicode(model._meta.verbose_name)
259-
if name == "__str__":
260-
return smart_str(model._meta.verbose_name)
261-
if callable(name):
262-
attr = name
263-
elif hasattr(model_admin, name):
264-
attr = getattr(model_admin, name)
265-
elif hasattr(model, name):
266-
attr = getattr(model, name)
259+
label = force_unicode(model._meta.verbose_name)
260+
elif name == "__str__":
261+
label = smart_str(model._meta.verbose_name)
267262
else:
268-
raise AttributeError
263+
if callable(name):
264+
attr = name
265+
elif model_admin is not None and hasattr(model_admin, name):
266+
attr = getattr(model_admin, name)
267+
elif hasattr(model, name):
268+
attr = getattr(model, name)
269+
else:
270+
message = "Unable to lookup '%s' on %s" % (name, model._meta.object_name)
271+
if model_admin:
272+
message += " or %s" % (model_admin.__name__,)
273+
raise AttributeError(message)
269274

270-
if hasattr(attr, "short_description"):
271-
return attr.short_description
272-
elif callable(attr):
273-
if attr.__name__ == "<lambda>":
274-
return "--"
275+
if hasattr(attr, "short_description"):
276+
label = attr.short_description
277+
elif callable(attr):
278+
if attr.__name__ == "<lambda>":
279+
label = "--"
280+
else:
281+
label = attr.__name__
275282
else:
276-
return attr.__name__
277-
else:
278-
return name
283+
label = name
284+
if return_attr:
285+
return (label, attr)
286+
else:
287+
return label
279288

280289

281290
def display_for_field(value, field):

‎tests/regressiontests/admin_util/models.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,18 @@
1-
# needed for tests
1+
from django.db import models
2+
3+
4+
5+
class Article(models.Model):
6+
"""
7+
A simple Article model for testing
8+
"""
9+
title = models.CharField(max_length=100)
10+
title2 = models.CharField(max_length=100, verbose_name="another name")
11+
created = models.DateTimeField()
12+
13+
def test_from_model(self):
14+
return "nothing"
15+
16+
def test_from_model_with_override(self):
17+
return "nothing"
18+
test_from_model_with_override.short_description = "not what you expect"

‎tests/regressiontests/admin_util/tests.py

Lines changed: 80 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,15 @@
33
from django.db import models
44

55
from django.contrib import admin
6-
from django.contrib.admin.util import display_for_field
6+
from django.contrib.admin.util import display_for_field, label_for_field
77
from django.contrib.admin.views.main import EMPTY_CHANGELIST_VALUE
88

9+
from models import Article
10+
911

1012

1113
class UtilTests(unittest.TestCase):
14+
1215
def test_null_display_for_field(self):
1316
"""
1417
Regression test for #12550: display_for_field should handle None
@@ -38,3 +41,79 @@ def test_null_display_for_field(self):
3841

3942
display_value = display_for_field(None, models.FloatField())
4043
self.assertEqual(display_value, EMPTY_CHANGELIST_VALUE)
44+
45+
def test_label_for_field(self):
46+
"""
47+
Tests for label_for_field
48+
"""
49+
self.assertEquals(
50+
label_for_field("title", Article),
51+
"title"
52+
)
53+
self.assertEquals(
54+
label_for_field("title2", Article),
55+
"another name"
56+
)
57+
self.assertEquals(
58+
label_for_field("title2", Article, return_attr=True),
59+
("another name", None)
60+
)
61+
62+
self.assertEquals(
63+
label_for_field("__unicode__", Article),
64+
"article"
65+
)
66+
self.assertEquals(
67+
label_for_field("__str__", Article),
68+
"article"
69+
)
70+
71+
self.assertRaises(
72+
AttributeError,
73+
lambda: label_for_field("unknown", Article)
74+
)
75+
76+
def test_callable(obj):
77+
return "nothing"
78+
self.assertEquals(
79+
label_for_field(test_callable, Article),
80+
"test_callable"
81+
)
82+
self.assertEquals(
83+
label_for_field(test_callable, Article, return_attr=True),
84+
("test_callable", test_callable)
85+
)
86+
87+
self.assertEquals(
88+
label_for_field("test_from_model", Article),
89+
"test_from_model"
90+
)
91+
self.assertEquals(
92+
label_for_field("test_from_model", Article, return_attr=True),
93+
("test_from_model", Article.test_from_model)
94+
)
95+
self.assertEquals(
96+
label_for_field("test_from_model_with_override", Article),
97+
"not what you expect"
98+
)
99+
100+
self.assertEquals(
101+
label_for_field(lambda x: "nothing", Article),
102+
"--"
103+
)
104+
105+
class MockModelAdmin(object):
106+
def test_from_model(self, obj):
107+
return "nothing"
108+
test_from_model.short_description = "not really the model"
109+
self.assertEquals(
110+
label_for_field("test_from_model", Article, model_admin=MockModelAdmin),
111+
"not really the model"
112+
)
113+
self.assertEquals(
114+
label_for_field("test_from_model", Article,
115+
model_admin = MockModelAdmin,
116+
return_attr = True
117+
),
118+
("not really the model", MockModelAdmin.test_from_model)
119+
)

0 commit comments

Comments
 (0)