launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #24992
[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..16f4e4c 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,52 @@ 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 that targets a user.
+ if IPerson.providedBy(self.context.target):
+ return False
+ # User cannot fork repositories they already own (note 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):
+ new_owner = data.get("owner")
+ if not new_owner or not self.user.inTeam(new_owner):
+ self.request.response.addNotification(
+ "You should select a valid user to fork the repository.")
+ return
+ forked = getUtility(IGitRepositorySet).fork(self.context, new_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..90dc30f 100644
--- a/lib/lp/code/browser/tests/test_gitrepository.py
+++ b/lib/lp/code/browser/tests/test_gitrepository.py
@@ -18,6 +18,11 @@ from textwrap import dedent
from fixtures import FakeLogger
import pytz
import soupmatchers
+from soupmatchers import (
+ Tag,
+ HTMLContains,
+ )
+from storm.expr import Desc
from storm.store import Store
from testtools.matchers import (
AfterPreprocessing,
@@ -28,6 +33,7 @@ from testtools.matchers import (
MatchesListwise,
MatchesSetwise,
MatchesStructure,
+ Not,
)
import transaction
from zope.component import getUtility
@@ -52,6 +58,7 @@ from lp.code.enums import (
)
from lp.code.interfaces.revision import IRevisionSet
from lp.code.model.gitjob import GitRefScanJob
+from lp.code.model.gitrepository import GitRepository
from lp.code.tests.helpers import GitHostingFixture
from lp.registry.enums import (
BranchSharingPolicy,
@@ -64,6 +71,7 @@ from lp.registry.interfaces.person import (
)
from lp.services.beautifulsoup import BeautifulSoup
from lp.services.database.constants import UTC_NOW
+from lp.services.database.interfaces import IStore
from lp.services.features.testing import FeatureFixture
from lp.services.job.interfaces.job import JobStatus
from lp.services.webapp.publisher import canonical_url
@@ -471,6 +479,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.
@@ -1969,3 +2010,116 @@ class TestGitRepositoryActivityView(BrowserTestCase):
create_activity,
2)
self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
+
+
+class TestGitRepositoryForkView(BrowserTestCase):
+
+ layer = DatabaseFunctionalLayer
+
+ def getReposOwnedBy(self, user):
+ rs = IStore(GitRepository).find(GitRepository)
+ return rs.find(GitRepository.owner == user).order_by(
+ Desc(GitRepository.date_created))
+
+ def test_fork_page_shows_input(self):
+ with admin_logged_in():
+ repository = self.factory.makeGitRepository()
+ owner = removeSecurityProxy(repository.owner)
+
+ team = self.factory.makeTeam(members=[owner])
+ another_person = self.factory.makePerson()
+
+ select_owner = Tag(
+ "owner selector", "select",
+ attrs={"id": "field.owner", "name": "field.owner"})
+
+ def person_option_tag(person):
+ return Tag(
+ "option for %s" % person, "option",
+ text="%s (%s)" % (person.displayname, person.name),
+ attrs=dict(value=person.name))
+
+ option_owner = person_option_tag(owner)
+ option_team = person_option_tag(team)
+ option_another_person = person_option_tag(another_person)
+
+ browser = self.getViewBrowser(
+ repository, "+fork", rootsite="code", user=owner)
+
+ html = browser.contents
+ self.assertThat(html, HTMLContains(select_owner))
+ self.assertThat(html, HTMLContains(option_owner))
+ self.assertThat(html, HTMLContains(option_team))
+ self.assertThat(html, Not(HTMLContains(option_another_person)))
+
+ def test_fork_page_submit_to_self(self):
+ self.useFixture(GitHostingFixture())
+ repository = self.factory.makeGitRepository()
+ another_person = self.factory.makePerson()
+
+ with person_logged_in(another_person):
+ view = create_initialized_view(repository, name="+fork", form={
+ "field.owner": another_person.name,
+ "field.actions.fork": "Fork it"})
+
+ forked = self.getReposOwnedBy(another_person)[0]
+ self.assertNotEqual(forked, repository)
+ self.assertEqual(another_person, forked.owner)
+
+ notifications = view.request.response.notifications
+ self.assertEqual(1, len(notifications))
+ self.assertEqual("Repository forked.", notifications[0].message)
+
+ self.assertEqual(canonical_url(forked), view.next_url)
+
+ def test_fork_page_submit_to_team(self):
+ self.useFixture(GitHostingFixture())
+ repository = self.factory.makeGitRepository()
+ another_person = self.factory.makePerson()
+ team = self.factory.makeTeam(members=[another_person])
+
+ with person_logged_in(another_person):
+ view = create_initialized_view(repository, name="+fork", form={
+ "field.owner": team.name,
+ "field.actions.fork": "Fork it"})
+
+ forked = self.getReposOwnedBy(team)[0]
+ self.assertNotEqual(forked, repository)
+ self.assertEqual(team, forked.owner)
+
+ notifications = view.request.response.notifications
+ self.assertEqual(1, len(notifications))
+ self.assertEqual("Repository forked.", notifications[0].message)
+
+ self.assertEqual(canonical_url(forked), view.next_url)
+
+ def test_fork_page_submit_missing_user(self):
+ self.useFixture(GitHostingFixture())
+ repository = self.factory.makeGitRepository()
+ another_person = self.factory.makePerson()
+
+ with person_logged_in(another_person):
+ view = create_initialized_view(repository, name="+fork", form={
+ "field.actions.fork": "Fork it"})
+
+ # No repository should have been created.
+ self.assertEqual(0, self.getReposOwnedBy(another_person).count())
+ notifications = view.request.response.notifications
+ self.assertEqual(1, len(notifications))
+ self.assertEqual(
+ "You should select a valid user to fork the repository.",
+ notifications[0].message)
+
+ self.assertEqual(None, view.next_url)
+
+ def test_fork_page_submit_invalid_user(self):
+ self.useFixture(GitHostingFixture())
+ repository = self.factory.makeGitRepository()
+ another_person = self.factory.makePerson()
+ invalid_person = self.factory.makePerson()
+
+ with person_logged_in(another_person):
+ create_initialized_view(repository, name="+fork", form={
+ "field.owner": invalid_person.name,
+ "field.actions.fork": "Fork it"})
+ self.assertEqual(0, self.getReposOwnedBy(invalid_person).count())
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_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>
