Code review comment for lp:~allenap/launchpad/ui-convert-bug-activity-bug-412523

Revision history for this message
Gavin Panella (allenap) wrote :

Several tests broke, and one of them required quite a lot of work to
fix. Basically, breadcrumbs were still being generated when viewing a
bug that the user did not have permission to view. I've added a test to
test_breadcrumbs and changed the adapter to cope with this scenario.

I've attached an incremental diff.

Thanks in advance.

1=== modified file 'lib/lp/bugs/browser/bugtask.py'
2--- lib/lp/bugs/browser/bugtask.py 2009-08-13 14:07:18 +0000
3+++ lib/lp/bugs/browser/bugtask.py 2009-08-14 16:30:27 +0000
4@@ -62,6 +62,7 @@
5 from zope.schema.interfaces import IContextSourceBinder, IList
6 from zope.schema.vocabulary import (
7 getVocabularyRegistry, SimpleVocabulary, SimpleTerm)
8+from zope.security.interfaces import Unauthorized
9 from zope.security.proxy import (
10 isinstance as zope_isinstance, removeSecurityProxy)
11 from zope.traversing.interfaces import IPathAdapter
12@@ -3495,6 +3496,18 @@
13
14 class BugTaskBreadcrumbBuilder(BreadcrumbBuilder):
15 """Builds a breadcrumb for an `IBugTask`."""
16+
17 @property
18 def text(self):
19 return self.context.bug.displayname
20+
21+ def make_breadcrumb(self):
22+ """Return a breadcrumb for this `BugTask`.
23+
24+ If the user does not have permission to view the bug for
25+ whatever reason, don't return a breadcrumb.
26+ """
27+ try:
28+ return super(BugTaskBreadcrumbBuilder, self).make_breadcrumb()
29+ except Unauthorized:
30+ return None
31
32=== modified file 'lib/lp/bugs/stories/bug-release-management/nomination-navigation.txt'
33--- lib/lp/bugs/stories/bug-release-management/nomination-navigation.txt 2009-07-01 13:16:44 +0000
34+++ lib/lp/bugs/stories/bug-release-management/nomination-navigation.txt 2009-08-14 14:44:07 +0000
35@@ -11,7 +11,7 @@
36 ... 'http://bugs.launchpad.dev/ubuntu/+source/mozilla-firefox/+bug/1'
37 ... '/nominations/2/+editstatus')
38 >>> print_location(admin_browser.contents)
39- Hierarchy: Launchpad > Ubuntu > ?mozilla-firefox? package
40+ Hierarchy: Launchpad > Ubuntu > ?mozilla-firefox? package > Bug #1
41 Tabs:
42 * Overview - http://launchpad.dev/ubuntu/+source/mozilla-firefox
43 * Code - http://code.launchpad.dev/ubuntu/+source/mozilla-firefox
44
45=== modified file 'lib/lp/bugs/stories/bugs/xx-bug-index.txt'
46--- lib/lp/bugs/stories/bugs/xx-bug-index.txt 2009-07-31 08:12:47 +0000
47+++ lib/lp/bugs/stories/bugs/xx-bug-index.txt 2009-08-14 15:11:58 +0000
48@@ -10,9 +10,25 @@
49 >>> anon_browser.title
50 '...Blackhole Trash folder...'
51
52-The bug id in the upper left corner of the page is linked to the bug itself.
53-
54- >>> anon_browser.getLink('Bug #2').click()
55+The breadcrumbs and other navigation include a link to the bug itself.
56+
57+ >>> print_location(anon_browser.contents)
58+ Hierarchy: Launchpad > Debian > ?mozilla-firefox? package > Bug #2 (blackhole)
59+ Tabs:
60+ * Overview - http://launchpad.dev/debian/+source/mozilla-firefox
61+ * Code - http://code.launchpad.dev/debian/+source/mozilla-firefox
62+ * Bugs (selected) - http://bugs.launchpad.dev/debian/+source/mozilla-firefox
63+ * Blueprints - not linked
64+ * Translations - not linked
65+ * Answers - http://answers.launchpad.dev/debian/+source/mozilla-firefox
66+ Main heading: Blackhole Trash folder
67+
68+The bug id in the upper left corner of the main page content is also
69+linked to the bug itself.
70+
71+ >>> bug_link = first_tag_by_class(
72+ ... anon_browser.contents, 'object identifier').a
73+ >>> anon_browser.open(bug_link.get('href'))
74 >>> print anon_browser.title
75 Bug #2 in Tomcat:...Blackhole Trash folder...
76
77
78=== modified file 'lib/lp/bugs/tests/test_breadcrumbs.py'
79--- lib/lp/bugs/tests/test_breadcrumbs.py 2009-08-13 13:57:47 +0000
80+++ lib/lp/bugs/tests/test_breadcrumbs.py 2009-08-14 16:32:02 +0000
81@@ -10,7 +10,8 @@
82 from canonical.lazr.testing.menus import make_fake_request
83 from canonical.launchpad.webapp.publisher import RootObject
84 from canonical.testing import DatabaseFunctionalLayer
85-from lp.testing import TestCaseWithFactory
86+from lp.testing import (
87+ ANONYMOUS, TestCaseWithFactory, login)
88
89
90 class TestBugTaskBreadcrumbBuilder(TestCaseWithFactory):
91@@ -18,7 +19,7 @@
92 layer = DatabaseFunctionalLayer
93
94 def setUp(self):
95- TestCaseWithFactory.setUp(self)
96+ super(TestBugTaskBreadcrumbBuilder, self).setUp()
97 self.product = self.factory.makeProduct(
98 name='crumb-tester', displayname="Crumb Tester")
99 self.bug = self.factory.makeBug(product=self.product)
100@@ -66,6 +67,23 @@
101 ["Crumb Tester", "Bug #%d" % self.bug.id],
102 [crumb.text for crumb in hierarchy.items()])
103
104+ def test_bugtask_private_bug(self):
105+ # A breadcrumb is not generated for a bug that the user does
106+ # not have permission to view.
107+ login('foo.bar@canonical.com')
108+ self.bug.setPrivate(True, self.bug.owner)
109+ login(ANONYMOUS)
110+ request = make_fake_request(
111+ 'http://bugs.launchpad.dev/%s/+bug/%d/+activity' % (
112+ self.product.name, self.bug.id),
113+ [self.root, self.product, self.bug.default_bugtask])
114+ hierarchy = getMultiAdapter((self.root, request), name='+hierarchy')
115+ self.assertEquals(
116+ ['http://bugs.launchpad.dev/crumb-tester'],
117+ [crumb.url for crumb in hierarchy.items()])
118+ self.assertEquals(
119+ ["Crumb Tester"], [crumb.text for crumb in hierarchy.items()])
120+
121
122 def test_suite():
123 return unittest.TestLoader().loadTestsFromName(__name__)

« Back to merge proposal