Merge lp:~salgado/launchpad/bug-538207 into lp:launchpad

Proposed by Guilherme Salgado
Status: Merged
Approved by: Aaron Bentley
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~salgado/launchpad/bug-538207
Merge into: lp:launchpad
Diff against target: 135 lines (+23/-20)
6 files modified
lib/canonical/launchpad/doc/launchpadview.txt (+0/-10)
lib/canonical/launchpad/templates/oops.pt (+1/-1)
lib/canonical/launchpad/webapp/publisher.py (+0/-4)
lib/canonical/launchpad/webapp/servers.py (+5/-1)
lib/canonical/launchpad/webapp/tests/test_servers.py (+15/-2)
lib/lp/app/templates/base-layout-macros.pt (+2/-2)
To merge this branch: bzr merge lp:~salgado/launchpad/bug-538207
Reviewer Review Type Date Requested Status
Aaron Bentley (community) Approve
Review via email: mp+21387@code.launchpad.net

Description of the change

= Summary =

Move isRedirectInhibited from LaunchpadView to LaunchpadBrowserRequest
so that views that don't inherit from LaunchpadView don't OOPS on edge.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/webapp/publisher.py
  lib/lp/app/templates/base-layout-macros.pt
  lib/canonical/launchpad/doc/launchpadview.txt
  lib/canonical/launchpad/webapp/servers.py
  lib/canonical/launchpad/webapp/tests/test_servers.py

== Pylint notices ==

lib/canonical/launchpad/webapp/servers.py
    47: [F0401] Unable to import 'lazr.restful.interfaces' (No module named restful)
    49: [F0401] Unable to import 'lazr.restful.publisher' (No module named restful)
    73: [F0401] Unable to import 'lazr.uri' (No module named uri)

lib/canonical/launchpad/webapp/tests/test_servers.py
    18: [F0401] Unable to import 'lazr.restful.interfaces' (No module named restful)
    20: [F0401] Unable to import 'lazr.restful.simple' (No module named restful)
    21: [F0401] Unable to import 'lazr.restful.tests.test_webservice' (No module named restful)

To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) wrote :

The code looks good, but your comments about E1002 are irrelevant to the issue. TestWebServiceRequestTraversal, TestVhostWebserviceFactory, FeedsPublication, WebServicePublication, PrivateXMLRPCPublication and ProtocolErrorPublication are all derived from classes defined outside their file, and so their status as new-style classes is determined by the classes they derive from. __metaclass__ = type has absolutely no impact on these classes.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/doc/launchpadview.txt'
--- lib/canonical/launchpad/doc/launchpadview.txt 2009-10-21 23:51:08 +0000
+++ lib/canonical/launchpad/doc/launchpadview.txt 2010-03-15 23:15:37 +0000
@@ -102,13 +102,3 @@
102 >>> view.error_message = structured('Information overload.')102 >>> view.error_message = structured('Information overload.')
103 >>> view.error_message.escapedtext103 >>> view.error_message.escapedtext
104 u'Information overload.'104 u'Information overload.'
105
106Every Launchpad view also knows whether edge redirection has been inhibited.
107
108 >>> view.isRedirectInhibited()
109 False
110 >>> new_request = LaunchpadTestRequest(
111 ... HTTP_COOKIE="inhibit_beta_redirect=1")
112 >>> view = MyView(context, new_request)
113 >>> view.isRedirectInhibited()
114 True
115105
=== modified file 'lib/canonical/launchpad/templates/oops.pt'
--- lib/canonical/launchpad/templates/oops.pt 2010-03-10 19:10:04 +0000
+++ lib/canonical/launchpad/templates/oops.pt 2010-03-15 23:15:37 +0000
@@ -11,7 +11,7 @@
11 </style>11 </style>
12 <script type="text/javascript"12 <script type="text/javascript"
13 tal:content="string:var cookie_scope = '${request/lp:cookie_scope}';"></script>13 tal:content="string:var cookie_scope = '${request/lp:cookie_scope}';"></script>
14 <script type="text/javascript" src="/+icing/rev5/build/lp/lp.js" />14 <script type="text/javascript" src="/+icing/rev5/build/lp/lp.js"></script>
15 </head>15 </head>
16 <body>16 <body>
17 <div class="yui-d0">17 <div class="yui-d0">
1818
=== modified file 'lib/canonical/launchpad/webapp/publisher.py'
--- lib/canonical/launchpad/webapp/publisher.py 2010-03-02 16:48:08 +0000
+++ lib/canonical/launchpad/webapp/publisher.py 2010-03-15 23:15:37 +0000
@@ -270,10 +270,6 @@
270 """270 """
271 return self.request.response.getStatus() in [301, 302, 303, 307]271 return self.request.response.getStatus() in [301, 302, 303, 307]
272272
273 def isRedirectInhibited(self):
274 """Returns True if redirection has been inhibited."""
275 return self.request.cookies.get('inhibit_beta_redirect', '0') == '1'
276
277 def __call__(self):273 def __call__(self):
278 self.initialize()274 self.initialize()
279 if self._isRedirected():275 if self._isRedirected():
280276
=== modified file 'lib/canonical/launchpad/webapp/servers.py'
--- lib/canonical/launchpad/webapp/servers.py 2010-03-02 03:31:42 +0000
+++ lib/canonical/launchpad/webapp/servers.py 2010-03-15 23:15:37 +0000
@@ -1,7 +1,7 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4# pylint: disable-msg=W02314# pylint: disable-msg=W0231,E1002
55
6"""Definition of the internet servers that Launchpad uses."""6"""Definition of the internet servers that Launchpad uses."""
77
@@ -579,6 +579,10 @@
579 """As per zope.publisher.browser.BrowserRequest._createResponse"""579 """As per zope.publisher.browser.BrowserRequest._createResponse"""
580 return LaunchpadBrowserResponse()580 return LaunchpadBrowserResponse()
581581
582 def isRedirectInhibited(self):
583 """Returns True if edge redirection has been inhibited."""
584 return self.cookies.get('inhibit_beta_redirect', '0') == '1'
585
582 @cachedproperty586 @cachedproperty
583 def form_ng(self):587 def form_ng(self):
584 """See ILaunchpadBrowserApplicationRequest."""588 """See ILaunchpadBrowserApplicationRequest."""
585589
=== modified file 'lib/canonical/launchpad/webapp/tests/test_servers.py'
--- lib/canonical/launchpad/webapp/tests/test_servers.py 2010-02-24 14:45:33 +0000
+++ lib/canonical/launchpad/webapp/tests/test_servers.py 2010-03-15 23:15:37 +0000
@@ -1,13 +1,14 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4# pylint: disable-msg=E1002
5
4__metaclass__ = type6__metaclass__ = type
57
6import StringIO8import StringIO
7import unittest9import unittest
810
9from zope.component import getGlobalSiteManager, getUtility11from zope.component import getGlobalSiteManager, getUtility
10from zope.publisher.base import DefaultPublication
11from zope.testing.doctest import DocTestSuite, NORMALIZE_WHITESPACE, ELLIPSIS12from zope.testing.doctest import DocTestSuite, NORMALIZE_WHITESPACE, ELLIPSIS
12from zope.interface import implements, Interface13from zope.interface import implements, Interface
1314
@@ -260,7 +261,6 @@
260 # named 'foo'.261 # named 'foo'.
261 class GenericCollection:262 class GenericCollection:
262 implements(IGenericCollection)263 implements(IGenericCollection)
263 pass
264264
265 class MyRootResource(RootResource):265 class MyRootResource(RootResource):
266 def _build_top_level_objects(self):266 def _build_top_level_objects(self):
@@ -469,6 +469,19 @@
469 "The query_string_params dict correctly interprets encoded "469 "The query_string_params dict correctly interprets encoded "
470 "parameters.")470 "parameters.")
471471
472 def test_isRedirectInhibited_without_cookie(self):
473 # When the request doesn't include the inhibit_beta_redirect cookie,
474 # isRedirectInhibited() returns False.
475 request = LaunchpadBrowserRequest('', {})
476 self.assertFalse(request.isRedirectInhibited())
477
478 def test_isRedirectInhibited_with_cookie(self):
479 # When the request includes the inhibit_beta_redirect cookie,
480 # isRedirectInhibited() returns True.
481 request = LaunchpadBrowserRequest(
482 '', dict(HTTP_COOKIE="inhibit_beta_redirect=1"))
483 self.assertTrue(request.isRedirectInhibited())
484
472485
473def test_suite():486def test_suite():
474 suite = unittest.TestSuite()487 suite = unittest.TestSuite()
475488
=== modified file 'lib/lp/app/templates/base-layout-macros.pt'
--- lib/lp/app/templates/base-layout-macros.pt 2010-03-12 03:53:50 +0000
+++ lib/lp/app/templates/base-layout-macros.pt 2010-03-15 23:15:37 +0000
@@ -393,11 +393,11 @@
393 </tal:site_message>393 </tal:site_message>
394 <tal:edge_only condition="is_edge">394 <tal:edge_only condition="is_edge">
395 <a href="#" class="js-action" onclick="setBetaRedirect(false)"395 <a href="#" class="js-action" onclick="setBetaRedirect(false)"
396 tal:condition="not:view/isRedirectInhibited">396 tal:condition="not:request/isRedirectInhibited">
397 Disable edge redirect.397 Disable edge redirect.
398 </a>398 </a>
399 <a href="#" class="js-action" onclick="setBetaRedirect(true)"399 <a href="#" class="js-action" onclick="setBetaRedirect(true)"
400 tal:condition="view/isRedirectInhibited">400 tal:condition="request/isRedirectInhibited">
401 Enable edge redirect.401 Enable edge redirect.
402 </a>402 </a>
403 </tal:edge_only>403 </tal:edge_only>