launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #18403
[Merge] lp:~cjwatson/launchpad/git-mp-register into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/git-mp-register into lp:launchpad with lp:~cjwatson/launchpad/git-mp-basic-browser as a prerequisite.
Commit message:
Add a basic GitRef:+register-merge view.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1445017 in Launchpad itself: "Support for Launchpad Git merge proposals "
https://bugs.launchpad.net/launchpad/+bug/1445017
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-mp-register/+merge/257676
Add a basic GitRef:+register-merge view.
The repository/path split in the UI is pretty gross, and some JavaScript doesn't work yet, but it should do the job until we have time to improve it.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-mp-register into lp:launchpad.
=== modified file 'lib/lp/app/widgets/suggestion.py'
--- lib/lp/app/widgets/suggestion.py 2013-04-10 08:05:17 +0000
+++ lib/lp/app/widgets/suggestion.py 2015-04-28 16:51:38 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Widgets related to IBranch."""
@@ -7,6 +7,7 @@
__all__ = [
'RecipeOwnerWidget',
'TargetBranchWidget',
+ 'TargetGitRepositoryWidget',
]
@@ -32,6 +33,8 @@
)
from lp.app.widgets.itemswidgets import LaunchpadRadioWidget
+from lp.code.interfaces.gitcollection import IGitCollection
+from lp.code.interfaces.gitrepository import IGitRepositorySet
from lp.services.webapp import canonical_url
from lp.services.webapp.escaping import (
html_escape,
@@ -275,6 +278,77 @@
self.other_selection_widget.onKeyPress = on_key_press
+class TargetGitRepositoryWidget(SuggestionWidget):
+ """Widget for selecting a target Git repository.
+
+ The default repository for a new Git merge proposal should be the
+ default repository for the target if there is one.
+
+ Also in the initial radio button selector are other repositories for the
+ target that the repository owner has specified as target repositories
+ for other merge proposals.
+
+ Finally there is an "other" button that gets the user to use the normal
+ branch selector.
+ """
+
+ @staticmethod
+ def _generateSuggestionVocab(repository, full_vocabulary):
+ """Generate the vocabulary for the radio buttons.
+
+ The generated vocabulary contains the default repository for the
+ target if there is one, and also any other repositories that the
+ user has specified recently as a target for a proposed merge.
+ """
+ default_target = getUtility(IGitRepositorySet).getDefaultRepository(
+ repository.target)
+ logged_in_user = getUtility(ILaunchBag).user
+ since = datetime.now(utc) - timedelta(days=90)
+ collection = IGitCollection(repository.target).targetedBy(
+ logged_in_user, since)
+ collection = collection.visibleByUser(logged_in_user)
+ # May actually need some eager loading, but the API isn't fine grained
+ # yet.
+ repositories = collection.getRepositories(eager_load=False).config(
+ distinct=True)
+ target_repositories = list(repositories.config(limit=5))
+ # If there is a default repository, make sure it is always shown,
+ # and as the first item.
+ if default_target is not None:
+ if default_target in target_repositories:
+ target_repositories.remove(default_target)
+ target_repositories.insert(0, default_target)
+
+ terms = []
+ for repository in target_repositories:
+ terms.append(SimpleTerm(repository, repository.unique_name))
+
+ return SimpleVocabulary(terms)
+
+ def _renderSuggestionLabel(self, repository, index):
+ """Render a label for the option based on a repository."""
+ # To aid usability there needs to be some text connected with the
+ # radio buttons that is not a hyperlink in order to select the radio
+ # button. It was decided not to have the entire text as a link, but
+ # instead to have a separate link to the repository details.
+ text = '%s (<a href="%s">repository details</a>)' % (
+ html_escape(repository.display_name),
+ html_escape(canonical_url(repository)))
+ # If the repository is the default for the target, say so.
+ default_target = getUtility(IGitRepositorySet).getDefaultRepository(
+ repository.target)
+ if repository == default_target:
+ text += "– <em>default repository</em>"
+ label = u'<label for="%s" style="font-weight: normal">%s</label>' % (
+ self._optionId(index), text)
+ return structured(label)
+
+ def _autoselectOther(self):
+ """Select "other" on keypress."""
+ on_key_press = "selectWidget('%s', event);" % self._otherId()
+ self.other_selection_widget.onKeyPress = on_key_press
+
+
class RecipeOwnerWidget(SuggestionWidget):
"""Widget for selecting a recipe owner.
=== modified file 'lib/lp/app/widgets/tests/test_suggestion.py'
--- lib/lp/app/widgets/tests/test_suggestion.py 2015-04-21 14:32:31 +0000
+++ lib/lp/app/widgets/tests/test_suggestion.py 2015-04-28 16:51:38 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011 Canonical Ltd. This software is licensed under the
+# Copyright 2011-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -24,7 +24,10 @@
from lp.app.widgets.suggestion import (
SuggestionWidget,
TargetBranchWidget,
+ TargetGitRepositoryWidget,
)
+from lp.code.interfaces.gitrepository import GIT_FEATURE_FLAG
+from lp.services.features.testing import FeatureFixture
from lp.services.webapp.servers import LaunchpadTestRequest
from lp.services.webapp.vocabulary import (
FilteredVocabularyBase,
@@ -173,3 +176,43 @@
timedelta(days=91))
widget = make_target_branch_widget(source)
self.assertNotIn(target, widget.suggestion_vocab)
+
+
+def make_target_git_repository_widget(repository):
+ """Given a Git repository, return a widget for selecting where to land
+ it."""
+ choice = Choice(vocabulary='GitRepository').bind(repository)
+ request = LaunchpadTestRequest()
+ return TargetGitRepositoryWidget(choice, None, request)
+
+
+class TestTargetGitRepositoryWidget(TestCaseWithFactory):
+ """Test the TargetGitRepositoryWidget class."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestTargetGitRepositoryWidget, self).setUp()
+ self.useFixture(FeatureFixture({GIT_FEATURE_FLAG: u"on"}))
+
+ def makeRepositoryAndOldMergeProposal(self, timedelta):
+ """Make an old merge proposal and a repository with the same target."""
+ bmp = self.factory.makeBranchMergeProposalForGit(
+ date_created=datetime.now(utc) - timedelta)
+ login_person(bmp.registrant)
+ target = bmp.target_git_repository
+ return target, self.factory.makeGitRepository(target=target.target)
+
+ def test_recent_target(self):
+ """Targets for proposals newer than 90 days are included."""
+ target, source = self.makeRepositoryAndOldMergeProposal(
+ timedelta(days=89))
+ widget = make_target_git_repository_widget(source)
+ self.assertIn(target, widget.suggestion_vocab)
+
+ def test_stale_target(self):
+ """Targets for proposals older than 90 days are not considered."""
+ target, source = self.makeRepositoryAndOldMergeProposal(
+ timedelta(days=91))
+ widget = make_target_git_repository_widget(source)
+ self.assertNotIn(target, widget.suggestion_vocab)
=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml 2015-04-28 16:51:38 +0000
+++ lib/lp/code/browser/configure.zcml 2015-04-28 16:51:38 +0000
@@ -838,6 +838,15 @@
permission="zope.Public"
name="+ref-listing"
template="../templates/gitref-listing.pt"/>
+ <browser:page
+ for="lp.code.interfaces.gitref.IGitRef"
+ class="lp.code.browser.gitref.GitRefRegisterMergeProposalView"
+ name="+register-merge"
+ permission="launchpad.AnyPerson"
+ template="../templates/gitref-register-merge.pt"/>
+ <browser:menus
+ classes="GitRefContextMenu"
+ module="lp.code.browser.gitref"/>
<browser:defaultView
for="lp.code.interfaces.gitsubscription.IGitSubscription"
=== modified file 'lib/lp/code/browser/gitref.py'
--- lib/lp/code/browser/gitref.py 2015-04-28 16:51:38 +0000
+++ lib/lp/code/browser/gitref.py 2015-04-28 16:51:38 +0000
@@ -6,19 +6,50 @@
__metaclass__ = type
__all__ = [
+ 'GitRefContextMenu',
'GitRefNavigation',
+ 'GitRefRegisterMergeProposalView',
'GitRefView',
]
+import json
+
+from lazr.restful.interface import copy_field
+from zope.component import getUtility
+from zope.formlib.widgets import TextAreaWidget
+from zope.interface import Interface
+from zope.publisher.interfaces import NotFound
+from zope.schema import (
+ Bool,
+ Choice,
+ Text,
+ TextLine,
+ )
+
+from lp import _
+from lp.app.browser.launchpadform import (
+ action,
+ custom_widget,
+ LaunchpadFormView,
+ )
+from lp.app.widgets.suggestion import TargetGitRepositoryWidget
from lp.code.browser.branchmergeproposal import (
latest_proposals_for_each_branch,
)
+from lp.code.errors import InvalidBranchMergeProposal
+from lp.code.interfaces.branchmergeproposal import IBranchMergeProposal
+from lp.code.interfaces.codereviewvote import ICodeReviewVoteReference
from lp.code.interfaces.gitref import IGitRef
+from lp.code.interfaces.gitrepository import IGitRepositorySet
from lp.code.model.gitrepository import GitRepository
from lp.services.database.bulk import load_related
+from lp.services.helpers import english_list
from lp.services.propertycache import cachedproperty
from lp.services.webapp import (
+ canonical_url,
+ ContextMenu,
LaunchpadView,
+ Link,
Navigation,
stepthrough,
)
@@ -42,6 +73,19 @@
return proposal
+class GitRefContextMenu(ContextMenu):
+ """Context menu for Git references."""
+
+ usedfor = IGitRef
+ facet = 'branches'
+ links = ['register_merge']
+
+ def register_merge(self):
+ text = 'Propose for merging'
+ enabled = self.context.namespace.supports_merge_proposals
+ return Link('+register-merge', text, icon='add', enabled=enabled)
+
+
class GitRefView(LaunchpadView):
@property
@@ -110,3 +154,177 @@
@cachedproperty
def dependent_landing_count_text(self):
return self._getBranchCountText(len(self.dependent_landings))
+
+
+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_path = TextLine(
+ title=_("Target reference path"), required=True, readonly=True,
+ description=_(
+ "The reference 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=_("The repository that the source will be merged into."))
+
+ prerequisite_git_path = TextLine(
+ title=_("Prerequisite reference path"), required=False, readonly=True,
+ description=_(
+ "The reference within the target repository that the source will "
+ "be merged into."))
+
+ comment = Text(
+ title=_('Description of the Change'), required=False,
+ description=_('Describe what changes your branch introduces, '
+ 'what bugs it fixes, or what features it implements. '
+ 'Ideally include rationale and how to test.'))
+
+ reviewer = copy_field(
+ ICodeReviewVoteReference['reviewer'], required=False)
+
+ review_type = copy_field(
+ ICodeReviewVoteReference['review_type'],
+ description=u'Lowercase keywords describing the type of review you '
+ 'would like to be performed.')
+
+ commit_message = IBranchMergeProposal['commit_message']
+
+ needs_review = Bool(
+ title=_("Needs review"), required=True, default=True,
+ description=_(
+ "Is the proposal ready for review now?"))
+
+
+class GitRefRegisterMergeProposalView(LaunchpadFormView):
+ """The view to register new Git merge proposals."""
+
+ schema = GitRefRegisterMergeProposalSchema
+ for_input = True
+
+ custom_widget('target_git_repository', TargetGitRepositoryWidget)
+ custom_widget('comment', TextAreaWidget, cssClass='comment-text')
+
+ page_title = label = 'Propose for merging'
+
+ @property
+ def cancel_url(self):
+ return canonical_url(self.context)
+
+ def initialize(self):
+ """Show a 404 if the repository namespace doesn't support proposals."""
+ if not self.context.namespace.supports_merge_proposals:
+ raise NotFound(self.context, '+register-merge')
+ super(GitRefRegisterMergeProposalView, self).initialize()
+
+ @action('Propose Merge', name='register',
+ failure=LaunchpadFormView.ajax_failure_handler)
+ def register_action(self, action, data):
+ """Register the new merge proposal."""
+
+ 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
+
+ review_requests = []
+ reviewer = data.get('reviewer')
+ review_type = data.get('review_type')
+ if reviewer is None:
+ reviewer = target_ref.code_reviewer
+ if reviewer is not None:
+ review_requests.append((reviewer, review_type))
+
+ repository_names = [
+ ref.repository.unique_name for ref in (source_ref, target_ref)]
+ repository_set = getUtility(IGitRepositorySet)
+ visibility_info = repository_set.getRepositoryVisibilityInfo(
+ self.user, reviewer, repository_names)
+ visible_repositories = list(visibility_info['visible_repositories'])
+ if self.request.is_ajax and len(visible_repositories) < 2:
+ self.request.response.setStatus(400, "Repository Visibility")
+ self.request.response.setHeader(
+ 'Content-Type', 'application/json')
+ return json.dumps({
+ 'person_name': visibility_info['person_name'],
+ 'repositories_to_check': repository_names,
+ 'visible_repositories': visible_repositories,
+ })
+
+ try:
+ proposal = source_ref.addLandingTarget(
+ registrant=registrant, merge_target=target_ref,
+ merge_prerequisite=prerequisite_ref,
+ needs_review=data['needs_review'],
+ description=data.get('comment'),
+ review_requests=review_requests,
+ commit_message=data.get('commit_message'))
+ if len(visible_repositories) < 2:
+ invisible_repositories = [
+ ref.repository.unique_name
+ for ref in (source_ref, target_ref)
+ if ref.repository.unique_name not in visible_repositories]
+ self.request.response.addNotification(
+ 'To ensure visibility, %s is now subscribed to: %s'
+ % (visibility_info['person_name'],
+ english_list(invisible_repositories)))
+ # Success so we do a client redirect to the new mp page.
+ if self.request.is_ajax:
+ self.request.response.setStatus(201)
+ self.request.response.setHeader(
+ 'Location', canonical_url(proposal))
+ return None
+ else:
+ self.next_url = canonical_url(proposal)
+ except InvalidBranchMergeProposal as error:
+ self.addError(str(error))
+
+ def validate(self, data):
+ source_ref = self.context
+ target_repository = data['target_git_repository']
+ target_ref = target_repository.getRefByPath(data['target_git_path'])
+ if target_ref is None:
+ self.setFieldError(
+ 'target_git_path',
+ "The target path must be the path of a reference in the "
+ "target repository.")
+ elif source_ref == target_ref:
+ self.setFieldError(
+ 'target_git_path',
+ "The target repository and path together cannot be the same "
+ "as the source repository and path.")
+ elif not target_repository.isRepositoryMergeable(self.context):
+ self.setFieldError(
+ 'target_git_repository',
+ "This repository is not mergeable into %s." %
+ target_repository.identity)
+ elif (data.get('prerequisite_git_repository') is not None and
+ data.get('prerequisite_git_path') is not None):
+ prerequisite_repository = data['prerequisite_git_repository']
+ prerequisite_ref = prerequisite_repository.getRefByPath(
+ data['prerequisite_git_path'])
+ if prerequisite_ref is None:
+ self.setFieldError(
+ 'prerequisite_git_path',
+ "The prerequisite path must be the path of a reference in "
+ "the prerequisite repository.")
+ elif not target_repository.isRepositoryMergeable(
+ prerequisite_repository):
+ self.setFieldError(
+ 'prerequisite_git_repository',
+ "This repository is not mergeable into %s." %
+ target_repository.identity)
=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
--- lib/lp/code/browser/tests/test_branchmergeproposal.py 2015-04-28 16:51:38 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposal.py 2015-04-28 16:51:38 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
@@ -42,6 +42,7 @@
latest_proposals_for_each_branch,
)
from lp.code.browser.codereviewcomment import CodeReviewDisplayComment
+from lp.code.browser.gitref import GitRefRegisterMergeProposalView
from lp.code.enums import (
BranchMergeProposalStatus,
CodeReviewVote,
@@ -402,41 +403,28 @@
view.render()
-class TestRegisterBranchMergeProposalView(BrowserTestCase):
+class TestRegisterBranchMergeProposalViewMixin:
"""Test the merge proposal registration view."""
layer = LaunchpadFunctionalLayer
def setUp(self):
- TestCaseWithFactory.setUp(self)
- self.source_branch = self.factory.makeProductBranch()
+ super(TestRegisterBranchMergeProposalViewMixin, self).setUp()
+ self.source_branch = self._makeBranch()
self.user = self.factory.makePerson()
login_person(self.user)
- def _makeTargetBranch(self, **kwargs):
- return self.factory.makeProductBranch(
- product=self.source_branch.product, **kwargs)
-
def _makeTargetBranchWithReviewer(self):
albert = self.factory.makePerson(name='albert')
- target_branch = self.factory.makeProductBranch(
- reviewer=albert, product=self.source_branch.product)
+ target_branch = self._makeTargetBranch(reviewer=albert)
return target_branch, albert
- def _createView(self, request=None):
- # Construct the view and initialize it.
- if not request:
- request = LaunchpadTestRequest()
- view = RegisterBranchMergeProposalView(self.source_branch, request)
- view.initialize()
- return view
-
def _getSourceProposal(self, target_branch):
# There will only be one proposal.
landing_targets = list(self.source_branch.landing_targets)
self.assertEqual(1, len(landing_targets))
proposal = landing_targets[0]
- self.assertEqual(target_branch, proposal.target_branch)
+ self.assertEqual(target_branch, proposal.merge_target)
return proposal
def assertOnePendingReview(self, proposal, reviewer, review_type=None):
@@ -458,72 +446,12 @@
# therefore be set to the branch owner.
target_branch = self._makeTargetBranch()
view = self._createView()
- view.register_action.success(
- {'target_branch': target_branch,
- 'needs_review': True})
+ view.register_action.success(self._getFormValues(
+ target_branch, {'needs_review': True}))
proposal = self._getSourceProposal(target_branch)
self.assertOnePendingReview(proposal, target_branch.owner)
self.assertIs(None, proposal.description)
- def test_register_ajax_request_with_confirmation(self):
- # Ajax submits return json data containing info about what the visible
- # branches are if they are not all visible to the reviewer.
-
- # Make a branch the reviewer cannot see.
- owner = self.factory.makePerson()
- target_branch = self._makeTargetBranch(
- owner=owner, information_type=InformationType.USERDATA)
- reviewer = self.factory.makePerson()
- extra = {'HTTP_X_REQUESTED_WITH': 'XMLHttpRequest'}
- request = LaunchpadTestRequest(
- method='POST', principal=owner, **extra)
- view = self._createView(request=request)
- with person_logged_in(owner):
- branches_to_check = [self.source_branch.unique_name,
- target_branch.unique_name]
- expected_data = {
- 'person_name': reviewer.displayname,
- 'branches_to_check': branches_to_check,
- 'visible_branches': [self.source_branch.unique_name]}
- result_data = view.register_action.success(
- {'target_branch': target_branch,
- 'reviewer': reviewer,
- 'needs_review': True})
- self.assertEqual(
- '400 Branch Visibility',
- view.request.response.getStatusString())
- self.assertEqual(expected_data, simplejson.loads(result_data))
-
- def test_register_ajax_request_with_validation_errors(self):
- # Ajax submits where there is a validation error in the submitted data
- # return the expected json response containing the error info.
- owner = self.factory.makePerson()
- target_branch = self._makeTargetBranch(
- owner=owner, information_type=InformationType.USERDATA)
- extra = {'HTTP_X_REQUESTED_WITH': 'XMLHttpRequest'}
- with person_logged_in(owner):
- request = LaunchpadTestRequest(
- method='POST', principal=owner,
- form={
- 'field.actions.register': 'Propose Merge',
- 'field.target_branch.target_branch':
- target_branch.unique_name},
- **extra)
- view = create_initialized_view(
- target_branch,
- name='+register-merge',
- request=request)
- self.assertEqual(
- '400 Validation', view.request.response.getStatusString())
- self.assertEqual(
- {'error_summary': 'There is 1 error.',
- 'errors': {
- 'field.target_branch':
- ('The target branch cannot be the same as the '
- 'source branch.')},
- 'form_wide_errors': []},
- simplejson.loads(view.form_result))
-
def test_register_ajax_request_with_no_confirmation(self):
# Ajax submits where there is no confirmation required return a 201
# with the new location.
@@ -535,10 +463,11 @@
method='POST', principal=owner, **extra)
view = self._createView(request=request)
with person_logged_in(owner):
- result_data = view.register_action.success(
- {'target_branch': target_branch,
- 'reviewer': reviewer,
- 'needs_review': True})
+ result_data = view.register_action.success(self._getFormValues(
+ target_branch, {
+ 'reviewer': reviewer,
+ 'needs_review': True,
+ }))
self.assertEqual(None, result_data)
self.assertEqual(201, view.request.response.getStatus())
mp = target_branch.getMergeProposals()[0]
@@ -550,9 +479,8 @@
# progress proposal.
target_branch = self._makeTargetBranch()
view = self._createView()
- view.register_action.success(
- {'target_branch': target_branch,
- 'needs_review': False})
+ view.register_action.success(self._getFormValues(
+ target_branch, {'needs_review': False}))
proposal = self._getSourceProposal(target_branch)
self.assertEqual(
BranchMergeProposalStatus.WORK_IN_PROGRESS,
@@ -562,10 +490,11 @@
# A commit message can also be set during the register process.
target_branch = self._makeTargetBranch()
view = self._createView()
- view.register_action.success(
- {'target_branch': target_branch,
- 'needs_review': True,
- 'commit_message': 'Fixed the bug!'})
+ view.register_action.success(self._getFormValues(
+ target_branch, {
+ 'needs_review': True,
+ 'commit_message': 'Fixed the bug!',
+ }))
proposal = self._getSourceProposal(target_branch)
self.assertEqual('Fixed the bug!', proposal.commit_message)
@@ -574,10 +503,11 @@
# proposal.
target_branch = self._makeTargetBranch()
view = self._createView()
- view.register_action.success(
- {'target_branch': target_branch,
- 'comment': "This is the description.",
- 'needs_review': True})
+ view.register_action.success(self._getFormValues(
+ target_branch, {
+ 'comment': "This is the description.",
+ 'needs_review': True,
+ }))
proposal = self._getSourceProposal(target_branch)
self.assertOnePendingReview(proposal, target_branch.owner)
@@ -589,10 +519,11 @@
target_branch = self._makeTargetBranch()
reviewer = self.factory.makePerson()
view = self._createView()
- view.register_action.success(
- {'target_branch': target_branch,
- 'reviewer': reviewer,
- 'needs_review': True})
+ view.register_action.success(self._getFormValues(
+ target_branch, {
+ 'reviewer': reviewer,
+ 'needs_review': True,
+ }))
proposal = self._getSourceProposal(target_branch)
self.assertOnePendingReview(proposal, reviewer)
@@ -604,11 +535,12 @@
target_branch = self._makeTargetBranch()
reviewer = self.factory.makePerson()
view = self._createView()
- view.register_action.success(
- {'target_branch': target_branch,
- 'reviewer': reviewer,
- 'review_type': 'god-like',
- 'needs_review': True})
+ view.register_action.success(self._getFormValues(
+ target_branch, {
+ 'reviewer': reviewer,
+ 'review_type': 'god-like',
+ 'needs_review': True,
+ }))
proposal = self._getSourceProposal(target_branch)
self.assertOnePendingReview(proposal, reviewer, 'god-like')
@@ -620,12 +552,13 @@
target_branch = self._makeTargetBranch()
reviewer = self.factory.makePerson()
view = self._createView()
- view.register_action.success(
- {'target_branch': target_branch,
- 'reviewer': reviewer,
- 'review_type': 'god-like',
- 'comment': "This is the description.",
- 'needs_review': True})
+ view.register_action.success(self._getFormValues(
+ target_branch, {
+ 'reviewer': reviewer,
+ 'review_type': 'god-like',
+ 'comment': "This is the description.",
+ 'needs_review': True,
+ }))
proposal = self._getSourceProposal(target_branch)
self.assertOnePendingReview(proposal, reviewer, 'god-like')
@@ -637,9 +570,8 @@
# has a reviewer so that reviewer should be used
target_branch, reviewer = self._makeTargetBranchWithReviewer()
view = self._createView()
- view.register_action.success(
- {'target_branch': target_branch,
- 'needs_review': True})
+ view.register_action.success(self._getFormValues(
+ target_branch, {'needs_review': True}))
proposal = self._getSourceProposal(target_branch)
self.assertOnePendingReview(proposal, reviewer)
self.assertIs(None, proposal.description)
@@ -649,16 +581,17 @@
# reviewer so that reviewer should be used.
target_branch, reviewer = self._makeTargetBranchWithReviewer()
view = self._createView()
- view.register_action.success(
- {'target_branch': target_branch,
- 'review_type': 'god-like',
- 'needs_review': True})
+ view.register_action.success(self._getFormValues(
+ target_branch, {
+ 'review_type': 'god-like',
+ 'needs_review': True,
+ }))
proposal = self._getSourceProposal(target_branch)
self.assertOnePendingReview(proposal, reviewer, 'god-like')
self.assertIs(None, proposal.description)
def test_register_reviewer_not_hidden(self):
- branch = self.factory.makeBranch()
+ branch = self._makeBranch()
browser = self.getViewBrowser(branch, '+register-merge')
extra = Tag(
'extra', 'fieldset', attrs={'id': 'mergeproposal-extra-options'})
@@ -675,10 +608,11 @@
reviewer = self.factory.makePerson()
with person_logged_in(owner):
view = self._createView()
- view.register_action.success(
- {'target_branch': target_branch,
- 'reviewer': reviewer,
- 'needs_review': True})
+ view.register_action.success(self._getFormValues(
+ target_branch, {
+ 'reviewer': reviewer,
+ 'needs_review': True,
+ }))
(notification,) = view.request.response.notifications
self.assertThat(
@@ -687,6 +621,194 @@
self.assertEqual(BrowserNotificationLevel.INFO, notification.level)
+class TestRegisterBranchMergeProposalViewBzr(
+ TestRegisterBranchMergeProposalViewMixin, BrowserTestCase):
+ """Test the merge proposal registration view for Bazaar."""
+
+ def _makeBranch(self):
+ return self.factory.makeProductBranch()
+
+ def _makeTargetBranch(self, **kwargs):
+ return self.factory.makeProductBranch(
+ product=self.source_branch.product, **kwargs)
+
+ def _makeTargetBranchWithReviewer(self):
+ albert = self.factory.makePerson(name='albert')
+ target_branch = self._makeTargetBranch(reviewer=albert)
+ return target_branch, albert
+
+ def _createView(self, request=None):
+ # Construct the view and initialize it.
+ if not request:
+ request = LaunchpadTestRequest()
+ view = RegisterBranchMergeProposalView(self.source_branch, request)
+ view.initialize()
+ return view
+
+ @staticmethod
+ def _getFormValues(target_branch, extras):
+ values = {'target_branch': target_branch}
+ values.update(extras)
+ return values
+
+ def test_register_ajax_request_with_confirmation(self):
+ # Ajax submits return json data containing info about what the visible
+ # branches are if they are not all visible to the reviewer.
+
+ # Make a branch the reviewer cannot see.
+ owner = self.factory.makePerson()
+ target_branch = self._makeTargetBranch(
+ owner=owner, information_type=InformationType.USERDATA)
+ reviewer = self.factory.makePerson()
+ extra = {'HTTP_X_REQUESTED_WITH': 'XMLHttpRequest'}
+ request = LaunchpadTestRequest(
+ method='POST', principal=owner, **extra)
+ view = self._createView(request=request)
+ with person_logged_in(owner):
+ branches_to_check = [self.source_branch.unique_name,
+ target_branch.unique_name]
+ expected_data = {
+ 'person_name': reviewer.displayname,
+ 'branches_to_check': branches_to_check,
+ 'visible_branches': [self.source_branch.unique_name]}
+ result_data = view.register_action.success(self._getFormValues(
+ target_branch, {
+ 'reviewer': reviewer,
+ 'needs_review': True,
+ }))
+ self.assertEqual(
+ '400 Branch Visibility',
+ view.request.response.getStatusString())
+ self.assertEqual(expected_data, simplejson.loads(result_data))
+
+ def test_register_ajax_request_with_validation_errors(self):
+ # Ajax submits where there is a validation error in the submitted data
+ # return the expected json response containing the error info.
+ owner = self.factory.makePerson()
+ target_branch = self._makeTargetBranch(
+ owner=owner, information_type=InformationType.USERDATA)
+ extra = {'HTTP_X_REQUESTED_WITH': 'XMLHttpRequest'}
+ with person_logged_in(owner):
+ request = LaunchpadTestRequest(
+ method='POST', principal=owner,
+ form={
+ 'field.actions.register': 'Propose Merge',
+ 'field.target_branch.target_branch':
+ target_branch.unique_name},
+ **extra)
+ view = create_initialized_view(
+ target_branch,
+ name='+register-merge',
+ request=request)
+ self.assertEqual(
+ '400 Validation', view.request.response.getStatusString())
+ self.assertEqual(
+ {'error_summary': 'There is 1 error.',
+ 'errors': {
+ 'field.target_branch':
+ ('The target branch cannot be the same as the '
+ 'source branch.')},
+ 'form_wide_errors': []},
+ simplejson.loads(view.form_result))
+
+
+class TestRegisterBranchMergeProposalViewGit(
+ TestRegisterBranchMergeProposalViewMixin, BrowserTestCase):
+ """Test the merge proposal registration view for Git."""
+
+ def setUp(self):
+ self.useFixture(FeatureFixture({GIT_FEATURE_FLAG: u"on"}))
+ super(TestRegisterBranchMergeProposalViewGit, self).setUp()
+
+ def _makeBranch(self):
+ return self.factory.makeGitRefs()[0]
+
+ def _makeTargetBranch(self, **kwargs):
+ return self.factory.makeGitRefs(
+ target=self.source_branch.target, **kwargs)[0]
+
+ def _createView(self, request=None):
+ # Construct the view and initialize it.
+ if not request:
+ request = LaunchpadTestRequest()
+ view = GitRefRegisterMergeProposalView(self.source_branch, request)
+ view.initialize()
+ return view
+
+ @staticmethod
+ def _getFormValues(target_branch, extras):
+ values = {
+ 'target_git_repository': target_branch.repository,
+ 'target_git_path': target_branch.path,
+ }
+ values.update(extras)
+ return values
+
+ def test_register_ajax_request_with_confirmation(self):
+ # Ajax submits return json data containing info about what the visible
+ # repositories are if they are not all visible to the reviewer.
+
+ # Make a branch the reviewer cannot see.
+ owner = self.factory.makePerson()
+ target_branch = self._makeTargetBranch(
+ owner=owner, information_type=InformationType.USERDATA)
+ reviewer = self.factory.makePerson()
+ extra = {'HTTP_X_REQUESTED_WITH': 'XMLHttpRequest'}
+ request = LaunchpadTestRequest(
+ method='POST', principal=owner, **extra)
+ view = self._createView(request=request)
+ with person_logged_in(owner):
+ repositories_to_check = [
+ self.source_branch.repository.unique_name,
+ target_branch.repository.unique_name]
+ expected_data = {
+ 'person_name': reviewer.displayname,
+ 'repositories_to_check': repositories_to_check,
+ 'visible_repositories':
+ [self.source_branch.repository.unique_name]}
+ result_data = view.register_action.success(self._getFormValues(
+ target_branch, {
+ 'reviewer': reviewer,
+ 'needs_review': True,
+ }))
+ self.assertEqual(
+ '400 Repository Visibility',
+ view.request.response.getStatusString())
+ self.assertEqual(expected_data, simplejson.loads(result_data))
+
+ def test_register_ajax_request_with_validation_errors(self):
+ # Ajax submits where there is a validation error in the submitted data
+ # return the expected json response containing the error info.
+ owner = self.factory.makePerson()
+ target_branch = self._makeTargetBranch(
+ owner=owner, information_type=InformationType.USERDATA)
+ extra = {'HTTP_X_REQUESTED_WITH': 'XMLHttpRequest'}
+ with person_logged_in(owner):
+ request = LaunchpadTestRequest(
+ method='POST', principal=owner,
+ form={
+ 'field.actions.register': 'Propose Merge',
+ 'field.target_git_repository.target_git_repository':
+ target_branch.repository.unique_name,
+ 'field.target_git_path': target_branch.path,
+ },
+ **extra)
+ view = create_initialized_view(
+ target_branch,
+ name='+register-merge',
+ request=request)
+ self.assertEqual(
+ '400 Validation', view.request.response.getStatusString())
+ self.assertEqual(
+ {'error_summary': 'There is 1 error.',
+ 'errors': {
+ 'field.target_git_path':
+ ('The target repository and path together cannot be the '
+ 'same as the source repository and path.')},
+ 'form_wide_errors': []},
+ simplejson.loads(view.form_result))
+
+
class TestBranchMergeProposalResubmitViewMixin:
"""Test BranchMergeProposalResubmitView."""
=== modified file 'lib/lp/code/templates/gitref-pending-merges.pt'
--- lib/lp/code/templates/gitref-pending-merges.pt 2015-04-28 16:51:38 +0000
+++ lib/lp/code/templates/gitref-pending-merges.pt 2015-04-28 16:51:38 +0000
@@ -38,6 +38,11 @@
</div>
</div>
+ <div
+ tal:define="link context_menu/register_merge"
+ tal:condition="link/enabled"
+ tal:replace="structure link/render"
+ />
</div>
</div>
=== added file 'lib/lp/code/templates/gitref-register-merge.pt'
--- lib/lp/code/templates/gitref-register-merge.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/code/templates/gitref-register-merge.pt 2015-04-28 16:51:38 +0000
@@ -0,0 +1,81 @@
+<html
+ xmlns="http://www.w3.org/1999/xhtml";
+ xmlns:tal="http://xml.zope.org/namespaces/tal";
+ xmlns:metal="http://xml.zope.org/namespaces/metal";
+ xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+ metal:use-macro="view/macro:page/main_only"
+ i18n:domain="launchpad">
+ <body>
+ <div metal:fill-slot="main">
+ <div metal:use-macro="context/@@launchpad_form/form">
+
+ <metal:formbody fill-slot="widgets">
+ <table class="form">
+
+ <tal:widget define="widget nocall:view/widgets/target_git_repository">
+ <metal:block use-macro="context/@@launchpad_form/widget_row" />
+ </tal:widget>
+
+ <tal:widget define="widget nocall:view/widgets/target_git_path">
+ <metal:block use-macro="context/@@launchpad_form/widget_row" />
+ </tal:widget>
+
+ <tal:widget define="widget nocall:view/widgets/comment">
+ <metal:block use-macro="context/@@launchpad_form/widget_row" />
+ </tal:widget>
+
+ <tal:widget define="widget nocall:view/widgets/reviewer">
+ <metal:block use-macro="context/@@launchpad_form/widget_row" />
+ </tal:widget>
+
+ <tal:widget define="widget nocall:view/widgets/review_type">
+ <metal:block use-macro="context/@@launchpad_form/widget_row" />
+ </tal:widget>
+
+ <td colspan="2">
+ <fieldset id="mergeproposal-extra-options"
+ class="collapsible">
+ <legend>Extra options</legend>
+ <div class="hide-on-load"><!-- hidden by default -->
+ <table class="extra-options">
+ <tal:widget define="widget nocall:view/widgets/commit_message">
+ <metal:block use-macro="context/@@launchpad_form/widget_row" />
+ </tal:widget>
+
+ <tal:widget define="widget nocall:view/widgets/needs_review">
+ <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>
+ </div>
+ </fieldset>
+ </td>
+
+ </table>
+ </metal:formbody>
+ </div>
+
+<!-- XXX cjwatson 2015-04-18: Hook this up once the JS handles repositories.
+ <tal:script>
+ <script type="text/javascript">
+ LPJS.use('lp.code.branchmergeproposal.nominate', function(Y) {
+ Y.on('domready',
+ function(e) {
+ Y.lp.code.branchmergeproposal.nominate.setup();
+ },
+ window);
+ });
+ </script>
+ </tal:script>
+-->
+
+ </div>
+ </body>
+</html>
Follow ups
