From 019e5490fbd8475483e9f33a74e127522426bec6 Mon Sep 17 00:00:00 2001 From: Timur Sufiev Date: Mon, 5 Dec 2016 21:05:06 +0300 Subject: [PATCH] Remove additional response.render() for tabs Right now TabbedView has additional .render() call. Thus get_context_data() for TabbedView returns already rendered response with rendered=True flag, which makes it read-only. This blocks Profiler middleware in modifying the response. For example, we cannot add any messages to response from tabbed view. Simple views have no additional rendering. It was done because of issues with raising redirect exceptions from tabbed views long ago. Looks like now there is no such issues and raising redirects from tabbed views is covered with unit tests. To ensure that nothing broke, a test with a tab raising Http302 was added - it still redirects to the url provided as before. Change-Id: I37076abc15008a523f37da0d09b5b041ef77844e Closes-Bug: #1595095 --- horizon/tabs/views.py | 10 ---------- horizon/test/tests/tabs.py | 29 ++++++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/horizon/tabs/views.py b/horizon/tabs/views.py index 6122d72f0..a6ccb3baf 100644 --- a/horizon/tabs/views.py +++ b/horizon/tabs/views.py @@ -71,16 +71,6 @@ class TabView(views.HorizonTemplateView): context = self.get_context_data(**kwargs) return self.handle_tabbed_response(context["tab_group"], context) - def render_to_response(self, *args, **kwargs): - response = super(TabView, self).render_to_response(*args, **kwargs) - # Because Django's TemplateView uses the TemplateResponse class - # to provide deferred rendering (which is usually helpful), if - # a tab group raises an Http302 redirect (from exceptions.handle for - # example) the exception is actually raised *after* the final pass - # of the exception-handling middleware. - response.render() - return response - class TabbedTableView(tables.MultiTableMixin, TabView): def __init__(self, *args, **kwargs): diff --git a/horizon/test/tests/tabs.py b/horizon/test/tests/tabs.py index af2ed2760..59650ef4d 100644 --- a/horizon/test/tests/tabs.py +++ b/horizon/test/tests/tabs.py @@ -16,11 +16,13 @@ import copy +from django.conf import settings from django import http import six from horizon import exceptions +from horizon import middleware from horizon import tabs as horizon_tabs from horizon.test import helpers as test @@ -95,6 +97,19 @@ class RecoverableErrorTab(horizon_tabs.Tab): raise exc +class RedirectExceptionTab(horizon_tabs.Tab): + name = "Redirect Exception Tab" + slug = "redirect_exception_tab" + template_name = "_tab.html" + url = settings.TESTSERVER + settings.LOGIN_URL + + def get_context_data(self, request): + # Raise a known recoverable error. + exc = exceptions.Http302(self.url) + exc.silence_logging = True + raise exc + + class TableTabGroup(horizon_tabs.TabGroup): slug = "tab_group" tabs = [TabWithTable] @@ -304,14 +319,26 @@ class TabExceptionTests(test.TestCase): def setUp(self): super(TabExceptionTests, self).setUp() self._original_tabs = copy.copy(TabWithTableView.tab_group_class.tabs) - TabWithTableView.tab_group_class.tabs.append(RecoverableErrorTab) def tearDown(self): super(TabExceptionTests, self).tearDown() TabWithTableView.tab_group_class.tabs = self._original_tabs def test_tab_view_exception(self): + TabWithTableView.tab_group_class.tabs.append(RecoverableErrorTab) view = TabWithTableView.as_view() req = self.factory.get("/") res = view(req) self.assertMessageCount(res, error=1) + + def test_tab_302_exception(self): + TabWithTableView.tab_group_class.tabs.append(RedirectExceptionTab) + view = TabWithTableView.as_view() + req = self.factory.get("/") + mw = middleware.HorizonMiddleware() + try: + resp = view(req) + except Exception as e: + resp = mw.process_exception(req, e) + resp.client = self.client + self.assertRedirects(resp, RedirectExceptionTab.url)