From 65cf82bd08631a7aa8d9dd007b2527476fa3304f Mon Sep 17 00:00:00 2001 From: Rainer Koirikivi Date: Wed, 21 Aug 2013 19:22:22 +0300 Subject: [PATCH] Fixed #20934 -- Avoided NoReverseMatch in ModelAdmin.changelist_view The view tried to display links to a ModelAdmin's change_view, which resulted in NoReverseMatches if get_urls was overridden to remove the corresponding url. --- .../contrib/admin/templatetags/admin_list.py | 42 ++++++++++++------- tests/admin_views/tests.py | 13 ++++++ 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/django/contrib/admin/templatetags/admin_list.py b/django/contrib/admin/templatetags/admin_list.py index 6c3c3e8511..ba109d1454 100644 --- a/django/contrib/admin/templatetags/admin_list.py +++ b/django/contrib/admin/templatetags/admin_list.py @@ -9,6 +9,7 @@ from django.contrib.admin.views.main import (ALL_VAR, EMPTY_CHANGELIST_VALUE, ORDER_VAR, PAGE_VAR, SEARCH_VAR) from django.contrib.admin.templatetags.admin_static import static from django.core.exceptions import ObjectDoesNotExist +from django.core.urlresolvers import NoReverseMatch from django.db import models from django.utils import formats from django.utils.html import escapejs, format_html @@ -216,25 +217,36 @@ def items_for_result(cl, result, form): row_class = mark_safe(' class="%s"' % ' '.join(row_classes)) # If list_display_links not defined, add the link tag to the first field if (first and not cl.list_display_links) or field_name in cl.list_display_links: - table_tag = {True:'th', False:'td'}[first] + table_tag = 'th' if first else 'td' first = False - url = cl.url_for_result(result) - url = add_preserved_filters({'preserved_filters': cl.preserved_filters, 'opts': cl.opts}, url) - # Convert the pk to something that can be used in Javascript. - # Problem cases are long ints (23L) and non-ASCII strings. - if cl.to_field: - attr = str(cl.to_field) + + # Display link to the result's change_view if the url exists, else + # display just the result's representation. + try: + url = cl.url_for_result(result) + except NoReverseMatch: + link_or_text = result_repr else: - attr = pk - value = result.serializable_value(attr) - result_id = escapejs(value) - yield format_html('<{0}{1}>{4}', + url = add_preserved_filters({'preserved_filters': cl.preserved_filters, 'opts': cl.opts}, url) + # Convert the pk to something that can be used in Javascript. + # Problem cases are long ints (23L) and non-ASCII strings. + if cl.to_field: + attr = str(cl.to_field) + else: + attr = pk + value = result.serializable_value(attr) + result_id = escapejs(value) + link_or_text = format_html( + '{2}', + url, + format_html(' onclick="opener.dismissRelatedLookupPopup(window, '{0}'); return false;"', result_id) + if cl.is_popup else '', + result_repr) + + yield format_html('<{0}{1}>{2}', table_tag, row_class, - url, - format_html(' onclick="opener.dismissRelatedLookupPopup(window, '{0}'); return false;"', result_id) - if cl.is_popup else '', - result_repr, + link_or_text, table_tag) else: # By default the fields come from ModelAdmin.list_editable, but if we pull diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index 8f83324a37..66a5735b6c 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -630,6 +630,19 @@ class AdminViewBasicTest(AdminViewBasicTestCase): with self.assertRaises(AttributeError): self.client.get('/test_admin/%s/admin_views/simple/' % self.urlbit) + def test_changelist_with_no_change_url(self): + """ + ModelAdmin.changelist_view shouldn't result in a NoReverseMatch if url + for change_view is removed from get_urls + + Regression test for #20934 + """ + UnchangeableObject.objects.create() + response = self.client.get('/test_admin/admin/admin_views/unchangeableobject/') + self.assertEqual(response.status_code, 200) + # Check the format of the shown object -- shouldn't contain a change link + self.assertContains(response, 'UnchangeableObject object', html=True) + @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',)) class AdminViewFormUrlTest(TestCase):