launchpad-reviewers team mailing list archive

[Benji York <benji@xxxxxxxxxxxxx>]

Benji York has proposed merging lp:~benji/launchpad/bug-798945 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #798945 in Launchpad itself: "KeyError: 'product' raised browsing +subscribedbranches page"
  https://bugs.launchpad.net/launchpad/+bug/798945

For more details, see:
https://code.launchpad.net/~benji/launchpad/bug-798945/+merge/70909

This is a fix for bug 798945.  The breadcrumb machinery reads page_title which was early-bound to the label property, but the label property was overridden in a subclass.  That meant that page_title still referred to the label property from the superclass.  This branch just changes page_title from a class attribute to a property so the correct label property is invoked (plus tests).

Test: bin/test -c -m lp.code.browser.tests.test_branchlisting -t TestPageTitle

Lint: I fixed some pre-existing lint.

QA: visit one of the links in the bug and make sure they don't OOPS.
-- 
https://code.launchpad.net/~benji/launchpad/bug-798945/+merge/70909
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~benji/launchpad/bug-798945 into lp:launchpad.

=== modified file 'lib/lp/code/browser/branchlisting.py'
--- lib/lp/code/browser/branchlisting.py	2011-07-21 02:42:17 +0000
+++ lib/lp/code/browser/branchlisting.py	2011-08-09 16:01:39 +0000
@@ -30,7 +30,6 @@
     'SourcePackageBranchesView',
     ]
 
-from datetime import datetime
 from operator import attrgetter
 
 from lazr.delegates import delegates
@@ -38,8 +37,6 @@
     EnumeratedType,
     Item,
     )
-import pytz
-import simplejson
 from storm.expr import (
     Asc,
     Desc,
@@ -557,9 +554,11 @@
             'displayname': self.context.displayname,
             'title': getattr(self.context, 'title', 'no-title')}
 
-    # Provide a default page_title for distros and other things without
-    # breadcrumbs..
-    page_title = label
+    @property
+    def page_title(self):
+        """Provide a default for distros and other things without breadcrumbs.
+        """
+        return self.label
 
     @property
     def initial_values(self):

=== modified file 'lib/lp/code/browser/tests/test_branchlisting.py'
--- lib/lp/code/browser/tests/test_branchlisting.py	2011-08-03 11:00:11 +0000
+++ lib/lp/code/browser/tests/test_branchlisting.py	2011-08-09 16:01:39 +0000
@@ -27,6 +27,7 @@
     BranchListingSort,
     BranchListingView,
     GroupedDistributionSourcePackageBranchesView,
+    PersonProductSubscribedBranchesView,
     SourcePackageBranchesView,
     )
 from lp.code.enums import BranchVisibilityRule
@@ -525,3 +526,29 @@
             self.project, name='+branches', rootsite='code')
         table = find_tag_by_id(view(), "branchtable")
         self.assertIsNot(None, table)
+
+
+class FauxPageTitleContext:
+
+    displayname = 'DISPLAY-NAME'
+
+    class person:
+        displayname = 'PERSON'
+
+    class product:
+        displayname = 'PRODUCT'
+
+
+class TestPageTitle(TestCase):
+    """The various views should have a page_title attribute/property."""
+
+    def test_branch_listing_view(self):
+        view = BranchListingView(FauxPageTitleContext, None)
+        self.assertEqual(
+            'Bazaar branches for DISPLAY-NAME', view.page_title)
+
+    def test_person_product_subscribed_branches_view(self):
+        view = PersonProductSubscribedBranchesView(FauxPageTitleContext, None)
+        self.assertEqual(
+            'Bazaar Branches of PRODUCT subscribed to by PERSON',
+            view.page_title)