Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fixes #21587 Added Deprecation Warning for upcoming permanent default #2432

Closed
wants to merge 2 commits into from

5 participants

@kcphysics

As noted in the ticket, we need a deprecation warning. This adds the warning into the current version, a test to make sure the warning indeed warns, and then changes to the documentation about the upcoming change.

@Gwildor

I'm not sure if it's going to work like that (but correct me if I'm wrong!), because what if the user has set the flag as an attribute on the view instead of passing it along when initializing the view?

My suggestion:

permanent = None

def __init__(self, *args, **kwargs):
    super(RedirectView, self).__init__(*args, **kwargs)

    if self.permanent is None:
        self.permanent = True  # set old value for the backwards compatibility
        # warning here
@erikr
Collaborator

I agree with gwildor's comment, but not with that particular code: that would ignore a permanent kwarg. Also, a change like this should be added to the release notes.

@Gwildor

@erikr, I was under the assumption that the __init__ method in View would set the kwargs as attribute on the view, via the initkwargs argument passed onto the as_view method. Actually I moved the super up to the first line just for that reason alone.

@timgraham
Owner

Please see Deprecating a feature for a checklist of items that must be completed. Note that 1.7 is feature frozen pending the release of the beta, so you should probably hold off on this until we cut the 1.7 branch and master is reopened for changes.

@bmispelon
Collaborator

Closing this in favor of #2564.

@bmispelon bmispelon closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
15 django/views/generic/base.py
@@ -1,6 +1,7 @@
from __future__ import unicode_literals
import logging
+import warnings
from functools import update_wrapper
from django import http
@@ -164,6 +165,20 @@ class RedirectView(View):
pattern_name = None
query_string = False
+ def __init__(self, *args, **kwargs):
+ """
+ When initializing the RedirectView we check if permanent is given,
+ if not we raise a deprecation warning and continue on as normal.
+ """
+ if 'permanent' not in kwargs:
+ warnings.warn(
+ "Deprecation Warning: Default value of permanent for RedirectViews will change from True to False in" \
+ " the next release",
+ DeprecationWarning
+ )
+
+ super(RedirectView, self).__init__(*args, **kwargs)
+
def get_redirect_url(self, *args, **kwargs):
"""
Return the URL redirect to. Keyword arguments from the
View
3  docs/ref/class-based-views/base.txt
@@ -223,7 +223,8 @@ RedirectView
Whether the redirect should be permanent. The only difference here is
the HTTP status code returned. If ``True``, then the redirect will use
status code 301. If ``False``, then the redirect will use status code
- 302. By default, ``permanent`` is ``True``.
+ 302. By default, ``permanent`` is ``True``. The default will change from
+ ``True`` to ``False`` in the next release.
.. attribute:: query_string
View
16 tests/generic_views/test_base.py
@@ -2,6 +2,7 @@
import time
import unittest
+import warnings
from django.core.exceptions import ImproperlyConfigured
from django.http import HttpResponse
@@ -439,6 +440,21 @@ def test_direct_instantiation(self):
response = view.dispatch(self.rf.head('/foo/'))
self.assertEqual(response.status_code, 410)
+ def test_permanent_default_deprecation_warning(self):
+ """
+ Tests to make sure that a DeprecationWarning is raised if 'permanent' is not included during instantiation
+ """
+ with warnings.catch_warnings(record=True) as warns:
+ # Cause all warnings to always be triggered.
+ warnings.simplefilter('always')
+ view = RedirectView(url='/bar/')
+ view = RedirectView.as_view(url='/bar/')(self.rf.request(PATH_INFO='/foo/'))
+
+ self.assertEqual(
+ len(warns),
+ 2,
+ msg="Failed to generate deprecation warning when calling RedirectView without permanent kwargs"
+ )
class GetContextDataTest(unittest.TestCase):
Something went wrong with that request. Please try again.