Launchpad itself

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

  1. bug-538207
  2. Merge into devel
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
1=== modified file 'lib/canonical/launchpad/doc/launchpadview.txt'
2--- lib/canonical/launchpad/doc/launchpadview.txt 2009-10-21 23:51:08 +0000
3+++ lib/canonical/launchpad/doc/launchpadview.txt 2010-03-15 23:15:37 +0000
4@@ -102,13 +102,3 @@
5 >>> view.error_message = structured('Information overload.')
6 >>> view.error_message.escapedtext
7 u'Information overload.'
8-
9-Every Launchpad view also knows whether edge redirection has been inhibited.
10-
11- >>> view.isRedirectInhibited()
12- False
13- >>> new_request = LaunchpadTestRequest(
14- ... HTTP_COOKIE="inhibit_beta_redirect=1")
15- >>> view = MyView(context, new_request)
16- >>> view.isRedirectInhibited()
17- True
18
19=== modified file 'lib/canonical/launchpad/templates/oops.pt'
20--- lib/canonical/launchpad/templates/oops.pt 2010-03-10 19:10:04 +0000
21+++ lib/canonical/launchpad/templates/oops.pt 2010-03-15 23:15:37 +0000
22@@ -11,7 +11,7 @@
23 </style>
24 <script type="text/javascript"
25 tal:content="string:var cookie_scope = '${request/lp:cookie_scope}';"></script>
26- <script type="text/javascript" src="/+icing/rev5/build/lp/lp.js" />
27+ <script type="text/javascript" src="/+icing/rev5/build/lp/lp.js"></script>
28 </head>
29 <body>
30 <div class="yui-d0">
31
32=== modified file 'lib/canonical/launchpad/webapp/publisher.py'
33--- lib/canonical/launchpad/webapp/publisher.py 2010-03-02 16:48:08 +0000
34+++ lib/canonical/launchpad/webapp/publisher.py 2010-03-15 23:15:37 +0000
35@@ -270,10 +270,6 @@
36 """
37 return self.request.response.getStatus() in [301, 302, 303, 307]
38
39- def isRedirectInhibited(self):
40- """Returns True if redirection has been inhibited."""
41- return self.request.cookies.get('inhibit_beta_redirect', '0') == '1'
42-
43 def __call__(self):
44 self.initialize()
45 if self._isRedirected():
46
47=== modified file 'lib/canonical/launchpad/webapp/servers.py'
48--- lib/canonical/launchpad/webapp/servers.py 2010-03-02 03:31:42 +0000
49+++ lib/canonical/launchpad/webapp/servers.py 2010-03-15 23:15:37 +0000
50@@ -1,7 +1,7 @@
51 # Copyright 2009 Canonical Ltd. This software is licensed under the
52 # GNU Affero General Public License version 3 (see the file LICENSE).
53
54-# pylint: disable-msg=W0231
55+# pylint: disable-msg=W0231,E1002
56
57 """Definition of the internet servers that Launchpad uses."""
58
59@@ -579,6 +579,10 @@
60 """As per zope.publisher.browser.BrowserRequest._createResponse"""
61 return LaunchpadBrowserResponse()
62
63+ def isRedirectInhibited(self):
64+ """Returns True if edge redirection has been inhibited."""
65+ return self.cookies.get('inhibit_beta_redirect', '0') == '1'
66+
67 @cachedproperty
68 def form_ng(self):
69 """See ILaunchpadBrowserApplicationRequest."""
70
71=== modified file 'lib/canonical/launchpad/webapp/tests/test_servers.py'
72--- lib/canonical/launchpad/webapp/tests/test_servers.py 2010-02-24 14:45:33 +0000
73+++ lib/canonical/launchpad/webapp/tests/test_servers.py 2010-03-15 23:15:37 +0000
74@@ -1,13 +1,14 @@
75 # Copyright 2009 Canonical Ltd. This software is licensed under the
76 # GNU Affero General Public License version 3 (see the file LICENSE).
77
78+# pylint: disable-msg=E1002
79+
80 __metaclass__ = type
81
82 import StringIO
83 import unittest
84
85 from zope.component import getGlobalSiteManager, getUtility
86-from zope.publisher.base import DefaultPublication
87 from zope.testing.doctest import DocTestSuite, NORMALIZE_WHITESPACE, ELLIPSIS
88 from zope.interface import implements, Interface
89
90@@ -260,7 +261,6 @@
91 # named 'foo'.
92 class GenericCollection:
93 implements(IGenericCollection)
94- pass
95
96 class MyRootResource(RootResource):
97 def _build_top_level_objects(self):
98@@ -469,6 +469,19 @@
99 "The query_string_params dict correctly interprets encoded "
100 "parameters.")
101
102+ def test_isRedirectInhibited_without_cookie(self):
103+ # When the request doesn't include the inhibit_beta_redirect cookie,
104+ # isRedirectInhibited() returns False.
105+ request = LaunchpadBrowserRequest('', {})
106+ self.assertFalse(request.isRedirectInhibited())
107+
108+ def test_isRedirectInhibited_with_cookie(self):
109+ # When the request includes the inhibit_beta_redirect cookie,
110+ # isRedirectInhibited() returns True.
111+ request = LaunchpadBrowserRequest(
112+ '', dict(HTTP_COOKIE="inhibit_beta_redirect=1"))
113+ self.assertTrue(request.isRedirectInhibited())
114+
115
116 def test_suite():
117 suite = unittest.TestSuite()
118
119=== modified file 'lib/lp/app/templates/base-layout-macros.pt'
120--- lib/lp/app/templates/base-layout-macros.pt 2010-03-12 03:53:50 +0000
121+++ lib/lp/app/templates/base-layout-macros.pt 2010-03-15 23:15:37 +0000
122@@ -393,11 +393,11 @@
123 </tal:site_message>
124 <tal:edge_only condition="is_edge">
125 <a href="#" class="js-action" onclick="setBetaRedirect(false)"
126- tal:condition="not:view/isRedirectInhibited">
127+ tal:condition="not:request/isRedirectInhibited">
128 Disable edge redirect.
129 </a>
130 <a href="#" class="js-action" onclick="setBetaRedirect(true)"
131- tal:condition="view/isRedirectInhibited">
132+ tal:condition="request/isRedirectInhibited">
133 Enable edge redirect.
134 </a>
135 </tal:edge_only>