launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/destroy-simplified-branch-ff into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/destroy-simplified-branch-ff/+merge/127630

The code.simplified_branch_menu.enabled feature flag has been on by default for a while. Destroy it, and clean up afterwards.
-- 
https://code.launchpad.net/~stevenk/launchpad/destroy-simplified-branch-ff/+merge/127630
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/destroy-simplified-branch-ff into lp:launchpad.

=== modified file 'lib/lp/code/browser/branchlisting.py'
--- lib/lp/code/browser/branchlisting.py	2012-09-28 06:15:58 +0000
+++ lib/lp/code/browser/branchlisting.py	2012-10-03 03:46:25 +0000
@@ -70,7 +70,6 @@
 from lp.code.browser.branch import BranchMirrorMixin
 from lp.code.browser.branchmergeproposallisting import (
     ActiveReviewsView,
-    PersonActiveReviewsView,
     PersonProductActiveReviewsView,
     )
 from lp.code.browser.branchmergequeuelisting import HasMergeQueuesMenuMixin
@@ -114,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,
@@ -158,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):
@@ -557,9 +555,7 @@
 
     @property
     def initial_values(self):
-        return {
-            'lifecycle': BranchLifecycleStatusFilter.CURRENT,
-            }
+        return {'lifecycle': BranchLifecycleStatusFilter.CURRENT}
 
     @cachedproperty
     def selected_lifecycle_status(self):
@@ -853,17 +849,13 @@
     usedfor = IPerson
     facet = 'branches'
     links = ['registered', 'owned', 'subscribed',
-             'active_reviews', 'mergequeues', 'source_package_recipes',
-             'simplified_subscribed', 'simplified_registered',
-             'simplified_owned', 'simplified_active_reviews']
+             'active_reviews', 'mergequeues', 'source_package_recipes']
     extra_attributes = [
         'active_review_count',
         'owned_branch_count',
         'registered_branch_count',
-        'show_summary',
         'subscribed_branch_count',
         'mergequeue_count',
-        'simplified_branches_menu',
         ]
 
     def _getCountCollection(self):
@@ -887,121 +879,36 @@
         """
         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 (
+    @cachedproperty
+    def owned(self):
+        return Link(
+            canonical_url(self.context, rootsite='code'), 'Owned branches')
+
+    def registered(self):
+        enabled = (
+            not self.person.is_team and
             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')
+        return Link(
+            '+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(
-            '+recipes',
-            'Source package recipes',
+            '+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'))
-
-    def registered(self):
-        person_is_individual = (not self.person.is_team)
-        return Link(
-            '+registeredbranches',
-            get_plural_text(
-                self.registered_branch_count,
-                'registered branch', 'registered branches'),
-            enabled=person_is_individual)
-
-    def subscribed(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)
-
 
 class PersonProductBranchesMenu(PersonBranchesMenu):
 
     usedfor = IPersonProduct
     links = ['registered', 'owned', 'subscribed', 'active_reviews',
-             'source_package_recipes',
-             'simplified_subscribed', 'simplified_registered',
-             'simplified_owned', 'simplified_active_reviews']
+             'source_package_recipes']
 
     def _getCountCollection(self):
         """See `PersonBranchesMenu`."""

=== 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-03 03:46:25 +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):

=== 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-03 03:46:25 +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-03 03:46:25 +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.',