Code review comment for lp:~salgado/launchpad/real-breadcrumbs-2

Revision history for this message
Guilherme Salgado (salgado) wrote :

= Summary =

In 3.0 we'll have real breadcrumbs, and this branch will make it
possible to construct them.

== Proposed fix ==

Basically, what this branch does is have the URL of all breadcrumbs
default to the object's canonical_url on the mainsite, as well as
sneaking an extra breadcrumb item for the object on the vhost we are on
(in case we are on a vhost other than the mainsite).

E.g. the breadcrumbs for bugs.lp.net/ubuntu/+bug/1 will be
  Ubuntu > Bugs on Ubuntu > Bug #1

This is accomplished by looking up IBreadcrumbBuilder adapters with
name=vhost for the traversed objects, starting from the end. When one is
found, we use it to generate the extra breadcrumb and insert the
breadcrumb right after the current one.

For some types of objects we will not want the URL on their breadcrumbs
to point to the mainsite, so they can just specify the rootsite they
want in their custom IBreadcrumbBuilder adapter.

== Pre-implementation notes ==

== Implementation details ==

In lib/canonical/launchpad/webapp/publisher.py I reorganized some code
slightly to make it more readable.

The XXX I left in lib/canonical/launchpad/webapp/breadcrumb.py I'm
planning to fix as soon as I get this through review. I didn't do it
here as it would pollute the diff.

== Tests ==

./bin/test -vvt test_breadcrumbs -t hierarchical-menu.txt

== Demo and Q/A ==

https://bugs.launchpad.dev/firefox/+bug/1

= Launchpad lint =

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

Linting changed files:
  lib/lp/code/browser/branch.py
  lib/canonical/launchpad/webapp/publisher.py
  lib/canonical/launchpad/webapp/tests/test_breadcrumbs.py
  lib/lp/registry/configure.zcml
  lib/canonical/launchpad/browser/launchpad.py
  lib/canonical/launchpad/webapp/breadcrumb.py
  lib/lp/registry/browser/pillar.py
  lib/canonical/lazr/testing/menus.py
  lib/canonical/launchpad/doc/hierarchical-menu.txt

== Pylint notices ==

lib/lp/code/browser/branch.py
    48: [F0401] Unable to import 'lazr.uri' (No module named uri)

lib/canonical/launchpad/browser/launchpad.py
    103: [F0401] Unable to import 'lazr.uri' (No module named uri)

--
Guilherme Salgado <email address hidden>

« Back to merge proposal