Merge lp:~thomir-deactivatedaccount/launchpad/devel-fix-git-links into lp:launchpad

[Thomi Richards]

Links to git repositories are now linked correctly.

Make links to git repositories work in launchpad.

There are several changes here:

 * Links to bzr branches or git repos in the form lp:<path> now generate expanded links in the form: /+code/<path> instead of /+branch/<path>. The +branch form still exists, as it's used by bzr clients, but it only deals with bazaar branches.

 * The link checker API method now deals with +code links, instead of +branch links.

 * +code links now traverse correctly to git repositories or bzr branches.

[Colin Watson (cjwatson)]

I won't mention it at each site, but please sort imports. utilities/format-new-and-modified-imports can help with this (though it's worth double-checking what it does). Note also that imports are normally sorted case-insensitively in LP, even though I personally find that a bit weird.

I think some more tests should be updated:

lib/lp/app/doc/displaying-paragraphs-of-text.txt
lib/lp/app/javascript/tests/test_lp_links.*

... and the comment in lib/lp/app/javascript/lp-links.js should probably be updated too.

review: Needs Fixing

[Thomi Richards (thomir-deactivatedaccount)]

Thanks for the review - I've made the changes requested.

I also have a few questions:

I notice we have a few instances of "+branch" in blueprints - I'm not sure what for, or if they can be trivially updated to "+code" - it seemed like it would be more than a simple s/branch/code/ operation...

Should lp.code.browser.tests.test_branchtraversal be updated to use +code instead of +branch?

There are various other instances of '+branch' in the codebase, and I'm not sure which of them, if any need updating.

[Thomi Richards (thomir-deactivatedaccount)]

Ugh - there's a number of failing tests in doctests. I need to spend some time figuring out how to fix them. In the meantime, this isn't ready for your review.

[Thomi Richards (thomir-deactivatedaccount)]

Tests are fixed now. William helped point me at the problem & solution.

Ready for a review, if you have the time.

[Colin Watson (cjwatson)]

The instances of +branch in blueprints are traversals under Specification, which shouldn't be changed here. (Linking directly to Git branches there wouldn't be appropriate, but we may at some point allow them to be linked to merge proposals, much like the similar work I'm doing for bugs.)

I think the traversal tests in lp.code.browser.tests.test_branchtraversal are under Person rather than under the root, so they also shouldn't be changed.

I'm happy with most of this now, but there is one oddity. Revision 18107 in your branch seems to have partially reverted some previous fixes you applied in response to my previous review feedback, so e.g. it still has the dubious check for '+git' as a path substring. Was there a particular reason for that, or was it an accident? If that was in response to some test failures (query counts maybe?), I think we need to find a different fix to those. So I'm casting an Approve review in order not to block you, but please do fix that up before landing.

review: Approve

[Thomi Richards (thomir-deactivatedaccount)]

> I'm happy with most of this now, but there is one oddity. Revision 18107 in
> your branch seems to have partially reverted some previous fixes you applied
> in response to my previous review feedback, so e.g. it still has the dubious
> check for '+git' as a path substring. Was there a particular reason for that,
> or was it an accident?

Ugh - that was an accident. Reverting now.

=== modified file 'lib/lp/app/browser/configure.zcml'
--- lib/lp/app/browser/configure.zcml	2016-05-12 14:08:38 +0000
+++ lib/lp/app/browser/configure.zcml	2016-06-22 21:05:27 +0000
@@ -1118,7 +1118,7 @@
     <!-- virtual host: code -->
     <browser:defaultView
         for="lp.services.webapp.interfaces.ILaunchpadRoot"
-        name="+code"
+        name="+code-index"
         layer="lp.code.publisher.CodeLayer"
         />
     <browser:page

=== modified file 'lib/lp/app/browser/launchpad.py'
--- lib/lp/app/browser/launchpad.py	2016-05-10 10:53:32 +0000
+++ lib/lp/app/browser/launchpad.py	2016-06-22 21:05:27 +0000
@@ -78,6 +78,7 @@
     )
 from lp.app.errors import (
     GoneError,
+    NameLookupFailed,
     NotFoundError,
     POSTToNonCanonicalURL,
     )
@@ -103,9 +104,11 @@
 from lp.code.interfaces.branchlookup import IBranchLookup
 from lp.code.interfaces.codehosting import IBazaarApplication
 from lp.code.interfaces.codeimport import ICodeImportSet
+from lp.code.interfaces.gitlookup import IGitLookup
 from lp.code.interfaces.gitrepository import IGitRepositorySet
 from lp.hardwaredb.interfaces.hwdb import IHWDBApplication
 from lp.layers import WebServiceLayer
+from lp.registry.enums import VCSType
 from lp.registry.interfaces.announcement import IAnnouncementSet
 from lp.registry.interfaces.codeofconduct import ICodeOfConductSet
 from lp.registry.interfaces.distribution import IDistributionSet
@@ -767,6 +770,68 @@
 
         return self.redirectSubTree(target_url)
 
+    @stepto('+code')
+    def redirect_branch_or_repo(self):
+        """Redirect /+code/<foo> to the branch or repository named 'foo'.
+
+        'foo' can be the unique name of the branch/repo, or any of the aliases
+        for the branch/repo.
+
+        If 'foo' is invalid, or exists but the user does not have permission to
+        view 'foo', a NotFoundError is raised, resulting in a 404 error.
+
+        Unlike +branch, this works for both git and bzr repositories/branches.
+        """
+        target_url = None
+        path = '/'.join(self.request.stepstogo)
+
+        # Try a Git repository lookup first, since the schema is simpler and
+        # so it's quicker.
+        try:
+            repository, trailing = getUtility(IGitLookup).getByPath(path)
+            if repository is not None:
+                target_url = canonical_url(repository)
+                if trailing:
+                    target_url = urlappend(target_url, trailing)
+        except (InvalidNamespace, InvalidProductName, NameLookupFailed,
+                Unauthorized):
+            # Either the git repository wasn't found, or it was found but we
+            # lack authority to access it. In either case, attempt a bzr lookup
+            # so don't set target_url
+            pass
+
+        # Attempt a bzr lookup as well:
+        try:
+            branch, trailing = getUtility(IBranchLookup).getByLPPath(path)
+            bzr_url = canonical_url(branch)
+            if trailing != '':
+                bzr_url = urlappend(bzr_url, trailing)
+
+            if target_url and branch.product is not None:
+                # Project has both a bzr branch and a git repo. There's no
+                # right thing we can do here, so pretend we didn't see
+                # anything at all.
+                if branch.product.vcs is None:
+                    target_url = None
+                # if it's set to BZR, then set this branch as the target
+                if branch.product.vcs == VCSType.BZR:
+                    target_url = bzr_url
+            else:
+                target_url = bzr_url
+        except (NoLinkedBranch, CannotHaveLinkedBranch, InvalidNamespace,
+                InvalidProductName, NotFoundError):
+            # No bzr branch found either.
+            pass
+
+        # Either neither bzr nor git returned matches, or they did but we're
+        # not authorised to view them, or they both did and the project has not
+        # set its 'vcs' property to indicate which one to prefer. In all cases
+        # raise a 404:
+        if not target_url:
+            raise NotFoundError
+
+        return self.redirectSubTree(target_url)
+
     @stepto('+builds')
     def redirect_buildfarm(self):
         """Redirect old /+builds requests to new URL, /builders."""
@@ -785,7 +850,7 @@
         'branches': IBranchSet,
         'bugs': IMaloneApplication,
         'builders': IBuilderSet,
-        '+code': IBazaarApplication,
+        '+code-index': IBazaarApplication,
         '+code-imports': ICodeImportSet,
         'codeofconduct': ICodeOfConductSet,
         '+countries': ICountrySet,

=== modified file 'lib/lp/app/browser/linkchecker.py'
--- lib/lp/app/browser/linkchecker.py	2013-01-07 02:40:55 +0000
+++ lib/lp/app/browser/linkchecker.py	2016-06-22 21:05:27 +0000
@@ -19,6 +19,7 @@
     NoSuchBranch,
     )
 from lp.code.interfaces.branchlookup import IBranchLookup
+from lp.code.interfaces.gitlookup import IGitLookup
 from lp.registry.interfaces.product import InvalidProductName
 from lp.services.searchbuilder import any
 from lp.services.webapp import LaunchpadView
@@ -34,7 +35,7 @@
     something appropriate.
 
     This initial implementation supports processing links like the following:
-        /+branch/foo/bar
+        /+code/foo/bar
 
     The implementation can easily be extended to handle other forms by
     providing a method to handle the link type extracted from the json
@@ -68,17 +69,19 @@
         return simplejson.dumps(result)
 
     def check_branch_links(self, links):
-        """Check links of the form /+branch/foo/bar"""
+        """Check links of the form /+code/foo/bar"""
         invalid_links = {}
-        branch_lookup = getUtility(IBranchLookup)
+        bzr_branch_lookup = getUtility(IBranchLookup)
+        git_branch_lookup = getUtility(IGitLookup)
         for link in links:
-            path = link.strip('/')[len('+branch/'):]
-            try:
-                branch_lookup.getByLPPath(path)
-            except (CannotHaveLinkedBranch, InvalidNamespace,
-                    InvalidProductName, NoLinkedBranch, NoSuchBranch,
-                    NotFoundError) as e:
-                invalid_links[link] = self._error_message(e)
+            path = link.strip('/')[len('+code/'):]
+            if git_branch_lookup.getByPath(path)[0] is None:
+                try:
+                    bzr_branch_lookup.getByLPPath(path)
+                except (CannotHaveLinkedBranch, InvalidNamespace,
+                        InvalidProductName, NoLinkedBranch, NoSuchBranch,
+                        NotFoundError) as e:
+                    invalid_links[link] = self._error_message(e)
         return {'invalid': invalid_links}
 
     def check_bug_links(self, links):

=== modified file 'lib/lp/app/browser/stringformatter.py'
--- lib/lp/app/browser/stringformatter.py	2015-10-26 14:54:43 +0000
+++ lib/lp/app/browser/stringformatter.py	2016-06-22 21:05:27 +0000
@@ -455,7 +455,7 @@
             if path.isdigit():
                 return FormattersAPI._linkify_bug_number(
                     lp_url, path, trailers)
-            url = '/+branch/%s' % path
+            url = '/+code/%s' % path
             # Mark the links with a 'branch-short-link' class so they can be
             # harvested and validated when the page is rendered.
             return structured(

=== modified file 'lib/lp/app/browser/tests/test_launchpad.py'
--- lib/lp/app/browser/tests/test_launchpad.py	2016-04-11 06:38:48 +0000
+++ lib/lp/app/browser/tests/test_launchpad.py	2016-06-22 21:05:27 +0000
@@ -21,10 +21,12 @@
 from lp.app.errors import GoneError
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.app.interfaces.services import IService
+from lp.code.interfaces.gitrepository import IGitRepositorySet
 from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch
 from lp.registry.enums import (
     PersonVisibility,
     SharingPermission,
+    VCSType,
     )
 from lp.registry.interfaces.person import IPersonSet
 from lp.services.identity.interfaces.account import AccountStatus
@@ -37,10 +39,11 @@
 from lp.services.webapp.servers import LaunchpadTestRequest
 from lp.services.webapp.url import urlappend
 from lp.testing import (
+    admin_logged_in,
     ANONYMOUS,
     celebrity_logged_in,
+    login_person,
     login,
-    login_person,
     person_logged_in,
     TestCaseWithFactory,
     )
@@ -332,6 +335,253 @@
         self.assertNotFound("_foo", use_default_referer=False)
 
 
+class TestCodeTraversal(TestCaseWithFactory, TraversalMixin):
+
+    layer = DatabaseFunctionalLayer
+
+    def traverse(self, path, **kwargs):
+        return super(TestCodeTraversal, self).traverse(
+            path, '+code', **kwargs)
+
+    def test_project_bzr_branch(self):
+        branch = self.factory.makeAnyBranch()
+        self.assertRedirects(branch.unique_name, canonical_url(branch))
+
+    def test_project_git_branch(self):
+        git_repo = self.factory.makeGitRepository()
+        self.assertRedirects(git_repo.unique_name, canonical_url(git_repo))
+
+    def test_no_such_bzr_unique_name(self):
+        branch = self.factory.makeAnyBranch()
+        bad_name = branch.unique_name + 'wibble'
+        self.assertNotFound(bad_name)
+
+    def test_no_such_git_unique_name(self):
+        repo = self.factory.makeGitRepository()
+        bad_name = repo.unique_name + 'wibble'
+        self.assertNotFound(bad_name)
+
+    def test_private_bzr_branch(self):
+        branch = self.factory.makeProductBranch(
+            information_type=InformationType.USERDATA)
+        branch_unique_name = removeSecurityProxy(branch).unique_name
+        login(ANONYMOUS)
+        self.assertNotFound(branch_unique_name)
+
+    def test_private_git_branch(self):
+        git_repo = self.factory.makeGitRepository(
+            information_type=InformationType.USERDATA)
+        repo_unique_name = removeSecurityProxy(git_repo).unique_name
+        login(ANONYMOUS)
+        self.assertNotFound(repo_unique_name)
+
+    def test_product_alias_bzr(self):
+        branch = self.factory.makeProductBranch()
+        naked_product = removeSecurityProxy(branch.product)
+        ICanHasLinkedBranch(naked_product).setBranch(branch)
+        self.assertRedirects(naked_product.name, canonical_url(branch))
+
+    def test_product_alias_git(self):
+        project = self.factory.makeProduct()
+        repo = self.factory.makeGitRepository(target=project)
+        naked_project = removeSecurityProxy(project)
+        with person_logged_in(repo.target.owner):
+            getUtility(IGitRepositorySet).setDefaultRepository(
+                repo.target, repo)
+        self.assertRedirects(naked_project.name, canonical_url(repo))
+
+    def test_private_bzr_branch_for_product(self):
+        branch = self.factory.makeProductBranch()
+        naked_product = removeSecurityProxy(branch.product)
+        ICanHasLinkedBranch(naked_product).setBranch(branch)
+        removeSecurityProxy(branch).information_type = (
+            InformationType.USERDATA)
+        login(ANONYMOUS)
+        self.assertNotFound(naked_product.name)
+
+    def test_private_git_branch_for_product(self):
+        project = self.factory.makeProduct()
+        repo = self.factory.makeGitRepository(target=project)
+        with person_logged_in(repo.target.owner):
+            getUtility(IGitRepositorySet).setDefaultRepository(
+                repo.target, repo)
+
+        removeSecurityProxy(repo).information_type = (
+            InformationType.USERDATA)
+        login(ANONYMOUS)
+
+        naked_project = removeSecurityProxy(project)
+        self.assertNotFound(naked_project.name)
+
+    def test_nonexistent_product(self):
+        non_existent = 'non-existent'
+        self.assertNotFound(non_existent)
+
+    def test_product_without_dev_focus(self):
+        product = self.factory.makeProduct()
+        self.assertNotFound(product.name)
+
+    def test_distro_package_alias_bzr(self):
+        sourcepackage = self.factory.makeSourcePackage()
+        branch = self.factory.makePackageBranch(sourcepackage=sourcepackage)
+        distro_package = sourcepackage.distribution_sourcepackage
+        registrant = distro_package.distribution.owner
+        target = ICanHasLinkedBranch(distro_package)
+        with person_logged_in(registrant):
+            target.setBranch(branch, registrant)
+        self.assertRedirects("%s" % target.bzr_path, canonical_url(branch))
+
+    def test_distro_package_alias_git(self):
+        sourcepackage = self.factory.makeSourcePackage()
+        distro_package = sourcepackage.distribution_sourcepackage
+        repo = self.factory.makeGitRepository(target=distro_package)
+
+        with admin_logged_in():
+            getUtility(IGitRepositorySet).setDefaultRepository(
+                distro_package, repo)
+
+        self.assertRedirects("%s" % repo.shortened_path, canonical_url(repo))
+
+    def test_private_branch_for_distro_package_bzr(self):
+        sourcepackage = self.factory.makeSourcePackage()
+        branch = self.factory.makePackageBranch(
+            sourcepackage=sourcepackage,
+            information_type=InformationType.USERDATA)
+        distro_package = sourcepackage.distribution_sourcepackage
+        registrant = distro_package.distribution.owner
+        with person_logged_in(registrant):
+            ICanHasLinkedBranch(distro_package).setBranch(branch, registrant)
+        login(ANONYMOUS)
+        path = ICanHasLinkedBranch(distro_package).bzr_path
+        self.assertNotFound(path)
+
+    def test_private_branch_for_distro_package_git(self):
+        sourcepackage = self.factory.makeSourcePackage()
+        distro_package = sourcepackage.distribution_sourcepackage
+        repo = self.factory.makeGitRepository(
+            target=distro_package,
+            information_type=InformationType.USERDATA)
+        with admin_logged_in():
+            getUtility(IGitRepositorySet).setDefaultRepository(
+                distro_package, repo)
+        login(ANONYMOUS)
+        path = removeSecurityProxy(repo).shortened_path
+        self.assertNotFound(path)
+
+    def test_trailing_path_redirect_bzr(self):
+        branch = self.factory.makeAnyBranch()
+        path = urlappend(branch.unique_name, '+edit')
+        self.assertRedirects(path, canonical_url(branch, view_name='+edit'))
+
+    def test_trailing_path_redirect_git(self):
+        repo = self.factory.makeGitRepository()
+        path = urlappend(repo.unique_name, '+edit')
+        self.assertRedirects(path, canonical_url(repo, view_name='+edit'))
+
+    def test_alias_trailing_path_redirect_bzr(self):
+        branch = self.factory.makeProductBranch()
+        with person_logged_in(branch.product.owner):
+            branch.product.development_focus.branch = branch
+        path = '%s/+edit' % branch.product.name
+        self.assertRedirects(path, canonical_url(branch, view_name='+edit'))
+
+    def test_alias_trailing_path_redirect_git(self):
+        project = self.factory.makeProduct()
+        repo = self.factory.makeGitRepository(target=project)
+        with admin_logged_in():
+            getUtility(IGitRepositorySet).setDefaultRepository(
+                project, repo)
+        path = '%s/+edit' % project.name
+        self.assertRedirects(path, canonical_url(repo, view_name='+edit'))
+
+    def test_product_series_redirect_bzr(self):
+        branch = self.factory.makeBranch()
+        series = self.factory.makeProductSeries(branch=branch)
+        self.assertRedirects(
+            ICanHasLinkedBranch(series).bzr_path, canonical_url(branch))
+
+    def test_no_branch_for_series(self):
+        # If there's no branch for a product series, display a
+        # message telling the user there is no linked branch.
+        series = self.factory.makeProductSeries()
+        path = ICanHasLinkedBranch(series).bzr_path
+        self.assertNotFound(path)
+
+    def test_private_branch_for_series(self):
+        # If the development focus of a product series is private, display a
+        # message telling the user there is no linked branch.
+        branch = self.factory.makeBranch(
+            information_type=InformationType.USERDATA)
+        series = self.factory.makeProductSeries(branch=branch)
+        login(ANONYMOUS)
+        path = ICanHasLinkedBranch(series).bzr_path
+        self.assertNotFound(path)
+
+    def test_too_short_branch_name(self):
+        owner = self.factory.makePerson()
+        self.assertNotFound('~%s' % owner.name)
+
+    def test_invalid_product_name(self):
+        self.assertNotFound('_foo')
+
+    def test_invalid_product_name_without_referer(self):
+        self.assertNotFound("_foo", use_default_referer=False)
+
+    def test_ambiguous_project_default_repo_bzr(self):
+        project = self.factory.makeProduct()
+        bzr_branch = self.factory.makeBranch(target=project)
+        self.factory.makeGitRepository(target=project)
+        with person_logged_in(project.owner):
+            ICanHasLinkedBranch(project).setBranch(bzr_branch, project.owner)
+
+        self.assertRedirects(project.name, canonical_url(bzr_branch))
+
+    def test_ambiguous_project_without_vcs_set(self):
+        project = self.factory.makeProduct()
+        bzr_branch = self.factory.makeBranch(target=project)
+        repo = self.factory.makeGitRepository(target=project)
+        with person_logged_in(project.owner):
+            ICanHasLinkedBranch(project).setBranch(bzr_branch, project.owner)
+            getUtility(IGitRepositorySet).setDefaultRepository(
+                project, repo)
+
+        self.assertNotFound(project.name)
+
+    def test_ambiguous_project_with_vcs_set_to_git(self):
+        project = self.factory.makeProduct()
+        bzr_branch = self.factory.makeBranch(target=project)
+        repo = self.factory.makeGitRepository(target=project)
+        with person_logged_in(project.owner):
+            ICanHasLinkedBranch(project).setBranch(bzr_branch, project.owner)
+            getUtility(IGitRepositorySet).setDefaultRepository(
+                project, repo)
+            project.vcs = VCSType.GIT
+
+        self.assertRedirects(project.name, canonical_url(repo))
+
+    def test_ambiguous_project_with_vcs_set_to_bzr(self):
+        project = self.factory.makeProduct()
+        bzr_branch = self.factory.makeBranch(target=project)
+        repo = self.factory.makeGitRepository(target=project)
+        with person_logged_in(project.owner):
+            ICanHasLinkedBranch(project).setBranch(bzr_branch, project.owner)
+            getUtility(IGitRepositorySet).setDefaultRepository(
+                project, repo)
+            project.vcs = VCSType.BZR
+
+        self.assertRedirects(project.name, canonical_url(bzr_branch))
+
+    def test_personal_branch_bzr(self):
+        person = self.factory.makePerson()
+        branch = self.factory.makePersonalBranch(owner=person)
+        self.assertRedirects(branch.unique_name, canonical_url(branch))
+
+    def test_personal_branch_git(self):
+        person = self.factory.makePerson()
+        repo = self.factory.makeGitRepository(owner=person, target=person)
+        self.assertRedirects(repo.unique_name, canonical_url(repo))
+
+
 class TestPersonTraversal(TestCaseWithFactory, TraversalMixin):
 
     layer = DatabaseFunctionalLayer

=== modified file 'lib/lp/app/browser/tests/test_linkchecker.py'
--- lib/lp/app/browser/tests/test_linkchecker.py	2012-09-17 15:19:10 +0000
+++ lib/lp/app/browser/tests/test_linkchecker.py	2016-06-22 21:05:27 +0000
@@ -21,7 +21,7 @@
 
     layer = DatabaseFunctionalLayer
 
-    BRANCH_URL_TEMPLATE = '/+branch/%s'
+    BRANCH_URL_TEMPLATE = '/+code/%s'
 
     def check_invalid_links(self, result_json, link_type, invalid_links):
         link_dict = simplejson.loads(result_json)
@@ -43,9 +43,22 @@
         removeSecurityProxy(product).development_focus.branch = product_branch
         valid_product_url = self.BRANCH_URL_TEMPLATE % product.name
 
+        # git branches are a thing now as well!
+        project_git_repo = removeSecurityProxy(
+            self.factory.makeGitRepository(target=product))
+        dsp = self.factory.makeDistributionSourcePackage()
+        dsp_git_repo = removeSecurityProxy(
+            self.factory.makeGitRepository(target=dsp))
+        person = self.factory.makePerson()
+        person_git_repo = removeSecurityProxy(
+            self.factory.makeGitRepository(target=person))
+
         return [
             valid_branch_url,
             valid_product_url,
+            self.BRANCH_URL_TEMPLATE % project_git_repo.unique_name,
+            self.BRANCH_URL_TEMPLATE % dsp_git_repo.unique_name,
+            self.BRANCH_URL_TEMPLATE % person_git_repo.unique_name,
         ]
 
     def make_invalid_branch_links(self):

=== modified file 'lib/lp/app/doc/displaying-paragraphs-of-text.txt'
--- lib/lp/app/doc/displaying-paragraphs-of-text.txt	2015-07-21 09:04:01 +0000
+++ lib/lp/app/doc/displaying-paragraphs-of-text.txt	2016-06-22 21:05:27 +0000
@@ -366,17 +366,17 @@
     ...     'lp:///foo\n'
     ...     'lp:/foo\n')
     >>> print test_tales('foo/fmt:text-to-html', foo=text)
-    <p><a href="/+branch/~foo/bar/baz" class="...">lp:~foo/bar/baz</a><br />
-    <a href="/+branch/~foo/bar/bug-123" class="...">lp:~foo/bar/bug-123</a><br />
-    <a href="/+branch/~foo/+junk/baz" class="...">lp:~foo/+junk/baz</a><br />
-    <a href="/+branch/~foo/ubuntu/jaunty/evolution/baz" class="...">lp:~foo/ubuntu/jaunty/evolution/baz</a><br />
-    <a href="/+branch/foo/bar" class="...">lp:foo/bar</a><br />
-    <a href="/+branch/foo" class="...">lp:foo</a><br />
-    <a href="/+branch/foo" class="...">lp:foo</a>,<br />
-    <a href="/+branch/foo/bar" class="...">lp:foo/bar</a>.<br />
-    <a href="/+branch/foo/bar/baz" class="...">lp:foo/bar/baz</a><br />
-    <a href="/+branch/foo" class="...">lp:///foo</a><br />
-    <a href="/+branch/foo" class="...">lp:/foo</a></p>
+    <p><a href="/+code/~foo/bar/baz" class="...">lp:~foo/bar/baz</a><br />
+    <a href="/+code/~foo/bar/bug-123" class="...">lp:~foo/bar/bug-123</a><br />
+    <a href="/+code/~foo/+junk/baz" class="...">lp:~foo/+junk/baz</a><br />
+    <a href="/+code/~foo/ubuntu/jaunty/evolution/baz" class="...">lp:~foo/ubuntu/jaunty/evolution/baz</a><br />
+    <a href="/+code/foo/bar" class="...">lp:foo/bar</a><br />
+    <a href="/+code/foo" class="...">lp:foo</a><br />
+    <a href="/+code/foo" class="...">lp:foo</a>,<br />
+    <a href="/+code/foo/bar" class="...">lp:foo/bar</a>.<br />
+    <a href="/+code/foo/bar/baz" class="...">lp:foo/bar/baz</a><br />
+    <a href="/+code/foo" class="...">lp:///foo</a><br />
+    <a href="/+code/foo" class="...">lp:/foo</a></p>
 
 Text that looks like a branch reference, but is followed only by digits is
 treated as a link to a bug.

=== modified file 'lib/lp/app/javascript/lp-links.js'
--- lib/lp/app/javascript/lp-links.js	2012-05-16 19:21:27 +0000
+++ lib/lp/app/javascript/lp-links.js	2016-06-22 21:05:27 +0000
@@ -56,7 +56,7 @@
     Y.lp.app.links.check_valid_lp_links = function(io_provider) {
         // Grabs any lp: style links on the page and checks that they are
         // valid. Invalid ones have their class changed to "invalid-link".
-        // ATM, we only handle +branch and bug links.
+        // ATM, we only handle +code and bug links.
 
         var links_to_check = {};
 
@@ -108,4 +108,3 @@
     };
 
 }, "0.1", {"requires": ["base", "node", "io", "dom", "json", "lp.client"]});
-

=== modified file 'lib/lp/app/javascript/tests/test_lp_links.html'
--- lib/lp/app/javascript/tests/test_lp_links.html	2012-10-26 09:54:28 +0000
+++ lib/lp/app/javascript/tests/test_lp_links.html	2016-06-22 21:05:27 +0000
@@ -50,8 +50,8 @@
         <div id="expected-id">
           <a href="/bugs/14" class="bug-link" id="valid-bug">bug 14</a>
           <a href="/bugs/200" class="bug-link" id="invalid-bug">bug 200</a>
-          <a href="/+branch/valid" class="branch-short-link" id="valid-branch">lp:valid</a>
-          <a href="/+branch/invalid" class="branch-short-link" id="invalid-branch">lp:invalid</a>
+          <a href="/+code/valid" class="branch-short-link" id="valid-branch">lp:valid</a>
+          <a href="/+code/invalid" class="branch-short-link" id="invalid-branch">lp:invalid</a>
         </div>
     </body>
 </html>

=== modified file 'lib/lp/app/javascript/tests/test_lp_links.js'
--- lib/lp/app/javascript/tests/test_lp_links.js	2012-10-26 10:00:20 +0000
+++ lib/lp/app/javascript/tests/test_lp_links.js	2016-06-22 21:05:27 +0000
@@ -20,7 +20,7 @@
                         "/bugs/200": "Bug 200 cannot be found"}},
                 "branch_links": {
                     "invalid": {
-                        "/+branch/invalid":
+                        "/+code/invalid":
                             "No such product: 'invalid'."}}};
             mock_io.success({
                 responseText: Y.JSON.stringify(response),
@@ -83,7 +83,7 @@
             var response = {
                 "branch_links": {
                     "invalid": {
-                        "/+branch/invalid":
+                        "/+code/invalid":
                             "No such product: 'invalid'."}}};
             mock_io.success({
                 responseText: Y.JSON.stringify(response),

=== modified file 'lib/lp/app/stories/basics/notfound-traversals.txt'
--- lib/lp/app/stories/basics/notfound-traversals.txt	2015-10-06 06:48:01 +0000
+++ lib/lp/app/stories/basics/notfound-traversals.txt	2016-06-22 21:05:27 +0000
@@ -95,6 +95,10 @@
     >>> check_redirect(
     ...     "/~name12/+branch/gnome-terminal/pushed/+edit",
     ...     auth=True, status=301)
+    >>> check_redirect("/~name12/+branch/gnome-terminal/pushed/", status=301)
+    >>> check_redirect(
+    ...     "/~name12/+branch/gnome-terminal/pushed/+edit",
+    ...     auth=True, status=301)
     >>> check_redirect("/~name16/+packages", status=301)
     >>> check_redirect("/~name16/+projects", status=301)
     >>> check_redirect("/+builds", status=301)