launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #12827
[Merge] lp:~stevenk/launchpad/skip-private-dev-focus into lp:launchpad
Steve Kowalik has proposed merging lp:~stevenk/launchpad/skip-private-dev-focus into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #947195 in Launchpad itself: "LocationError: (None, 'branch_type') at +code-index"
https://bugs.launchpad.net/launchpad/+bug/947195
For more details, see:
https://code.launchpad.net/~stevenk/launchpad/skip-private-dev-focus/+merge/127622
Currently, Product:+code-index assumes that the development focus is visible if it's an imported branch. This can be false, as proved by the linked OOPS. I've cleaned up the template in question and written a property in the method that will deal with the scenario.
To force this branch back into net-negative, I've removed the code.simplified_branch_menu.enabled feature flag, and done some whitespace cleanups.
--
https://code.launchpad.net/~stevenk/launchpad/skip-private-dev-focus/+merge/127622
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/skip-private-dev-focus 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 02:00:30 +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).
"""Base class view for branch listings."""
@@ -61,7 +61,10 @@
LaunchpadFormView,
)
from lp.app.browser.tales import MenuAPI
-from lp.app.enums import PRIVATE_INFORMATION_TYPES
+from lp.app.enums import (
+ PRIVATE_INFORMATION_TYPES,
+ ServiceUsage,
+ )
from lp.app.widgets.itemswidgets import LaunchpadDropdownWidget
from lp.blueprints.interfaces.specificationbranch import (
ISpecificationBranchSet,
@@ -70,7 +73,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 +116,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 +159,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 +558,7 @@
@property
def initial_values(self):
- return {
- 'lifecycle': BranchLifecycleStatusFilter.CURRENT,
- }
+ return {'lifecycle': BranchLifecycleStatusFilter.CURRENT}
@cachedproperty
def selected_lifecycle_status(self):
@@ -853,17 +852,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 +882,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`."""
@@ -1406,6 +1316,12 @@
set_branch.text = 'Configure code hosting'
return set_branch
+ @property
+ def external_visible(self):
+ return (
+ self.context.codehosting_usage == ServiceUsage.EXTERNAL
+ and self.branch)
+
class ProductBranchesView(ProductBranchListingView):
"""View for branch listing for a product."""
=== 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 02:00:30 +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/browser/tests/test_product.py'
--- lib/lp/code/browser/tests/test_product.py 2012-09-19 01:19:35 +0000
+++ lib/lp/code/browser/tests/test_product.py 2012-10-03 02:00:30 +0000
@@ -31,6 +31,7 @@
login,
login_person,
logout,
+ person_logged_in,
TestCaseWithFactory,
time_counter,
)
@@ -196,6 +197,20 @@
content = view()
self.assertIn('bzr push lp:~', content)
+ def test_product_code_index_with_private_imported_branch(self):
+ # Product:+code-index will not crash if the devfoocs is an imported
+ # branch.
+ product, branch = self.makeProductAndDevelopmentFocusBranch(
+ information_type=InformationType.USERDATA,
+ branch_type=BranchType.IMPORTED)
+ user = self.factory.makePerson()
+ with person_logged_in(user):
+ view = create_initialized_view(
+ product, '+code-index', rootsite='code', principal=user)
+ html = view()
+ expected = 'There are no branches for %s' % product.displayname
+ self.assertIn(expected, html)
+
class TestProductCodeIndexServiceUsages(ProductTestBase, BrowserTestCase):
"""Tests for the product code page, especially the usage messasges."""
@@ -247,10 +262,8 @@
# A remote branch says code is hosted externally, and displays
# upstream data.
product, branch = self.makeProductAndDevelopmentFocusBranch(
- branch_type=BranchType.REMOTE,
- url="http://example.com/mybranch";)
- self.assertEqual(ServiceUsage.EXTERNAL,
- product.codehosting_usage)
+ branch_type=BranchType.REMOTE, url="http://example.com/mybranch";)
+ self.assertEqual(ServiceUsage.EXTERNAL, product.codehosting_usage)
browser = self.getUserBrowser(canonical_url(product, rootsite='code'))
login(ANONYMOUS)
content = find_tag_by_id(browser.contents, 'external')
@@ -305,10 +318,8 @@
# Mirrors show the correct upstream url as the mirror location.
url = "http://example.com/mybranch";
product, branch = self.makeProductAndDevelopmentFocusBranch(
- branch_type=BranchType.MIRRORED,
- url=url)
- view = create_initialized_view(product, '+code-index',
- rootsite='code')
+ branch_type=BranchType.MIRRORED, url=url)
+ view = create_initialized_view(product, '+code-index', rootsite='code')
self.assertEqual(url, view.mirror_location)
@@ -343,8 +354,7 @@
# If the BranchUsage is EXTERNAL then the portlets are shown.
url = "http://example.com/mybranch";
product, branch = self.makeProductAndDevelopmentFocusBranch(
- branch_type=BranchType.MIRRORED,
- url=url)
+ branch_type=BranchType.MIRRORED, url=url)
browser = self.getUserBrowser(canonical_url(product, rootsite='code'))
contents = browser.contents
self.assertIsNot(None, find_tag_by_id(contents, 'branch-portlet'))
@@ -397,4 +407,4 @@
product = self.factory.makeProduct()
login_person(product.owner)
view = create_view(product, '+branches', layer=CodeLayer)
- self.assertEqual(True, view.can_configure_branches())
+ self.assertTrue(view.can_configure_branches())
=== 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 02:00:30 +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/code/templates/product-branch-summary.pt'
--- lib/lp/code/templates/product-branch-summary.pt 2012-07-12 13:15:10 +0000
+++ lib/lp/code/templates/product-branch-summary.pt 2012-10-03 02:00:30 +0000
@@ -19,8 +19,7 @@
</div>
<div id="external"
- tal:condition="context/codehosting_usage/enumvalue:EXTERNAL"
- tal:define="branch view/branch">
+ tal:condition="view/external_visible" tal:define="branch view/branch">
<strong>
<p tal:condition="not: branch/branch_type/enumvalue:IMPORTED">
<tal:project_title replace="context/title" /> hosts its code at
@@ -43,16 +42,16 @@
You can learn more at the project's
<a tal:attributes="href context/homepageurl">web page</a>.
</p>
- <p tal:condition="view/branch/branch_type/enumvalue:MIRRORED">
+ <p tal:condition="branch/branch_type/enumvalue:MIRRORED">
Launchpad has a mirror of the master branch and you can create branches
from it.
</p>
- <p tal:condition="view/branch/branch_type/enumvalue:IMPORTED">
+ <p tal:condition="branch/branch_type/enumvalue:IMPORTED">
Launchpad imports the master branch and you can create branches from
it.
</p>
- <p tal:condition="view/branch/branch_type/enumvalue:REMOTE">
+ <p tal:condition="branch/branch_type/enumvalue:REMOTE">
Launchpad does not have a copy of the remote branch.
</p>
</div>
=== 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 02:00:30 +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.',
