Merge lp:~cjwatson/launchpad/codeimport-git-read-only-views into lp:launchpad

[Colin Watson]

Add read-only views for Git-targeted code imports.

We don't have much in the way of tests for this yet, but that will be easier to bring up once we have creation webservice/view handling in place.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py	2016-05-07 00:40:18 +0000
+++ lib/lp/code/browser/branch.py	2016-10-14 15:45:46 +0000
@@ -89,12 +89,12 @@
     latest_proposals_for_each_branch,
     )
 from lp.code.browser.branchref import BranchRef
+from lp.code.browser.codeimport import CodeImportTargetMixin
 from lp.code.browser.decorations import DecoratedBranch
 from lp.code.browser.sourcepackagerecipelisting import HasRecipesMenuMixin
 from lp.code.browser.widgets.branchtarget import BranchTargetWidget
 from lp.code.enums import (
     BranchType,
-    CodeImportResultStatus,
     CodeImportReviewStatus,
     )
 from lp.code.errors import (
@@ -240,9 +240,6 @@
     links = (
         'edit', 'reviewer', 'edit_whiteboard', 'webhooks', 'delete')
 
-    def branch_is_import(self):
-        return self.context.branch_type == BranchType.IMPORTED
-
     @enabled_with_permission('launchpad.Edit')
     def edit(self):
         text = 'Change branch details'
@@ -256,7 +253,7 @@
     @enabled_with_permission('launchpad.AnyPerson')
     def edit_whiteboard(self):
         text = 'Edit whiteboard'
-        enabled = self.branch_is_import()
+        enabled = self.context.branch_type == BranchType.IMPORTED
         return Link(
             '+whiteboard', text, icon='edit', enabled=enabled)
 
@@ -403,7 +400,7 @@
 
 
 class BranchView(InformationTypePortletMixin, FeedsMixin, BranchMirrorMixin,
-                 LaunchpadView, HasSnapsViewMixin):
+                 LaunchpadView, HasSnapsViewMixin, CodeImportTargetMixin):
 
     feed_types = (
         BranchFeedLink,
@@ -448,8 +445,7 @@
 
         The whiteboard is only shown for import branches.
         """
-        if (self.context.branch_type == BranchType.IMPORTED and
-            self.context.whiteboard):
+        if self.is_imported and self.context.whiteboard:
             return True
         else:
             return False
@@ -536,11 +532,23 @@
                 count).escapedtext
 
     @property
+    def is_imported(self):
+        """Is this an imported branch?"""
+        return self.context.branch_type == BranchType.IMPORTED
+
+    @property
+    def can_edit_import(self):
+        """Can the user edit this import?"""
+        # XXX cjwatson 2016-10-14: Delete this once we have views for
+        # editing Git imports.
+        return True
+
+    @property
     def is_import_branch_with_no_landing_candidates(self):
         """Is the branch an import branch with no landing candidates?"""
         if self.landing_candidates:
             return False
-        if not self.context.branch_type == BranchType.IMPORTED:
+        if not self.is_imported:
             return False
         return True
 
@@ -596,31 +604,6 @@
         return collection.getExtendedRevisionDetails(
             self.user, self.context.latest_revisions)
 
-    @cachedproperty
-    def latest_code_import_results(self):
-        """Return the last 10 CodeImportResults."""
-        return list(self.context.code_import.results[:10])
-
-    def iconForCodeImportResultStatus(self, status):
-        """The icon to represent the `CodeImportResultStatus` `status`."""
-        if status == CodeImportResultStatus.SUCCESS_PARTIAL:
-            return "/@@/yes-gray"
-        elif status in CodeImportResultStatus.successes:
-            return "/@@/yes"
-        else:
-            return "/@@/no"
-
-    @property
-    def url_is_web(self):
-        """True if an imported branch's URL is HTTP or HTTPS."""
-        # You should only be calling this if it's an SVN, BZR or GIT code
-        # import
-        assert self.context.code_import
-        url = self.context.code_import.url
-        assert url
-        # https starts with http too!
-        return url.startswith("http")
-
     @property
     def show_merge_links(self):
         """Return whether or not merge proposal links should be shown.
@@ -799,7 +782,7 @@
                 target is not None and target != self.context.target):
                 try:
                     self.context.setTarget(self.user, project=target)
-                except BranchTargetError, e:
+                except BranchTargetError as e:
                     self.setFieldError('target', e.message)
                     return
 

=== modified file 'lib/lp/code/browser/codeimport.py'
--- lib/lp/code/browser/codeimport.py	2016-10-06 15:59:34 +0000
+++ lib/lp/code/browser/codeimport.py	2016-10-14 15:45:46 +0000
@@ -12,6 +12,7 @@
     'CodeImportSetBreadcrumb',
     'CodeImportSetNavigation',
     'CodeImportSetView',
+    'CodeImportTargetMixin',
     'validate_import_url',
     ]
 
@@ -49,6 +50,7 @@
 from lp.code.enums import (
     BranchSubscriptionDiffSize,
     BranchSubscriptionNotificationLevel,
+    CodeImportResultStatus,
     CodeImportReviewStatus,
     CodeReviewNotificationLevel,
     NON_CVS_RCS_TYPES,
@@ -191,8 +193,8 @@
                 self.addError(structured("""
                     Those CVS details are already specified for
                     the imported branch <a href="%s">%s</a>.""",
-                    canonical_url(code_import.branch),
-                    code_import.branch.unique_name))
+                    canonical_url(code_import.target),
+                    code_import.target.unique_name))
 
     def _validateURL(self, url, rcs_type, existing_import=None,
                      field_name='url'):
@@ -495,12 +497,12 @@
 class CodeImportEditView(CodeImportBaseView):
     """View for editing code imports.
 
-    This view is registered against the branch, but mostly edits the code
-    import for that branch -- the exception being that it also allows the
-    editing of the branch whiteboard.  If the branch has no associated code
-    import, then the result is a 404.  If the branch does have a code import,
-    then the adapters property allows the form internals to do the associated
-    mappings.
+    This view is registered against the target, but mostly edits the code
+    import for that target -- the exception being that it also allows the
+    editing of the branch whiteboard in the case of Bazaar branches.  If the
+    target has no associated code import, then the result is a 404.  If the
+    target does have a code import, then the adapters property allows the
+    form internals to do the associated mappings.
     """
 
     schema = EditCodeImportForm
@@ -513,16 +515,20 @@
 
     @property
     def initial_values(self):
-        return {'whiteboard': self.context.whiteboard}
+        if (self.code_import.target_rcs_type ==
+                TargetRevisionControlSystems.BZR):
+            return {'whiteboard': self.context.whiteboard}
+        else:
+            return {}
 
     def initialize(self):
-        """Show a 404 if the branch has no code import."""
+        """Show a 404 if the target has no code import."""
         self.code_import = self.context.code_import
         if self.code_import is None:
             raise NotFoundError
         if not self._super_user:
             raise Unauthorized
-        # The next and cancel location is the branch details page.
+        # The next and cancel location is the target details page.
         self.cancel_url = self.next_url = canonical_url(self.context)
         super(CodeImportEditView, self).initialize()
 
@@ -543,6 +549,10 @@
         else:
             raise AssertionError('Unknown rcs_type for code import.')
 
+        if (self.code_import.target_rcs_type !=
+                TargetRevisionControlSystems.BZR):
+            self.form_fields = self.form_fields.omit('whiteboard')
+
     def _showButtonForStatus(self, status):
         """If the status is different, and the user is super, show button."""
         return self._super_user and self.code_import.review_status != status
@@ -604,3 +614,32 @@
             "This foreign branch URL is already specified for the imported "
             "branch <a href='%s'>%s</a>.", canonical_url(code_import.branch),
             code_import.branch.unique_name)
+
+
+class CodeImportTargetMixin:
+    """Common code import methods for Branch and GitRepository views."""
+
+    @cachedproperty
+    def latest_code_import_results(self):
+        """Return the last 10 CodeImportResults."""
+        return list(self.context.code_import.results[:10])
+
+    def iconForCodeImportResultStatus(self, status):
+        """The icon to represent the `CodeImportResultStatus` `status`."""
+        if status == CodeImportResultStatus.SUCCESS_PARTIAL:
+            return "/@@/yes-gray"
+        elif status in CodeImportResultStatus.successes:
+            return "/@@/yes"
+        else:
+            return "/@@/no"
+
+    @property
+    def url_is_web(self):
+        """True if an imported branch's URL is HTTP or HTTPS."""
+        # You should only be calling this if it's an SVN, BZR or GIT code
+        # import
+        assert self.context.code_import
+        url = self.context.code_import.url
+        assert url
+        # https starts with http too!
+        return url.startswith("http")

=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml	2016-06-25 08:05:06 +0000
+++ lib/lp/code/browser/configure.zcml	2016-10-14 15:45:46 +0000
@@ -361,7 +361,7 @@
             template="../templates/branch-recipes.pt" />
         <browser:page
             name="++branch-import-details"
-            template="../templates/branch-import-details.pt" />
+            template="../templates/import-details.pt" />
         <browser:page
             name="++branch-revisions"
             template="../templates/branch-revisions.pt" />
@@ -629,7 +629,7 @@
             permission="launchpad.AnyPerson"/>
     <browser:url
         for="lp.code.interfaces.codeimport.ICodeImport"
-        attribute_to_parent="branch"
+        attribute_to_parent="target"
         path_expression="string:+code-import"
         rootsite="code"/>
     <browser:navigation
@@ -815,6 +815,9 @@
         <browser:page
             name="++repository-recipes"
             template="../templates/gitrepository-recipes.pt"/>
+        <browser:page
+            name="++repository-import-details"
+            template="../templates/import-details.pt"/>
     </browser:pages>
     <browser:page
         for="lp.code.interfaces.gitrepository.IGitRepository"

=== modified file 'lib/lp/code/browser/gitrepository.py'
--- lib/lp/code/browser/gitrepository.py	2016-10-05 10:07:08 +0000
+++ lib/lp/code/browser/gitrepository.py	2016-10-14 15:45:46 +0000
@@ -53,6 +53,7 @@
 from lp.app.vocabularies import InformationTypeVocabulary
 from lp.app.widgets.itemswidgets import LaunchpadRadioWidgetWithDescription
 from lp.code.browser.branch import CodeEditOwnerMixin
+from lp.code.browser.codeimport import CodeImportTargetMixin
 from lp.code.browser.sourcepackagerecipelisting import HasRecipesMenuMixin
 from lp.code.browser.widgets.gitrepositorytarget import (
     GitRepositoryTargetDisplayWidget,
@@ -191,6 +192,11 @@
             return None
         return GitRepositoryDiffView(self.context, self.request, old, new)
 
+    @stepto("+code-import")
+    def traverse_code_import(self):
+        """Traverses to the `ICodeImport` for the repository."""
+        return self.context.code_import
+
 
 class GitRepositoryEditMenu(NavigationMenu):
     """Edit menu for `IGitRepository`."""
@@ -294,7 +300,7 @@
 
 
 class GitRepositoryView(InformationTypePortletMixin, LaunchpadView,
-                        HasSnapsViewMixin):
+                        HasSnapsViewMixin, CodeImportTargetMixin):
 
     related_features = {
         "code.git.recipes.enabled": False,
@@ -345,6 +351,18 @@
                 '<a href="+recipes">%s recipes</a> using this repository.',
                 count).escapedtext
 
+    @property
+    def is_imported(self):
+        """Is this an imported repository?"""
+        return self.context.repository_type == GitRepositoryType.IMPORTED
+
+    @property
+    def can_edit_import(self):
+        """Can the user edit this import?"""
+        # XXX cjwatson 2016-10-14: Delete this once we have views for
+        # editing Git imports.
+        return False
+
 
 class GitRepositoryEditFormView(LaunchpadEditFormView):
     """Base class for forms that edit a Git repository."""

=== modified file 'lib/lp/code/browser/tests/test_codeimport.py'
--- lib/lp/code/browser/tests/test_codeimport.py	2014-02-24 07:19:52 +0000
+++ lib/lp/code/browser/tests/test_codeimport.py	2016-10-14 15:45:46 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for the code import browser code."""
@@ -7,9 +7,14 @@
 
 import re
 
+from testtools.matchers import StartsWith
 from zope.security.interfaces import Unauthorized
 
-from lp.code.enums import RevisionControlSystems
+from lp.code.enums import (
+    RevisionControlSystems,
+    TargetRevisionControlSystems,
+    )
+from lp.code.tests.helpers import GitHostingFixture
 from lp.services.webapp import canonical_url
 from lp.testing import (
     person_logged_in,
@@ -27,28 +32,43 @@
 
     layer = DatabaseFunctionalLayer
 
-    def assertSvnDetailsDisplayed(self, svn_details, rcs_type):
-        """Assert the `svn_details` tag describes a Subversion import.
+    def assertImportDetailsDisplayed(self, code_import, details_id,
+                                     prefix_text, span_title=None):
+        """A code import has its details displayed properly.
 
-        :param svn_details: The BeautifulSoup object for the
-            'svn-import-details' area.
-        :param rcs_type: SVN or BZR_SVN from RevisionControlSystems.
+        :param code_import: An `ICodeImport`.
+        :param details_id: The HTML tag id to search for.
+        :param prefix_text: An expected prefix of the details text.
+        :param span_title: If present, the expected contents of a span title
+            attribute.
         """
-        self.assertEquals(rcs_type.title, svn_details.span['title'])
-        text = re.sub('\s+', ' ', extract_text(svn_details))
-        self.assertTrue(
-            text.startswith(
-                'This branch is an import of the Subversion branch'))
+        browser = self.getUserBrowser(canonical_url(code_import.target))
+        details = find_tag_by_id(browser.contents, details_id)
+        if span_title is not None:
+            self.assertEqual(span_title, details.span['title'])
+        text = re.sub('\s+', ' ', extract_text(details))
+        self.assertThat(text, StartsWith(prefix_text))
 
     def test_bzr_svn_import(self):
-        # The branch page for a bzr-svn-imported branch contains
+        # The branch page for a bzr-svn-imported branch contains a summary
+        # of the import details.
+        code_import = self.factory.makeCodeImport(
+            rcs_type=RevisionControlSystems.BZR_SVN)
+        self.assertImportDetailsDisplayed(
+            code_import, 'svn-import-details',
+            'This branch is an import of the Subversion branch',
+            span_title=RevisionControlSystems.BZR_SVN.title)
+
+    def test_git_to_git_import(self):
+        # The repository page for a git-to-git-imported repository contains
         # a summary of the import details.
-        bzr_svn_import = self.factory.makeCodeImport(
-            rcs_type=RevisionControlSystems.BZR_SVN)
-        browser = self.getUserBrowser(canonical_url(bzr_svn_import.branch))
-        svn_details = find_tag_by_id(browser.contents, 'svn-import-details')
-        self.assertSvnDetailsDisplayed(
-            svn_details, RevisionControlSystems.BZR_SVN)
+        self.useFixture(GitHostingFixture())
+        code_import = self.factory.makeCodeImport(
+            rcs_type=RevisionControlSystems.GIT,
+            target_rcs_type=TargetRevisionControlSystems.GIT)
+        self.assertImportDetailsDisplayed(
+            code_import, 'git-import-details',
+            'This repository is an import of the Git repository')
 
     def test_branch_owner_of_import_forbidden(self):
         # Unauthorized users are forbidden to edit an import.

=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2016-10-07 15:50:10 +0000
+++ lib/lp/code/configure.zcml	2016-10-14 15:45:46 +0000
@@ -642,6 +642,7 @@
                        series
                        review_status
                        rcs_type
+                       target_rcs_type
                        cvs_root
                        cvs_module
                        url

=== modified file 'lib/lp/code/interfaces/codeimport.py'
--- lib/lp/code/interfaces/codeimport.py	2016-10-03 17:00:56 +0000
+++ lib/lp/code/interfaces/codeimport.py	2016-10-14 15:45:46 +0000
@@ -41,6 +41,7 @@
 from lp.code.enums import (
     CodeImportReviewStatus,
     RevisionControlSystems,
+    TargetRevisionControlSystems,
     )
 from lp.code.interfaces.branch import IBranch
 from lp.code.interfaces.gitrepository import IGitRepository
@@ -112,11 +113,16 @@
             description=_("Only reviewed imports are processed.")))
 
     rcs_type = exported(
-        Choice(title=_("Type of RCS"), readonly=True,
+        Choice(
+            title=_("Type of RCS"), readonly=True,
             required=True, vocabulary=RevisionControlSystems,
-            description=_(
-                "The version control system to import from. "
-                "Can be CVS or Subversion.")))
+            description=_("The revision control system to import from.")))
+
+    target_rcs_type = exported(
+        Choice(
+            title=_("Type of target RCS"), readonly=True,
+            required=True, vocabulary=TargetRevisionControlSystems,
+            description=_("The revision control system to import to.")))
 
     url = exported(
         URIField(title=_("URL"), required=False, readonly=True,

=== modified file 'lib/lp/code/model/codeimport.py'
--- lib/lp/code/model/codeimport.py	2016-10-11 14:26:31 +0000
+++ lib/lp/code/model/codeimport.py	2016-10-14 15:45:46 +0000
@@ -121,6 +121,13 @@
     rcs_type = EnumCol(schema=RevisionControlSystems,
         notNull=False, default=None)
 
+    @property
+    def target_rcs_type(self):
+        if self.branch is not None:
+            return TargetRevisionControlSystems.BZR
+        else:
+            return TargetRevisionControlSystems.GIT
+
     cvs_root = StringCol(default=None)
 
     cvs_module = StringCol(default=None)

=== modified file 'lib/lp/code/templates/branch-index.pt'
--- lib/lp/code/templates/branch-index.pt	2015-09-17 12:13:31 +0000
+++ lib/lp/code/templates/branch-index.pt	2016-10-14 15:45:46 +0000
@@ -113,7 +113,7 @@
        tal:condition="context/branch_type/enumvalue:IMPORTED">
     <div class="portlet">
       <h2>Import details</h2>
-      <tal:branch-pending-merges
+      <tal:branch-import-details
           replace="structure context/@@++branch-import-details" />
     </div>
   </div>

=== modified file 'lib/lp/code/templates/codeimport-machine-index.pt'
--- lib/lp/code/templates/codeimport-machine-index.pt	2009-08-18 00:25:14 +0000
+++ lib/lp/code/templates/codeimport-machine-index.pt	2016-10-14 15:45:46 +0000
@@ -42,7 +42,7 @@
         <table class="simple">
           <tal:jobs repeat="job jobs">
             <tr>
-              <td><tal:branch replace="structure job/code_import/branch/fmt:link"/></td>
+              <td><tal:target replace="structure job/code_import/target/fmt:link"/></td>
               <td>Started: <tal:started replace="job/date_started/fmt:approximatedate"/></td>
               <td>Last heartbeat: <tal:heartbeat replace="job/heartbeat/fmt:approximatedate"/></td>
             </tr>
@@ -71,7 +71,7 @@
             </tal:has-person>
             <tal:event replace="event/event_type/title" />
             <tal:code-import condition="event/code_import"
-                             replace="structure event/code_import/branch/fmt:link"/>
+                             replace="structure event/code_import/target/fmt:link"/>
           </dt>
           <dd tal:condition="event/items">
             <tal:items repeat="item event/items">

=== modified file 'lib/lp/code/templates/codeimport-machines.pt'
--- lib/lp/code/templates/codeimport-machines.pt	2009-08-18 00:22:50 +0000
+++ lib/lp/code/templates/codeimport-machines.pt	2016-10-14 15:45:46 +0000
@@ -40,9 +40,9 @@
                 <table class="simple">
                   <tal:jobs repeat="job jobs">
                     <tr>
-                      <td><tal:branch replace="structure job/code_import/branch/fmt:link"/></td>
-                      <td>Started: <tal:branch replace="job/date_started/fmt:approximatedate"/></td>
-                      <td>Last heartbeat: <tal:branch replace="job/heartbeat/fmt:approximatedate"/></td>
+                      <td><tal:target replace="structure job/code_import/target/fmt:link"/></td>
+                      <td>Started: <tal:started replace="job/date_started/fmt:approximatedate"/></td>
+                      <td>Last heartbeat: <tal:heartbeat replace="job/heartbeat/fmt:approximatedate"/></td>
                     </tr>
                   </tal:jobs>
                 </table>

=== modified file 'lib/lp/code/templates/gitrepository-index.pt'
--- lib/lp/code/templates/gitrepository-index.pt	2016-01-12 15:10:57 +0000
+++ lib/lp/code/templates/gitrepository-index.pt	2016-10-14 15:45:46 +0000
@@ -54,6 +54,16 @@
     </div>
   </div>
 
+  <div id="repository-import-details"
+       class="yui-g"
+       tal:condition="context/repository_type/enumvalue:IMPORTED">
+    <div class="portlet">
+      <h2>Import details</h2>
+      <tal:repository-import-details
+          replace="structure context/@@++repository-import-details" />
+    </div>
+  </div>
+
   <div class="yui-g">
     <div id="repository-branches" class="portlet"
          tal:define="branches view/branches">

=== renamed file 'lib/lp/code/templates/branch-import-details.pt' => 'lib/lp/code/templates/import-details.pt'
--- lib/lp/code/templates/branch-import-details.pt	2014-02-24 07:19:52 +0000
+++ lib/lp/code/templates/import-details.pt	2016-10-14 15:45:46 +0000
@@ -4,17 +4,19 @@
   xmlns:i18n="http://xml.zope.org/namespaces/i18n"
   tal:define="context_menu view/context/menu:context">
 
-  <tal:imported-branch tal:condition="context/branch_type/enumvalue:IMPORTED">
-    <div id="import-details" tal:define="branch context;
-                                         code_import branch/code_import">
-      <tal:has-codeimport condition="branch/code_import"
-                          define="code_import branch/code_import">
+  <tal:imported tal:condition="view/is_imported">
+    <div id="import-details"
+         tal:define="branch_or_repository context;
+                     code_import branch_or_repository/code_import">
+      <tal:has-codeimport
+          condition="branch_or_repository/code_import"
+          define="code_import branch_or_repository/code_import">
 
         <div><strong>Import Status:</strong>
           <span tal:attributes="class string:codeimport${code_import/review_status/name}"
                 tal:content="code_import/review_status/title"/>
             <form tal:attributes="action string:${context/fmt:url}/@@+try-again"
-                  tal:condition="python:view.user and code_import.review_status.name == 'FAILING'"
+                  tal:condition="python:view.can_edit_import and view.user and code_import.review_status.name == 'FAILING'"
                   style="display: inline; padding-left: 1em"
                   id="tryagain"
                   name="tryagain"
@@ -39,7 +41,16 @@
         </div>
 
         <tal:git-import condition="code_import/rcs_type/enumvalue:GIT">
-          <p>This branch is an import of the HEAD branch of the Git repository at
+          <p id="git-import-details">
+            <tal:git-to-bzr-import
+                condition="code_import/target_rcs_type/enumvalue:BZR">
+              This branch is an import of the HEAD branch
+            </tal:git-to-bzr-import>
+            <tal:git-to-git-import
+                condition="code_import/target_rcs_type/enumvalue:GIT">
+              This repository is an import
+            </tal:git-to-git-import>
+            of the Git repository at
             <tal:is-web-url condition="view/url_is_web">
               <a tal:attributes="href code_import/url"
                  tal:content="code_import/url" />.
@@ -117,7 +128,7 @@
                 in 2 hours
               </tal:date-started>.
               <tal:button
-                 condition="view/user"
+                 condition="python:view.can_edit_import and view.user"
                  replace="structure view/context/@@+request-import" />
             </tal:not-overdue>
           </tal:not-running>
@@ -147,7 +158,7 @@
           </tal:result>
         </div>
 
-        <div class="actions">
+        <div class="actions" tal:condition="view/can_edit_import">
           <div
             tal:define="link context_menu/edit_import"
             tal:condition="link/enabled"
@@ -160,6 +171,6 @@
         </div>
       </tal:has-codeimport>
     </div>
-  </tal:imported-branch>
+  </tal:imported>
 
 </div>