launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #21097
[Merge] lp:~cjwatson/launchpad/codeimport-git-edit-views into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/codeimport-git-edit-views into lp:launchpad with lp:~cjwatson/launchpad/codeimport-list-git as a prerequisite.
Commit message:
Add GitRepository:+edit-import, GitRepository:+request-import, and GitRepository:+try-again views.
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-edit-views/+merge/308398
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/codeimport-git-edit-views into lp:launchpad.
=== modified file 'lib/lp/code/browser/codeimport.py'
--- lib/lp/code/browser/codeimport.py 2016-10-13 15:49:56 +0000
+++ lib/lp/code/browser/codeimport.py 2016-10-13 15:49:57 +0000
@@ -207,15 +207,16 @@
canonical_url(code_import.target),
code_import.target.unique_name))
- def _validateURL(self, url, rcs_type, existing_import=None,
- field_name='url'):
+ def _validateURL(self, url, rcs_type, target_rcs_type,
+ existing_import=None, field_name='url'):
"""If the user has specified a url, we need to make sure that there
isn't already an import with that url."""
if url is None:
self.setSecondaryFieldError(
field_name, 'Enter the URL of a foreign VCS branch.')
else:
- reason = validate_import_url(url, rcs_type, existing_import)
+ reason = validate_import_url(
+ url, rcs_type, target_rcs_type, existing_import)
if reason:
self.setFieldError(field_name, reason)
@@ -445,20 +446,22 @@
% 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:
self._validateCVS(data.get('cvs_root'), data.get('cvs_module'))
elif rcs_type == RevisionControlSystems.BZR_SVN:
self._validateURL(
- data.get('svn_branch_url'), rcs_type,
+ data.get('svn_branch_url'), rcs_type, target_rcs_type,
field_name='svn_branch_url')
elif rcs_type == RevisionControlSystems.GIT:
self._validateURL(
- data.get('git_repo_url'), rcs_type, field_name='git_repo_url')
+ data.get('git_repo_url'), rcs_type, target_rcs_type,
+ field_name='git_repo_url')
elif rcs_type == RevisionControlSystems.BZR:
self._validateURL(
- data.get('bzr_branch_url'), rcs_type,
+ data.get('bzr_branch_url'), rcs_type, target_rcs_type,
field_name='bzr_branch_url')
else:
raise AssertionError(
@@ -592,7 +595,8 @@
self.code_import)
elif self.code_import.rcs_type in NON_CVS_RCS_TYPES:
self._validateURL(
- data.get('url'), self.code_import.rcs_type, self.code_import)
+ data.get('url'), self.code_import.rcs_type,
+ self.code_import.target_rcs_type, self.code_import)
else:
raise AssertionError('Unknown rcs_type for code import.')
@@ -608,23 +612,25 @@
return getUtility(ICodeImportMachineSet).getAll()
-def validate_import_url(url, rcs_type, existing_import=None):
+def validate_import_url(url, rcs_type, target_rcs_type, existing_import=None):
"""Validate the given import URL."""
- # XXX cjwatson 2015-06-12: Once we have imports into Git, this should be
- # extended to prevent Git-to-Git self-imports as well.
- if (rcs_type == RevisionControlSystems.BZR and
+ if (rcs_type.name == target_rcs_type.name and
urlparse(url).netloc.endswith('launchpad.net')):
return (
- "You cannot create imports for Bazaar branches that are hosted by "
- "Launchpad.")
+ "You cannot create same-VCS imports for branches or repositories "
+ "that are hosted by Launchpad.")
code_import = getUtility(ICodeImportSet).getByURL(url)
if code_import is not None:
if existing_import and code_import == existing_import:
return None
+ if code_import.target_rcs_type == TargetRevisionControlSystems.BZR:
+ target_type = "branch"
+ else:
+ target_type = "repository"
return structured(
"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)
+ "%s <a href='%s'>%s</a>.", target_type,
+ canonical_url(code_import.target), code_import.target.unique_name)
class CodeImportTargetMixin:
=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml 2016-10-13 15:49:56 +0000
+++ lib/lp/code/browser/configure.zcml 2016-10-13 15:49:57 +0000
@@ -853,6 +853,12 @@
permission="launchpad.Edit"
name="+edit"
template="../templates/gitrepository-edit.pt"/>
+ <browser:page
+ for="lp.code.interfaces.gitrepository.IGitRepository"
+ class="lp.code.browser.codeimport.CodeImportEditView"
+ permission="launchpad.Edit"
+ name="+edit-import"
+ template="../../app/templates/generic-edit.pt"/>
<class class="lp.code.browser.gitrepository.GitRepositoryDiffView">
<allow interface="zope.publisher.interfaces.browser.IBrowserPublisher"
attributes="__call__"/>
@@ -881,6 +887,18 @@
permission="launchpad.AnyPerson"
name="+edit-subscription"
template="../templates/gitrepository-edit-subscription.pt"/>
+ <browser:page
+ for="lp.code.interfaces.gitrepository.IGitRepository"
+ class="lp.code.browser.gitrepository.GitRepositoryRequestImportView"
+ permission="launchpad.AnyPerson"
+ name="+request-import"
+ template="../templates/inline-form-only-buttons.pt"/>
+ <browser:page
+ for="lp.code.interfaces.gitrepository.IGitRepository"
+ class="lp.code.browser.gitrepository.GitRepositoryTryImportAgainView"
+ permission="launchpad.AnyPerson"
+ name="+try-again"
+ template="../templates/inline-form-only-buttons.pt"/>
<adapter
provides="lp.services.webapp.interfaces.IBreadcrumb"
for="lp.code.interfaces.gitrepository.IGitRepository"
=== modified file 'lib/lp/code/browser/gitrepository.py'
--- lib/lp/code/browser/gitrepository.py 2016-10-13 15:49:56 +0000
+++ lib/lp/code/browser/gitrepository.py 2016-10-13 15:49:57 +0000
@@ -26,7 +26,10 @@
copy_field,
use_template,
)
-from zope.component import getUtility
+from zope.component import (
+ getUtility,
+ queryAdapter,
+ )
from zope.event import notify
from zope.formlib import form
from zope.interface import (
@@ -40,6 +43,7 @@
SimpleTerm,
SimpleVocabulary,
)
+from zope.traversing.interfaces import IPathAdapter
from lp import _
from lp.app.browser.informationtype import InformationTypePortletMixin
@@ -59,8 +63,14 @@
GitRepositoryTargetDisplayWidget,
GitRepositoryTargetWidget,
)
-from lp.code.enums import GitRepositoryType
+from lp.code.enums import (
+ CodeImportReviewStatus,
+ GitRepositoryType,
+ )
from lp.code.errors import (
+ CodeImportAlreadyRequested,
+ CodeImportAlreadyRunning,
+ CodeImportNotInReviewedState,
GitDefaultConflict,
GitRepositoryCreationForbidden,
GitRepositoryExists,
@@ -235,8 +245,8 @@
usedfor = IGitRepository
facet = "branches"
links = [
- "add_subscriber", "create_recipe", "source", "subscription",
- "view_recipes", "visibility"]
+ "add_subscriber", "create_recipe", "edit_import", "source",
+ "subscription", "view_recipes", "visibility"]
@enabled_with_permission("launchpad.AnyPerson")
def subscription(self):
@@ -267,6 +277,13 @@
text = "Change information type"
return Link("+edit-information-type", text)
+ def edit_import(self):
+ text = "Edit import source or review import"
+ enabled = (
+ self.context.repository_type == GitRepositoryType.IMPORTED and
+ check_permission("launchpad.Edit", self.context.code_import))
+ return Link("+edit-import", text, icon="edit", enabled=enabled)
+
def create_recipe(self):
# You can't create a recipe for a private repository.
enabled = (
@@ -655,6 +672,83 @@
return self, ()
+class GitRepositoryRequestImportView(LaunchpadFormView):
+ """The view to provide an 'Import now' button on the repository index page.
+
+ This only appears on the page of a repository with an associated code
+ import that is being actively imported and where there is a import
+ scheduled at some point in the future.
+ """
+
+ schema = IGitRepository
+ field_names = []
+
+ form_style = "display: inline"
+
+ @property
+ def next_url(self):
+ return canonical_url(self.context)
+
+ @action("Import Now", name="request")
+ def request_import_action(self, action, data):
+ try:
+ self.context.code_import.requestImport(
+ self.user, error_if_already_requested=True)
+ self.request.response.addNotification(
+ "Import will run as soon as possible.")
+ except CodeImportNotInReviewedState:
+ self.request.response.addNotification(
+ "This import is no longer being updated automatically.")
+ except CodeImportAlreadyRunning:
+ self.request.response.addNotification(
+ "The import is already running.")
+ except CodeImportAlreadyRequested as e:
+ user = e.requesting_user
+ adapter = queryAdapter(user, IPathAdapter, 'fmt')
+ self.request.response.addNotification(
+ structured("The import has already been requested by %s." %
+ adapter.link(None)))
+
+ @property
+ def prefix(self):
+ return "request%s" % self.context.id
+
+ @property
+ def action_url(self):
+ return "%s/@@+request-import" % canonical_url(self.context)
+
+
+class GitRepositoryTryImportAgainView(LaunchpadFormView):
+ """The view to provide an 'Try again' button on the repository index page.
+
+ This only appears on the page of a repository with an associated code
+ import that is marked as failing.
+ """
+
+ schema = IGitRepository
+ field_names = []
+
+ @property
+ def next_url(self):
+ return canonical_url(self.context)
+
+ @action("Try Again", name="tryagain")
+ def request_try_again(self, action, data):
+ if (self.context.code_import.review_status !=
+ CodeImportReviewStatus.FAILING):
+ self.request.response.addNotification(
+ "The import is now %s."
+ % self.context.code_import.review_status.name)
+ else:
+ self.context.code_import.tryFailingImportAgain(self.user)
+ self.request.response.addNotification(
+ "Import will be tried again as soon as possible.")
+
+ @property
+ def prefix(self):
+ return "tryagain"
+
+
class GitRepositoryDeletionView(LaunchpadFormView):
schema = IGitRepository
=== modified file 'lib/lp/code/javascript/tests/test_util.js'
--- lib/lp/code/javascript/tests/test_util.js 2014-07-24 08:02:19 +0000
+++ lib/lp/code/javascript/tests/test_util.js 2016-10-13 15:49:57 +0000
@@ -64,9 +64,9 @@
Y.one('#filter_form_submit').hasClass('hidden'));
},
- test_hookUpRetyImportSubmission: function() {
+ test_hookUpRetryImportSubmission: function() {
this._setup_fixture('#retry-import-form');
- module.hookUpRetyImportSubmission();
+ module.hookUpRetryImportSubmission();
this._add_submit_listener('#tryagain');
var try_again_link = Y.one("#tryagainlink");
try_again_link.simulate('click');
=== modified file 'lib/lp/code/javascript/util.js'
--- lib/lp/code/javascript/util.js 2014-12-06 02:10:46 +0000
+++ lib/lp/code/javascript/util.js 2016-10-13 15:49:57 +0000
@@ -53,7 +53,7 @@
Y.one('#filter_form_submit').addClass('hidden');
};
-var hookUpRetyImportSubmission = function() {
+var hookUpRetryImportSubmission = function() {
var try_again_link = Y.one("#tryagainlink");
try_again_link.on('click', function (e) {
Y.one('#tryagain').submit();
@@ -65,7 +65,7 @@
ns.hookUpBranchFieldFunctions = hookUpBranchFieldFunctions;
ns.hookUpBranchFilterSubmission = hookUpBranchFilterSubmission;
ns.hookUpMergeProposalFilterSubmission = hookUpMergeProposalFilterSubmission;
-ns.hookUpRetyImportSubmission = hookUpRetyImportSubmission;
+ns.hookUpRetryImportSubmission = hookUpRetryImportSubmission;
}, "0.1", {"requires": ["node", "dom"]});
=== modified file 'lib/lp/code/stories/codeimport/xx-admin-codeimport.txt'
--- lib/lp/code/stories/codeimport/xx-admin-codeimport.txt 2013-09-27 04:13:23 +0000
+++ lib/lp/code/stories/codeimport/xx-admin-codeimport.txt 2016-10-13 15:49:57 +0000
@@ -6,7 +6,11 @@
(member of VCS imports or Launchpad admin) then they can see a link
to edit the details.
- >>> from lp.code.enums import RevisionControlSystems
+ >>> from lp.code.enums import (
+ ... RevisionControlSystems,
+ ... TargetRevisionControlSystems,
+ ... )
+ >>> from lp.code.tests.helpers import GitHostingFixture
>>> from lp.testing import ANONYMOUS, login, logout
>>> login('test@xxxxxxxxxxxxx')
@@ -33,6 +37,15 @@
>>> git_import_location = str(canonical_url(git_import.branch))
>>> git_import_branch_unique_name = git_import.branch.unique_name
+ >>> with GitHostingFixture():
+ ... git_to_git_import = factory.makeProductCodeImport(
+ ... git_repo_url="git://git.example.org/fooix2",
+ ... target_rcs_type=TargetRevisionControlSystems.GIT)
+ >>> git_to_git_import_location = str(
+ ... canonical_url(git_to_git_import.git_repository))
+ >>> git_to_git_repository_unique_name = (
+ ... git_to_git_import.git_repository.unique_name)
+
>>> package_import = factory.makePackageCodeImport(
... git_repo_url="http://git.example.org/zap";)
>>> package_import_location = str(canonical_url(package_import.branch))
@@ -51,9 +64,13 @@
import:
>>> def print_import_details(browser):
- ... div = find_tag_by_id(
- ... browser.contents, 'branch-import-details').div.div
- ... print extract_text(div)
+ ... details = find_tag_by_id(
+ ... browser.contents, 'branch-import-details')
+ ... if details is None:
+ ... details = find_tag_by_id(
+ ... browser.contents, 'repository-import-details')
+ ... print extract_text(details.div.div)
+
>>> import_browser.open(svn_import_location)
>>> print_import_details(import_browser)
Import Status: Reviewed
@@ -63,6 +80,15 @@
as soon as possible.
Edit import source or review import
+ >>> import_browser.open(git_to_git_import_location)
+ >>> print_import_details(import_browser)
+ Import Status: Reviewed
+ This repository is an import of the Git repository
+ at git://git.example.org/fooix2.
+ The next import is scheduled to run
+ as soon as possible.
+ Edit import source or review import
+
Editing details
---------------
@@ -70,6 +96,7 @@
There are a number of buttons shown on the editing page for
import operators.
+ >>> import_browser.open(svn_import_location)
>>> import_browser.getLink('Edit import source or review import').click()
>>> print_submit_buttons(import_browser.contents)
Update
@@ -105,6 +132,11 @@
>>> print_form_fields(import_browser)
field.url: git://git.example.org/fooix
+ >>> import_browser.open(git_to_git_import_location)
+ >>> import_browser.getLink('Edit import source or review import').click()
+ >>> print_form_fields(import_browser)
+ field.url: git://git.example.org/fooix2
+
>>> import_browser.open(package_import_location)
>>> import_browser.getLink('Edit import source or review import').click()
>>> print_form_fields(import_browser)
@@ -147,7 +179,7 @@
>>> print_feedback_messages(import_browser.contents)
The code import has been updated.
-Git imports,
+Git-to-Bazaar imports,
>>> import_browser.open(git_import_location + '/+edit-import')
>>> import_browser.getControl('URL').value = \
@@ -156,6 +188,15 @@
>>> print_feedback_messages(import_browser.contents)
The code import has been updated.
+Git-to-Git imports,
+
+ >>> import_browser.open(git_to_git_import_location + '/+edit-import')
+ >>> import_browser.getControl('URL').value = \
+ ... 'git://user:password@xxxxxxxxxxxxxxxxxxx/fooix2'
+ >>> import_browser.getControl('Update').click()
+ >>> print_feedback_messages(import_browser.contents)
+ The code import has been updated.
+
and imports targetting source packages.
>>> import_browser.open(package_import_location + '/+edit-import')
=== modified file 'lib/lp/code/stories/codeimport/xx-create-codeimport.txt'
--- lib/lp/code/stories/codeimport/xx-create-codeimport.txt 2015-06-15 08:35:10 +0000
+++ lib/lp/code/stories/codeimport/xx-create-codeimport.txt 2016-10-13 15:49:57 +0000
@@ -109,8 +109,8 @@
>>> browser.getControl('Request Import').click()
>>> print_feedback_messages(browser.contents)
There is 1 error.
- You cannot create imports for Bazaar branches that are hosted by
- Launchpad.
+ You cannot create same-VCS imports for branches or repositories that are
+ hosted by Launchpad.
But a Launchpad Git URL is OK.
=== modified file 'lib/lp/code/templates/branch-import-details.pt'
--- lib/lp/code/templates/branch-import-details.pt 2014-02-24 07:19:52 +0000
+++ lib/lp/code/templates/branch-import-details.pt 2016-10-13 15:49:57 +0000
@@ -32,7 +32,7 @@
<script type="text/javascript">
LPJS.use('lp.code.util', function(Y) {
Y.on("domready", function () {
- Y.lp.code.util.hookUpRetyImportSubmission(Y);
+ Y.lp.code.util.hookUpRetryImportSubmission(Y);
}, window);});
</script>
</form>
=== modified file 'lib/lp/code/templates/gitrepository-import-details.pt'
--- lib/lp/code/templates/gitrepository-import-details.pt 2016-10-13 15:49:56 +0000
+++ lib/lp/code/templates/gitrepository-import-details.pt 2016-10-13 15:49:57 +0000
@@ -14,6 +14,29 @@
<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'"
+ style="display: inline; padding-left: 1em"
+ id="tryagain"
+ name="tryagain"
+ method="post"
+ enctype="multipart/form-data" accept-charset="UTF-8">
+ <input
+ type="hidden"
+ name="tryagain.actions.tryagain"
+ value="" />
+ <input type="submit" id="tryagain.actions.tryagain"
+ name="tryagain.actions.tryagain" value="Try Again"
+ class="button" />
+ <a href="#" class="hidden sprite retry"
+ id="tryagainlink">Try again</a>
+ <script type="text/javascript">
+ LPJS.use('lp.code.util', function(Y) {
+ Y.on("domready", function () {
+ Y.lp.code.util.hookUpRetryImportSubmission(Y);
+ }, window);});
+ </script>
+ </form>
</div>
<tal:git-import condition="code_import/rcs_type/enumvalue:GIT">
@@ -61,6 +84,9 @@
<tal:date-started replace="job/date_due/fmt:displaydate">
in 2 hours
</tal:date-started>.
+ <tal:button
+ condition="view/user"
+ replace="structure view/context/@@+request-import" />
</tal:not-overdue>
</tal:not-running>
</div>
@@ -88,6 +114,18 @@
<metal:result use-macro="code_import/@@+macros/show_result"/>
</tal:result>
</div>
+
+ <div class="actions">
+ <div
+ tal:define="link context_menu/edit_import"
+ tal:condition="link/enabled"
+ >
+ <a id="edit-import"
+ class="sprite add"
+ tal:attributes="href link/url"
+ tal:content="link/text" />
+ </div>
+ </div>
</tal:has-codeimport>
</div>
</tal:imported-repository>
=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py 2016-10-03 17:00:56 +0000
+++ lib/lp/registry/browser/product.py 2016-10-13 15:49:57 +0000
@@ -1847,7 +1847,8 @@
self.setFieldError(
'repo_url', 'You must set the external repository URL.')
else:
- reason = validate_import_url(repo_url, rcs_type)
+ reason = validate_import_url(
+ repo_url, rcs_type, TargetRevisionControlSystems.BZR)
if reason:
self.setFieldError('repo_url', reason)
Follow ups
