launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/breadcrumbs-and-stepthroughs into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/breadcrumbs-and-stepthroughs/+merge/102942

Summary
=======
Breadcrumbs get broken whenever we use the stepthrough decorater (e.g.
+sharing/$personname, +archivesubscriptions/$archiveid). This is because
stepthrough traversal doesn't add anything to the publishers
traveresed_objects, which IHeirarchy uses to produce the breadcrumbs.

The solution is to add a placeholder object into traversed_objects when we
have a stepthrough_traveral.

Preimp
======
Spoke with Curtis Hovey.

Implementation
==============
The easiest thing to insert into traversed_objects when we have no actual
object is a breadcrumb. This avoids the difficulty of creating an object that
can accurately adapt to breadcrumb, as we can set the breadcrumb data
directly. To that end, Breadcrumb has been updated so that __init__ can take a
url and a text parameter (both default to None). If provided, the breadcrumb
sets its information from these params.

The publisher looks up the url and the page_title for the view that would
normally be presented if the stepthrough were the endpoint of traversal (as
stepthroughs are viewnames). It creates a breadcrumb object with this
information and adds it to traversed_objects. Breadcrumbs can obviously be
adapted via IBreadcrumb, so IHeirarchy is able to create a breadcrumb for
these stepthroughs.

Tests
=====
I could find no proper way to test this that was not in fact a full
integration test. Testing that this works requires a significant amount of
infrastructure in place since it is a result of the interaction of several
objects. The stepthrough functions, breadcrumb behavior, and traversal
behavior are all pretty thoroughly tested, and should detect any problems.

If the reviewer has suggestions for tests, I'm *happy* to implement them.

QA
==
Go to qastaging.launchpad.net/launchpad/+sharing and click a "View details"
link in the table. The resulting view
(qastaging.launchpad.net/launchpad/+sharing/$person) should have

Launchpad >> Sharing >> Personname

as its breadcrumbs.

Lint
====

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/services/webapp/publisher.py
  lib/lp/services/webapp/breadcrumb.py
-- 
https://code.launchpad.net/~jcsackett/launchpad/breadcrumbs-and-stepthroughs/+merge/102942
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/breadcrumbs-and-stepthroughs into lp:launchpad.

=== modified file 'lib/lp/services/webapp/breadcrumb.py'
--- lib/lp/services/webapp/breadcrumb.py	2012-02-21 18:35:14 +0000
+++ lib/lp/services/webapp/breadcrumb.py	2012-04-20 21:28:26 +0000
@@ -33,8 +33,12 @@
     _detail = None
     _url = None
 
-    def __init__(self, context):
+    def __init__(self, context, url=None, text=None):
         self.context = context
+        if url is not None:
+            self._url = url
+        if text is not None:
+            self.text = text
 
     @property
     def rootsite(self):

=== modified file 'lib/lp/services/webapp/publisher.py'
--- lib/lp/services/webapp/publisher.py	2012-04-13 18:02:59 +0000
+++ lib/lp/services/webapp/publisher.py	2012-04-20 21:28:26 +0000
@@ -905,6 +905,7 @@
                     nextobj = handler(self)
                 except NotFoundError:
                     nextobj = None
+
                 return self._handle_next_object(nextobj, request, name)
 
         # Next, see if we have at least two path steps in total to traverse;
@@ -929,6 +930,21 @@
                             nextobj = handler(self, nextstep)
                         except NotFoundError:
                             nextobj = None
+                        else:
+                            # Circular import; breaks make.
+                            from lp.services.webapp.breadcrumb import Breadcrumb
+
+                            stepthrough_page = queryMultiAdapter(
+                                (self.context, self.request), name=name)
+                            stepthrough_text = stepthrough_page.page_title
+                            stepthrough_url = canonical_url(
+                                self.context, view_name=name)
+                            stepthrough_breadcrumb = Breadcrumb(
+                                context=self.context,
+                                url=stepthrough_url,
+                                text=stepthrough_text)
+                            self.request.traversed_objects.append(
+                                stepthrough_breadcrumb)
                         return self._handle_next_object(nextobj, request,
                             nextstep)