launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #19597
[Merge] lp:~cjwatson/launchpad/dependent-merges into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/dependent-merges into lp:launchpad.
Commit message:
Add Branch:+dependent-merges and GitRef:+dependent-merges views.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #496056 in Launchpad itself: "Dependent branch list is empty"
https://bugs.launchpad.net/launchpad/+bug/496056
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/dependent-merges/+merge/274132
Add Branch:+dependent-merges and GitRef:+dependent-merges views, making the "<n> branch/branches dependent on this one" links useful again.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/dependent-merges into lp:launchpad.
=== modified file 'lib/lp/code/browser/branchmergeproposallisting.py'
--- lib/lp/code/browser/branchmergeproposallisting.py 2015-10-04 16:38:18 +0000
+++ lib/lp/code/browser/branchmergeproposallisting.py 2015-10-12 13:06:46 +0000
@@ -8,6 +8,7 @@
__all__ = [
'ActiveReviewsView',
'BranchActiveReviewsView',
+ 'BranchDependentMergesView',
'BranchMergeProposalListingItem',
'BranchMergeProposalListingView',
'PersonActiveReviewsView',
@@ -264,6 +265,15 @@
self.context.displayname, self.status_value.title)
+class BranchDependentMergesView(BranchMergeProposalListingView):
+ """Branch merge proposals that list this branch as a prerequisite."""
+
+ def getVisibleProposalsForUser(self):
+ """See `BranchMergeProposalListingView`."""
+ return self.context.getDependentMergeProposals(
+ self.status_filter, self.user, eager_load=True)
+
+
class ActiveReviewsView(BranchMergeProposalListingView):
"""Branch merge proposals for a context that are needing review."""
=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml 2015-09-18 15:41:08 +0000
+++ lib/lp/code/browser/configure.zcml 2015-10-12 13:06:46 +0000
@@ -503,6 +503,12 @@
name="+activereviews"
template="../templates/active-reviews.pt"/>
<browser:page
+ for="lp.code.interfaces.branch.IBranch"
+ class="lp.code.browser.branchmergeproposallisting.BranchDependentMergesView"
+ permission="zope.Public"
+ name="+dependent-merges"
+ template="../templates/branchmergeproposal-generic-listing.pt"/>
+ <browser:page
for="lp.code.interfaces.branch.IBranchBatchNavigator"
name="+branch-listing"
template="../templates/branch-listing.pt"
@@ -909,6 +915,12 @@
permission="zope.Public"
name="+activereviews"
template="../templates/active-reviews.pt"/>
+ <browser:page
+ for="lp.code.interfaces.gitref.IGitRef"
+ class="lp.code.browser.branchmergeproposallisting.BranchDependentMergesView"
+ permission="zope.Public"
+ name="+dependent-merges"
+ template="../templates/branchmergeproposal-generic-listing.pt"/>
<browser:menus
classes="GitRefContextMenu"
module="lp.code.browser.gitref"/>
=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposallisting.py'
--- lib/lp/code/browser/tests/test_branchmergeproposallisting.py 2015-09-30 10:25:58 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposallisting.py 2015-10-12 13:06:46 +0000
@@ -364,6 +364,51 @@
self.assertIn("has no merge proposals", browser.contents)
+class DependentMergesTestMixin(BranchMergeProposalListingTestMixin):
+
+ view_name = '+dependent-merges'
+
+ def makeBzrMergeProposal(self):
+ information_type = (
+ InformationType.USERDATA if self.supports_privacy else None)
+ prerequisite = self.bzr_branch
+ if prerequisite is None:
+ prerequisite = self.factory.makeBranch(
+ target=self.bzr_target, information_type=information_type)
+ target = self.factory.makeBranch(
+ target=self.bzr_target, information_type=information_type)
+ source = self.factory.makeBranch(
+ target=self.bzr_target, owner=self.owner,
+ information_type=information_type)
+ return self.factory.makeBranchMergeProposal(
+ source_branch=source, target_branch=target,
+ prerequisite_branch=prerequisite,
+ set_state=BranchMergeProposalStatus.NEEDS_REVIEW)
+
+ def makeGitMergeProposal(self):
+ information_type = (
+ InformationType.USERDATA if self.supports_privacy else None)
+ prerequisite = self.git_ref
+ if prerequisite is None:
+ [prerequisite] = self.factory.makeGitRefs(
+ target=self.git_target, information_type=information_type)
+ [target] = self.factory.makeGitRefs(
+ target=self.git_target, information_type=information_type)
+ [source] = self.factory.makeGitRefs(
+ target=self.git_target, owner=self.owner,
+ information_type=information_type)
+ return self.factory.makeBranchMergeProposalForGit(
+ source_ref=source, target_ref=target,
+ prerequisite_ref=prerequisite,
+ set_state=BranchMergeProposalStatus.NEEDS_REVIEW)
+
+ def test_none(self):
+ """The dependent merges view should be enabled for the target."""
+ browser = self.getViewBrowser(
+ self.context, self.view_name, rootsite='code', user=self.user)
+ self.assertIn("has no merge proposals", browser.contents)
+
+
class ActiveReviewsTestMixin(BranchMergeProposalListingTestMixin):
view_name = '+activereviews'
@@ -524,6 +569,18 @@
pass
+class TestBranchDependentMerges(
+ BranchContextMixin, DependentMergesTestMixin, BrowserTestCase):
+
+ pass
+
+
+class TestGitRefDependentMerges(
+ GitRefContextMixin, DependentMergesTestMixin, BrowserTestCase):
+
+ pass
+
+
class TestProductActiveReviews(
ProductContextMixin, ActiveReviewsTestMixin, BrowserTestCase):
=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py 2015-09-29 15:54:05 +0000
+++ lib/lp/code/interfaces/branch.py 2015-10-12 13:06:46 +0000
@@ -661,6 +661,10 @@
merged_revnos=None, eager_load=False):
"""Return matching BranchMergeProposals."""
+ def getDependentMergeProposals(status=None, visible_by_user=None,
+ eager_load=False):
+ """Return BranchMergeProposals dependent on merging this branch."""
+
def getMergeProposalByID(id):
"""Return this branch's merge proposal with this id, or None."""
=== modified file 'lib/lp/code/interfaces/branchcollection.py'
--- lib/lp/code/interfaces/branchcollection.py 2015-09-17 11:12:42 +0000
+++ lib/lp/code/interfaces/branchcollection.py 2015-10-12 13:06:46 +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).
"""A collection of branches.
@@ -73,8 +73,8 @@
def getBranchIds():
"""Return a result set of all branch ids in this collection."""
- def getMergeProposals(statuses=None, for_branches=None,
- target_branch=None, eager_load=False):
+ def getMergeProposals(statuses=None, for_branches=None, target_branch=None,
+ prerequisite_branch=None, eager_load=False):
"""Return a result set of merge proposals for the branches in this
collection.
@@ -85,6 +85,8 @@
branch is one of the branches specified.
:param target_branch: If specified, only return merge proposals
that target the specified branch.
+ :param prerequisite_branch: If specified, only return merge proposals
+ that require the specified branch to be merged first.
:param eager_load: If True, preloads all the related information for
merge proposals like PreviewDiffs and Branches.
"""
=== modified file 'lib/lp/code/interfaces/gitcollection.py'
--- lib/lp/code/interfaces/gitcollection.py 2015-09-21 09:59:13 +0000
+++ lib/lp/code/interfaces/gitcollection.py 2015-10-12 13:06:46 +0000
@@ -74,7 +74,8 @@
# XXX cjwatson 2015-04-16: Add something like for_repositories or
# for_refs once we know exactly what we need.
def getMergeProposals(statuses=None, target_repository=None,
- target_path=None, eager_load=False):
+ target_path=None, prerequisite_repository=None,
+ prerequisite_path=None, eager_load=False):
"""Return a result set of merge proposals for the repositories in
this collection.
@@ -84,6 +85,12 @@
that target the specified repository.
:param target_path: If specified, only return merge proposals that
target the specified path.
+ :param prerequisite_repository: If specified, only return merge
+ proposals that require a reference in the specified repository to
+ be merged first.
+ :param prerequisite_path: If specified, only return merge proposals
+ that require a reference with the specified path to be merged
+ first.
:param eager_load: If True, preloads all the related information for
merge proposals like PreviewDiffs and GitRepositories.
"""
=== modified file 'lib/lp/code/interfaces/gitref.py'
--- lib/lp/code/interfaces/gitref.py 2015-09-30 08:58:44 +0000
+++ lib/lp/code/interfaces/gitref.py 2015-10-12 13:06:46 +0000
@@ -305,6 +305,10 @@
merged_revision_ids=None, eager_load=False):
"""Return matching BranchMergeProposals."""
+ def getDependentMergeProposals(status=None, visible_by_user=None,
+ eager_load=False):
+ """Return BranchMergeProposals dependent on merging this reference."""
+
def getMergeProposalByID(id):
"""Return this reference's merge proposal with this id, or None."""
=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py 2015-10-06 16:08:27 +0000
+++ lib/lp/code/model/branch.py 2015-10-12 13:06:46 +0000
@@ -526,6 +526,19 @@
status, target_branch=self, merged_revnos=merged_revnos,
eager_load=eager_load)
+ def getDependentMergeProposals(self, status=None, visible_by_user=None,
+ eager_load=False):
+ """See `IBranch`."""
+ if not status:
+ status = (
+ BranchMergeProposalStatus.CODE_APPROVED,
+ BranchMergeProposalStatus.NEEDS_REVIEW,
+ BranchMergeProposalStatus.WORK_IN_PROGRESS)
+
+ collection = getUtility(IAllBranches).visibleByUser(visible_by_user)
+ return collection.getMergeProposals(
+ status, prerequisite_branch=self, eager_load=eager_load)
+
def getMergeProposalByID(self, id):
"""See `IBranch`."""
return Store.of(self).find(
=== modified file 'lib/lp/code/model/branchcollection.py'
--- lib/lp/code/model/branchcollection.py 2015-09-17 11:12:42 +0000
+++ lib/lp/code/model/branchcollection.py 2015-10-12 13:06:46 +0000
@@ -307,8 +307,9 @@
return self.getBranches(find_expr=Branch.id).get_plain_result_set()
def getMergeProposals(self, statuses=None, for_branches=None,
- target_branch=None, merged_revnos=None,
- merged_revision=None, eager_load=False):
+ target_branch=None, prerequisite_branch=None,
+ merged_revnos=None, merged_revision=None,
+ eager_load=False):
"""See `IBranchCollection`."""
if for_branches is not None and not for_branches:
# We have an empty branches list, so we can shortcut.
@@ -319,11 +320,12 @@
elif (self._asymmetric_filter_expressions or
for_branches is not None or
target_branch is not None or
+ prerequisite_branch is not None or
merged_revnos is not None or
merged_revision is not None):
return self._naiveGetMergeProposals(
- statuses, for_branches, target_branch, merged_revnos,
- merged_revision, eager_load=eager_load)
+ statuses, for_branches, target_branch, prerequisite_branch,
+ merged_revnos, merged_revision, eager_load=eager_load)
else:
# When examining merge proposals in a scope, this is a moderately
# effective set of constrained queries. It is not effective when
@@ -332,8 +334,9 @@
statuses, eager_load=eager_load)
def _naiveGetMergeProposals(self, statuses=None, for_branches=None,
- target_branch=None, merged_revnos=None,
- merged_revision=None, eager_load=False):
+ target_branch=None, prerequisite_branch=None,
+ merged_revnos=None, merged_revision=None,
+ eager_load=False):
Target = ClassAlias(Branch, "target")
extra_tables = list(set(
self._tables.values() + self._asymmetric_tables.values()))
@@ -353,6 +356,9 @@
if target_branch is not None:
expressions.append(
BranchMergeProposal.target_branch == target_branch)
+ if prerequisite_branch is not None:
+ expressions.append(
+ BranchMergeProposal.prerequisite_branch == prerequisite_branch)
if merged_revnos is not None:
expressions.append(
BranchMergeProposal.merged_revno.is_in(merged_revnos))
=== modified file 'lib/lp/code/model/gitcollection.py'
--- lib/lp/code/model/gitcollection.py 2015-09-21 09:59:13 +0000
+++ lib/lp/code/model/gitcollection.py 2015-10-12 13:06:46 +0000
@@ -45,8 +45,8 @@
from lp.code.model.codereviewcomment import CodeReviewComment
from lp.code.model.codereviewvote import CodeReviewVoteReference
from lp.code.model.gitrepository import (
+ get_git_repository_privacy_filter,
GitRepository,
- get_git_repository_privacy_filter,
)
from lp.code.model.gitsubscription import GitSubscription
from lp.registry.enums import EXCLUSIVE_TEAM_POLICY
@@ -237,7 +237,8 @@
find_expr=GitRepository.id).get_plain_result_set()
def getMergeProposals(self, statuses=None, target_repository=None,
- target_path=None, merged_revision_ids=None,
+ target_path=None, prerequisite_repository=None,
+ prerequisite_path=None, merged_revision_ids=None,
eager_load=False):
"""See `IGitCollection`."""
if merged_revision_ids is not None and not merged_revision_ids:
@@ -246,10 +247,13 @@
elif (self._asymmetric_filter_expressions or
target_repository is not None or
target_path is not None or
+ prerequisite_repository is not None or
+ prerequisite_path is not None or
merged_revision_ids is not None):
return self._naiveGetMergeProposals(
- statuses, target_repository, target_path, merged_revision_ids,
- eager_load=eager_load)
+ statuses, target_repository, target_path,
+ prerequisite_repository, prerequisite_path,
+ merged_revision_ids, eager_load=eager_load)
else:
# When examining merge proposals in a scope, this is a moderately
# effective set of constrained queries. It is not effective when
@@ -258,8 +262,9 @@
statuses, eager_load=eager_load)
def _naiveGetMergeProposals(self, statuses=None, target_repository=None,
- target_path=None, merged_revision_ids=None,
- eager_load=False):
+ target_path=None, prerequisite_repository=None,
+ prerequisite_path=None,
+ merged_revision_ids=None, eager_load=False):
Target = ClassAlias(GitRepository, "target")
extra_tables = list(set(
self._tables.values() + self._asymmetric_tables.values()))
@@ -280,6 +285,13 @@
if target_path is not None:
expressions.append(
BranchMergeProposal.target_git_path == target_path)
+ if prerequisite_repository is not None:
+ expressions.append(
+ BranchMergeProposal.prerequisite_git_repository ==
+ prerequisite_repository)
+ if prerequisite_path is not None:
+ expressions.append(
+ BranchMergeProposal.prerequisite_git_path == prerequisite_path)
if merged_revision_ids is not None:
expressions.append(
BranchMergeProposal.merged_revision_id.is_in(
=== modified file 'lib/lp/code/model/gitref.py'
--- lib/lp/code/model/gitref.py 2015-10-06 16:08:27 +0000
+++ lib/lp/code/model/gitref.py 2015-10-12 13:06:46 +0000
@@ -222,6 +222,21 @@
status, target_repository=self.repository, target_path=self.path,
merged_revision_ids=merged_revision_ids, eager_load=eager_load)
+ def getDependentMergeProposals(self, status=None, visible_by_user=None,
+ eager_load=False):
+ """See `IGitRef`."""
+ if not status:
+ status = (
+ BranchMergeProposalStatus.CODE_APPROVED,
+ BranchMergeProposalStatus.NEEDS_REVIEW,
+ BranchMergeProposalStatus.WORK_IN_PROGRESS)
+
+ collection = getUtility(IAllGitRepositories).visibleByUser(
+ visible_by_user)
+ return collection.getMergeProposals(
+ status, prerequisite_repository=self.repository,
+ prerequisite_path=self.path, eager_load=eager_load)
+
def getMergeProposalByID(self, id):
"""See `IGitRef`."""
return self.landing_targets.find(BranchMergeProposal.id == id).one()
=== modified file 'lib/lp/code/model/tests/test_branch.py'
--- lib/lp/code/model/tests/test_branch.py 2015-09-29 00:17:48 +0000
+++ lib/lp/code/model/tests/test_branch.py 2015-10-12 13:06:46 +0000
@@ -698,6 +698,20 @@
[product.development_focus],
list(branch.associatedProductSeries()))
+ def test_getMergeProposals(self):
+ target_branch = self.factory.makeProductBranch()
+ bmp = self.factory.makeBranchMergeProposal(target_branch=target_branch)
+ self.factory.makeBranchMergeProposal()
+ self.assertEqual([bmp], list(target_branch.getMergeProposals()))
+
+ def test_getDependentMergeProposals(self):
+ prerequisite_branch = self.factory.makeProductBranch()
+ bmp = self.factory.makeBranchMergeProposal(
+ prerequisite_branch=prerequisite_branch)
+ self.factory.makeBranchMergeProposal()
+ self.assertEqual(
+ [bmp], list(prerequisite_branch.getDependentMergeProposals()))
+
class TestBranchUpgrade(TestCaseWithFactory):
"""Test the upgrade functionalities of branches."""
=== modified file 'lib/lp/code/model/tests/test_branchcollection.py'
--- lib/lp/code/model/tests/test_branchcollection.py 2015-04-22 12:03:05 +0000
+++ lib/lp/code/model/tests/test_branchcollection.py 2015-10-12 13:06:46 +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).
"""Tests for branch collections."""
@@ -1052,6 +1052,17 @@
target_branch=mp1.target_branch)
self.assertEqual([mp1], list(proposals))
+ def test_specifying_prerequisite_branch(self):
+ # If the prerequisite_branch is specified, only merge proposals
+ # where that branch is the prerequisite are returned.
+ prerequisite = self.factory.makeProductBranch()
+ mp1 = self.factory.makeBranchMergeProposal(
+ prerequisite_branch=prerequisite)
+ self.factory.makeBranchMergeProposal()
+ proposals = self.all_branches.getMergeProposals(
+ prerequisite_branch=prerequisite)
+ self.assertEqual([mp1], list(proposals))
+
class TestBranchMergeProposalsForReviewer(TestCaseWithFactory):
"""Tests for IBranchCollection.getProposalsForReviewer()."""
=== modified file 'lib/lp/code/model/tests/test_gitcollection.py'
--- lib/lp/code/model/tests/test_gitcollection.py 2015-05-14 13:57:51 +0000
+++ lib/lp/code/model/tests/test_gitcollection.py 2015-10-12 13:06:46 +0000
@@ -12,11 +12,11 @@
from operator import attrgetter
import pytz
-from testtools.matchers import Equals
from storm.store import (
EmptyResultSet,
Store,
)
+from testtools.matchers import Equals
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
@@ -598,6 +598,32 @@
target_path=mp1.target_git_path)
self.assertEqual([mp1], list(proposals))
+ def test_specifying_prerequisite_repository(self):
+ # If the prerequisite_repository is specified but not the
+ # prerequisite_path, only merge proposals where that repository is
+ # the prerequisite are returned.
+ [ref1, ref2] = self.factory.makeGitRefs(
+ paths=[u"refs/heads/ref1", u"refs/heads/ref2"])
+ mp1 = self.factory.makeBranchMergeProposalForGit(prerequisite_ref=ref1)
+ mp2 = self.factory.makeBranchMergeProposalForGit(prerequisite_ref=ref2)
+ self.factory.makeBranchMergeProposalForGit()
+ proposals = self.all_repositories.getMergeProposals(
+ prerequisite_repository=ref1.repository)
+ self.assertEqual(sorted([mp1, mp2]), sorted(proposals))
+
+ def test_specifying_prerequisite_ref(self):
+ # If the prerequisite_repository and prerequisite_path are
+ # specified, only merge proposals where that ref is the prerequisite
+ # are returned.
+ [prerequisite] = self.factory.makeGitRefs()
+ mp1 = self.factory.makeBranchMergeProposalForGit(
+ prerequisite_ref=prerequisite)
+ self.factory.makeBranchMergeProposalForGit()
+ proposals = self.all_repositories.getMergeProposals(
+ prerequisite_repository=prerequisite.repository,
+ prerequisite_path=prerequisite.path)
+ self.assertEqual([mp1], list(proposals))
+
class TestBranchMergeProposalsForReviewer(TestCaseWithFactory):
"""Tests for IGitCollection.getProposalsForReviewer()."""
=== modified file 'lib/lp/code/model/tests/test_gitref.py'
--- lib/lp/code/model/tests/test_gitref.py 2015-06-05 11:05:03 +0000
+++ lib/lp/code/model/tests/test_gitref.py 2015-10-12 13:06:46 +0000
@@ -39,8 +39,17 @@
def test_getMergeProposals(self):
[target_ref] = self.factory.makeGitRefs()
bmp = self.factory.makeBranchMergeProposalForGit(target_ref=target_ref)
+ self.factory.makeBranchMergeProposalForGit()
self.assertEqual([bmp], list(target_ref.getMergeProposals()))
+ def test_getDependentMergeProposals(self):
+ [prerequisite_ref] = self.factory.makeGitRefs()
+ bmp = self.factory.makeBranchMergeProposalForGit(
+ prerequisite_ref=prerequisite_ref)
+ self.factory.makeBranchMergeProposalForGit()
+ self.assertEqual(
+ [bmp], list(prerequisite_ref.getDependentMergeProposals()))
+
def test_implements_IInformationType(self):
[ref] = self.factory.makeGitRefs()
verifyObject(IInformationType, ref)
=== modified file 'lib/lp/code/templates/branch-pending-merges.pt'
--- lib/lp/code/templates/branch-pending-merges.pt 2015-04-19 12:56:32 +0000
+++ lib/lp/code/templates/branch-pending-merges.pt 2015-10-12 13:06:46 +0000
@@ -29,7 +29,7 @@
<div id="dependent-branches" tal:condition="context/dependent_branches">
<img src="/@@/merge-proposal-icon" />
- <a href="+merges" tal:content="structure view/dependent_branch_count_text">
+ <a href="+dependent-merges" tal:content="structure view/dependent_branch_count_text">
1 branch
</a>
dependent on this one.
=== modified file 'lib/lp/code/templates/gitref-pending-merges.pt'
--- lib/lp/code/templates/gitref-pending-merges.pt 2015-04-28 16:48:22 +0000
+++ lib/lp/code/templates/gitref-pending-merges.pt 2015-10-12 13:06:46 +0000
@@ -24,7 +24,7 @@
<div id="dependent-landings" tal:condition="view/dependent_landings">
<img src="/@@/merge-proposal-icon" />
- <a href="+merges" tal:content="structure view/dependent_landing_count_text">
+ <a href="+dependent-merges" tal:content="structure view/dependent_landing_count_text">
1 branch
</a>
dependent on this one.
Follow ups
