launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #24989
[Merge] ~pappacena/launchpad:fork-button into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:fork-button into launchpad:master with ~pappacena/launchpad:git-fork-backend as a prerequisite.
Commit message:
Adding UI to allow forking a git repository
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/387157
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:fork-button into launchpad:master.
diff --git a/lib/lp/code/browser/configure.zcml b/lib/lp/code/browser/configure.zcml
index 499a86a..e7943c0 100644
--- a/lib/lp/code/browser/configure.zcml
+++ b/lib/lp/code/browser/configure.zcml
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+<!-- Copyright 2009-2020 Canonical Ltd. This software is licensed under the
GNU Affero General Public License version 3 (see the file LICENSE).
-->
@@ -885,6 +885,12 @@
template="../templates/gitrepository-rescan.pt"/>
<browser:page
for="lp.code.interfaces.gitrepository.IGitRepository"
+ class="lp.code.browser.gitrepository.GitRepositoryForkView"
+ permission="launchpad.View"
+ name="+fork"
+ template="../templates/gitrepository-fork.pt"/>
+ <browser:page
+ for="lp.code.interfaces.gitrepository.IGitRepository"
class="lp.code.browser.codeimport.CodeImportEditView"
permission="launchpad.Edit"
name="+edit-import"
diff --git a/lib/lp/code/browser/gitrepository.py b/lib/lp/code/browser/gitrepository.py
index 7dc1c32..e57225d 100644
--- a/lib/lp/code/browser/gitrepository.py
+++ b/lib/lp/code/browser/gitrepository.py
@@ -39,6 +39,7 @@ from six.moves.urllib_parse import (
from zope.component import getUtility
from zope.event import notify
from zope.formlib import form
+from zope.formlib.form import FormFields
from zope.formlib.textwidgets import IntWidget
from zope.formlib.widget import CustomWidgetFactory
from zope.interface import (
@@ -99,7 +100,10 @@ from lp.code.errors import (
)
from lp.code.interfaces.gitnamespace import get_git_namespace
from lp.code.interfaces.gitref import IGitRefBatchNavigator
-from lp.code.interfaces.gitrepository import IGitRepository
+from lp.code.interfaces.gitrepository import (
+ IGitRepository,
+ IGitRepositorySet,
+ )
from lp.code.vocabularies.gitrule import GitPermissionsVocabulary
from lp.registry.interfaces.person import (
IPerson,
@@ -454,12 +458,48 @@ class GitRepositoryView(InformationTypePortletMixin, LaunchpadView,
return False
return scan_job.job.status == JobStatus.FAILED
- @property
def warning_message(self):
if self.context.status == GitRepositoryStatus.CREATING:
return "This repository is being created."
return None
+ @property
+ def allow_fork(self):
+ # Users cannot fork repositories which target is a user.
+ if IPerson.providedBy(self.context.target):
+ return False
+ # User cannot fork repositories they already own (not that forking a
+ # repository owned by a team the user is in is still fine).
+ if self.context.owner == self.user:
+ return False
+ return True
+
+
+class GitRepositoryForkView(LaunchpadEditFormView):
+
+ schema = Interface
+
+ field_names = []
+
+ def setUpFields(self):
+ super(GitRepositoryForkView, self).setUpFields()
+ owner_field = Choice(
+ vocabulary='AllUserTeamsParticipationPlusSelf',
+ title=u'Fork to the following owner', required=True,
+ __name__=u'owner')
+ self.form_fields += FormFields(owner_field)
+
+ @action('Fork it', name='fork')
+ def fork(self, action, data):
+ forked = getUtility(IGitRepositorySet).fork(
+ self.context, data['owner'])
+ self.request.response.addNotification("Repository forked.")
+ self.next_url = canonical_url(forked)
+
+ @property
+ def cancel_url(self):
+ return canonical_url(self.context)
+
class GitRepositoryRescanView(LaunchpadEditFormView):
diff --git a/lib/lp/code/browser/tests/test_gitrepository.py b/lib/lp/code/browser/tests/test_gitrepository.py
index d323976..e1f0e57 100644
--- a/lib/lp/code/browser/tests/test_gitrepository.py
+++ b/lib/lp/code/browser/tests/test_gitrepository.py
@@ -18,6 +18,10 @@ from textwrap import dedent
from fixtures import FakeLogger
import pytz
import soupmatchers
+from soupmatchers import (
+ Tag,
+ HTMLContains,
+ )
from storm.store import Store
from testtools.matchers import (
AfterPreprocessing,
@@ -28,6 +32,7 @@ from testtools.matchers import (
MatchesListwise,
MatchesSetwise,
MatchesStructure,
+ Not,
)
import transaction
from zope.component import getUtility
@@ -471,6 +476,39 @@ class TestGitRepositoryView(BrowserTestCase):
result = view.show_rescan_link
self.assertTrue(result)
+ def test_hide_fork_link_for_repos_targeting_person(self):
+ person = self.factory.makePerson()
+ another_person = self.factory.makePerson()
+ repository = self.factory.makeGitRepository(target=person)
+ browser = self.getViewBrowser(
+ repository, '+index', user=another_person)
+ fork_link = Tag(
+ "fork link", "a",
+ text="Fork it to your account",
+ attrs={"class": "sprite add subscribe-self",
+ "href": "+fork"})
+ self.assertThat(browser.contents, Not(HTMLContains(fork_link)))
+
+ def test_show_fork_link_for_the_right_users(self):
+ another_person = self.factory.makePerson()
+ repository = self.factory.makeGitRepository()
+ repo_owner = repository.owner
+
+ fork_link = Tag(
+ "fork link", "a",
+ text="Fork it to your account",
+ attrs={"class": "sprite add subscribe-self",
+ "href": "+fork"})
+
+ # Do not show the link for the repository owner.
+ browser = self.getViewBrowser(repository, '+index', user=repo_owner)
+ self.assertThat(browser.contents, Not(HTMLContains(fork_link)))
+
+ # Shows for another person.
+ browser = self.getViewBrowser(
+ repository, '+index', user=another_person)
+ self.assertThat(browser.contents, HTMLContains(fork_link))
+
class TestGitRepositoryViewPrivateArtifacts(BrowserTestCase):
"""Tests that Git repositories with private team artifacts can be viewed.
diff --git a/lib/lp/code/interfaces/gitnamespace.py b/lib/lp/code/interfaces/gitnamespace.py
index fd7e3e7..c933b67 100644
--- a/lib/lp/code/interfaces/gitnamespace.py
+++ b/lib/lp/code/interfaces/gitnamespace.py
@@ -40,8 +40,16 @@ class IGitNamespace(Interface):
def createRepository(repository_type, registrant, name,
information_type=None, date_created=None,
target_default=False, owner_default=False,
- with_hosting=False, status=None):
- """Create and return an `IGitRepository` in this namespace."""
+ with_hosting=False, async_hosting=False, status=None):
+ """Create and return an `IGitRepository` in this namespace.
+
+ :param with_hosting: If True, also creates the repository on git
+ hosting service.
+ :param async_hosting: If with_hosting is True, this controls if the
+ call to create repository on hosting service will be done
+ asynchronously, or it will block until the service creates the
+ repository.
+ """
def isNameUsed(name):
"""Is 'name' already used in this namespace?"""
diff --git a/lib/lp/code/model/githosting.py b/lib/lp/code/model/githosting.py
index e2a4462..60dff43 100644
--- a/lib/lp/code/model/githosting.py
+++ b/lib/lp/code/model/githosting.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2019 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Communication with the Git hosting service."""
diff --git a/lib/lp/code/model/tests/test_githosting.py b/lib/lp/code/model/tests/test_githosting.py
index d966dbe..16717c4 100644
--- a/lib/lp/code/model/tests/test_githosting.py
+++ b/lib/lp/code/model/tests/test_githosting.py
@@ -2,7 +2,7 @@
# NOTE: The first line above must stay first; do not move the copyright
# notice to the top. See http://www.python.org/dev/peps/pep-0263/.
#
-# Copyright 2016-2019 Canonical Ltd. This software is licensed under the
+# Copyright 2016-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Unit tests for `GitHostingClient`.
@@ -23,7 +23,6 @@ from lazr.restful.utils import get_current_browser_request
import responses
from six.moves.urllib.parse import (
parse_qsl,
- urljoin,
urlsplit,
)
from testtools.matchers import (
@@ -130,6 +129,13 @@ class TestGitHostingClient(TestCase):
"repo", method="POST",
json_data={"repo_path": "123", "clone_from": "122"})
+ def test_create_async(self):
+ with self.mockRequests("POST"):
+ self.client.create("123", clone_from="122", async_create=True)
+ self.assertRequest(
+ "repo", method="POST",
+ json_data={"repo_path": "123", "clone_from": "122", "async": True})
+
def test_create_failure(self):
with self.mockRequests("POST", status=400):
self.assertRaisesWithContent(
diff --git a/lib/lp/code/model/tests/test_gitnamespace.py b/lib/lp/code/model/tests/test_gitnamespace.py
index 5fed799..df01b08 100644
--- a/lib/lp/code/model/tests/test_gitnamespace.py
+++ b/lib/lp/code/model/tests/test_gitnamespace.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2019 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for `IGitNamespace` implementations."""
@@ -24,6 +24,7 @@ from lp.code.errors import (
GitRepositoryCreatorNotOwner,
GitRepositoryExists,
)
+from lp.code.interfaces.githosting import IGitHostingClient
from lp.code.interfaces.gitnamespace import (
get_git_namespace,
IGitNamespace,
@@ -45,10 +46,12 @@ from lp.registry.interfaces.accesspolicy import (
IAccessPolicyGrantFlatSource,
IAccessPolicySource,
)
+from lp.services.compat import mock
from lp.testing import (
person_logged_in,
TestCaseWithFactory,
)
+from lp.testing.fixture import ZopeUtilityFixture
from lp.testing.layers import DatabaseFunctionalLayer
@@ -100,6 +103,36 @@ class NamespaceMixin:
GitRepositoryType.HOSTED, registrant, repository_name)
self.assertEqual([owner], list(repository.subscribers))
+ def test_createRepository_creates_on_githosting_sync(self):
+ hosting_client = mock.Mock()
+ self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
+ owner = self.factory.makeTeam()
+ namespace = self.getNamespace(owner)
+ repository_name = self.factory.getUniqueUnicode()
+ registrant = owner.teamowner
+ repository = namespace.createRepository(
+ GitRepositoryType.HOSTED, registrant, repository_name,
+ with_hosting=True)
+ path = repository.getInternalPath()
+ self.assertEqual(
+ [mock.call(path, clone_from=None, async_create=False)],
+ hosting_client.create.call_args_list)
+
+ def test_createRepository_creates_on_githosting_async(self):
+ hosting_client = mock.Mock()
+ self.useFixture(ZopeUtilityFixture(hosting_client, IGitHostingClient))
+ owner = self.factory.makeTeam()
+ namespace = self.getNamespace(owner)
+ repository_name = self.factory.getUniqueUnicode()
+ registrant = owner.teamowner
+ repository = namespace.createRepository(
+ GitRepositoryType.HOSTED, registrant, repository_name,
+ with_hosting=True, async_hosting=True)
+ path = repository.getInternalPath()
+ self.assertEqual(
+ [mock.call(path, clone_from=None, async_create=True)],
+ hosting_client.create.call_args_list)
+
def test_getRepositories_no_repositories(self):
# getRepositories on an IGitNamespace returns a result set of
# repositories in that namespace. If there are no repositories, the
diff --git a/lib/lp/code/templates/git-macros.pt b/lib/lp/code/templates/git-macros.pt
index 9407df1..d8f26bc 100644
--- a/lib/lp/code/templates/git-macros.pt
+++ b/lib/lp/code/templates/git-macros.pt
@@ -34,6 +34,10 @@
</tt>
</tal:ssh>
</dd>
+
+ <dt tal:condition="view/allow_fork">
+ <a href="+fork" class="sprite add subscribe-self">Fork it to your account</a>
+ </dt>
</dl>
<div id="push-directions"
diff --git a/lib/lp/code/templates/gitrepository-fork.pt b/lib/lp/code/templates/gitrepository-fork.pt
new file mode 100644
index 0000000..32e2221
--- /dev/null
+++ b/lib/lp/code/templates/gitrepository-fork.pt
@@ -0,0 +1,23 @@
+<html
+ xmlns="http://www.w3.org/1999/xhtml";
+ xmlns:tal="http://xml.zope.org/namespaces/tal";
+ xmlns:metal="http://xml.zope.org/namespaces/metal";
+ xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+ metal:use-macro="view/macro:page/main_only"
+ i18n:domain="launchpad">
+ <body>
+ <div metal:fill-slot="main">
+ <p>
+ This will create a copy of
+ lp:<tal:repository replace="context/name"/> repository in your
+ account.
+ </p>
+ <p>
+ After the fork process, you will be able to git clone the
+ repository, push to it and create Merge Proposals to the original
+ repository.
+ </p>
+ <div metal:use-macro="context/@@launchpad_form/form" />
+ </div>
+ </body>
+</html>
diff --git a/lib/lp/code/templates/gitrepository-index.pt b/lib/lp/code/templates/gitrepository-index.pt
index 89f202f..8e87244 100644
--- a/lib/lp/code/templates/gitrepository-index.pt
+++ b/lib/lp/code/templates/gitrepository-index.pt
@@ -45,7 +45,8 @@
tal:content="structure context/description/fmt:text-to-html" />
<div class="yui-g first">
- <div id="repository-management" class="portlet">
+ <div id="repository-management" class="portlet"
+ tal:define="allow_fork view/allow_fork">
<tal:repository-management
replace="structure context/@@++repository-management" />
</div>
References
