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
This commit is contained in:
Timur Sufiev 2016-12-05 21:05:06 +03:00
parent 40e73533fb
commit 019e5490fb
2 changed files with 28 additions and 11 deletions

View File

@ -71,16 +71,6 @@ class TabView(views.HorizonTemplateView):
context = self.get_context_data(**kwargs) context = self.get_context_data(**kwargs)
return self.handle_tabbed_response(context["tab_group"], context) 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): class TabbedTableView(tables.MultiTableMixin, TabView):
def __init__(self, *args, **kwargs): def __init__(self, *args, **kwargs):

View File

@ -16,11 +16,13 @@
import copy import copy
from django.conf import settings
from django import http from django import http
import six import six
from horizon import exceptions from horizon import exceptions
from horizon import middleware
from horizon import tabs as horizon_tabs from horizon import tabs as horizon_tabs
from horizon.test import helpers as test from horizon.test import helpers as test
@ -95,6 +97,19 @@ class RecoverableErrorTab(horizon_tabs.Tab):
raise exc 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): class TableTabGroup(horizon_tabs.TabGroup):
slug = "tab_group" slug = "tab_group"
tabs = [TabWithTable] tabs = [TabWithTable]
@ -304,14 +319,26 @@ class TabExceptionTests(test.TestCase):
def setUp(self): def setUp(self):
super(TabExceptionTests, self).setUp() super(TabExceptionTests, self).setUp()
self._original_tabs = copy.copy(TabWithTableView.tab_group_class.tabs) self._original_tabs = copy.copy(TabWithTableView.tab_group_class.tabs)
TabWithTableView.tab_group_class.tabs.append(RecoverableErrorTab)
def tearDown(self): def tearDown(self):
super(TabExceptionTests, self).tearDown() super(TabExceptionTests, self).tearDown()
TabWithTableView.tab_group_class.tabs = self._original_tabs TabWithTableView.tab_group_class.tabs = self._original_tabs
def test_tab_view_exception(self): def test_tab_view_exception(self):
TabWithTableView.tab_group_class.tabs.append(RecoverableErrorTab)
view = TabWithTableView.as_view() view = TabWithTableView.as_view()
req = self.factory.get("/") req = self.factory.get("/")
res = view(req) res = view(req)
self.assertMessageCount(res, error=1) 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)