launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #18432
Re: [Merge] lp:~cjwatson/launchpad/git-mp-register into lp:launchpad
Diff comments:
> === 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)
Please use structured properly here.
> +
> + 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>
>
--
https://code.launchpad.net/~cjwatson/launchpad/git-mp-register/+merge/257676
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
References
