Launchpad itself

Merge lp:~thumper/launchpad/fix-canonical-url-for-api-2 into lp:launchpad

  1. fix-canonical-url-for-api-2
  2. Merge into devel
Proposed by Tim Penhey
Status: Merged
Approved by: Tim Penhey
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~thumper/launchpad/fix-canonical-url-for-api-2
Merge into: lp:launchpad
Diff against target: 122 lines (+32/-15)
4 files modified
lib/canonical/launchpad/doc/canonical_url.txt (+9/-0)
lib/canonical/launchpad/doc/tales.txt (+6/-1)
lib/canonical/launchpad/webapp/publisher.py (+6/-4)
lib/canonical/launchpad/webapp/tales.py (+11/-10)
To merge this branch: bzr merge lp:~thumper/launchpad/fix-canonical-url-for-api-2
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
Review via email: mp+14876@code.launchpad.net

Commit message

Fix the fmt:api_url to return the relative path only for URLs.

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

The current implementation of the api_url tales formatter doesn't do what it thinks it does. Setting the rootsite to None does not mean that the site isn't shown.

To fix this, I've added an option to canonical_url to force the local path only. I've also fixed the doctest for tales to make a distribution instead of a product where the example clearly indicates that the object should be a distribution. Also added the missing api_url to the Branch formatter, and added tests to cover these areas.

Revision history for this message
Curtis Hovey (sinzui) wrote :

This looks good to land. I have no comment.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/doc/canonical_url.txt'
--- lib/canonical/launchpad/doc/canonical_url.txt 2009-08-19 23:13:23 +0000
+++ lib/canonical/launchpad/doc/canonical_url.txt 2009-11-14 22:50:26 +0000
@@ -520,6 +520,15 @@
520 >>> canonical_url(countryset_instance, request=api_request)520 >>> canonical_url(countryset_instance, request=api_request)
521 u'http://api.launchpad.dev/countries'521 u'http://api.launchpad.dev/countries'
522522
523It is often the case that the web application wants to provide URLs that will
524be written out onto the pages that the Javascript can process using the
525LP.client code to get access to the object entries using the API. In these
526cases, the "force_local_path" parameter can be passed to canonical_url to have
527only the relative local path returned.
528
529 >>> canonical_url(countryset_instance, force_local_path=True)
530 u'/countries'
531
523532
524== The end ==533== The end ==
525534
526535
=== modified file 'lib/canonical/launchpad/doc/tales.txt'
--- lib/canonical/launchpad/doc/tales.txt 2009-11-05 10:51:36 +0000
+++ lib/canonical/launchpad/doc/tales.txt 2009-11-14 22:50:26 +0000
@@ -319,10 +319,15 @@
319 >>> print test_tales("product/fmt:api_url", product=freewidget)319 >>> print test_tales("product/fmt:api_url", product=freewidget)
320 /freewidget320 /freewidget
321321
322 >>> debuntu = factory.makeProduct(name='debuntu')322 >>> debuntu = factory.makeDistribution(name='debuntu')
323 >>> print test_tales("distro/fmt:api_url", distro=debuntu)323 >>> print test_tales("distro/fmt:api_url", distro=debuntu)
324 /debuntu324 /debuntu
325325
326 >>> branch = factory.makeProductBranch(
327 ... owner=bob, product=freewidget, name='fix-bug')
328 >>> print test_tales("branch/fmt:api_url", branch=branch)
329 /~bob/freewidget/fix-bug
330
326 >>> login(ANONYMOUS)331 >>> login(ANONYMOUS)
327332
328333
329334
=== modified file 'lib/canonical/launchpad/webapp/publisher.py'
--- lib/canonical/launchpad/webapp/publisher.py 2009-09-18 01:34:06 +0000
+++ lib/canonical/launchpad/webapp/publisher.py 2009-11-14 22:50:26 +0000
@@ -430,7 +430,7 @@
430430
431def canonical_url(431def canonical_url(
432 obj, request=None, rootsite=None, path_only_if_possible=False,432 obj, request=None, rootsite=None, path_only_if_possible=False,
433 view_name=None):433 view_name=None, force_local_path=False):
434 """Return the canonical URL string for the object.434 """Return the canonical URL string for the object.
435435
436 If the canonical url configuration for the given object binds it to a436 If the canonical url configuration for the given object binds it to a
@@ -455,6 +455,7 @@
455 for the current request, return a url containing only the path.455 for the current request, return a url containing only the path.
456 :param view_name: Provide the canonical url for the specified view,456 :param view_name: Provide the canonical url for the specified view,
457 rather than the default view.457 rather than the default view.
458 :param force_local_path: Strip off the site no matter what.
458 :raises: NoCanonicalUrl if a canonical url is not available.459 :raises: NoCanonicalUrl if a canonical url is not available.
459 """460 """
460 urlparts = [urldata.path461 urlparts = [urldata.path
@@ -517,9 +518,10 @@
517 root_url = request.getRootURL(rootsite)518 root_url = request.getRootURL(rootsite)
518519
519 path = u'/'.join(reversed(urlparts))520 path = u'/'.join(reversed(urlparts))
520 if (path_only_if_possible and521 if ((path_only_if_possible and
521 request is not None and522 request is not None and
522 root_url.startswith(request.getApplicationURL())523 root_url.startswith(request.getApplicationURL()))
524 or force_local_path
523 ):525 ):
524 return unicode('/' + path)526 return unicode('/' + path)
525 return unicode(root_url + path)527 return unicode(root_url + path)
526528
=== modified file 'lib/canonical/launchpad/webapp/tales.py'
--- lib/canonical/launchpad/webapp/tales.py 2009-10-22 13:06:53 +0000
+++ lib/canonical/launchpad/webapp/tales.py 2009-11-14 22:50:26 +0000
@@ -471,8 +471,7 @@
471 try:471 try:
472 url = canonical_url(472 url = canonical_url(
473 self._context, path_only_if_possible=True,473 self._context, path_only_if_possible=True,
474 rootsite=rootsite,474 rootsite=rootsite, view_name=view_name)
475 view_name=view_name)
476 except Unauthorized:475 except Unauthorized:
477 url = ""476 url = ""
478 return url477 return url
@@ -480,14 +479,15 @@
480 def api_url(self, context):479 def api_url(self, context):
481 """Return the object's (partial) canonical web service URL.480 """Return the object's (partial) canonical web service URL.
482481
483 This method returns everything that goes after the web service482 This method returns everything that goes after the web service version
484 version number. It's the same as 'url', but without any view483 number. Effectively the canonical URL but only the relative part with
485 name.484 no site.
486 """485 """
487486 try:
488 # Some classes override the rootsite. We always want a path-only487 url = canonical_url(self._context, force_local_path=True)
489 # URL, so we override it to nothing.488 except Unauthorized:
490 return self.url(rootsite=None)489 url = ""
490 return url
491491
492 def traverse(self, name, furtherPath):492 def traverse(self, name, furtherPath):
493 if name.startswith('link:') or name.startswith('url:'):493 if name.startswith('link:') or name.startswith('url:'):
@@ -1368,7 +1368,8 @@
13681368
1369 traversable_names = {1369 traversable_names = {
1370 'link': 'link', 'url': 'url', 'project-link': 'projectLink',1370 'link': 'link', 'url': 'url', 'project-link': 'projectLink',
1371 'title-link': 'titleLink', 'bzr-link': 'bzrLink'}1371 'title-link': 'titleLink', 'bzr-link': 'bzrLink',
1372 'api_url': 'api_url'}
13721373
1373 def _args(self, view_name):1374 def _args(self, view_name):
1374 """Generate a dict of attributes for string template expansion."""1375 """Generate a dict of attributes for string template expansion."""