Merge lp:~salgado/launchpad/breadcrumbs-for-leafs into lp:launchpad
- breadcrumbs-for-leafs
- Merge into devel
Proposed by
Guilherme Salgado
| Status: | Merged |
|---|---|
| Approved by: | Paul Hummer |
| Approved revision: | no longer in the source branch. |
| Merged at revision: | not available |
| Proposed branch: | lp:~salgado/launchpad/breadcrumbs-for-leafs |
| Merge into: | lp:launchpad |
| Diff against target: | None lines |
| To merge this branch: | bzr merge lp:~salgado/launchpad/breadcrumbs-for-leafs |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Paul Hummer (community) | Approve | ||
|
Review via email:
|
|||
Commit message
Description of the change
To post a comment you must log in.
Revision history for this message
| Guilherme Salgado (salgado) wrote : | # |
Revision history for this message
| Paul Hummer (rockstar) wrote : | # |
As discussed in IRC, please convert the one XXX comment to a regular comment, and file a bug for the second XXX comment.
review:
Approve
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
| 1 | === modified file 'lib/canonical/launchpad/browser/launchpad.py' |
| 2 | --- lib/canonical/launchpad/browser/launchpad.py 2009-09-16 19:56:45 +0000 |
| 3 | +++ lib/canonical/launchpad/browser/launchpad.py 2009-09-17 14:29:22 +0000 |
| 4 | @@ -103,6 +103,7 @@ |
| 5 | from canonical.launchpad.webapp.publisher import RedirectionView |
| 6 | from canonical.launchpad.webapp.authorization import check_permission |
| 7 | from lazr.uri import URI |
| 8 | +from canonical.launchpad.webapp.tales import PageTemplateContextsAPI |
| 9 | from canonical.launchpad.webapp.url import urlappend |
| 10 | from canonical.launchpad.webapp.vhosts import allvhosts |
| 11 | from canonical.widgets.project import ProjectScopeWidget |
| 12 | @@ -263,16 +264,27 @@ |
| 13 | URL and the page's name (i.e. the last path segment of the URL). |
| 14 | |
| 15 | If the requested page (as specified in self.request) is the default |
| 16 | - one for the last traversed object, return None. |
| 17 | + one for our parent view's context, return None. |
| 18 | """ |
| 19 | url = self.request.getURL() |
| 20 | - last_segment = URI(url).path.split('/')[-1] |
| 21 | - default_view_name = zapi.getDefaultViewName( |
| 22 | - self.request.traversed_objects[-1], self.request) |
| 23 | - if last_segment.startswith('+') and last_segment != default_view_name: |
| 24 | + from zope.security.proxy import removeSecurityProxy |
| 25 | + view = removeSecurityProxy(self.request.traversed_objects[-1]) |
| 26 | + obj = self.request.traversed_objects[-2] |
| 27 | + default_view_name = zapi.getDefaultViewName(obj, self.request) |
| 28 | + if view.__name__ != default_view_name: |
| 29 | + title = getattr(view, 'page_title', None) |
| 30 | + if title is None: |
| 31 | + template = getattr(view, 'template', None) |
| 32 | + if template is None: |
| 33 | + template = view.index |
| 34 | + template_api = PageTemplateContextsAPI( |
| 35 | + dict(context=obj, template=template, view=view)) |
| 36 | + title = template_api.pagetitle() |
| 37 | + if len(title) > 30: |
| 38 | + title = "%s..." % title[:30] |
| 39 | breadcrumb = Breadcrumb(None) |
| 40 | breadcrumb._url = url |
| 41 | - breadcrumb.text = last_segment |
| 42 | + breadcrumb.text = title |
| 43 | return breadcrumb |
| 44 | else: |
| 45 | return None |
| 46 | |
| 47 | === modified file 'lib/canonical/launchpad/browser/multistep.py' |
| 48 | --- lib/canonical/launchpad/browser/multistep.py 2009-06-25 05:30:52 +0000 |
| 49 | +++ lib/canonical/launchpad/browser/multistep.py 2009-09-10 20:54:19 +0000 |
| 50 | @@ -57,6 +57,14 @@ |
| 51 | """ |
| 52 | raise NotImplementedError |
| 53 | |
| 54 | + # XXX: salgado, 2009-09-10: A page title should not be needed here, but |
| 55 | + # our auto generated breadcrumbs will expect it to exist, so it must be |
| 56 | + # defined in subclasses. |
| 57 | + @property |
| 58 | + def page_title(self): |
| 59 | + """Must override in subclasses for breadcrumbs to work.""" |
| 60 | + raise NotImplementedError |
| 61 | + |
| 62 | def initialize(self): |
| 63 | """Initialize the view and handle stepping through sub-views.""" |
| 64 | view = self.first_step(self.context, self.request) |
| 65 | |
| 66 | === modified file 'lib/canonical/launchpad/doc/hierarchical-menu.txt' |
| 67 | --- lib/canonical/launchpad/doc/hierarchical-menu.txt 2009-08-26 09:33:33 +0000 |
| 68 | +++ lib/canonical/launchpad/doc/hierarchical-menu.txt 2009-09-17 15:14:02 +0000 |
| 69 | @@ -82,6 +82,12 @@ |
| 70 | breadcrumbs starting with the breadcrumb closest to the hierarchy root. |
| 71 | |
| 72 | >>> from canonical.launchpad.webapp.breadcrumb import Breadcrumb |
| 73 | + >>> from canonical.launchpad.browser.launchpad import Hierarchy |
| 74 | + # Monkey patch Hierarchy.makeBreadcrumbForRequestedPage so that we don't |
| 75 | + # have to create fake views and other stuff to test breadcrumbs here. The |
| 76 | + # functionality provided by that method is tested in |
| 77 | + # webapp/tests/test_breadcrumbs.py. |
| 78 | + >>> Hierarchy.makeBreadcrumbForRequestedPage = lambda self: None |
| 79 | |
| 80 | # Note that the Hierarchy assigns the breadcrumb's URL, but we need to |
| 81 | # give it a valid .text attribute. |
| 82 | @@ -171,7 +177,7 @@ |
| 83 | text='Joy of cooking'> |
| 84 | |
| 85 | Breadcrumbs may have icons. The icon is only set for a breadcrumb if |
| 86 | -the builder's context has an IPathAdapter registration. |
| 87 | +the adapter's context has an IPathAdapter registration. |
| 88 | |
| 89 | >>> from zope.traversing.interfaces import IPathAdapter |
| 90 | >>> from canonical.launchpad.webapp.tales import ( |
| 91 | @@ -220,7 +226,6 @@ |
| 92 | itself. It should let the IBreadcrumbBuilder handle it: this ensures |
| 93 | consistency across the site. |
| 94 | |
| 95 | - >>> from canonical.launchpad.browser.launchpad import Hierarchy |
| 96 | >>> class CustomHierarchy(Hierarchy): |
| 97 | ... @property |
| 98 | ... def objects(self): |
| 99 | |
| 100 | === modified file 'lib/canonical/launchpad/doc/navigation.txt' |
| 101 | --- lib/canonical/launchpad/doc/navigation.txt 2009-06-11 01:28:55 +0000 |
| 102 | +++ lib/canonical/launchpad/doc/navigation.txt 2009-09-17 15:16:46 +0000 |
| 103 | @@ -635,11 +635,11 @@ |
| 104 | >>> class DupeNames(Navigation): |
| 105 | ... |
| 106 | ... @stepto('foo') |
| 107 | - ... def doit(self): |
| 108 | + ... def doit_foo(self): |
| 109 | ... return 'foo' |
| 110 | ... |
| 111 | ... @stepto('bar') |
| 112 | - ... def doit(self): |
| 113 | + ... def doit_bar(self): |
| 114 | ... return 'bar' |
| 115 | |
| 116 | >>> instance_of_dupenames = DupeNames(thingset, request) |
| 117 | @@ -652,85 +652,3 @@ |
| 118 | |
| 119 | >>> instance_of_dupenames.doit() |
| 120 | 'bar' |
| 121 | - |
| 122 | - |
| 123 | -== Inspecting the traversed objects == |
| 124 | - |
| 125 | -As the object heirarchy is traversed, a list of the traversed objects |
| 126 | -is built up in the request. |
| 127 | - |
| 128 | -Create a navigation class for IThing's: |
| 129 | - |
| 130 | - >>> class ThingNavigation(Navigation): |
| 131 | - ... usedfor = IThing |
| 132 | - ... |
| 133 | - ... @stepto('foo') |
| 134 | - ... def traverse_foo(self): |
| 135 | - ... return 'bar' |
| 136 | - ... |
| 137 | - ... @stepto('something') |
| 138 | - ... def traverse_something(self): |
| 139 | - ... return Thing('something') |
| 140 | - |
| 141 | - |
| 142 | -Traverse from the thingset to a particular thing: |
| 143 | - |
| 144 | - >>> request = Request() |
| 145 | - >>> navigation = ThingSetNavigation(thingset, request) |
| 146 | - >>> thing = navigation.publishTraverse(request, 'ttt') |
| 147 | - >>> navigation2 = ThingNavigation(thing, request) |
| 148 | - >>> navigation2.publishTraverse(request, 'foo') |
| 149 | - 'bar' |
| 150 | - |
| 151 | - |
| 152 | -The traversed objects are available, in the order they were traversed: |
| 153 | - |
| 154 | - >>> request.traversed_objects |
| 155 | - [<ThingSet>, <Thing 'TTT'>] |
| 156 | - |
| 157 | -We can ask the request for the nearest traversed object that |
| 158 | -implements a particular interface: |
| 159 | - |
| 160 | - >>> request.getNearest(IThingSet) |
| 161 | - (<ThingSet>, <InterfaceClass __builtin__.IThingSet>) |
| 162 | - >>> request.getNearest(IThing) |
| 163 | - (<Thing 'TTT'>, <InterfaceClass __builtin__.IThing>) |
| 164 | - |
| 165 | -The second argument is the matched interface. This is useful when |
| 166 | -multiple interfaces are passed to getNearest(): |
| 167 | - |
| 168 | - >>> request.getNearest(IThingSet, IThing) |
| 169 | - (<Thing 'TTT'>, <InterfaceClass __builtin__.IThing>) |
| 170 | - |
| 171 | -If more than one object of a particular interface type has been |
| 172 | -traversed, the most recently traversed one is returned: |
| 173 | - |
| 174 | - >>> request = Request() |
| 175 | - >>> navigation = ThingSetNavigation(thingset, request) |
| 176 | - >>> thing = navigation.publishTraverse(request, 'ttt') |
| 177 | - >>> navigation2 = ThingNavigation(thing, request) |
| 178 | - >>> thing2 = navigation2.publishTraverse(request, 'something') |
| 179 | - >>> navigation3 = ThingNavigation(thing2, request) |
| 180 | - >>> navigation3.publishTraverse(request, 'foo') |
| 181 | - 'bar' |
| 182 | - |
| 183 | - >>> request.traversed_objects |
| 184 | - [<ThingSet>, <Thing 'TTT'>, <Thing 'something'>] |
| 185 | - >>> request.getNearest(IThing) |
| 186 | - (<Thing 'something'>, <InterfaceClass __builtin__.IThing>) |
| 187 | - |
| 188 | -If a particular interface has not been traversed, the tuple |
| 189 | -(None, None) is returned: |
| 190 | - |
| 191 | - >>> request = Request() |
| 192 | - >>> navigation = ThingSetNavigation(thingset, request) |
| 193 | - >>> thing = navigation.publishTraverse(request, 'xyz') |
| 194 | - Traceback (most recent call last): |
| 195 | - ... |
| 196 | - NotFound: ...ThingSet..., name: 'xyz' |
| 197 | - >>> request.getNearest(IThing) |
| 198 | - (None, None) |
| 199 | - |
| 200 | -Note that a tuple is returned in the error case so that the result can |
| 201 | -unpacked unconditionally, rather than needing to check for an error |
| 202 | -return before unpacking. |
| 203 | |
| 204 | === modified file 'lib/canonical/launchpad/webapp/publication.py' |
| 205 | --- lib/canonical/launchpad/webapp/publication.py 2009-09-15 10:28:23 +0000 |
| 206 | +++ lib/canonical/launchpad/webapp/publication.py 2009-09-17 14:29:22 +0000 |
| 207 | @@ -428,6 +428,11 @@ |
| 208 | def callTraversalHooks(self, request, ob): |
| 209 | """ We don't want to call _maybePlacefullyAuthenticate as does |
| 210 | zopepublication """ |
| 211 | + # In some cases we seem to be called more than once for a given |
| 212 | + # traversed object, so we need to be careful here and only append an |
| 213 | + # object the first time we see it. |
| 214 | + if ob not in request.traversed_objects: |
| 215 | + request.traversed_objects.append(ob) |
| 216 | notify(BeforeTraverseEvent(ob, request)) |
| 217 | |
| 218 | def afterTraversal(self, request, ob): |
| 219 | |
| 220 | === modified file 'lib/canonical/launchpad/webapp/publisher.py' |
| 221 | --- lib/canonical/launchpad/webapp/publisher.py 2009-08-20 07:15:35 +0000 |
| 222 | +++ lib/canonical/launchpad/webapp/publisher.py 2009-09-10 20:30:44 +0000 |
| 223 | @@ -697,10 +697,6 @@ |
| 224 | if self.newlayer is not None: |
| 225 | setFirstLayer(request, self.newlayer) |
| 226 | |
| 227 | - # store the current context object in the request's |
| 228 | - # traversed_objects list: |
| 229 | - request.traversed_objects.append(self.context) |
| 230 | - |
| 231 | # Next, see if we're being asked to stepto somewhere. |
| 232 | stepto_traversals = self.stepto_traversals |
| 233 | if stepto_traversals is not None: |
| 234 | |
| 235 | === modified file 'lib/canonical/launchpad/webapp/tales.py' |
| 236 | --- lib/canonical/launchpad/webapp/tales.py 2009-09-14 14:48:51 +0000 |
| 237 | +++ lib/canonical/launchpad/webapp/tales.py 2009-09-16 13:44:15 +0000 |
| 238 | @@ -2233,7 +2233,6 @@ |
| 239 | name = name.replace('-', '_') |
| 240 | titleobj = getattr(canonical.launchpad.pagetitles, name, None) |
| 241 | if titleobj is None: |
| 242 | - # sabdfl 25/0805 page titles are now mandatory hence the assert |
| 243 | raise AssertionError( |
| 244 | "No page title in canonical.launchpad.pagetitles " |
| 245 | "for %s" % name) |
| 246 | |
| 247 | === modified file 'lib/canonical/launchpad/webapp/tests/breadcrumbs.py' |
| 248 | --- lib/canonical/launchpad/webapp/tests/breadcrumbs.py 2009-08-27 20:00:05 +0000 |
| 249 | +++ lib/canonical/launchpad/webapp/tests/breadcrumbs.py 2009-09-17 14:23:16 +0000 |
| 250 | @@ -3,9 +3,11 @@ |
| 251 | |
| 252 | __metaclass__ = type |
| 253 | |
| 254 | -from zope.component import getMultiAdapter |
| 255 | +from zope.app import zapi |
| 256 | +from zope.component import ComponentLookupError, getMultiAdapter |
| 257 | |
| 258 | from canonical.lazr.testing.menus import make_fake_request |
| 259 | +from canonical.launchpad.layers import setFirstLayer |
| 260 | from canonical.launchpad.webapp.publisher import RootObject |
| 261 | from canonical.testing import DatabaseFunctionalLayer |
| 262 | from lp.testing import TestCaseWithFactory |
| 263 | @@ -14,13 +16,14 @@ |
| 264 | class BaseBreadcrumbTestCase(TestCaseWithFactory): |
| 265 | |
| 266 | layer = DatabaseFunctionalLayer |
| 267 | + request_layer = None |
| 268 | |
| 269 | def setUp(self): |
| 270 | super(BaseBreadcrumbTestCase, self).setUp() |
| 271 | self.root = RootObject() |
| 272 | |
| 273 | def _getHierarchyView(self, url, traversed_objects): |
| 274 | - request = make_fake_request(url, traversed_objects) |
| 275 | + request = self._make_request(url, traversed_objects) |
| 276 | return getMultiAdapter((self.root, request), name='+hierarchy') |
| 277 | |
| 278 | def _getBreadcrumbs(self, url, traversed_objects): |
| 279 | @@ -28,10 +31,38 @@ |
| 280 | return view.items |
| 281 | |
| 282 | def _getBreadcrumbsTexts(self, url, traversed_objects): |
| 283 | - return [crumb.text |
| 284 | - for crumb in self._getBreadcrumbs(url, traversed_objects)] |
| 285 | + crumbs = self._getBreadcrumbs(url, traversed_objects) |
| 286 | + return [crumb.text for crumb in crumbs] |
| 287 | |
| 288 | def _getBreadcrumbsURLs(self, url, traversed_objects): |
| 289 | - return [crumb.url |
| 290 | - for crumb in self._getBreadcrumbs(url, traversed_objects)] |
| 291 | - |
| 292 | + crumbs = self._getBreadcrumbs(url, traversed_objects) |
| 293 | + return [crumb.url for crumb in crumbs] |
| 294 | + |
| 295 | + def _make_request(self, url, traversed_objects): |
| 296 | + """Create and return a LaunchpadTestRequest. |
| 297 | + |
| 298 | + Set the given list of traversed objects as request.traversed_objects, |
| 299 | + also appending the view that the given URL points to, to mimic how |
| 300 | + request.traversed_objects behave in a real request. |
| 301 | + |
| 302 | + XXX: salgado, 2009-09-17: Instead of setting request.traversed_objects |
| 303 | + manually, we should duplicate parts of zope.publisher.publish.publish |
| 304 | + here (or in make_fake_request()) so that tests don't have to specify |
| 305 | + the list of traversed objects for us to set here. |
| 306 | + """ |
| 307 | + request = make_fake_request(url, traversed_objects=traversed_objects) |
| 308 | + if self.request_layer is not None: |
| 309 | + setFirstLayer(request, self.request_layer) |
| 310 | + last_segment = request._traversed_names[-1] |
| 311 | + if traversed_objects: |
| 312 | + obj = traversed_objects[-1] |
| 313 | + # Assume the last_segment is the name of the view on the last |
| 314 | + # traversed object, and if we fail to find a view with that name, |
| 315 | + # use the default view. |
| 316 | + try: |
| 317 | + view = getMultiAdapter((obj, request), name=last_segment) |
| 318 | + except ComponentLookupError: |
| 319 | + default_view_name = zapi.getDefaultViewName(obj, request) |
| 320 | + view = getMultiAdapter((obj, request), name=default_view_name) |
| 321 | + request.traversed_objects.append(view) |
| 322 | + return request |
| 323 | |
| 324 | === modified file 'lib/canonical/launchpad/webapp/tests/test_breadcrumbs.py' |
| 325 | --- lib/canonical/launchpad/webapp/tests/test_breadcrumbs.py 2009-08-31 17:40:56 +0000 |
| 326 | +++ lib/canonical/launchpad/webapp/tests/test_breadcrumbs.py 2009-09-03 13:25:04 +0000 |
| 327 | @@ -48,6 +48,25 @@ |
| 328 | self.product = self.factory.makeProduct(name='crumb-tester') |
| 329 | self.product_url = canonical_url(self.product) |
| 330 | |
| 331 | + def test_breadcrumb_text_for_page_with_short_title(self): |
| 332 | + # When the page title is less than 30 characters long, we use the |
| 333 | + # complete title as the breadcrumb's text. |
| 334 | + downloads_url = "%s/+download" % self.product_url |
| 335 | + texts = self._getBreadcrumbsTexts( |
| 336 | + downloads_url, [self.root, self.product]) |
| 337 | + self.assertEquals(texts[-1], |
| 338 | + '%s project files' % self.product.displayname) |
| 339 | + |
| 340 | + def test_breadcrumb_text_for_page_with_long_title(self): |
| 341 | + # When the page title is more than 30 characters long, we use only the |
| 342 | + # first 30 as the breadcrumb's text. |
| 343 | + downloads_url = "%s/+purchase-subscription" % self.product_url |
| 344 | + texts = self._getBreadcrumbsTexts( |
| 345 | + downloads_url, [self.root, self.product]) |
| 346 | + self.assertEquals( |
| 347 | + texts[-1], |
| 348 | + 'Purchase Subscription for %s...' % self.product.displayname[:4]) |
| 349 | + |
| 350 | def test_default_page(self): |
| 351 | urls = self._getBreadcrumbsURLs( |
| 352 | self.product_url, [self.root, self.product]) |
| 353 | |
| 354 | === modified file 'lib/canonical/launchpad/webapp/tests/test_publication.py' |
| 355 | --- lib/canonical/launchpad/webapp/tests/test_publication.py 2009-09-15 10:28:23 +0000 |
| 356 | +++ lib/canonical/launchpad/webapp/tests/test_publication.py 2009-09-17 14:29:22 +0000 |
| 357 | @@ -10,12 +10,10 @@ |
| 358 | |
| 359 | from contrib.oauth import OAuthRequest, OAuthSignatureMethod_PLAINTEXT |
| 360 | |
| 361 | -from psycopg2.extensions import TransactionRollbackError |
| 362 | from storm.exceptions import DisconnectionError |
| 363 | from zope.component import getUtility |
| 364 | from zope.error.interfaces import IErrorReportingUtility |
| 365 | from zope.publisher.interfaces import Retry |
| 366 | -from zope.security.management import endInteraction |
| 367 | |
| 368 | from canonical.testing import DatabaseFunctionalLayer |
| 369 | from canonical.launchpad.interfaces.oauth import IOAuthConsumerSet |
| 370 | @@ -23,11 +21,37 @@ |
| 371 | from lp.testing import TestCase, TestCaseWithFactory |
| 372 | import canonical.launchpad.webapp.adapter as da |
| 373 | from canonical.launchpad.webapp.interfaces import OAuthPermission |
| 374 | -from canonical.launchpad.webapp.publication import is_browser |
| 375 | +from canonical.launchpad.webapp.publication import ( |
| 376 | + is_browser, LaunchpadBrowserPublication) |
| 377 | from canonical.launchpad.webapp.servers import ( |
| 378 | LaunchpadTestRequest, WebServicePublication) |
| 379 | |
| 380 | |
| 381 | +class TestLaunchpadBrowserPublication(TestCase): |
| 382 | + |
| 383 | + def test_callTraversalHooks_appends_to_traversed_objects(self): |
| 384 | + # Traversed objects are appended to request.traversed_objects in the |
| 385 | + # order they're traversed. |
| 386 | + obj1 = object() |
| 387 | + obj2 = object() |
| 388 | + request = LaunchpadTestRequest() |
| 389 | + publication = LaunchpadBrowserPublication(None) |
| 390 | + publication.callTraversalHooks(request, obj1) |
| 391 | + publication.callTraversalHooks(request, obj2) |
| 392 | + self.assertEquals(request.traversed_objects, [obj1, obj2]) |
| 393 | + |
| 394 | + def test_callTraversalHooks_appends_only_once_to_traversed_objects(self): |
| 395 | + # callTraversalHooks() may be called more than once for a given |
| 396 | + # traversed object, but if that's the case we won't add the same |
| 397 | + # object twice to traversed_objects. |
| 398 | + obj1 = obj2 = object() |
| 399 | + request = LaunchpadTestRequest() |
| 400 | + publication = LaunchpadBrowserPublication(None) |
| 401 | + publication.callTraversalHooks(request, obj1) |
| 402 | + publication.callTraversalHooks(request, obj2) |
| 403 | + self.assertEquals(request.traversed_objects, [obj1]) |
| 404 | + |
| 405 | + |
| 406 | class TestWebServicePublication(TestCaseWithFactory): |
| 407 | layer = DatabaseFunctionalLayer |
| 408 | |
| 409 | |
| 410 | === modified file 'lib/canonical/launchpad/webapp/tests/test_servers.py' |
| 411 | --- lib/canonical/launchpad/webapp/tests/test_servers.py 2009-07-27 14:08:17 +0000 |
| 412 | +++ lib/canonical/launchpad/webapp/tests/test_servers.py 2009-09-17 14:23:16 +0000 |
| 413 | @@ -8,6 +8,9 @@ |
| 414 | |
| 415 | from zope.publisher.base import DefaultPublication |
| 416 | from zope.testing.doctest import DocTestSuite, NORMALIZE_WHITESPACE, ELLIPSIS |
| 417 | +from zope.interface import implements, Interface |
| 418 | + |
| 419 | +from lp.testing import TestCase |
| 420 | |
| 421 | from canonical.launchpad.webapp.servers import ( |
| 422 | AnswersBrowserRequest, ApplicationServerSettingRequestFactory, |
| 423 | @@ -15,10 +18,10 @@ |
| 424 | TranslationsBrowserRequest, VHostWebServiceRequestPublicationFactory, |
| 425 | VirtualHostRequestPublicationFactory, WebServiceRequestPublicationFactory, |
| 426 | WebServiceClientRequest, WebServicePublication, WebServiceTestRequest) |
| 427 | - |
| 428 | from canonical.launchpad.webapp.tests import DummyConfigurationTestCase |
| 429 | |
| 430 | -class SetInWSGIEnvironmentTestCase(unittest.TestCase): |
| 431 | + |
| 432 | +class SetInWSGIEnvironmentTestCase(TestCase): |
| 433 | |
| 434 | def test_set(self): |
| 435 | # Test that setInWSGIEnvironment() can set keys in the WSGI |
| 436 | @@ -61,7 +64,7 @@ |
| 437 | self.assertEqual(new_request._orig_env['key'], 'second value') |
| 438 | |
| 439 | |
| 440 | -class TestApplicationServerSettingRequestFactory(unittest.TestCase): |
| 441 | +class TestApplicationServerSettingRequestFactory(TestCase): |
| 442 | """Tests for the ApplicationServerSettingRequestFactory.""" |
| 443 | |
| 444 | def test___call___should_set_HTTPS_env_on(self): |
| 445 | @@ -274,7 +277,7 @@ |
| 446 | "request traversal stack: %r" % stack) |
| 447 | |
| 448 | |
| 449 | -class TestWebServiceRequest(unittest.TestCase): |
| 450 | +class TestWebServiceRequest(TestCase): |
| 451 | |
| 452 | def test_application_url(self): |
| 453 | """Requests to the /api path should return the original request's |
| 454 | @@ -300,7 +303,7 @@ |
| 455 | 'Cookie, Authorization, Accept') |
| 456 | |
| 457 | |
| 458 | -class TestBasicLaunchpadRequest(unittest.TestCase): |
| 459 | +class TestBasicLaunchpadRequest(TestCase): |
| 460 | """Tests for the base request class""" |
| 461 | |
| 462 | def test_baserequest_response_should_vary(self): |
| 463 | @@ -318,7 +321,56 @@ |
| 464 | 'Cookie, Authorization') |
| 465 | |
| 466 | |
| 467 | -class TestAnswersBrowserRequest(unittest.TestCase): |
| 468 | +class IThingSet(Interface): |
| 469 | + """Marker interface for a set of things.""" |
| 470 | + |
| 471 | + |
| 472 | +class IThing(Interface): |
| 473 | + """Marker interface for a thing.""" |
| 474 | + |
| 475 | + |
| 476 | +class Thing: |
| 477 | + implements(IThing) |
| 478 | + |
| 479 | + |
| 480 | +class ThingSet: |
| 481 | + implements(IThingSet) |
| 482 | + |
| 483 | + |
| 484 | +class TestLaunchpadBrowserRequest_getNearest(TestCase): |
| 485 | + |
| 486 | + def setUp(self): |
| 487 | + super(TestLaunchpadBrowserRequest_getNearest, self).setUp() |
| 488 | + self.request = LaunchpadBrowserRequest('', {}) |
| 489 | + self.thing_set = ThingSet() |
| 490 | + self.thing = Thing() |
| 491 | + |
| 492 | + def test_return_value(self): |
| 493 | + # .getNearest() returns a two-tuple with the object and the interface |
| 494 | + # that matched. The second item in the tuple is useful when multiple |
| 495 | + # interfaces are passed to getNearest(). |
| 496 | + request = self.request |
| 497 | + request.traversed_objects.extend([self.thing_set, self.thing]) |
| 498 | + self.assertEquals(request.getNearest(IThing), (self.thing, IThing)) |
| 499 | + self.assertEquals( |
| 500 | + request.getNearest(IThingSet), (self.thing_set, IThingSet)) |
| 501 | + |
| 502 | + def test_multiple_traversed_objects_with_common_interface(self): |
| 503 | + # If more than one object of a particular interface type has been |
| 504 | + # traversed, the most recently traversed one is returned. |
| 505 | + thing2 = Thing() |
| 506 | + self.request.traversed_objects.extend( |
| 507 | + [self.thing_set, self.thing, thing2]) |
| 508 | + self.assertEquals(self.request.getNearest(IThing), (thing2, IThing)) |
| 509 | + |
| 510 | + def test_interface_not_traversed(self): |
| 511 | + # If a particular interface has not been traversed, the tuple |
| 512 | + # (None, None) is returned. |
| 513 | + self.request.traversed_objects.extend([self.thing_set]) |
| 514 | + self.assertEquals(self.request.getNearest(IThing), (None, None)) |
| 515 | + |
| 516 | + |
| 517 | +class TestAnswersBrowserRequest(TestCase): |
| 518 | """Tests for the Answers request class.""" |
| 519 | |
| 520 | def test_response_should_vary_based_on_language(self): |
| 521 | @@ -328,7 +380,7 @@ |
| 522 | 'Cookie, Authorization, Accept-Language') |
| 523 | |
| 524 | |
| 525 | -class TestTranslationsBrowserRequest(unittest.TestCase): |
| 526 | +class TestTranslationsBrowserRequest(TestCase): |
| 527 | """Tests for the Translations request class.""" |
| 528 | |
| 529 | def test_response_should_vary_based_on_language(self): |
| 530 | @@ -338,7 +390,7 @@ |
| 531 | 'Cookie, Authorization, Accept-Language') |
| 532 | |
| 533 | |
| 534 | -class TestLaunchpadBrowserRequest(unittest.TestCase): |
| 535 | +class TestLaunchpadBrowserRequest(TestCase): |
| 536 | |
| 537 | def prepareRequest(self, form): |
| 538 | """Return a `LaunchpadBrowserRequest` with the given form. |
| 539 | |
| 540 | === modified file 'lib/canonical/lazr/testing/menus.py' |
| 541 | --- lib/canonical/lazr/testing/menus.py 2009-08-18 12:21:51 +0000 |
| 542 | +++ lib/canonical/lazr/testing/menus.py 2009-09-17 14:23:16 +0000 |
| 543 | @@ -61,7 +61,7 @@ |
| 544 | PATH_INFO=path_info) |
| 545 | request._traversed_names = path_info.split('/')[1:] |
| 546 | if traversed_objects is not None: |
| 547 | - request.traversed_objects = traversed_objects |
| 548 | + request.traversed_objects = traversed_objects[:] |
| 549 | # After making the request, setup a new interaction. |
| 550 | endInteraction() |
| 551 | newInteraction(request) |
| 552 | |
| 553 | === modified file 'lib/lp/bugs/browser/bugalsoaffects.py' |
| 554 | --- lib/lp/bugs/browser/bugalsoaffects.py 2009-07-30 00:42:18 +0000 |
| 555 | +++ lib/lp/bugs/browser/bugalsoaffects.py 2009-09-09 14:32:45 +0000 |
| 556 | @@ -53,12 +53,16 @@ |
| 557 | |
| 558 | |
| 559 | class BugAlsoAffectsProductMetaView(MultiStepView): |
| 560 | + page_title = 'Record as affecting another project' |
| 561 | + |
| 562 | @property |
| 563 | def first_step(self): |
| 564 | return ChooseProductStep |
| 565 | |
| 566 | |
| 567 | class BugAlsoAffectsDistroMetaView(MultiStepView): |
| 568 | + page_title = 'Record as affecting another distribution/package' |
| 569 | + |
| 570 | @property |
| 571 | def first_step(self): |
| 572 | return DistroBugTaskCreationStep |
| 573 | |
| 574 | === modified file 'lib/lp/bugs/browser/cve.py' |
| 575 | --- lib/lp/bugs/browser/cve.py 2009-09-07 19:43:17 +0000 |
| 576 | +++ lib/lp/bugs/browser/cve.py 2009-09-16 13:44:15 +0000 |
| 577 | @@ -105,6 +105,7 @@ |
| 578 | self.request.response.addInfoNotification( |
| 579 | 'CVE-%s removed.' % data['sequence']) |
| 580 | |
| 581 | + @property |
| 582 | def label(self): |
| 583 | return 'Bug # %s Remove link to CVE report' % self.context.bug.id |
| 584 | |
| 585 | |
| 586 | === modified file 'lib/lp/bugs/browser/tests/test_breadcrumbs.py' |
| 587 | --- lib/lp/bugs/browser/tests/test_breadcrumbs.py 2009-09-09 13:06:32 +0000 |
| 588 | +++ lib/lp/bugs/browser/tests/test_breadcrumbs.py 2009-09-10 18:23:44 +0000 |
| 589 | @@ -41,7 +41,6 @@ |
| 590 | self.assertEquals(urls[-1], "%s/+activity" % self.bugtask_url) |
| 591 | self.assertEquals(urls[-2], self.bugtask_url) |
| 592 | texts = self._getBreadcrumbsTexts(url, self.traversed_objects) |
| 593 | - self.assertEquals(texts[-1], "+activity") |
| 594 | self.assertEquals(texts[-2], "Bug #%d" % self.bug.id) |
| 595 | |
| 596 | def test_bugtask_private_bug(self): |
| 597 | |
| 598 | === modified file 'lib/lp/registry/browser/branding.py' |
| 599 | --- lib/lp/registry/browser/branding.py 2009-08-26 16:34:38 +0000 |
| 600 | +++ lib/lp/registry/browser/branding.py 2009-09-03 13:25:04 +0000 |
| 601 | @@ -23,6 +23,7 @@ |
| 602 | (some subset of icon, logo, mugshot). |
| 603 | """ |
| 604 | |
| 605 | + @property |
| 606 | def label(self): |
| 607 | return ('Change the images used to represent %s in Launchpad' |
| 608 | % self.context.displayname) |
| 609 | |
| 610 | === modified file 'lib/lp/registry/browser/product.py' |
| 611 | --- lib/lp/registry/browser/product.py 2009-09-15 20:41:36 +0000 |
| 612 | +++ lib/lp/registry/browser/product.py 2009-09-17 15:16:46 +0000 |
| 613 | @@ -1425,7 +1425,7 @@ |
| 614 | """View for searching products to be reviewed.""" |
| 615 | |
| 616 | schema = IProductReviewSearch |
| 617 | - label= 'Review projects' |
| 618 | + label = 'Review projects' |
| 619 | |
| 620 | full_row_field_names = [ |
| 621 | 'search_text', |
| 622 | @@ -1568,7 +1568,7 @@ |
| 623 | schema = IProduct |
| 624 | step_name = 'projectaddstep2' |
| 625 | template = ViewPageTemplateFile('../templates/product-new.pt') |
| 626 | - page_title = "Register a project in Launchpad" |
| 627 | + page_title = ProjectAddStepOne.page_title |
| 628 | |
| 629 | product = None |
| 630 | |
| 631 | @@ -1690,6 +1690,7 @@ |
| 632 | class ProductAddView(MultiStepView): |
| 633 | """The controlling view for product/+new.""" |
| 634 | |
| 635 | + page_title = ProjectAddStepOne.page_title |
| 636 | total_steps = 2 |
| 637 | |
| 638 | @property |
| 639 | |
| 640 | === modified file 'lib/lp/registry/stories/distribution/xx-distribution-packages.txt' |
| 641 | --- lib/lp/registry/stories/distribution/xx-distribution-packages.txt 2009-09-12 01:57:22 +0000 |
| 642 | +++ lib/lp/registry/stories/distribution/xx-distribution-packages.txt 2009-09-16 13:44:15 +0000 |
| 643 | @@ -70,7 +70,7 @@ |
| 644 | >>> field.value = 'commercialpackage' |
| 645 | >>> browser.getControl('Search', index=0).click() |
| 646 | >>> extract_text(find_main_content(browser.contents)) |
| 647 | - u'...commercialpackage... package...' |
| 648 | + u"...commercialpackage... package..." |
| 649 | |
| 650 | Now try searching for text that we know to be in a change log entry, to |
| 651 | prove that FTI works on change logs. The text we're looking for is |
| 652 | |
| 653 | === modified file 'lib/lp/soyuz/stories/ppa/xx-ppa-navigation.txt' |
| 654 | --- lib/lp/soyuz/stories/ppa/xx-ppa-navigation.txt 2009-09-12 02:09:47 +0000 |
| 655 | +++ lib/lp/soyuz/stories/ppa/xx-ppa-navigation.txt 2009-09-17 15:14:02 +0000 |
| 656 | @@ -96,14 +96,14 @@ |
| 657 | |
| 658 | >>> anon_browser.getLink('View package details').click() |
| 659 | >>> print anon_browser.title |
| 660 | - +packages : Default PPA : Celso Providelo |
| 661 | + Packages in... |
| 662 | |
| 663 | You can see the build details of the packages in the archive by using |
| 664 | the 'View all builds' link. |
| 665 | |
| 666 | >>> anon_browser.getLink('View all builds').click() |
| 667 | >>> print anon_browser.title |
| 668 | - +builds : Default PPA : Celso Providelo |
| 669 | + Builds for... |
| 670 | |
| 671 | >>> anon_browser.url |
| 672 | 'http://launchpad.dev/~cprov/+archive/ppa/+builds' |
| 673 | |
| 674 | === modified file 'lib/lp/soyuz/stories/ppa/xx-ppa-packages.txt' |
| 675 | --- lib/lp/soyuz/stories/ppa/xx-ppa-packages.txt 2009-09-15 15:28:47 +0000 |
| 676 | +++ lib/lp/soyuz/stories/ppa/xx-ppa-packages.txt 2009-09-17 15:14:02 +0000 |
| 677 | @@ -10,7 +10,7 @@ |
| 678 | http://launchpad.dev/~cprov/+archive/ppa/+packages |
| 679 | |
| 680 | >>> print anon_browser.title |
| 681 | - +packages : PPA for Celso Providelo : Celso Providelo |
| 682 | + Packages in... |
| 683 | |
| 684 | |
| 685 | Page structure |
| 686 | @@ -20,7 +20,7 @@ |
| 687 | a descriptive main heading. |
| 688 | |
| 689 | >>> print_location(anon_browser.contents) |
| 690 | - Hierarchy: Celso Providelo > PPA for Celso Providelo > +packages |
| 691 | + Hierarchy: Celso Providelo > PPA for Celso Providelo > Packages in... |
| 692 | Tabs: |
| 693 | * Overview (selected) - http://launchpad.dev/~cprov |
| 694 | * Code - http://code.launchpad.dev/~cprov |
| 695 | @@ -35,7 +35,7 @@ |
| 696 | |
| 697 | >>> anon_browser.getLink('View all builds').click() |
| 698 | >>> print anon_browser.title |
| 699 | - +builds : PPA for Celso Providelo : Celso Providelo |
| 700 | + Builds for... |
| 701 | |
| 702 | >>> print anon_browser.url |
| 703 | http://launchpad.dev/~cprov/+archive/ppa/+builds |
| 704 | |
| 705 | === modified file 'lib/lp/soyuz/stories/soyuz/xx-builder-page.txt' |
| 706 | --- lib/lp/soyuz/stories/soyuz/xx-builder-page.txt 2009-09-12 06:49:56 +0000 |
| 707 | +++ lib/lp/soyuz/stories/soyuz/xx-builder-page.txt 2009-09-17 15:14:02 +0000 |
| 708 | @@ -114,7 +114,7 @@ |
| 709 | # We use backslashreplace because the page title includes smart quotes. |
| 710 | >>> from canonical.launchpad.helpers import backslashreplace |
| 711 | >>> print backslashreplace(cprov_browser.title) |
| 712 | - +edit : Bob The Builder : Build Farm |
| 713 | + Change details for... |
| 714 | |
| 715 | >>> title = cprov_browser.getControl(name="field.title") |
| 716 | >>> original_title = title.value |
| 717 | |
| 718 | === modified file 'lib/lp/soyuz/stories/soyuz/xx-buildfarm-index.txt' |
| 719 | --- lib/lp/soyuz/stories/soyuz/xx-buildfarm-index.txt 2009-09-10 22:08:36 +0000 |
| 720 | +++ lib/lp/soyuz/stories/soyuz/xx-buildfarm-index.txt 2009-09-17 15:14:02 +0000 |
| 721 | @@ -115,7 +115,7 @@ |
| 722 | >>> admin_browser.getLink("Register a new build machine").click() |
| 723 | |
| 724 | >>> print admin_browser.title |
| 725 | - +new : Build Farm |
| 726 | + Register a new... |
| 727 | |
| 728 | Registering a new builder involves setting its name, title, |
| 729 | description and corresponding location. |
| 730 | |
| 731 | === modified file 'lib/lp/translations/browser/pofile.py' |
| 732 | --- lib/lp/translations/browser/pofile.py 2009-09-17 08:41:05 +0000 |
| 733 | +++ lib/lp/translations/browser/pofile.py 2009-09-17 14:29:22 +0000 |
| 734 | @@ -509,6 +509,7 @@ |
| 735 | else: |
| 736 | return self.person.displayname |
| 737 | |
| 738 | + @property |
| 739 | def page_title(self): |
| 740 | """See `LaunchpadView`.""" |
| 741 | return smartquote('Translations by %s in "%s"') % ( |
| 742 | |
| 743 | === modified file 'lib/lp/translations/browser/tests/test_breadcrumbs.py' |
| 744 | --- lib/lp/translations/browser/tests/test_breadcrumbs.py 2009-09-17 10:22:14 +0000 |
| 745 | +++ lib/lp/translations/browser/tests/test_breadcrumbs.py 2009-09-17 14:29:22 +0000 |
| 746 | @@ -7,6 +7,7 @@ |
| 747 | |
| 748 | from canonical.lazr.utils import smartquote |
| 749 | |
| 750 | +from canonical.launchpad.layers import TranslationsLayer |
| 751 | from canonical.launchpad.webapp.publisher import canonical_url |
| 752 | from canonical.launchpad.webapp.tests.breadcrumbs import ( |
| 753 | BaseBreadcrumbTestCase) |
| 754 | @@ -18,7 +19,10 @@ |
| 755 | IProductSeriesLanguageSet) |
| 756 | from lp.translations.interfaces.translationgroup import ITranslationGroupSet |
| 757 | |
| 758 | + |
| 759 | class BaseTranslationsBreadcrumbTestCase(BaseBreadcrumbTestCase): |
| 760 | + request_layer = TranslationsLayer |
| 761 | + |
| 762 | def setUp(self): |
| 763 | super(BaseTranslationsBreadcrumbTestCase, self).setUp() |
| 764 | self.traversed_objects = [self.root] |
| 765 | @@ -103,12 +107,10 @@ |
| 766 | def test_translationgroupset(self): |
| 767 | group_set = getUtility(ITranslationGroupSet) |
| 768 | url = canonical_url(group_set, rootsite='translations') |
| 769 | - # Translation group listing is top-level, so no breadcrumbs show up. |
| 770 | - # Note that the first parameter is an empty list because |
| 771 | - # ITranslationGroupSet doesn't register Navigation class, and |
| 772 | - # thus doesn't show up in request.traversed_objects. |
| 773 | self._testContextBreadcrumbs( |
| 774 | - [], [], [], |
| 775 | + [group_set], |
| 776 | + ['http://translations.launchpad.dev/+groups'], |
| 777 | + ['Translation groups'], |
| 778 | url=url) |
| 779 | |
| 780 | def test_translationgroup(self): |
| 781 |
= Summary =
Change text of leaf breadcrumbs from the page name (+export, etc) to its
title, fixing https:/
== Proposed fix ==
Previously, only objects which had a Navigation class would be appended
to request.
objects will be appended there, including the views. This fixes
https:/
Since we now have the view in traversed_objects, we can just use
its .page_title (falling back to its entry on pagetitles.py) as the text
of the leaf breadcrumb. Like before, though, this leaf breadcrumb is
only added when the view is not the default one for the context.
== Implementation details ==
MultiStepViews don't define a .page_title because what actually gets
rendered is the (step) view that they contain, and those define
a .page_title. However, we don't want to do that when looking up the
text for the breadcrumbs, so MultiStepViews are now required to provide
a .page_title.
Some tests from doc/navigation.txt where moved to
webapp/
webapp/
== Tests ==
./bin/test -vvt test_breadcrumbs
== Demo and Q/A ==
= Launchpad lint =
Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.
Linting changed files:
lib/canonical
lib/canonical
lib/lp/
lib/canonical
lib/canonical
lib/canonical
lib/canonical
lib/lp/
lib/lp/
lib/canonical
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/canonical
lib/lp/
lib/lp/
lib/canonical
lib/canonical
lib/canonical
lib/canonical
lib/lp/
lib/lp/
== Pylint notices ==
lib/canonical/
551: [E1002,
LaunchpadBrowse
an old style class
lib/lp/
57: [F0401] Unable to import 'lazr.delegates' (No module named
delegates)
lib/lp/
21: [F0401] Unable to import 'lazr.enum' (No module named enum)
22: [F0401] Unable to import 'lazr.lifecycle
lifecycle)
lib/canonical/
23: [F0401] Unable to import 'lazr.enum' (No module named enum)