Merge lp:~salgado/launchpad/edge-redirect-bugs into lp:launchpad

[Guilherme Salgado]

This branch fixes bug 403863, bug 512295 and bug 528184.

I've changed lib/lp/app/templates/base-layout-macros.pt to always show
the enable/disable redirect link when is_edge is True.

In oops.pt I added a notification message (together with a button to
disable redirects) when we're on edge. Also had to include lp.js, for
the JS function that actually disables redirection (setBetaRedirect).
Need to do all this here because oops.pt doesn't use our main template.

And in launchpad-requestexpired.pt I replaced the JS that would show a
hidden (display:none) div when it was seen on edge with a tal:condition
around that div. However, thanks to the change to base-layout-macros.pt
above, the timeout page now has one link to disable redirection at the
bottom and one button for the same purpose at the top. I think we
should remove the notification and leave just the link at the bottom.

== Demo and Q/A ==

First, set is_edge to true on configs/schema-lazr.conf.

To see the new notification on the OOPS page you need to manually edit a
template so that it crashes when rendering and load a page that uses
that template. This will cause an OOPS and you should see the
notification.

To see it on the timeout page, set db_statement_timeout to 1 in
configs/schema-lazr.conf and open any page.

= 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/templates/oops.pt
  lib/canonical/launchpad/webapp/error.py
  lib/canonical/launchpad/doc/timeout.txt
  lib/lp/app/templates/base-layout-macros.pt
  lib/canonical/launchpad/templates/launchpad-requestexpired.pt
  lib/canonical/launchpad/pagetests/basics/demo-and-lpnet.txt
  lib/lp/app/stories/basics/xx-beta-testers-redirection.txt

[Gavin Panella (allenap)]

Hi,

The "Disable edge redirect" message is only shown when is_edge is true
*and* site_message is set. The cover letter didn't mention that, but I
assume that's fine.

I have a few comments, but nothing terrible :)

Gavin.

> === modified file 'lib/canonical/launchpad/doc/timeout.txt'
> --- lib/canonical/launchpad/doc/timeout.txt 2008-05-17 02:36:00 +0000
> +++ lib/canonical/launchpad/doc/timeout.txt 2010-03-10 16:32:41 +0000
> @@ -12,7 +12,7 @@
> >>> from canonical.lazr.timeout import get_default_timeout_function
> >>> from canonical.launchpad.webapp.servers import (
> ... set_launchpad_default_timeout)
> - >>> old_func = get_default_timeout_function()
> + >>> old_func = get_default_timeout_function()
>
> >>> from zope.app.appsetup import ProcessStarting
>
>
> === modified file 'lib/canonical/launchpad/pagetests/basics/demo-and-lpnet.txt'
> --- lib/canonical/launchpad/pagetests/basics/demo-and-lpnet.txt 2010-02-09 17:25:13 +0000
> +++ lib/canonical/launchpad/pagetests/basics/demo-and-lpnet.txt 2010-03-10 16:32:41 +0000
> @@ -103,16 +103,18 @@
> ... 'a', onclick="setBetaRedirect(false)"))
> Disable edge redirect.
>
> -The disable-redirect link will not appear in the site_message when
> -browsed by non-beta users.
> +The disable-redirect link will also appear in the site_message when browsed by
> +non-beta/anonymous users. This is to reduce the annoyance when users are
> +logged into launchpad.net but haven't noticed yet that they need to log into
> +edge as well (https://launchpad.net/bugs/160191).
>
> >>> browser.open('http://launchpad.dev/ubuntu')
> >>> print extract_text(find_tags_by_class(
> ... browser.contents, 'sitemessage')[0])
> - This is a beta site.
> + This is a beta site. Disable edge redirect.
>
> -Similarly, once the redirection has been inhibited, the link changes to
> -enable redirects..
> +Once the redirection has been inhibited, the link changes to enable
> +redirects.
>
> # Workaround bug in mechanize where you cannot use the Cookie
> # header with the CookieJar
> @@ -135,7 +137,6 @@
> ... 'a', onclick="setBetaRedirect(true)"))
> Enable edge redirect.
>
> -
> # Remove the specific site-message config data before continuing.
> >>> dummy = config.pop('edge_config_data')
>
>
> === modified file 'lib/canonical/launchpad/templates/launchpad-requestexpired.pt'
> --- lib/canonical/launchpad/templates/launchpad-requestexpired.pt 2009-07-28 17:19:42 +0000
> +++ lib/canonical/launchpad/templates/launchpad-requestexpired.pt 2010-03-10 16:32:41 +0000
> @@ -9,27 +9,16 @@
> <body>
> <div class="top-portlet" metal:fill-slot="main">
> <h1 class="exception">Timeout error</h1>
> - <div
> - id="redirect_notice" style="display:none" class="informational message">
> - <p>Our edge server has a lower timeout threshold than launchpad.net,
> - so we can catch those before they hit a wider audience.
> - As a member of the Launchpad Beta Testers team, you're more
> - likely to experience them. If this is blocking your work, you
> - can disable redirection.</p>
> - <...

review: Approve (code)

[Guilherme Salgado (salgado)]

On Wed, 2010-03-10 at 17:06 +0000, Gavin Panella wrote:
> Review: Approve code
> Hi,
>
> The "Disable edge redirect" message is only shown when is_edge is true
> *and* site_message is set. The cover letter didn't mention that, but I
> assume that's fine.

Indeed, I forgot to mention that.

Thanks for the review!

>
> I have a few comments, but nothing terrible :)
>

> > === modified file 'lib/canonical/launchpad/templates/launchpad-requestexpired.pt'
> > --- lib/canonical/launchpad/templates/launchpad-requestexpired.pt 2009-07-28 17:19:42 +0000
> > +++ lib/canonical/launchpad/templates/launchpad-requestexpired.pt 2010-03-10 16:32:41 +0000
> > @@ -9,27 +9,16 @@
> > <body>
> > <div class="top-portlet" metal:fill-slot="main">
> > <h1 class="exception">Timeout error</h1>
> > - <div
> > - id="redirect_notice" style="display:none" class="informational message">
> > - <p>Our edge server has a lower timeout threshold than launchpad.net,
> > - so we can catch those before they hit a wider audience.
> > - As a member of the Launchpad Beta Testers team, you're more
> > - likely to experience them. If this is blocking your work, you
> > - can disable redirection.</p>
> > - <p><button onclick="setBetaRedirect(false)">Disable redirection
> > - for 2 hours</button></p>
> > + <div tal:condition="is_edge"
> > + id="redirect_notice" class="informational message">
> > + <p>Our edge server has a lower timeout threshold than launchpad.net,
> > + so we can catch those before they hit a wider audience.
> > + As a member of the Launchpad Beta Testers team, you're more
>
> You haven't changed it, but this isn't strictly true - that the viewer
> is a beta tester - any more is it? Doesn't matter much though.

You're right. How about this:

  <p>Our edge server has a lower timeout threshold than launchpad.net,
  so we can catch those before they hit a wider audience.
  If this is blocking your work, you can disable automatic redirection
  to edge in order to use launchpad.net.</p>
?

>
> > + likely to experience them. If this is blocking your work, you
> > + can disable redirection.</p>
> > + <p><button onclick="setBetaRedirect(false)">Disable redirection
> > + for 2 hours</button></p>
> > </div>
> > - <script type="text/javascript">
> > - var edge_cookie = document.cookie.match('edge');
> > - var edge_host = document.location.hostname.match('edge');
> > -
> > - // Logic has been inverted in the next line to avoid breaking
> > - // XHTML compliance of the template due to ampersand usage.
> > - if (!(edge_cookie || edge_host)) {
> > - redirect_notice_div = document.getElementById('redirect_notice');
> > - redirect_notice_div.style.display = "block";
> > - };
> > - </script>
> > <p>
> > Sorry, something just went wrong in Launchpad.
> > </p>
> >
> > === modified file 'lib/canonical/launchpad/templates/oops.pt'
> > --- lib/canonical/launchpad/templates/oops.pt 2010-01-13 17:58:30 +0000
> > +++ lib/canonical/launchpad/te...

[Guilherme Salgado (salgado)]

On Wed, 2010-03-10 at 14:42 -0300, Guilherme Salgado wrote:
> > > === modified file 'lib/canonical/launchpad/templates/launchpad-requestexpired.pt'
> > > --- lib/canonical/launchpad/templates/launchpad-requestexpired.pt 2009-07-28 17:19:42 +0000
> > > +++ lib/canonical/launchpad/templates/launchpad-requestexpired.pt 2010-03-10 16:32:41 +0000
> > > @@ -9,27 +9,16 @@
> > > <body>
> > > <div class="top-portlet" metal:fill-slot="main">
> > > <h1 class="exception">Timeout error</h1>
> > > - <div
> > > - id="redirect_notice" style="display:none" class="informational message">
> > > - <p>Our edge server has a lower timeout threshold than launchpad.net,
> > > - so we can catch those before they hit a wider audience.
> > > - As a member of the Launchpad Beta Testers team, you're more
> > > - likely to experience them. If this is blocking your work, you
> > > - can disable redirection.</p>
> > > - <p><button onclick="setBetaRedirect(false)">Disable redirection
> > > - for 2 hours</button></p>
> > > + <div tal:condition="is_edge"
> > > + id="redirect_notice" class="informational message">
> > > + <p>Our edge server has a lower timeout threshold than launchpad.net,
> > > + so we can catch those before they hit a wider audience.
> > > + As a member of the Launchpad Beta Testers team, you're more
> >
> > You haven't changed it, but this isn't strictly true - that the viewer
> > is a beta tester - any more is it? Doesn't matter much though.
>
> You're right. How about this:
>
> <p>Our edge server has a lower timeout threshold than launchpad.net,
> so we can catch those before they hit a wider audience.
> If this is blocking your work, you can disable automatic redirection
> to edge in order to use launchpad.net.</p>
> ?

Or maybe

    If this is blocking your work and you are a member of the Launchpad
    Beta Testers team, you can disable automatic redirection
    to edge in order to use launchpad.net.</p>

[Gavin Panella (allenap)]

I think that reads well, and the second version is better. It might
be nice if "disable automatic redirection" in the paragraph was also
clickable, but I suspect that enough work has been done on this
already.

=== modified file 'lib/canonical/launchpad/doc/timeout.txt'
--- lib/canonical/launchpad/doc/timeout.txt	2008-05-17 02:36:00 +0000
+++ lib/canonical/launchpad/doc/timeout.txt	2010-03-10 19:12:31 +0000
@@ -12,7 +12,7 @@
     >>> from canonical.lazr.timeout import get_default_timeout_function
     >>> from canonical.launchpad.webapp.servers import (
     ...     set_launchpad_default_timeout)
-    >>> old_func =  get_default_timeout_function()
+    >>> old_func = get_default_timeout_function()
 
     >>> from zope.app.appsetup import ProcessStarting
 
 
=== modified file 'lib/canonical/launchpad/pagetests/basics/demo-and-lpnet.txt'
--- lib/canonical/launchpad/pagetests/basics/demo-and-lpnet.txt	2010-02-09 17:25:13 +0000
+++ lib/canonical/launchpad/pagetests/basics/demo-and-lpnet.txt	2010-03-10 19:12:31 +0000
@@ -103,16 +103,18 @@
     ...     'a', onclick="setBetaRedirect(false)"))
     Disable edge redirect.
 
-The disable-redirect link will not appear in the site_message when
-browsed by non-beta users.
+The disable-redirect link will also appear in the site_message when browsed by
+non-beta/anonymous users. This is to reduce the annoyance when users are
+logged into launchpad.net but haven't noticed yet that they need to log into
+edge as well (https://launchpad.net/bugs/160191).
 
     >>> browser.open('http://launchpad.dev/ubuntu')
     >>> print extract_text(find_tags_by_class(
     ...     browser.contents, 'sitemessage')[0])
-    This is a beta site.
+    This is a beta site. Disable edge redirect.
 
-Similarly, once the redirection has been inhibited, the link changes to
-enable redirects..
+Once the redirection has been inhibited, the link changes to enable
+redirects.
 
     # Workaround bug in mechanize where you cannot use the Cookie
     # header with the CookieJar
@@ -135,7 +137,6 @@
     ...     'a', onclick="setBetaRedirect(true)"))
     Enable edge redirect.
 
-
     # Remove the specific site-message config data before continuing.
     >>> dummy = config.pop('edge_config_data')
 
 
=== modified file 'lib/canonical/launchpad/templates/launchpad-requestexpired.pt'
--- lib/canonical/launchpad/templates/launchpad-requestexpired.pt	2009-07-28 17:19:42 +0000
+++ lib/canonical/launchpad/templates/launchpad-requestexpired.pt	2010-03-10 19:12:31 +0000
@@ -9,27 +9,16 @@
   <body>
     <div class="top-portlet" metal:fill-slot="main">
       <h1 class="exception">Timeout error</h1>
-      <div
-        id="redirect_notice" style="display:none" class="informational message">
-          <p>Our edge server has a lower timeout threshold than launchpad.net,
-          so we can catch those before they hit a wider audience.
-          As a member of the Launchpad Beta Testers team, you're more
-          likely to experience them. If this is blocking your work, you
-          can disable redirection.</p>
-          <p><button onclick="setBetaRedirect(false)">Disable redirection
-          for 2 hours</button></p>
+      <div tal:condition="is_edge"
+           id="redirect_notice" class="informational message">
+        <p>Our edge server has a lower timeout threshold than launchpad.net,
+        so we can catch those before they hit a wider audience.
+        If this is blocking your work and you are a member of the Launchpad
+        Beta Testers team, you can disable automatic redirection
+        to edge in order to use launchpad.net.</p>
+        <p><button onclick="setBetaRedirect(false)">Disable redirection
+        for 2 hours</button></p>
       </div>
-      <script type="text/javascript">
-        var edge_cookie = document.cookie.match('edge');
-        var edge_host = document.location.hostname.match('edge');
-
-	      // Logic has been inverted in the next line to avoid breaking
-	      // XHTML compliance of the template due to ampersand usage.
-        if (!(edge_cookie || edge_host)) {
-          redirect_notice_div = document.getElementById('redirect_notice');
-          redirect_notice_div.style.display = "block";
-        };
-      </script>
       <p>
         Sorry, something just went wrong in Launchpad.
       </p>
 
=== modified file 'lib/canonical/launchpad/templates/oops.pt'
--- lib/canonical/launchpad/templates/oops.pt	2010-01-13 17:58:30 +0000
+++ lib/canonical/launchpad/templates/oops.pt	2010-03-10 19:12:31 +0000
@@ -9,6 +9,9 @@
     <style type="text/css" media="screen, print">
       @import url(/+icing/rev5/combo.css);
     </style>
+    <script type="text/javascript"
+        tal:content="string:var cookie_scope = '${request/lp:cookie_scope}';"></script>
+    <script type="text/javascript" src="/+icing/rev5/build/lp/lp.js" />
   </head>
   <body>
     <div class="yui-d0">
@@ -27,6 +30,27 @@
           (Error <abbr>ID</abbr>:
           <tal:oops replace="structure view/oops_id_text" />)
         </p>
+
+        <div id="redirect_notice" style="display:none"
+             class="informational message">
+          <p>This server runs pre-release code, so it's possible this problem
+          doesn't affect launchpad.net. If you're a member of the Launchpad
+          Beta Testers team, you can disable redirection in order to use
+          launchpad.net.</p>
+          <p><button onclick="setBetaRedirect(false)">Disable redirection
+          for 2 hours</button></p>
+        </div>
+        <tal:comment condition="nothing">
+          Can't use the 'is_edge' global because we don't use our page
+          macros here.
+        </tal:comment>
+        <script type="text/javascript">
+          if (document.location.hostname.match('edge.')) {
+            redirect_notice_div = document.getElementById('redirect_notice');
+            redirect_notice_div.style.display = "block";
+          };
+        </script>
+
         <tal:traceback replace="structure view/maybeShowTraceback" />
 
         <div class="related">
 
=== modified file 'lib/canonical/launchpad/webapp/error.py'
--- lib/canonical/launchpad/webapp/error.py	2009-12-23 16:17:12 +0000
+++ lib/canonical/launchpad/webapp/error.py	2010-03-10 19:12:31 +0000
@@ -26,12 +26,13 @@
 from canonical.cachedproperty import cachedproperty
 from canonical.config import config
 import canonical.launchpad.layers
+from canonical.launchpad.webapp.publisher import LaunchpadView
 from canonical.launchpad.webapp.adapter import (
     clear_request_started, set_request_started)
 from canonical.launchpad.webapp.interfaces import ILaunchBag
 
 
-class SystemErrorView:
+class SystemErrorView(LaunchpadView):
     """Helper class for views on exceptions.
 
     Also, sets a 500 response code.
@@ -64,8 +65,7 @@
     safe_to_show_in_restricted_mode = False
 
     def __init__(self, context, request):
-        self.context = context
-        self.request = request
+        super(SystemErrorView, self).__init__(context, request)
         self.request.response.removeAllNotifications()
         if self.response_code is not None:
             self.request.response.setStatus(self.response_code)
 
=== modified file 'lib/lp/app/stories/basics/xx-beta-testers-redirection.txt'
--- lib/lp/app/stories/basics/xx-beta-testers-redirection.txt	2010-02-09 17:25:13 +0000
+++ lib/lp/app/stories/basics/xx-beta-testers-redirection.txt	2010-03-10 19:12:31 +0000
@@ -216,11 +216,12 @@
 
     >>> logout()
 
-Decrease the timeout values for launchpad.dev
+Decrease the timeout values for launchpad.dev and pretend we're on the edge
+server.
 
     >>> beta_data = """
     ...     [launchpad]
-    ...     beta_testers_redirection_host = edge.launchpad.dev
+    ...     is_edge: True
     ...     [database]
     ...     db_statement_timeout: 1
     ...     soft_request_timeout: 2
@@ -240,15 +241,14 @@
     <title>Error: Timeout</title>
     ...
     <h1 class="exception">Timeout error</h1>
-    <div id="redirect_notice" style="display:none" class="informational message">
+    ...
     <p>Our edge server has a lower timeout threshold than launchpad.net,
     so we can catch those before they hit a wider audience.
-    As a member of the Launchpad Beta Testers team, you're more
-    likely to experience them. If this is blocking your work, you
-    can disable redirection.</p>
+    If this is blocking your work and you are a member of the Launchpad
+    Beta Testers team, you can disable automatic redirection
+    to edge in order to use launchpad.net.</p>
     <p><button onclick="setBetaRedirect(false)">Disable redirection
     for 2 hours</button></p>
-    </div>
     ...
 
 Restore the config to state it was in at the start of the test.
 
=== modified file 'lib/lp/app/templates/base-layout-macros.pt'
--- lib/lp/app/templates/base-layout-macros.pt	2010-03-05 02:57:25 +0000
+++ lib/lp/app/templates/base-layout-macros.pt	2010-03-10 19:12:31 +0000
@@ -379,16 +379,14 @@
       This site is running pre-release code.
     </tal:site_message>
     <tal:edge_only condition="is_edge">
-      <tal:beta_user condition="view/macro:isbetauser">
-        <a href="#" class="js-action" onclick="setBetaRedirect(false)"
-           tal:condition="not:view/isRedirectInhibited">
-          Disable edge redirect.
-        </a>
-        <a href="#" class="js-action" onclick="setBetaRedirect(true)"
-           tal:condition="view/isRedirectInhibited">
-          Enable edge redirect.
-        </a>
-      </tal:beta_user>
+      <a href="#" class="js-action" onclick="setBetaRedirect(false)"
+         tal:condition="not:view/isRedirectInhibited">
+        Disable edge redirect.
+      </a>
+      <a href="#" class="js-action" onclick="setBetaRedirect(true)"
+         tal:condition="view/isRedirectInhibited">
+        Enable edge redirect.
+      </a>
     </tal:edge_only>
   </div>
 </metal:site-message>