Code review comment for lp:~michael.nelson/launchpad/sprint-index-and-attend-3.0

Michael Nelson (michael.nelson) wrote :

Michael Nelson wrote:
> Barry Warsaw wrote:
>> Review: Approve code ui*
> Thanks Barry - incremental attached.
>
> I'll send it off to ec2test now, but not to land, just in case you're
> around and have any changes. I'll land it if the tests all succeed.
>
>
>

Hi Barry,

My ec2test failed (an error in devel) and when I re-merged RF this
morning, I found that, due to the work-around that we added (setting
display_breadcrumbs to False for SprintIndexHierarchy) that I was also
affecting most other sprint views (which had perfect breadcrumbs
otherwise - but none with this fix).

I created:

https://bugs.edge.launchpad.net/launchpad-foundations/+bug/433852

and have included a fix in this branch. This enables us to correctly
suppress the breadcrumbs when the view provides IMajorHeadingView, but
display them for other sub-pages (that share the same context).

Let me know if you agree or not. I'll then try to get an RC to get these
landed.

Thanks!

--
Michael

1=== modified file 'lib/canonical/launchpad/browser/launchpad.py'
2--- lib/canonical/launchpad/browser/launchpad.py 2009-09-20 19:40:47 +0000
3+++ lib/canonical/launchpad/browser/launchpad.py 2009-09-21 12:43:33 +0000
4@@ -52,6 +52,7 @@
5 from canonical.launchpad.helpers import intOrZero
6 from canonical.launchpad.layers import WebServiceLayer
7
8+from lp.app.interfaces.headings import IMajorHeadingView
9 from lp.registry.interfaces.announcement import IAnnouncementSet
10 from lp.soyuz.interfaces.binarypackagename import (
11 IBinaryPackageNameSet)
12@@ -259,6 +260,12 @@
13 breadcrumbs.append(page_crumb)
14 return breadcrumbs
15
16+ @property
17+ def _context_view(self):
18+ """Return the unproxied view for the context of the hierarchy."""
19+ from zope.security.proxy import removeSecurityProxy
20+ return removeSecurityProxy(self.request.traversed_objects[-1])
21+
22 def makeBreadcrumbForRequestedPage(self):
23 """Return an `IBreadcrumb` for the requested page.
24
25@@ -269,10 +276,9 @@
26 one for our parent view's context, return None.
27 """
28 url = self.request.getURL()
29- from zope.security.proxy import removeSecurityProxy
30- view = removeSecurityProxy(self.request.traversed_objects[-1])
31 obj = self.request.traversed_objects[-2]
32 default_view_name = zapi.getDefaultViewName(obj, self.request)
33+ view = self._context_view
34 if view.__name__ != default_view_name:
35 title = getattr(view, 'page_title', None)
36 if title is None:
37@@ -296,7 +302,10 @@
38 """Return whether the breadcrumbs should be displayed."""
39 # If there is only one breadcrumb then it does not make sense
40 # to display it as it will simply repeat the context.title.
41- return len(self.items) > 1
42+ # If the view is an IMajorHeadingView then we do not want
43+ # to display breadcrumbs either.
44+ return (len(self.items) > 1 and
45+ not IMajorHeadingView.providedBy(self._context_view))
46
47
48 class MaintenanceMessage:
49
50=== modified file 'lib/canonical/launchpad/doc/hierarchical-menu.txt'
51--- lib/canonical/launchpad/doc/hierarchical-menu.txt 2009-09-18 12:00:56 +0000
52+++ lib/canonical/launchpad/doc/hierarchical-menu.txt 2009-09-21 12:28:14 +0000
53@@ -143,6 +143,25 @@
54 >>> cooker_hierarchy.display_breadcrumbs
55 False
56
57+Additionally, if the view implements IMajorHeadingView then the breadcrumbs
58+will not be displayed.
59+
60+ >>> ham_recipe = Recipe('ham', cookbook)
61+ >>> ham_request = make_fake_request(
62+ ... 'http://launchpad.dev/joy-of-cooking/ham',
63+ ... [root, cookbook, ham_recipe])
64+
65+ >>> ham_hierarchy = getMultiAdapter(
66+ ... (ham_recipe, ham_request), name='+hierarchy')
67+ >>> hierarchy.display_breadcrumbs
68+ True
69+
70+ >>> from zope.interface import alsoProvides
71+ >>> from lp.app.interfaces.headings import IMajorHeadingView
72+ >>> alsoProvides(ham_recipe, IMajorHeadingView)
73+ >>> ham_hierarchy.display_breadcrumbs
74+ False
75+
76
77 == Building IBreadcrumb objects ==
78
79
80=== modified file 'lib/lp/blueprints/browser/configure.zcml'
81--- lib/lp/blueprints/browser/configure.zcml 2009-09-19 04:50:52 +0000
82+++ lib/lp/blueprints/browser/configure.zcml 2009-09-21 12:39:15 +0000
83@@ -23,13 +23,6 @@
84 for="lp.blueprints.interfaces.sprint.ISprint"
85 path_expression="name"
86 parent_utility="lp.blueprints.interfaces.sprint.ISprintSet"/>
87- <browser:page
88- for="lp.blueprints.interfaces.sprint.ISprint"
89- name="+hierarchy"
90- class="lp.blueprints.browser.sprint.SprintIndexHierarchy"
91- template="../../app/templates/launchpad-hierarchy.pt"
92- permission="zope.Public"
93- />
94 <browser:pages
95 for="lp.blueprints.interfaces.sprint.ISprint"
96 class="lp.blueprints.browser.sprint.SprintView"
97
98=== modified file 'lib/lp/blueprints/browser/sprint.py'
99--- lib/lp/blueprints/browser/sprint.py 2009-09-21 07:13:45 +0000
100+++ lib/lp/blueprints/browser/sprint.py 2009-09-21 12:39:15 +0000
101@@ -10,7 +10,6 @@
102 'SprintBrandingView',
103 'SprintEditView',
104 'SprintFacets',
105- 'SprintIndexHierarchy',
106 'SprintMeetingExportView',
107 'SprintNavigation',
108 'SprintOverviewMenu',
109@@ -156,12 +155,6 @@
110 enable_only = ['overview', ]
111
112
113-class SprintIndexHierarchy(Hierarchy):
114- """We force the breadcrumbs not to display for the sprint index."""
115-
116- display_breadcrumbs = False
117-
118-
119 class SprintView(HasSpecificationsView, LaunchpadView):
120
121 __used_for__ = ISprint
122@@ -302,7 +295,6 @@
123
124 schema = ISprint
125 label = "Edit sprint details"
126- page_title = label
127
128 field_names = ['name', 'title', 'summary', 'home_page', 'driver',
129 'time_zone', 'time_starts', 'time_ends', 'address',
130
131=== modified file 'lib/lp/blueprints/stories/sprints/20-sprint-registration.txt'
132--- lib/lp/blueprints/stories/sprints/20-sprint-registration.txt 2009-09-19 04:50:52 +0000
133+++ lib/lp/blueprints/stories/sprints/20-sprint-registration.txt 2009-09-21 12:39:15 +0000
134@@ -11,7 +11,7 @@
135 'http://launchpad.dev/sprints/ubz/+attend'
136
137 >>> print browser.title
138- Register your attendance at Ubuntu Below Zero
139+ Register your attendance : Ubuntu Below Zero : Meetings
140
141 Invalid dates, for instance entering a starting date after the ending date,
142 are reported as errors to the users. (See also the tests in

« Back to merge proposal