From f48e2258a96a08dcec843921206bcf7656e3ae45 Mon Sep 17 00:00:00 2001 From: Claude Paroz Date: Mon, 12 Jan 2015 12:00:27 +0100 Subject: [PATCH] Fixed #24133 -- Replaced formatting syntax in success_url placeholders Thanks Laurent Payot for the report, and Markus Holtermann, Tim Graham for the reviews. --- django/views/generic/edit.py | 21 +++++++++- docs/internals/deprecation.txt | 3 ++ docs/ref/class-based-views/mixins-editing.txt | 16 ++++++- docs/releases/1.8.txt | 12 ++++++ tests/generic_views/test_edit.py | 42 +++++++++++++++---- tests/generic_views/urls.py | 13 +++++- 6 files changed, 94 insertions(+), 13 deletions(-) diff --git a/django/views/generic/edit.py b/django/views/generic/edit.py index e88dd82958..ef70501bc2 100644 --- a/django/views/generic/edit.py +++ b/django/views/generic/edit.py @@ -1,4 +1,5 @@ import inspect +import re import warnings from django.core.exceptions import ImproperlyConfigured @@ -162,7 +163,15 @@ class ModelFormMixin(FormMixin, SingleObjectMixin): Returns the supplied URL. """ if self.success_url: - url = self.success_url % self.object.__dict__ + if re.search(r'%\([^\)]+\)', self.success_url): + warnings.warn( + "%()s placeholder style in success_url is deprecated. " + "Please replace them by the {} Python format syntax.", + RemovedInDjango20Warning, stacklevel=2 + ) + url = self.success_url % self.object.__dict__ + else: + url = self.success_url.format(**self.object.__dict__) else: try: url = self.object.get_absolute_url() @@ -288,7 +297,15 @@ class DeletionMixin(object): def get_success_url(self): if self.success_url: - return self.success_url % self.object.__dict__ + if re.search(r'%\([^\)]+\)', self.success_url): + warnings.warn( + "%()s placeholder style in success_url is deprecated. " + "Please replace them by the {} Python format syntax.", + RemovedInDjango20Warning, stacklevel=2 + ) + return self.success_url % self.object.__dict__ + else: + return self.success_url.format(**self.object.__dict__) else: raise ImproperlyConfigured( "No URL to redirect to. Provide a success_url.") diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 4e03378f06..0a460b7739 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -158,6 +158,9 @@ details on these changes. and ``Storage.save()`` to be defined without a ``max_length`` argument will be removed. +* Support for the legacy ``%()s`` syntax in ``ModelFormMixin.success_url`` + will be removed. + .. _deprecation-removed-in-1.9: 1.9 diff --git a/docs/ref/class-based-views/mixins-editing.txt b/docs/ref/class-based-views/mixins-editing.txt index 3dcf28dae3..0bfbcbd3f7 100644 --- a/docs/ref/class-based-views/mixins-editing.txt +++ b/docs/ref/class-based-views/mixins-editing.txt @@ -160,9 +160,15 @@ ModelFormMixin ``success_url`` may contain dictionary string formatting, which will be interpolated against the object's field attributes. For - example, you could use ``success_url="/polls/%(slug)s/"`` to + example, you could use ``success_url="/polls/{slug}/"`` to redirect to a URL composed out of the ``slug`` field on a model. + .. versionchanged:: 1.8 + + Support for the new brace-based Python formatting syntax has been + added. The old ``%(slug)s`` placeholder syntax support has been + deprecated and will be removed in Django 2.0. + .. method:: get_form_class() Retrieve the form class to instantiate. If @@ -248,9 +254,15 @@ DeletionMixin ``success_url`` may contain dictionary string formatting, which will be interpolated against the object's field attributes. For example, you - could use ``success_url="/parent/%(parent_id)s/"`` to redirect to a URL + could use ``success_url="/parent/{parent_id}/"`` to redirect to a URL composed out of the ``parent_id`` field on a model. + .. versionchanged:: 1.8 + + Support for the new brace-based Python formatting syntax has been + added. The old ``%(slug)s`` placeholder syntax support has been + deprecated and will be removed in Django 2.0. + .. method:: get_success_url() Returns the url to redirect to when the nominated object has been diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt index a07a209347..cd9a5de682 100644 --- a/docs/releases/1.8.txt +++ b/docs/releases/1.8.txt @@ -388,6 +388,11 @@ Generic Views require a ``form_class`` to be provided anymore. If not provided ``form_class`` defaults to :meth:`~django.views.generic.edit.FormMixin.get_form_class()`. +* Placeholders in :attr:`ModelFormMixin.success_url + ` now support the Python + :py:meth:`str.format()` syntax. The legacy ``%()s`` syntax is still + supported but will be removed in Django 2.0. + Internationalization ^^^^^^^^^^^^^^^^^^^^ @@ -1550,6 +1555,13 @@ will be removed in Django 2.0. Using a single equals sign with the ``{% if %}`` template tag for equality testing was undocumented and untested. It's now deprecated in favor of ``==``. +``%()s`` syntax in ``ModelFormMixin.success_url`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The legacy ``%()s`` syntax in :attr:`ModelFormMixin.success_url + ` is deprecated and +will be removed in Django 2.0. + .. removed-features-1.8: Features removed in 1.8 diff --git a/tests/generic_views/test_edit.py b/tests/generic_views/test_edit.py index d8e0193574..b61a11086a 100644 --- a/tests/generic_views/test_edit.py +++ b/tests/generic_views/test_edit.py @@ -5,7 +5,7 @@ import warnings from django.core.exceptions import ImproperlyConfigured from django.core.urlresolvers import reverse from django import forms -from django.test import TestCase, override_settings +from django.test import TestCase, ignore_warnings, override_settings from django.test.client import RequestFactory from django.utils.deprecation import RemovedInDjango20Warning from django.views.generic.base import View @@ -143,13 +143,24 @@ class CreateViewTests(TestCase): self.assertRedirects(res, 'http://testserver/edit/authors/create/') self.assertQuerysetEqual(Author.objects.all(), ['']) + @ignore_warnings(category=RemovedInDjango20Warning) def test_create_with_interpolated_redirect(self): - res = self.client.post('/edit/authors/create/interpolate_redirect/', - {'name': 'Randall Munroe', 'slug': 'randall-munroe'}) + res = self.client.post( + '/edit/authors/create/interpolate_redirect/', + {'name': 'Randall Munroe', 'slug': 'randall-munroe'} + ) self.assertQuerysetEqual(Author.objects.all(), ['']) self.assertEqual(res.status_code, 302) - pk = Author.objects.all()[0].pk + pk = Author.objects.first().pk self.assertRedirects(res, 'http://testserver/edit/author/%d/update/' % pk) + # Also test with escaped chars in URL + res = self.client.post( + '/edit/authors/create/interpolate_redirect_nonascii/', + {'name': 'John Doe', 'slug': 'john-doe'} + ) + self.assertEqual(res.status_code, 302) + pk = Author.objects.get(name='John Doe').pk + self.assertRedirects(res, 'http://testserver/%C3%A9dit/author/{}/update/'.format(pk)) def test_create_with_special_properties(self): res = self.client.get('/edit/authors/create/special/') @@ -272,17 +283,28 @@ class UpdateViewTests(TestCase): self.assertRedirects(res, 'http://testserver/edit/authors/create/') self.assertQuerysetEqual(Author.objects.all(), ['']) + @ignore_warnings(category=RemovedInDjango20Warning) def test_update_with_interpolated_redirect(self): a = Author.objects.create( name='Randall Munroe', slug='randall-munroe', ) - res = self.client.post('/edit/author/%d/update/interpolate_redirect/' % a.pk, - {'name': 'Randall Munroe (author of xkcd)', 'slug': 'randall-munroe'}) + res = self.client.post( + '/edit/author/%d/update/interpolate_redirect/' % a.pk, + {'name': 'Randall Munroe (author of xkcd)', 'slug': 'randall-munroe'} + ) self.assertQuerysetEqual(Author.objects.all(), ['']) self.assertEqual(res.status_code, 302) - pk = Author.objects.all()[0].pk + pk = Author.objects.first().pk self.assertRedirects(res, 'http://testserver/edit/author/%d/update/' % pk) + # Also test with escaped chars in URL + res = self.client.post( + '/edit/author/%d/update/interpolate_redirect_nonascii/' % a.pk, + {'name': 'John Doe', 'slug': 'john-doe'} + ) + self.assertEqual(res.status_code, 302) + pk = Author.objects.get(name='John Doe').pk + self.assertRedirects(res, 'http://testserver/%C3%A9dit/author/{}/update/'.format(pk)) def test_update_with_special_properties(self): a = Author.objects.create( @@ -368,12 +390,18 @@ class DeleteViewTests(TestCase): self.assertRedirects(res, 'http://testserver/edit/authors/create/') self.assertQuerysetEqual(Author.objects.all(), []) + @ignore_warnings(category=RemovedInDjango20Warning) def test_delete_with_interpolated_redirect(self): a = Author.objects.create(**{'name': 'Randall Munroe', 'slug': 'randall-munroe'}) res = self.client.post('/edit/author/%d/delete/interpolate_redirect/' % a.pk) self.assertEqual(res.status_code, 302) self.assertRedirects(res, 'http://testserver/edit/authors/create/?deleted=%d' % a.pk) self.assertQuerysetEqual(Author.objects.all(), []) + # Also test with escaped chars in URL + a = Author.objects.create(**{'name': 'Randall Munroe', 'slug': 'randall-munroe'}) + res = self.client.post('/edit/author/{}/delete/interpolate_redirect_nonascii/'.format(a.pk)) + self.assertEqual(res.status_code, 302) + self.assertRedirects(res, 'http://testserver/%C3%A9dit/authors/create/?deleted={}'.format(a.pk)) def test_delete_with_special_properties(self): a = Author.objects.create(**{'name': 'Randall Munroe', 'slug': 'randall-munroe'}) diff --git a/tests/generic_views/urls.py b/tests/generic_views/urls.py index f4965e2317..1bef0a8b77 100644 --- a/tests/generic_views/urls.py +++ b/tests/generic_views/urls.py @@ -1,3 +1,6 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + from django.conf.urls import url from django.contrib.auth import views as auth_views from django.views.decorators.cache import cache_page @@ -74,9 +77,11 @@ urlpatterns = [ views.NaiveAuthorCreate.as_view(success_url='/edit/authors/create/')), url(r'^edit/authors/create/interpolate_redirect/$', views.NaiveAuthorCreate.as_view(success_url='/edit/author/%(id)d/update/')), + url(r'^edit/authors/create/interpolate_redirect_nonascii/$', + views.NaiveAuthorCreate.as_view(success_url='/%C3%A9dit/author/{id}/update/')), url(r'^edit/authors/create/restricted/$', views.AuthorCreateRestricted.as_view()), - url(r'^edit/authors/create/$', + url(r'^[eé]dit/authors/create/$', views.AuthorCreate.as_view()), url(r'^edit/authors/create/special/$', views.SpecializedAuthorCreate.as_view()), @@ -87,7 +92,9 @@ urlpatterns = [ views.NaiveAuthorUpdate.as_view(success_url='/edit/authors/create/')), url(r'^edit/author/(?P[0-9]+)/update/interpolate_redirect/$', views.NaiveAuthorUpdate.as_view(success_url='/edit/author/%(id)d/update/')), - url(r'^edit/author/(?P[0-9]+)/update/$', + url(r'^edit/author/(?P[0-9]+)/update/interpolate_redirect_nonascii/$', + views.NaiveAuthorUpdate.as_view(success_url='/%C3%A9dit/author/{id}/update/')), + url(r'^[eé]dit/author/(?P[0-9]+)/update/$', views.AuthorUpdate.as_view()), url(r'^edit/author/update/$', views.OneAuthorUpdate.as_view()), @@ -99,6 +106,8 @@ urlpatterns = [ views.NaiveAuthorDelete.as_view(success_url='/edit/authors/create/')), url(r'^edit/author/(?P[0-9]+)/delete/interpolate_redirect/$', views.NaiveAuthorDelete.as_view(success_url='/edit/authors/create/?deleted=%(id)s')), + url(r'^edit/author/(?P[0-9]+)/delete/interpolate_redirect_nonascii/$', + views.NaiveAuthorDelete.as_view(success_url='/%C3%A9dit/authors/create/?deleted={id}')), url(r'^edit/author/(?P[0-9]+)/delete/$', views.AuthorDelete.as_view()), url(r'^edit/author/(?P[0-9]+)/delete/special/$',