launchpad-reviewers team mailing list archive

[Ioana Lasc <ioana.lasc@xxxxxxxxxxxxx>]

Ioana Lasc has proposed merging ~ilasc/launchpad:git-push-url-hint into launchpad:master.

Commit message:
Add git push URL hinting

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ilasc/launchpad/+git/launchpad/+merge/386567

Add git push URL hinting to LP UI on both repository and branch pages when the user doesn’t have permission to push to the repository.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilasc/launchpad:git-push-url-hint into launchpad:master.

diff --git a/lib/lp/code/browser/gitref.py b/lib/lp/code/browser/gitref.py
index 25021bc..a44f55a 100644
--- a/lib/lp/code/browser/gitref.py
+++ b/lib/lp/code/browser/gitref.py
@@ -119,6 +119,16 @@ class GitRefView(LaunchpadView, HasSnapsViewMixin):
         return urlunsplit(url)
 
     @property
+    def git_ssh_url_non_owner(self):
+        """The git+ssh:// URL for this repository, adjusted for this user.
+        The user is not the owner of the repository."""
+        base_url = urlsplit(self.git_ssh_url)
+        url = list(base_url)
+        url[1] = "{}@{}".format(self.user.name, base_url.hostname)
+        url[2] = url[2].replace(self.context.owner.name, self.user.name)
+        return urlunsplit(url)
+
+    @property
     def user_can_push(self):
         """Whether the user can push to this branch."""
         return (
diff --git a/lib/lp/code/browser/gitrepository.py b/lib/lp/code/browser/gitrepository.py
index 4f38702..3f8be7f 100644
--- a/lib/lp/code/browser/gitrepository.py
+++ b/lib/lp/code/browser/gitrepository.py
@@ -384,6 +384,16 @@ class GitRepositoryView(InformationTypePortletMixin, LaunchpadView,
         return urlunsplit(url)
 
     @property
+    def git_ssh_url_non_owner(self):
+        """The git+ssh:// URL for this repository, adjusted for this user.
+        The user is not the owner of the repository."""
+        base_url = urlsplit(self.git_ssh_url)
+        url = list(base_url)
+        url[1] = "{}@{}".format(self.user.name, base_url.hostname)
+        url[2] = url[2].replace(self.context.owner.name, self.user.name)
+        return urlunsplit(url)
+
+    @property
     def user_can_push(self):
         """Whether the user can push to this branch."""
         return (
diff --git a/lib/lp/code/browser/tests/test_gitrepository.py b/lib/lp/code/browser/tests/test_gitrepository.py
index a38120c..0aefb17 100644
--- a/lib/lp/code/browser/tests/test_gitrepository.py
+++ b/lib/lp/code/browser/tests/test_gitrepository.py
@@ -232,30 +232,46 @@ class TestGitRepositoryView(BrowserTestCase):
         # If the user is logged in but cannot push to a repository owned by
         # a person, we explain who can push.
         repository = self.factory.makeGitRepository()
-        browser = self.getViewBrowser(repository)
-        directions = find_tag_by_id(browser.contents, "push-directions")
         login_person(self.user)
-        self.assertThat(directions.renderContents(), DocTestMatches(dedent("""
-            You cannot push to this repository. Only <a
-            href="http://launchpad.test/~{owner.name}";>{owner.display_name}</a>
-            can push to this repository.
-            """).format(owner=repository.owner),
-            flags=doctest.NORMALIZE_WHITESPACE))
+        view = create_initialized_view(
+            repository, '+index', principal=self.user)
+        git_push_url_text_match = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Push url text', 'dt',
+                text='To fork this repository and propose '
+                     'fixes from there, push to this repository:'))
+        git_push_url_hint_match = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Push url hint', 'span',
+                text='git+ssh://%s@xxxxxxxxxxxxxxxxxx/~%s/%s/+git/%s' %
+                     (self.user.name, self.user.name,
+                      repository.target.name, repository.name)))
+        with person_logged_in(self.user):
+            self.assertThat(view.render(), git_push_url_text_match)
+            self.assertThat(view.render(), git_push_url_hint_match)
 
     def test_push_directions_logged_in_cannot_push_team(self):
         # If the user is logged in but cannot push to a repository owned by
         # a team, we explain who can push.
         team = self.factory.makeTeam()
         repository = self.factory.makeGitRepository(owner=team)
-        browser = self.getViewBrowser(repository)
-        directions = find_tag_by_id(browser.contents, "push-directions")
         login_person(self.user)
-        self.assertThat(directions.renderContents(), DocTestMatches(dedent("""
-            You cannot push to this repository. Members of <a
-            href="http://launchpad.test/~{owner.name}";>{owner.display_name}</a>
-            can push to this repository.
-            """).format(owner=repository.owner),
-            flags=doctest.NORMALIZE_WHITESPACE))
+        view = create_initialized_view(
+            repository, '+index', principal=self.user)
+        git_push_url_text_match = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Push url text', 'dt',
+                text='To fork this repository and propose '
+                     'fixes from there, push to this repository:'))
+        git_push_url_hint_match = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Push url hint', 'span',
+                text='git+ssh://%s@xxxxxxxxxxxxxxxxxx/~%s/%s/+git/%s' %
+                     (self.user.name, self.user.name,
+                      repository.target.name, repository.name)))
+        with person_logged_in(self.user):
+            self.assertThat(view.render(), git_push_url_text_match)
+            self.assertThat(view.render(), git_push_url_hint_match)
 
     def test_no_push_directions_for_imported_repository(self):
         # Imported repositories never show push directions.
diff --git a/lib/lp/code/templates/git-macros.pt b/lib/lp/code/templates/git-macros.pt
index 9407df1..f13657d 100644
--- a/lib/lp/code/templates/git-macros.pt
+++ b/lib/lp/code/templates/git-macros.pt
@@ -77,17 +77,32 @@
 
       <tal:cannot-push condition="not:view/user_can_push">
         <tal:individual condition="not:context/owner/is_team">
-          You cannot push to this <tal:kind replace="kind" />. Only
+          You cannot push directly to this <tal:kind replace="kind" />. Only
           <a tal:attributes="href context/owner/fmt:url"
              tal:content="context/owner/displayname">Person</a>
           can push to this <tal:kind replace="kind" />.
         </tal:individual>
         <tal:team condition="context/owner/is_team">
-          You cannot push to this <tal:kind replace="kind" />. Members of
+          You cannot push directly to this <tal:kind replace="kind" />. Members of
           <a tal:attributes="href context/owner/fmt:url"
              tal:content="context/owner/displayname">Team</a>
           can push to this <tal:kind replace="kind" />.
         </tal:team>
+          <dl id="push-url">
+            <dt>To fork this repository and propose fixes from there, push to this repository:</dt>
+            <dd>
+              <tt class="command">
+              git push
+              <span class="ssh-url" tal:content="view/git_ssh_url_non_owner" />
+              <tal:branch condition="branch_name|nothing" replace="branch_name" />
+              </tt>
+            </dd>
+          </dl>
+          <p tal:condition="not:view/user/sshkeys" id="ssh-key-directions">
+            To authenticate with the Launchpad Git hosting service, you need to
+            <a tal:attributes="href string:${view/user/fmt:url}/+editsshkeys">
+              register an SSH key</a>.
+          </p>
       </tal:cannot-push>
     </tal:logged-in>