diff --git a/django/contrib/admin/templates/admin/change_form.html b/django/contrib/admin/templates/admin/change_form.html index e27875cdad..4962e732a2 100644 --- a/django/contrib/admin/templates/admin/change_form.html +++ b/django/contrib/admin/templates/admin/change_form.html @@ -29,7 +29,7 @@ {% if change %}{% if not is_popup %} diff --git a/django/contrib/admin/templates/admin/submit_line.html b/django/contrib/admin/templates/admin/submit_line.html index d6f854a233..8c9d22752d 100644 --- a/django/contrib/admin/templates/admin/submit_line.html +++ b/django/contrib/admin/templates/admin/submit_line.html @@ -1,8 +1,8 @@ -{% load i18n %} +{% load i18n admin_urls %}
{% if show_save %}{% endif %} -{% if show_delete_link %}{% endif %} +{% if show_delete_link %}{% endif %} {% if show_save_as_new %}{%endif%} -{% if show_save_and_add_another %}{% endif %} +{% if show_save_and_add_another %}{% endif %} {% if show_save_and_continue %}{% endif %}
diff --git a/django/contrib/admin/templatetags/admin_modify.py b/django/contrib/admin/templatetags/admin_modify.py index c190533f95..f6ac59635a 100644 --- a/django/contrib/admin/templatetags/admin_modify.py +++ b/django/contrib/admin/templatetags/admin_modify.py @@ -28,7 +28,8 @@ def submit_row(context): change = context['change'] is_popup = context['is_popup'] save_as = context['save_as'] - return { + ctx = { + 'opts': opts, 'onclick_attrib': (opts.get_ordered_objects() and change and 'onclick="submitOrderForm();"' or ''), 'show_delete_link': (not is_popup and context['has_delete_permission'] @@ -40,6 +41,9 @@ def submit_row(context): 'is_popup': is_popup, 'show_save': True } + if context.get('original') is not None: + ctx['original'] = context['original'] + return ctx @register.filter def cell_count(inline_admin_form): diff --git a/django/contrib/admin/util.py b/django/contrib/admin/util.py index f95fe53de1..74eef2e733 100644 --- a/django/contrib/admin/util.py +++ b/django/contrib/admin/util.py @@ -48,9 +48,9 @@ def prepare_lookup_value(key, value): def quote(s): """ Ensure that primary key values do not confuse the admin URLs by escaping - any '/', '_' and ':' characters. Similar to urllib.quote, except that the - quoting is slightly different so that it doesn't get automatically - unquoted by the Web browser. + any '/', '_' and ':' and similarly problematic characters. + Similar to urllib.quote, except that the quoting is slightly different so + that it doesn't get automatically unquoted by the Web browser. """ if not isinstance(s, six.string_types): return s diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index 74ef095b4b..5033ba98bc 100644 --- a/django/contrib/admin/views/main.py +++ b/django/contrib/admin/views/main.py @@ -3,6 +3,7 @@ from functools import reduce from django.core.exceptions import SuspiciousOperation, ImproperlyConfigured from django.core.paginator import InvalidPage +from django.core.urlresolvers import reverse from django.db import models from django.db.models.fields import FieldDoesNotExist from django.utils.datastructures import SortedDict @@ -376,4 +377,8 @@ class ChangeList(object): return qs def url_for_result(self, result): - return "%s/" % quote(getattr(result, self.pk_attname)) + pk = getattr(result, self.pk_attname) + return reverse('admin:%s_%s_change' % (self.opts.app_label, + self.opts.module_name), + args=(quote(pk),), + current_app=self.model_admin.admin_site.name) diff --git a/tests/regressiontests/admin_changelist/tests.py b/tests/regressiontests/admin_changelist/tests.py index be88c9a161..2b1c1a9bcf 100644 --- a/tests/regressiontests/admin_changelist/tests.py +++ b/tests/regressiontests/admin_changelist/tests.py @@ -6,6 +6,7 @@ from django.contrib import admin from django.contrib.admin.options import IncorrectLookupParameters from django.contrib.admin.views.main import ChangeList, SEARCH_VAR, ALL_VAR from django.contrib.auth.models import User +from django.core.urlresolvers import reverse from django.template import Context, Template from django.test import TestCase from django.test.client import RequestFactory @@ -65,7 +66,8 @@ class ChangeListTests(TestCase): template = Template('{% load admin_list %}{% spaceless %}{% result_list cl %}{% endspaceless %}') context = Context({'cl': cl}) table_output = template.render(context) - row_html = 'name(None)' % new_child.id + link = reverse('admin:admin_changelist_child_change', args=(new_child.id,)) + row_html = 'name(None)' % link self.assertFalse(table_output.find(row_html) == -1, 'Failed to find expected row element: %s' % table_output) @@ -87,7 +89,8 @@ class ChangeListTests(TestCase): template = Template('{% load admin_list %}{% spaceless %}{% result_list cl %}{% endspaceless %}') context = Context({'cl': cl}) table_output = template.render(context) - row_html = 'nameParent object' % new_child.id + link = reverse('admin:admin_changelist_child_change', args=(new_child.id,)) + row_html = 'nameParent object' % link self.assertFalse(table_output.find(row_html) == -1, 'Failed to find expected row element: %s' % table_output) @@ -425,7 +428,8 @@ class ChangeListTests(TestCase): request = self._mocked_authenticated_request('/child/', superuser) response = m.changelist_view(request) for i in range(1, 10): - self.assertContains(response, '%s' % (i, i)) + link = reverse('admin:admin_changelist_child_change', args=(i,)) + self.assertContains(response, '%s' % (link, i)) list_display = m.get_list_display(request) list_display_links = m.get_list_display_links(request, list_display) diff --git a/tests/regressiontests/admin_custom_urls/fixtures/actions.json b/tests/regressiontests/admin_custom_urls/fixtures/actions.json index a63cf8135c..7c6341d71d 100644 --- a/tests/regressiontests/admin_custom_urls/fixtures/actions.json +++ b/tests/regressiontests/admin_custom_urls/fixtures/actions.json @@ -40,12 +40,5 @@ "fields": { "description": "An action with a name suspected of being a XSS attempt" } - }, - { - "pk": "The name of an action", - "model": "admin_custom_urls.action", - "fields": { - "description": "A generic action" - } } ] diff --git a/tests/regressiontests/admin_custom_urls/tests.py b/tests/regressiontests/admin_custom_urls/tests.py index 64ff9f6692..3e9cf28965 100644 --- a/tests/regressiontests/admin_custom_urls/tests.py +++ b/tests/regressiontests/admin_custom_urls/tests.py @@ -1,5 +1,6 @@ from __future__ import absolute_import, unicode_literals +from django.contrib.admin.util import quote from django.core.urlresolvers import reverse from django.template.response import TemplateResponse from django.test import TestCase @@ -67,7 +68,7 @@ class AdminCustomUrlsTest(TestCase): # Ditto, but use reverse() to build the URL url = reverse('admin:%s_action_change' % Action._meta.app_label, - args=('add',)) + args=(quote('add'),)) response = self.client.get(url) self.assertEqual(response.status_code, 200) self.assertContains(response, 'Change action') @@ -75,19 +76,8 @@ class AdminCustomUrlsTest(TestCase): # Should correctly get the change_view for the model instance with the # funny-looking PK (the one wth a 'path/to/html/document.html' value) url = reverse('admin:%s_action_change' % Action._meta.app_label, - args=("path/to/html/document.html",)) + args=(quote("path/to/html/document.html"),)) response = self.client.get(url) self.assertEqual(response.status_code, 200) self.assertContains(response, 'Change action') self.assertContains(response, 'value="path/to/html/document.html"') - - def testChangeViewHistoryButton(self): - url = reverse('admin:%s_action_change' % Action._meta.app_label, - args=('The name of an action',)) - response = self.client.get(url) - self.assertEqual(response.status_code, 200) - expected_link = reverse('admin:%s_action_history' % - Action._meta.app_label, - args=('The name of an action',)) - self.assertContains(response, 'Horizontal', msg_prefix=fail_msg, html=True) - self.assertContains(response, 'Vertical', msg_prefix=fail_msg, html=True) + self.assertContains(response, 'Horizontal' % link1, msg_prefix=fail_msg, html=True) + self.assertContains(response, 'Vertical' % link2, msg_prefix=fail_msg, html=True) def testNamedGroupFieldChoicesFilter(self): """ @@ -1371,9 +1381,12 @@ class AdminViewStringPrimaryKeyTest(TestCase): self.assertEqual(response.status_code, 200) def test_changelist_to_changeform_link(self): - "The link from the changelist referring to the changeform of the object should be quoted" - response = self.client.get('/test_admin/admin/admin_views/modelwithstringprimarykey/') - should_contain = """%s""" % (escape(quote(self.pk)), escape(self.pk)) + "Link to the changeform of the object in changelist should use reverse() and be quoted -- #18072" + prefix = '/test_admin/admin/admin_views/modelwithstringprimarykey/' + response = self.client.get(prefix) + # this URL now comes through reverse(), thus iri_to_uri encoding + pk_final_url = escape(iri_to_uri(quote(self.pk))) + should_contain = """%s""" % (prefix, pk_final_url, escape(self.pk)) self.assertContains(response, should_contain) def test_recentactions_link(self): @@ -1441,6 +1454,18 @@ class AdminViewStringPrimaryKeyTest(TestCase): should_contain = '/%s/" class="viewsitelink">' % model.pk self.assertContains(response, should_contain) + def test_change_view_history_link(self): + """Object history button link should work and contain the pk value quoted.""" + url = reverse('admin:%s_modelwithstringprimarykey_change' % + ModelWithStringPrimaryKey._meta.app_label, + args=(quote(self.pk),)) + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + expected_link = reverse('admin:%s_modelwithstringprimarykey_history' % + ModelWithStringPrimaryKey._meta.app_label, + args=(quote(self.pk),)) + self.assertContains(response, '%d' % (story1.id, story1.id), 1) - self.assertContains(response, '%d' % (story2.id, story2.id), 1) + self.assertContains(response, '%d' % (link1, story1.id), 1) + self.assertContains(response, '%d' % (link2, story2.id), 1) @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',))