From cf1f36bb6eb34fafe6c224003ad585a647f6117b Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Sun, 14 Dec 2014 17:48:51 +0100 Subject: [PATCH] Deprecated current_app in TemplateResponse and render(_to_response). --- django/contrib/admin/actions.py | 4 +- django/contrib/admin/options.py | 17 ++++++-- django/contrib/admin/sites.py | 10 +++-- django/contrib/auth/admin.py | 5 ++- django/contrib/auth/views.py | 56 +++++++++++++++++++-------- django/shortcuts.py | 15 ++++++- django/template/context.py | 32 +++++++++++---- django/template/defaulttags.py | 11 +++++- django/template/response.py | 15 +++++-- docs/internals/deprecation.txt | 8 ++++ docs/ref/contrib/admin/index.txt | 15 ++++--- docs/ref/template-response.txt | 5 +++ docs/releases/1.8.txt | 15 +++++++ docs/topics/http/shortcuts.txt | 5 +++ docs/topics/http/urls.txt | 18 ++++++--- tests/shortcuts/tests.py | 4 +- tests/template_tests/test_custom.py | 8 +++- tests/template_tests/test_response.py | 10 +++-- 18 files changed, 197 insertions(+), 56 deletions(-) diff --git a/django/contrib/admin/actions.py b/django/contrib/admin/actions.py index 7374d9cea8..bd14057ffd 100644 --- a/django/contrib/admin/actions.py +++ b/django/contrib/admin/actions.py @@ -76,11 +76,13 @@ def delete_selected(modeladmin, request, queryset): action_checkbox_name=helpers.ACTION_CHECKBOX_NAME, ) + request.current_app = modeladmin.admin_site.name + # Display the confirmation page return TemplateResponse(request, modeladmin.delete_selected_confirmation_template or [ "admin/%s/%s/delete_selected_confirmation.html" % (app_label, opts.model_name), "admin/%s/delete_selected_confirmation.html" % app_label, "admin/delete_selected_confirmation.html" - ], context, current_app=modeladmin.admin_site.name) + ], context) delete_selected.short_description = ugettext_lazy("Delete selected %(verbose_name_plural)s") diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 062f0d6cf4..47c57aa4db 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -1120,11 +1120,13 @@ class ModelAdmin(BaseModelAdmin): else: form_template = self.change_form_template + request.current_app = self.admin_site.name + return TemplateResponse(request, form_template or [ "admin/%s/%s/change_form.html" % (app_label, opts.model_name), "admin/%s/change_form.html" % app_label, "admin/change_form.html" - ], context, current_app=self.admin_site.name) + ], context) def response_add(self, request, obj, post_url_continue=None): """ @@ -1349,12 +1351,14 @@ class ModelAdmin(BaseModelAdmin): opts = self.model._meta app_label = opts.app_label + request.current_app = self.admin_site.name + return TemplateResponse(request, self.delete_confirmation_template or [ "admin/{}/{}/delete_confirmation.html".format(app_label, opts.model_name), "admin/{}/delete_confirmation.html".format(app_label), "admin/delete_confirmation.html" - ], context, current_app=self.admin_site.name) + ], context) def get_inline_formsets(self, request, formsets, inline_instances, obj=None): @@ -1632,11 +1636,13 @@ class ModelAdmin(BaseModelAdmin): ) context.update(extra_context or {}) + request.current_app = self.admin_site.name + return TemplateResponse(request, self.change_list_template or [ 'admin/%s/%s/change_list.html' % (app_label, opts.model_name), 'admin/%s/change_list.html' % app_label, 'admin/change_list.html' - ], context, current_app=self.admin_site.name) + ], context) @csrf_protect_m @transaction.atomic @@ -1723,11 +1729,14 @@ class ModelAdmin(BaseModelAdmin): preserved_filters=self.get_preserved_filters(request), ) context.update(extra_context or {}) + + request.current_app = self.admin_site.name + return TemplateResponse(request, self.object_history_template or [ "admin/%s/%s/object_history.html" % (app_label, opts.model_name), "admin/%s/object_history.html" % app_label, "admin/object_history.html" - ], context, current_app=self.admin_site.name) + ], context) def _create_formsets(self, request, obj, change): "Helper function to generate formsets for add/change_view." diff --git a/django/contrib/admin/sites.py b/django/contrib/admin/sites.py index 8b46a7e546..4926e3c721 100644 --- a/django/contrib/admin/sites.py +++ b/django/contrib/admin/sites.py @@ -436,9 +436,11 @@ class AdminSite(object): app_list=app_list, ) context.update(extra_context or {}) + + request.current_app = self.name + return TemplateResponse(request, self.index_template or - 'admin/index.html', context, - current_app=self.name) + 'admin/index.html', context) def app_index(self, request, app_label, extra_context=None): app_name = apps.get_app_config(app_label).verbose_name @@ -494,10 +496,12 @@ class AdminSite(object): ) context.update(extra_context or {}) + request.current_app = self.name + return TemplateResponse(request, self.app_index_template or [ 'admin/%s/app_index.html' % app_label, 'admin/app_index.html' - ], context, current_app=self.name) + ], context) # This global object represents the default admin site, for the common case. # You can instantiate AdminSite in your own code to create a custom admin site. diff --git a/django/contrib/auth/admin.py b/django/contrib/auth/admin.py index 9b75dc59e7..8b7f80ee39 100644 --- a/django/contrib/auth/admin.py +++ b/django/contrib/auth/admin.py @@ -158,10 +158,13 @@ class UserAdmin(admin.ModelAdmin): 'show_save': True, } context.update(admin.site.each_context()) + + request.current_app = self.admin_site.name + return TemplateResponse(request, self.change_user_password_template or 'admin/auth/user/change_password.html', - context, current_app=self.admin_site.name) + context) def response_add(self, request, obj, post_url_continue=None): """ diff --git a/django/contrib/auth/views.py b/django/contrib/auth/views.py index ed39c53b00..348fca8745 100644 --- a/django/contrib/auth/views.py +++ b/django/contrib/auth/views.py @@ -60,8 +60,11 @@ def login(request, template_name='registration/login.html', } if extra_context is not None: context.update(extra_context) - return TemplateResponse(request, template_name, context, - current_app=current_app) + + if current_app is not None: + request.current_app = current_app + + return TemplateResponse(request, template_name, context) def logout(request, next_page=None, @@ -96,8 +99,11 @@ def logout(request, next_page=None, } if extra_context is not None: context.update(extra_context) - return TemplateResponse(request, template_name, context, - current_app=current_app) + + if current_app is not None: + request.current_app = current_app + + return TemplateResponse(request, template_name, context) def logout_then_login(request, login_url=None, current_app=None, extra_context=None): @@ -179,8 +185,11 @@ def password_reset(request, is_admin_site=False, } if extra_context is not None: context.update(extra_context) - return TemplateResponse(request, template_name, context, - current_app=current_app) + + if current_app is not None: + request.current_app = current_app + + return TemplateResponse(request, template_name, context) def password_reset_done(request, @@ -191,8 +200,11 @@ def password_reset_done(request, } if extra_context is not None: context.update(extra_context) - return TemplateResponse(request, template_name, context, - current_app=current_app) + + if current_app is not None: + request.current_app = current_app + + return TemplateResponse(request, template_name, context) # Doesn't need csrf_protect since no-one can guess the URL @@ -241,8 +253,11 @@ def password_reset_confirm(request, uidb64=None, token=None, } if extra_context is not None: context.update(extra_context) - return TemplateResponse(request, template_name, context, - current_app=current_app) + + if current_app is not None: + request.current_app = current_app + + return TemplateResponse(request, template_name, context) def password_reset_complete(request, @@ -254,8 +269,11 @@ def password_reset_complete(request, } if extra_context is not None: context.update(extra_context) - return TemplateResponse(request, template_name, context, - current_app=current_app) + + if current_app is not None: + request.current_app = current_app + + return TemplateResponse(request, template_name, context) @sensitive_post_parameters() @@ -288,8 +306,11 @@ def password_change(request, } if extra_context is not None: context.update(extra_context) - return TemplateResponse(request, template_name, context, - current_app=current_app) + + if current_app is not None: + request.current_app = current_app + + return TemplateResponse(request, template_name, context) @login_required @@ -301,5 +322,8 @@ def password_change_done(request, } if extra_context is not None: context.update(extra_context) - return TemplateResponse(request, template_name, context, - current_app=current_app) + + if current_app is not None: + request.current_app = current_app + + return TemplateResponse(request, template_name, context) diff --git a/django/shortcuts.py b/django/shortcuts.py index 780f89a6b5..a8f7278bcb 100644 --- a/django/shortcuts.py +++ b/django/shortcuts.py @@ -3,6 +3,9 @@ This module collects helper functions and classes that "span" multiple levels of MVC. In other words, these functions/classes introduce controlled coupling for convenience's sake. """ + +import warnings + from django.template import loader, RequestContext from django.template.context import _current_app_undefined from django.template.engine import ( @@ -14,6 +17,7 @@ from django.db.models.manager import Manager from django.db.models.query import QuerySet from django.core import urlresolvers from django.utils import six +from django.utils.deprecation import RemovedInDjango20Warning def render_to_response(template_name, context=None, @@ -61,7 +65,16 @@ def render(request, template_name, context=None, raise ValueError('If you provide a context_instance you must ' 'set its current_app before calling render()') else: - context_instance = RequestContext(request, current_app=current_app) + context_instance = RequestContext(request) + if current_app is not _current_app_undefined: + warnings.warn( + "The current_app argument of render is deprecated. " + "Set the current_app attribute of request instead.", + RemovedInDjango20Warning, stacklevel=2) + request.current_app = current_app + # Directly set the private attribute to avoid triggering the + # warning in RequestContext.__init__. + context_instance._current_app = current_app content = loader.render_to_string( template_name, context, context_instance, dirs, dictionary) diff --git a/django/template/context.py b/django/template/context.py index ff77e25e34..efbee55a04 100644 --- a/django/template/context.py +++ b/django/template/context.py @@ -1,4 +1,7 @@ from copy import copy +import warnings + +from django.utils.deprecation import RemovedInDjango20Warning # Hard-coded processor for easier use of CSRF protection. @@ -122,16 +125,23 @@ class Context(BaseContext): def __init__(self, dict_=None, autoescape=True, current_app=_current_app_undefined, use_l10n=None, use_tz=None, engine=None): - if current_app is _current_app_undefined: - current_app = None + if current_app is not _current_app_undefined: + warnings.warn( + "The current_app argument of Context is deprecated. Use " + "RequestContext and set the current_app attribute of its " + "request instead.", RemovedInDjango20Warning, stacklevel=2) self.autoescape = autoescape - self.current_app = current_app + self._current_app = current_app self.use_l10n = use_l10n self.use_tz = use_tz self.engine = engine self.render_context = RenderContext() super(Context, self).__init__(dict_) + @property + def current_app(self): + return None if self._current_app is _current_app_undefined else self._current_app + def __copy__(self): duplicate = super(Context, self).__copy__() duplicate.render_context = copy(self.render_context) @@ -184,9 +194,17 @@ class RequestContext(Context): def __init__(self, request, dict_=None, processors=None, current_app=_current_app_undefined, use_l10n=None, use_tz=None, engine=None): - Context.__init__(self, dict_, current_app=current_app, - use_l10n=use_l10n, use_tz=use_tz, engine=engine) - self._request = request + # current_app isn't passed here to avoid triggering the deprecation + # warning in Context.__init__. + super(RequestContext, self).__init__( + dict_, use_l10n=use_l10n, use_tz=use_tz, engine=engine) + if current_app is not _current_app_undefined: + warnings.warn( + "The current_app argument of RequestContext is deprecated. " + "Set the current_app attribute of its request instead.", + RemovedInDjango20Warning, stacklevel=2) + self._current_app = current_app + self.request = request self._processors = () if processors is None else tuple(processors) self._processors_index = len(self.dicts) self.update({}) # placeholder for context processors output @@ -207,7 +225,7 @@ class RequestContext(Context): # Set context processors for this engine. updates = {} for processor in engine.template_context_processors + self._processors: - updates.update(processor(self._request)) + updates.update(processor(self.request)) self.dicts[self._processors_index] = updates def new(self, values=None): diff --git a/django/template/defaulttags.py b/django/template/defaulttags.py index 176601baa4..edd8260aad 100644 --- a/django/template/defaulttags.py +++ b/django/template/defaulttags.py @@ -476,13 +476,20 @@ class URLNode(Node): view_name = self.view_name.resolve(context) + try: + current_app = context.request.current_app + except AttributeError: + # Change the fallback value to None when the deprecation path for + # Context.current_app completes in Django 2.0. + current_app = context.current_app + # Try to look up the URL twice: once given the view name, and again # relative to what we guess is the "main" app. If they both fail, # re-raise the NoReverseMatch unless we're using the # {% url ... as var %} construct in which case return nothing. url = '' try: - url = reverse(view_name, args=args, kwargs=kwargs, current_app=context.current_app) + url = reverse(view_name, args=args, kwargs=kwargs, current_app=current_app) except NoReverseMatch: exc_info = sys.exc_info() if settings.SETTINGS_MODULE: @@ -490,7 +497,7 @@ class URLNode(Node): try: url = reverse(project_name + '.' + view_name, args=args, kwargs=kwargs, - current_app=context.current_app) + current_app=current_app) except NoReverseMatch: if self.asvar is None: # Re-raise the original exception, not the one with diff --git a/django/template/response.py b/django/template/response.py index 53d2795868..6f23f66b7d 100644 --- a/django/template/response.py +++ b/django/template/response.py @@ -1,6 +1,10 @@ +import warnings + from django.http import HttpResponse from django.template import loader, Context, RequestContext +from django.template.context import _current_app_undefined from django.utils import six +from django.utils.deprecation import RemovedInDjango20Warning class ContentNotRenderedError(Exception): @@ -137,14 +141,19 @@ class TemplateResponse(SimpleTemplateResponse): rendering_attrs = SimpleTemplateResponse.rendering_attrs + ['_request', '_current_app'] def __init__(self, request, template, context=None, content_type=None, - status=None, current_app=None, charset=None): + status=None, current_app=_current_app_undefined, charset=None): # self.request gets over-written by django.test.client.Client - and # unlike context_data and template_name the _request should not # be considered part of the public API. self._request = request # As a convenience we'll allow callers to provide current_app without # having to avoid needing to create the RequestContext directly - self._current_app = current_app + if current_app is not _current_app_undefined: + warnings.warn( + "The current_app argument of TemplateResponse is deprecated. " + "Set the current_app attribute of its request instead.", + RemovedInDjango20Warning, stacklevel=2) + request.current_app = current_app super(TemplateResponse, self).__init__( template, context, content_type, status, charset) @@ -154,7 +163,7 @@ class TemplateResponse(SimpleTemplateResponse): """ if isinstance(context, Context): return context - context_instance = RequestContext(self._request, current_app=self._current_app) + context_instance = RequestContext(self._request) if context: context_instance.push(context) return context_instance diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 0adf08ed5e..bf659d8c62 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -90,6 +90,14 @@ details on these changes. * The backwards compatibility alias ``django.template.loader.BaseLoader`` will be removed. +* The ``current_app`` parameter for the following function and classes will be + removed: + + * ``django.shortcuts.render()`` + * ``django.template.Context()`` + * ``django.template.RequestContext()`` + * ``django.template.response.TemplateResponse()`` + * The ``dictionary`` and ``context_instance`` parameters for the following functions will be removed: diff --git a/docs/ref/contrib/admin/index.txt b/docs/ref/contrib/admin/index.txt index a1d4c3a563..a6a315cc88 100644 --- a/docs/ref/contrib/admin/index.txt +++ b/docs/ref/contrib/admin/index.txt @@ -2628,11 +2628,16 @@ a pattern for your new view. .. note:: Any view you render that uses the admin templates, or extends the base - admin template, should provide the ``current_app`` argument to - :class:`~django.template.RequestContext` or - :class:`~django.template.Context` when rendering the template. It should - be set to either ``self.name`` if your view is on an ``AdminSite`` or - ``self.admin_site.name`` if your view is on a ``ModelAdmin``. + admin template, should set ``request.current_app`` before rendering the + template. It should be set to either ``self.name`` if your view is on an + ``AdminSite`` or ``self.admin_site.name`` if your view is on a + ``ModelAdmin``. + + .. versionchanged:: 1.8 + + In previous versions of Django, you had to provide the ``current_app`` + argument to :class:`~django.template.RequestContext` or + :class:`~django.template.Context` when rendering the template. .. _auth_password_reset: diff --git a/docs/ref/template-response.txt b/docs/ref/template-response.txt index bfee0e29af..3f7572b557 100644 --- a/docs/ref/template-response.txt +++ b/docs/ref/template-response.txt @@ -182,6 +182,11 @@ Methods :ref:`namespaced URL resolution strategy ` for more information. + .. deprecated:: 1.8 + + The ``current_app`` argument is deprecated. Instead you should set + ``request.current_app``. + ``charset`` The charset in which the response will be encoded. If not given it will be extracted from ``content_type``, and if that is unsuccessful, the diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt index 8f871cb96c..39fc642690 100644 --- a/docs/releases/1.8.txt +++ b/docs/releases/1.8.txt @@ -1204,6 +1204,21 @@ to construct the "view on site" URL. This URL is now accessible using the sure to provide a default value for the ``form_class`` argument since it's now optional. +``current_app`` argument of template-related APIs +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The following functions and classes will no longer accept a ``current_app`` +parameter to set an URL namespace in Django 2.0: + +* ``django.shortcuts.render()`` +* ``django.template.Context()`` +* ``django.template.RequestContext()`` +* ``django.template.response.TemplateResponse()`` + +Set ``request.current_app`` instead, where ``request`` is the first argument +to these functions or classes. If you're using a plain ``Context``, use a +``RequestContext`` instead. + ``dictionary`` and ``context_instance`` arguments of rendering functions ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/topics/http/shortcuts.txt b/docs/topics/http/shortcuts.txt index 5c356c2b3b..d3971fb711 100644 --- a/docs/topics/http/shortcuts.txt +++ b/docs/topics/http/shortcuts.txt @@ -72,6 +72,11 @@ Optional arguments :ref:`namespaced URL resolution strategy ` for more information. + .. deprecated:: 1.8 + + The ``current_app`` argument is deprecated. Instead you should set + ``request.current_app``. + .. versionchanged:: 1.7 The ``dirs`` parameter was added. diff --git a/docs/topics/http/urls.txt b/docs/topics/http/urls.txt index a63eff075d..a7dbb72462 100644 --- a/docs/topics/http/urls.txt +++ b/docs/topics/http/urls.txt @@ -633,10 +633,16 @@ the fully qualified name into parts and then tries the following lookup: 2. If there is a *current* application defined, Django finds and returns the URL resolver for that instance. The *current* application can be - specified as an attribute on the template context - applications that - expect to have multiple deployments should set the ``current_app`` - attribute on any :class:`~django.template.Context` or - :class:`~django.template.RequestContext` that is used to render a template. + specified as an attribute on the request. Applications that expect to + have multiple deployments should set the ``current_app`` attribute on + the ``request`` being processed. + + .. versionchanged:: 1.8 + + In previous versions of Django, you had to set the ``current_app`` + attribute on any :class:`~django.template.Context` or + :class:`~django.template.RequestContext` that is used to render a + template. The current application can also be specified manually as an argument to the :func:`~django.core.urlresolvers.reverse` function. @@ -707,10 +713,10 @@ Using this setup, the following lookups are possible: {% url 'polls:index' %} Note that reversing in the template requires the ``current_app`` be added as - an attribute to the template context like this:: + an attribute to the ``request`` like this:: def render_to_response(self, context, **response_kwargs): - response_kwargs['current_app'] = self.request.resolver_match.namespace + self.request.current_app = self.request.resolver_match.namespace return super(DetailView, self).render_to_response(context, **response_kwargs) * If there is no current instance - say, if we were rendering a page diff --git a/tests/shortcuts/tests.py b/tests/shortcuts/tests.py index 429738b22f..984373bfe7 100644 --- a/tests/shortcuts/tests.py +++ b/tests/shortcuts/tests.py @@ -54,7 +54,7 @@ class ShortcutTests(TestCase): self.assertEqual(response.status_code, 200) self.assertEqual(response.content, b'FOO.BAR../path/to/static/media/\n') self.assertEqual(response['Content-Type'], 'text/html; charset=utf-8') - self.assertEqual(response.context.current_app, None) + self.assertFalse(hasattr(response.context.request, 'current_app')) def test_render_with_base_context(self): with warnings.catch_warnings(): @@ -79,7 +79,7 @@ class ShortcutTests(TestCase): with warnings.catch_warnings(): warnings.filterwarnings("ignore", category=RemovedInDjango20Warning) response = self.client.get('/render/current_app/') - self.assertEqual(response.context.current_app, "foobar_app") + self.assertEqual(response.context.request.current_app, "foobar_app") def test_render_with_dirs(self): with warnings.catch_warnings(): diff --git a/tests/template_tests/test_custom.py b/tests/template_tests/test_custom.py index c2dff03b10..f0768ba2a5 100644 --- a/tests/template_tests/test_custom.py +++ b/tests/template_tests/test_custom.py @@ -1,9 +1,11 @@ from __future__ import unicode_literals from unittest import TestCase +import warnings from django import template from django.utils import six +from django.utils.deprecation import RemovedInDjango20Warning from .templatetags import custom @@ -253,8 +255,10 @@ class CustomTagTests(TestCase): t = template.Template('{% load custom %}{% inclusion_tag_current_app %}') self.assertEqual(t.render(c).strip(), 'None') - c.current_app = 'advanced' - self.assertEqual(t.render(c).strip(), 'advanced') + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=RemovedInDjango20Warning) + c = template.Context({}, current_app='advanced') + self.assertEqual(t.render(c).strip(), 'advanced') def test_15070_use_l10n(self): """ diff --git a/tests/template_tests/test_response.py b/tests/template_tests/test_response.py index f827b2eeab..ab01da5804 100644 --- a/tests/template_tests/test_response.py +++ b/tests/template_tests/test_response.py @@ -1,9 +1,10 @@ from __future__ import unicode_literals +from datetime import datetime import os import pickle import time -from datetime import datetime +import warnings from django.test import RequestFactory, SimpleTestCase from django.conf import settings @@ -12,6 +13,7 @@ from django.template.response import (TemplateResponse, SimpleTemplateResponse, ContentNotRenderedError) from django.test import override_settings from django.utils._os import upath +from django.utils.deprecation import RemovedInDjango20Warning def test_processor(request): @@ -252,11 +254,13 @@ class TemplateResponseTest(SimpleTestCase): self.assertEqual(response.status_code, 504) def test_custom_app(self): - response = self._response('{{ foo }}', current_app="foobar") + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=RemovedInDjango20Warning) + response = self._response('{{ foo }}', current_app="foobar") rc = response.resolve_context(response.context_data) - self.assertEqual(rc.current_app, 'foobar') + self.assertEqual(rc.request.current_app, 'foobar') def test_pickling(self): # Create a template response. The context is