launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/dependent-merges-label into lp:launchpad.

Commit message:
Improve the page_title and label of +dependent-merges.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/dependent-merges-label/+merge/274387

Improve the page_title and label of +dependent-merges.  It now says "Dependent merge proposals" rather than "Merge proposals" in the breadcrumb, and "Merge proposals dependent on <branch>" rather than "Merge proposals for <branch>" in the heading.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/dependent-merges-label into lp:launchpad.

=== modified file 'lib/lp/code/browser/branchmergeproposallisting.py'
--- lib/lp/code/browser/branchmergeproposallisting.py	2015-10-12 12:58:32 +0000
+++ lib/lp/code/browser/branchmergeproposallisting.py	2015-10-14 13:04:02 +0000
@@ -268,6 +268,12 @@
 class BranchDependentMergesView(BranchMergeProposalListingView):
     """Branch merge proposals that list this branch as a prerequisite."""
 
+    page_title = 'Dependent merge proposals'
+
+    @property
+    def label(self):
+        return "Merge proposals dependent on %s" % self.context.displayname
+
     def getVisibleProposalsForUser(self):
         """See `BranchMergeProposalListingView`."""
         return self.context.getDependentMergeProposals(

=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposallisting.py'
--- lib/lp/code/browser/tests/test_branchmergeproposallisting.py	2015-10-14 09:51:18 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposallisting.py	2015-10-14 13:04:02 +0000
@@ -10,10 +10,7 @@
 import pytz
 from testtools.content import Content
 from testtools.content_type import UTF8_TEXT
-from testtools.matchers import (
-    Equals,
-    LessThan,
-    )
+from testtools.matchers import LessThan
 import transaction
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
@@ -274,6 +271,7 @@
     supports_privacy = True
     supports_git = True
     supports_bzr = True
+    label_describes_context = True
 
     bzr_branch = None
     git_ref = None
@@ -306,6 +304,12 @@
             source_ref=source, target_ref=target,
             set_state=BranchMergeProposalStatus.NEEDS_REVIEW)
 
+    def getExpectedLabel(self):
+        if self.label_describes_context:
+            return "%s for %s" % (self.page_title, self.context.displayname)
+        else:
+            return self.page_title
+
     def test_bzr(self):
         """The merges view should be enabled for the target."""
         if not self.supports_bzr:
@@ -313,8 +317,10 @@
         with admin_logged_in():
             bmp = self.makeBzrMergeProposal()
             url = canonical_url(bmp, force_local_path=True)
+            label = self.getExpectedLabel()
         browser = self.getViewBrowser(
             self.context, self.view_name, rootsite='code', user=self.user)
+        self.assertIn(label, browser.contents)
         self.assertIn(url, browser.contents)
 
     def test_git(self):
@@ -324,8 +330,10 @@
         with admin_logged_in():
             bmp = self.makeGitMergeProposal()
             url = canonical_url(bmp, force_local_path=True)
+            label = self.getExpectedLabel()
         browser = self.getViewBrowser(
             self.context, self.view_name, rootsite='code', user=self.user)
+        self.assertIn(label, browser.contents)
         self.assertIn(url, browser.contents)
 
     def test_query_count_bzr(self):
@@ -356,6 +364,7 @@
 class MergesTestMixin(BranchMergeProposalListingTestMixin):
 
     view_name = '+merges'
+    page_title = 'Merge proposals'
 
     def test_none(self):
         """The merges view should be enabled for the target."""
@@ -367,6 +376,7 @@
 class DependentMergesTestMixin(BranchMergeProposalListingTestMixin):
 
     view_name = '+dependent-merges'
+    page_title = 'Dependent merge proposals'
 
     def makeBzrMergeProposal(self):
         information_type = (
@@ -402,6 +412,9 @@
             prerequisite_ref=prerequisite,
             set_state=BranchMergeProposalStatus.NEEDS_REVIEW)
 
+    def getExpectedLabel(self):
+        return "Merge proposals dependent on %s" % self.context.displayname
+
     def test_none(self):
         """The dependent merges view should be enabled for the target."""
         browser = self.getViewBrowser(
@@ -412,6 +425,7 @@
 class ActiveReviewsTestMixin(BranchMergeProposalListingTestMixin):
 
     view_name = '+activereviews'
+    page_title = 'Active reviews'
 
     def test_none(self):
         """The active reviews view should be enabled for the target."""
@@ -422,6 +436,8 @@
 
 class ProductContextMixin:
 
+    label_describes_context = False
+
     def setUp(self):
         super(ProductContextMixin, self).setUp()
         self.git_target = self.bzr_target = self.context = (
@@ -432,6 +448,8 @@
 
 class ProjectGroupContextMixin:
 
+    label_describes_context = False
+
     def setUp(self):
         super(ProjectGroupContextMixin, self).setUp()
         self.context = self.factory.makeProject()
@@ -445,6 +463,7 @@
 
     # Distribution branches don't have access_policy set.
     supports_privacy = False
+    label_describes_context = False
 
     def setUp(self):
         super(DistributionSourcePackageContextMixin, self).setUp()
@@ -478,6 +497,8 @@
 
 class PersonContextMixin:
 
+    label_describes_context = False
+
     def setUp(self):
         super(PersonContextMixin, self).setUp()
         self.context = self.factory.makePerson()
@@ -488,6 +509,8 @@
 
 class PersonProductContextMixin:
 
+    label_describes_context = False
+
     def setUp(self):
         super(PersonProductContextMixin, self).setUp()
         self.context = PersonProduct(