launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #21095
[Merge] lp:~cjwatson/launchpad/codeimport-git-read-only-views into lp:launchpad
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">
Follow ups
