launchpad-reviewers team mailing list archive

[Martin Pool <martinpool@xxxxxxxxx>]

Martin Pool has proposed merging lp:~mbp/launchpad/squiggle-me into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #785800 in Launchpad itself: "/people/+me is long; hard to remember"
  https://bugs.launchpad.net/launchpad/+bug/785800

For more details, see:
https://code.launchpad.net/~mbp/launchpad/squiggle-me/+merge/66007

https://launchpad.net/~ now redirects you to your home page; also https://launchpad.net/~/+editsshkeys and https://bugs.launchpad.net/~/+assignedbugs

Thanks sinzui, flacoste

per bug 785800
-- 
https://code.launchpad.net/~mbp/launchpad/squiggle-me/+merge/66007
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~mbp/launchpad/squiggle-me into lp:launchpad.

=== modified file 'lib/lp/app/browser/launchpad.py'
--- lib/lp/app/browser/launchpad.py	2011-06-20 14:12:13 +0000
+++ lib/lp/app/browser/launchpad.py	2011-06-27 15:39:31 +0000
@@ -647,10 +647,20 @@
         if name in self.stepto_utilities:
             return getUtility(self.stepto_utilities[name])
 
-        # Allow traversal to ~foo for People
-        if name.startswith('~'):
-            # account for common typing mistakes
+        if name == '~':
+            person = getUtility(ILaunchBag).user
+            if person is None:
+                raise Unauthorized()
+            # Keep the context and the subtree so that
+            # bugs.l.n/~/+assignedbugs goes to the person's canonical
+            # assigned list.
+            return self.redirectSubTree(
+                canonical_url(self.context) + "~"
+                + canonical_name(person.name),
+                status=302)
+        elif name.startswith('~'):  # Allow traversal to ~foo for People
             if canonical_name(name) != name:
+                # (for instance, uppercase username?)
                 if self.request.method == 'POST':
                     raise POSTToNonCanonicalURL
                 return self.redirectSubTree(

=== modified file 'lib/lp/app/browser/tests/test_launchpad.py'
--- lib/lp/app/browser/tests/test_launchpad.py	2011-06-03 20:13:14 +0000
+++ lib/lp/app/browser/tests/test_launchpad.py	2011-06-27 15:39:31 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for traversal from the root branch object."""
@@ -9,11 +9,17 @@
 
 from zope.component import getUtility
 from zope.publisher.interfaces import NotFound
+from zope.security.interfaces import Unauthorized
 from zope.security.proxy import removeSecurityProxy
 
 from canonical.launchpad.interfaces.account import AccountStatus
-from canonical.launchpad.webapp import canonical_url
-from canonical.launchpad.webapp.interfaces import BrowserNotificationLevel
+from canonical.launchpad.webapp import (
+    canonical_url,
+    )
+from canonical.launchpad.webapp.interfaces import (
+    BrowserNotificationLevel,
+    ILaunchpadRoot,
+    )
 from canonical.launchpad.webapp.servers import LaunchpadTestRequest
 from canonical.launchpad.webapp.url import urlappend
 from canonical.testing.layers import DatabaseFunctionalLayer
@@ -32,6 +38,7 @@
     person_logged_in,
     TestCaseWithFactory,
     )
+from lp.testing.publication import test_traverse
 from lp.testing.views import create_view
 
 # We set the request header HTTP_REFERER  when we want to simulate navigation
@@ -93,24 +100,30 @@
         self.assertEqual(url, redirection.target)
 
     def traverse(self, path, first_segment, use_default_referer=True):
-        """Traverse to 'segments' using a 'LaunchpadRootNavigation' object.
+        """Traverse to 'path' using a 'LaunchpadRootNavigation' object.
 
         Using the Zope traversal machinery, traverse to the path given by
         'segments', starting at a `LaunchpadRootNavigation` object.
 
-        :param segments: A list of path segments.
+        CAUTION: Prefer test_traverse to this method, because it correctly
+        establishes the global request.
+
+        :param path: A slash-delimited path.
         :param use_default_referer: If True, set the referer attribute in the
             request header to DEFAULT_REFERER = "http://launchpad.dev";
             (otherwise it remains as None)
         :return: The object found.
         """
+        # XXX: What's the difference between first_segment and path? -- mbp
+        # 2011-06-27.
         extra = {'PATH_INFO': urlappend('/%s' % first_segment, path)}
         if use_default_referer:
             extra['HTTP_REFERER'] = DEFAULT_REFERER
         request = LaunchpadTestRequest(**extra)
         segments = reversed(path.split('/'))
         request.setTraversalStack(segments)
-        traverser = LaunchpadRootNavigation(None, request=request)
+        traverser = LaunchpadRootNavigation(
+            getUtility(ILaunchpadRoot), request=request)
         return traverser.publishTraverse(request, first_segment)
 
 
@@ -368,6 +381,42 @@
         login_person(self.any_user)
         self.assertRaises(NotFound, self.traverse, segment, segment)
 
+    def test_self_url_traversal(self):
+        # Just /~/ expands to the current user.  (Bug 785800).
+        person = self.factory.makePerson()
+        login_person(person)
+        obj, view, req = test_traverse('http://launchpad.dev/~')
+        view = removeSecurityProxy(view)
+        self.assertEqual(
+            canonical_url(person),
+            view.target.rstrip('/'))
+
+    def test_self_url_not_logged_in(self):
+        # /~/ when not logged in asks you to log in.
+        self.assertRaises(Unauthorized,
+            test_traverse, 'http://launchpad.dev/~')
+
+    def test_self_url_pathinfo(self):
+        # You can traverse below /~/.
+        person = self.factory.makePerson()
+        login_person(person)
+        obj, view, req = test_traverse('http://launchpad.dev/~/+editsshkeys')
+        view = removeSecurityProxy(view)
+        self.assertEqual(
+            canonical_url(person) + '/+editsshkeys',
+            view.target)
+
+    def test_self_url_app_domain(self):
+        # You can traverse below /~/.
+        person = self.factory.makePerson()
+        login_person(person)
+        obj, view, req = test_traverse('http://bugs.launchpad.dev/~')
+        view = removeSecurityProxy(view)
+        self.assertEqual(
+            canonical_url(person, rootsite='bugs'),
+            view.target.rstrip('/'))
+
+
 
 class TestErrorViews(TestCaseWithFactory):