Merge lp:~cjwatson/launchpad/codeimport-git-new-view into lp:launchpad

[Colin Watson]

Extend CodeImportNewView to be able to create Git-to-Git code imports.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/code/browser/codeimport.py'
--- lib/lp/code/browser/codeimport.py	2016-10-15 01:12:01 +0000
+++ lib/lp/code/browser/codeimport.py	2016-10-15 02:22:43 +0000
@@ -68,6 +68,7 @@
     CodeImportAlreadyRequested,
     CodeImportAlreadyRunning,
     CodeImportNotInReviewedState,
+    GitRepositoryExists,
     )
 from lp.code.interfaces.branch import (
     IBranch,
@@ -82,6 +83,10 @@
     ICodeImportSet,
     )
 from lp.code.interfaces.codeimportmachine import ICodeImportMachineSet
+from lp.code.interfaces.gitnamespace import (
+    get_git_namespace,
+    IGitNamespacePolicy,
+    )
 from lp.registry.interfaces.product import IProduct
 from lp.registry.interfaces.role import IPersonRoles
 from lp.services.fields import URIField
@@ -254,9 +259,10 @@
     git_repo_url = URIField(
         title=_("Repo URL"), required=False,
         description=_(
-            "The URL of the git repository. The HEAD branch will be "
-            "imported. You can import different branches by appending "
-            "',branch=$name' to the URL."),
+            "The URL of the Git repository.  For imports to Bazaar, the "
+            "HEAD branch will be imported by default, but you can import "
+            "different branches by appending ',branch=$name' to the URL.  "
+            "For imports to Git, the entire repository will be imported."),
         allowed_schemes=["git", "http", "https"],
         allow_userinfo=True,
         allow_port=True,
@@ -264,6 +270,13 @@
         allow_fragment=False,
         trailing_slash=False)
 
+    git_target_rcs_type = Choice(
+        title=_("Target version control system"),
+        description=_(
+            "The version control system that the source code should be "
+            "imported into on the Launchpad side."),
+        required=False, vocabulary=TargetRevisionControlSystems)
+
     bzr_branch_url = URIField(
         title=_("Branch URL"), required=False,
         description=_("The URL of the Bazaar branch."),
@@ -277,10 +290,10 @@
     branch_name = copy_field(
         IBranch['name'],
         __name__='branch_name',
-        title=_('Branch Name'),
+        title=_('Name'),
         description=_(
-            "This will be used in the branch URL to identify the "
-            "imported branch.  Examples: main, trunk."),
+            "This will be used in the branch or repository URL to identify "
+            "the import.  Examples: main, trunk."),
         )
 
     product = Choice(
@@ -297,6 +310,7 @@
     for_input = True
 
     custom_widget('rcs_type', LaunchpadRadioWidget)
+    custom_widget('git_target_rcs_type', LaunchpadRadioWidget)
 
     @property
     def initial_values(self):
@@ -304,6 +318,7 @@
             'owner': self.user,
             'rcs_type': RevisionControlSystems.BZR,
             'branch_name': 'trunk',
+            'git_target_rcs_type': TargetRevisionControlSystems.BZR,
             }
 
     @property
@@ -365,6 +380,9 @@
         self.rcs_type_git = str(git_button)
         self.rcs_type_bzr = str(bzr_button)
         self.rcs_type_emptymarker = str(empty_marker)
+        # This widget is only conditionally required in the rcs_type == GIT
+        # case, but we still don't want a "(nothing selected)" item.
+        self.widgets['git_target_rcs_type']._displayItemForMissingValue = False
 
     def _getImportLocation(self, data):
         """Return the import location based on type."""
@@ -385,13 +403,18 @@
         """Create the code import."""
         product = self.getProduct(data)
         cvs_root, cvs_module, url = self._getImportLocation(data)
+        if data['rcs_type'] == RevisionControlSystems.GIT:
+            target_rcs_type = data.get(
+                'git_target_rcs_type', TargetRevisionControlSystems.BZR)
+        else:
+            target_rcs_type = TargetRevisionControlSystems.BZR
         return getUtility(ICodeImportSet).new(
             registrant=self.user,
             owner=data['owner'],
             context=product,
             branch_name=data['branch_name'],
             rcs_type=data['rcs_type'],
-            target_rcs_type=TargetRevisionControlSystems.BZR,
+            target_rcs_type=target_rcs_type,
             url=url,
             cvs_root=cvs_root,
             cvs_module=cvs_module,
@@ -419,16 +442,19 @@
         except BranchExists as e:
             self._setBranchExists(e.existing_branch)
             return
+        except GitRepositoryExists as e:
+            self._setBranchExists(e.existing_repository)
+            return
 
         # Subscribe the user.
-        code_import.branch.subscribe(
+        code_import.target.subscribe(
             self.user,
             BranchSubscriptionNotificationLevel.FULL,
             BranchSubscriptionDiffSize.NODIFF,
             CodeReviewNotificationLevel.NOEMAIL,
             self.user)
 
-        self.next_url = canonical_url(code_import.branch)
+        self.next_url = canonical_url(code_import.target)
 
         self.request.response.addNotification("""
             New code import created. The code import will start shortly.""")
@@ -440,24 +466,41 @@
         else:
             return data.get('product')
 
+    def validate_widgets(self, data, names=None):
+        """See `LaunchpadFormView`."""
+        self.widgets['git_target_rcs_type'].context.required = (
+            data.get('rcs_type') == RevisionControlSystems.GIT)
+        super(CodeImportNewView, self).validate_widgets(data, names=names)
+
     def validate(self, data):
         """See `LaunchpadFormView`."""
-        # Make sure that the user is able to create branches for the specified
-        # namespace.
+        rcs_type = data['rcs_type']
+        if rcs_type == RevisionControlSystems.GIT:
+            target_rcs_type = data.get(
+                'git_target_rcs_type', TargetRevisionControlSystems.BZR)
+        else:
+            target_rcs_type = TargetRevisionControlSystems.BZR
+
+        # Make sure that the user is able to create branches/repositories
+        # for the specified namespace.
         product = self.getProduct(data)
         # 'owner' in data may be None if it failed validation.
         owner = data.get('owner')
         if product is not None and owner is not None:
-            namespace = get_branch_namespace(owner, product)
-            policy = IBranchNamespacePolicy(namespace)
-            if not policy.canCreateBranches(self.user):
+            if target_rcs_type == TargetRevisionControlSystems.BZR:
+                namespace = get_branch_namespace(owner, product)
+                policy = IBranchNamespacePolicy(namespace)
+                can_create = policy.canCreateBranches(self.user)
+            else:
+                namespace = get_git_namespace(product, owner)
+                policy = IGitNamespacePolicy(namespace)
+                can_create = policy.canCreateRepositories(self.user)
+            if not can_create:
                 self.setFieldError(
                     'product',
                     "You are not allowed to register imports for %s."
                     % product.displayname)
 
-        rcs_type = data['rcs_type']
-        target_rcs_type = TargetRevisionControlSystems.BZR
         # Make sure fields for unselected revision control systems
         # are blanked out:
         if rcs_type == RevisionControlSystems.CVS:

=== modified file 'lib/lp/code/interfaces/gitnamespace.py'
--- lib/lp/code/interfaces/gitnamespace.py	2016-10-12 23:24:24 +0000
+++ lib/lp/code/interfaces/gitnamespace.py	2016-10-15 02:22:43 +0000
@@ -104,6 +104,13 @@
         "Can recipe names reasonably be generated from the target name "
         "rather than the branch name?")
 
+    def canCreateRepositories(user):
+        """Is the user allowed to create repositories for this namespace?
+
+        :param user: An `IPerson`.
+        :return: A Boolean value.
+        """
+
     def getAllowedInformationTypes(who):
         """Get the information types that a repository in this namespace can
         have.

=== modified file 'lib/lp/code/model/codeimport.py'
--- lib/lp/code/model/codeimport.py	2016-10-15 01:47:26 +0000
+++ lib/lp/code/model/codeimport.py	2016-10-15 02:22:43 +0000
@@ -63,7 +63,6 @@
 from lp.code.interfaces.githosting import IGitHostingClient
 from lp.code.interfaces.gitnamespace import get_git_namespace
 from lp.code.interfaces.gitrepository import IGitRepository
-from lp.code.interfaces.hasbranches import IHasCodeImports
 from lp.code.interfaces.hasgitrepositories import IHasGitRepositories
 from lp.code.mail.codeimport import code_import_updated
 from lp.code.model.codeimportjob import CodeImportJobWorkflow

=== modified file 'lib/lp/code/model/gitnamespace.py'
--- lib/lp/code/model/gitnamespace.py	2016-10-12 23:24:24 +0000
+++ lib/lp/code/model/gitnamespace.py	2016-10-15 02:22:43 +0000
@@ -211,6 +211,19 @@
             match = default
         return match
 
+    def canCreateRepositories(self, user):
+        """See `IGitNamespacePolicy`."""
+        try:
+            self.validateRegistrant(user)
+        except GitRepositoryCreatorNotMemberOfOwnerTeam:
+            return False
+        except GitRepositoryCreatorNotOwner:
+            return False
+        except GitRepositoryCreationForbidden:
+            return False
+        else:
+            return True
+
     def getAllowedInformationTypes(self, who=None):
         """See `IGitNamespace`."""
         raise NotImplementedError

=== modified file 'lib/lp/code/model/tests/test_gitnamespace.py'
--- lib/lp/code/model/tests/test_gitnamespace.py	2016-10-05 10:18:56 +0000
+++ lib/lp/code/model/tests/test_gitnamespace.py	2016-10-15 02:22:43 +0000
@@ -520,6 +520,105 @@
         self.assertEqual(dsp, namespace.target)
 
 
+class BaseCanCreateRepositoriesMixin:
+    """Common tests for all namespaces."""
+
+    layer = DatabaseFunctionalLayer
+
+    def _getNamespace(self, owner):
+        # Return a namespace appropriate for the owner specified.
+        raise NotImplementedError(self._getNamespace)
+
+    def test_individual(self):
+        # For a GitNamespace for an individual, only the individual can own
+        # repositories there.
+        person = self.factory.makePerson()
+        namespace = self._getNamespace(person)
+        self.assertTrue(namespace.canCreateRepositories(person))
+
+    def test_other_user(self):
+        # Any other individual cannot own repositories targeted to the
+        # person.
+        person = self.factory.makePerson()
+        namespace = self._getNamespace(person)
+        self.assertFalse(
+            namespace.canCreateRepositories(self.factory.makePerson()))
+
+    def test_team_member(self):
+        # A member of a team is able to create a repository in this
+        # namespace.
+        person = self.factory.makePerson()
+        self.factory.makeTeam(owner=person)
+        namespace = self._getNamespace(person)
+        self.assertTrue(namespace.canCreateRepositories(person))
+
+    def test_team_non_member(self):
+        # A person who is not part of the team cannot create repositories
+        # for the personal team target.
+        person = self.factory.makePerson()
+        self.factory.makeTeam(owner=person)
+        namespace = self._getNamespace(person)
+        self.assertFalse(
+            namespace.canCreateRepositories(self.factory.makePerson()))
+
+
+class TestPersonalGitNamespaceCanCreateRepositories(
+    TestCaseWithFactory, BaseCanCreateRepositoriesMixin):
+
+    def _getNamespace(self, owner):
+        return PersonalGitNamespace(owner)
+
+
+class TestPackageGitNamespaceCanCreateBranches(
+    TestCaseWithFactory, BaseCanCreateRepositoriesMixin):
+
+    def _getNamespace(self, owner):
+        source_package = self.factory.makeSourcePackage()
+        return PackageGitNamespace(owner, source_package)
+
+
+class TestProjectGitNamespaceCanCreateBranches(
+    TestCaseWithFactory, BaseCanCreateRepositoriesMixin):
+
+    def _getNamespace(self, owner,
+                      branch_sharing_policy=BranchSharingPolicy.PUBLIC):
+        project = self.factory.makeProduct(
+            branch_sharing_policy=branch_sharing_policy)
+        return ProjectGitNamespace(owner, project)
+
+    def setUp(self):
+        # Setting visibility policies is an admin-only task.
+        super(TestProjectGitNamespaceCanCreateBranches, self).setUp(
+            "[e-mail]")
+
+    def test_any_person(self):
+        # If there is no privacy set up, any person can create a personal
+        # branch on the product.
+        person = self.factory.makePerson()
+        namespace = self._getNamespace(person, BranchSharingPolicy.PUBLIC)
+        self.assertTrue(namespace.canCreateRepositories(person))
+
+    def test_any_person_with_proprietary_repositories(self):
+        # If the sharing policy defaults to PROPRIETARY, then non-privileged
+        # users cannot create a repository.
+        person = self.factory.makePerson()
+        namespace = self._getNamespace(person, BranchSharingPolicy.PROPRIETARY)
+        self.assertFalse(namespace.canCreateRepositories(person))
+
+    def test_grantee_with_proprietary_repositories(self):
+        # If the sharing policy defaults to PROPRIETARY, then non-privileged
+        # users cannot create a repository.
+        person = self.factory.makePerson()
+        other_person = self.factory.makePerson()
+        team = self.factory.makeTeam(members=[person])
+        namespace = self._getNamespace(team, BranchSharingPolicy.PROPRIETARY)
+        getUtility(IService, "sharing").sharePillarInformation(
+            namespace.target, team, namespace.target.owner,
+            {InformationType.PROPRIETARY: SharingPermission.ALL})
+        self.assertTrue(namespace.canCreateRepositories(person))
+        self.assertFalse(namespace.canCreateRepositories(other_person))
+
+
 class TestNamespaceSet(TestCaseWithFactory):
     """Tests for `get_namespace`."""
 

=== modified file 'lib/lp/code/stories/codeimport/xx-create-codeimport.txt'
--- lib/lp/code/stories/codeimport/xx-create-codeimport.txt	2016-10-13 14:46:41 +0000
+++ lib/lp/code/stories/codeimport/xx-create-codeimport.txt	2016-10-15 02:22:43 +0000
@@ -66,7 +66,7 @@
 The user is required to enter a project that the import is for, a name
 for the import branch, and a Bazaar branch location.
 
-    >>> browser.getControl('Branch Name').value = "mirrored"
+    >>> browser.getControl('Name').value = "mirrored"
     >>> browser.getControl('Branch URL', index=0).value = (
     ...     "http://bzr.example.com/firefox/trunk")
     >>> browser.getControl('Request Import').click()
@@ -87,7 +87,7 @@
 URL.
 
     >>> browser.open("http://code.launchpad.dev/+code-imports/+new")
-    >>> browser.getControl('Branch Name').value = "with-password"
+    >>> browser.getControl('Name').value = "with-password"
     >>> browser.getControl('Branch URL', index=0).value = (
     ...     "http://user:[e-mail]/firefox/trunk")
     >>> browser.getControl('Project').value = "firefox"
@@ -102,7 +102,7 @@
 Specifying a Launchpad URL results in an error.
 
     >>> browser.open("http://code.launchpad.dev/+code-imports/+new")
-    >>> browser.getControl('Branch Name').value = "invalid"
+    >>> browser.getControl('Name').value = "invalid"
     >>> browser.getControl('Branch URL', index=0).value = (
     ...     "http://bazaar.launchpad.net/firefox/trunk")
     >>> browser.getControl('Project').value = "firefox"
@@ -116,7 +116,7 @@
 
     >>> browser.open("http://code.launchpad.dev/+code-imports/+new")
     >>> browser.getControl('Project').value = "firefox"
-    >>> browser.getControl('Branch Name').value = "lp-git-import"
+    >>> browser.getControl('Name').value = "lp-git-import"
     >>> browser.getControl('Git').click()
     >>> browser.getControl('Repo URL', index=0).value = (
     ...     "git://git.launchpad.net/firefox.git")
@@ -136,7 +136,7 @@
     >>> browser.open("http://code.launchpad.dev/+code-imports/+new")
     >>> browser.getControl('Subversion').click()
     >>> browser.getControl('Project').value = "firefox"
-    >>> browser.getControl('Branch Name').value = "imported"
+    >>> browser.getControl('Name').value = "imported"
     >>> browser.getControl('Branch URL', index=1).value = (
     ...     "http://svn.example.com/firefox/trunk")
     >>> browser.getControl('Request Import').click()
@@ -167,7 +167,7 @@
 
     >>> browser.open("http://code.launchpad.dev/+code-imports/+new")
     >>> browser.getControl('Subversion').click()
-    >>> browser.getControl('Branch Name').value = "svn-with-password"
+    >>> browser.getControl('Name').value = "svn-with-password"
     >>> browser.getControl('Branch URL', index=1).value = (
     ...     "http://user:[e-mail]/firefox/trunk")
     >>> browser.getControl('Project').value = "firefox"
@@ -180,21 +180,21 @@
     as soon as possible.
 
 
-Requesting a Git import
-=======================
+Requesting a Git-to-Bazaar import
+=================================
 
 The user is required to enter a project that the import is for,
-a name for the import branch, and a subversion branch location.
+a name for the import branch, and a Git repository location.
 
     >>> browser.open("http://code.launchpad.dev/+code-imports/+new")
     >>> browser.getControl('Project').value = "firefox"
-    >>> browser.getControl('Branch Name').value = "git-import"
+    >>> browser.getControl('Name').value = "git-import"
     >>> browser.getControl('Git').click()
     >>> browser.getControl('Repo URL', index=0).value = (
     ...     "git://example.com/firefox.git")
     >>> browser.getControl('Request Import').click()
 
-When the user clicks continue, the approved import branch is created
+When the user clicks continue, the approved import branch is created.
 
     >>> print extract_text(find_tag_by_id(browser.contents, "import-details"))
     Import Status: Reviewed
@@ -203,6 +203,38 @@
     The next import is scheduled to run as soon as possible.
 
 
+Requesting a Git-to-Git import
+==============================
+
+The user is required to enter a project that the import is for,
+a name for the import repository, and a Git repository location.
+
+    >>> from lp.code.interfaces.codeimport import (
+    ...     CODE_IMPORT_GIT_TARGET_FEATURE_FLAG,
+    ...     )
+    >>> from lp.code.tests.helpers import GitHostingFixture
+    >>> from lp.services.features.testing import FeatureFixture
+
+    >>> with FeatureFixture({CODE_IMPORT_GIT_TARGET_FEATURE_FLAG: u'on'}):
+    ...     browser.open("http://code.launchpad.dev/+code-imports/+new")
+    ...     browser.getControl('Project').value = "firefox"
+    ...     browser.getControl('Name').value = "upstream"
+    ...     browser.getControl('Git', index=0).click()
+    ...     browser.getControl('Git', index=1).click()
+    ...     browser.getControl('Repo URL', index=0).value = (
+    ...         "git://example.com/firefox2.git")
+    ...     with GitHostingFixture():
+    ...         browser.getControl('Request Import').click()
+
+When the user clicks continue, the approved import repository is created.
+
+    >>> print extract_text(find_tag_by_id(browser.contents, "import-details"))
+    Import Status: Reviewed
+    This repository is an import of the Git repository at
+    git://example.com/firefox2.git.
+    The next import is scheduled to run as soon as possible.
+
+
 Requesting a CVS import
 =======================
 
@@ -211,7 +243,7 @@
 
     >>> browser.open("http://code.launchpad.dev/+code-imports/+new")
     >>> browser.getControl('Project').value = "firefox"
-    >>> browser.getControl('Branch Name').value = "import2"
+    >>> browser.getControl('Name').value = "import2"
     >>> browser.getControl('CVS').click()
     >>> browser.getControl('Repository').value = (
     ...     ":pserver:[e-mail]:/mozilla/cvs")
@@ -233,7 +265,7 @@
 
     >>> browser.open("http://code.launchpad.dev/+code-imports/+new")
     >>> browser.getControl('Project').value = "firefox"
-    >>> browser.getControl('Branch Name').value = "import2"
+    >>> browser.getControl('Name').value = "import2"
     >>> browser.getControl('CVS').click()
     >>> browser.getControl('Repository').value = (
     ...     ":[e-mail]:/mozilla/cvs")
@@ -256,7 +288,7 @@
 The error is shown even if the project is different.
 
     >>> browser.getControl('Project').value = "thunderbird"
-    >>> browser.getControl('Branch Name').value = "imported"
+    >>> browser.getControl('Name').value = "imported"
     >>> browser.getControl('CVS').click()
     >>> browser.getControl('Repository').value = (
     ...     ":pserver:[e-mail]:/mozilla/cvs")
@@ -290,7 +322,7 @@
     >>> browser.open("http://code.launchpad.dev/+code-imports/+new")
     >>> browser.getControl('Subversion').click()
     >>> browser.getControl('Project').value = "firefox"
-    >>> browser.getControl('Branch Name').value = "imported"
+    >>> browser.getControl('Name').value = "imported"
     >>> browser.getControl('Branch URL', index=1).value = (
     ...     "http://svn.example.com/firefox/other")
     >>> browser.getControl('Request Import').click()
@@ -307,7 +339,7 @@
 
     >>> browser.open("http://code.launchpad.dev/+code-imports/+new")
     >>> browser.getControl('Project').value = "launchpad"
-    >>> browser.getControl('Branch Name').value = "imported"
+    >>> browser.getControl('Name').value = "imported"
     >>> browser.getControl('Branch URL', index=0).value = (
     ...     "http://svn.example.com/launchpage/fake")
     >>> browser.getControl('Request Import').click()
@@ -324,7 +356,7 @@
 
     >>> browser.open("http://code.launchpad.dev/+code-imports/+new")
     >>> browser.getControl('Project').value = "no-such-product"
-    >>> browser.getControl('Branch Name').value = "imported"
+    >>> browser.getControl('Name').value = "imported"
     >>> browser.getControl('Branch URL', index=0).value = (
     ...     "http://svn.example.com/launchpage/fake")
     >>> browser.getControl('Request Import').click()

=== modified file 'lib/lp/code/templates/codeimport-new.pt'
--- lib/lp/code/templates/codeimport-new.pt	2012-10-09 01:07:52 +0000
+++ lib/lp/code/templates/codeimport-new.pt	2016-10-15 02:22:43 +0000
@@ -45,7 +45,7 @@
         <tr>
           <td colspan="2">
             <div class="formHelp">
-              Enter details for the selected repository type.
+              Enter details for the selected version control system.
             </div>
           </td>
         </tr>
@@ -74,6 +74,10 @@
               <tal:widget define="widget nocall:view/widgets/git_repo_url">
                 <metal:block use-macro="context/@@launchpad_form/widget_row" />
               </tal:widget>
+              <tal:widget define="widget nocall:view/widgets/git_target_rcs_type"
+                          condition="request/features/code.import.git_target">
+                <metal:block use-macro="context/@@launchpad_form/widget_row" />
+              </tal:widget>
             </table>
           </td>
         </tr>
@@ -131,6 +135,12 @@
                 }
             }
             updateField(form['field.git_repo_url'], rcs_type === 'GIT');
+            for (i = 0; i < form.elements.length; i++) {
+                if (form.elements[i].id.startsWith(
+                        'field.git_target_rcs_type.')) {
+                    updateField(form.elements[i], rcs_type === 'GIT');
+                }
+            }
             updateField(form['field.cvs_root'], rcs_type === 'CVS');
             updateField(form['field.cvs_module'], rcs_type === 'CVS');
             updateField(form['field.svn_branch_url'], rcs_type === 'BZR_SVN');