launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/launchpad/branches-timeout-bug-827935-always-display into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #827935 in Launchpad itself: "Person:+branches timeouts"
  https://bugs.launchpad.net/launchpad/+bug/827935

For more details, see:
https://code.launchpad.net/~rvb/launchpad/branches-timeout-bug-827935-always-display/+merge/81022

This is a followup on https://code.launchpad.net/~rvb/launchpad/branches-timeout-bug-827935/+merge/80875

This branch modify the new "simplified" branch menu in such a way that the links to the "Owned branches" and to the "Registered branches" pages are always displayed.  The reason behind this change is that checking if there is any "owned branch" or "registered branch" (to display or not the links) is expensive if there is a large number of public branches. (this takes 2.6sec on qastaging).

= Tests =

./bin/test -vvc test_branchlisting test_branch_list_

= FF =

All of this new code is still behind a FF (code.simplified_branches_menu.enabled).

-- 
https://code.launchpad.net/~rvb/launchpad/branches-timeout-bug-827935-always-display/+merge/81022
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/branches-timeout-bug-827935-always-display into lp:launchpad.

=== modified file 'lib/lp/code/browser/branchlisting.py'
--- lib/lp/code/browser/branchlisting.py	2011-11-01 14:05:19 +0000
+++ lib/lp/code/browser/branchlisting.py	2011-11-02 14:12:31 +0000
@@ -894,12 +894,7 @@
         """Should the template show the summary view with the links."""
 
         if self.simplified_branches_menu:
-            return (
-                self.registered_branches_not_empty or
-                self.owned_branches_not_empty or
-                self.subscribed_branches_not_empty or
-                self.active_reviews_not_empty
-                )
+            return True
         else:
             return (
                 self.owned_branch_count or
@@ -922,20 +917,6 @@
                 self.person).is_empty())
 
     @cachedproperty
-    def owned_branches_not_empty(self):
-        """False if the number of branches owned by self.person is zero."""
-        return not self._getCountCollection().ownedBy(self.person).is_empty()
-
-    @cachedproperty
-    def subscribed_branches_not_empty(self):
-        """False if the number of branches subscribed to by self.person
-        is zero.
-        """
-        return (
-            not self._getCountCollection().subscribedBy(
-                self.person).is_empty())
-
-    @cachedproperty
     def active_reviews_not_empty(self):
         """Return the number of active reviews for self.person's branches."""
         active_reviews = PersonActiveReviewsView(self.context, self.request)
@@ -944,8 +925,7 @@
     def simplified_owned(self):
         return Link(
             canonical_url(self.context, rootsite='code'),
-            'Owned branches',
-            enabled=self.owned_branches_not_empty)
+            'Owned branches')
 
     def simplified_registered(self):
         person_is_individual = (not self.person.is_team)
@@ -959,8 +939,7 @@
     def simplified_subscribed(self):
         return Link(
             '+subscribedbranches',
-            'Subscribed branches',
-            enabled=self.subscribed_branches_not_empty)
+            'Subscribed branches')
 
     def simplified_active_reviews(self):
         return Link(

=== modified file 'lib/lp/code/browser/tests/test_branchlisting.py'
--- lib/lp/code/browser/tests/test_branchlisting.py	2011-11-01 14:07:25 +0000
+++ lib/lp/code/browser/tests/test_branchlisting.py	2011-11-02 14:12:31 +0000
@@ -317,21 +317,21 @@
         self.assertThat(page, self.registered_branches_matcher)
 
     def test_branch_list_owned_link(self):
+        # The link to the owned branches is always displayed.
         owned_branches_matcher = soupmatchers.HTMLContains(
             soupmatchers.Tag(
                 'Owned link', 'a', text='Owned branches',
                 attrs={'href': 'http://code.launchpad.dev/~barney'}))
-        self.factory.makeAnyBranch(owner=self.person)
         page = self.get_branch_list_page('+subscribedbranches')
         self.assertThat(page, owned_branches_matcher)
 
     def test_branch_list_subscribed_link(self):
+        # The link to the subscribed branches is always displayed.
         subscribed_branches_matcher = soupmatchers.HTMLContains(
             soupmatchers.Tag(
                 'Subscribed link', 'a', text='Subscribed branches',
                 attrs={'href': 'http://launchpad.dev/~barney'
                                '/+subscribedbranches'}))
-        self.factory.makeAnyBranch(owner=self.person)
         page = self.get_branch_list_page()
         self.assertThat(page, subscribed_branches_matcher)