launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #25703
[Merge] ~twom/launchpad:git-branch-picker-unpicked-with-merge-pickers into launchpad:master
Tom Wardill has proposed merging ~twom/launchpad:git-branch-picker-unpicked-with-merge-pickers into launchpad:master with ~twom/launchpad:git-branch-picker-unpicked as a prerequisite.
Commit message:
Add autocomplete branch picker to git merge proposal page
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~twom/launchpad/+git/launchpad/+merge/394183
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~twom/launchpad:git-branch-picker-unpicked-with-merge-pickers into launchpad:master.
diff --git a/lib/lp/code/browser/gitref.py b/lib/lp/code/browser/gitref.py
index f17207e..d4aaff4 100644
--- a/lib/lp/code/browser/gitref.py
+++ b/lib/lp/code/browser/gitref.py
@@ -27,9 +27,7 @@ from zope.interface import Interface
from zope.publisher.interfaces import NotFound
from zope.schema import (
Bool,
- Choice,
Text,
- TextLine,
)
from lp import _
@@ -37,11 +35,11 @@ from lp.app.browser.launchpadform import (
action,
LaunchpadFormView,
)
-from lp.app.widgets.suggestion import TargetGitRepositoryWidget
from lp.code.browser.branchmergeproposal import (
latest_proposals_for_each_branch,
)
from lp.code.browser.sourcepackagerecipelisting import HasRecipesMenuMixin
+from lp.code.browser.widgets.gitref import GitRefWidget
from lp.code.enums import GitRepositoryType
from lp.code.errors import InvalidBranchMergeProposal
from lp.code.interfaces.branchmergeproposal import IBranchMergeProposal
@@ -247,30 +245,11 @@ class GitRefView(LaunchpadView, HasSnapsViewMixin):
class GitRefRegisterMergeProposalSchema(Interface):
"""The schema to define the form for registering a new merge proposal."""
- target_git_repository = Choice(
- title=_("Target repository"),
- vocabulary="GitRepository", required=True, readonly=True,
- description=_("The repository that the source will be merged into."))
+ target_git_ref = copy_field(
+ IBranchMergeProposal['target_git_ref'], required=True)
- target_git_path = TextLine(
- title=_("Target branch"), required=True, readonly=True,
- description=_(
- "The branch within the target repository that the source will "
- "be merged into."))
-
- prerequisite_git_repository = Choice(
- title=_("Prerequisite repository"),
- vocabulary="GitRepository", required=False, readonly=True,
- description=_(
- "A repository containing a branch that should be merged before "
- "this one. (Its changes will not be shown in the diff.)"))
-
- prerequisite_git_path = TextLine(
- title=_("Prerequisite branch"), required=False, readonly=True,
- description=_(
- "A branch within the prerequisite repository that should be "
- "merged before this one. (Its changes will not be shown in the "
- "diff.)"))
+ prerequisite_git_ref = copy_field(
+ IBranchMergeProposal['prerequisite_git_ref'], required=False)
comment = Text(
title=_('Description of the change'), required=False,
@@ -302,7 +281,10 @@ class GitRefRegisterMergeProposalView(LaunchpadFormView):
schema = GitRefRegisterMergeProposalSchema
for_input = True
- custom_widget_target_git_repository = TargetGitRepositoryWidget
+ custom_widget_target_git_ref = CustomWidgetFactory(
+ GitRefWidget, require_branch=True)
+ custom_widget_prerequisite_git_ref = CustomWidgetFactory(
+ GitRefWidget, require_branch=True)
custom_widget_commit_message = CustomWidgetFactory(
TextAreaWidget, cssClass='comment-text')
custom_widget_comment = CustomWidgetFactory(
@@ -323,15 +305,27 @@ class GitRefRegisterMergeProposalView(LaunchpadFormView):
def setUpWidgets(self, context=None):
super(GitRefRegisterMergeProposalView, self).setUpWidgets(
context=context)
- term = next(
- iter(self.widgets['target_git_repository'].vocabulary),
- None)
- # If we have a target, and the user hasn't entered a value.
- if term and not self.widgets['target_git_path'].hasInput():
- branch_display = term.value.default_branch
- if branch_display and branch_display.startswith("refs/heads/"):
- branch_display = branch_display[len("refs/heads/"):]
- self.widgets['target_git_path'].setRenderedValue(branch_display)
+
+ if not self.widgets['target_git_ref'].hasInput():
+ if IPerson.providedBy(self.context.repository.target):
+ default_ref = self.context.repository.getRefByPath(
+ self.context.repository.default_branch)
+ with_path = False
+ else:
+ repo_set = getUtility(IGitRepositorySet)
+ default_repo = repo_set.getDefaultRepository(
+ self.context.repository.target)
+ if not default_repo:
+ default_repo = self.context.repository
+ if default_repo.default_branch:
+ default_ref = default_repo.getRefByPath(
+ default_repo.default_branch)
+ with_path = True
+ else:
+ default_ref = self.context
+ with_path = False
+ self.widgets["target_git_ref"].setRenderedValue(
+ default_ref, with_path=with_path)
@action('Propose Merge', name='register',
failure=LaunchpadFormView.ajax_failure_handler)
@@ -340,15 +334,8 @@ class GitRefRegisterMergeProposalView(LaunchpadFormView):
registrant = self.user
source_ref = self.context
- target_ref = data['target_git_repository'].getRefByPath(
- data['target_git_path'])
- if (data.get('prerequisite_git_repository') is not None and
- data.get('prerequisite_git_path') is not None):
- prerequisite_ref = (
- data['prerequisite_git_repository'].getRefByPath(
- data['prerequisite_git_path']))
- else:
- prerequisite_ref = None
+ target_ref = data['target_git_ref']
+ prerequisite_ref = data.get('prerequisite_git_ref')
review_requests = []
reviewer = data.get('reviewer')
@@ -403,41 +390,31 @@ class GitRefRegisterMergeProposalView(LaunchpadFormView):
self.addError(str(error))
def _validateRef(self, data, name):
- repository = data.get('%s_git_repository' % name)
- path = data.get('%s_git_path' % name)
- if path:
- ref = repository.getRefByPath(path)
- else:
- ref = None
- if ref is None:
- self.setFieldError(
- '%s_git_path' % name,
- "The %s path must be the path of a reference in the "
- "%s repository." % (name, name))
- elif ref == self.context:
+ ref = data['{}_git_ref'.format(name)]
+ if ref == self.context:
self.setFieldError(
- '%s_git_path' % name,
+ '%s_git_ref' % name,
"The %s repository and path together cannot be the same "
"as the source repository and path." % name)
- return repository
+ return ref.repository
def validate(self, data):
source_ref = self.context
# The existence of target_git_repository is handled by the form
# machinery.
- if data.get('target_git_repository') is not None:
+ if data.get('target_git_ref') is not None:
target_repository = self._validateRef(data, 'target')
if not target_repository.isRepositoryMergeable(
source_ref.repository):
self.setFieldError(
- 'target_git_repository',
+ 'target_git_ref',
"%s is not mergeable into this repository." %
source_ref.repository.identity)
- if data.get('prerequisite_git_repository') is not None:
+ if data.get('prerequisite_git_ref') is not None:
prerequisite_repository = self._validateRef(data, 'prerequisite')
if not target_repository.isRepositoryMergeable(
prerequisite_repository):
self.setFieldError(
- 'prerequisite_git_repository',
+ 'prerequisite_git_ref',
"This repository is not mergeable into %s." %
target_repository.identity)
diff --git a/lib/lp/code/browser/tests/test_branchmergeproposal.py b/lib/lp/code/browser/tests/test_branchmergeproposal.py
index a9ac39b..fedb00d 100644
--- a/lib/lp/code/browser/tests/test_branchmergeproposal.py
+++ b/lib/lp/code/browser/tests/test_branchmergeproposal.py
@@ -821,6 +821,7 @@ class TestRegisterBranchMergeProposalViewBzr(
target_branch.unique_name},
**extra)
request.setPrincipal(owner)
+ transaction.commit()
view = create_initialized_view(
target_branch,
name='+register-merge',
@@ -858,10 +859,7 @@ class TestRegisterBranchMergeProposalViewGit(
@staticmethod
def _getFormValues(target_branch, extras):
- values = {
- 'target_git_repository': target_branch.repository,
- 'target_git_path': target_branch.path,
- }
+ values = {'target_git_ref': target_branch}
values.update(extras)
return values
@@ -873,17 +871,19 @@ class TestRegisterBranchMergeProposalViewGit(
view = self._createView()
self.assertEqual(
target_branch.repository.default_branch.split('/')[-1],
- view.widgets['target_git_path']._getCurrentValue())
+ view.widgets['target_git_ref'].path_widget._getCurrentValue())
def test_default_branch_no_default_set(self):
with admin_logged_in():
self._makeTargetBranch(target_default=True)
view = self._createView()
- self.assertIsNone(view.widgets['target_git_path']._getCurrentValue())
+ self.assertIsNone(
+ view.widgets['target_git_ref'].path_widget._getCurrentValue())
def test_default_branch_no_target(self):
view = self._createView()
- self.assertIsNone(view.widgets['target_git_path']._getCurrentValue())
+ self.assertIsNone(
+ view.widgets['target_git_ref'].path_widget._getCurrentValue())
def test_register_ajax_request_with_confirmation(self):
# Ajax submits return json data containing info about what the visible
@@ -929,9 +929,9 @@ class TestRegisterBranchMergeProposalViewGit(
method='POST',
form={
'field.actions.register': 'Propose Merge',
- 'field.target_git_repository.target_git_repository':
+ 'field.target_git_ref.repository':
target_branch.repository.unique_name,
- 'field.target_git_path': target_branch.path,
+ 'field.target_git_ref.path': target_branch.path,
},
**extra)
request.setPrincipal(owner)
@@ -944,7 +944,7 @@ class TestRegisterBranchMergeProposalViewGit(
self.assertEqual(
{'error_summary': 'There is 1 error.',
'errors': {
- 'field.target_git_path':
+ 'field.target_git_ref':
('The target repository and path together cannot be the '
'same as the source repository and path.')},
'form_wide_errors': []},
@@ -959,9 +959,9 @@ class TestRegisterBranchMergeProposalViewGit(
method='POST',
form={
'field.actions.register': 'Propose Merge',
- 'field.target_git_repository.target_git_repository': '',
- 'field.target_git_repository-empty-marker': '1',
- 'field.target_git_path': 'master',
+ 'field.target_git_ref.repository': '',
+ 'field.target_git_ref.repository-empty-marker': '1',
+ 'field.target_git_ref.path': 'master',
},
**extra)
request.setPrincipal(owner)
@@ -974,7 +974,7 @@ class TestRegisterBranchMergeProposalViewGit(
self.assertEqual(
{'error_summary': 'There is 1 error.',
'errors': {
- 'field.target_git_repository': 'Required input is missing.',
+ 'field.target_git_ref': 'Required input is missing.',
},
'form_wide_errors': []},
simplejson.loads(view.form_result))
@@ -984,13 +984,14 @@ class TestRegisterBranchMergeProposalViewGit(
owner = self.factory.makePerson()
target_branch = self._makeTargetBranch(
owner=owner, information_type=InformationType.USERDATA)
+ transaction.commit()
extra = {'HTTP_X_REQUESTED_WITH': 'XMLHttpRequest'}
with person_logged_in(owner):
request = LaunchpadTestRequest(
method='POST',
form={
'field.actions.register': 'Propose Merge',
- 'field.target_git_repository.target_git_repository':
+ 'field.target_git_ref.repository':
target_branch.repository.unique_name,
},
**extra)
@@ -1004,9 +1005,8 @@ class TestRegisterBranchMergeProposalViewGit(
self.assertEqual(
{'error_summary': 'There is 1 error.',
'errors': {
- 'field.target_git_path':
- ('The target path must be the path of a reference in the '
- 'target repository.')},
+ 'field.target_git_ref':
+ 'Please enter a Git branch path.'},
'form_wide_errors': []},
simplejson.loads(view.form_result))
@@ -1018,16 +1018,17 @@ class TestRegisterBranchMergeProposalViewGit(
owner=owner, information_type=InformationType.USERDATA)
prerequisite_branch = self._makeTargetBranch(
owner=owner, information_type=InformationType.USERDATA)
+ transaction.commit()
extra = {'HTTP_X_REQUESTED_WITH': 'XMLHttpRequest'}
with person_logged_in(owner):
request = LaunchpadTestRequest(
method='POST',
form={
'field.actions.register': 'Propose Merge',
- 'field.target_git_repository.target_git_repository':
+ 'field.target_git_ref.repository':
target_branch.repository.unique_name,
- 'field.target_git_path': target_branch.path,
- 'field.prerequisite_git_repository':
+ 'field.target_git_ref.path': target_branch.path,
+ 'field.prerequisite_git_ref.repository':
prerequisite_branch.repository.unique_name,
},
**extra)
@@ -1041,9 +1042,8 @@ class TestRegisterBranchMergeProposalViewGit(
self.assertEqual(
{'error_summary': 'There is 1 error.',
'errors': {
- 'field.prerequisite_git_path':
- ('The prerequisite path must be the path of a reference '
- 'in the prerequisite repository.')},
+ 'field.prerequisite_git_ref':
+ 'Please enter a Git branch path.'},
'form_wide_errors': []},
simplejson.loads(view.form_result))
diff --git a/lib/lp/code/templates/gitref-register-merge.pt b/lib/lp/code/templates/gitref-register-merge.pt
index 91a06b9..6dd4d3f 100644
--- a/lib/lp/code/templates/gitref-register-merge.pt
+++ b/lib/lp/code/templates/gitref-register-merge.pt
@@ -12,11 +12,11 @@
<metal:formbody fill-slot="widgets">
<table class="form">
- <tal:widget define="widget nocall:view/widgets/target_git_repository">
+ <tal:widget define="widget nocall:view/widgets/target_git_ref">
<metal:block use-macro="context/@@launchpad_form/widget_row" />
</tal:widget>
- <tal:widget define="widget nocall:view/widgets/target_git_path">
+ <tal:widget define="widget nocall:view/widgets/prerequisite_git_ref">
<metal:block use-macro="context/@@launchpad_form/widget_row" />
</tal:widget>
@@ -40,14 +40,6 @@
<metal:block use-macro="context/@@launchpad_form/widget_row" />
</tal:widget>
- <tal:widget define="widget nocall:view/widgets/prerequisite_git_repository">
- <metal:block use-macro="context/@@launchpad_form/widget_row" />
- </tal:widget>
-
- <tal:widget define="widget nocall:view/widgets/prerequisite_git_path">
- <metal:block use-macro="context/@@launchpad_form/widget_row" />
- </tal:widget>
-
</table>
</metal:formbody>
</div>
Follow ups
