Code review comment for lp:~michael.nelson/launchpad/429353-site-message-to-footer

Revision history for this message
Michael Nelson (michael.nelson) wrote :

= Summary =

This branch addresses bug 429353 by moving the site_message into the
footer (for both pre-3.0 and 3-0 pages).

As requested on the bug, an extra link is included in the site message
for edge to disable redirection - iff (1) edge redirection is enabled,
(2) the user is a beta user, and (3) the user has not already got edge
redirection disabled. This enables users to disable edge redirection
without having to go back to the root page.

== Proposed fix ==

Initially we were simply going to include the link to disable
redirections in the site-message config option, but as I implemented it
I realised that this would mean it's always there, even when (1) the
user is not a beta user (or even logged in for that matter), or (2) when
the user has already disabled redirection.

== Pre-implementation notes ==

See https://bugs.launchpad.net/launchpad-foundations/+bug/429353

== Implementation details ==

I'm a bit uncertain about moving isRedirectInhibited() and canRedirect
from LaunchpadRootIndexView to LaunchpadView - but I can't see any other
way to enable this on every LP page.

== Tests ==

bin/test -vv -t launchpadview.txt -t demo-and-lpnet.txt -t
xx-beta-testers-redirection.txt

== Demo and Q/A ==

Because the local dev environment can't be run with redirects enabled,
getting the actual re-direct link to display is a pain, but it's
straight-forward enough to see what the site-message looks like in the
footer:

1. After merging, patch with http://pastebin.ubuntu.com/271505/ (note,
this patch will cause the above tests to fail, so revert it before testing).
2. Visit https://launchpad.dev/ubuntu and check the footer.

Q/A
1. Visit https://edge.launchpad.net/ubuntu and verify the site-message
has the option to disable redirects.
2. Click the link to disable the redirect, and verify that it's worked.
3. Re-visit https://edge.launchpad.net/ubuntu and verify that the
site-message does not include the link to redirect.
4. Re-enable redirections via the launchpad.net homepage.
5. Re-affirm that the site-message now includes the link to disable again.

= 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/icing/style.css
  lib/lp/registry/browser/root.py
  lib/canonical/launchpad/webapp/publisher.py
  lib/canonical/launchpad/templates/main-template.pt
  lib/lp/app/templates/base-layout.pt
  lib/lp/app/templates/base-layout-macros.pt
  lib/canonical/launchpad/doc/launchpadview.txt
  lib/canonical/launchpad/pagetests/basics/demo-and-lpnet.txt

lib/canonical/launchpad/pagetests/standalone/xx-beta-testers-redirection.txt

== Pylint notices ==

lib/lp/registry/browser/root.py
    40: [F0401] Unable to import 'lazr.batchnavigator.z3batching' (No
module named batchnavigator)
    473: [E1002, WindowedListBatch.__iter__] Use super on an old style class

--
Michael

« Back to merge proposal