Merge lp:~blr/launchpad/bug-1600055-configure-code-remote-username into lp:launchpad

[Kit Randel]

Add view property to format git ssh url with username.

Unless your launchpad username aligns with your local username, or you have configured your lp username in ~/.ssh/config, instructions for adding remote without a username are invalid.

[Colin Watson (cjwatson)]

In addition to the comments below, please could you add some tests for this?

review: Needs Fixing

[Kit Randel (blr)]

Colin, have moved the tests in browser.tests.test_product_views to browser.tests.test_product as the bulk of product view tests were already there. I'm not entirely sure why these were in different modules, unless we had intended to move to a file name convention of test_foo_views.py for view tests. Given the browser namespace, I think it is already reasonably obvious that these are view tests and we could probably avoid the explicit 'views' postfix.

[Colin Watson (cjwatson)]

review: Approve

=== modified file 'lib/lp/code/templates/configure-code-macros.pt'
--- lib/lp/code/templates/configure-code-macros.pt	2016-01-14 13:55:43 +0000
+++ lib/lp/code/templates/configure-code-macros.pt	2016-07-25 04:46:32 +0000
@@ -20,7 +20,7 @@
     command:</p>
     <p>
       <code class="command command-block">
-        git remote add origin <tal:project replace="modules/lp.services.config/config/codehosting/git_ssh_root"/><tal:project replace="context/name"/><br />
+        git remote add origin <span tal:content="string:${view/git_ssh_url}"></span><br />
     </code></p>
     <p>&hellip; and push the Git branch to Launchpad with:</p>
     <p>

=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py	2016-04-14 05:16:26 +0000
+++ lib/lp/registry/browser/product.py	2016-07-25 04:46:32 +0000
@@ -42,7 +42,9 @@
 
 
 from operator import attrgetter
+from urlparse import urlunsplit
 
+from bzrlib import urlutils
 from bzrlib.revision import NULL_REVISION
 from lazr.delegates import delegate_to
 from lazr.restful.interface import (
@@ -212,6 +214,7 @@
 from lp.services.webapp.breadcrumb import Breadcrumb
 from lp.services.webapp.interfaces import UnsafeFormGetSubmissionError
 from lp.services.webapp.menu import NavigationMenu
+from lp.services.webapp.url import urlsplit
 from lp.services.webapp.vhosts import allvhosts
 from lp.services.worlddata.helpers import browser_languages
 from lp.services.worlddata.interfaces.country import ICountry
@@ -1743,6 +1746,14 @@
                 self.context.pillar))
 
     @property
+    def git_ssh_url(self):
+        base_url = urlsplit(
+            urlutils.join(config.codehosting.git_ssh_root, self.context.name))
+        url = list(base_url)
+        url[1] = "{}@{}".format(self.user.name, base_url.hostname)
+        return urlunsplit(url)
+
+    @property
     def next_url(self):
         """Return the next_url.
 

=== modified file 'lib/lp/registry/browser/tests/test_product.py'
--- lib/lp/registry/browser/tests/test_product.py	2015-10-01 17:32:41 +0000
+++ lib/lp/registry/browser/tests/test_product.py	2016-07-25 04:46:32 +0000
@@ -7,6 +7,9 @@
 
 __all__ = ['make_product_form']
 
+import re
+from urlparse import urlsplit
+
 from lazr.restful.interfaces import IJSONRequestCache
 from soupmatchers import (
     HTMLContains,
@@ -834,3 +837,81 @@
         with person_logged_in(owner):
             self.assertIn(public.name, browser.contents)
             self.assertIn(proprietary.name, browser.contents)
+
+
+class TestProductSetBranchView(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_git_ssh_url(self):
+        project = self.factory.makeProduct()
+        with person_logged_in(project.owner):
+            view = create_initialized_view(
+                project, '+configure-code', principal=project.owner,
+                method='GET')
+            git_ssh_url = 'git+ssh://{username}@{host}/{project}'.format(
+                username=project.owner.name,
+                host=urlsplit(config.codehosting.git_ssh_root).hostname,
+                project=project.name)
+            self.assertEqual(git_ssh_url, view.git_ssh_url)
+
+
+class TestBrowserProductSetBranchView(BrowserTestCase):
+
+    layer = DatabaseFunctionalLayer
+
+    editsshkeys_tag = Tag(
+        'edit SSH keys', 'a', text=re.compile('register an SSH key'),
+        attrs={'href': re.compile(r'/\+editsshkeys$')})
+
+    def getBrowser(self, project, view_name=None):
+        project = removeSecurityProxy(project)
+        url = canonical_url(project, view_name=view_name)
+        return self.getUserBrowser(url, project.owner)
+
+    def test_no_initial_git_repository(self):
+        # If a project has no default Git repository, its "Git repository"
+        # control defaults to empty.
+        project = self.factory.makeProduct()
+        browser = self.getBrowser(project, '+configure-code')
+        self.assertEqual('', browser.getControl('Git repository').value)
+
+    def test_initial_git_repository(self):
+        # If a project has a default Git repository, its "Git repository"
+        # control defaults to the unique name of that repository.
+        project = self.factory.makeProduct()
+        repo = self.factory.makeGitRepository(target=project)
+        with person_logged_in(project.owner):
+            getUtility(IGitRepositorySet).setDefaultRepository(project, repo)
+        unique_name = repo.unique_name
+        browser = self.getBrowser(project, '+configure-code')
+        self.assertEqual(
+            unique_name, browser.getControl('Git repository').value)
+
+    def test_link_existing_git_repository(self):
+        repo = removeSecurityProxy(self.factory.makeGitRepository(
+            target=self.factory.makeProduct()))
+        browser = self.getBrowser(repo.project, '+configure-code')
+        browser.getControl('Git', index=0).click()
+        browser.getControl('Git repository').value = repo.shortened_path
+        browser.getControl('Update').click()
+
+        tag = Tag(
+            'success-div', 'div', attrs={'class': 'informational message'},
+             text='Project settings updated.')
+        self.assertThat(browser.contents, HTMLContains(tag))
+
+    def test_editsshkeys_link_if_no_keys_registered(self):
+        project = self.factory.makeProduct()
+        browser = self.getBrowser(project, '+configure-code')
+        self.assertThat(
+            browser.contents, HTMLContains(self.editsshkeys_tag))
+
+    def test_no_editsshkeys_link_if_keys_registered(self):
+        project = self.factory.makeProduct()
+        with person_logged_in(project.owner):
+            self.factory.makeSSHKey(person=project.owner)
+        browser = self.getBrowser(project, '+configure-code')
+        self.assertThat(
+            browser.contents,
+            Not(HTMLContains(self.editsshkeys_tag)))

=== removed file 'lib/lp/registry/browser/tests/test_product_views.py'
--- lib/lp/registry/browser/tests/test_product_views.py	2016-01-14 13:55:43 +0000
+++ lib/lp/registry/browser/tests/test_product_views.py	1970-01-01 00:00:00 +0000
@@ -1,82 +0,0 @@
-# Copyright 2011-2016 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""View tests for Product pages."""
-
-__metaclass__ = type
-
-import re
-
-from testtools.matchers import Not
-import soupmatchers
-from zope.component import getUtility
-from zope.security.proxy import removeSecurityProxy
-
-from lp.code.interfaces.gitrepository import IGitRepositorySet
-from lp.services.webapp import canonical_url
-from lp.testing import (
-    BrowserTestCase,
-    person_logged_in,
-    )
-from lp.testing.layers import DatabaseFunctionalLayer
-
-
-class TestProductSetBranchView(BrowserTestCase):
-
-    layer = DatabaseFunctionalLayer
-
-    editsshkeys_tag = soupmatchers.Tag(
-        'edit SSH keys', 'a', text=re.compile('register an SSH key'),
-        attrs={'href': re.compile(r'/\+editsshkeys$')})
-
-    def getBrowser(self, project, view_name=None):
-        project = removeSecurityProxy(project)
-        url = canonical_url(project, view_name=view_name)
-        return self.getUserBrowser(url, project.owner)
-
-    def test_no_initial_git_repository(self):
-        # If a project has no default Git repository, its "Git repository"
-        # control defaults to empty.
-        project = self.factory.makeProduct()
-        browser = self.getBrowser(project, '+configure-code')
-        self.assertEqual('', browser.getControl('Git repository').value)
-
-    def test_initial_git_repository(self):
-        # If a project has a default Git repository, its "Git repository"
-        # control defaults to the unique name of that repository.
-        project = self.factory.makeProduct()
-        repo = self.factory.makeGitRepository(target=project)
-        with person_logged_in(project.owner):
-            getUtility(IGitRepositorySet).setDefaultRepository(project, repo)
-        unique_name = repo.unique_name
-        browser = self.getBrowser(project, '+configure-code')
-        self.assertEqual(
-            unique_name, browser.getControl('Git repository').value)
-
-    def test_link_existing_git_repository(self):
-        repo = removeSecurityProxy(self.factory.makeGitRepository(
-            target=self.factory.makeProduct()))
-        browser = self.getBrowser(repo.project, '+configure-code')
-        browser.getControl('Git', index=0).click()
-        browser.getControl('Git repository').value = repo.shortened_path
-        browser.getControl('Update').click()
-
-        tag = soupmatchers.Tag(
-            'success-div', 'div', attrs={'class': 'informational message'},
-             text='Project settings updated.')
-        self.assertThat(browser.contents, soupmatchers.HTMLContains(tag))
-
-    def test_editsshkeys_link_if_no_keys_registered(self):
-        project = self.factory.makeProduct()
-        browser = self.getBrowser(project, '+configure-code')
-        self.assertThat(
-            browser.contents, soupmatchers.HTMLContains(self.editsshkeys_tag))
-
-    def test_no_editsshkeys_link_if_keys_registered(self):
-        project = self.factory.makeProduct()
-        with person_logged_in(project.owner):
-            self.factory.makeSSHKey(person=project.owner)
-        browser = self.getBrowser(project, '+configure-code')
-        self.assertThat(
-            browser.contents,
-            Not(soupmatchers.HTMLContains(self.editsshkeys_tag)))