Colin Watson has proposed merging 
lp:~cjwatson/launchpad/codeimport-git-read-only-views into lp:launchpad with 
lp:~cjwatson/launchpad/git-code-import-security-deletion as a prerequisite.

Commit message:
Add read-only views for Git-targeted code imports.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1469459 in Launchpad itself: "import external code into a LP git repo 
(natively)"
  https://bugs.launchpad.net/launchpad/+bug/1469459

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/codeimport-git-read-only-views/+merge/308374

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.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
lp:~cjwatson/launchpad/codeimport-git-read-only-views into lp:launchpad.
=== 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-13 12:55:20 +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 (
@@ -403,7 +403,7 @@
 
 
 class BranchView(InformationTypePortletMixin, FeedsMixin, BranchMirrorMixin,
-                 LaunchpadView, HasSnapsViewMixin):
+                 LaunchpadView, HasSnapsViewMixin, CodeImportTargetMixin):
 
     feed_types = (
         BranchFeedLink,
@@ -596,31 +596,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 +774,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-13 12:55:20 +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.GIT):
+            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-13 12:55:20 +0000
@@ -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/gitrepository-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-13 12:55:20 +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,

=== 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-13 12:55:20 +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-13 12:55:20 +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-13 12:55:20 +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-13 12:55:20 +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-13 12:55:20 +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-13 12:55:20 +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-13 12:55:20 +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>

=== added file 'lib/lp/code/templates/gitrepository-import-details.pt'
--- lib/lp/code/templates/gitrepository-import-details.pt	1970-01-01 00:00:00 +0000
+++ lib/lp/code/templates/gitrepository-import-details.pt	2016-10-13 12:55:20 +0000
@@ -0,0 +1,95 @@
+<div
+  xmlns:tal="http://xml.zope.org/namespaces/tal";
+  xmlns:metal="http://xml.zope.org/namespaces/metal";
+  xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+  tal:define="context_menu view/context/menu:context">
+
+  <tal:imported-repository
+      condition="context/repository_type/enumvalue:IMPORTED">
+    <div id="import-details" tal:define="repository context;
+                                         code_import repository/code_import">
+      <tal:has-codeimport condition="repository/code_import"
+                          define="code_import 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"/>
+        </div>
+
+        <tal:git-import condition="code_import/rcs_type/enumvalue:GIT">
+          <p id="git-import-details">
+            This repository is an 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" />.
+            </tal:is-web-url>
+            <tal:not-web-url condition="not: view/url_is_web">
+              <span tal:replace="code_import/url" />.
+            </tal:not-web-url>
+          </p>
+        </tal:git-import>
+
+        <tal:has-job define="job code_import/import_job"
+                     condition="job">
+          <div>
+          <tal:is-running condition="job/state/enumvalue:RUNNING">
+            An import is currently running on
+            <tal:machine content="structure job/machine/fmt:link" />,
+            and was started
+            <tal:date-started replace="job/date_started/fmt:displaydate">
+              2 hours ago
+            </tal:date-started>.
+            <tal:is-logtail condition="job/logtail">
+              The last few lines of the job's output were:
+              <div class="logtail">
+                <tal:logtail content="structure job/logtail/fmt:nice_pre" />
+              </div>
+            </tal:is-logtail>
+          </tal:is-running>
+          <tal:not-running condition="not: job/state/enumvalue:RUNNING">
+            The next import is scheduled to run
+            <tal:overdue condition="job/isOverdue">
+              as soon as possible<tal:requested-by
+                condition="job/requesting_user">
+                (requested by
+                <tal:requested-by-user
+                   replace="structure job/requesting_user/fmt:link">
+                  Some user.
+                </tal:requested-by-user>)</tal:requested-by>.
+            </tal:overdue>
+            <tal:not-overdue condition="not: job/isOverdue">
+              <tal:date-started replace="job/date_due/fmt:displaydate">
+                in 2 hours
+              </tal:date-started>.
+            </tal:not-overdue>
+          </tal:not-running>
+          </div>
+        </tal:has-job>
+
+        <tal:failing condition="code_import/review_status/enumvalue:FAILING">
+          <div id="failing-try-again" class="message warning">
+            The import has been suspended because it failed
+            <tal:failure-limit content="modules/lp.services.config/config/codeimport/consecutive_failure_limit"/>
+            or more times in succession.
+          </div>
+        </tal:failing>
+
+        <tal:last-successful condition="code_import/date_last_successful">
+          <p>
+            Last successful import was
+            <tal:last-successful replace="code_import/date_last_successful/fmt:displaydate">
+              2 hours ago
+            </tal:last-successful>.
+          </p>
+        </tal:last-successful>
+
+        <div id="import-results" tal:condition="view/latest_code_import_results">
+          <tal:result repeat="result view/latest_code_import_results">
+            <metal:result use-macro="code_import/@@+macros/show_result"/>
+          </tal:result>
+        </div>
+      </tal:has-codeimport>
+    </div>
+  </tal:imported-repository>
+
+</div>

=== 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-13 12:55:20 +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">

_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to     : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Reply via email to