From 35d1cd0b28d1d9cd7bffbfbc6cc2e02b58404415 Mon Sep 17 00:00:00 2001
From: Julien Phalip <jphalip@gmail.com>
Date: Sat, 22 Dec 2012 20:00:08 +0100
Subject: [PATCH] Fixed #19505 -- A more flexible implementation for
customizable admin redirect urls.
Work by Julien Phalip.
Refs #8001, #18310, #19505. See also 0b908b92a2ca4fb74a103e96bb75c53c05d0a428.
---
django/contrib/admin/options.py | 165 ++++++------------
django/contrib/auth/admin.py | 5 +-
docs/internals/deprecation.txt | 6 +
.../admin_custom_urls/models.py | 63 ++++---
.../admin_custom_urls/tests.py | 79 +++++----
5 files changed, 144 insertions(+), 174 deletions(-)
diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py
index 1827d40159..fa6d288f58 100644
--- a/django/contrib/admin/options.py
+++ b/django/contrib/admin/options.py
@@ -9,7 +9,7 @@ from django.forms.models import (modelform_factory, modelformset_factory,
inlineformset_factory, BaseInlineFormSet)
from django.contrib.contenttypes.models import ContentType
from django.contrib.admin import widgets, helpers
-from django.contrib.admin.util import quote, unquote, flatten_fieldsets, get_deleted_objects, model_format_dict
+from django.contrib.admin.util import unquote, flatten_fieldsets, get_deleted_objects, model_format_dict
from django.contrib.admin.templatetags.admin_static import static
from django.contrib import messages
from django.views.decorators.csrf import csrf_protect
@@ -38,6 +38,7 @@ HORIZONTAL, VERTICAL = 1, 2
# returns the <ul> class for a given radio_admin field
get_ul_class = lambda x: 'radiolist%s' % ((x == HORIZONTAL) and ' inline' or '')
+
class IncorrectLookupParameters(Exception):
pass
@@ -62,6 +63,7 @@ FORMFIELD_FOR_DBFIELD_DEFAULTS = {
csrf_protect_m = method_decorator(csrf_protect)
+
class BaseModelAdmin(six.with_metaclass(forms.MediaDefiningClass)):
"""Functionality common to both ModelAdmin and InlineAdmin."""
@@ -150,7 +152,7 @@ class BaseModelAdmin(six.with_metaclass(forms.MediaDefiningClass)):
})
if 'choices' not in kwargs:
kwargs['choices'] = db_field.get_choices(
- include_blank = db_field.blank,
+ include_blank=db_field.blank,
blank_choice=[('', _('None'))]
)
return db_field.formfield(**kwargs)
@@ -787,49 +789,37 @@ class ModelAdmin(BaseModelAdmin):
"admin/change_form.html"
], context, current_app=self.admin_site.name)
- def response_add(self, request, obj, post_url_continue='../%s/',
- continue_editing_url=None, add_another_url=None,
- hasperm_url=None, noperm_url=None):
+ def response_add(self, request, obj, post_url_continue=None):
"""
Determines the HttpResponse for the add_view stage.
-
- :param request: HttpRequest instance.
- :param obj: Object just added.
- :param post_url_continue: Deprecated/undocumented.
- :param continue_editing_url: URL where user will be redirected after
- pressing 'Save and continue editing'.
- :param add_another_url: URL where user will be redirected after
- pressing 'Save and add another'.
- :param hasperm_url: URL to redirect after a successful object creation
- when the user has change permissions.
- :param noperm_url: URL to redirect after a successful object creation
- when the user has no change permissions.
"""
- if post_url_continue != '../%s/':
- warnings.warn("The undocumented 'post_url_continue' argument to "
- "ModelAdmin.response_add() is deprecated, use the new "
- "*_url arguments instead.", DeprecationWarning,
- stacklevel=2)
opts = obj._meta
- pk_value = obj.pk
- app_label = opts.app_label
- model_name = opts.module_name
- site_name = self.admin_site.name
+ pk_value = obj._get_pk_val()
msg_dict = {'name': force_text(opts.verbose_name), 'obj': force_text(obj)}
-
# Here, we distinguish between different save types by checking for
# the presence of keys in request.POST.
if "_continue" in request.POST:
msg = _('The %(name)s "%(obj)s" was added successfully. You may edit it again below.') % msg_dict
self.message_user(request, msg)
- if continue_editing_url is None:
- continue_editing_url = 'admin:%s_%s_change' % (app_label, model_name)
- url = reverse(continue_editing_url, args=(quote(pk_value),),
- current_app=site_name)
+ if post_url_continue is None:
+ post_url_continue = reverse('admin:%s_%s_change' %
+ (opts.app_label, opts.module_name),
+ args=(pk_value,),
+ current_app=self.admin_site.name)
+ else:
+ try:
+ post_url_continue = post_url_continue % pk_value
+ warnings.warn(
+ "The use of string formats for post_url_continue "
+ "in ModelAdmin.response_add() is deprecated. Provide "
+ "a pre-formatted url instead.",
+ DeprecationWarning, stacklevel=2)
+ except TypeError:
+ pass
if "_popup" in request.POST:
- url += "?_popup=1"
- return HttpResponseRedirect(url)
+ post_url_continue += "?_popup=1"
+ return HttpResponseRedirect(post_url_continue)
if "_popup" in request.POST:
return HttpResponse(
@@ -840,102 +830,61 @@ class ModelAdmin(BaseModelAdmin):
elif "_addanother" in request.POST:
msg = _('The %(name)s "%(obj)s" was added successfully. You may add another %(name)s below.') % msg_dict
self.message_user(request, msg)
- if add_another_url is None:
- add_another_url = 'admin:%s_%s_add' % (app_label, model_name)
- url = reverse(add_another_url, current_app=site_name)
- return HttpResponseRedirect(url)
+ return HttpResponseRedirect(request.path)
else:
msg = _('The %(name)s "%(obj)s" was added successfully.') % msg_dict
self.message_user(request, msg)
+ return self.response_post_save(request, obj)
- # Figure out where to redirect. If the user has change permission,
- # redirect to the change-list page for this object. Otherwise,
- # redirect to the admin index.
- if self.has_change_permission(request, None):
- if hasperm_url is None:
- hasperm_url = 'admin:%s_%s_changelist' % (app_label, model_name)
- url = reverse(hasperm_url, current_app=site_name)
- else:
- if noperm_url is None:
- noperm_url = 'admin:index'
- url = reverse(noperm_url, current_app=site_name)
- return HttpResponseRedirect(url)
-
- def response_change(self, request, obj, continue_editing_url=None,
- save_as_new_url=None, add_another_url=None,
- hasperm_url=None, noperm_url=None):
+ def response_change(self, request, obj):
"""
Determines the HttpResponse for the change_view stage.
-
- :param request: HttpRequest instance.
- :param obj: Object just modified.
- :param continue_editing_url: URL where user will be redirected after
- pressing 'Save and continue editing'.
- :param save_as_new_url: URL where user will be redirected after pressing
- 'Save as new' (when applicable).
- :param add_another_url: URL where user will be redirected after pressing
- 'Save and add another'.
- :param hasperm_url: URL to redirect after a successful object edition when
- the user has change permissions.
- :param noperm_url: URL to redirect after a successful object edition when
- the user has no change permissions.
"""
- opts = obj._meta
+ opts = self.model._meta
- app_label = opts.app_label
- model_name = opts.module_name
- site_name = self.admin_site.name
- verbose_name = opts.verbose_name
- # Handle proxy models automatically created by .only() or .defer().
- # Refs #14529
- if obj._deferred:
- opts_ = opts.proxy_for_model._meta
- verbose_name = opts_.verbose_name
- model_name = opts_.module_name
-
- msg_dict = {'name': force_text(verbose_name), 'obj': force_text(obj)}
+ pk_value = obj._get_pk_val()
+ msg_dict = {'name': force_text(opts.verbose_name), 'obj': force_text(obj)}
if "_continue" in request.POST:
msg = _('The %(name)s "%(obj)s" was changed successfully. You may edit it again below.') % msg_dict
self.message_user(request, msg)
- if continue_editing_url is None:
- continue_editing_url = 'admin:%s_%s_change' % (app_label, model_name)
- url = reverse(continue_editing_url, args=(quote(obj.pk),),
- current_app=site_name)
- if "_popup" in request.POST:
- url += "?_popup=1"
- return HttpResponseRedirect(url)
+ if "_popup" in request.REQUEST:
+ return HttpResponseRedirect(request.path + "?_popup=1")
+ else:
+ return HttpResponseRedirect(request.path)
elif "_saveasnew" in request.POST:
msg = _('The %(name)s "%(obj)s" was added successfully. You may edit it again below.') % msg_dict
self.message_user(request, msg)
- if save_as_new_url is None:
- save_as_new_url = 'admin:%s_%s_change' % (app_label, model_name)
- url = reverse(save_as_new_url, args=(quote(obj.pk),),
- current_app=site_name)
- return HttpResponseRedirect(url)
+ return HttpResponseRedirect(reverse('admin:%s_%s_change' %
+ (opts.app_label, opts.module_name),
+ args=(pk_value,),
+ current_app=self.admin_site.name))
elif "_addanother" in request.POST:
msg = _('The %(name)s "%(obj)s" was changed successfully. You may add another %(name)s below.') % msg_dict
self.message_user(request, msg)
- if add_another_url is None:
- add_another_url = 'admin:%s_%s_add' % (app_label, model_name)
- url = reverse(add_another_url, current_app=site_name)
- return HttpResponseRedirect(url)
+ return HttpResponseRedirect(reverse('admin:%s_%s_add' %
+ (opts.app_label, opts.module_name),
+ current_app=self.admin_site.name))
else:
msg = _('The %(name)s "%(obj)s" was changed successfully.') % msg_dict
self.message_user(request, msg)
- # Figure out where to redirect. If the user has change permission,
- # redirect to the change-list page for this object. Otherwise,
- # redirect to the admin index.
- if self.has_change_permission(request, None):
- if hasperm_url is None:
- hasperm_url = 'admin:%s_%s_changelist' % (app_label,
- model_name)
- url = reverse(hasperm_url, current_app=site_name)
- else:
- if noperm_url is None:
- noperm_url = 'admin:index'
- url = reverse(noperm_url, current_app=site_name)
- return HttpResponseRedirect(url)
+ return self.response_post_save(request, obj)
+
+ def response_post_save(self, request, obj):
+ """
+ Figure out where to redirect after the 'Save' button has been pressed.
+ If the user has change permission, redirect to the change-list page for
+ this object. Otherwise, redirect to the admin index.
+ """
+ opts = self.model._meta
+ if self.has_change_permission(request, None):
+ post_url = reverse('admin:%s_%s_changelist' %
+ (opts.app_label, opts.module_name),
+ current_app=self.admin_site.name)
+ else:
+ post_url = reverse('admin:index',
+ current_app=self.admin_site.name)
+ return HttpResponseRedirect(post_url)
def response_action(self, request, queryset):
"""
diff --git a/django/contrib/auth/admin.py b/django/contrib/auth/admin.py
index d15a387a7e..7b816674d3 100644
--- a/django/contrib/auth/admin.py
+++ b/django/contrib/auth/admin.py
@@ -153,7 +153,7 @@ class UserAdmin(admin.ModelAdmin):
'admin/auth/user/change_password.html',
context, current_app=self.admin_site.name)
- def response_add(self, request, obj, **kwargs):
+ def response_add(self, request, obj, post_url_continue=None):
"""
Determines the HttpResponse for the add_view stage. It mostly defers to
its superclass implementation but is customized because the User model
@@ -166,7 +166,8 @@ class UserAdmin(admin.ModelAdmin):
# * We are adding a user in a popup
if '_addanother' not in request.POST and '_popup' not in request.POST:
request.POST['_continue'] = 1
- return super(UserAdmin, self).response_add(request, obj, **kwargs)
+ return super(UserAdmin, self).response_add(request, obj,
+ post_url_continue)
admin.site.register(Group, GroupAdmin)
admin.site.register(User, UserAdmin)
diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt
index 414da30ff8..386dfc0b00 100644
--- a/docs/internals/deprecation.txt
+++ b/docs/internals/deprecation.txt
@@ -268,6 +268,12 @@ these changes.
* ``django.contrib.markup`` will be removed following an accelerated
deprecation.
+* The value for the ``post_url_continue`` parameter in
+ ``ModelAdmin.response_add()`` will have to be either ``None`` (to redirect
+ to the newly created object's edit page) or a pre-formatted url. String
+ formats, such as the previous default ``'../%s/'``, will not be accepted any
+ more.
+
1.7
---
diff --git a/tests/regressiontests/admin_custom_urls/models.py b/tests/regressiontests/admin_custom_urls/models.py
index b9b3285463..cc8e730c26 100644
--- a/tests/regressiontests/admin_custom_urls/models.py
+++ b/tests/regressiontests/admin_custom_urls/models.py
@@ -1,7 +1,9 @@
from functools import update_wrapper
from django.contrib import admin
+from django.core.urlresolvers import reverse
from django.db import models
+from django.http import HttpResponseRedirect
from django.utils.encoding import python_2_unicode_compatible
@@ -49,41 +51,38 @@ class ActionAdmin(admin.ModelAdmin):
) + self.remove_url(view_name)
-admin.site.register(Action, ActionAdmin)
-
-
class Person(models.Model):
- nick = models.CharField(max_length=20)
-
-
-class PersonAdmin(admin.ModelAdmin):
- """A custom ModelAdmin that customizes the deprecated post_url_continue
- argument to response_add()"""
- def response_add(self, request, obj, post_url_continue='../%s/continue/',
- continue_url=None, add_url=None, hasperm_url=None,
- noperm_url=None):
- return super(PersonAdmin, self).response_add(request, obj,
- post_url_continue,
- continue_url, add_url,
- hasperm_url, noperm_url)
-
-
-admin.site.register(Person, PersonAdmin)
-
-
-class City(models.Model):
name = models.CharField(max_length=20)
+class PersonAdmin(admin.ModelAdmin):
-class CityAdmin(admin.ModelAdmin):
- """A custom ModelAdmin that redirects to the changelist when the user
- presses the 'Save and add another' button when adding a model instance."""
- def response_add(self, request, obj,
- add_another_url='admin:admin_custom_urls_city_changelist',
- **kwargs):
- return super(CityAdmin, self).response_add(request, obj,
- add_another_url=add_another_url,
- **kwargs)
+ def response_post_save(self, request, obj):
+ return HttpResponseRedirect(
+ reverse('admin:admin_custom_urls_person_history', args=[obj.pk]))
-admin.site.register(City, CityAdmin)
+class Car(models.Model):
+ name = models.CharField(max_length=20)
+
+class CarAdmin(admin.ModelAdmin):
+
+ def response_add(self, request, obj, post_url_continue=None):
+ return super(CarAdmin, self).response_add(
+ request, obj, post_url_continue=reverse('admin:admin_custom_urls_car_history', args=[obj.pk]))
+
+
+class CarDeprecated(models.Model):
+ """ This class must be removed in Django 1.6 """
+ name = models.CharField(max_length=20)
+
+class CarDeprecatedAdmin(admin.ModelAdmin):
+ """ This class must be removed in Django 1.6 """
+ def response_add(self, request, obj, post_url_continue=None):
+ return super(CarDeprecatedAdmin, self).response_add(
+ request, obj, post_url_continue='../%s/history/')
+
+
+admin.site.register(Action, ActionAdmin)
+admin.site.register(Person, PersonAdmin)
+admin.site.register(Car, CarAdmin)
+admin.site.register(CarDeprecated, CarDeprecatedAdmin)
\ No newline at end of file
diff --git a/tests/regressiontests/admin_custom_urls/tests.py b/tests/regressiontests/admin_custom_urls/tests.py
index 87c72e2e71..d691a97557 100644
--- a/tests/regressiontests/admin_custom_urls/tests.py
+++ b/tests/regressiontests/admin_custom_urls/tests.py
@@ -1,5 +1,4 @@
from __future__ import absolute_import, unicode_literals
-
import warnings
from django.contrib.admin.util import quote
@@ -8,7 +7,7 @@ from django.template.response import TemplateResponse
from django.test import TestCase
from django.test.utils import override_settings
-from .models import Action, Person, City
+from .models import Action, Person, Car, CarDeprecated
@override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',))
@@ -86,8 +85,8 @@ class AdminCustomUrlsTest(TestCase):
@override_settings(PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',))
-class CustomUrlsWorkflowTests(TestCase):
- fixtures = ['users.json']
+class CustomRedirects(TestCase):
+ fixtures = ['users.json', 'actions.json']
def setUp(self):
self.client.login(username='super', password='secret')
@@ -95,33 +94,49 @@ class CustomUrlsWorkflowTests(TestCase):
def tearDown(self):
self.client.logout()
- def test_old_argument_deprecation(self):
- """Test reporting of post_url_continue deprecation."""
- post_data = {
- 'nick': 'johndoe',
- }
- cnt = Person.objects.count()
- with warnings.catch_warnings(record=True) as w:
- warnings.simplefilter("always")
- response = self.client.post(reverse('admin:admin_custom_urls_person_add'), post_data)
- self.assertEqual(response.status_code, 302)
- self.assertEqual(Person.objects.count(), cnt + 1)
- # We should get a DeprecationWarning
- self.assertEqual(len(w), 1)
- self.assertTrue(isinstance(w[0].message, DeprecationWarning))
+ def test_post_save_redirect(self):
+ """
+ Ensures that ModelAdmin.response_post_save() controls the redirection
+ after the 'Save' button has been pressed.
+ Refs 8001, 18310, 19505.
+ """
+ post_data = { 'name': 'John Doe', }
+ self.assertEqual(Person.objects.count(), 0)
+ response = self.client.post(
+ reverse('admin:admin_custom_urls_person_add'), post_data)
+ persons = Person.objects.all()
+ self.assertEqual(len(persons), 1)
+ self.assertRedirects(
+ response, reverse('admin:admin_custom_urls_person_history', args=[persons[0].pk]))
- def test_custom_add_another_redirect(self):
- """Test customizability of post-object-creation redirect URL."""
- post_data = {
- 'name': 'Rome',
- '_addanother': '1',
- }
- cnt = City.objects.count()
+ def test_post_url_continue(self):
+ """
+ Ensures that the ModelAdmin.response_add()'s parameter `post_url_continue`
+ controls the redirection after an object has been created.
+ """
+ post_data = { 'name': 'SuperFast', '_continue': '1' }
+ self.assertEqual(Car.objects.count(), 0)
+ response = self.client.post(
+ reverse('admin:admin_custom_urls_car_add'), post_data)
+ cars = Car.objects.all()
+ self.assertEqual(len(cars), 1)
+ self.assertRedirects(
+ response, reverse('admin:admin_custom_urls_car_history', args=[cars[0].pk]))
+
+ def test_post_url_continue_string_formats(self):
+ """
+ Ensures that string formats are accepted for post_url_continue. This
+ is a deprecated functionality that will be removed in Django 1.6 along
+ with this test.
+ """
with warnings.catch_warnings(record=True) as w:
- # POST to the view whose post-object-creation redir URL argument we
- # are customizing (object creation)
- response = self.client.post(reverse('admin:admin_custom_urls_city_add'), post_data)
- self.assertEqual(City.objects.count(), cnt + 1)
- # Check that it redirected to the URL we set
- self.assertRedirects(response, reverse('admin:admin_custom_urls_city_changelist'))
- self.assertEqual(len(w), 0) # We should get no DeprecationWarning
+ post_data = { 'name': 'SuperFast', '_continue': '1' }
+ self.assertEqual(Car.objects.count(), 0)
+ response = self.client.post(
+ reverse('admin:admin_custom_urls_cardeprecated_add'), post_data)
+ cars = CarDeprecated.objects.all()
+ self.assertEqual(len(cars), 1)
+ self.assertRedirects(
+ response, reverse('admin:admin_custom_urls_cardeprecated_history', args=[cars[0].pk]))
+ self.assertEqual(len(w), 1)
+ self.assertTrue(isinstance(w[0].message, DeprecationWarning))
\ No newline at end of file