From 1791a7e75af8c9e7ca54425592379fd1f840fe88 Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Sat, 7 Feb 2015 17:55:47 +0100 Subject: [PATCH] Fixed #15779 -- Allowed 'add' primary key in admin edition Thanks Marwan Alsabbagh for the report, and Simon Charette and Tim Graham for the reviews. --- django/contrib/admin/options.py | 7 ++++++- docs/releases/1.9.txt | 8 ++++++++ tests/admin_custom_urls/tests.py | 5 ----- tests/admin_views/admin.py | 2 +- tests/admin_views/tests.py | 18 ++++++++++++++++-- 5 files changed, 31 insertions(+), 9 deletions(-) diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 06d8e417a8c..87c6d0093f4 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -46,6 +46,7 @@ from django.utils.safestring import mark_safe from django.utils.text import capfirst, get_text_list from django.utils.translation import string_concat, ugettext as _, ungettext from django.views.decorators.csrf import csrf_protect +from django.views.generic import RedirectView IS_POPUP_VAR = '_popup' TO_FIELD_VAR = '_to_field' @@ -554,7 +555,11 @@ class ModelAdmin(BaseModelAdmin): url(r'^add/$', wrap(self.add_view), name='%s_%s_add' % info), url(r'^(.+)/history/$', wrap(self.history_view), name='%s_%s_history' % info), url(r'^(.+)/delete/$', wrap(self.delete_view), name='%s_%s_delete' % info), - url(r'^(.+)/$', wrap(self.change_view), name='%s_%s_change' % info), + url(r'^(.+)/change/$', wrap(self.change_view), name='%s_%s_change' % info), + # For backwards compatibility (was the change url before 1.9) + url(r'^(.+)/$', wrap(RedirectView.as_view( + pattern_name='%s:%s_%s_change' % ((self.admin_site.name,) + info) + ))), ] return urlpatterns diff --git a/docs/releases/1.9.txt b/docs/releases/1.9.txt index 9264adf81fe..51bbc917d8f 100644 --- a/docs/releases/1.9.txt +++ b/docs/releases/1.9.txt @@ -35,6 +35,14 @@ Minor features * Admin views now have ``model_admin`` or ``admin_site`` attributes. +* The URL of the admin change view has been changed (was at + ``/admin////`` by default and is now at + ``/admin////change/``). This should not affect your + application unless you have hardcoded admin URLs. In that case, replace those + links by :ref:`reversing admin URLs ` instead. Note that + the old URL still redirects to the new one for backwards compatibility, but + it may be removed in a future version. + :mod:`django.contrib.auth` ^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/admin_custom_urls/tests.py b/tests/admin_custom_urls/tests.py index c2d5ad49946..5ef9f61f67b 100644 --- a/tests/admin_custom_urls/tests.py +++ b/tests/admin_custom_urls/tests.py @@ -61,11 +61,6 @@ class AdminCustomUrlsTest(TestCase): """ # Should get the change_view for model instance with PK 'add', not show # the add_view - response = self.client.get('/admin/admin_custom_urls/action/add/') - self.assertEqual(response.status_code, 200) - self.assertContains(response, 'Change action') - - # Ditto, but use reverse() to build the URL url = reverse('admin:%s_action_change' % Action._meta.app_label, args=(quote('add'),)) response = self.client.get(url) diff --git a/tests/admin_views/admin.py b/tests/admin_views/admin.py index c044a6ac217..aa5a336f4f9 100644 --- a/tests/admin_views/admin.py +++ b/tests/admin_views/admin.py @@ -693,7 +693,7 @@ class UnchangeableObjectAdmin(admin.ModelAdmin): def get_urls(self): # Disable change_view, but leave other urls untouched urlpatterns = super(UnchangeableObjectAdmin, self).get_urls() - return [p for p in urlpatterns if not p.name.endswith("_change")] + return [p for p in urlpatterns if p.name and not p.name.endswith("_change")] def callable_on_unknown(obj): diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index a2c742f3ede..4b662a948e3 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -137,6 +137,15 @@ class AdminViewBasicTest(AdminViewBasicTestCase): response = self.client.get(reverse('admin:admin_views_section_change', args=('abc',))) self.assertEqual(response.status_code, 404) + def test_basic_edit_GET_old_url_redirect(self): + """ + The change URL changed in Django 1.9, but the old one still redirects. + """ + response = self.client.get( + reverse('admin:admin_views_section_change', args=(1,)).replace('change/', '') + ) + self.assertRedirects(response, reverse('admin:admin_views_section_change', args=(1,))) + def test_basic_inheritance_GET_string_PK(self): """ Ensure GET on the change_view works on inherited models (returns an @@ -2022,8 +2031,8 @@ class AdminViewStringPrimaryKeyTest(TestCase): self.assertContains(response, should_contain) def test_url_conflicts_with_add(self): - "A model with a primary key that ends with add should be visible" - add_model = ModelWithStringPrimaryKey(pk="i have something to add") + "A model with a primary key that ends with add or is `add` should be visible" + add_model = ModelWithStringPrimaryKey.objects.create(pk="i have something to add") add_model.save() response = self.client.get( reverse('admin:admin_views_modelwithstringprimarykey_change', args=(quote(add_model.pk),)) @@ -2031,6 +2040,11 @@ class AdminViewStringPrimaryKeyTest(TestCase): should_contain = """

Change model with string primary key

""" self.assertContains(response, should_contain) + add_model2 = ModelWithStringPrimaryKey.objects.create(pk="add") + add_url = reverse('admin:admin_views_modelwithstringprimarykey_add') + change_url = reverse('admin:admin_views_modelwithstringprimarykey_change', args=(quote(add_model2.pk),)) + self.assertNotEqual(add_url, change_url) + def test_url_conflicts_with_delete(self): "A model with a primary key that ends with delete should be visible" delete_model = ModelWithStringPrimaryKey(pk="delete")