diff options
| -rw-r--r-- | django/contrib/admin/templates/admin/change_form.html | 2 | ||||
| -rw-r--r-- | django/contrib/admin/templates/admin/submit_line.html | 6 | ||||
| -rw-r--r-- | django/contrib/admin/templatetags/admin_modify.py | 6 | ||||
| -rw-r--r-- | django/contrib/admin/util.py | 6 | ||||
| -rw-r--r-- | django/contrib/admin/views/main.py | 7 | ||||
| -rw-r--r-- | tests/regressiontests/admin_changelist/tests.py | 10 | ||||
| -rw-r--r-- | tests/regressiontests/admin_custom_urls/fixtures/actions.json | 7 | ||||
| -rw-r--r-- | tests/regressiontests/admin_custom_urls/tests.py | 16 | ||||
| -rw-r--r-- | tests/regressiontests/admin_views/tests.py | 75 |
9 files changed, 79 insertions, 56 deletions
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 %} <ul class="object-tools"> {% block object-tools-items %} - <li><a href="{% url opts|admin_urlname:'history' original.pk %}" class="historylink">{% trans "History" %}</a></li> + <li><a href="{% url opts|admin_urlname:'history' original.pk|admin_urlquote %}" class="historylink">{% trans "History" %}</a></li> {% if has_absolute_url %}<li><a href="{% url 'admin:view_on_site' content_type_id original.pk %}" class="viewsitelink">{% trans "View on site" %}</a></li>{% endif%} {% endblock %} </ul> 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 %} <div class="submit-row"> {% if show_save %}<input type="submit" value="{% trans 'Save' %}" class="default" name="_save" {{ onclick_attrib }}/>{% endif %} -{% if show_delete_link %}<p class="deletelink-box"><a href="delete/" class="deletelink">{% trans "Delete" %}</a></p>{% endif %} +{% if show_delete_link %}<p class="deletelink-box"><a href="{% url opts|admin_urlname:'delete' original.pk|admin_urlquote %}" class="deletelink">{% trans "Delete" %}</a></p>{% endif %} {% if show_save_as_new %}<input type="submit" value="{% trans 'Save as new' %}" name="_saveasnew" {{ onclick_attrib }}/>{%endif%} -{% if show_save_and_add_another %}<input type="submit" value="{% trans 'Save and add another' %}" name="_addanother" {{ onclick_attrib }} />{% endif %} +{% if show_save_and_add_another %}<input type="submit" value="{% trans 'Save and add another' %}" name="_addanother" {{ onclick_attrib }}/>{% endif %} {% if show_save_and_continue %}<input type="submit" value="{% trans 'Save and continue editing' %}" name="_continue" {{ onclick_attrib }}/>{% endif %} </div> 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 = '<tbody><tr class="row1"><th><a href="%d/">name</a></th><td class="nowrap">(None)</td></tr></tbody>' % new_child.id + link = reverse('admin:admin_changelist_child_change', args=(new_child.id,)) + row_html = '<tbody><tr class="row1"><th><a href="%s">name</a></th><td class="nowrap">(None)</td></tr></tbody>' % 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 = '<tbody><tr class="row1"><th><a href="%d/">name</a></th><td class="nowrap">Parent object</td></tr></tbody>' % new_child.id + link = reverse('admin:admin_changelist_child_change', args=(new_child.id,)) + row_html = '<tbody><tr class="row1"><th><a href="%s">name</a></th><td class="nowrap">Parent object</td></tr></tbody>' % 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, '<a href="%s/">%s</a>' % (i, i)) + link = reverse('admin:admin_changelist_child_change', args=(i,)) + self.assertContains(response, '<a href="%s">%s</a>' % (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, '<a href="%s" class="historylink"' % - expected_link) diff --git a/tests/regressiontests/admin_views/tests.py b/tests/regressiontests/admin_views/tests.py index 36fea59f2e..8151c8c854 100644 --- a/tests/regressiontests/admin_views/tests.py +++ b/tests/regressiontests/admin_views/tests.py @@ -260,19 +260,21 @@ class AdminViewBasicTest(TestCase): p1 = Person.objects.create(name="Chris", gender=1, alive=True) p2 = Person.objects.create(name="Chris", gender=2, alive=True) p3 = Person.objects.create(name="Bob", gender=1, alive=True) - link = '<a href="%s/' + link1 = reverse('admin:admin_views_person_change', args=(p1.pk,)) + link2 = reverse('admin:admin_views_person_change', args=(p2.pk,)) + link3 = reverse('admin:admin_views_person_change', args=(p3.pk,)) # Sort by name, gender # This hard-codes the URL because it'll fail if it runs against the # 'admin2' custom admin (which doesn't have the Person model). response = self.client.get('/test_admin/admin/admin_views/person/', {'o': '1.2'}) - self.assertContentBefore(response, link % p3.id, link % p1.id) - self.assertContentBefore(response, link % p1.id, link % p2.id) + self.assertContentBefore(response, link3, link1) + self.assertContentBefore(response, link1, link2) # Sort by gender descending, name response = self.client.get('/test_admin/admin/admin_views/person/', {'o': '-2.1'}) - self.assertContentBefore(response, link % p2.id, link % p3.id) - self.assertContentBefore(response, link % p3.id, link % p1.id) + self.assertContentBefore(response, link2, link3) + self.assertContentBefore(response, link3, link1) def testChangeListSortingPreserveQuerySetOrdering(self): """ @@ -284,37 +286,41 @@ class AdminViewBasicTest(TestCase): p1 = Person.objects.create(name="Amy", gender=1, alive=True, age=80) p2 = Person.objects.create(name="Bob", gender=1, alive=True, age=70) p3 = Person.objects.create(name="Chris", gender=2, alive=False, age=60) - link = '<a href="%s/' + link1 = reverse('admin:admin_views_person_change', args=(p1.pk,)) + link2 = reverse('admin:admin_views_person_change', args=(p2.pk,)) + link3 = reverse('admin:admin_views_person_change', args=(p3.pk,)) # This hard-codes the URL because it'll fail if it runs against the # 'admin2' custom admin (which doesn't have the Person model). response = self.client.get('/test_admin/admin/admin_views/person/', {}) - self.assertContentBefore(response, link % p3.id, link % p2.id) - self.assertContentBefore(response, link % p2.id, link % p1.id) + self.assertContentBefore(response, link3, link2) + self.assertContentBefore(response, link2, link1) def testChangeListSortingModelMeta(self): # Test ordering on Model Meta is respected l1 = Language.objects.create(iso='ur', name='Urdu') l2 = Language.objects.create(iso='ar', name='Arabic') - link = '<a href="%s/' + link1 = reverse('admin:admin_views_language_change', args=(quote(l1.pk),)) + link2 = reverse('admin:admin_views_language_change', args=(quote(l2.pk),)) response = self.client.get('/test_admin/admin/admin_views/language/', {}) - self.assertContentBefore(response, link % l2.pk, link % l1.pk) + self.assertContentBefore(response, link2, link1) # Test we can override with query string response = self.client.get('/test_admin/admin/admin_views/language/', {'o':'-1'}) - self.assertContentBefore(response, link % l1.pk, link % l2.pk) + self.assertContentBefore(response, link1, link2) def testChangeListSortingOverrideModelAdmin(self): # Test ordering on Model Admin is respected, and overrides Model Meta dt = datetime.datetime.now() p1 = Podcast.objects.create(name="A", release_date=dt) p2 = Podcast.objects.create(name="B", release_date=dt - datetime.timedelta(10)) + link1 = reverse('admin:admin_views_podcast_change', args=(p1.pk,)) + link2 = reverse('admin:admin_views_podcast_change', args=(p2.pk,)) - link = '<a href="%s/' response = self.client.get('/test_admin/admin/admin_views/podcast/', {}) - self.assertContentBefore(response, link % p1.pk, link % p2.pk) + self.assertContentBefore(response, link1, link2) def testMultipleSortSameField(self): # Check that we get the columns we expect if we have two columns @@ -322,14 +328,16 @@ class AdminViewBasicTest(TestCase): dt = datetime.datetime.now() p1 = Podcast.objects.create(name="A", release_date=dt) p2 = Podcast.objects.create(name="B", release_date=dt - datetime.timedelta(10)) + link1 = reverse('admin:admin_views_podcast_change', args=(quote(p1.pk),)) + link2 = reverse('admin:admin_views_podcast_change', args=(quote(p2.pk),)) - link = '<a href="%s/' response = self.client.get('/test_admin/admin/admin_views/podcast/', {}) - self.assertContentBefore(response, link % p1.pk, link % p2.pk) + self.assertContentBefore(response, link1, link2) p1 = ComplexSortedPerson.objects.create(name="Bob", age=10) p2 = ComplexSortedPerson.objects.create(name="Amy", age=20) - link = '<a href="%s/' + link1 = reverse('admin:admin_views_complexsortedperson_change', args=(p1.pk,)) + link2 = reverse('admin:admin_views_complexsortedperson_change', args=(p2.pk,)) response = self.client.get('/test_admin/admin/admin_views/complexsortedperson/', {}) # Should have 5 columns (including action checkbox col) @@ -342,7 +350,7 @@ class AdminViewBasicTest(TestCase): self.assertContentBefore(response, 'Name', 'Colored name') # Check sorting - should be by name - self.assertContentBefore(response, link % p2.id, link % p1.id) + self.assertContentBefore(response, link2, link1) def testSortIndicatorsAdminOrder(self): """ @@ -461,10 +469,12 @@ class AdminViewBasicTest(TestCase): for rows corresponding to instances of a model in which a named group has been used in the choices option of a field. """ + link1 = reverse('admin:admin_views_fabric_change', args=(1,), current_app=self.urlbit) + link2 = reverse('admin:admin_views_fabric_change', args=(2,), current_app=self.urlbit) response = self.client.get('/test_admin/%s/admin_views/fabric/' % self.urlbit) fail_msg = "Changelist table isn't showing the right human-readable values set by a model field 'choices' option named group." - self.assertContains(response, '<a href="1/">Horizontal</a>', msg_prefix=fail_msg, html=True) - self.assertContains(response, '<a href="2/">Vertical</a>', msg_prefix=fail_msg, html=True) + self.assertContains(response, '<a href="%s">Horizontal</a>' % link1, msg_prefix=fail_msg, html=True) + self.assertContains(response, '<a href="%s">Vertical</a>' % 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 = """<th><a href="%s/">%s</a></th></tr>""" % (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 = """<th><a href="%s%s/">%s</a></th>""" % (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, '<a href="%s" class="historylink"' % expected_link) + @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',)) class SecureViewTests(TestCase): @@ -2023,12 +2048,14 @@ class AdminViewListEditable(TestCase): """ story1 = OtherStory.objects.create(title='The adventures of Guido', content='Once upon a time in Djangoland...') story2 = OtherStory.objects.create(title='Crouching Tiger, Hidden Python', content='The Python was sneaking into...') + link1 = reverse('admin:admin_views_otherstory_change', args=(story1.pk,)) + link2 = reverse('admin:admin_views_otherstory_change', args=(story2.pk,)) response = self.client.get('/test_admin/admin/admin_views/otherstory/') self.assertContains(response, 'id="id_form-0-id"', 1) # Only one hidden field, in a separate place than the table. self.assertContains(response, 'id="id_form-1-id"', 1) self.assertContains(response, '<div class="hiddenfields">\n<input type="hidden" name="form-0-id" value="%d" id="id_form-0-id" /><input type="hidden" name="form-1-id" value="%d" id="id_form-1-id" />\n</div>' % (story2.id, story1.id), html=True) - self.assertContains(response, '<th><a href="%d/">%d</a></th>' % (story1.id, story1.id), 1) - self.assertContains(response, '<th><a href="%d/">%d</a></th>' % (story2.id, story2.id), 1) + self.assertContains(response, '<th><a href="%s">%d</a></th>' % (link1, story1.id), 1) + self.assertContains(response, '<th><a href="%s">%d</a></th>' % (link2, story2.id), 1) @override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',)) |