Skip to content

Commit

Permalink
Fixed #21587 -- Added a warning for changing default of RedirectView.…
Browse files Browse the repository at this point in the history
…permanent.
  • Loading branch information
berkerpeksag authored and timgraham committed Nov 25, 2014
1 parent d43dd03 commit 9a30aca
Show file tree
Hide file tree
Showing 7 changed files with 136 additions and 22 deletions.
18 changes: 16 additions & 2 deletions django/views/generic/base.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
from __future__ import unicode_literals

import logging
import warnings
from functools import update_wrapper

from django import http
from django.core.exceptions import ImproperlyConfigured
from django.core.urlresolvers import reverse, NoReverseMatch
from django.template.response import TemplateResponse
from django.utils.decorators import classonlymethod
from django.utils.deprecation import RemovedInDjango19Warning
from django.utils import six

_sentinel = object()
logger = logging.getLogger('django.request')


Expand Down Expand Up @@ -48,7 +51,6 @@ def as_view(cls, **initkwargs):
"""
Main entry point for a request-response process.
"""
# sanitize keyword arguments
for key in initkwargs:
if key in cls.http_method_names:
raise TypeError("You tried to pass in the %s method name as a "
Expand Down Expand Up @@ -159,11 +161,23 @@ class RedirectView(View):
"""
A view that provides a redirect on any GET request.
"""
permanent = True
permanent = _sentinel
url = None
pattern_name = None
query_string = False

def __init__(self, *args, **kwargs):
if 'permanent' not in kwargs and self.permanent is _sentinel:
warnings.warn(
"Default value of 'RedirectView.permanent' will change "
"from True to False in Django 1.9. Set an explicit value "
"to silence this warning.",
RemovedInDjango19Warning,
stacklevel=3
)
self.permanent = True
super(RedirectView, self).__init__(*args, **kwargs)

def get_redirect_url(self, *args, **kwargs):
"""
Return the URL redirect to. Keyword arguments from the
Expand Down
4 changes: 4 additions & 0 deletions docs/internals/deprecation.txt
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,10 @@ details on these changes.
* The `cache_choices` option to :class:`~django.forms.ModelChoiceField` and
:class:`~django.forms.ModelMultipleChoiceField` will be removed.

* The default value of the
:attr:`RedirectView.permanent <django.views.generic.base.RedirectView.permanent>`
attribute will change from ``True`` to ``False``.

.. _deprecation-removed-in-1.8:

1.8
Expand Down
5 changes: 5 additions & 0 deletions docs/ref/class-based-views/base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,11 @@ RedirectView
status code 301. If ``False``, then the redirect will use status code
302. By default, ``permanent`` is ``True``.

.. deprecated:: 1.8

The default value of the ``permanent`` attribute will change from
``True`` to ``False`` in Django 1.9.

.. attribute:: query_string

Whether to pass along the GET query string to the new location. If
Expand Down
7 changes: 7 additions & 0 deletions docs/releases/1.8.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1097,6 +1097,13 @@ deprecated: you should rename your ``qn`` arguments to ``compiler``, and call
``compiler.quote_name_unless_alias(...)`` where you previously called
``qn(...)``.

Default value of ``RedirectView.permanent``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The default value of the
:attr:`RedirectView.permanent <django.views.generic.base.RedirectView.permanent>`
attribute will change from ``True`` to ``False`` in Django 1.9.

.. removed-features-1.8:

Features removed in 1.8
Expand Down
69 changes: 68 additions & 1 deletion tests/generic_views/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,14 @@

import time
import unittest
import warnings

from django.core.exceptions import ImproperlyConfigured
from django.http import HttpResponse
from django.utils import six
from django.utils.deprecation import RemovedInDjango19Warning
from django.test import TestCase, RequestFactory, override_settings
from django.test.utils import IgnoreDeprecationWarningsMixin
from django.views.generic import View, TemplateView, RedirectView

from . import views
Expand Down Expand Up @@ -328,7 +332,7 @@ def test_content_type(self):


@override_settings(ROOT_URLCONF='generic_views.urls')
class RedirectViewTest(TestCase):
class RedirectViewTest(IgnoreDeprecationWarningsMixin, TestCase):

rf = RequestFactory()

Expand Down Expand Up @@ -440,6 +444,69 @@ def test_direct_instantiation(self):
self.assertEqual(response.status_code, 410)


@override_settings(ROOT_URLCONF='generic_views.urls')
class RedirectViewDeprecationTest(TestCase):

rf = RequestFactory()

def test_deprecation_warning_init(self):
with warnings.catch_warnings(record=True) as warns:
warnings.simplefilter('always')

view = RedirectView()
response = view.dispatch(self.rf.head('/python/'))

self.assertEqual(response.status_code, 410)
self.assertEqual(len(warns), 1)
self.assertIs(warns[0].category, RemovedInDjango19Warning)
self.assertEqual(
six.text_type(warns[0].message),
"Default value of 'RedirectView.permanent' will change "
"from True to False in Django 1.9. Set an explicit value "
"to silence this warning.",
)

def test_deprecation_warning_raised_when_permanent_not_passed(self):
with warnings.catch_warnings(record=True) as warns:
warnings.simplefilter('always')

view_function = RedirectView.as_view(url='/bbb/')
request = self.rf.request(PATH_INFO='/aaa/')
view_function(request)

self.assertEqual(len(warns), 1)
self.assertIs(warns[0].category, RemovedInDjango19Warning)
self.assertEqual(
six.text_type(warns[0].message),
"Default value of 'RedirectView.permanent' will change "
"from True to False in Django 1.9. Set an explicit value "
"to silence this warning.",
)

def test_no_deprecation_warning_when_permanent_passed(self):
with warnings.catch_warnings(record=True) as warns:
warnings.simplefilter('always')

view_function = RedirectView.as_view(url='/bar/', permanent=False)
request = self.rf.request(PATH_INFO='/foo/')
view_function(request)

self.assertEqual(len(warns), 0)

def test_no_deprecation_warning_with_custom_redirectview(self):
class CustomRedirectView(RedirectView):
permanent = False

with warnings.catch_warnings(record=True) as warns:
warnings.simplefilter('always')

view_function = CustomRedirectView.as_view(url='/eggs/')
request = self.rf.request(PATH_INFO='/spam/')
view_function(request)

self.assertEqual(len(warns), 0)


class GetContextDataTest(unittest.TestCase):

def test_get_context_data_super(self):
Expand Down
47 changes: 31 additions & 16 deletions tests/test_client/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,11 @@
"""
from __future__ import unicode_literals

import warnings

from django.core import mail
from django.http import HttpResponse
from django.utils.deprecation import RemovedInDjango19Warning
from django.test import Client, TestCase, RequestFactory
from django.test import override_settings

Expand Down Expand Up @@ -174,14 +177,18 @@ def test_redirect_with_query(self):

def test_permanent_redirect(self):
"GET a URL that redirects permanently elsewhere"
response = self.client.get('/permanent_redirect_view/')
# Check that the response was a 301 (permanent redirect)
self.assertRedirects(response, 'http://testserver/get_view/', status_code=301)

client_providing_host = Client(HTTP_HOST='django.testserver')
response = client_providing_host.get('/permanent_redirect_view/')
# Check that the response was a 301 (permanent redirect) with absolute URI
self.assertRedirects(response, 'http://django.testserver/get_view/', status_code=301)
with warnings.catch_warnings():
warnings.simplefilter("ignore", RemovedInDjango19Warning)
response = self.client.get('/permanent_redirect_view/')
# Check that the response was a 301 (permanent redirect)
self.assertRedirects(response, 'http://testserver/get_view/', status_code=301)

with warnings.catch_warnings():
warnings.simplefilter("ignore", RemovedInDjango19Warning)
client_providing_host = Client(HTTP_HOST='django.testserver')
response = client_providing_host.get('/permanent_redirect_view/')
# Check that the response was a 301 (permanent redirect) with absolute URI
self.assertRedirects(response, 'http://django.testserver/get_view/', status_code=301)

def test_temporary_redirect(self):
"GET a URL that does a non-permanent redirect"
Expand All @@ -191,26 +198,34 @@ def test_temporary_redirect(self):

def test_redirect_to_strange_location(self):
"GET a URL that redirects to a non-200 page"
response = self.client.get('/double_redirect_view/')
with warnings.catch_warnings():
warnings.simplefilter("ignore", RemovedInDjango19Warning)
response = self.client.get('/double_redirect_view/')

# Check that the response was a 302, and that
# the attempt to get the redirection location returned 301 when retrieved
self.assertRedirects(response, 'http://testserver/permanent_redirect_view/', target_status_code=301)
# Check that the response was a 302, and that
# the attempt to get the redirection location returned 301 when retrieved
self.assertRedirects(response, 'http://testserver/permanent_redirect_view/', target_status_code=301)

def test_follow_redirect(self):
"A URL that redirects can be followed to termination."
response = self.client.get('/double_redirect_view/', follow=True)
self.assertRedirects(response, 'http://testserver/get_view/', status_code=302, target_status_code=200)
with warnings.catch_warnings():
warnings.simplefilter("ignore", RemovedInDjango19Warning)
response = self.client.get('/double_redirect_view/', follow=True)
self.assertRedirects(response, 'http://testserver/get_view/', status_code=302, target_status_code=200)
self.assertEqual(len(response.redirect_chain), 2)

def test_redirect_http(self):
"GET a URL that redirects to an http URI"
response = self.client.get('/http_redirect_view/', follow=True)
with warnings.catch_warnings():
warnings.simplefilter("ignore", RemovedInDjango19Warning)
response = self.client.get('/http_redirect_view/', follow=True)
self.assertFalse(response.test_was_secure_request)

def test_redirect_https(self):
"GET a URL that redirects to an https URI"
response = self.client.get('/https_redirect_view/', follow=True)
with warnings.catch_warnings():
warnings.simplefilter("ignore", RemovedInDjango19Warning)
response = self.client.get('/https_redirect_view/', follow=True)
self.assertTrue(response.test_was_secure_request)

def test_notfound_response(self):
Expand Down
8 changes: 5 additions & 3 deletions tests/urlpatterns_reverse/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from django.shortcuts import redirect
from django.test import TestCase, override_settings
from django.utils import six
from django.utils.deprecation import RemovedInDjango20Warning
from django.utils.deprecation import RemovedInDjango19Warning, RemovedInDjango20Warning

from admin_scripts.tests import AdminScriptTestCase

Expand Down Expand Up @@ -309,8 +309,10 @@ def test_404_tried_urls_have_names(self):
class ReverseLazyTest(TestCase):

def test_redirect_with_lazy_reverse(self):
response = self.client.get('/redirect/')
self.assertRedirects(response, "/redirected_to/", status_code=301)
with warnings.catch_warnings():
warnings.simplefilter("ignore", RemovedInDjango19Warning)
response = self.client.get('/redirect/')
self.assertRedirects(response, "/redirected_to/", status_code=301)

def test_user_permission_with_lazy_reverse(self):
User.objects.create_user('alfred', '[email protected]', password='testpw')
Expand Down

0 comments on commit 9a30aca

Please sign in to comment.