Merge lp:~stevenk/launchpad/destroy-simplified-branch-ff into lp:launchpad

[Steve Kowalik]

The code.simplified_branch_menu.enabled feature flag has been on by default for a while. Destroy it, and clean up afterwards.

[Curtis Hovey (sinzui)]

Thank you.

review: Approve (code)

=== modified file 'lib/lp/code/browser/branchlisting.py'
--- lib/lp/code/browser/branchlisting.py	2012-10-03 03:07:36 +0000
+++ lib/lp/code/browser/branchlisting.py	2012-10-04 01:12:22 +0000
@@ -71,11 +71,7 @@
     )
 from lp.bugs.interfaces.bugbranch import IBugBranchSet
 from lp.code.browser.branch import BranchMirrorMixin
-from lp.code.browser.branchmergeproposallisting import (
-    ActiveReviewsView,
-    PersonActiveReviewsView,
-    PersonProductActiveReviewsView,
-    )
+from lp.code.browser.branchmergeproposallisting import ActiveReviewsView
 from lp.code.browser.branchmergequeuelisting import HasMergeQueuesMenuMixin
 from lp.code.browser.branchvisibilitypolicy import BranchVisibilityPolicyMixin
 from lp.code.browser.summary import BranchCountSummaryView
@@ -117,7 +113,6 @@
 from lp.registry.model.sourcepackage import SourcePackage
 from lp.services.browser_helpers import get_plural_text
 from lp.services.config import config
-from lp.services.features import getFeatureFlag
 from lp.services.feeds.browser import (
     FeedsMixin,
     PersonBranchesFeedLink,
@@ -161,7 +156,7 @@
             return Badge('/@@/warning', '/@@/warning-large', '',
                          'Branch has errors')
         else:
-            return HasBadgeBase.getBadge(self, badge_name)
+            return super(BranchBadges, self).getBadge(badge_name)
 
 
 class BranchListingItem(BzrIdentityMixin, BranchBadges):
@@ -560,9 +555,7 @@
 
     @property
     def initial_values(self):
-        return {
-            'lifecycle': BranchLifecycleStatusFilter.CURRENT,
-            }
+        return {'lifecycle': BranchLifecycleStatusFilter.CURRENT}
 
     @cachedproperty
     def selected_lifecycle_status(self):
@@ -856,30 +849,8 @@
     usedfor = IPerson
     facet = 'branches'
     links = ['registered', 'owned', 'subscribed',
-             'active_reviews', 'mergequeues', 'source_package_recipes',
-             'simplified_subscribed', 'simplified_registered',
-             'simplified_owned', 'simplified_active_reviews']
-    extra_attributes = [
-        'active_review_count',
-        'owned_branch_count',
-        'registered_branch_count',
-        'show_summary',
-        'subscribed_branch_count',
-        'mergequeue_count',
-        'simplified_branches_menu',
-        ]
-
-    def _getCountCollection(self):
-        """The base collection of branches which should be counted.
-
-        This collection will be further restricted to, e.g., the
-        branches registered by a particular user for the counts that
-        appear at the top of a branch listing page.
-
-        This should be overridden in subclasses to restrict to, for
-        example, the set of branches of a particular product.
-        """
-        return getUtility(IAllBranches).visibleByUser(self.user)
+             'active_reviews', 'mergequeues', 'source_package_recipes']
+    extra_attributes = ['mergequeue_count']
 
     @property
     def person(self):
@@ -890,139 +861,38 @@
         """
         return self.context
 
-    @property
-    def show_summary(self):
-        """Should the template show the summary view with the links."""
-
-        if self.simplified_branches_menu:
-            return True
-        else:
-            return (
-                self.owned_branch_count or
-                self.registered_branch_count or
-                self.subscribed_branch_count or
-                self.active_review_count
-                )
-
-    @cachedproperty
-    def simplified_branches_menu(self):
-        return getFeatureFlag('code.simplified_branches_menu.enabled')
-
-    @cachedproperty
-    def registered_branches_not_empty(self):
-        """False if the number of branches registered by self.person
-        is zero.
-        """
-        return (
-            not self._getCountCollection().registeredBy(
-                self.person).is_empty())
-
-    def simplified_owned(self):
-        return Link(
-            canonical_url(self.context, rootsite='code'),
-            'Owned branches')
-
-    def simplified_registered(self):
-        person_is_individual = (not self.person.is_team)
-        return Link(
-            '+registeredbranches',
-            'Registered branches',
-            enabled=(
-                person_is_individual and
-                self.registered_branches_not_empty))
-
-    def simplified_subscribed(self):
-        return Link(
-            '+subscribedbranches',
-            'Subscribed branches')
-
-    def simplified_active_reviews(self):
-        return Link(
-            '+activereviews',
-            'Active reviews')
-
-    def source_package_recipes(self):
-        return Link(
-            '+recipes',
-            'Source package recipes',
-            enabled=IPerson.providedBy(self.context))
-
-    @cachedproperty
-    def registered_branch_count(self):
-        """Return the number of branches registered by self.person."""
-        return self._getCountCollection().registeredBy(self.person).count()
-
-    @cachedproperty
-    def owned_branch_count(self):
-        """Return the number of branches owned by self.person."""
-        return self._getCountCollection().ownedBy(self.person).count()
-
-    @cachedproperty
-    def subscribed_branch_count(self):
-        """Return the number of branches subscribed to by self.person."""
-        return self._getCountCollection().subscribedBy(self.person).count()
-
     def owned(self):
         return Link(
-            canonical_url(self.context, rootsite='code'),
-            get_plural_text(
-                self.owned_branch_count, 'owned branch', 'owned branches'))
+            canonical_url(self.context, rootsite='code'), 'Owned branches')
 
     def registered(self):
-        person_is_individual = (not self.person.is_team)
+        enabled = not self.person.is_team
         return Link(
-            '+registeredbranches',
-            get_plural_text(
-                self.registered_branch_count,
-                'registered branch', 'registered branches'),
-            enabled=person_is_individual)
+            '+registeredbranches', 'Registered branches', enabled=enabled)
 
     def subscribed(self):
+        return Link('+subscribedbranches', 'Subscribed branches')
+
+    def active_reviews(self):
+        return Link('+activereviews', 'Active reviews')
+
+    def source_package_recipes(self):
         return Link(
-            '+subscribedbranches',
-            get_plural_text(
-                self.subscribed_branch_count,
-                'subscribed branch', 'subscribed branches'))
-
-    @cachedproperty
-    def active_review_count(self):
-        """Return the number of active reviews for self.person's branches."""
-        active_reviews = PersonActiveReviewsView(self.context, self.request)
-        return active_reviews.getProposals().count()
-
-    def active_reviews(self):
-        text = get_plural_text(
-            self.active_review_count,
-            'active review',
-            'active reviews')
-        return Link('+activereviews', text)
+            '+recipes', 'Source package recipes',
+            enabled=IPerson.providedBy(self.context))
 
 
 class PersonProductBranchesMenu(PersonBranchesMenu):
 
     usedfor = IPersonProduct
     links = ['registered', 'owned', 'subscribed', 'active_reviews',
-             'source_package_recipes',
-             'simplified_subscribed', 'simplified_registered',
-             'simplified_owned', 'simplified_active_reviews']
-
-    def _getCountCollection(self):
-        """See `PersonBranchesMenu`."""
-        collection = getUtility(IAllBranches).visibleByUser(self.user)
-        return collection.inProduct(self.context.product)
+             'source_package_recipes']
 
     @property
     def person(self):
         """See `PersonBranchesMenu`."""
         return self.context.person
 
-    @cachedproperty
-    def active_review_count(self):
-        """Return the number of active reviews for self.person's branches."""
-        active_reviews = PersonProductActiveReviewsView(
-            self.context, self.request)
-        return active_reviews.getProposals().count()
-
 
 class PersonBaseBranchListingView(BranchListingView):
     """Base class used for different person listing views."""
 
=== modified file 'lib/lp/code/browser/tests/test_branchlisting.py'
--- lib/lp/code/browser/tests/test_branchlisting.py	2012-09-06 00:01:38 +0000
+++ lib/lp/code/browser/tests/test_branchlisting.py	2012-10-04 01:12:22 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for branch listing."""
@@ -265,10 +265,6 @@
         self._test_batch_template(self.barney)
 
 
-SIMPLIFIED_BRANCHES_MENU_FLAG = {
-    'code.simplified_branches_menu.enabled': 'on'}
-
-
 class TestSimplifiedPersonBranchesView(TestCaseWithFactory):
 
     layer = LaunchpadFunctionalLayer
@@ -294,19 +290,16 @@
     def get_branch_list_page(self, target=None, page_name='+branches'):
         if target is None:
             target = self.default_target
-        with FeatureFixture(SIMPLIFIED_BRANCHES_MENU_FLAG):
-            with person_logged_in(self.user):
-                return create_initialized_view(
-                    target, page_name, rootsite='code',
-                    principal=self.user)()
+        with person_logged_in(self.user):
+            return create_initialized_view(
+                target, page_name, rootsite='code', principal=self.user)()
 
     def test_branch_list_h1(self):
         self.makeABranch()
         page = self.get_branch_list_page()
         h1_matcher = soupmatchers.HTMLContains(
             soupmatchers.Tag(
-                'Title', 'h1',
-                text='Bazaar branches owned by Barney'))
+                'Title', 'h1', text='Bazaar branches owned by Barney'))
         self.assertThat(page, h1_matcher)
 
     def test_branch_list_empty(self):
@@ -317,7 +310,6 @@
                 text='There are no branches related to Barney '
                      'in Launchpad today.'))
         self.assertThat(page, empty_message_matcher)
-        self.assertThat(page, Not(self.registered_branches_matcher))
 
     def test_branch_list_registered_link(self):
         self.makeABranch()
@@ -408,7 +400,6 @@
                 text='There are no branches of Bambam owned by Barney '
                      'in Launchpad today.'))
         self.assertThat(page, empty_message_matcher)
-        self.assertThat(page, Not(self.registered_branches_matcher))
 
 
 class TestSourcePackageBranchesView(TestCaseWithFactory):
 
=== modified file 'lib/lp/code/stories/branches/xx-branch-listings.txt'
--- lib/lp/code/stories/branches/xx-branch-listings.txt	2012-07-17 14:29:17 +0000
+++ lib/lp/code/stories/branches/xx-branch-listings.txt	2012-10-04 01:12:22 +0000
@@ -65,7 +65,7 @@
 needed, then the table is sortable and no batching navigation links are shown.
 
     >>> browser.open('http://code.launchpad.dev/~name12')
-    >>> browser.getLink('subscribed').click()
+    >>> browser.getLink('Subscribed').click()
     >>> links = find_tag_by_id(browser.contents, 'branch-batch-links')
     >>> links is None
     True
 
=== modified file 'lib/lp/code/stories/branches/xx-branchmergeproposal-listings.txt'
--- lib/lp/code/stories/branches/xx-branchmergeproposal-listings.txt	2012-08-21 11:01:47 +0000
+++ lib/lp/code/stories/branches/xx-branchmergeproposal-listings.txt	2012-10-04 01:12:22 +0000
@@ -93,10 +93,11 @@
 
     >>> browser.open('http://code.launchpad.dev/~albert')
     >>> print_tag_with_id(browser.contents, 'portlet-person-codesummary')
-    1 owned branch
-    1 registered branch
-    1 subscribed branch
-    1 active review
+    Owned branches
+    Registered branches
+    Subscribed branches
+    Active reviews
+    Source package recipes
 
 The person's active reviews also lists all of their currently requested
 reviews.
@@ -128,7 +129,7 @@
 Since Albert is in the A-Team, he can do the pending review.
 
     >>> browser.open('http://code.launchpad.dev/~a-team')
-    >>> browser.getLink('active review').click()
+    >>> browser.getLink('Active reviews').click()
     >>> print_tag_with_id(browser.contents, 'proposals')
     Requested reviews I can do
     Branch Merge Proposal             Requested By    Lines Activity
 
=== modified file 'lib/lp/code/stories/branches/xx-person-branches.txt'
--- lib/lp/code/stories/branches/xx-person-branches.txt	2012-01-15 13:32:27 +0000
+++ lib/lp/code/stories/branches/xx-person-branches.txt	2012-10-04 01:12:22 +0000
@@ -64,7 +64,7 @@
 There is also a link which points to the registered branches page.
 
     >>> browser.open('http://code.launchpad.dev/~name12')
-    >>> browser.getLink('registered').click()
+    >>> browser.getLink('Registered').click()
     >>> print browser.title
     Registered : Code : Sample Person
     >>> table = find_tag_by_id(browser.contents, 'branchtable')
@@ -81,7 +81,7 @@
 branches.
 
     >>> browser.open('http://code.launchpad.dev/~name12')
-    >>> browser.getLink('subscribed').click()
+    >>> browser.getLink('Subscribed').click()
     >>> print browser.title
     Subscribed : Code : Sample Person
     >>> table = find_tag_by_id(browser.contents, 'branchtable')
@@ -112,10 +112,11 @@
     >>> eric_browser = setupBrowser(auth="Basic [e-mail]:test")
     >>> eric_browser.open('http://code.launchpad.dev/~eric')
     >>> print_tag_with_id(eric_browser.contents, 'portlet-person-codesummary')
-    1 owned branch
-    1 registered branch
-    1 subscribed branch
-    0 active reviews
+    Owned branches
+    Registered branches
+    Subscribed branches
+    Active reviews
+    Source package recipes
 
 Now we'll create another branch, and unsubscribe the owner from it.
 
@@ -127,11 +128,11 @@
     >>> eric_browser.open('http://code.launchpad.dev/~eric')
     >>> print_tag_with_id(
     ...     eric_browser.contents, 'portlet-person-codesummary')
-    2 owned branches
-    2 registered branches
-    1 subscribed branch
-    0 active reviews
-
+    Owned branches
+    Registered branches
+    Subscribed branches
+    Active reviews
+    Source package recipes
 
 Teams do not show registered branches
 -------------------------------------
@@ -142,9 +143,10 @@
     >>> browser.open('http://code.launchpad.dev/~landscape-developers')
     >>> print_tag_with_id(
     ...     browser.contents, 'portlet-person-codesummary')
-    1 owned branch
-    2 subscribed branches
-    0 active reviews
+    Owned branches
+    Subscribed branches
+    Active reviews
+    Source package recipes
 
     >>> browser.getLink('registered').click()
     Traceback (most recent call last):
 
=== modified file 'lib/lp/code/stories/branches/xx-personproduct-branch-listings.txt'
--- lib/lp/code/stories/branches/xx-personproduct-branch-listings.txt	2010-10-15 16:41:50 +0000
+++ lib/lp/code/stories/branches/xx-personproduct-branch-listings.txt	2012-10-04 01:12:22 +0000
@@ -39,7 +39,7 @@
 
     >>> browser.open('http://code.launchpad.dev/~eric/fooix')
     >>> print_tag_with_id(browser.contents, 'portlet-person-codesummary')
-    2 owned branches ...
+    Owned branches ...
     >>> print_tag_with_id(browser.contents, 'branchtable')
     Name                         ...
     lp://dev/~eric/fooix/feature ...
 
=== modified file 'lib/lp/code/templates/person-codesummary.pt'
--- lib/lp/code/templates/person-codesummary.pt	2012-02-21 12:11:11 +0000
+++ lib/lp/code/templates/person-codesummary.pt	2012-10-04 01:12:22 +0000
@@ -4,62 +4,28 @@
   xmlns:i18n="http://xml.zope.org/namespaces/i18n"
   id="portlet-person-codesummary"
   class="portlet"
-  tal:define="menu context/menu:branches;
-      features request/features"
-  tal:condition="menu/show_summary">
+  tal:define="menu context/menu:branches; features request/features">
 
-  <table tal:condition="not: menu/simplified_branches_menu">
-    <tr class="code-links">
-      <td class="code-count" tal:content="menu/owned_branch_count">100</td>
-      <td tal:content="structure menu/owned/render"
-          />
+  <table>
+    <tr class="code-links" tal:condition="menu/owned/enabled">
+      <td tal:content="structure menu/owned/render" />
     </tr>
-    <tr class="code-links"
-        tal:condition="menu/registered/enabled">
-      <td class="code-count" tal:content="menu/registered_branch_count">100</td>
+    <tr class="code-links" tal:condition="menu/registered/enabled">
       <td tal:content="structure menu/registered/render" />
     </tr>
-    <tr class="code-links">
-      <td class="code-count" tal:content="menu/subscribed_branch_count">100</td>
+    <tr class="code-links" tal:condition="menu/subscribed/enabled">
       <td tal:content="structure menu/subscribed/render" />
     </tr>
-    <tr class="code-links" id="merge-counts">
-      <td class="code-count" tal:content="menu/active_review_count">5</td>
+    <tr class="code-links"
+        tal:condition="menu/active_reviews/enabled">
       <td tal:content="structure menu/active_reviews/render" />
     </tr>
     <tr tal:condition="features/code.branchmergequeue" id="mergequeue-counts">
       <td class="code-count" tal:content="menu/mergequeue_count">5</td>
       <td tal:condition="menu"
-          tal:content="structure menu/mergequeues/render"
-          />
-    </tr>
-  </table>
-
-  <table tal:condition="menu/simplified_branches_menu">
-    <tr class="code-links"
-        tal:condition="menu/simplified_owned/enabled">
-      <td tal:content="structure menu/simplified_owned/render" />
-    </tr>
-    <tr class="code-links"
-        tal:condition="menu/simplified_registered/enabled">
-      <td tal:content="structure menu/simplified_registered/render" />
-    </tr>
-    <tr class="code-links"
-        tal:condition="menu/simplified_subscribed/enabled">
-      <td tal:content="structure menu/simplified_subscribed/render" />
-    </tr>
-    <tr class="code-links"
-        tal:condition="menu/simplified_active_reviews/enabled">
-      <td tal:content="structure menu/simplified_active_reviews/render" />
-    </tr>
-    <tr tal:condition="features/code.branchmergequeue" id="mergequeue-counts">
-      <td class="code-count" tal:content="menu/mergequeue_count">5</td>
-      <td tal:condition="menu"
-          tal:content="structure menu/mergequeues/render"
-          />
-    </tr>
-    <tr class="code-links"
-        tal:condition="menu/source_package_recipes/enabled">
+          tal:content="structure menu/mergequeues/render" />
+    </tr>
+    <tr class="code-links" tal:condition="menu/source_package_recipes/enabled">
       <td tal:content="structure menu/source_package_recipes/render" />
     </tr>
   </table>
 
=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py	2012-10-02 06:36:44 +0000
+++ lib/lp/services/features/flags.py	2012-10-04 01:12:22 +0000
@@ -106,12 +106,6 @@
      '',
      '',
      ''),
-    ('code.simplified_branches_menu.enabled',
-     'boolean',
-     ('Display a simplified version of the branch menu (omit the counts).'),
-     '',
-     '',
-     ''),
     ('hard_timeout',
      'float',
      'Sets the hard request timeout in milliseconds.',