Code review comment for lp:~michael.nelson/launchpad/sprint-index-and-attend-3.0
- sprint-index-and-attend-3.0
- Merge into devel
Revision history for this message
| Michael Nelson (michael.nelson) wrote : | # |
| 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 |
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 SprintIndexHier
affecting most other sprint views (which had perfect breadcrumbs
otherwise - but none with this fix).
I created:
https:/
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