Merge lp:~jcsackett/launchpad/sharing-details-breadcrumb into lp:launchpad

Proposed by j.c.sackett
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 15069
Proposed branch: lp:~jcsackett/launchpad/sharing-details-breadcrumb
Merge into: lp:launchpad
Diff against target: 119 lines (+27/-9)
5 files modified
lib/lp/registry/browser/configure.zcml (+6/-1)
lib/lp/registry/browser/pillar.py (+13/-2)
lib/lp/registry/browser/tests/test_pillar_sharing.py (+6/-4)
lib/lp/registry/javascript/sharing/shareetable.js (+1/-1)
lib/lp/registry/javascript/sharing/tests/test_shareetable.js (+1/-1)
To merge this branch: bzr merge lp:~jcsackett/launchpad/sharing-details-breadcrumb
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+100498@code.launchpad.net

Commit message

Sets up the breadcrumb for the sharing details page.

Description of the change

Summary
=======
This branch adds breadcrumb info for the sharing details page. It also alters
the url for the sharing details page, slightly, from
+sharingdetails/$personname to +sharing/$personname, which is closer to the
url structure we agreed upon previously. Ideally, it would be
+sharing/details/person, but that doesn't seem entirely possible within zope's
traversal structure. The previous url (+sharingdetails) was believed to be
necessary at the time, that's not true.

Of note, this doesn't complete the breadcrumb setup as seen in mockups[1], but
as the managing disclosure view doesn't have any corresponding interface
beyond the pillar, it's not added into the traversed_objects for the Hierarchy
interface to convert into a breadcrumb. There are probably ways around this,
but sorting them out was delaying the useful bit of code contained in this
branch, i.e. a proper breadcrumb for the sharing details page.

[1]: http://people.canonical.com/~curtis/disclosure/details.html?context=albert

Preimp
======
None

Implementation
==============
This branch adds a simple IBreadCrumb adapter for the PillarPerson interface,
providing the name of the person in the pillar person as the breadcrumb text.

The zcml for the details page has been updated, as has the stepthrough
decorator for the NavigationMixin to alter the url from +sharingdetails to
+sharing.

Tests
=====
bin/test -vvct pillar_sharing

QA
==
Go to a sharing details page via +sharing/$personname; it should open fine,
verifying the url change.

Additionally, the page should have a breadcrumb of $pillarname >> $personname

Lint
====
Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/browser/configure.zcml
  lib/lp/registry/browser/pillar.py

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) wrote :

Until we sort out the intermediate breadcrumb, I think the text should probably include "sharing details" somewhere.

review: Approve (code)
Revision history for this message
j.c.sackett (jcsackett) wrote :

> Until we sort out the intermediate breadcrumb, I think the text should
> probably include "sharing details" somewhere.

Agreed, I'll add that.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/browser/configure.zcml'
2--- lib/lp/registry/browser/configure.zcml 2012-04-04 22:30:45 +0000
3+++ lib/lp/registry/browser/configure.zcml 2012-04-05 18:52:25 +0000
4@@ -1433,9 +1433,14 @@
5 name="+sharing"
6 class="lp.registry.browser.pillar.PillarSharingView"
7 template="../templates/pillar-sharing.pt"/>
8+ <adapter
9+ provides="lp.services.webapp.interfaces.IBreadcrumb"
10+ for="lp.registry.interfaces.pillar.IPillarPerson"
11+ factory="lp.registry.browser.pillar.PillarPersonBreadcrumb"
12+ permission="zope.Public"/>
13 <browser:url
14 for="lp.registry.interfaces.pillar.IPillarPerson"
15- path_expression="string:+sharingdetails/${person/name}"
16+ path_expression="string:+sharing/${person/name}"
17 rootsite="mainsite"
18 attribute_to_parent="pillar"/>
19 <browser:page
20
21=== modified file 'lib/lp/registry/browser/pillar.py'
22--- lib/lp/registry/browser/pillar.py 2012-04-05 03:35:59 +0000
23+++ lib/lp/registry/browser/pillar.py 2012-04-05 18:52:25 +0000
24@@ -6,7 +6,9 @@
25 __metaclass__ = type
26
27 __all__ = [
28- 'InvolvedMenu', 'PillarBugsMenu', 'PillarView',
29+ 'InvolvedMenu',
30+ 'PillarBugsMenu',
31+ 'PillarView',
32 'PillarNavigationMixin',
33 'PillarPersonSharingView',
34 'PillarSharingView',
35@@ -59,6 +61,7 @@
36 BatchNavigator,
37 StormRangeFactory,
38 )
39+from lp.services.webapp.breadcrumb import Breadcrumb
40 from lp.services.webapp.menu import (
41 ApplicationMenu,
42 enabled_with_permission,
43@@ -73,9 +76,17 @@
44 )
45
46
47+class PillarPersonBreadcrumb(Breadcrumb):
48+ """Builds a breadcrumb for an `IPillarPerson`."""
49+
50+ @property
51+ def text(self):
52+ return "Sharing details for %s" % self.context.person.displayname
53+
54+
55 class PillarNavigationMixin:
56
57- @stepthrough('+sharingdetails')
58+ @stepthrough('+sharing')
59 def traverse_details(self, name):
60 """Traverse to the sharing details for a given person."""
61 person = getUtility(IPersonSet).getByName(name)
62
63=== modified file 'lib/lp/registry/browser/tests/test_pillar_sharing.py'
64--- lib/lp/registry/browser/tests/test_pillar_sharing.py 2012-04-05 03:38:38 +0000
65+++ lib/lp/registry/browser/tests/test_pillar_sharing.py 2012-04-05 18:52:25 +0000
66@@ -91,13 +91,15 @@
67 return PillarPerson(self.pillar, person)
68
69 def test_view_traverses_plus_sharingdetails(self):
70- # The traversed url in the app is pillar/+sharingdetails/person
71+ # The traversed url in the app is pillar/+sharing/person
72 with FeatureFixture(DETAILS_ENABLED_FLAG):
73 # We have to do some fun url hacking to force the traversal a user
74 # encounters.
75 pillarperson = self.getPillarPerson()
76- expected = pillarperson.person.displayname
77- url = 'http://launchpad.dev/%s/+sharingdetails/%s' % (
78+ expected = "Sharing details for %s : %s" % (
79+ pillarperson.person.displayname,
80+ pillarperson.pillar.displayname)
81+ url = 'http://launchpad.dev/%s/+sharing/%s' % (
82 pillarperson.pillar.name, pillarperson.person.name)
83 browser = self.getUserBrowser(user=self.owner, url=url)
84 self.assertEqual(expected, browser.title)
85@@ -109,7 +111,7 @@
86 # We have to do some fun url hacking to force the traversal a user
87 # encounters.
88 pillarperson = self.getPillarPerson(with_sharing=False)
89- url = 'http://launchpad.dev/%s/+sharingdetails/%s' % (
90+ url = 'http://launchpad.dev/%s/+sharing/%s' % (
91 pillarperson.pillar.name, pillarperson.person.name)
92 browser = self.getUserBrowser(user=self.owner, url=url)
93 self.assertIn(
94
95=== modified file 'lib/lp/registry/javascript/sharing/shareetable.js'
96--- lib/lp/registry/javascript/sharing/shareetable.js 2012-04-02 03:12:58 +0000
97+++ lib/lp/registry/javascript/sharing/shareetable.js 2012-04-05 18:52:25 +0000
98@@ -177,7 +177,7 @@
99 '<td></td>',
100 '<td>',
101 '{{#shared_items_exist}}',
102- '<a href="+sharingdetails/{{name}}">View shared items.</a>',
103+ '<a href="+sharing/{{name}}">View shared items.</a>',
104 '{{/shared_items_exist}}',
105 '{{^shared_items_exist}}',
106 '<span class="formHelp">No items shared.</span>',
107
108=== modified file 'lib/lp/registry/javascript/sharing/tests/test_shareetable.js'
109--- lib/lp/registry/javascript/sharing/tests/test_shareetable.js 2012-04-04 04:51:50 +0000
110+++ lib/lp/registry/javascript/sharing/tests/test_shareetable.js 2012-04-05 18:52:25 +0000
111@@ -156,7 +156,7 @@
112 if (sharee.shared_items_exist) {
113 Y.Assert.isNotNull(
114 shared_items_cell.one(
115- 'a[href=+sharingdetails/' + sharee.name + ']'));
116+ 'a[href=+sharing/' + sharee.name + ']'));
117 } else {
118 Y.Assert.areEqual(
119 'No items shared.',