Merge lp:~wgrant/launchpad/fix-ajax-subscription-bug-415166 into lp:launchpad

[William Grant]

[William Grant (wgrant)]

= Summary =

fmt:link and fmt:url were recently changed to always return links to the mainsite for people and pillars. This change broke an untested formatter -- fmt:api_url -- in a way which causes LP.client.links['me'] to be a full webapp URL with domain, instead of the previous path-only URL. This breaks inline bug subscription (bug #415166).

== Proposed fix ==

The change causing problems changed the default value of rootsite from None to 'mainsite' in {Person,Team,Pillar}FormatterAPI.{url,link}. The proposed fix is to change ObjectFormatterAPI.api_url to force a rootsite of None, which causes URLs to be path-only again.

== Pre-implementation notes ==

This was discussed with BjornT and deryck. They agreed with the change as described above.

== Implementation details ==

See above.

== Tests ==

There were previously no tests of fmt:api_url, so I added a new section to lib/canonical/launchpad/doc/tales.txt. This also required modification of a later test in that file to not depend on the order in which factory objects are created.

 $ bin/test -vv -t tales.txt

== Demo and Q/A ==

To Q/A:

 * Log on as any user.
 * Navigate to a bug.
 * Click on 'Subscribe'.
 * Verify that you were subscribed via AJAX.

[William Grant (wgrant)]

There are still a couple of subscription issues which are separate enough that they don't really belong in this branch. Team unsubscription and incorrect URLs in generated links are bug #415229.

[Henning Eggers (henninge)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,
thanks for fixing this bug which just bit me this morning and I thought
"I ought to file a bug for this." And there you come along with a fix
already. Great!

I also like your fix very much, especially fixing the test to not rely
on (kind of) random numbers is much appreciated.

So, nothing I can find to complain about ;-), this branch is good to land.

 review: approve

Cheers,
Henning
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkqKX/gACgkQBT3oW1L17ihcFgCdELSgLqw0uRwR2eprULotwQDJ
OFAAnA8N+1awSiti8o/JkRwbhC6Rpdnx
=1AgZ
-----END PGP SIGNATURE-----

review: Approve

=== modified file 'lib/canonical/launchpad/doc/tales.txt'
--- lib/canonical/launchpad/doc/tales.txt	2009-08-13 21:32:59 +0000
+++ lib/canonical/launchpad/doc/tales.txt	2009-08-18 05:00:54 +0000
@@ -289,6 +289,30 @@
     >>> login(ANONYMOUS)
 
 
+The fmt: namespace to get a web service URL
+-------------------------------------------
+
+The `fmt:api_url` expression gives you the absolute API path to an object.
+This path is everything after the web service version number.
+
+    >>> login(ANONYMOUS,
+    ...     LaunchpadTestRequest(SERVER_URL='http://bugs.launchpad.net'))
+
+    >>> bob = factory.makePerson(name='bob')
+    >>> print test_tales("person/fmt:api_url", person=bob)
+    /~bob
+
+    >>> freewidget = factory.makeProduct(name='freewidget')
+    >>> print test_tales("product/fmt:api_url", product=freewidget)
+    /freewidget
+
+    >>> debuntu = factory.makeProduct(name='debuntu')
+    >>> print test_tales("distro/fmt:api_url", distro=debuntu)
+    /debuntu
+
+    >>> login(ANONYMOUS)
+
+
 The fmt: namespace to get links
 -------------------------------
 
@@ -456,20 +480,22 @@
 Branch subscriptions show the person and branch name.  For users without
 adequate permissions, a link is not generated.
 
-    >>> branch = factory.makeAnyBranch(title='My Branch')
-    >>> person = factory.makePerson(displayname='My Person')
+    >>> branch = factory.makeProductBranch(
+    ...     owner=eric, product=fooix, name='my-branch', title='My Branch')
+    >>> michael = factory.makePerson(
+    ...     name='michael', displayname='Michael the Viking')
     >>> subscription = factory.makeBranchSubscription(
-    ...     branch=branch, person=person)
+    ...     branch=branch, person=michael)
     >>> test_tales("subscription/fmt:link", subscription=subscription)
-    u'Subscription of My Person to
-      lp://dev/~person-name17/product-name12/branch19'
+    u'Subscription of Michael the Viking to
+      lp://dev/~eric/fooix/my-branch'
 
 But if we log in as the subscriber, a link is presented.
 
     >>> login_person(subscription.person)
     >>> test_tales("subscription/fmt:link", subscription=subscription)
-    u'<a href="http://.../+subscription/person-name21">Subscription
-      of My Person to lp://dev/~person-name17/product-name12/branch19</a>'
+    u'<a href="http://.../+subscription/michael">Subscription
+      of Michael the Viking to lp://dev/~eric/fooix/my-branch</a>'
 
 Merge proposals also have a link formatter, which displays branch
 titles:

=== modified file 'lib/canonical/launchpad/webapp/tales.py'
--- lib/canonical/launchpad/webapp/tales.py	2009-08-14 12:56:19 +0000
+++ lib/canonical/launchpad/webapp/tales.py	2009-08-18 05:03:00 +0000
@@ -455,7 +455,10 @@
         version number. It's the same as 'url', but without any view
         name.
         """
-        return self.url()
+
+        # Some classes override the rootsite. We always want a path-only
+        # URL, so we override it to nothing.
+        return self.url(rootsite=None)
 
     def traverse(self, name, furtherPath):
         if name.startswith('link:') or name.startswith('url:'):